diff --git a/clients/clients.go b/clients/clients.go index bba1801c1..fbf4086a0 100644 --- 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 index b9ccb1c78..8ac7fc719 100644 --- 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 index 9e969ff57..72f0fb531 100644 --- 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 index 30dede521..44b3b035b 100644 --- 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 +} diff --git a/cloud/scope/firewall_test.go b/cloud/scope/firewall_test.go index c201db6fb..12031330b 100644 --- a/cloud/scope/firewall_test.go +++ b/cloud/scope/firewall_test.go @@ -94,39 +94,6 @@ func TestNewFirewallScope(t *testing.T) { }) }, }, - { - name: "Success - Validate getCredentialDataFromRef() returns some apiKey data and we create a valid FirewallScope", - args: args{ - apiKey: "test-key", - params: FirewallScopeParams{ - LinodeFirewall: &infrav1alpha2.LinodeFirewall{ - Spec: infrav1alpha2.LinodeFirewallSpec{ - CredentialsRef: &corev1.SecretReference{ - Namespace: "test-namespace", - Name: "test-name", - }, - }, - }, - }, - }, - 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{ - Data: map[string][]byte{ - "apiToken": []byte("example-api-token"), - }, - } - *obj = cred - return nil - }) - }, - }, { name: "Error - Pass in invalid args and get an error", args: args{ @@ -136,26 +103,6 @@ func TestNewFirewallScope(t *testing.T) { expects: func(mock *mock.MockK8sClient) {}, expectedError: fmt.Errorf("linodeFirewall is required when creating a FirewallScope"), }, - { - name: "Error - Pass in valid args but get an error when getting the credentials secret", - args: args{ - apiKey: "test-key", - params: FirewallScopeParams{ - LinodeFirewall: &infrav1alpha2.LinodeFirewall{ - Spec: infrav1alpha2.LinodeFirewallSpec{ - CredentialsRef: &corev1.SecretReference{ - Namespace: "test-namespace", - Name: "test-name", - }, - }, - }, - }, - }, - expects: func(mock *mock.MockK8sClient) { - mock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("test error")) - }, - expectedError: fmt.Errorf("credentials from secret ref: get credentials secret test-namespace/test-name: test error"), - }, { name: "Error - Pass in valid args but get an error when creating a new linode client", args: args{ @@ -319,7 +266,7 @@ func TestFirewallAddCredentialsRefFinalizer(t *testing.T) { *obj = cred return nil - }).Times(2) + }).Times(1) mock.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) }, }, @@ -459,3 +406,100 @@ func TestFirewallRemoveCredentialsRefFinalizer(t *testing.T) { }) } } +func TestFirewallSetCredentialRefTokenForLinodeClients(t *testing.T) { + t.Parallel() + tests := []struct { + name string + LinodeFirewall *infrav1alpha2.LinodeFirewall + expects func(mock *mock.MockK8sClient) + expectedError error + }{ + { + name: "Success - Set credential Ref finalizer ", + LinodeFirewall: &infrav1alpha2.LinodeFirewall{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-fw", + }, + Spec: infrav1alpha2.LinodeFirewallSpec{ + 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 - Get an error when getting the credentials secret", + LinodeFirewall: &infrav1alpha2.LinodeFirewall{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-fw", + }, + Spec: infrav1alpha2.LinodeFirewallSpec{ + CredentialsRef: &corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + }, + }, + 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")) + }, + expectedError: fmt.Errorf("credentials from secret ref: get credentials secret test/example: 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) + + pgScope, err := NewFirewallScope( + context.Background(), + ClientConfig{Token: "test-key"}, + FirewallScopeParams{ + Client: mockK8sClient, + LinodeFirewall: testcase.LinodeFirewall, + }, + ) + if err != nil { + t.Errorf("NewFirewallScope() error = %v", err) + } + + if err := pgScope.SetCredentialRefTokenForLinodeClients(context.Background()); err != nil { + assert.ErrorContains(t, err, testcase.expectedError.Error()) + } + }) + } +} diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index a6d82133d..5efecc3a8 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -55,36 +55,6 @@ func NewMachineScope(ctx context.Context, linodeClientConfig ClientConfig, param if err := validateMachineScopeParams(params); err != nil { return nil, err } - - // Override the controller credentials with ones from the Machine's Secret reference (if supplied). - // Credentials will be used in the following order: - // 1. LinodeMachine - // 2. Owner LinodeCluster - // 3. Controller - var ( - credentialRef *corev1.SecretReference - defaultNamespace string - ) - switch { - case params.LinodeMachine.Spec.CredentialsRef != nil: - credentialRef = params.LinodeMachine.Spec.CredentialsRef - defaultNamespace = params.LinodeMachine.GetNamespace() - case params.LinodeCluster.Spec.CredentialsRef != nil: - credentialRef = params.LinodeCluster.Spec.CredentialsRef - defaultNamespace = params.LinodeCluster.GetNamespace() - default: - // Use default (controller) credentials - } - - if credentialRef != nil { - // TODO: This key is hard-coded (for now) to match the externally-managed `manager-credentials` Secret. - apiToken, err := getCredentialDataFromRef(ctx, params.Client, *credentialRef, defaultNamespace, "apiToken") - if err != nil { - return nil, fmt.Errorf("credentials from secret ref: %w", err) - } - linodeClientConfig.Token = string(apiToken) - } - linodeClient, err := CreateLinodeClient(linodeClientConfig, WithRetryCount(0), ) @@ -180,3 +150,25 @@ func (s *MachineScope) RemoveCredentialsRefFinalizer(ctx context.Context) error *s.LinodeMachine.Spec.CredentialsRef, s.LinodeMachine.GetNamespace(), toFinalizer(s.LinodeMachine)) } + +func (s *MachineScope) SetCredentialRefTokenForLinodeClients(ctx context.Context) error { + var ( + credentialRef *corev1.SecretReference + defaultNamespace string + ) + switch { + case s.LinodeMachine.Spec.CredentialsRef != nil: + credentialRef = s.LinodeMachine.Spec.CredentialsRef + defaultNamespace = s.LinodeMachine.GetNamespace() + default: + credentialRef = s.LinodeCluster.Spec.CredentialsRef + defaultNamespace = s.LinodeCluster.GetNamespace() + } + // TODO: This key is hard-coded (for now) to match the externally-managed `manager-credentials` Secret. + apiToken, err := getCredentialDataFromRef(ctx, s.Client, *credentialRef, defaultNamespace, "apiToken") + if err != nil { + return fmt.Errorf("credentials from secret ref: %w", err) + } + s.LinodeClient = s.LinodeClient.SetToken(string(apiToken)) + return nil +} diff --git a/cloud/scope/machine_test.go b/cloud/scope/machine_test.go index 4241c982d..b8b1b6415 100644 --- a/cloud/scope/machine_test.go +++ b/cloud/scope/machine_test.go @@ -3,6 +3,7 @@ package scope import ( "context" "errors" + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -202,58 +203,38 @@ func TestNewMachineScope(t *testing.T) { NewSuite(t, mock.MockK8sClient{}).Run( OneOf( - Path(Result("invalid params", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope( - ctx, - ClientConfig{Token: "apiToken"}, - MachineScopeParams{}, - ) - require.ErrorContains(t, err, "is required") - assert.Nil(t, mScope) - })), - Path(Result("no token", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope(ctx, ClientConfig{Token: ""}, MachineScopeParams{ - Client: mck.K8sClient, - Cluster: &clusterv1.Cluster{}, - Machine: &clusterv1.Machine{}, - LinodeCluster: &infrav1alpha2.LinodeCluster{}, - LinodeMachine: &infrav1alpha2.LinodeMachine{}, - }) - require.ErrorContains(t, err, "failed to create linode client") - assert.Nil(t, mScope) - })), Path( - Call("no secret", func(ctx context.Context, mck Mock) { - mck.K8sClient.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "example")) - }), - Result("error", func(ctx context.Context, mck Mock) { + Result("invalid params", func(ctx context.Context, mck Mock) { + mScope, err := NewMachineScope( + ctx, + ClientConfig{Token: "apiToken"}, + MachineScopeParams{}, + ) + require.ErrorContains(t, err, "is required") + assert.Nil(t, mScope) + })), + Path( + Result("no token", func(ctx context.Context, mck Mock) { mScope, err := NewMachineScope(ctx, ClientConfig{Token: ""}, MachineScopeParams{ Client: mck.K8sClient, Cluster: &clusterv1.Cluster{}, Machine: &clusterv1.Machine{}, LinodeCluster: &infrav1alpha2.LinodeCluster{}, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - Spec: infrav1alpha2.LinodeMachineSpec{ - CredentialsRef: &corev1.SecretReference{ - Name: "example", - Namespace: "test", - }, - }, - }, + LinodeMachine: &infrav1alpha2.LinodeMachine{}, }) - require.ErrorContains(t, err, "credentials from secret ref") + require.ErrorContains(t, err, "failed to create linode client") assert.Nil(t, mScope) - }), - ), + })), ), OneOf( - Path(Call("valid scheme", func(ctx context.Context, mck Mock) { - mck.K8sClient.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme { - s := runtime.NewScheme() - infrav1alpha2.AddToScheme(s) - return s - }).AnyTimes() - })), + Path( + Call("valid scheme", func(ctx context.Context, mck Mock) { + mck.K8sClient.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme { + s := runtime.NewScheme() + infrav1alpha2.AddToScheme(s) + return s + }).AnyTimes() + })), Path( Call("invalid scheme", func(ctx context.Context, mck Mock) { mck.K8sClient.EXPECT().Scheme().Return(runtime.NewScheme()).AnyTimes() @@ -272,18 +253,7 @@ func TestNewMachineScope(t *testing.T) { ), ), OneOf( - Path(Call("credentials in secret", func(ctx context.Context, mck Mock) { - mck.K8sClient.EXPECT().Get(ctx, gomock.Any(), gomock.Any()). - DoAndReturn(func(ctx context.Context, key client.ObjectKey, obj *corev1.Secret, opts ...client.GetOption) error { - *obj = corev1.Secret{ - Data: map[string][]byte{ - "apiToken": []byte("apiToken"), - }, - } - return nil - }).AnyTimes() - })), - Path(Result("default credentials", func(ctx context.Context, mck Mock) { + Path(Result("default credentials used", func(ctx context.Context, mck Mock) { mScope, err := NewMachineScope(ctx, ClientConfig{Token: "apiToken"}, MachineScopeParams{ Client: mck.K8sClient, Cluster: &clusterv1.Cluster{}, @@ -295,44 +265,6 @@ func TestNewMachineScope(t *testing.T) { assert.NotNil(t, mScope) })), ), - OneOf( - Path(Result("credentials from LinodeMachine credentialsRef", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope(ctx, ClientConfig{Token: ""}, MachineScopeParams{ - Client: mck.K8sClient, - Cluster: &clusterv1.Cluster{}, - Machine: &clusterv1.Machine{}, - LinodeCluster: &infrav1alpha2.LinodeCluster{}, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - Spec: infrav1alpha2.LinodeMachineSpec{ - CredentialsRef: &corev1.SecretReference{ - Name: "example", - Namespace: "test", - }, - }, - }, - }) - require.NoError(t, err) - assert.NotNil(t, mScope) - })), - Path(Result("credentials from LinodeCluster credentialsRef", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope(ctx, ClientConfig{Token: "apiToken"}, MachineScopeParams{ - Client: mck.K8sClient, - Cluster: &clusterv1.Cluster{}, - Machine: &clusterv1.Machine{}, - LinodeCluster: &infrav1alpha2.LinodeCluster{ - Spec: infrav1alpha2.LinodeClusterSpec{ - CredentialsRef: &corev1.SecretReference{ - Name: "example", - Namespace: "test", - }, - }, - }, - LinodeMachine: &infrav1alpha2.LinodeMachine{}, - }) - require.NoError(t, err) - assert.NotNil(t, mScope) - })), - ), ) } @@ -594,3 +526,168 @@ func TestMachineRemoveCredentialsRefFinalizer(t *testing.T) { }) } } + +func TestMachineSetCredentialRefTokenForLinodeClients(t *testing.T) { + t.Parallel() + type fields struct { + LinodeMachine *infrav1alpha2.LinodeMachine + LinodeCluster *infrav1alpha2.LinodeCluster + } + tests := []struct { + name string + fields fields + expects func(mock *mock.MockK8sClient) + expectedError error + }{ + { + name: "Success - Using LinodeMachine.Spec.CredentialsRef", + fields: fields{ + LinodeMachine: &infrav1alpha2.LinodeMachine{ + Spec: infrav1alpha2.LinodeMachineSpec{ + CredentialsRef: &corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + }, + }, + LinodeCluster: &infrav1alpha2.LinodeCluster{}, + }, + 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()).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 getting Linode Machine credentials Secret", + fields: fields{ + LinodeMachine: &infrav1alpha2.LinodeMachine{ + Spec: infrav1alpha2.LinodeMachineSpec{ + CredentialsRef: &corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + }, + }, + LinodeCluster: &infrav1alpha2.LinodeCluster{}, + }, + 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 + }).AnyTimes() + mock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("test error")) + }, + }, + { + name: "Success - Using LinodeCluster.Spec.CredentialsRef", + fields: fields{ + LinodeCluster: &infrav1alpha2.LinodeCluster{ + Spec: infrav1alpha2.LinodeClusterSpec{ + CredentialsRef: &corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + }, + }, + LinodeMachine: &infrav1alpha2.LinodeMachine{}, + }, + 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()).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 getting Linode Cluster credentials Secret", + fields: fields{ + LinodeCluster: &infrav1alpha2.LinodeCluster{ + Spec: infrav1alpha2.LinodeClusterSpec{ + CredentialsRef: &corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + }, + }, + LinodeMachine: &infrav1alpha2.LinodeMachine{}, + }, + 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 + }).AnyTimes() + 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) + + mScope, err := NewMachineScope( + context.Background(), + ClientConfig{Token: "apiToken"}, + MachineScopeParams{ + Client: mockK8sClient, + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{}, + LinodeCluster: testcase.fields.LinodeCluster, + LinodeMachine: testcase.fields.LinodeMachine, + }, + ) + if err != nil { + t.Errorf("NewMachineScope() error = %v", err) + } + + if err := mScope.SetCredentialRefTokenForLinodeClients(context.Background()); err != nil { + assert.ErrorContains(t, err, testcase.expectedError.Error()) + } + }) + } +} diff --git a/cloud/scope/object_storage_key.go b/cloud/scope/object_storage_key.go index 7bbd248ed..1d0e7dbb9 100644 --- a/cloud/scope/object_storage_key.go +++ b/cloud/scope/object_storage_key.go @@ -49,16 +49,6 @@ func NewObjectStorageKeyScope(ctx context.Context, linodeClientConfig ClientConf if err := validateObjectStorageKeyScopeParams(params); err != nil { return nil, err } - - // Override the controller credentials with ones from the Cluster's Secret reference (if supplied). - if params.Key.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.Key.Spec.CredentialsRef, params.Key.GetNamespace(), "apiToken") - if err != nil || len(apiToken) == 0 { - return nil, fmt.Errorf("credentials from secret ref: %w", err) - } - linodeClientConfig.Token = string(apiToken) - } linodeClientConfig.Timeout = clientTimeout linodeClient, err := CreateLinodeClient(linodeClientConfig) if err != nil { @@ -177,3 +167,17 @@ func (s *ObjectStorageKeyScope) ShouldRotateKey() bool { return s.Key.Status.LastKeyGeneration != nil && s.Key.Spec.KeyGeneration != *s.Key.Status.LastKeyGeneration } + +// Override the controller credentials with ones from the Cluster's Secret reference (if supplied). +func (s *ObjectStorageKeyScope) SetCredentialRefTokenForLinodeClients(ctx context.Context) error { + if s.Key.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.Key.Spec.CredentialsRef, s.Key.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 +} diff --git a/cloud/scope/object_storage_key_test.go b/cloud/scope/object_storage_key_test.go index 4f0a8a70b..a87b113c8 100644 --- a/cloud/scope/object_storage_key_test.go +++ b/cloud/scope/object_storage_key_test.go @@ -104,41 +104,6 @@ func TestNewObjectStorageKeyScope(t *testing.T) { }) }, }, - { - name: "with credentials from secret", - args: args{ - apiKey: "apikey", - params: ObjectStorageKeyScopeParams{ - Client: nil, - Key: &infrav1alpha2.LinodeObjectStorageKey{ - Spec: infrav1alpha2.LinodeObjectStorageKeySpec{ - CredentialsRef: &corev1.SecretReference{ - Name: "example", - Namespace: "test", - }, - }, - }, - Logger: &logr.Logger{}, - }, - }, - expectedErr: nil, - expects: func(k8s *mock.MockK8sClient) { - k8s.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme { - s := runtime.NewScheme() - infrav1alpha2.AddToScheme(s) - return s - }) - k8s.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, name types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { - cred := corev1.Secret{ - Data: map[string][]byte{ - "apiToken": []byte("example"), - }, - } - *obj = cred - return nil - }) - }, - }, { name: "empty params", args: args{ @@ -163,28 +128,6 @@ func TestNewObjectStorageKeyScope(t *testing.T) { k8s.EXPECT().Scheme().Return(runtime.NewScheme()) }, }, - { - name: "credentials from ref fail", - args: args{ - apiKey: "apikey", - params: ObjectStorageKeyScopeParams{ - Client: nil, - Key: &infrav1alpha2.LinodeObjectStorageKey{ - Spec: infrav1alpha2.LinodeObjectStorageKeySpec{ - CredentialsRef: &corev1.SecretReference{ - Name: "example", - Namespace: "test", - }, - }, - }, - Logger: &logr.Logger{}, - }, - }, - expectedErr: 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: "empty apiKey", args: args{ @@ -224,7 +167,7 @@ func TestNewObjectStorageKeyScope(t *testing.T) { } } -func TestObjectStrorageKeyAddFinalizer(t *testing.T) { +func TestObjectStorageKeyAddFinalizer(t *testing.T) { t.Parallel() tests := []struct { @@ -639,3 +582,106 @@ func TestShouldRotateKey(t *testing.T) { }, }).ShouldRotateKey()) } +func TestObjectStorageKeySetCredentialRefTokenForLinodeClients(t *testing.T) { + t.Parallel() + + type args struct { + apiKey string + params ObjectStorageKeyScopeParams + } + tests := []struct { + name string + args args + expectedErr error + expects func(k8s *mock.MockK8sClient) + clientBuildFunc func(apiKey string) (LinodeClient, error) + }{ + { + name: "with credentials from secret", + args: args{ + apiKey: "apikey", + params: ObjectStorageKeyScopeParams{ + Client: nil, + Key: &infrav1alpha2.LinodeObjectStorageKey{ + Spec: infrav1alpha2.LinodeObjectStorageKeySpec{ + CredentialsRef: &corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + }, + }, + Logger: &logr.Logger{}, + }, + }, + expectedErr: nil, + expects: func(k8s *mock.MockK8sClient) { + k8s.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme { + s := runtime.NewScheme() + infrav1alpha2.AddToScheme(s) + return s + }) + k8s.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, name types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + cred := corev1.Secret{ + Data: map[string][]byte{ + "apiToken": []byte("example"), + }, + } + *obj = cred + return nil + }) + }, + }, + { + name: "credentials from ref fail", + args: args{ + apiKey: "apikey", + params: ObjectStorageKeyScopeParams{ + Client: nil, + Key: &infrav1alpha2.LinodeObjectStorageKey{ + Spec: infrav1alpha2.LinodeObjectStorageKeySpec{ + CredentialsRef: &corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + }, + }, + Logger: &logr.Logger{}, + }, + }, + expectedErr: fmt.Errorf("credentials from secret ref: get credentials secret test/example: failed to get secret"), + 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("failed to get secret")) + }, + }, + } + 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) + + testcase.args.params.Client = mockK8sClient + + kscope, err := NewObjectStorageKeyScope(context.Background(), ClientConfig{Token: testcase.args.apiKey}, testcase.args.params) + + if err != nil { + t.Errorf("NewObjectStorageKeyScope() error = %v", err) + } + + if err := kscope.SetCredentialRefTokenForLinodeClients(context.Background()); err != nil { + assert.ErrorContains(t, err, testcase.expectedErr.Error()) + } + }) + } +} diff --git a/cloud/scope/placement_group.go b/cloud/scope/placement_group.go index 79c9a83f5..4c62cfdd9 100644 --- a/cloud/scope/placement_group.go +++ b/cloud/scope/placement_group.go @@ -100,16 +100,6 @@ func NewPlacementGroupScope(ctx context.Context, linodeClientConfig ClientConfig if err := validatePlacementGroupScope(params); err != nil { return nil, err } - - // Override the controller credentials with ones from the Placement Groups's Secret reference (if supplied). - if params.LinodePlacementGroup.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.LinodePlacementGroup.Spec.CredentialsRef, params.LinodePlacementGroup.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), @@ -130,3 +120,16 @@ func NewPlacementGroupScope(ctx context.Context, linodeClientConfig ClientConfig PatchHelper: helper, }, nil } + +func (s *PlacementGroupScope) SetCredentialRefTokenForLinodeClients(ctx context.Context) error { + if s.LinodePlacementGroup.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.LinodePlacementGroup.Spec.CredentialsRef, s.LinodePlacementGroup.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 +} diff --git a/cloud/scope/placement_group_test.go b/cloud/scope/placement_group_test.go index dc5e7e1a8..9f11e3ea1 100644 --- a/cloud/scope/placement_group_test.go +++ b/cloud/scope/placement_group_test.go @@ -94,39 +94,6 @@ func TestNewPlacementGroupScope(t *testing.T) { }) }, }, - { - name: "Success - Validate getCredentialDataFromRef() returns some apiKey data and we create a valid PlacementGroupScope", - args: args{ - apiKey: "test-key", - params: PlacementGroupScopeParams{ - LinodePlacementGroup: &infrav1alpha2.LinodePlacementGroup{ - Spec: infrav1alpha2.LinodePlacementGroupSpec{ - CredentialsRef: &corev1.SecretReference{ - Namespace: "test-namespace", - Name: "test-name", - }, - }, - }, - }, - }, - 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{ - Data: map[string][]byte{ - "apiToken": []byte("example-api-token"), - }, - } - *obj = cred - return nil - }) - }, - }, { name: "Error - Pass in invalid args and get an error", args: args{ @@ -136,26 +103,6 @@ func TestNewPlacementGroupScope(t *testing.T) { expects: func(mock *mock.MockK8sClient) {}, expectedError: fmt.Errorf("linodePlacementGroup is required when creating a PlacementGroupScope"), }, - { - name: "Error - Pass in valid args but get an error when getting the credentials secret", - args: args{ - apiKey: "test-key", - params: PlacementGroupScopeParams{ - LinodePlacementGroup: &infrav1alpha2.LinodePlacementGroup{ - Spec: infrav1alpha2.LinodePlacementGroupSpec{ - CredentialsRef: &corev1.SecretReference{ - Namespace: "test-namespace", - Name: "test-name", - }, - }, - }, - }, - }, - expects: func(mock *mock.MockK8sClient) { - mock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("test error")) - }, - expectedError: fmt.Errorf("credentials from secret ref: get credentials secret test-namespace/test-name: test error"), - }, { name: "Error - Pass in valid args but get an error when creating a new linode client", args: args{ @@ -319,7 +266,7 @@ func TestPlacementGroupAddCredentialsRefFinalizer(t *testing.T) { *obj = cred return nil - }).Times(2) + }).Times(1) mock.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) }, }, @@ -409,7 +356,7 @@ func TestPlacementGroupRemoveCredentialsRefFinalizer(t *testing.T) { *obj = cred return nil - }).Times(2) + }).Times(1) mock.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) }, }, @@ -459,3 +406,88 @@ func TestPlacementGroupRemoveCredentialsRefFinalizer(t *testing.T) { }) } } +func TestPlacementGroupSetCredentialRefTokenForLinodeClients(t *testing.T) { + t.Parallel() + tests := []struct { + name string + LinodePlacementGroup *infrav1alpha2.LinodePlacementGroup + expects func(mock *mock.MockK8sClient) + expectedError error + }{ + { + name: "Success - Validate getCredentialDataFromRef() returns some apiKey data", + LinodePlacementGroup: &infrav1alpha2.LinodePlacementGroup{ + Spec: infrav1alpha2.LinodePlacementGroupSpec{ + CredentialsRef: &corev1.SecretReference{ + Namespace: "test-namespace", + Name: "test-name", + }, + }, + }, + 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{ + Data: map[string][]byte{ + "apiToken": []byte("example-api-token"), + }, + } + *obj = cred + return nil + }) + }, + }, + { + name: "Error - Get an error when getting the credentials secret", + LinodePlacementGroup: &infrav1alpha2.LinodePlacementGroup{ + Spec: infrav1alpha2.LinodePlacementGroupSpec{ + CredentialsRef: &corev1.SecretReference{ + Namespace: "test-namespace", + Name: "test-name", + }, + }, + }, + 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")) + }, + expectedError: fmt.Errorf("credentials from secret ref: get credentials secret test-namespace/test-name: 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) + + pgScope, err := NewPlacementGroupScope( + context.Background(), + ClientConfig{Token: "test-key"}, + PlacementGroupScopeParams{ + Client: mockK8sClient, + LinodePlacementGroup: testcase.LinodePlacementGroup, + }, + ) + if err != nil { + t.Errorf("NewPGScope() error = %v", err) + } + if err := pgScope.SetCredentialRefTokenForLinodeClients(context.Background()); err != nil { + assert.ErrorContains(t, err, testcase.expectedError.Error()) + } + }) + } +} diff --git a/cloud/scope/vpc.go b/cloud/scope/vpc.go index e87312495..c25df5746 100644 --- a/cloud/scope/vpc.go +++ b/cloud/scope/vpc.go @@ -60,16 +60,6 @@ func NewVPCScope(ctx context.Context, linodeClientConfig ClientConfig, params VP if err := validateVPCScopeParams(params); err != nil { return nil, err } - - // Override the controller credentials with ones from the VPC's Secret reference (if supplied). - if params.LinodeVPC.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.LinodeVPC.Spec.CredentialsRef, params.LinodeVPC.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), ) @@ -129,3 +119,16 @@ func (s *VPCScope) RemoveCredentialsRefFinalizer(ctx context.Context) error { *s.LinodeVPC.Spec.CredentialsRef, s.LinodeVPC.GetNamespace(), toFinalizer(s.LinodeVPC)) } + +func (s *VPCScope) SetCredentialRefTokenForLinodeClients(ctx context.Context) error { + if s.LinodeVPC.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.LinodeVPC.Spec.CredentialsRef, s.LinodeVPC.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 +} diff --git a/cloud/scope/vpc_test.go b/cloud/scope/vpc_test.go index 02e79d4a5..ee78e0c95 100644 --- a/cloud/scope/vpc_test.go +++ b/cloud/scope/vpc_test.go @@ -94,39 +94,6 @@ func TestNewVPCScope(t *testing.T) { }) }, }, - { - name: "Success - Validate getCredentialDataFromRef() returns some apiKey data and we create a valid ClusterScope", - args: args{ - apiKey: "test-key", - params: VPCScopeParams{ - LinodeVPC: &infrav1alpha2.LinodeVPC{ - Spec: infrav1alpha2.LinodeVPCSpec{ - CredentialsRef: &corev1.SecretReference{ - Namespace: "test-namespace", - Name: "test-name", - }, - }, - }, - }, - }, - 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{ - Data: map[string][]byte{ - "apiToken": []byte("example-api-token"), - }, - } - *obj = cred - return nil - }) - }, - }, { name: "Error - Pass in invalid args and get an error", args: args{ @@ -136,26 +103,6 @@ func TestNewVPCScope(t *testing.T) { expects: func(mock *mock.MockK8sClient) {}, expectedError: fmt.Errorf("linodeVPC is required when creating a VPCScope"), }, - { - name: "Error - Pass in valid args but get an error when getting the credentials secret", - args: args{ - apiKey: "test-key", - params: VPCScopeParams{ - LinodeVPC: &infrav1alpha2.LinodeVPC{ - Spec: infrav1alpha2.LinodeVPCSpec{ - CredentialsRef: &corev1.SecretReference{ - Namespace: "test-namespace", - Name: "test-name", - }, - }, - }, - }, - }, - expects: func(mock *mock.MockK8sClient) { - mock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("test error")) - }, - expectedError: fmt.Errorf("credentials from secret ref: get credentials secret test-namespace/test-name: test error"), - }, { name: "Error - Pass in valid args but get an error when creating a new linode client", args: args{ @@ -319,7 +266,7 @@ func TestVPCAddCredentialsRefFinalizer(t *testing.T) { *obj = cred return nil - }).Times(2) + }).Times(1) mock.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) }, }, @@ -409,7 +356,7 @@ func TestVPCRemoveCredentialsRefFinalizer(t *testing.T) { *obj = cred return nil - }).Times(2) + }).Times(1) mock.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) }, }, @@ -459,3 +406,89 @@ func TestVPCRemoveCredentialsRefFinalizer(t *testing.T) { }) } } + +func TestVPCSetCredentialRefTokenForLinodeClients(t *testing.T) { + t.Parallel() + tests := []struct { + name string + LinodeVPC *infrav1alpha2.LinodeVPC + expects func(mock *mock.MockK8sClient) + expectedError error + }{ + { + name: "Success - Validate getCredentialDataFromRef() returns some apiKey data and we create a valid ClusterScope", + LinodeVPC: &infrav1alpha2.LinodeVPC{ + Spec: infrav1alpha2.LinodeVPCSpec{ + CredentialsRef: &corev1.SecretReference{ + Namespace: "test-namespace", + Name: "test-name", + }, + }, + }, + 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{ + Data: map[string][]byte{ + "apiToken": []byte("example-api-token"), + }, + } + *obj = cred + return nil + }) + }, + }, + { + name: "Error - error when getting the credentials secret", + LinodeVPC: &infrav1alpha2.LinodeVPC{ + Spec: infrav1alpha2.LinodeVPCSpec{ + CredentialsRef: &corev1.SecretReference{ + Namespace: "test-namespace", + Name: "test-name", + }, + }, + }, + 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")) + }, + expectedError: fmt.Errorf("credentials from secret ref: get credentials secret test-namespace/test-name: 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) + + vScope, err := NewVPCScope( + context.Background(), + ClientConfig{Token: "test-key"}, + VPCScopeParams{ + Client: mockK8sClient, + LinodeVPC: testcase.LinodeVPC, + }, + ) + if err != nil { + t.Errorf("NewVPCScope() error = %v", err) + } + if err := vScope.SetCredentialRefTokenForLinodeClients(context.Background()); err != nil { + assert.ErrorContains(t, err, testcase.expectedError.Error()) + } + }) + } +} diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index ec1023fcb..74a106d92 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -140,6 +140,11 @@ func (r *LinodeClusterReconciler) reconcile( return res, err } + if err := clusterScope.SetCredentialRefTokenForLinodeClients(ctx); err != nil { + logger.Error(err, "failed to update linode client token from Credential Ref") + return res, err + } + // Handle deleted clusters if !clusterScope.LinodeCluster.DeletionTimestamp.IsZero() { if err := r.reconcileDelete(ctx, logger, clusterScope); err != nil { diff --git a/controller/linodefirewall_controller.go b/controller/linodefirewall_controller.go index fe2da5097..9924a498d 100644 --- a/controller/linodefirewall_controller.go +++ b/controller/linodefirewall_controller.go @@ -127,6 +127,12 @@ func (r *LinodeFirewallReconciler) reconcile( } }() + // Override the controller credentials with ones from the Firewall's Secret reference (if supplied). + if err := fwScope.SetCredentialRefTokenForLinodeClients(ctx); err != nil { + logger.Error(err, "failed to update linode client token from Credential Ref") + return ctrl.Result{}, err + } + // Delete if !fwScope.LinodeFirewall.ObjectMeta.DeletionTimestamp.IsZero() { failureReason = infrav1alpha2.DeleteFirewallError diff --git a/controller/linodemachine_controller.go b/controller/linodemachine_controller.go index 177d50b81..09244a382 100644 --- a/controller/linodemachine_controller.go +++ b/controller/linodemachine_controller.go @@ -191,6 +191,18 @@ func (r *LinodeMachineReconciler) reconcile(ctx context.Context, logger logr.Log return ctrl.Result{}, err } + // Override the controller credentials with ones from the Machine's Secret reference (if supplied). + // Credentials will be used in the following order: + // 1. LinodeMachine + // 2. Owner LinodeCluster + // 3. Controller + if machineScope.LinodeMachine.Spec.CredentialsRef != nil || machineScope.LinodeCluster.Spec.CredentialsRef != nil { + if err := machineScope.SetCredentialRefTokenForLinodeClients(ctx); err != nil { + logger.Error(err, "failed to update linode client token from Credential Ref") + return ctrl.Result{}, err + } + } + // Delete if !machineScope.LinodeMachine.ObjectMeta.DeletionTimestamp.IsZero() { failureReason = cerrs.DeleteMachineError diff --git a/controller/linodeobjectstoragekey_controller.go b/controller/linodeobjectstoragekey_controller.go index cccffa5d0..4507e771b 100644 --- a/controller/linodeobjectstoragekey_controller.go +++ b/controller/linodeobjectstoragekey_controller.go @@ -123,6 +123,12 @@ func (r *LinodeObjectStorageKeyReconciler) reconcile(ctx context.Context, keySco } }() + // Override the controller credentials with ones from the Key's Secret reference (if supplied). + if err := keyScope.SetCredentialRefTokenForLinodeClients(ctx); err != nil { + keyScope.Logger.Error(err, "failed to update linode client token from Credential Ref") + return res, err + } + if !keyScope.Key.DeletionTimestamp.IsZero() { return res, r.reconcileDelete(ctx, keyScope) } diff --git a/controller/linodeplacementgroup_controller.go b/controller/linodeplacementgroup_controller.go index 61079cf12..f57bc7505 100644 --- a/controller/linodeplacementgroup_controller.go +++ b/controller/linodeplacementgroup_controller.go @@ -135,6 +135,12 @@ func (r *LinodePlacementGroupReconciler) reconcile( } }() + // Override the controller credentials with ones from the Placement Groups's Secret reference (if supplied). + if err := pgScope.SetCredentialRefTokenForLinodeClients(ctx); err != nil { + logger.Error(err, "failed to update linode client token from Credential Ref") + return res, err + } + // Delete if !pgScope.LinodePlacementGroup.ObjectMeta.DeletionTimestamp.IsZero() { failureReason = infrav1alpha2.DeletePlacementGroupError diff --git a/controller/linodevpc_controller.go b/controller/linodevpc_controller.go index c504c4051..abbba1044 100644 --- a/controller/linodevpc_controller.go +++ b/controller/linodevpc_controller.go @@ -141,6 +141,12 @@ func (r *LinodeVPCReconciler) reconcile( } }() + // Override the controller credentials with ones from the VPC's Secret reference (if supplied). + if err := vpcScope.SetCredentialRefTokenForLinodeClients(ctx); err != nil { + logger.Error(err, "failed to update linode client token from Credential Ref") + return res, err + } + // Delete if !vpcScope.LinodeVPC.ObjectMeta.DeletionTimestamp.IsZero() { failureReason = infrav1alpha2.DeleteVPCError diff --git a/mock/client.go b/mock/client.go index 7d0127be9..011b17312 100644 --- a/mock/client.go +++ b/mock/client.go @@ -754,6 +754,20 @@ func (mr *MockLinodeClientMockRecorder) ResizeInstanceDisk(ctx, linodeID, diskID return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResizeInstanceDisk", reflect.TypeOf((*MockLinodeClient)(nil).ResizeInstanceDisk), ctx, linodeID, diskID, size) } +// SetToken mocks base method. +func (m *MockLinodeClient) SetToken(token string) *linodego.Client { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetToken", token) + ret0, _ := ret[0].(*linodego.Client) + return ret0 +} + +// SetToken indicates an expected call of SetToken. +func (mr *MockLinodeClientMockRecorder) SetToken(token any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetToken", reflect.TypeOf((*MockLinodeClient)(nil).SetToken), token) +} + // UnassignPlacementGroupLinodes mocks base method. func (m *MockLinodeClient) UnassignPlacementGroupLinodes(ctx context.Context, id int, options linodego.PlacementGroupUnAssignOptions) (*linodego.PlacementGroup, error) { m.ctrl.T.Helper() @@ -2263,3 +2277,40 @@ func (mr *MockK8sClientMockRecorder) Update(ctx, obj any, opts ...any) *gomock.C varargs := append([]any{ctx, obj}, opts...) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Update", reflect.TypeOf((*MockK8sClient)(nil).Update), varargs...) } + +// MockLinodeTokenClient is a mock of LinodeTokenClient interface. +type MockLinodeTokenClient struct { + ctrl *gomock.Controller + recorder *MockLinodeTokenClientMockRecorder +} + +// MockLinodeTokenClientMockRecorder is the mock recorder for MockLinodeTokenClient. +type MockLinodeTokenClientMockRecorder struct { + mock *MockLinodeTokenClient +} + +// NewMockLinodeTokenClient creates a new mock instance. +func NewMockLinodeTokenClient(ctrl *gomock.Controller) *MockLinodeTokenClient { + mock := &MockLinodeTokenClient{ctrl: ctrl} + mock.recorder = &MockLinodeTokenClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockLinodeTokenClient) EXPECT() *MockLinodeTokenClientMockRecorder { + return m.recorder +} + +// SetToken mocks base method. +func (m *MockLinodeTokenClient) SetToken(token string) *linodego.Client { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetToken", token) + ret0, _ := ret[0].(*linodego.Client) + return ret0 +} + +// SetToken indicates an expected call of SetToken. +func (mr *MockLinodeTokenClientMockRecorder) SetToken(token any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetToken", reflect.TypeOf((*MockLinodeTokenClient)(nil).SetToken), token) +}