feat(controlplane): move ControlPlane image validation to CRD CEL rule (

#984) * feat(controlplane): move ControlPlane image validation to CRD CEL rule * tests: add CRD validation tests * chore: use kcfg from main
Kong · Jan 13, 2025 · 5cd4d5d · 5cd4d5d
1 parent d36d691
commit 5cd4d5d
Show file tree

Hide file tree

Showing 11 changed files with 107 additions and 69 deletions.
diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml
@@ -174,6 +174,22 @@ jobs:
       with:
         name: tests-report-unit-tests
         path: unit-tests.xml
+
+  CRDs-validation:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+
+    - uses: actions/setup-go@v5
+      with:
+        go-version-file: go.mod
+
+    - uses: jdx/mise-action@5083fe46898c414b2475087cc79da59e7da859e8 # v2.1.11
+      with:
+        install: false
+
+    - name: Run the crds validation tests
+      run: make test.crds-validation
 
   envtest-tests:
     runs-on: ubuntu-latest
@@ -505,6 +521,7 @@ jobs:
       - build
       - unit-tests
       - envtest-tests
+      - CRDs-validation
       - samples
       - conformance-tests
       - integration-tests

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -44,6 +44,8 @@
 - Removed support for the migration of legacy labels so upgrading the operator from 1.3 (or older) to 1.5.0,
   should be done through 1.4.1
   [#976](https://github.com/Kong/gateway-operator/pull/976)
+- Move `ControlPlane` `image` validation to CRD CEL rules.
+  [#984](https://github.com/Kong/gateway-operator/pull/984)
 
 ### Fixes
 

diff --git a/Makefile b/Makefile
@@ -388,6 +388,14 @@ test.envtest:
 test.envtest.pretty:
 	$(MAKE) _test.envtest GOTESTSUM_FORMAT=testname
 
+.PHONY: test.crds-validation
+test.crds-validation:
+	$(MAKE) _test.envtest GOTESTSUM_FORMAT=standard-verbose ENVTEST_TEST_PATHS=./test/crdsvalidation/...
+
+.PHONY: test.crds-validation.pretty
+test.crds-validation.pretty:
+	$(MAKE) _test.envtest GOTESTSUM_FORMAT=testname ENVTEST_TEST_PATHS=./test/crdsvalidation/...
+
 .PHONY: _test.integration
 _test.integration: webhook-certs-dir gotestsum
 	GOFLAGS=$(GOFLAGS) \

diff --git a/api/v1beta1/controlplane_types.go b/api/v1beta1/controlplane_types.go
@@ -56,6 +56,8 @@ type ControlPlaneList struct {
 	Items           []ControlPlane `json:"items"`
 }
 
+// +kubebuilder:validation:XValidation:message="ControlPlane requires an image to be set on controller container",rule="has(self.deployment.podTemplateSpec) && has(self.deployment.podTemplateSpec.spec.containers) && self.deployment.podTemplateSpec.spec.containers.exists(c, c.name == 'controller' && has(c.image))"
+
 // ControlPlaneSpec defines the desired state of ControlPlane
 // +apireference:kgo:include
 type ControlPlaneSpec struct {

diff --git a/config/crd/bases/gateway-operator.konghq.com_controlplanes.yaml b/config/crd/bases/gateway-operator.konghq.com_controlplanes.yaml
@@ -8256,6 +8256,11 @@ spec:
                   If omitted, Ingress resources will not be supported by the ControlPlane.
                 type: string
             type: object
+            x-kubernetes-validations:
+            - message: ControlPlane requires an image to be set on controller container
+              rule: has(self.deployment.podTemplateSpec) && has(self.deployment.podTemplateSpec.spec.containers)
+                && self.deployment.podTemplateSpec.spec.containers.exists(c, c.name
+                == 'controller' && has(c.image))
           status:
             description: ControlPlaneStatus defines the observed state of ControlPlane
             properties:

diff --git a/go.mod b/go.mod
@@ -15,7 +15,7 @@ require (
 	github.com/go-logr/logr v1.4.2
 	github.com/google/go-containerregistry v0.20.2
 	github.com/google/uuid v1.6.0
-	github.com/kong/kubernetes-configuration v1.0.4
+	github.com/kong/kubernetes-configuration v1.0.6-0.20250113094037-f1f2a6542010
 	github.com/kong/kubernetes-telemetry v0.1.7
 	github.com/kong/kubernetes-testing-framework v0.47.2
 	github.com/kong/semver/v4 v4.0.1
@@ -222,7 +222,7 @@ require (
 	github.com/tidwall/pretty v1.2.1
 	go.uber.org/multierr v1.11.0 // indirect
 	go.uber.org/zap v1.27.0
-	golang.org/x/net v0.33.0 // indirect
+	golang.org/x/net v0.33.0
 	golang.org/x/oauth2 v0.24.0 // indirect
 	golang.org/x/sys v0.28.0 // indirect
 	golang.org/x/term v0.27.0 // indirect

diff --git a/go.sum b/go.sum
@@ -299,8 +299,8 @@ github.com/klauspost/compress v1.17.9 h1:6KIumPrER1LHsvBVuDa0r5xaG0Es51mhhB9BQB2
 github.com/klauspost/compress v1.17.9/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw=
 github.com/kong/go-kong v0.62.0 h1:Mu0ANZ0xgLQrM1hLoOAvfF+8qsbIVLfixzESXtZRFMI=
 github.com/kong/go-kong v0.62.0/go.mod h1:ma9GWnhkxtrXZlLFfED955HjVzmUojYEHet3lm+PDik=
-github.com/kong/kubernetes-configuration v1.0.4 h1:GHSVzrBb07a7hTcuPjns2lJfDfzGLw2pWu0QGrB8u+g=
-github.com/kong/kubernetes-configuration v1.0.4/go.mod h1:SGdKGe2IEohf7cXp2NhDgbn64mw6VvOgogQprq6eGmM=
+github.com/kong/kubernetes-configuration v1.0.6-0.20250113094037-f1f2a6542010 h1:qN9xDc/y2RIu9s1X1SZe5kypvI1gnrxDc01Qj4GrgeA=
+github.com/kong/kubernetes-configuration v1.0.6-0.20250113094037-f1f2a6542010/go.mod h1:lN4oTvW3S8zFh4oYTFUMj6rXwOV1gz1fwEXlIjtwppA=
 github.com/kong/kubernetes-telemetry v0.1.7 h1:R4NUpvbF5uZ+5kgSQsIcf/oulRBGQCHsffFRDE4wxV4=
 github.com/kong/kubernetes-telemetry v0.1.7/go.mod h1:USy5pcD1+Mm9NtKuz3Pb/rSx71VN76gHCFhdbAB4/lg=
 github.com/kong/kubernetes-testing-framework v0.47.2 h1:+2Z9anTpbV/hwNeN+NFQz53BMU+g3QJydkweBp3tULo=

diff --git a/internal/validation/controlplane/validation.go b/internal/validation/controlplane/validation.go
@@ -1,13 +1,9 @@
 package controlplane
 
 import (
-	"errors"
-
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	operatorv1beta1 "github.com/kong/gateway-operator/api/v1beta1"
-	"github.com/kong/gateway-operator/pkg/consts"
-	k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes"
 )
 
 // Validator validates ControlPlane objects.
@@ -30,25 +26,5 @@ func (v *Validator) Validate(controlplane *operatorv1beta1.ControlPlane) error {
 
 // ValidateDeploymentOptions validates the DeploymentOptions field of ControlPlane object.
 func (v *Validator) ValidateDeploymentOptions(opts *operatorv1beta1.ControlPlaneDeploymentOptions) error {
-	if opts == nil || opts.PodTemplateSpec == nil {
-		// Can't use empty DeploymentOptions because we still require users
-		// to provide an image
-		// Related: https://github.com/Kong/gateway-operator-archive/issues/754.
-		return errors.New("ControlPlane requires an image")
-	}
-
-	container := k8sutils.GetPodContainerByName(&opts.PodTemplateSpec.Spec, consts.ControlPlaneControllerContainerName)
-	if container == nil {
-		// We need the controller container for e.g. specifying an image which
-		// is still required.
-		// Ref: https://github.com/Kong/gateway-operator-archive/issues/754.
-		return errors.New("no controller container found in ControlPlane spec")
-	}
-
-	// Ref: https://github.com/Kong/gateway-operator-archive/issues/754.
-	if container.Image == "" {
-		return errors.New("ControlPlane requires an image")
-	}
-
 	return nil
 }
diff --git a/internal/validation/controlplane/validation_test.go b/internal/validation/controlplane/validation_test.go
@@ -3,7 +3,6 @@ package controlplane
 import (
 	"testing"
 
-	"github.com/samber/lo"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 
@@ -34,46 +33,6 @@ func TestValidator_ValidateDeploymentOptions(t *testing.T) {
 			},
 			wantErr: false,
 		},
-		{
-			name: "not specifying the image is an error",
-			v:    &Validator{},
-			opts: &operatorv1beta1.ControlPlaneDeploymentOptions{
-				PodTemplateSpec: &corev1.PodTemplateSpec{
-					Spec: corev1.PodSpec{
-						Containers: []corev1.Container{
-							{
-								Name: "controller",
-							},
-						},
-					},
-				},
-			},
-			wantErr: true,
-		},
-		{
-			name:    "not specifying the controller container is an error",
-			v:       &Validator{},
-			opts:    &operatorv1beta1.ControlPlaneDeploymentOptions{},
-			wantErr: true,
-		},
-		{
-			// TODO: https://github.com/Kong/gateway-operator/issues/736
-			name: "using more than 1 replica is an error",
-			v:    &Validator{},
-			opts: &operatorv1beta1.ControlPlaneDeploymentOptions{
-				Replicas: lo.ToPtr(int32(2)),
-				PodTemplateSpec: &corev1.PodTemplateSpec{
-					Spec: corev1.PodSpec{
-						Containers: []corev1.Container{
-							{
-								Name: "controller",
-							},
-						},
-					},
-				},
-			},
-			wantErr: true,
-		},
 		{
 			name: "volumes and volume mounts can be specified on ControlPlane deployment options",
 			v:    &Validator{},

diff --git a/test/crdsvalidation/controlplane_test.go b/test/crdsvalidation/controlplane_test.go
@@ -0,0 +1,63 @@
+package crdsvalidation
+
+import (
+	"testing"
+
+	"github.com/samber/lo"
+	"golang.org/x/net/context"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	operatorv1beta1 "github.com/kong/gateway-operator/api/v1beta1"
+	"github.com/kong/gateway-operator/modules/manager/scheme"
+	"github.com/kong/gateway-operator/test/envtest"
+
+	kcfgcrdsvalidation "github.com/kong/kubernetes-configuration/test/crdsvalidation"
+)
+
+func TestControlPlane(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	cfg, ns := envtest.Setup(t, ctx, scheme.Get())
+
+	t.Run("spec", func(t *testing.T) {
+		kcfgcrdsvalidation.TestCasesGroup[*operatorv1beta1.ControlPlane]{
+			{
+				Name: "not providing image fails",
+				TestObject: &operatorv1beta1.ControlPlane{
+					ObjectMeta: metav1.ObjectMeta{
+						GenerateName: "cp-",
+						Namespace:    ns.Name,
+					},
+					Spec: operatorv1beta1.ControlPlaneSpec{},
+				},
+				ExpectedErrorMessage: lo.ToPtr("ControlPlane requires an image to be set on controller container"),
+			},
+			{
+				Name: "providing image succeeds",
+				TestObject: &operatorv1beta1.ControlPlane{
+					ObjectMeta: metav1.ObjectMeta{
+						GenerateName: "cp-",
+						Namespace:    ns.Name,
+					},
+					Spec: operatorv1beta1.ControlPlaneSpec{
+						ControlPlaneOptions: operatorv1beta1.ControlPlaneOptions{
+							Deployment: operatorv1beta1.ControlPlaneDeploymentOptions{
+								PodTemplateSpec: &corev1.PodTemplateSpec{
+									Spec: corev1.PodSpec{
+										Containers: []corev1.Container{
+											{
+												Name:  "controller",
+												Image: "kong/kubernetes-ingress-controller:3.4.1",
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		}.RunWithConfig(t, cfg, scheme.Get())
+	})
+}
diff --git a/test/envtest/setup.go b/test/envtest/setup.go
@@ -31,6 +31,12 @@ func Setup(t *testing.T, ctx context.Context, scheme *k8sruntime.Scheme) (*rest.
 
 	testEnv := &envtest.Environment{
 		ControlPlaneStopTimeout: time.Second * 60,
+		CRDInstallOptions: envtest.CRDInstallOptions{
+			Paths: []string{
+				// TODO: remove this when CRDs are moved (e.g. to kubernetes-configuration repository)
+				"../../config/crd/bases",
+			},
+		},
 	}
 
 	t.Logf("starting envtest environment for test %s...", t.Name())