Repository: kubean-io/kubean (go)

Issue:
when ops is created, some key value such as namespace/name is better to validate if they are matched
In KuBeanClusterOps, there are some important keys between 4 yml files; such as:

1. In kubeanCluster.ymlm spec.hostsConfRef.namespace should be the same as metadata.namespace in hosts-conf-cm.yml
2. In kubeanCluster.ymlm spec.hostsConfRef.name should be the same as metadata.name in hosts-conf-cm.yml


Pull request:
check DataRef in KubeanCluster
Signed-off-by: hang.jiang <hang.jiang@daocloud.io>

<!--  

Thanks for sending a pull request!  Here are some tips for you:

* make sure your commit is signed off

-->

**What type of PR is this?**
<!-- 
Uncomment only one ` /kind <>` line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake
-->

/kind bug

**What this PR does / why we need it**:

* check the hostsConf varsConf or sshAuth whether existing and in the same namespace

**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #19 

**Special notes for your reviewer**:



Patch:
diff --git a/pkg/controllers/clusterops/controller.go b/pkg/controllers/clusterops/controller.go
index 0b9346b49..fcda4faa4 100644
--- a/pkg/controllers/clusterops/controller.go
+++ b/pkg/controllers/clusterops/controller.go
@@ -205,6 +205,12 @@ func (c *Controller) Reconcile(ctx context.Context, req controllerruntime.Reques
 		return controllerruntime.Result{Requeue: false}, nil
 	}
 
+	cluster, err := c.GetKuBeanCluster(clusterOps)
+	if err != nil {
+		klog.Error(err)
+		return controllerruntime.Result{RequeueAfter: RequeueAfter}, err
+	}
+
 	if !IsValidImageName(clusterOps.Spec.Image) {
 		klog.Errorf("clusterOps %s has wrong image format and update status Failed", clusterOps.Name)
 		clusterOps.Status.Status = kubeanclusteropsv1alpha1.FailedStatus
@@ -214,11 +220,15 @@ func (c *Controller) Reconcile(ctx context.Context, req controllerruntime.Reques
 		return controllerruntime.Result{Requeue: false}, nil
 	}
 
-	cluster, err := c.GetKuBeanCluster(clusterOps)
-	if err != nil {
-		klog.Error(err)
-		return controllerruntime.Result{RequeueAfter: RequeueAfter}, err
+	if err := c.CheckClusterDataRef(cluster, clusterOps); err != nil {
+		klog.Error(err.Error())
+		clusterOps.Status.Status = kubeanclusteropsv1alpha1.FailedStatus
+		if err := c.Client.Status().Update(ctx, clusterOps); err != nil {
+			klog.Error(err)
+		}
+		return controllerruntime.Result{Requeue: false}, nil
 	}
+
 	needRequeue, err := c.UpdateClusterOpsStatusDigest(clusterOps)
 	if err != nil {
 		klog.Error(err)
@@ -628,3 +638,54 @@ func (c *Controller) SetupWithManager(mgr controllerruntime.Manager) error {
 		mgr.Add(c),
 	})
 }
+
+func (c *Controller) CheckConfigMapExist(namespace, name string) bool {
+	if _, err := c.ClientSet.CoreV1().ConfigMaps(namespace).Get(context.Background(), name, metav1.GetOptions{}); err != nil && apierrors.IsNotFound(err) {
+		return false
+	}
+	return true
+}
+
+func (c *Controller) CheckSecretExist(namespace, name string) bool {
+	if _, err := c.ClientSet.CoreV1().Secrets(namespace).Get(context.Background(), name, metav1.GetOptions{}); err != nil && apierrors.IsNotFound(err) {
+		return false
+	}
+	return true
+}
+
+func (c *Controller) CheckClusterDataRef(cluster *kubeanclusterv1alpha1.KuBeanCluster, clusterOPS *kubeanclusteropsv1alpha1.KuBeanClusterOps) error {
+	namespaceSet := map[string]struct{}{}
+	if clusterOPS.Spec.HostsConfRef.IsEmpty() {
+		// check HostsConfRef in cluster before clusterSpec is not assigned backup data.
+		hostsConfRef := cluster.Spec.HostsConfRef
+		if hostsConfRef.IsEmpty() {
+			return fmt.Errorf("kubeanCluster %s hostsConfRef is empty", cluster.Name)
+		}
+		if !c.CheckConfigMapExist(hostsConfRef.NameSpace, hostsConfRef.Name) {
+			return fmt.Errorf("kubeanCluster %s hostsConfRef %s,%s not found", cluster.Name, hostsConfRef.NameSpace, hostsConfRef.Name)
+		}
+		namespaceSet[hostsConfRef.NameSpace] = struct{}{}
+	}
+	if clusterOPS.Spec.VarsConfRef.IsEmpty() {
+		varsConfRef := cluster.Spec.VarsConfRef
+		if varsConfRef.IsEmpty() {
+			return fmt.Errorf("kubeanCluster %s varsConfRef is empty", cluster.Name)
+		}
+		if !c.CheckConfigMapExist(varsConfRef.NameSpace, varsConfRef.Name) {
+			return fmt.Errorf("kubeanCluster %s varsConfRef %s,%s not found", cluster.Name, varsConfRef.NameSpace, varsConfRef.Name)
+		}
+		namespaceSet[varsConfRef.NameSpace] = struct{}{}
+	}
+	if clusterOPS.Spec.SSHAuthRef.IsEmpty() && !cluster.Spec.SSHAuthRef.IsEmpty() {
+		// check SSHAuthRef optionally.
+		sshAuthRef := cluster.Spec.SSHAuthRef
+		if !c.CheckSecretExist(sshAuthRef.NameSpace, sshAuthRef.Name) {
+			return fmt.Errorf("kubeanCluster %s sshAuthRef %s,%s not found", cluster.Name, sshAuthRef.NameSpace, sshAuthRef.Name)
+		}
+		namespaceSet[sshAuthRef.NameSpace] = struct{}{}
+	}
+	if len(namespaceSet) > 1 {
+		return fmt.Errorf("kubeanCluster %s hostsConfRef varsConfRef or sshAuthRef not in the same namespace", cluster.Name)
+	}
+	return nil
+}


Test patch:
diff --git a/pkg/controllers/clusterops/controller_test.go b/pkg/controllers/clusterops/controller_test.go
index e9899350a..e26514152 100644
--- a/pkg/controllers/clusterops/controller_test.go
+++ b/pkg/controllers/clusterops/controller_test.go
@@ -2,7 +2,9 @@ package clusterops
 
 import (
 	"context"
+	"encoding/base64"
 	"fmt"
+	"strings"
 	"testing"
 	"time"
 
@@ -714,3 +716,245 @@ func TestCreateKubeSprayJob(t *testing.T) {
 		})
 	}
 }
