Add CEL validation for RegistrySources in Image API

muraee · muraee · commit 79fcfda22e0f · 2025-02-12T18:00:06.000+01:00
diff --git a/config/v1/tests/images.config.openshift.io/AAA_ungated.yaml b/config/v1/tests/images.config.openshift.io/AAA_ungated.yaml
@@ -12,3 +12,105 @@ tests:
         apiVersion: config.openshift.io/v1
         kind: Image
         spec: {}
+  onUpdate:
+    - name: Should allow updating other fields with an invalid persisted registrySources in spec
+      initialCRDPatches:
+        - op: remove
+          path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/registrySources/x-kubernetes-validations
+      initial: |
+        apiVersion: config.openshift.io/v1
+        kind: Image
+        spec:
+          registrySources:
+            blockedRegistries: ["test"]
+            allowedRegistries: ["test"]
+      updated: |
+        apiVersion: config.openshift.io/v1
+        kind: Image
+        spec:
+          # imageStreamImportMode: Legacy
+          externalRegistryHostnames: ["test"]
+          registrySources:
+            blockedRegistries: ["test"]
+            allowedRegistries: ["test"]
+      expected: |
+        apiVersion: config.openshift.io/v1
+        kind: Image
+        spec:
+          # imageStreamImportMode: Legacy
+          externalRegistryHostnames: ["test"]
+          registrySources:
+            blockedRegistries: ["test"]
+            allowedRegistries: ["test"]
+    - name: Should allow removing one of blockedRegistries or allowedRegistries with an invalid persisted registrySources in spec
+      initialCRDPatches:
+        - op: remove
+          path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/registrySources/x-kubernetes-validations
+      initial: |
+        apiVersion: config.openshift.io/v1
+        kind: Image
+        spec:
+          registrySources:
+            blockedRegistries: ["test"]
+            allowedRegistries: ["test"]
+      updated: |
+        apiVersion: config.openshift.io/v1
+        kind: Image
+        spec:
+          registrySources:
+            allowedRegistries: ["test"]
+      expected: |
+        apiVersion: config.openshift.io/v1
+        kind: Image
+        spec:
+          registrySources:
+            allowedRegistries: ["test"]
+    - name: Should not allow adding another slice entry with an invalid persisted registrySources in spec
+      initialCRDPatches:
+        - op: remove
+          path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/registrySources/x-kubernetes-validations
+      initial: |
+        apiVersion: config.openshift.io/v1
+        kind: Image
+        spec:
+          registrySources:
+            blockedRegistries: ["test"]
+            allowedRegistries: ["test"]
+      updated: |
+        apiVersion: config.openshift.io/v1
+        kind: Image
+        spec:
+          registrySources:
+            blockedRegistries: ["test", "test2"]
+            allowedRegistries: ["test"]
+      expectedError: 'Only one of blockedRegistries or allowedRegistries may be set'
+    - name: Should not allow adding blockedRegistries field when a valid registrySources with allowedRegistries is persisted in spec
+      initial: |
+        apiVersion: config.openshift.io/v1
+        kind: Image
+        spec:
+          registrySources:
+            allowedRegistries: ["test"]
+      updated: |
+        apiVersion: config.openshift.io/v1
+        kind: Image
+        spec:
+          registrySources:
+            allowedRegistries: ["test"]
+            blockedRegistries: ["test"]
+      expectedError: 'Only one of blockedRegistries or allowedRegistries may be set'
+    - name: Should not allow adding allowedRegistries field when a valid registrySources with blockedRegistries is persisted in spec
+      initial: |
+        apiVersion: config.openshift.io/v1
+        kind: Image
+        spec:
+          registrySources:
+            blockedRegistries: ["test"]
+      updated: |
+        apiVersion: config.openshift.io/v1
+        kind: Image
+        spec:
+          registrySources:
+            allowedRegistries: ["test"]
+            blockedRegistries: ["test"]
+      expectedError: 'Only one of blockedRegistries or allowedRegistries may be set'
diff --git a/config/v1/types_image.go b/config/v1/types_image.go
@@ -161,6 +161,8 @@ type RegistryLocation struct {
 }
 
 // RegistrySources holds cluster-wide information about how to handle the registries config.
