Enforced immutable fields using CEL rules (crossplane#5682)

Enforce immutable fields using CEL rules Signed-off-by: Neeraj Nagure <[email protected]>
negz · May 29, 2024 · ed0fb98 · ed0fb98
1 parent 2937cd8
commit ed0fb98
Show file tree

Hide file tree

Showing 11 changed files with 114 additions and 9 deletions.
diff --git a/apis/apiextensions/v1/composition_revision_types.go b/apis/apiextensions/v1/composition_revision_types.go
@@ -38,7 +38,7 @@ const (
 type CompositionRevisionSpec struct {
 	// CompositeTypeRef specifies the type of composite resource that this
 	// composition is compatible with.
-	// +immutable
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	CompositeTypeRef TypeReference `json:"compositeTypeRef"`
 
 	// Mode controls what type or "mode" of Composition will be used.
@@ -118,7 +118,7 @@ type CompositionRevisionSpec struct {
 	PublishConnectionDetailsWithStoreConfigRef *StoreConfigReference `json:"publishConnectionDetailsWithStoreConfigRef,omitempty"`
 
 	// Revision number. Newer revisions have larger numbers.
-	// +immutable
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	Revision int64 `json:"revision"`
 }
 

diff --git a/apis/apiextensions/v1/composition_types.go b/apis/apiextensions/v1/composition_types.go
@@ -24,7 +24,7 @@ import (
 type CompositionSpec struct {
 	// CompositeTypeRef specifies the type of composite resource that this
 	// composition is compatible with.
-	// +immutable
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	CompositeTypeRef TypeReference `json:"compositeTypeRef"`
 
 	// Mode controls what type or "mode" of Composition will be used.

diff --git a/apis/apiextensions/v1/xrd_types.go b/apis/apiextensions/v1/xrd_types.go
@@ -30,12 +30,12 @@ type CompositeResourceDefinitionSpec struct {
 	// Group specifies the API group of the defined composite resource.
 	// Composite resources are served under `/apis/<group>/...`. Must match the
 	// name of the XRD (in the form `<names.plural>.<group>`).
-	// +immutable
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	Group string `json:"group"`
 
 	// Names specifies the resource and kind names of the defined composite
 	// resource.
-	// +immutable
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	Names extv1.CustomResourceDefinitionNames `json:"names"`
 
 	// ClaimNames specifies the names of an optional composite resource claim.
@@ -46,8 +46,8 @@ type CompositeResourceDefinitionSpec struct {
 	// create, update, or delete a corresponding composite resource. You may add
 	// claim names to an existing CompositeResourceDefinition, but they cannot
 	// be changed or removed once they have been set.
-	// +immutable
 	// +optional
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	ClaimNames *extv1.CustomResourceDefinitionNames `json:"claimNames,omitempty"`
 
 	// ConnectionSecretKeys is the list of keys that will be exposed to the end
@@ -70,7 +70,7 @@ type CompositeResourceDefinitionSpec struct {
 	// EnforcedCompositionRef refers to the Composition resource that will be used
 	// by all composite instances whose schema is defined by this definition.
 	// +optional
-	// +immutable
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	EnforcedCompositionRef *CompositionReference `json:"enforcedCompositionRef,omitempty"`
 
 	// DefaultCompositionUpdatePolicy is the policy used when updating composites after a new

diff --git a/apis/apiextensions/v1beta1/zz_generated.composition_revision_types.go b/apis/apiextensions/v1beta1/zz_generated.composition_revision_types.go
@@ -40,7 +40,7 @@ const (
 type CompositionRevisionSpec struct {
 	// CompositeTypeRef specifies the type of composite resource that this
 	// composition is compatible with.
-	// +immutable
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	CompositeTypeRef TypeReference `json:"compositeTypeRef"`
 
 	// Mode controls what type or "mode" of Composition will be used.
@@ -120,7 +120,7 @@ type CompositionRevisionSpec struct {
 	PublishConnectionDetailsWithStoreConfigRef *StoreConfigReference `json:"publishConnectionDetailsWithStoreConfigRef,omitempty"`
 
 	// Revision number. Newer revisions have larger numbers.
-	// +immutable
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	Revision int64 `json:"revision"`
 }
 

diff --git a/cluster/crds/apiextensions.crossplane.io_compositeresourcedefinitions.yaml b/cluster/crds/apiextensions.crossplane.io_compositeresourcedefinitions.yaml
@@ -114,6 +114,9 @@ spec:
                 - kind
                 - plural
                 type: object
+                x-kubernetes-validations:
+                - message: Value is immutable
+                  rule: self == oldSelf
               connectionSecretKeys:
                 description: |-
                   ConnectionSecretKeys is the list of keys that will be exposed to the end
@@ -272,12 +275,18 @@ spec:
                 required:
                 - name
                 type: object
+                x-kubernetes-validations:
+                - message: Value is immutable
+                  rule: self == oldSelf
               group:
                 description: |-
                   Group specifies the API group of the defined composite resource.
                   Composite resources are served under `/apis/<group>/...`. Must match the
                   name of the XRD (in the form `<names.plural>.<group>`).
                 type: string
+                x-kubernetes-validations:
+                - message: Value is immutable
+                  rule: self == oldSelf
               metadata:
                 description: Metadata specifies the desired metadata for the defined
                   composite resource and claim CRD's.
@@ -350,6 +359,9 @@ spec:
                 - kind
                 - plural
                 type: object
+                x-kubernetes-validations:
+                - message: Value is immutable
+                  rule: self == oldSelf
               versions:
                 description: |-
                   Versions is the list of all API versions of the defined composite

diff --git a/cluster/crds/apiextensions.crossplane.io_compositionrevisions.yaml b/cluster/crds/apiextensions.crossplane.io_compositionrevisions.yaml
@@ -79,6 +79,9 @@ spec:
                 - apiVersion
                 - kind
                 type: object
+                x-kubernetes-validations:
+                - message: Value is immutable
+                  rule: self == oldSelf
               environment:
                 description: |-
                   Environment configures the environment in which resources are rendered.
@@ -1579,6 +1582,9 @@ spec:
                 description: Revision number. Newer revisions have larger numbers.
                 format: int64
                 type: integer
+                x-kubernetes-validations:
+                - message: Value is immutable
+                  rule: self == oldSelf
               writeConnectionSecretsToNamespace:
                 description: |-
                   WriteConnectionSecretsToNamespace specifies the namespace in which the
@@ -1712,6 +1718,9 @@ spec:
                 - apiVersion
                 - kind
                 type: object
+                x-kubernetes-validations:
+                - message: Value is immutable
+                  rule: self == oldSelf
               environment:
                 description: |-
                   Environment configures the environment in which resources are rendered.
@@ -3212,6 +3221,9 @@ spec:
                 description: Revision number. Newer revisions have larger numbers.
                 format: int64
                 type: integer
+                x-kubernetes-validations:
+                - message: Value is immutable
+                  rule: self == oldSelf
               writeConnectionSecretsToNamespace:
                 description: |-
                   WriteConnectionSecretsToNamespace specifies the namespace in which the

diff --git a/cluster/crds/apiextensions.crossplane.io_compositions.yaml b/cluster/crds/apiextensions.crossplane.io_compositions.yaml
@@ -74,6 +74,9 @@ spec:
                 - apiVersion
                 - kind
                 type: object
+                x-kubernetes-validations:
+                - message: Value is immutable
+                  rule: self == oldSelf
               environment:
                 description: |-
                   Environment configures the environment in which resources are rendered.

diff --git a/test/e2e/comp_schema_validation_test.go b/test/e2e/comp_schema_validation_test.go
@@ -47,6 +47,11 @@ func TestCompositionValidation(t *testing.T) {
 				funcs.ResourcesCreatedWithin(30*time.Second, manifests, "composition-warn-valid.yaml"),
 			),
 		},
+		{
+			// A composition that updates immutable fields should be rejected when validated in strict mode.
+			Name:       "ImmutableCompositionFieldUpdateIsRejectedStrictMode",
+			Assessment: funcs.ResourcesFailToApply(FieldManager, manifests, "composition-invalid-immutable.yaml"),
+		},
 	}
 	environment.Test(t,
 		cases.Build(t.Name()).

diff --git a/test/e2e/manifests/apiextensions/composition/validation/composition-invalid-immutable.yaml b/test/e2e/manifests/apiextensions/composition/validation/composition-invalid-immutable.yaml
@@ -0,0 +1,36 @@
+apiVersion: apiextensions.crossplane.io/v1
+kind: Composition
+metadata:
+  name: valid
+  annotations:
+    crossplane.io/composition-schema-aware-validation-mode: strict
+spec:
+  compositeTypeRef:
+    apiVersion: nop.example.org/v1alpha1
+    kind: NopResource   # <-- invalid, field is immutable
+  resources:
+  - name: nop-resource-1
+    base:
+     apiVersion: nop.crossplane.io/v1alpha1
+     kind: NopResource
+     spec:
+      forProvider:
+        conditionAfter:
+        - conditionType: Ready
+          conditionStatus: "False"
+          time: 0s
+        - conditionType: Ready
+          conditionStatus: "True"
+          time: 1s
+    patches:
+    - type: FromCompositeFieldPath
+      fromFieldPath: spec.coolField
+      toFieldPath: metadata.annotations[cool-field]
+      transforms:
+      - type: string
+        string:
+         type: Convert
+         convert: ToUpper
+    - type: ToCompositeFieldPath
+      fromFieldPath: metadata.annotations[cool-field]
+      toFieldPath: status.coolerField
diff --git a/test/e2e/manifests/apiextensions/xrd/validation/xrd-immutable-updated.yaml b/test/e2e/manifests/apiextensions/xrd/validation/xrd-immutable-updated.yaml
@@ -0,0 +1,31 @@
+apiVersion: apiextensions.crossplane.io/v1
+kind: CompositeResourceDefinition
+metadata:
+  name: xnopresources.nop.example.org
+spec:
+  group: nope.example.org # <-- invalid, field is immutable
+  names:
+    kind: XNopResource
+    plural: xnopresources
+  claimNames:
+    kind: NopResource
+    plural: nopresources
+  connectionSecretKeys:
+  - test
+  versions:
+  - name: v1alpha1
+    served: true
+    referenceable: true
+    schema:
+     openAPIV3Schema:
+       type: object
+       properties:
+        spec:
+          type: object
+          properties:
+            coolField:
+              type: string
+            coolerField:
+              type: string
+          required:
+          - coolField
diff --git a/test/e2e/xrd_validation_test.go b/test/e2e/xrd_validation_test.go
@@ -41,6 +41,12 @@ func TestXRDValidation(t *testing.T) {
 			Description: "An invalid update to an XRD should be rejected.",
 			Assessment:  funcs.ResourcesFailToApply(FieldManager, manifests, "xrd-valid-updated-invalid.yaml"),
 		},
+		{
+			// An update to immutable XRD fields should be rejected.
+			Name:        "ImmutableXRDFieldUpdateIsRejected",
+			Description: "An update to immutable XRD field should be rejected.",
+			Assessment:  funcs.ResourcesFailToApply(FieldManager, manifests, "xrd-immutable-updated.yaml"),
+		},
 		{
 			// An invalid XRD should be rejected.
 			Name:        "InvalidXRDIsRejected",