From d8c855facf03957eeb932a6b0b107b3f70657c0f Mon Sep 17 00:00:00 2001 From: Kevin McDermott Date: Mon, 11 Dec 2023 16:14:27 +0000 Subject: [PATCH] Verify configuration for the required type. This verifies that the config for `eks` or `aks` has been configured appropriately. --- .../automatedclusterdiscovery_controller.go | 16 +- ...tomatedclusterdiscovery_controller_test.go | 208 +++++++++--------- 2 files changed, 113 insertions(+), 111 deletions(-) diff --git a/internal/controller/automatedclusterdiscovery_controller.go b/internal/controller/automatedclusterdiscovery_controller.go index 4231d69..4fb9f1b 100644 --- a/internal/controller/automatedclusterdiscovery_controller.go +++ b/internal/controller/automatedclusterdiscovery_controller.go @@ -18,6 +18,7 @@ package controller import ( "context" + "errors" "fmt" "sort" "time" @@ -368,10 +369,21 @@ func (r *AutomatedClusterDiscoveryReconciler) reconcileClusters(ctx context.Cont return inventoryResources, nil } +// DefaultProviderFactory creates an appropriate factory for creating provider +// clients based on the spec of the AutomatedClusterDiscovery. func DefaultProviderFactory(acd *clustersv1alpha1.AutomatedClusterDiscovery) (providers.Provider, error) { - if acd.Spec.Type == "aks" { + switch acd.Spec.Type { + case "aks": + if acd.Spec.AKS == nil { + return nil, errors.New("discovery .spec.type = aks but no AKS configuration provided") + } + return azure.NewAzureProvider(acd.Spec.AKS.SubscriptionID), nil - } else if acd.Spec.Type == "eks" { + case "eks": + if acd.Spec.EKS == nil { + return nil, errors.New("discovery .spec.type = eks but no EKS configuration provided") + } + return aws.NewAWSProvider(acd.Spec.EKS.Region), nil } diff --git a/internal/controller/automatedclusterdiscovery_controller_test.go b/internal/controller/automatedclusterdiscovery_controller_test.go index d8d1c2f..c571abb 100644 --- a/internal/controller/automatedclusterdiscovery_controller_test.go +++ b/internal/controller/automatedclusterdiscovery_controller_test.go @@ -2,6 +2,7 @@ package controller import ( "context" + "fmt" "path/filepath" "sort" "testing" @@ -60,19 +61,8 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) { assert.NoError(t, err) t.Run("Reconcile with AKS", 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}, - }, - } + aksCluster := newAutomatedClusterDiscovery("test-aks", + aksProviderOption("subscription-123")) testProvider := stubProvider{ response: []*providers.ProviderCluster{ @@ -166,24 +156,13 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) { "test.example.com/annotation": "test", "example.com/test": "annotation", } - - 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}, - CommonLabels: map[string]string{ + aksCluster := newAutomatedClusterDiscovery("test-aks", + aksProviderOption("subscription-123"), + commonLabels( + map[string]string{ "example.com/label": "test", }, - CommonAnnotations: wantAnnotations, - }, - } + ), commonAnnotations(wantAnnotations)) testProvider := stubProvider{ response: []*providers.ProviderCluster{ @@ -250,19 +229,8 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) { }) 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}, - }, - } + aksCluster := newAutomatedClusterDiscovery("test-aks", + aksProviderOption("subscription-123")) testProvider := stubProvider{ response: []*providers.ProviderCluster{ @@ -333,20 +301,11 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) { }) 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}, - }, - } + aksCluster := newAutomatedClusterDiscovery("test-aks-disabled-tags", + aksProviderOption("subscription-123"), + func(a *clustersv1alpha1.AutomatedClusterDiscovery) { + a.Spec.DisableTags = true + }) testProvider := stubProvider{ response: []*providers.ProviderCluster{ @@ -415,19 +374,8 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) { }) t.Run("Reconcile when executing in cluster and cluster matches reflector 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}, - }, - } + aksCluster := newAutomatedClusterDiscovery("test-aks-disabled-tags", + aksProviderOption("subscription-123")) testClusterID := "/subscriptions/ace37984-aaaa-1234-1234-a1a12c0ae14b/resourcegroups/team-pesto-use1/providers/Microsoft.ContainerService/managedClusters/test-cluster" testProvider := stubProvider{ @@ -484,19 +432,8 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) { t.Run("Reconcile when cluster has been removed from AKS", func(t *testing.T) { ctx := context.TODO() - 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}, - }, - } + aksCluster := newAutomatedClusterDiscovery("test-aks-disabled-tags", + aksProviderOption("subscription-123")) err := k8sClient.Create(ctx, aksCluster) assert.NoError(t, err) @@ -555,19 +492,8 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) { t.Run("Reconcile updates Secret value for existing clusters", func(t *testing.T) { ctx := context.TODO() - 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}, - }, - } + aksCluster := newAutomatedClusterDiscovery("test-aks-disabled-tags", + aksProviderOption("subscription-123")) err := k8sClient.Create(ctx, aksCluster) assert.NoError(t, err) @@ -632,20 +558,11 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) { t.Run("Reconcile suspended cluster discovery resource", func(t *testing.T) { ctx := context.TODO() - 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}, - Suspend: true, - }, - } + aksCluster := newAutomatedClusterDiscovery("test-aks-disabled-tags", + aksProviderOption("subscription-123"), + func(a *clustersv1alpha1.AutomatedClusterDiscovery) { + a.Spec.Suspend = true + }) testProvider := stubProvider{ response: []*providers.ProviderCluster{ @@ -907,6 +824,40 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) { assert.Equal(t, "Cluster cluster-1 removed", mockEventRecorder.CapturedMessage) }) + t.Run("Reconcile with missing configuration for type", func(t *testing.T) { + reconciler := &AutomatedClusterDiscoveryReconciler{ + Client: k8sClient, + Scheme: scheme, + ProviderFactory: DefaultProviderFactory, + EventRecorder: &mockEventRecorder{}, + } + assert.NoError(t, reconciler.SetupWithManager(mgr)) + + typeTests := []struct { + discoveryType string + wantErr string + }{ + {"aks", "discovery .spec.type = aks but no AKS configuration provided"}, + {"eks", "discovery .spec.type = eks but no EKS configuration provided"}, + } + + for _, tt := range typeTests { + t.Run(fmt.Sprintf("type %s", tt.discoveryType), func(t *testing.T) { + aksCluster := newAutomatedClusterDiscovery("test-aks", + func(a *clustersv1alpha1.AutomatedClusterDiscovery) { + a.Spec.Type = tt.discoveryType + }) + + ctx := context.TODO() + err = k8sClient.Create(ctx, aksCluster) + assert.NoError(t, err) + defer deleteClusterDiscoveryAndInventory(t, k8sClient, aksCluster) + + _, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(aksCluster)}) + assert.ErrorContains(t, err, tt.wantErr) + }) + } + }) } func TestReconcilingWithAnnotationChange(t *testing.T) { @@ -1134,3 +1085,42 @@ func isOwnerReferenceEqual(a, b metav1.OwnerReference) bool { (a.Name == b.Name) && (a.UID == b.UID) } + +func aksProviderOption(subscriptionID string) func(*clustersv1alpha1.AutomatedClusterDiscovery) { + return func(acd *clustersv1alpha1.AutomatedClusterDiscovery) { + acd.Spec.Type = "aks" + acd.Spec.AKS = &clustersv1alpha1.AKS{ + SubscriptionID: subscriptionID, + } + } +} + +func commonLabels(labels map[string]string) func(*clustersv1alpha1.AutomatedClusterDiscovery) { + return func(acd *clustersv1alpha1.AutomatedClusterDiscovery) { + acd.Spec.CommonLabels = labels + } +} + +func commonAnnotations(annotations map[string]string) func(*clustersv1alpha1.AutomatedClusterDiscovery) { + return func(acd *clustersv1alpha1.AutomatedClusterDiscovery) { + acd.Spec.CommonAnnotations = annotations + } +} + +func newAutomatedClusterDiscovery(name string, opts ...func(*clustersv1alpha1.AutomatedClusterDiscovery)) *clustersv1alpha1.AutomatedClusterDiscovery { + discovery := &clustersv1alpha1.AutomatedClusterDiscovery{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "default", + }, + Spec: clustersv1alpha1.AutomatedClusterDiscoverySpec{ + Interval: metav1.Duration{Duration: time.Minute}, + }, + } + + for _, opt := range opts { + opt(discovery) + } + + return discovery +}