[fix] create scope with default credentials for all resources (#507)

* updating scope for controllers * not mandating adding credential ref in Spec for LinodeMachine templates * adding testcases in vpc scope testing * adding testcases in pg scope testing * adding testcases for the new scope method for all resources * updating machine scope and added testcases * handling cyclomatic complexity * handling cyclomatic complexity for all controllers * making sure the update token is places right
linode · Sep 23, 2024 · 21b8cf2 · 21b8cf2
1 parent 17568b5
commit 21b8cf2
Show file tree

Hide file tree

Showing 20 changed files with 870 additions and 466 deletions.
diff --git a/clients/clients.go b/clients/clients.go
@@ -19,6 +19,7 @@ type LinodeClient interface {
 	LinodeDNSClient
 	LinodePlacementGroupClient
 	LinodeFirewallClient
+	LinodeTokenClient
 }
 
 type AkamClient interface {
@@ -118,3 +119,7 @@ type LinodeFirewallClient interface {
 type K8sClient interface {
 	client.Client
 }
+
+type LinodeTokenClient interface {
+	SetToken(token string) *linodego.Client
+}
diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go
@@ -56,20 +56,6 @@ func NewClusterScope(ctx context.Context, linodeClientConfig, dnsClientConfig Cl
 		return nil, err
 	}
 
-	// Override the controller credentials with ones from the Cluster's Secret reference (if supplied).
-	if params.LinodeCluster.Spec.CredentialsRef != nil {
-		// TODO: This key is hard-coded (for now) to match the externally-managed `manager-credentials` Secret.
-		apiToken, err := getCredentialDataFromRef(ctx, params.Client, *params.LinodeCluster.Spec.CredentialsRef, params.LinodeCluster.GetNamespace(), "apiToken")
-		if err != nil {
-			return nil, fmt.Errorf("credentials from secret ref: %w", err)
-		}
-		linodeClientConfig.Token = string(apiToken)
-		dnsToken, err := getCredentialDataFromRef(ctx, params.Client, *params.LinodeCluster.Spec.CredentialsRef, params.LinodeCluster.GetNamespace(), "dnsToken")
-		if err != nil || len(dnsToken) == 0 {
-			dnsToken = apiToken
-		}
-		dnsClientConfig.Token = string(dnsToken)
-	}
 	linodeClient, err := CreateLinodeClient(linodeClientConfig)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create linode client: %w", err)
@@ -152,3 +138,20 @@ func (s *ClusterScope) RemoveCredentialsRefFinalizer(ctx context.Context) error
 		*s.LinodeCluster.Spec.CredentialsRef, s.LinodeCluster.GetNamespace(),
 		toFinalizer(s.LinodeCluster))
 }
+
+func (s *ClusterScope) SetCredentialRefTokenForLinodeClients(ctx context.Context) error {
+	if s.LinodeCluster.Spec.CredentialsRef != nil {
+		apiToken, err := getCredentialDataFromRef(ctx, s.Client, *s.LinodeCluster.Spec.CredentialsRef, s.LinodeCluster.GetNamespace(), "apiToken")
+		if err != nil {
+			return fmt.Errorf("credentials from secret ref: %w", err)
+		}
+		s.LinodeClient = s.LinodeClient.SetToken(string(apiToken))
+		dnsToken, err := getCredentialDataFromRef(ctx, s.Client, *s.LinodeCluster.Spec.CredentialsRef, s.LinodeCluster.GetNamespace(), "dnsToken")
+		if err != nil || len(dnsToken) == 0 {
+			dnsToken = apiToken
+		}
+		s.LinodeDomainsClient = s.LinodeDomainsClient.SetToken(string(dnsToken))
+		return nil
+	}
+	return nil
+}
diff --git a/cloud/scope/cluster_test.go b/cloud/scope/cluster_test.go
@@ -214,51 +214,6 @@ func TestNewClusterScope(t *testing.T) {
 				})
 			},
 		},