+
+func Test_CheckConfigMapExist(t *testing.T) {
+	controller := Controller{
+		Client:              newFakeClient(),
+		ClientSet:           clientsetfake.NewSimpleClientset(),
+		KubeanClusterSet:    kubeanclusterv1alpha1fake.NewSimpleClientset(),
+		KubeanClusterOpsSet: kubeanclusteropsv1alpha1fake.NewSimpleClientset(),
+	}
+	tests := []struct {
+		name string
+		args func() bool
+		want bool
+	}{
+		{
+			name: "not exist configMap data",
+			args: func() bool {
+				return controller.CheckConfigMapExist("my_namespace_cm", "my_name_cm")
+			},
+			want: false,
+		},
+		{
+			name: "configMap data exists",
+			args: func() bool {
+				cm := &corev1.ConfigMap{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "ConfigMap",
+						APIVersion: "v1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "my_namespace_cm_1",
+						Name:      "my_name_cm_1",
+					},
+					Data: map[string]string{"ok": "ok123"},
+				}
+				controller.ClientSet.CoreV1().ConfigMaps("my_namespace_cm_1").Create(context.Background(), cm, metav1.CreateOptions{})
+				return controller.CheckConfigMapExist("my_namespace_cm_1", "my_name_cm_1")
+			},
+			want: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if test.args() != test.want {
+				t.Fatal()
+			}
+		})
+	}
+}
+
+func Test_CheckSecretExist(t *testing.T) {
+	controller := Controller{
+		Client:              newFakeClient(),
+		ClientSet:           clientsetfake.NewSimpleClientset(),
+		KubeanClusterSet:    kubeanclusterv1alpha1fake.NewSimpleClientset(),
+		KubeanClusterOpsSet: kubeanclusteropsv1alpha1fake.NewSimpleClientset(),
+	}
+	tests := []struct {
+		name string
+		args func() bool
+		want bool
+	}{
+		{
+			name: "secret data not exist",
+			args: func() bool {
+				return controller.CheckSecretExist("my_namespace_secret", "my_name_secret")
+			},
+			want: false,
+		},
+		{
+			name: "secret data exist",
+			args: func() bool {
+				secret := &corev1.Secret{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "Secret",
+						APIVersion: "v1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "my_namespace_secret_1",
+						Name:      "my_name_secret_1",
+					},
+					Data: map[string][]byte{"ok": []byte(base64.StdEncoding.EncodeToString([]byte("ok123")))},
+				}
+				controller.ClientSet.CoreV1().Secrets("my_namespace_secret_1").Create(context.Background(), secret, metav1.CreateOptions{})
+				return controller.CheckSecretExist("my_namespace_secret_1", "my_name_secret_1")
+			},
+			want: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if test.args() != test.want {
+				t.Fatal()
+			}
+		})
+	}
+}
+
+func Test_CheckClusterDataRef(t *testing.T) {
+	controller := Controller{
+		Client:              newFakeClient(),
+		ClientSet:           clientsetfake.NewSimpleClientset(),
+		KubeanClusterSet:    kubeanclusterv1alpha1fake.NewSimpleClientset(),
+		KubeanClusterOpsSet: kubeanclusteropsv1alpha1fake.NewSimpleClientset(),
+	}
+	cluster := &kubeanclusterv1alpha1.KuBeanCluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "my_kubean_cluster",
+		},
+	}
+	clusterOps := &kubeanclusteropsv1alpha1.KuBeanClusterOps{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "my_kubean_ops_cluster",
+		},
+	}
+	secret := &corev1.Secret{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "Secret",
+			APIVersion: "v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "my_namespace",
+			Name:      "my_name_secret_1",
+		},
+		Data: map[string][]byte{"ok": []byte(base64.StdEncoding.EncodeToString([]byte("ok123")))},
+	}
+	cmHosts := &corev1.ConfigMap{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "ConfigMap",
+			APIVersion: "v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "my_namespace",
+			Name:      "my_name_cm_1",
+		},
+		Data: map[string]string{"ok": "ok123"},
+	}
+	cmVars := &corev1.ConfigMap{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "ConfigMap",
+			APIVersion: "v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "my_namespace",
+			Name:      "my_name_cm_2",
+		},
+		Data: map[string]string{"ok": "ok123"},
+	}
+	tests := []struct {
+		name string
+		args func() bool
+		want bool
+	}{
+		{
+			name: "cluster.hostConf empty",
+			args: func() bool {
+				err := controller.CheckClusterDataRef(cluster, clusterOps)
+				return err != nil && strings.Contains(err.Error(), "hostsConfRef is empty")
+			},
+			want: true,
+		},
+		{
+			name: "cluster.hostConf not exist",
+			args: func() bool {
+				cluster.Spec.HostsConfRef = &apis.ConfigMapRef{
+					NameSpace: cmHosts.Namespace,
+					Name:      cmHosts.Name,
+				}
+				err := controller.CheckClusterDataRef(cluster, clusterOps)
+				return err != nil && strings.Contains(err.Error(), "hostsConfRef my_namespace,my_name_cm_1 not found")
+			},
+			want: true,
+		},
+		{
+			name: "cluster.varsConf empty",
+			args: func() bool {
+				controller.ClientSet.CoreV1().ConfigMaps(cmHosts.Namespace).Create(context.Background(), cmHosts, metav1.CreateOptions{})
+				err := controller.CheckClusterDataRef(cluster, clusterOps)
+				return err != nil && strings.Contains(err.Error(), "varsConfRef is empty")
+			},
+			want: true,
+		},
+		{
+			name: "cluster.varsConf not exist",
+			args: func() bool {
+				cluster.Spec.VarsConfRef = &apis.ConfigMapRef{
+					NameSpace: cmVars.Namespace,
+					Name:      cmVars.Name,
+				}
+				err := controller.CheckClusterDataRef(cluster, clusterOps)
+				fmt.Println(err)
+				return err != nil && strings.Contains(err.Error(), "varsConfRef my_namespace,my_name_cm_2 not found")
+			},
+			want: true,
+		},
+		{
+			name: "cluster.SSHAuthRef not exist",
+			args: func() bool {
+				controller.ClientSet.CoreV1().ConfigMaps(cmVars.Namespace).Create(context.Background(), cmVars, metav1.CreateOptions{})
+				cluster.Spec.SSHAuthRef = &apis.SecretRef{
+					NameSpace: secret.Namespace,
+					Name:      secret.Name,
+				}
+				err := controller.CheckClusterDataRef(cluster, clusterOps)
+				fmt.Println(err)
+				return err != nil && strings.Contains(err.Error(), "sshAuthRef my_namespace,my_name_secret_1 not found")
+			},
+			want: true,
+		},
+		{
+			name: "ok",
+			args: func() bool {
+				controller.ClientSet.CoreV1().Secrets(secret.Namespace).Create(context.Background(), secret, metav1.CreateOptions{})
+				err := controller.CheckClusterDataRef(cluster, clusterOps)
+				return err == nil
+			},
+			want: true,
+		},
+		{
+			name: "multi namespace",
+			args: func() bool {
+				secret.Namespace = "other_namespace"
+				cluster.Spec.SSHAuthRef = &apis.SecretRef{
+					NameSpace: secret.Namespace,
+					Name:      secret.Name,
+				}
+				controller.ClientSet.CoreV1().Secrets(secret.Namespace).Create(context.Background(), secret, metav1.CreateOptions{})
+				err := controller.CheckClusterDataRef(cluster, clusterOps)
+				return err != nil && strings.Contains(err.Error(), "hostsConfRef varsConfRef or sshAuthRef not in the same namespace")
+			},
+			want: true,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if test.args() != test.want {
+				t.Fatal()
+			}
+		})
+	}
+}