gitopsCluster support on non-OCP clussters

.gitignore

@@ -1,3 +1,4 @@
 build/_output
 build/_test
-bin/
+bin/
+*.out

Makefile

@@ -34,7 +34,7 @@ export GOPACKAGES   = $(shell go list ./... | grep -v /manager | grep -v /bindat
 
 TEST_TMP :=/tmp
 export KUBEBUILDER_ASSETS ?=$(TEST_TMP)/kubebuilder/bin
-K8S_VERSION ?=1.19.2
+K8S_VERSION ?=1.28.3
 GOHOSTOS ?=$(shell go env GOHOSTOS)
 GOHOSTARCH ?= $(shell go env GOHOSTARCH)
 KB_TOOLS_ARCHIVE_NAME :=kubebuilder-tools-$(K8S_VERSION)-$(GOHOSTOS)-$(GOHOSTARCH).tar.gz

pkg/controller/gitopscluster/gitopscluster_controller.go

@@ -63,6 +63,14 @@ type ReconcileGitOpsCluster struct {
 	lock       sync.Mutex
 }
 
+// TokenConfig defines a token configuration used in ArgoCD cluster secret
+type TokenConfig struct {
+	BearerToken     string `json:"bearerToken"`
+	TLSClientConfig struct {
+		Insecure bool `json:"insecure"`
+	} `json:"tlsClientConfig"`
+}
+
 // Add creates a new argocd cluster Controller and adds it to the Manager with default RBAC.
 // The Manager will set fields on the Controller and Start it when the Manager is Started.
 func Add(mgr manager.Manager) error {
@@ -470,7 +478,8 @@ func (r *ReconcileGitOpsCluster) GetAllManagedClusterSecretsInArgo() (v1.SecretL
 	return *secretList, nil
 }
 
-// GetAllManagedClusterSecretsInArgo returns list of secrets from all GitOps managed cluster
+// GetAllManagedClusterSecretsInArgo returns list of secrets from all GitOps managed cluster.
+// these secrets are not gnerated by ACM ArgoCD push model, they are created by end users themselves
 func (r *ReconcileGitOpsCluster) GetAllNonAcmManagedClusterSecretsInArgo(argoNs string) (map[string][]*v1.Secret, error) {
 	klog.Info("Getting all non-acm managed cluster secrets from argo namespaces")
 
@@ -960,6 +969,8 @@ func (r *ReconcileGitOpsCluster) CreateManagedClusterSecretInArgo(argoNamespace
 	// create the new cluster secret in the argocd server namespace
 	var newSecret *v1.Secret
 
+	clusterURL := ""
+
 	if createBlankClusterSecrets {
 		newSecret = &v1.Secret{
 			TypeMeta: metav1.TypeMeta{
@@ -983,6 +994,22 @@ func (r *ReconcileGitOpsCluster) CreateManagedClusterSecretInArgo(argoNamespace
 			},
 		}
 	} else {
+		if string(managedClusterSecret.Data["server"]) == "" {
+			clusterToken, err := getManagedClusterToken(managedClusterSecret.Data["config"])
+			if err != nil {
+				klog.Error(err)
+
+				return nil, err
+			}
+
+			clusterURL, err = getManagedClusterURL(managedCluster, clusterToken)
+			if err != nil {
+				klog.Error(err)
+
+				return nil, err
+			}
+		}
+
 		labels := managedClusterSecret.GetLabels()
 
 		newSecret = &v1.Secret{
@@ -1004,7 +1031,7 @@ func (r *ReconcileGitOpsCluster) CreateManagedClusterSecretInArgo(argoNamespace
 			StringData: map[string]string{
 				"config": string(managedClusterSecret.Data["config"]),
 				"name":   string(managedClusterSecret.Data["name"]),
-				"server": string(managedClusterSecret.Data["server"]),
+				"server": clusterURL,
 			},
 		}
 	}
@@ -1178,6 +1205,22 @@ func unionSecretData(newSecret, existingSecret *v1.Secret) *v1.Secret {
 	return newSecret
 }
 
+func getManagedClusterToken(dataConfig []byte) (string, error) {
+	if dataConfig == nil {
+		return "", fmt.Errorf("empty secrect data config")
+	}
+
+	// Unmarshal the decoded JSON into the Config struct
+	var config TokenConfig
+	err := json.Unmarshal(dataConfig, &config)
+
+	if err != nil {
+		return "", fmt.Errorf("failed to unmarshal JSON: %w", err)
+	}
+
+	return config.BearerToken, nil
+}
+
 func getManagedClusterURL(managedCluster *spokeclusterv1.ManagedCluster, token string) (string, error) {
 	clientConfigs := managedCluster.Spec.ManagedClusterClientConfigs
 	if len(clientConfigs) == 0 {

pkg/controller/gitopscluster/gitopscluster_controller_test.go

@@ -152,6 +152,31 @@ var (
 		},
 	}
 
+	// Test5 resources
+	test5Ns = &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test5",
+		},
+	}
+
+	test5Pl = &clusterv1beta1.Placement{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-placement-5",
+			Namespace: test5Ns.Name,
+		},
+		Spec: clusterv1beta1.PlacementSpec{},
+	}
+
+	test5PlDc = &clusterv1beta1.PlacementDecision{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-placement-decision-5",
+			Namespace: test5Ns.Name,
+			Labels: map[string]string{
+				"cluster.open-cluster-management.io/placement": "test-placement-5",
+			},
+		},
+	}
+
 	// Namespace where GitOpsCluster1 CR is
 	testNamespace1 = &corev1.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
@@ -216,6 +241,32 @@ var (
 		},
 	}
 
+	gitopsServerNamespace5 = &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "openshift-gitops5",
+		},
+	}
+
+	managedClusterSecret5 = &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "cluster1-cluster-secret",
+			Namespace: "cluster1",
+			Labels: map[string]string{
+				"apps.open-cluster-management.io/secret-type": "acm-cluster",
+			},
+		},
+		StringData: map[string]string{
+			"name":   "cluster1",
+			"server": "",
+			"config": "{\"bearerToken\": \"fakeToken1\", \"tlsClientConfig\": {\"insecure\": true}}",
+		},
+	}
+
+	gitOpsClusterSecret5Key = types.NamespacedName{
+		Name:      "cluster1-cluster-secret",
+		Namespace: gitopsServerNamespace5.Name,
+	}
+
 	managedClusterSecret10 = &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "cluster10-cluster-secret",
