Extract tags from ManagedClusters as labels. (#22)

These labels are then applied to the GeneratedClusters. This functionality can be disabled by setting `spec.disableTags` on the AutomatedClusterDiscovery to true.
weaveworks · Nov 22, 2023 · e2687cc · e2687cc
1 parent b5b83a7
commit e2687cc
Show file tree

Hide file tree

Showing 7 changed files with 222 additions and 17 deletions.
diff --git a/api/v1alpha1/automatedclusterdiscovery_types.go b/api/v1alpha1/automatedclusterdiscovery_types.go
@@ -33,10 +33,15 @@ type AutomatedClusterDiscoverySpec struct {
 	// Name is the name of the cluster
 	Name string `json:"name,omitempty"`
 
-	// Type is the provider type
+	// Type is the provider type.
 	// +kubebuilder:validation:Enum=aks
 	Type string `json:"type"`
 
+	// If DisableTags is true, labels will not be applied to the generated
+	// Clusters from the tags on the upstream Clusters.
+	// +optional
+	DisableTags bool `json:"disableTags"`
+
 	AKS *AKS `json:"aks,omitempty"`
 
 	// The interval at which to run the discovery

diff --git a/config/crd/bases/clusters.weave.works_automatedclusterdiscoveries.yaml b/config/crd/bases/clusters.weave.works_automatedclusterdiscoveries.yaml
@@ -62,6 +62,10 @@ spec:
                   type: string
                 description: Labels to add to all generated resources.
                 type: object
+              disableTags:
+                description: If DisableTags is true, labels will not be applied to
+                  the generated Clusters from the tags on the upstream Clusters.
+                type: boolean
               interval:
                 description: The interval at which to run the discovery
                 type: string
@@ -73,7 +77,7 @@ spec:
                   of this AutomatedClusterDiscovery.
                 type: boolean
               type:
-                description: Type is the provider type
+                description: Type is the provider type.
                 enum:
                 - aks
                 type: string

diff --git a/internal/controller/automatedclusterdiscovery_controller.go b/internal/controller/automatedclusterdiscovery_controller.go
@@ -248,7 +248,13 @@ func (r *AutomatedClusterDiscoveryReconciler) reconcileClusters(ctx context.Cont
 		if err := controllerutil.SetOwnerReference(acd, gitopsCluster, r.Scheme); err != nil {
 			return inventoryResources, fmt.Errorf("failed to set ownership on created GitopsCluster: %w", err)
 		}
-		gitopsCluster.SetLabels(labelsForResource(*acd))
+
+		clusterLabels := labelsForResource(*acd)
+		if !acd.Spec.DisableTags {
+			clusterLabels = mergeMaps(cluster.Labels, clusterLabels)
+		}
+
+		gitopsCluster.SetLabels(clusterLabels)
 
 		gitopsCluster.SetAnnotations(mergeMaps(acd.Spec.CommonAnnotations, map[string]string{
 			gitopsv1alpha1.GitOpsClusterNoSecretFinalizerAnnotation: "true",
@@ -359,7 +365,6 @@ func (r *AutomatedClusterDiscoveryReconciler) reconcileClusters(ctx context.Cont
 		if err := r.Client.Update(ctx, secretToUpdate); err != nil {
 			return inventoryResources, err
 		}
-
 	}
 
 	return inventoryResources, nil

diff --git a/internal/controller/automatedclusterdiscovery_controller_test.go b/internal/controller/automatedclusterdiscovery_controller_test.go
@@ -188,11 +188,11 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) {
 		testProvider := stubProvider{
 			response: []*providers.ProviderCluster{
 				{
-					Name: "cluster-1",
+					Name: "cluster-2",
 					KubeConfig: &kubeconfig.Config{
 						APIVersion: "v1",
 						Clusters: map[string]*kubeconfig.Cluster{
-							"cluster-1": {
+							"cluster-2": {
 								Server:                   "https://cluster-prod.example.com/",
 								CertificateAuthorityData: []uint8(testCAData),
 							},
@@ -232,23 +232,188 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) {
 		}
 
 		gitopsCluster := &gitopsv1alpha1.GitopsCluster{}
-		err = k8sClient.Get(ctx, types.NamespacedName{Name: "cluster-1", Namespace: aksCluster.Namespace}, gitopsCluster)
+		err = k8sClient.Get(ctx, types.NamespacedName{Name: "cluster-2", Namespace: aksCluster.Namespace}, gitopsCluster)
 		assert.NoError(t, err)
 		assert.Equal(t, gitopsv1alpha1.GitopsClusterSpec{
-			SecretRef: &meta.LocalObjectReference{Name: "cluster-1-kubeconfig"},
+			SecretRef: &meta.LocalObjectReference{Name: "cluster-2-kubeconfig"},
 		}, gitopsCluster.Spec)
 		assertHasLabels(t, gitopsCluster, wantLabels)
 		assertHasAnnotations(t, gitopsCluster, mergeMaps(wantAnnotations, map[string]string{
 			gitopsv1alpha1.GitOpsClusterNoSecretFinalizerAnnotation: "true",
 		}))
 
 		secret := &corev1.Secret{}
-		err = k8sClient.Get(ctx, types.NamespacedName{Name: "cluster-1-kubeconfig", Namespace: aksCluster.Namespace}, secret)
+		err = k8sClient.Get(ctx, types.NamespacedName{Name: "cluster-2-kubeconfig", Namespace: aksCluster.Namespace}, secret)
 		assert.NoError(t, err)
 		assertHasLabels(t, secret, wantLabels)
 		assertHasAnnotations(t, secret, wantAnnotations)
 	})
 
+	t.Run("Reconcile with cluster labels applies labels to generated cluster", func(t *testing.T) {
+		aksCluster := &clustersv1alpha1.AutomatedClusterDiscovery{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test-aks",
+				Namespace: "default",
+			},
+			Spec: clustersv1alpha1.AutomatedClusterDiscoverySpec{
+				Type: "aks",
+				AKS: &clustersv1alpha1.AKS{
+					SubscriptionID: "subscription-123",
+				},
+				Interval: metav1.Duration{Duration: time.Minute},
+			},
+		}
+
+		testProvider := stubProvider{
+			response: []*providers.ProviderCluster{
+				{
+					Name: "cluster-with-labels",
+					Labels: map[string]string{
+						"com.example/testing-tag": "test-value",
+					},
+					KubeConfig: &kubeconfig.Config{
+						APIVersion: "v1",
+						Clusters: map[string]*kubeconfig.Cluster{
+							"cluster-1": {
+								Server:                   "https://cluster-prod.example.com/",
+								CertificateAuthorityData: []uint8(testCAData),
+							},
+						},
+					},
+				},
+			},
+		}
+
+		reconciler := &AutomatedClusterDiscoveryReconciler{
+			Client: k8sClient,
+			Scheme: scheme,
+			AKSProvider: func(providerID string) providers.Provider {
+				return &testProvider
+			},
+			EventRecorder: &mockEventRecorder{},
+		}
+
+		assert.NoError(t, reconciler.SetupWithManager(mgr))
+
+		ctx := context.TODO()
+		key := types.NamespacedName{Name: aksCluster.Name, Namespace: aksCluster.Namespace}
+		err = k8sClient.Create(ctx, aksCluster)
+		assert.NoError(t, err)
+		defer deleteClusterDiscoveryAndInventory(t, k8sClient, aksCluster)
+
+		result, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: key})
+		assert.NoError(t, err)
+		assert.Equal(t, ctrl.Result{RequeueAfter: time.Minute}, result)
+
+		wantLabels := map[string]string{
+			"app.kubernetes.io/managed-by":          "cluster-reflector-controller",
+			"clusters.weave.works/origin-name":      "test-aks",
+			"clusters.weave.works/origin-namespace": "default",
+			"clusters.weave.works/origin-type":      "aks",
+		}
+
+		gitopsCluster := &gitopsv1alpha1.GitopsCluster{}
+		err = k8sClient.Get(ctx, types.NamespacedName{Name: "cluster-with-labels", Namespace: aksCluster.Namespace}, gitopsCluster)
+		assert.NoError(t, err)
+		assert.Equal(t, gitopsv1alpha1.GitopsClusterSpec{
+			SecretRef: &meta.LocalObjectReference{Name: "cluster-with-labels-kubeconfig"},
+		}, gitopsCluster.Spec)
+		assertHasLabels(t, gitopsCluster, mergeMaps(wantLabels, map[string]string{
+			"com.example/testing-tag": "test-value",
+		}))
+		assertHasAnnotations(t, gitopsCluster, map[string]string{
+			gitopsv1alpha1.GitOpsClusterNoSecretFinalizerAnnotation: "true",
+		})
+
+		secret := &corev1.Secret{}
+		err = k8sClient.Get(ctx, types.NamespacedName{Name: "cluster-with-labels-kubeconfig", Namespace: aksCluster.Namespace}, secret)
+		assert.NoError(t, err)
+		assertHasLabels(t, secret, wantLabels)
+		assertHasAnnotations(t, secret, map[string]string{})
+	})
+
+	t.Run("Reconcile with cluster labels does not apply labels to cluster when tags disabled", func(t *testing.T) {
+		aksCluster := &clustersv1alpha1.AutomatedClusterDiscovery{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test-aks-disabled-tags",
+				Namespace: "default",
+			},
+			Spec: clustersv1alpha1.AutomatedClusterDiscoverySpec{
+				Type:        "aks",
+				DisableTags: true,
+				AKS: &clustersv1alpha1.AKS{
+					SubscriptionID: "subscription-123",
+				},
+				Interval: metav1.Duration{Duration: time.Minute},
+			},
+		}
+
+		testProvider := stubProvider{
+			response: []*providers.ProviderCluster{
+				{
+					Name: "test-cluster",
+					Labels: map[string]string{
+						"com.example/testing-tag": "test-value",
+					},
+					KubeConfig: &kubeconfig.Config{
+						APIVersion: "v1",
+						Clusters: map[string]*kubeconfig.Cluster{
+							"cluster-1": {
+								Server:                   "https://cluster-prod.example.com/",
+								CertificateAuthorityData: []uint8(testCAData),
+							},
+						},
+					},
+				},
+			},
+		}
+
+		reconciler := &AutomatedClusterDiscoveryReconciler{
+			Client: k8sClient,
+			Scheme: scheme,
+			AKSProvider: func(providerID string) providers.Provider {
+				return &testProvider
+			},
+			EventRecorder: &mockEventRecorder{},
+		}
+
+		assert.NoError(t, reconciler.SetupWithManager(mgr))
+
+		ctx := context.TODO()
+		key := types.NamespacedName{Name: aksCluster.Name, Namespace: aksCluster.Namespace}
+		err = k8sClient.Create(ctx, aksCluster)
+		assert.NoError(t, err)
+		defer deleteClusterDiscoveryAndInventory(t, k8sClient, aksCluster)
+
+		result, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: key})
+		assert.NoError(t, err)
+		assert.Equal(t, ctrl.Result{RequeueAfter: time.Minute}, result)
+
+		wantLabels := map[string]string{
+			"app.kubernetes.io/managed-by":          "cluster-reflector-controller",
+			"clusters.weave.works/origin-name":      "test-aks-disabled-tags",
+			"clusters.weave.works/origin-namespace": "default",
+			"clusters.weave.works/origin-type":      "aks",
+		}
+
+		gitopsCluster := &gitopsv1alpha1.GitopsCluster{}
+		err = k8sClient.Get(ctx, types.NamespacedName{Name: "test-cluster", Namespace: aksCluster.Namespace}, gitopsCluster)
+		assert.NoError(t, err)
+		assert.Equal(t, gitopsv1alpha1.GitopsClusterSpec{
+			SecretRef: &meta.LocalObjectReference{Name: "test-cluster-kubeconfig"},
+		}, gitopsCluster.Spec)
+		assertHasLabels(t, gitopsCluster, wantLabels)
+		assertHasAnnotations(t, gitopsCluster, map[string]string{
+			gitopsv1alpha1.GitOpsClusterNoSecretFinalizerAnnotation: "true",
+		})
+
+		secret := &corev1.Secret{}
+		err = k8sClient.Get(ctx, types.NamespacedName{Name: "test-cluster-kubeconfig", Namespace: aksCluster.Namespace}, secret)
+		assert.NoError(t, err)
+		assertHasLabels(t, secret, wantLabels)
+		assertHasAnnotations(t, secret, map[string]string{})
+	})
+
 	t.Run("Reconcile when executing in cluster and cluster matches reflector cluster", func(t *testing.T) {
 		aksCluster := &clustersv1alpha1.AutomatedClusterDiscovery{
 			ObjectMeta: metav1.ObjectMeta{
@@ -305,6 +470,9 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Equal(t, ctrl.Result{RequeueAfter: time.Minute}, result)
 
+		cl := &gitopsv1alpha1.GitopsClusterList{}
+		assert.NoError(t, k8sClient.List(ctx, cl, &client.ListOptions{Namespace: corev1.NamespaceDefault}))
+
 		gitopsCluster := &gitopsv1alpha1.GitopsCluster{}
 		err = k8sClient.Get(ctx, types.NamespacedName{Name: "test-cluster", Namespace: aksCluster.Namespace}, gitopsCluster)
 		assert.True(t, apierrors.IsNotFound(err))
@@ -864,6 +1032,8 @@ func deleteClusterDiscoveryAndInventory(t *testing.T, cl client.Client, cd *clus
 	t.Helper()
 	ctx := context.TODO()
 
+	assert.NoError(t, cl.Get(ctx, client.ObjectKeyFromObject(cd), cd))
+
 	if cd.Status.Inventory != nil {
 		for _, v := range cd.Status.Inventory.Entries {
 			u, err := unstructuredFromResourceRef(v)
@@ -874,12 +1044,14 @@ func deleteClusterDiscoveryAndInventory(t *testing.T, cl client.Client, cd *clus
 			if err := client.IgnoreNotFound(cl.Delete(ctx, u)); err != nil {
 				t.Errorf("failed to delete %v: %s", u, err)
 			}
+			assert.NoError(t, client.IgnoreNotFound(cl.Get(ctx, client.ObjectKeyFromObject(u), u)))
 		}
 	}
 
 	if err := cl.Delete(ctx, cd); err != nil {
 		t.Fatal(err)
 	}
+	assert.NoError(t, client.IgnoreNotFound(cl.Get(ctx, client.ObjectKeyFromObject(cd), cd)))
 }
 
 func assertAutomatedClusterDiscoveryCondition(t *testing.T, acd *clustersv1alpha1.AutomatedClusterDiscovery, condType, msg string) {
@@ -937,18 +1109,14 @@ func assertHasOwnerReference(t *testing.T, obj metav1.Object, ownerRef metav1.Ow
 }
 
 func assertHasLabels(t *testing.T, o client.Object, want map[string]string) {
+	t.Helper()
 	labels := o.GetLabels()
-	for k, v := range want {
-		kv, ok := labels[k]
-		if !ok {
-			t.Errorf("%s %s/%s is missing label %q with value %q", o.GetObjectKind().GroupVersionKind().Kind, o.GetNamespace(), o.GetName(), k, v)
-			continue
-		}
-		assert.Equal(t, v, kv)
-	}
+
+	assert.Equal(t, want, labels)
 }
 
 func assertHasAnnotations(t *testing.T, o client.Object, want map[string]string) {
+	t.Helper()
 	annotations := o.GetAnnotations()
 	for k, v := range want {
 		kv, ok := annotations[k]

diff --git a/pkg/providers/azure/azure.go b/pkg/providers/azure/azure.go
@@ -60,6 +60,7 @@ func (p *AzureProvider) ListClusters(ctx context.Context) ([]*providers.Provider
 				Name:       *aksCluster.Name,
 				ID:         *aksCluster.ID,
 				KubeConfig: kubeConfig,
+				Labels:     tagsToLabels(aksCluster.Tags),
 			})
 		}
 	}
@@ -146,3 +147,13 @@ func clientFactory(subscriptionID string) (AKSClusterClient, error) {
 
 	return client, nil
 }
+
+func tagsToLabels(src map[string]*string) map[string]string {
+	labels := map[string]string{}
+
+	for k, v := range src {
+		labels[k] = *v
+	}
+
+	return labels
+}
diff --git a/pkg/providers/azure/azure_test.go b/pkg/providers/azure/azure_test.go
@@ -78,6 +78,9 @@ func TestClusterProvider_ListClusters(t *testing.T) {
 			{
 				ID:   to.Ptr("/subscriptions/ace37984-3d07-4051-9002-d5a52c0ae14b/resourcegroups/team-pesto-use1/providers/Microsoft.ContainerService/managedClusters/pestomarketplacetest"),
 				Name: to.Ptr("cluster-1"),
+				Tags: map[string]*string{
+					"test-tag": to.Ptr("testing"),
+				},
 			},
 		},
 	}
@@ -96,6 +99,9 @@ func TestClusterProvider_ListClusters(t *testing.T) {
 		{
 			ID:   "/subscriptions/ace37984-3d07-4051-9002-d5a52c0ae14b/resourcegroups/team-pesto-use1/providers/Microsoft.ContainerService/managedClusters/pestomarketplacetest",
 			Name: "cluster-1",
+			Labels: map[string]string{
+				"test-tag": "testing",
+			},
 		},
 	}
 	if diff := cmp.Diff(want, provided); diff != "" {

diff --git a/pkg/providers/provider.go b/pkg/providers/provider.go
@@ -31,4 +31,10 @@ type ProviderCluster struct {
 	// This is likely to be a high privilege token.
 	// The token MUST NOT require the execution of a binary (ExecConfig).
 	KubeConfig *api.Config
+
+	// Labels are labels to be applied to the generated Cluster.
+	//
+	// The could be generated from Labels or Tags on the API representation of
+	// the source Cluster.
+	Labels map[string]string
 }