From e68e5abbf2e9dc14d671ba45ad865839ff3515d4 Mon Sep 17 00:00:00 2001 From: cbzzz <69888673+cbzzz@users.noreply.github.com> Date: Thu, 2 May 2024 12:14:52 -0400 Subject: [PATCH] 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 --- cloud/scope/cluster.go | 20 ++ cloud/scope/cluster_test.go | 202 +++++++++++++++++ cloud/scope/common.go | 64 +++++- cloud/scope/common_test.go | 301 +++++++++++++++++++++++++ cloud/scope/machine.go | 22 ++ cloud/scope/machine_test.go | 187 +++++++++++++++ cloud/scope/vpc.go | 20 ++ cloud/scope/vpc_test.go | 180 +++++++++++++++ controller/linodecluster_controller.go | 15 ++ controller/linodemachine_controller.go | 15 ++ controller/linodevpc_controller.go | 10 + 11 files changed, 1030 insertions(+), 6 deletions(-) diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index 7769df967..a88c10dd8 100644 --- 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 index f85602a0a..861108949 100644 --- 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 index 53c06c2cd..3b014e622 100644 --- 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) } diff --git a/cloud/scope/common_test.go b/cloud/scope/common_test.go index 0b6f62ead..18c3f7566 100644 --- a/cloud/scope/common_test.go +++ b/cloud/scope/common_test.go @@ -8,9 +8,12 @@ import ( "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" "github.com/linode/cluster-api-provider-linode/mock" ) @@ -181,3 +184,301 @@ func TestGetCredentialDataFromRef(t *testing.T) { }) } } + +// Test_addCredentialsFinalizer tests the addCredentialsFinalizer function. +func Test_addCredentialsFinalizer(t *testing.T) { + t.Parallel() + + type clientBehavior struct { + Get func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error + Update func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error + } + + type args struct { + providedCredentialsRef corev1.SecretReference + clientBehavior clientBehavior + } + + tests := []struct { + name string + args args + expectedError string + }{ + { + name: "Testing functionality using valid/good data. No error should be returned", + args: args{ + providedCredentialsRef: corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + clientBehavior: clientBehavior{ + Get: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + cred := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "test", + }, + } + *obj = cred + + return nil + }, + Update: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return nil + }, + }, + }, + expectedError: "", + }, + { + name: "Handle error from crClient Get. Error should be returned.", + args: args{ + providedCredentialsRef: corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + clientBehavior: clientBehavior{ + Get: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + return errors.New("client get error") + }, + Update: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return nil + }, + }, + }, + expectedError: "get credentials secret test/example: client get error", + }, + { + name: "Handle error from crClient Update. Error should be returned.", + args: args{ + providedCredentialsRef: corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + clientBehavior: clientBehavior{ + Get: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + cred := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "test", + }, + } + *obj = cred + + return nil + }, + Update: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return errors.New("client update error") + }, + }, + }, + expectedError: "add finalizer to credentials secret test/example: client update error", + }, + } + + for _, tt := range tests { + testCase := tt + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // Create an instance of the mock K8sClient + mockClient := mock.NewMockK8sClient(ctrl) + + // Setup Expected behaviour + mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(testCase.args.clientBehavior.Get).AnyTimes() + mockClient.EXPECT().Update(gomock.Any(), gomock.Any()).DoAndReturn(testCase.args.clientBehavior.Update).AnyTimes() + + // Call addCredentialsFinalizer using the mock client + err := addCredentialsFinalizer(context.Background(), mockClient, testCase.args.providedCredentialsRef, "default", "test.test/test.test") + + // Check that the function returned the expected result + if testCase.expectedError != "" { + assert.EqualError(t, err, testCase.expectedError) + } else { + assert.NoError(t, err) + } + }) + } +} + +// Test_removeCredentialsFinalizer tests the removeCredentialsFinalizer function. +func Test_removeCredentialsFinalizer(t *testing.T) { + t.Parallel() + + type clientBehavior struct { + Get func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error + Update func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error + } + + type args struct { + providedCredentialsRef corev1.SecretReference + clientBehavior clientBehavior + } + + tests := []struct { + name string + args args + expectedError string + }{ + { + name: "Testing functionality using valid/good data. No error should be returned", + args: args{ + providedCredentialsRef: corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + clientBehavior: clientBehavior{ + Get: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + cred := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "test", + }, + } + *obj = cred + + return nil + }, + Update: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return nil + }, + }, + }, + expectedError: "", + }, + { + name: "Handle error from crClient Get. Error should be returned.", + args: args{ + providedCredentialsRef: corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + clientBehavior: clientBehavior{ + Get: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + return errors.New("client get error") + }, + Update: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return nil + }, + }, + }, + expectedError: "get credentials secret test/example: client get error", + }, + { + name: "Handle error from crClient Update. Error should be returned.", + args: args{ + providedCredentialsRef: corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + clientBehavior: clientBehavior{ + Get: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + cred := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "test", + }, + } + *obj = cred + + return nil + }, + Update: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return errors.New("client update error") + }, + }, + }, + expectedError: "remove finalizer from credentials secret test/example: client update error", + }, + } + + for _, tt := range tests { + testCase := tt + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // Create an instance of the mock K8sClient + mockClient := mock.NewMockK8sClient(ctrl) + + // Setup Expected behaviour + mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(testCase.args.clientBehavior.Get).AnyTimes() + mockClient.EXPECT().Update(gomock.Any(), gomock.Any()).DoAndReturn(testCase.args.clientBehavior.Update).AnyTimes() + + // Call removeCredentialsFinalizer using the mock client + err := removeCredentialsFinalizer(context.Background(), mockClient, testCase.args.providedCredentialsRef, "default", "test.test/test.test") + + // Check that the function returned the expected result + if testCase.expectedError != "" { + assert.EqualError(t, err, testCase.expectedError) + } else { + assert.NoError(t, err) + } + }) + } +} + +// Test_toFinalizer tests the toFinalizer function. +func Test_toFinalizer(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + gvk schema.GroupVersionKind + object client.Object + want string + }{ + { + "Namespaced Resources", + schema.GroupVersionKind{ + Group: infrav1alpha1.GroupVersion.Group, + Version: infrav1alpha1.GroupVersion.Version, + Kind: "LinodeCluster", + }, + &infrav1alpha1.LinodeCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "example", + }, + }, + "linodecluster.infrastructure.cluster.x-k8s.io/test.example", + }, + { + "Cluster Resources", + schema.GroupVersionKind{ + Group: infrav1alpha1.GroupVersion.Group, + Version: infrav1alpha1.GroupVersion.Version, + Kind: "LinodeCluster", + }, + &infrav1alpha1.LinodeCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + // NOTE: Fake a cluster resource by setting Namespace to the default value + // See: https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#Object + Namespace: "", + }, + }, + "linodecluster.infrastructure.cluster.x-k8s.io/example", + }, + } + + for _, tt := range tests { + testCase := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // Inject the unambiguous kind identity into the object + testCase.object.GetObjectKind().SetGroupVersionKind(testCase.gvk) + + got := toFinalizer(testCase.object) + if testCase.want != got { + assert.Equal(t, testCase.want, got) + } + }) + } +} diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index f05436f92..86e16c952 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -154,3 +154,25 @@ func (m *MachineScope) GetBootstrapData(ctx context.Context) ([]byte, error) { return value, nil } + +func (s *MachineScope) AddCredentialsRefFinalizer(ctx context.Context) error { + // Only add the finalizer if the machine has an override for the credentials reference + if s.LinodeMachine.Spec.CredentialsRef == nil { + return nil + } + + return addCredentialsFinalizer(ctx, s.Client, + *s.LinodeMachine.Spec.CredentialsRef, s.LinodeMachine.GetNamespace(), + toFinalizer(s.LinodeMachine)) +} + +func (s *MachineScope) RemoveCredentialsRefFinalizer(ctx context.Context) error { + // Only remove the finalizer if the machine has an override for the credentials reference + if s.LinodeMachine.Spec.CredentialsRef == nil { + return nil + } + + return removeCredentialsFinalizer(ctx, s.Client, + *s.LinodeMachine.Spec.CredentialsRef, s.LinodeMachine.GetNamespace(), + toFinalizer(s.LinodeMachine)) +} diff --git a/cloud/scope/machine_test.go b/cloud/scope/machine_test.go index c59951fa1..5b72e49cd 100644 --- a/cloud/scope/machine_test.go +++ b/cloud/scope/machine_test.go @@ -13,6 +13,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -393,3 +394,189 @@ func TestMachineScopeGetBootstrapData(t *testing.T) { }), ) } + +func TestMachineAddCredentialsRefFinalizer(t *testing.T) { + t.Parallel() + type fields struct { + LinodeMachine *infrav1alpha1.LinodeMachine + } + tests := []struct { + name string + fields fields + expects func(mock *mock.MockK8sClient) + }{ + { + "Success - finalizer should be added to the Linode Machine credentials Secret", + fields{ + LinodeMachine: &infrav1alpha1.LinodeMachine{ + Spec: infrav1alpha1.LinodeMachineSpec{ + CredentialsRef: &corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + }, + }, + }, + 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 Machine credentials Secret", + fields: fields{ + LinodeMachine: &infrav1alpha1.LinodeMachine{}, + }, + 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) + + mScope, err := NewMachineScope( + context.Background(), + "test-key", + MachineScopeParams{ + Client: mockK8sClient, + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + LinodeMachine: testcase.fields.LinodeMachine, + }, + ) + if err != nil { + t.Errorf("NewMachineScope() error = %v", err) + } + + if err := mScope.AddCredentialsRefFinalizer(context.Background()); err != nil { + t.Errorf("MachineScope.AddCredentialsRefFinalizer() error = %v", err) + } + }) + } +} + +func TestMachineRemoveCredentialsRefFinalizer(t *testing.T) { + t.Parallel() + type fields struct { + LinodeMachine *infrav1alpha1.LinodeMachine + } + tests := []struct { + name string + fields fields + expects func(mock *mock.MockK8sClient) + }{ + { + "Success - finalizer should be added to the Linode Machine credentials Secret", + fields{ + LinodeMachine: &infrav1alpha1.LinodeMachine{ + Spec: infrav1alpha1.LinodeMachineSpec{ + CredentialsRef: &corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + }, + }, + }, + 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 Machine credentials Secret", + fields: fields{ + LinodeMachine: &infrav1alpha1.LinodeMachine{}, + }, + 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) + + mScope, err := NewMachineScope( + context.Background(), + "test-key", + MachineScopeParams{ + Client: mockK8sClient, + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + LinodeMachine: testcase.fields.LinodeMachine, + }, + ) + if err != nil { + t.Errorf("NewMachineScope() error = %v", err) + } + + if err := mScope.RemoveCredentialsRefFinalizer(context.Background()); err != nil { + t.Errorf("MachineScope.RemoveCredentialsRefFinalizer() error = %v", err) + } + }) + } +} diff --git a/cloud/scope/vpc.go b/cloud/scope/vpc.go index 9db300a33..84dff7676 100644 --- a/cloud/scope/vpc.go +++ b/cloud/scope/vpc.go @@ -102,3 +102,23 @@ func (s *VPCScope) AddFinalizer(ctx context.Context) error { return nil } + +func (s *VPCScope) AddCredentialsRefFinalizer(ctx context.Context) error { + if s.LinodeVPC.Spec.CredentialsRef == nil { + return nil + } + + return addCredentialsFinalizer(ctx, s.Client, + *s.LinodeVPC.Spec.CredentialsRef, s.LinodeVPC.GetNamespace(), + toFinalizer(s.LinodeVPC)) +} + +func (s *VPCScope) RemoveCredentialsRefFinalizer(ctx context.Context) error { + if s.LinodeVPC.Spec.CredentialsRef == nil { + return nil + } + + return removeCredentialsFinalizer(ctx, s.Client, + *s.LinodeVPC.Spec.CredentialsRef, s.LinodeVPC.GetNamespace(), + toFinalizer(s.LinodeVPC)) +} diff --git a/cloud/scope/vpc_test.go b/cloud/scope/vpc_test.go index b1bfdc1ef..e95ad3b8e 100644 --- a/cloud/scope/vpc_test.go +++ b/cloud/scope/vpc_test.go @@ -279,3 +279,183 @@ func TestVPCScopeMethods(t *testing.T) { }) } } + +func TestVPCAddCredentialsRefFinalizer(t *testing.T) { + t.Parallel() + tests := []struct { + name string + LinodeVPC *infrav1alpha1.LinodeVPC + expects func(mock *mock.MockK8sClient) + }{ + { + name: "Success - finalizer should be added to the Linode VPC credentials Secret", + LinodeVPC: &infrav1alpha1.LinodeVPC{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vpc", + }, + Spec: infrav1alpha1.LinodeVPCSpec{ + 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", + LinodeVPC: &infrav1alpha1.LinodeVPC{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vpc", + }, + }, + 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) + + vScope, err := NewVPCScope( + context.Background(), + "test-key", + VPCScopeParams{ + Client: mockK8sClient, + LinodeVPC: testcase.LinodeVPC, + }, + ) + if err != nil { + t.Errorf("NewVPCScope() error = %v", err) + } + + if err := vScope.AddCredentialsRefFinalizer(context.Background()); err != nil { + t.Errorf("VPCScope.AddCredentialsRefFinalizer() error = %v", err) + } + }) + } +} + +func TestVPCRemoveCredentialsRefFinalizer(t *testing.T) { + t.Parallel() + tests := []struct { + name string + LinodeVPC *infrav1alpha1.LinodeVPC + expects func(mock *mock.MockK8sClient) + }{ + { + name: "Success - finalizer should be added to the Linode VPC credentials Secret", + LinodeVPC: &infrav1alpha1.LinodeVPC{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vpc", + }, + Spec: infrav1alpha1.LinodeVPCSpec{ + 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 VPC credentials Secret", + LinodeVPC: &infrav1alpha1.LinodeVPC{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vpc", + }, + }, + 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) + + vScope, err := NewVPCScope( + context.Background(), + "test-key", + VPCScopeParams{ + Client: mockK8sClient, + LinodeVPC: testcase.LinodeVPC, + }, + ) + if err != nil { + t.Errorf("NewVPCScope() error = %v", err) + } + + if err := vScope.RemoveCredentialsRefFinalizer(context.Background()); err != nil { + t.Errorf("VPCScope.RemoveCredentialsRefFinalizer() error = %v", err) + } + }) + } +} diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index 1b9763c3e..cf0a8094f 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -156,6 +156,11 @@ func setFailureReason(clusterScope *scope.ClusterScope, failureReason cerrs.Clus } func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { + if err := clusterScope.AddCredentialsRefFinalizer(ctx); err != nil { + setFailureReason(clusterScope, cerrs.CreateClusterError, err, r) + return err + } + linodeNB, err := services.CreateNodeBalancer(ctx, clusterScope, logger) if err != nil { setFailureReason(clusterScope, cerrs.CreateClusterError, err, r) @@ -191,6 +196,11 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo logger.Info("deleting cluster") if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == nil { logger.Info("NodeBalancer ID is missing, nothing to do") + + if err := clusterScope.RemoveCredentialsRefFinalizer(ctx); err != nil { + setFailureReason(clusterScope, cerrs.DeleteClusterError, err, r) + return err + } controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1alpha1.GroupVersion.String()) r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeWarning, "NodeBalancerIDMissing", "NodeBalancer ID is missing, nothing to do") @@ -207,6 +217,11 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = nil clusterScope.LinodeCluster.Spec.Network.NodeBalancerConfigID = nil + + if err := clusterScope.RemoveCredentialsRefFinalizer(ctx); err != nil { + setFailureReason(clusterScope, cerrs.DeleteClusterError, err, r) + return err + } controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1alpha1.GroupVersion.String()) return nil diff --git a/controller/linodemachine_controller.go b/controller/linodemachine_controller.go index 6454cadcc..713b87f1e 100644 --- a/controller/linodemachine_controller.go +++ b/controller/linodemachine_controller.go @@ -256,6 +256,11 @@ func (r *LinodeMachineReconciler) reconcileCreate( ) (ctrl.Result, error) { logger.Info("creating machine") + if err := machineScope.AddCredentialsRefFinalizer(ctx); err != nil { + logger.Error(err, "Failed to update credentials secret") + return ctrl.Result{}, err + } + tags := []string{machineScope.LinodeCluster.Name} listFilter := util.Filter{ @@ -629,6 +634,11 @@ func (r *LinodeMachineReconciler) reconcileDelete( if machineScope.LinodeMachine.Spec.InstanceID == nil { logger.Info("Machine ID is missing, nothing to do") + + if err := machineScope.RemoveCredentialsRefFinalizer(ctx); err != nil { + logger.Error(err, "Failed to update credentials secret") + return err + } controllerutil.RemoveFinalizer(machineScope.LinodeMachine, infrav1alpha1.GroupVersion.String()) return nil @@ -655,6 +665,11 @@ func (r *LinodeMachineReconciler) reconcileDelete( machineScope.LinodeMachine.Spec.ProviderID = nil machineScope.LinodeMachine.Spec.InstanceID = nil machineScope.LinodeMachine.Status.InstanceState = nil + + if err := machineScope.RemoveCredentialsRefFinalizer(ctx); err != nil { + logger.Error(err, "Failed to update credentials secret") + return err + } controllerutil.RemoveFinalizer(machineScope.LinodeMachine, infrav1alpha1.GroupVersion.String()) return nil diff --git a/controller/linodevpc_controller.go b/controller/linodevpc_controller.go index 0052e67b8..e80424dc4 100644 --- a/controller/linodevpc_controller.go +++ b/controller/linodevpc_controller.go @@ -175,6 +175,11 @@ func (r *LinodeVPCReconciler) reconcile( func (r *LinodeVPCReconciler) reconcileCreate(ctx context.Context, logger logr.Logger, vpcScope *scope.VPCScope) error { logger.Info("creating vpc") + if err := vpcScope.AddCredentialsRefFinalizer(ctx); err != nil { + logger.Error(err, "Failed to update credentials secret") + return err + } + if err := r.reconcileVPC(ctx, vpcScope, logger); err != nil { logger.Error(err, "Failed to create VPC") @@ -260,6 +265,11 @@ func (r *LinodeVPCReconciler) reconcileDelete(ctx context.Context, logger logr.L r.Recorder.Event(vpcScope.LinodeVPC, corev1.EventTypeNormal, clusterv1.DeletedReason, "VPC has cleaned up") vpcScope.LinodeVPC.Spec.VPCID = nil + + if err := vpcScope.RemoveCredentialsRefFinalizer(ctx); err != nil { + logger.Error(err, "Failed to update credentials secret") + return res, err + } controllerutil.RemoveFinalizer(vpcScope.LinodeVPC, infrav1alpha1.GroupVersion.String()) return res, nil