@@ -703,6 +754,128 @@ func TestReconcileCreateSecretInOpenshiftGitops(t *testing.T) {
 	}, 30*time.Second, 1*time.Second).Should(gomega.Succeed())
 }
 
+// test managed cluster secret creation for non OCP clusters
+func TestReconcileNonOCPCreateSecretInOpenshiftGitops(t *testing.T) {
+	g := gomega.NewGomegaWithT(t)
+
+	mgr, err := manager.New(cfg, manager.Options{
+		Metrics: metricsserver.Options{
+			BindAddress: "0",
+		},
+	})
+
+	g.Expect(err).NotTo(gomega.HaveOccurred())
+
+	c = mgr.GetClient()
+
+	reconciler, err := newReconciler(mgr)
+	g.Expect(err).NotTo(gomega.HaveOccurred())
+
+	recFn := SetupTestReconcile(reconciler)
+	g.Expect(add(mgr, recFn)).NotTo(gomega.HaveOccurred())
+
+	ctx, cancel := context.WithTimeout(context.TODO(), 5*time.Minute)
+	mgrStopped := StartTestManager(ctx, mgr, g)
+
+	defer func() {
+		cancel()
+		mgrStopped.Wait()
+	}()
+
+	// Set up test environment
+	c.Create(context.TODO(), test5Ns)
+
+	// Create placement
+	g.Expect(c.Create(context.TODO(), test5Pl.DeepCopy())).NotTo(gomega.HaveOccurred())
+
+	defer c.Delete(context.TODO(), test5Pl)
+
+	// Create placement decision
+	g.Expect(c.Create(context.TODO(), test5PlDc.DeepCopy())).NotTo(gomega.HaveOccurred())
+
+	defer c.Delete(context.TODO(), test5PlDc)
+
+	time.Sleep(time.Second * 3)
+
+	// Update placement decision status
+	placementDecision5 := &clusterv1beta1.PlacementDecision{}
+	g.Expect(c.Get(context.TODO(),
+		types.NamespacedName{Namespace: test5PlDc.Namespace, Name: test5PlDc.Name},
+		placementDecision5)).NotTo(gomega.HaveOccurred())
+
+	newPlacementDecision5 := placementDecision5.DeepCopy()
+	newPlacementDecision5.Status = *placementDecisionStatus
+
+	g.Expect(c.Status().Update(context.TODO(), newPlacementDecision5)).NotTo(gomega.HaveOccurred())
+
+	time.Sleep(time.Second * 3)
+
+	placementDecisionAfterupdate5 := &clusterv1beta1.PlacementDecision{}
+	g.Expect(c.Get(context.TODO(),
+		types.NamespacedName{Namespace: placementDecision5.Namespace, Name: placementDecision5.Name},
+		placementDecisionAfterupdate5)).NotTo(gomega.HaveOccurred())
+
+	g.Expect(placementDecisionAfterupdate5.Status.Decisions[0].ClusterName).To(gomega.Equal("cluster1"))
+
+	// Create Managed cluster secret with empty api server url
+	c.Create(context.TODO(), managedClusterNamespace1)
+
+	managedClusterSecret5 := managedClusterSecret5.DeepCopy()
+	managedClusterSecret5.StringData["server"] = ""
+
+	g.Expect(c.Create(context.TODO(), managedClusterSecret5.DeepCopy())).NotTo(gomega.HaveOccurred())
+
+	defer c.Delete(context.TODO(), managedClusterSecret5)
+
+	// Create Managed cluster with empty api server url data
+	mc1 := managedCluster1.DeepCopy()
+
+	g.Expect(c.Create(context.TODO(), mc1)).NotTo(gomega.HaveOccurred())
+
+	defer c.Delete(context.TODO(), mc1)
+
+	// Create Openshift-gitops5 namespace
+	c.Create(context.TODO(), gitopsServerNamespace5)
+
+	argoServiceInGitOps := argoService.DeepCopy()
+	argoServiceInGitOps.Namespace = gitopsServerNamespace5.Name
+
+	g.Expect(c.Create(context.TODO(), argoServiceInGitOps)).NotTo(gomega.HaveOccurred())
+
+	defer c.Delete(context.TODO(), argoServiceInGitOps)
+
+	// Create GitOpsCluster CR
+	goc := gitOpsCluster.DeepCopy()
+	goc.Namespace = test5Ns.Name
+	goc.Spec.ArgoServer.ArgoNamespace = gitopsServerNamespace5.Name
+	goc.Spec.PlacementRef = &corev1.ObjectReference{
+		Kind:       "Placement",
+		APIVersion: "cluster.open-cluster-management.io/v1beta1",
+		Name:       test5Pl.Name,
+	}
+
+	g.Expect(c.Create(context.TODO(), goc)).NotTo(gomega.HaveOccurred())
+	defer c.Delete(context.TODO(), goc)
+
+	// expect the gitopscluster CR fails to generate the cluster secret as both the managed cluster and the managed cluster secret don't have api server url
+	time.Sleep(3 * time.Second)
+
+	g.Expect(c.Get(context.TODO(), client.ObjectKeyFromObject(goc), goc)).NotTo(gomega.HaveOccurred())
+
+	g.Expect(goc.Status.Phase).To(gomega.Equal("failed"))
+
+	// append the api server url to the managed cluster, expect the managed cluster's secret is created in the Argo namespace
+	mc1.Spec.ManagedClusterClientConfigs = []clusterv1.ClientConfig{{URL: "https://local-cluster-5:6443", CABundle: []byte("abc")}}
+	g.Expect(c.Update(context.TODO(), mc1)).NotTo(gomega.HaveOccurred())
+
+	time.Sleep(3 * time.Second)
+
+	updatedSecret := expectedSecretCreated(c, gitOpsClusterSecret5Key)
+
+	g.Expect(updatedSecret).ToNot(gomega.BeNil())
+	g.Expect(string(updatedSecret.Data["server"])).To(gomega.Equal("https://local-cluster-5:6443"))
+}
+
 func expectedSecretCreated(c client.Client, expectedSecretKey types.NamespacedName) *corev1.Secret {
 	timeout := 0
 

pkg/controller/gitopssyncresc/gitopssyncresc_controller.go

@@ -66,6 +66,8 @@ type HTTPDataSender struct{}
 func (c *HTTPDataSender) Send(httpClient *http.Client, req *http.Request) (map[string]interface{}, error) {
 	respData := make(map[string]interface{})
 
+	klog.V(1).Infof("http reqeust: %v", req)
+
 	resp, err := httpClient.Do(req)
 	if err != nil {
 		klog.Info(err.Error())
@@ -152,6 +154,7 @@ func (r *GitOpsSyncResource) syncResources() error {
 	appReportsMap := make(map[string]*appsetreport.MulticlusterApplicationSetReport)
 
 	// Query search for argo apps
+	// get all managed clusters not containing the local-cluster: true label
 	managedclusters, err := getAllManagedClusterNames(r.Client)
 	if err != nil {
 		return err
@@ -165,11 +168,9 @@ func (r *GitOpsSyncResource) syncResources() error {
 		queryManagedClustersStr := []string{}
 
 		for len(queryManagedClusters) < 20 && iManagedCluster < mangedClusterTotal {
-			// Ignore local-cluster
-			if managedclusters[iManagedCluster].Name != "local-cluster" {
-				queryManagedClusters = append(queryManagedClusters, managedclusters[iManagedCluster])
-				queryManagedClustersStr = append(queryManagedClustersStr, managedclusters[iManagedCluster].Name)
-			}
+			// Ignore local-cluster. managedclusters only include all managed clusters not containing the local-cluster: true label
+			queryManagedClusters = append(queryManagedClusters, managedclusters[iManagedCluster])
+			queryManagedClustersStr = append(queryManagedClustersStr, managedclusters[iManagedCluster].Name)
 
 			iManagedCluster++
 		}
@@ -250,7 +251,16 @@ func getAllManagedClusterNames(c client.Client) ([]clusterv1.ManagedCluster, err
 		return nil, err
 	}
 
-	return managedclusters.Items, nil
+	filteredClusters := []clusterv1.ManagedCluster{}
+
+	for _, cluster := range managedclusters.Items {
+		if value, exists := cluster.Labels["local-cluster"]; exists && value == "true" {
+			continue
+		}
+		filteredClusters = append(filteredClusters, cluster)
+	}
+
+	return filteredClusters, nil
 }
 
 func (r *GitOpsSyncResource) getSearchURL() (string, error) {

pkg/controller/multiclusterstatusaggregation/multiclusterStatusAggregation_controller_test.go

@@ -456,17 +456,10 @@ func TestReconcilePullModel(t *testing.T) {
 
 	c = mgr.GetClient()
 
-	Add(mgr, 10, "../../../hack/test") // 10 second interval
-
 	ctx, cancel := context.WithTimeout(context.TODO(), 5*time.Minute)
-	mgrStopped := StartTestManager(ctx, mgr, g)
 
-	defer func() {
-		cancel()
-		mgrStopped.Wait()
-	}()
-
-	// Create appsets
+	// Create appsets before the aggregation controller is started.
+	// Or the hack/test/openshift-gitops_appset-1.yaml will be deleted as its related appset is not created yet
 	g.Expect(c.Create(ctx, sampleAppset1.DeepCopy())).NotTo(HaveOccurred())
 	g.Expect(c.Create(ctx, sampleAppset2.DeepCopy())).NotTo(HaveOccurred())
 	g.Expect(c.Create(ctx, sampleAppset3.DeepCopy())).NotTo(HaveOccurred())
@@ -478,6 +471,15 @@ func TestReconcilePullModel(t *testing.T) {
 	g.Expect(c.Create(ctx, sampleAppsetDummy.DeepCopy())).NotTo(HaveOccurred())
 	g.Expect(c.Create(ctx, longAppsetName.DeepCopy())).NotTo(HaveOccurred())
 
+	Add(mgr, 10, "../../../hack/test") // 10 second interval
+
+	mgrStopped := StartTestManager(ctx, mgr, g)
+
+	defer func() {
+		cancel()
+		mgrStopped.Wait()
+	}()
+
 	// Create appset reports
 	g.Expect(c.Create(ctx, sampleMulticlusterApplicationSetReport1.DeepCopy())).NotTo(HaveOccurred())
 	g.Expect(c.Create(ctx, sampleMulticlusterApplicationSetReport2.DeepCopy())).NotTo(HaveOccurred())

pkg/utils/cluster.go

@@ -60,6 +60,10 @@ var ClusterPredicateFunc = predicate.Funcs{
 			return true
 		}
 
+		if !reflect.DeepEqual(oldcl.Spec.ManagedClusterClientConfigs, newcl.Spec.ManagedClusterClientConfigs) {
+			return true
+		}
+
 		oldcondMap := make(map[string]metav1.ConditionStatus)
 		for _, cond := range oldcl.Status.Conditions {
 			oldcondMap[cond.Type] = cond.Status