From e2687cce6ef7d497c47bdfad79d8efbe1edb5e6e Mon Sep 17 00:00:00 2001 From: Kevin McDermott Date: Wed, 22 Nov 2023 16:27:32 +0000 Subject: [PATCH] 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. --- .../automatedclusterdiscovery_types.go | 7 +- ...ave.works_automatedclusterdiscoveries.yaml | 6 +- .../automatedclusterdiscovery_controller.go | 9 +- ...tomatedclusterdiscovery_controller_test.go | 194 ++++++++++++++++-- pkg/providers/azure/azure.go | 11 + pkg/providers/azure/azure_test.go | 6 + pkg/providers/provider.go | 6 + 7 files changed, 222 insertions(+), 17 deletions(-) diff --git a/api/v1alpha1/automatedclusterdiscovery_types.go b/api/v1alpha1/automatedclusterdiscovery_types.go index c009816..05f8038 100644 --- 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 index d867d6c..18772e9 100644 --- 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 index 2c2c56d..c096ec6 100644 --- 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 index a395a6c..b397a14 100644 --- 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,10 +232,10 @@ 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{ @@ -243,12 +243,177 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) { })) 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 index 2937cad..8a45f05 100644 --- 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 index 27497db..b809cb9 100644 --- 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 index 94e4552..30af949 100644 --- 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 }