fix: add finalizer to credentials reference (#288)

* fix: linodecluster: add finalizer to credentials reference * fix: linodemachine: add finalizer to credentials reference * fix: linodevpc: add finalizer to credentials reference
linode · May 2, 2024 · e68e5ab · e68e5ab
1 parent 5180d4c
commit e68e5ab
Show file tree

Hide file tree

Showing 11 changed files with 1,030 additions and 6 deletions.
diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go
@@ -108,3 +108,23 @@ func (s *ClusterScope) AddFinalizer(ctx context.Context) error {
 
 	return nil
 }
+
+func (s *ClusterScope) AddCredentialsRefFinalizer(ctx context.Context) error {
+	if s.LinodeCluster.Spec.CredentialsRef == nil {
+		return nil
+	}
+
+	return addCredentialsFinalizer(ctx, s.Client,
+		*s.LinodeCluster.Spec.CredentialsRef, s.LinodeCluster.GetNamespace(),
+		toFinalizer(s.LinodeCluster))
+}
+
+func (s *ClusterScope) RemoveCredentialsRefFinalizer(ctx context.Context) error {
+	if s.LinodeCluster.Spec.CredentialsRef == nil {
+		return nil
+	}
+
+	return removeCredentialsFinalizer(ctx, s.Client,
+		*s.LinodeCluster.Spec.CredentialsRef, s.LinodeCluster.GetNamespace(),
+		toFinalizer(s.LinodeCluster))
+}
diff --git a/cloud/scope/cluster_test.go b/cloud/scope/cluster_test.go
@@ -326,3 +326,205 @@ func TestNewClusterScope(t *testing.T) {
 		})
 	}
 }
+
+func TestClusterAddCredentialsRefFinalizer(t *testing.T) {
+	t.Parallel()
+	type fields struct {
+		Cluster       *clusterv1.Cluster
+		LinodeCluster *infrav1alpha1.LinodeCluster
+	}
+
+	tests := []struct {
+		name    string
+		fields  fields
+		expects func(mock *mock.MockK8sClient)
+	}{
+		{
+			name: "Success - finalizer should be added to the Linode Cluster credentials Secret",
+			fields: fields{
+				Cluster: &clusterv1.Cluster{},
+				LinodeCluster: &infrav1alpha1.LinodeCluster{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-cluster",
+					},
+					Spec: infrav1alpha1.LinodeClusterSpec{
+						CredentialsRef: &corev1.SecretReference{
+							Name:      "example",
+							Namespace: "test",
+						},
+					},
+				},
+			},
+			expects: func(mock *mock.MockK8sClient) {
+				mock.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme {
+					s := runtime.NewScheme()
+					infrav1alpha1.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
+				}).Times(2)
+				mock.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil)
+			},
+		},
+		{
+			name: "No-op - no Linode Cluster credentials Secret",
+			fields: fields{
+				Cluster: &clusterv1.Cluster{},
+				LinodeCluster: &infrav1alpha1.LinodeCluster{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-cluster",
+					},
+				},
+			},
+			expects: func(mock *mock.MockK8sClient) {
+				mock.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme {
+					s := runtime.NewScheme()
+					infrav1alpha1.AddToScheme(s)
+					return s
+				})
+			},
+		},
+	}
+	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(),
+				"test-key",
+				ClusterScopeParams{
+					Cluster:       testcase.fields.Cluster,
+					LinodeCluster: testcase.fields.LinodeCluster,
+					Client:        mockK8sClient,
+				})
+			if err != nil {
+				t.Errorf("NewClusterScope() error = %v", err)
+			}
+
+			if err := cScope.AddCredentialsRefFinalizer(context.Background()); err != nil {
+				t.Errorf("ClusterScope.AddCredentialsRefFinalizer() error = %v", err)
+			}
+		})
+	}
+}
+
+func TestRemoveCredentialsRefFinalizer(t *testing.T) {
+	t.Parallel()
+	type fields struct {
+		Cluster       *clusterv1.Cluster
+		LinodeCluster *infrav1alpha1.LinodeCluster
+	}
+
+	tests := []struct {
+		name    string
+		fields  fields
+		expects func(mock *mock.MockK8sClient)
+	}{
+		{
+			name: "Success - finalizer should be removed from the Linode Cluster credentials Secret",
+			fields: fields{
+				Cluster: &clusterv1.Cluster{},
+				LinodeCluster: &infrav1alpha1.LinodeCluster{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-cluster",
+					},
+					Spec: infrav1alpha1.LinodeClusterSpec{
+						CredentialsRef: &corev1.SecretReference{
+							Name:      "example",
+							Namespace: "test",
+						},
+					},
+				},
+			},
+			expects: func(mock *mock.MockK8sClient) {
+				mock.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme {
+					s := runtime.NewScheme()
+					infrav1alpha1.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
+				}).Times(2)
+				mock.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil)
+			},
+		},
+		{
+			name: "No-op - no Linode Cluster credentials Secret",
+			fields: fields{
+				Cluster: &clusterv1.Cluster{},
+				LinodeCluster: &infrav1alpha1.LinodeCluster{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "test-cluster",
+					},
+				},
+			},
+			expects: func(mock *mock.MockK8sClient) {
+				mock.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme {
+					s := runtime.NewScheme()
+					infrav1alpha1.AddToScheme(s)
+					return s
+				})
+			},
+		},
+	}
+	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(),
+				"test-key",
+				ClusterScopeParams{
+					Cluster:       testcase.fields.Cluster,
+					LinodeCluster: testcase.fields.LinodeCluster,
+					Client:        mockK8sClient,
+				})
+			if err != nil {
+				t.Errorf("NewClusterScope() error = %v", err)
+			}
+
+			if err := cScope.RemoveCredentialsRefFinalizer(context.Background()); err != nil {
+				t.Errorf("ClusterScope.RemoveCredentialsRefFinalizer() error = %v", err)
+			}
+		})
+	}
+}
diff --git a/cloud/scope/common.go b/cloud/scope/common.go
@@ -5,11 +5,13 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"strings"
 
 	"github.com/linode/linodego"
 	"golang.org/x/oauth2"
 	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	"github.com/linode/cluster-api-provider-linode/version"
 )