-		{
-			name: "Success - Validate getCredentialDataFromRef() returns some apiKey data and we create a valid ClusterScope",
-			args: args{
-				apiKey:    "test-key",
-				dnsApiKey: "test-key",
-				params: ClusterScopeParams{
-					Client:  nil,
-					Cluster: &clusterv1.Cluster{},
-					LinodeCluster: &infrav1alpha2.LinodeCluster{
-						Spec: infrav1alpha2.LinodeClusterSpec{
-							CredentialsRef: &corev1.SecretReference{
-								Name:      "example",
-								Namespace: "test",
-							},
-						},
-					},
-				},
-			},
-			expectedError: nil,
-			expects: func(mock *mock.MockK8sClient) {
-				mock.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme {
-					s := runtime.NewScheme()
-					infrav1alpha2.AddToScheme(s)
-					return s
-				}).AnyTimes()
-				mock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
-					cred := corev1.Secret{
-						Data: map[string][]byte{
-							"apiToken": []byte("example"),
-						},
-					}
-					*obj = cred
-					return nil
-				})
-				mock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
-					cred := corev1.Secret{
-						Data: map[string][]byte{
-							"dnsToken": []byte("example"),
-						},
-					}
-					*obj = cred
-					return nil
-				})
-			},
-		},
 		{
 			name: "Error - ValidateClusterScopeParams triggers error because ClusterScopeParams is empty",
 			args: args{
@@ -283,29 +238,6 @@ func TestNewClusterScope(t *testing.T) {
 				mock.EXPECT().Scheme().Return(runtime.NewScheme())
 			},
 		},
-		{
-			name: "Error - Using getCredentialDataFromRef(), func returns an error. Unable to create a valid ClusterScope",
-			args: args{
-				apiKey:    "test-key",
-				dnsApiKey: "test-key",
-				params: ClusterScopeParams{
-					Client:  nil,
-					Cluster: &clusterv1.Cluster{},
-					LinodeCluster: &infrav1alpha2.LinodeCluster{
-						Spec: infrav1alpha2.LinodeClusterSpec{
-							CredentialsRef: &corev1.SecretReference{
-								Name:      "example",
-								Namespace: "test",
-							},
-						},
-					},
-				},
-			},
-			expectedError: fmt.Errorf("credentials from secret ref: get credentials secret test/example: failed to get secret"),
-			expects: func(mock *mock.MockK8sClient) {
-				mock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("failed to get secret"))
-			},
-		},
 		{
 			name: "Error - createLinodeCluster throws an error for passing empty apiKey. Unable to create a valid ClusterScope",
 			args: args{
@@ -451,3 +383,118 @@ func TestRemoveCredentialsRefFinalizer(t *testing.T) {
 		})
 	}
 }
