feat: improve handling annotations; add e2e test

Signed-off-by: Artur Shad Nik <[email protected]>

Author: arturshadnik

pkg/cmd/join/cmd.go

@@ -83,6 +83,6 @@ func NewCmd(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, stream
 	cmd.Flags().Int32Var(&o.clientCertExpirationSeconds, "client-cert-expiration-seconds", 31536000, "clientCertExpirationSeconds represents the seconds of a client certificate to expire.")
 	cmd.Flags().StringVar(&o.registrationAuth, "registration-auth", "", "The type of authentication to use for registering and authenticating with hub")
 	cmd.Flags().StringVar(&o.hubClusterArn, "hub-cluster-arn", "", "The arn of the hub cluster(i.e. EKS cluster) to which managed-cluster will join")
-	cmd.Flags().StringArrayVar(&o.klusterletAnnotations, "klusterlet-annotation", []string{}, fmt.Sprintf("Annotations to set on the ManagedCluster, in key=value format. Note: %s will be prepended to each key", operatorv1.ClusterAnnotationsKeyPrefix))
+	cmd.Flags().StringArrayVar(&o.klusterletAnnotations, "klusterlet-annotation", []string{}, fmt.Sprintf("Annotations to set on the ManagedCluster, in key=value format. Note: each key will be automatically prefixed with '%s/', if not set.", operatorv1.ClusterAnnotationsKeyPrefix))
 	return cmd
 }

pkg/cmd/join/exec.go

@@ -753,15 +753,24 @@ func (o *Options) setKlusterletRegistrationAnnotations() {
 		o.klusterletChartConfig.Klusterlet.RegistrationConfiguration.ClusterAnnotations = map[string]string{}
 	}
 
-	for _, a := range o.klusterletAnnotations {
-		parts := strings.Split(a, "=")
-		if len(parts) < 2 {
+	for _, annotation := range o.klusterletAnnotations {
+		i := strings.Index(annotation, "=")
+		if i == -1 {
+			klog.Warningf("Skipping malformed annotation (missing '='): %s", annotation)
 			continue
 		}
-		k, v := parts[0], parts[1]
+
+		k, v := strings.TrimSpace(annotation[:i]), strings.TrimSpace(annotation[i+1:])
+
+		if k == "" {
+			klog.Warningf("Skipping annotation with empty key: %s", annotation)
+			continue
+		}
+
 		if !strings.HasPrefix(k, operatorv1.ClusterAnnotationsKeyPrefix) {
 			k = fmt.Sprintf("%s/%s", operatorv1.ClusterAnnotationsKeyPrefix, k)
 		}
+
 		o.klusterletChartConfig.Klusterlet.RegistrationConfiguration.ClusterAnnotations[k] = v
 	}
 }

test/e2e/clusteradm/joinhubscenario_annotate_test.go

@@ -0,0 +1,76 @@
+// Copyright Contributors to the Open Cluster Management project
+package clusteradme2e
+
+import (
+	"context"
+	"time"
+
+	"github.com/onsi/ginkgo/v2"
+	"github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	operatorv1 "open-cluster-management.io/api/operator/v1"
+	"open-cluster-management.io/clusteradm/test/e2e/util"
+)
+
+var _ = ginkgo.Describe("test clusteradm join with annotations", func() {
+	ginkgo.BeforeEach(func() {
+		ginkgo.By("clear e2e environment...")
+		err := e2e.ClearEnv()
+		gomega.Expect(err).NotTo(gomega.HaveOccurred())
+	})
+
+	ginkgo.Context("join hub scenario with annotations", func() {
+		var err error
+
+		ginkgo.It("should managedclusters join with annotations and be accepted successfully", func() {
+			ginkgo.By("init hub")
+			err = e2e.Clusteradm().Init(
+				"--context", e2e.Cluster().Hub().Context(),
+				"--bundle-version=latest",
+			)
+			gomega.Expect(err).NotTo(gomega.HaveOccurred(), "clusteradm init error")
+
+			ginkgo.By("managedcluster1 join hub with annotations")
+			err = e2e.Clusteradm().Join(
+				"--context", e2e.Cluster().ManagedCluster1().Context(),
+				"--hub-token", e2e.CommandResult().Token(), "--hub-apiserver", e2e.CommandResult().Host(),
+				"--cluster-name", e2e.Cluster().ManagedCluster1().Name(),
+				"--wait",
+				"--bundle-version=latest",
+				"--force-internal-endpoint-lookup",
+				"--klusterlet-annotation", "foo=bar",
+				"--klusterlet-annotation", "test=value",
+			)
+			gomega.Expect(err).NotTo(gomega.HaveOccurred(), "managedcluster1 join error")
+			gomega.Eventually(func() error {
+				return util.ValidateImagePullSecret(managedClusterKubeClient,
+					"e30=", "open-cluster-management")
+			}, time.Second*120, time.Second*2).ShouldNot(gomega.HaveOccurred())
+
+			ginkgo.By("hub accept managedcluster1")
+			err = e2e.Clusteradm().Accept(
+				"--clusters", e2e.Cluster().ManagedCluster1().Name(),
+				"--wait",
+				"--context", e2e.Cluster().Hub().Context(),
+			)
+			gomega.Expect(err).NotTo(gomega.HaveOccurred(), "clusteradm accept error")
+
+			ginkgo.By("verify managedcluster1 has correct annotations")
+			gomega.Eventually(func() map[string]string {
+				managedCluster, err := clusterClient.ClusterV1().ManagedClusters().Get(
+					context.TODO(), e2e.Cluster().ManagedCluster1().Name(), metav1.GetOptions{})
+				if err != nil {
+					return nil
+				}
+				return managedCluster.GetAnnotations()
+			}, time.Second*60, time.Second*2).Should(gomega.HaveKey(operatorv1.ClusterAnnotationsKeyPrefix + "foo"))
+			managedCluster, err := clusterClient.ClusterV1().ManagedClusters().Get(
+				context.TODO(), e2e.Cluster().ManagedCluster1().Name(), metav1.GetOptions{})
+			gomega.Expect(err).NotTo(gomega.HaveOccurred())
+			annotations := managedCluster.GetAnnotations()
+			gomega.Expect(annotations).NotTo(gomega.BeNil())
+			gomega.Expect(annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/foo"]).To(gomega.Equal("bar"))
+			gomega.Expect(annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/test"]).To(gomega.Equal("value"))
+		})
+	})
+})