@@ -34,6 +36,47 @@ func CreateLinodeClient(apiKey string) (*linodego.Client, error) {
 }
 
 func getCredentialDataFromRef(ctx context.Context, crClient K8sClient, credentialsRef corev1.SecretReference, defaultNamespace string) ([]byte, error) {
+	credSecret, err := getCredentials(ctx, crClient, credentialsRef, defaultNamespace)
+	if err != nil {
+		return nil, err
+	}
+
+	// TODO: This key is hard-coded (for now) to match the externally-managed `manager-credentials` Secret.
+	rawData, ok := credSecret.Data["apiToken"]
+	if !ok {
+		return nil, fmt.Errorf("no apiToken key in credentials secret %s/%s", credentialsRef.Namespace, credentialsRef.Name)
+	}
+
+	return rawData, nil
+}
+
+func addCredentialsFinalizer(ctx context.Context, crClient K8sClient, credentialsRef corev1.SecretReference, defaultNamespace, finalizer string) error {
+	secret, err := getCredentials(ctx, crClient, credentialsRef, defaultNamespace)
+	if err != nil {
+		return err
+	}
+
+	controllerutil.AddFinalizer(secret, finalizer)
+	if err := crClient.Update(ctx, secret); err != nil {
+		return fmt.Errorf("add finalizer to credentials secret %s/%s: %w", secret.Namespace, secret.Name, err)
+	}
+	return nil
+}
+
+func removeCredentialsFinalizer(ctx context.Context, crClient K8sClient, credentialsRef corev1.SecretReference, defaultNamespace, finalizer string) error {
+	secret, err := getCredentials(ctx, crClient, credentialsRef, defaultNamespace)
+	if err != nil {
+		return err
+	}
+
+	controllerutil.RemoveFinalizer(secret, finalizer)
+	if err := crClient.Update(ctx, secret); err != nil {
+		return fmt.Errorf("remove finalizer from credentials secret %s/%s: %w", secret.Namespace, secret.Name, err)
+	}
+	return nil
+}
+
+func getCredentials(ctx context.Context, crClient K8sClient, credentialsRef corev1.SecretReference, defaultNamespace string) (*corev1.Secret, error) {
 	secretRef := client.ObjectKey{
 		Name:      credentialsRef.Name,
 		Namespace: credentialsRef.Namespace,
@@ -47,11 +90,20 @@ func getCredentialDataFromRef(ctx context.Context, crClient K8sClient, credentia
 		return nil, fmt.Errorf("get credentials secret %s/%s: %w", secretRef.Namespace, secretRef.Name, err)
 	}
 
-	// TODO: This key is hard-coded (for now) to match the externally-managed `manager-credentials` Secret.
-	rawData, ok := credSecret.Data["apiToken"]
-	if !ok {
-		return nil, fmt.Errorf("no apiToken key in credentials secret %s/%s", secretRef.Namespace, secretRef.Name)
-	}
+	return &credSecret, nil
+}
 
-	return rawData, nil
+// toFinalizer converts an object into a valid finalizer key representation
+func toFinalizer(obj client.Object) string {
+	var (
+		gvk       = obj.GetObjectKind().GroupVersionKind()
+		group     = gvk.Group
+		kind      = strings.ToLower(gvk.Kind)
+		namespace = obj.GetNamespace()
+		name      = obj.GetName()
+	)
+	if namespace == "" {
+		return fmt.Sprintf("%s.%s/%s", kind, group, name)
+	}
+	return fmt.Sprintf("%s.%s/%s.%s", kind, group, namespace, name)
 }