+
+func TestClusterSetCredentialRefTokenForLinodeClients(t *testing.T) {
+	t.Parallel()
+	type fields struct {
+		Cluster           *clusterv1.Cluster
+		LinodeCluster     *infrav1alpha2.LinodeCluster
+		LinodeMachineList infrav1alpha2.LinodeMachineList
+	}
+
+	tests := []struct {
+		name          string
+		fields        fields
+		expects       func(mock *mock.MockK8sClient)
+		expectedError error
+	}{
+		{
+			name: "Success - Validate getCredentialDataFromRef() returns some apiKey data",
+			fields: fields{
+				Cluster:           &clusterv1.Cluster{},
+				LinodeMachineList: infrav1alpha2.LinodeMachineList{},
+				LinodeCluster: &infrav1alpha2.LinodeCluster{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-cluster",
+					},
+					Spec: infrav1alpha2.LinodeClusterSpec{
+						CredentialsRef: &corev1.SecretReference{
+							Name:      "example",
+							Namespace: "test",
+						},
+					},
+				},
+			},
+			expectedError: nil,
+			expects: func(mock *mock.MockK8sClient) {
+				mock.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme {
+					s := runtime.NewScheme()
+					infrav1alpha2.AddToScheme(s)
+					return s
+				})
+				mock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
+					cred := corev1.Secret{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "example",
+							Namespace: "test",
+						},
+						Data: map[string][]byte{
+							"apiToken": []byte("example"),
+						},
+					}
+					*obj = cred
+
+					return nil
+				}).AnyTimes()
+			},
+		},
+		{
+			name: "Error -  error when getting the credentials secret",
+			fields: fields{
+				Cluster:           &clusterv1.Cluster{},
+				LinodeMachineList: infrav1alpha2.LinodeMachineList{},
+				LinodeCluster: &infrav1alpha2.LinodeCluster{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-cluster",
+					},
+					Spec: infrav1alpha2.LinodeClusterSpec{
+						CredentialsRef: &corev1.SecretReference{
+							Name:      "example",
+							Namespace: "test",
+						},
+					},
+				},
+			},
+			expectedError: fmt.Errorf("credentials from secret ref: get credentials secret test/example: test error"),
+			expects: func(mock *mock.MockK8sClient) {
+				mock.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme {
+					s := runtime.NewScheme()
+					infrav1alpha2.AddToScheme(s)
+					return s
+				})
+				mock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("test error"))
+			},
+		},
+	}
+	for _, tt := range tests {
+		testcase := tt
+		t.Run(testcase.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			mockK8sClient := mock.NewMockK8sClient(ctrl)
+
+			testcase.expects(mockK8sClient)
+
+			cScope, err := NewClusterScope(
+				context.Background(),
+				ClientConfig{Token: "test-key"},
+				ClientConfig{Token: "test-key"},
+				ClusterScopeParams{
+					Cluster:           testcase.fields.Cluster,
+					LinodeCluster:     testcase.fields.LinodeCluster,
+					LinodeMachineList: testcase.fields.LinodeMachineList,
+					Client:            mockK8sClient,
+				})
+			if err != nil {
+				t.Errorf("NewClusterScope() error = %v", err)
+			}
+
+			if err := cScope.SetCredentialRefTokenForLinodeClients(context.Background()); err != nil {
+				assert.ErrorContains(t, err, testcase.expectedError.Error())
+			}
+		})
+	}
+}
diff --git a/cloud/scope/firewall.go b/cloud/scope/firewall.go
@@ -57,16 +57,6 @@ func NewFirewallScope(ctx context.Context, linodeClientConfig ClientConfig, para
 	if err := validateFirewallScopeParams(params); err != nil {
 		return nil, err
 	}
-
-	// Override the controller credentials with ones from the Firewall's Secret reference (if supplied).
-	if params.LinodeFirewall.Spec.CredentialsRef != nil {
-		// TODO: This key is hard-coded (for now) to match the externally-managed `manager-credentials` Secret.
-		apiToken, err := getCredentialDataFromRef(ctx, params.Client, *params.LinodeFirewall.Spec.CredentialsRef, params.LinodeFirewall.GetNamespace(), "apiToken")
-		if err != nil {
-			return nil, fmt.Errorf("credentials from secret ref: %w", err)
-		}
-		linodeClientConfig.Token = string(apiToken)
-	}
 	linodeClient, err := CreateLinodeClient(linodeClientConfig,
 		WithRetryCount(0),
 	)
@@ -126,3 +116,16 @@ func (s *FirewallScope) RemoveCredentialsRefFinalizer(ctx context.Context) error
 		*s.LinodeFirewall.Spec.CredentialsRef, s.LinodeFirewall.GetNamespace(),
 		toFinalizer(s.LinodeFirewall))
 }
+
+func (s *FirewallScope) SetCredentialRefTokenForLinodeClients(ctx context.Context) error {
+	if s.LinodeFirewall.Spec.CredentialsRef != nil {
+		// TODO: This key is hard-coded (for now) to match the externally-managed `manager-credentials` Secret.
+		apiToken, err := getCredentialDataFromRef(ctx, s.Client, *s.LinodeFirewall.Spec.CredentialsRef, s.LinodeFirewall.GetNamespace(), "apiToken")
+		if err != nil {
+			return fmt.Errorf("credentials from secret ref: %w", err)
+		}
+		s.LinodeClient = s.LinodeClient.SetToken(string(apiToken))
+		return nil
+	}
+	return nil
+}