+//
+// +kubebuilder:validation:XValidation:rule="has(self.blockedRegistries) ? !has(self.allowedRegistries) : true",message="Only one of blockedRegistries or allowedRegistries may be set"
 type RegistrySources struct {
 	// insecureRegistries are registries which do not have a valid TLS certificates or only support HTTP connections.
 	// +optional
diff --git a/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_images-CustomNoUpgrade.crd.yaml b/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_images-CustomNoUpgrade.crd.yaml
@@ -164,6 +164,11 @@ spec:
                     type: array
                     x-kubernetes-list-type: atomic
                 type: object
+                x-kubernetes-validations:
+                - message: Only one of blockedRegistries or allowedRegistries may
+                    be set
+                  rule: 'has(self.blockedRegistries) ? !has(self.allowedRegistries)
+                    : true'
             type: object
           status:
             description: status holds observed values from the cluster. They may not
diff --git a/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_images-Default.crd.yaml b/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_images-Default.crd.yaml
@@ -146,6 +146,11 @@ spec:
                     type: array
                     x-kubernetes-list-type: atomic
                 type: object
+                x-kubernetes-validations:
+                - message: Only one of blockedRegistries or allowedRegistries may
+                    be set
+                  rule: 'has(self.blockedRegistries) ? !has(self.allowedRegistries)
+                    : true'
             type: object
           status:
             description: status holds observed values from the cluster. They may not
diff --git a/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_images-DevPreviewNoUpgrade.crd.yaml b/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_images-DevPreviewNoUpgrade.crd.yaml
@@ -164,6 +164,11 @@ spec:
                     type: array
                     x-kubernetes-list-type: atomic
                 type: object
+                x-kubernetes-validations:
+                - message: Only one of blockedRegistries or allowedRegistries may
+                    be set
+                  rule: 'has(self.blockedRegistries) ? !has(self.allowedRegistries)
+                    : true'
             type: object
           status:
             description: status holds observed values from the cluster. They may not
diff --git a/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_images-TechPreviewNoUpgrade.crd.yaml b/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_images-TechPreviewNoUpgrade.crd.yaml
@@ -164,6 +164,11 @@ spec:
                     type: array
                     x-kubernetes-list-type: atomic
                 type: object
+                x-kubernetes-validations:
+                - message: Only one of blockedRegistries or allowedRegistries may
+                    be set
+                  rule: 'has(self.blockedRegistries) ? !has(self.allowedRegistries)
+                    : true'
             type: object
           status:
             description: status holds observed values from the cluster. They may not
diff --git a/config/v1/zz_generated.featuregated-crd-manifests/images.config.openshift.io/AAA_ungated.yaml b/config/v1/zz_generated.featuregated-crd-manifests/images.config.openshift.io/AAA_ungated.yaml
@@ -146,6 +146,11 @@ spec:
                     type: array
                     x-kubernetes-list-type: atomic
                 type: object
+                x-kubernetes-validations:
+                - message: Only one of blockedRegistries or allowedRegistries may
+                    be set
+                  rule: 'has(self.blockedRegistries) ? !has(self.allowedRegistries)
+                    : true'
             type: object
           status:
             description: status holds observed values from the cluster. They may not
diff --git a/config/v1/zz_generated.featuregated-crd-manifests/images.config.openshift.io/ImageStreamImportMode.yaml b/config/v1/zz_generated.featuregated-crd-manifests/images.config.openshift.io/ImageStreamImportMode.yaml
@@ -164,6 +164,11 @@ spec:
                     type: array
                     x-kubernetes-list-type: atomic
                 type: object
+                x-kubernetes-validations:
+                - message: Only one of blockedRegistries or allowedRegistries may
+                    be set
+                  rule: 'has(self.blockedRegistries) ? !has(self.allowedRegistries)
+                    : true'
             type: object
           status:
             description: status holds observed values from the cluster. They may not
diff --git a/payload-manifests/crds/0000_10_config-operator_01_images-CustomNoUpgrade.crd.yaml b/payload-manifests/crds/0000_10_config-operator_01_images-CustomNoUpgrade.crd.yaml
@@ -164,6 +164,11 @@ spec:
                     type: array
                     x-kubernetes-list-type: atomic
                 type: object
+                x-kubernetes-validations:
+                - message: Only one of blockedRegistries or allowedRegistries may
+                    be set
+                  rule: 'has(self.blockedRegistries) ? !has(self.allowedRegistries)
+                    : true'
             type: object
           status:
             description: status holds observed values from the cluster. They may not
diff --git a/payload-manifests/crds/0000_10_config-operator_01_images-Default.crd.yaml b/payload-manifests/crds/0000_10_config-operator_01_images-Default.crd.yaml
@@ -146,6 +146,11 @@ spec:
                     type: array
                     x-kubernetes-list-type: atomic
                 type: object
+                x-kubernetes-validations:
+                - message: Only one of blockedRegistries or allowedRegistries may
+                    be set
+                  rule: 'has(self.blockedRegistries) ? !has(self.allowedRegistries)
+                    : true'
             type: object
           status:
             description: status holds observed values from the cluster. They may not
diff --git a/payload-manifests/crds/0000_10_config-operator_01_images-DevPreviewNoUpgrade.crd.yaml b/payload-manifests/crds/0000_10_config-operator_01_images-DevPreviewNoUpgrade.crd.yaml
@@ -164,6 +164,11 @@ spec:
                     type: array
                     x-kubernetes-list-type: atomic
                 type: object
+                x-kubernetes-validations:
+                - message: Only one of blockedRegistries or allowedRegistries may
+                    be set
+                  rule: 'has(self.blockedRegistries) ? !has(self.allowedRegistries)
+                    : true'
             type: object
           status:
             description: status holds observed values from the cluster. They may not
diff --git a/payload-manifests/crds/0000_10_config-operator_01_images-TechPreviewNoUpgrade.crd.yaml b/payload-manifests/crds/0000_10_config-operator_01_images-TechPreviewNoUpgrade.crd.yaml
@@ -164,6 +164,11 @@ spec:
                     type: array
                     x-kubernetes-list-type: atomic
                 type: object
+                x-kubernetes-validations:
+                - message: Only one of blockedRegistries or allowedRegistries may
+                    be set
+                  rule: 'has(self.blockedRegistries) ? !has(self.allowedRegistries)
+                    : true'
             type: object
           status:
             description: status holds observed values from the cluster. They may not

Original file line number	Diff line number	Diff line change
`@@ -161,6 +161,8 @@ type RegistryLocation struct {`
`161`	`161`	`}`
`162`	`162`
`163`	`163`	`// RegistrySources holds cluster-wide information about how to handle the registries config.`
	`164`	`+//`
	`165`	`+// +kubebuilder:validation:XValidation:rule="has(self.blockedRegistries) ? !has(self.allowedRegistries) : true",message="Only one of blockedRegistries or allowedRegistries may be set"`
`164`	`166`	`type RegistrySources struct {`
`165`	`167`	`// insecureRegistries are registries which do not have a valid TLS certificates or only support HTTP connections.`
`166`	`168`	`// +optional`