:sparkles: Add support for k8s:immutable by alvaroaleman · Pull Request #1354 · kubernetes-sigs/controller-tools

alvaroaleman · 2026-02-22T04:22:36Z

This change adds support for k8s:immutable as described in the KEP.

It replaces #1216 and follows @jpbetz suggestion for the CEL expression. It also adds integration tests.

Closes #1216

k8s-ci-robot · 2026-02-22T04:22:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [alvaroaleman]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

alvaroaleman · 2026-02-22T04:23:02Z

/assign @JoelSpeed @lalitc375

k8s-ci-robot · 2026-02-22T04:23:05Z

@alvaroaleman: GitHub didn't allow me to assign the following users: lalitc375.

Note that only kubernetes-sigs members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

Details

In response to this:

/assign @JoelSpeed @lalitc375

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

JoelSpeed · 2026-02-26T14:36:43Z

pkg/crd/markers/validation.go

+func (m Immutable) ApplyToSchema(schema *apiextensionsv1.JSONSchemaProps) error {
+	optionalOldSelf := true
+	schema.XValidations = append(schema.XValidations, apiextensionsv1.ValidationRule{
+		Rule:            "!oldSelf.hasValue() || self == oldSelf.value()",


Does this achieve the same thing if we use self == oldSelf and drop the OptionalOldSelf?

Yeah, good catch, updated it to do that instead

JoelSpeed · 2026-02-26T14:40:12Z

pkg/crd/markers/validation.go

+// Note that immutable fields that are nested below optional fields can still be
+// updated by unsetting the optional parent field and re-setting it again.


Is there anything we can do to handle the "no clear" element of the KEP?

The usual pattern is to find the nearest required pattern and add a validation to check that if the oldSelf has the immutable field set, that it cannot be unset

This is handled by the code in schema.go that adds a CEL rule to disallow unsetting once set

JoelSpeed · 2026-02-26T14:41:35Z

pkg/crd/testdata/immutable/types.go

+
+	// +k8s:immutable
+	// +optional
+	OptionalString *string `json:"optionalString,omitempty"`


Do we need to test a version of this where string isn't a pointer?

JoelSpeed · 2026-02-26T14:47:45Z

pkg/crd/testdata/immutable/types.go

+
+	// +k8s:immutable
+	// +optional
+	OptionalStruct *NestedStruct `json:"optionalStruct,omitempty"`


A version without a pointer, but with omitzero to test that combination?

Sure but at that point we are mostly testing the go serialization?

Are we actually using these structs in the tests? If not don't worry

I was just thinking we could example all of the combinations CRD authors might use

JoelSpeed · 2026-02-26T14:48:52Z

pkg/crd/testdata/immutable_test.go

+			Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(got), got)).To(Succeed())
+
+			mutate(got)
+			Expect(k8sClient.Update(ctx, got)).NotTo(Succeed())


Can we check the error contains the immutable substring to make sure this is doing what we think it is?

JoelSpeed · 2026-02-26T14:50:08Z

pkg/crd/testdata/immutable_test.go

+			clear(got)
+			Expect(k8sClient.Update(ctx, got)).NotTo(Succeed())


I thought the doc said we weren't expecting clear to be rejected?

The docs said we do expect clear to be rejected: https://github.com/kubernetes/enhancements/blob/c68dfb941894fc8859a951fe47a60b2161300b88/keps/sig-api-machinery/5073-declarative-validation-with-validation-gen/README.md#k8simmutable

Or what do you mean?

JoelSpeed · 2026-02-26T14:51:29Z

pkg/crd/schema.go

+	// For optional immutable fields, add a parent-level validation rule to prevent
+	// clearing the field once set. The field-level rule prevents value changes, but
+	// when an optional field is removed, the field-level rule doesn't execute.


If this is an optional parent, nothing stops them from removing the optional parent, you need to find the nearest required parent and add it there

No, the kep explicitly says that its valid to remove an optional parent and that that starts a new cycle: https://github.com/kubernetes/enhancements/blob/c68dfb941894fc8859a951fe47a60b2161300b88/keps/sig-api-machinery/5073-declarative-validation-with-validation-gen/README.md#L1143

Fair, I had missed that part of the EP

This surprised me initially as well which is why I left the comment on the marker. For the purpose of implementing the markers, I would just go by the assumption that the relevant discussions if that does or doesn't make sense were already had when the KEP was written/approved

JoelSpeed · 2026-02-26T15:00:26Z

/hold

Since this has approve, lets avoid an accidental merge with a fly by lgtm

This change adds support for `k8s:immutable` as described in the [KEP][0]. [0]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/5073-declarative-validation-with-validation-gen

JoelSpeed · 2026-03-02T12:47:49Z

/lgtm

@alvaroaleman having re-read the EP, I think this is good to go

Would be good if @lalitc375 could ack the behaviour as well, fields are immutable, and cannot be unset unless their parent is cleared

k8s-ci-robot · 2026-03-02T12:47:56Z

LGTM label has been added.

Details

Git tree hash: 201dc2db4af33e575a36a9b13bbcaf83716bb63a

lalitc375 · 2026-03-05T02:57:40Z

Would be good if @lalitc375 could ack the behaviour as well, fields are immutable, and cannot be unset unless their parent is cleared

I am out of office right now. @aaron-prindle Can you please take a look?

k8s-ci-robot requested review from JoelSpeed and vincepri February 22, 2026 04:22

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 22, 2026

k8s-ci-robot assigned JoelSpeed Feb 22, 2026

alvaroaleman mentioned this pull request Feb 22, 2026

✨ Change CRD generation logic to honor k8s:immutable #1216

Draft

alvaroaleman force-pushed the immutable branch 2 times, most recently from e986453 to c940070 Compare February 22, 2026 15:14

JoelSpeed reviewed Feb 26, 2026

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 26, 2026

alvaroaleman force-pushed the immutable branch from c940070 to dfeab69 Compare February 27, 2026 22:04

✨ Add support for k8s:immutable

5c0f006

This change adds support for `k8s:immutable` as described in the [KEP][0]. [0]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/5073-declarative-validation-with-validation-gen

alvaroaleman force-pushed the immutable branch from dfeab69 to 5c0f006 Compare February 27, 2026 22:08

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 27, 2026

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 2, 2026

		// Note that immutable fields that are nested below optional fields can still be
		// updated by unsetting the optional parent field and re-setting it again.

		clear(got)
		Expect(k8sClient.Update(ctx, got)).NotTo(Succeed())

Conversation

alvaroaleman commented Feb 22, 2026

Uh oh!

k8s-ci-robot commented Feb 22, 2026

Uh oh!

alvaroaleman commented Feb 22, 2026

Uh oh!

k8s-ci-robot commented Feb 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoelSpeed commented Feb 26, 2026

Uh oh!

JoelSpeed commented Mar 2, 2026

Uh oh!

k8s-ci-robot commented Mar 2, 2026

Uh oh!

lalitc375 commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants