[Add] CEL rules for validating App and PackageInstall Spec

This PR:
- Adds KB marker to ensure that either spec.ServiceAccount
  or spec.Cluster is present in App and PackageInstall CR.
- Bumps controller-tools to 0.10.0 to support CEL based
  validation marker. That is the latest version compatible with
  the k8s release the project is currently at.

Signed-off-by: Varsha Prasad Narsing <[email protected]>

Author: varshaprasad96

config/config/crds.yml

@@ -1434,6 +1434,9 @@ spec:
         required:
         - spec
         type: object
+        x-kubernetes-validations:
+        - message: Expected service account or cluster.
+          rule: has(self.spec.serviceAccountName) || has(self.spec.cluster)
     served: true
     storage: true
     subresources:
@@ -1594,6 +1597,9 @@ spec:
         required:
         - spec
         type: object
+        x-kubernetes-validations:
+        - message: Expected service account or cluster.
+          rule: has(self.spec.serviceAccountName) || has(self.spec.cluster)
     served: true
     storage: true
     subresources:

go.mod

@@ -22,7 +22,7 @@ require (
 	k8s.io/kube-aggregator v0.22.17
 	k8s.io/kube-openapi v0.0.0-20220803162953-67bda5d908f1
 	sigs.k8s.io/controller-runtime v0.13.1
-	sigs.k8s.io/controller-tools v0.7.0
+	sigs.k8s.io/controller-tools v0.10.0
 	sigs.k8s.io/yaml v1.3.0
 )
 
@@ -61,7 +61,7 @@ require (
 	github.com/go-openapi/jsonpointer v0.19.5 // indirect
 	github.com/go-openapi/jsonreference v0.19.6 // indirect
 	github.com/go-openapi/swag v0.19.15 // indirect
-	github.com/gobuffalo/flect v0.2.3 // indirect
+	github.com/gobuffalo/flect v0.2.5 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/google/gnostic v0.5.7-v3refs // indirect

go.sum

@@ -14,6 +14,7 @@ import (
 // +kubebuilder:printcolumn:name=Description,JSONPath=.status.friendlyDescription,description=Friendly description,type=string
 // +kubebuilder:printcolumn:name=Since-Deploy,JSONPath=.status.deploy.startedAt,description=Last time app started being deployed. Does not mean anything was changed.,type=date
 // +kubebuilder:printcolumn:name=Age,JSONPath=.metadata.creationTimestamp,description=Time since creation,type=date
+// +kubebuilder:validation:XValidation:rule="has(self.spec.serviceAccountName) || has(self.spec.cluster)", message="Expected service account or cluster."
 // +protobuf=false
 // An App is a set of Kubernetes resources. These resources could span any number of namespaces or could be cluster-wide (e.g. CRDs). An App is represented in kapp-controller using a App CR.
 // The App CR comprises of three main sections:

pkg/apis/kappctrl/v1alpha1/types.go

@@ -17,6 +17,7 @@ import (
 // +kubebuilder:printcolumn:name=Package version,JSONPath=.status.version,description=PackageMetadata version,type=string
 // +kubebuilder:printcolumn:name=Description,JSONPath=.status.friendlyDescription,description=Friendly description,type=string
 // +kubebuilder:printcolumn:name=Age,JSONPath=.metadata.creationTimestamp,description=Time since creation,type=date
+// +kubebuilder:validation:XValidation:rule="has(self.spec.serviceAccountName) || has(self.spec.cluster)", message="Expected service account or cluster."
 // A Package Install is an actual installation of a package and its underlying resources on a Kubernetes cluster.
 // It is represented in kapp-controller by a PackageInstall CR.
 // A PackageInstall CR must reference a Package CR.

pkg/apis/packaging/v1alpha1/package_install.go

@@ -950,3 +950,64 @@ spec:
 		assert.Equal(t, actualSyncPeriod, expectedSyncPeriod)
 	})
 }
+
+
+func TestCELValidation(t *testing.T) {
+	env := e2e.BuildEnv(t)
+	logger := e2e.Logger{}
+	kapp := e2e.Kapp{t, env.Namespace, logger}
+	kubectl := e2e.Kubectl{t, env.Namespace, logger}
+
+	name := "incorrect-spec-without-sa-cluster"
+
+	appYAML := fmt.Sprintf(`
+---
+apiVersion: kappctrl.k14s.io/v1alpha1
+kind: App
+metadata:
+  name: %s
+  annotations:
+    kapp.k14s.io/change-group: kappctrl-e2e.k14s.io/apps
+spec:
+  fetch:
+    - inline:
+        paths:
+          file.yml: |
+            apiVersion: v1
+            kind: ConfigMap
+            metadata:
+              name: configmap
+  template:
+  - ytt: {}
+  deploy:
+    - kapp: {}
+`, name)
+
+	pkginstallYAML := fmt.Sprintf(`
+---
+apiVersion: packaging.carvel.dev/v1alpha1
+kind: PackageInstall
+metadata:
+  name: %[2]s
+  namespace: %[1]s
+  annotations:
+    kapp.k14s.io/change-group: kappctrl-e2e.k14s.io/packageinstalls
+spec:
+  packageRef:
+    refName: pkg.incorrect.carvel.dev
+    versionSelection:
+      constraints: 1.0.0
+`, env.Namespace, name)
+
+	logger.Section("Create App CR with kubectl", func() {
+		_, err := kubectl.RunWithOpts([]string{"apply", "-f", "-"}, e2e.RunOpts{StdinReader: strings.NewReader(appYAML), AllowError: true})
+		require.Error(t, err)
+		require.ErrorContains(t, err, "Expected service account or cluster.")
+	})
+
+	logger.Section("Create PackageInstall with kapp", func() {
+		_, err := kapp.RunWithOpts([]string{"deploy", "-a", name, "-f", "-"}, e2e.RunOpts{StdinReader: strings.NewReader(pkginstallYAML), AllowError: true})
+		require.Error(t, err)
+		require.ErrorContains(t, err, "Expected service account or cluster.")
+	})
+}

test/e2e/kappcontroller/packageinstall_test.go

@@ -79,7 +79,7 @@ github.com/go-openapi/jsonreference
 # github.com/go-openapi/swag v0.19.15
 ## explicit; go 1.11
 github.com/go-openapi/swag
-# github.com/gobuffalo/flect v0.2.3
+# github.com/gobuffalo/flect v0.2.5
 ## explicit; go 1.13
 github.com/gobuffalo/flect
 # github.com/gogo/protobuf v1.3.2
@@ -1248,8 +1248,8 @@ sigs.k8s.io/controller-runtime/pkg/source/internal
 sigs.k8s.io/controller-runtime/pkg/webhook
 sigs.k8s.io/controller-runtime/pkg/webhook/admission
 sigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics
-# sigs.k8s.io/controller-tools v0.7.0
-## explicit; go 1.16
+# sigs.k8s.io/controller-tools v0.10.0
+## explicit; go 1.19
 sigs.k8s.io/controller-tools/cmd/controller-gen
 sigs.k8s.io/controller-tools/pkg/crd
 sigs.k8s.io/controller-tools/pkg/crd/markers