Validate CRD Spec fields (#83)

* Validate CRD Spec fields

This configures the CRD to require that we have either a CAPI Cluster
ref, or a Secret ref.

GitopsClusters will be rejected if they have neither, or have both.

Co-authored-by: Yiannis Triantafyllopoulos <[email protected]>

api/v1alpha1/gitopscluster_types.go

@@ -28,6 +28,8 @@ import (
 const GitOpsClusterNoSecretFinalizerAnnotation = "clusters.gitops.weave.works/no-secret-finalizer"
 
 // GitopsClusterSpec defines the desired state of GitopsCluster
+// +kubebuilder:validation:XValidation:rule="(has(self.secretRef) || has(self.capiClusterRef))",message="must provide a secretRef or capiClusterRef"
+// +kubebuilder:validation:XValidation:rule="!(has(self.secretRef) && has(self.capiClusterRef))",message="cannot provide both capiClusterRef and secretRef"
 type GitopsClusterSpec struct {
 	// SecretRef specifies the Secret containing the kubeconfig for a cluster.
 	// +optional
@@ -62,6 +64,7 @@ func (in *GitopsCluster) SetConditions(conditions []metav1.Condition) {
 // +kubebuilder:printcolumn:name="ClusterConnectivity",type="string",JSONPath=".status.conditions[?(@.type==\"ClusterConnectivity\")].status",description=""
 
 // GitopsCluster is the Schema for the gitopsclusters API
+// +kubebuilder:validation:XValidation:rule="has(self.spec)",message="must confgure spec"
 type GitopsCluster struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`

config/crd/bases/gitops.weave.works_gitopsclusters.yaml

@@ -67,6 +67,11 @@ spec:
                 - name
                 type: object
             type: object
+            x-kubernetes-validations:
+            - message: must provide a secretRef or capiClusterRef
+              rule: (has(self.secretRef) || has(self.capiClusterRef))
+            - message: cannot provide both capiClusterRef and secretRef
+              rule: '!(has(self.secretRef) && has(self.capiClusterRef))'
           status:
             description: GitopsClusterStatus defines the observed state of GitopsCluster
             properties:
@@ -141,6 +146,9 @@ spec:
                 type: array
             type: object
         type: object
+        x-kubernetes-validations:
+        - message: must configure spec
+          rule: has(self.spec)
     served: true
     storage: true
     subresources:

controllers/gitopscluster_controller_test.go

@@ -2,6 +2,7 @@ package controllers_test
 
 import (
 	"context"
+	"path/filepath"
 	"regexp"
 	"testing"
 	"time"
@@ -22,6 +23,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
@@ -514,6 +516,86 @@ func TestFinalizers(t *testing.T) {
 	}
 }
 
+func TestGitopsClusterValidation(t *testing.T) {
+	testEnv := &envtest.Environment{
+		CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")},
+	}
+	testCfg, err := testEnv.Start()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		if err := testEnv.Stop(); err != nil {
+			t.Fatalf("failed to shutdown testEnv: %s", err)
+		}
+	}()
+
+	s := runtime.NewScheme()
+	if err := gitopsv1alpha1.AddToScheme(s); err != nil {
+		t.Fatal(err)
+	}
+
+	cl, err := client.New(testCfg, client.Options{Scheme: s})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	t.Run("when neither the secret nor capi cluster are configured", func(t *testing.T) {
+		testCluster := makeTestCluster(func(c *gitopsv1alpha1.GitopsCluster) {
+			c.ObjectMeta.Name = "no-config"
+			c.ObjectMeta.Namespace = "default"
+			c.Spec.SecretRef = nil
+			c.Spec.CAPIClusterRef = nil
+		})
+
+		err := cl.Create(context.TODO(), testCluster)
+		assertErrorMatch(t, "must provide a secretRef or capiClusterRef", err)
+		assertErrorDoesNotMatch(t, "cannot provide both capiClusterRef and secretRef", err)
+	})
+
+	t.Run("when both the secret and capi cluster are configured", func(t *testing.T) {
+		testCluster := makeTestCluster(func(c *gitopsv1alpha1.GitopsCluster) {
+			c.ObjectMeta.Name = "both-configs"
+			c.ObjectMeta.Namespace = "default"
+			c.Spec.SecretRef = &meta.LocalObjectReference{
+				Name: "test-secret",
+			}
+			c.Spec.CAPIClusterRef = &meta.LocalObjectReference{
+				Name: "test-cluster",
+			}
+		})
+
+		err := cl.Create(context.TODO(), testCluster)
+		assertErrorMatch(t, "cannot provide both capiClusterRef and secretRef", err)
+		assertErrorDoesNotMatch(t, "must provide a secretRef or capiClusterRef", err)
+	})
+
+	t.Run("when the secret is configured", func(t *testing.T) {
+		testCluster := makeTestCluster(func(c *gitopsv1alpha1.GitopsCluster) {
+			c.ObjectMeta.Name = "only-secret-configured"
+			c.ObjectMeta.Namespace = "default"
+			c.Spec.SecretRef = &meta.LocalObjectReference{
+				Name: "test-secret",
+			}
+		})
+
+		assertNoError(t, cl.Create(context.TODO(), testCluster))
+	})
+
+	t.Run("when the capiClusterRef is configured", func(t *testing.T) {
+		testCluster := makeTestCluster(func(c *gitopsv1alpha1.GitopsCluster) {
+			c.ObjectMeta.Name = "only-capi-cluster-configured"
+			c.ObjectMeta.Namespace = "default"
+			c.Spec.CAPIClusterRef = &meta.LocalObjectReference{
+				Name: "test-cluster",
+			}
+		})
+
+		assertNoError(t, cl.Create(context.TODO(), testCluster))
+	})
+
+}
+
 func makeTestReconciler(t *testing.T, opts controllers.Options, objs ...runtime.Object) *controllers.GitopsClusterReconciler {
 	s, tc := makeTestClientAndScheme(t, opts, objs...)
 	return controllers.NewGitopsClusterReconciler(tc, s, opts)
@@ -592,6 +674,13 @@ func assertErrorMatch(t *testing.T, s string, e error) {
 	}
 }
 
+func assertErrorDoesNotMatch(t *testing.T, s string, e error) {
+	t.Helper()
+	if matchErrorString(t, s, e) {
+		t.Fatalf("error did match, got %s, should not contain %s", e, s)
+	}
+}
+
 func matchErrorString(t *testing.T, s string, e error) bool {
 	t.Helper()
 	if s == "" && e == nil {