Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 11 commits into from
Sep 23, 2024
5 changes: 5 additions & 0 deletions clients/clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ type LinodeClient interface {
LinodeDNSClient
LinodePlacementGroupClient
LinodeFirewallClient
LinodeTokenClient
}

type AkamClient interface {
Expand Down Expand Up @@ -118,3 +119,7 @@ type LinodeFirewallClient interface {
type K8sClient interface {
client.Client
}

type LinodeTokenClient interface {
SetToken(token string) *linodego.Client
}
31 changes: 17 additions & 14 deletions cloud/scope/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,6 @@
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)
Expand Down Expand Up @@ -152,3 +138,20 @@
*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

Check warning on line 156 in cloud/scope/cluster.go

View check run for this annotation

Codecov / codecov/patch

cloud/scope/cluster.go#L156

Added line #L156 was not covered by tests
}
183 changes: 115 additions & 68 deletions cloud/scope/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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())
}
})
}
}
23 changes: 13 additions & 10 deletions cloud/scope/firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,6 @@
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),
)
Expand Down Expand Up @@ -126,3 +116,16 @@
*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

Check warning on line 130 in cloud/scope/firewall.go

View check run for this annotation

Codecov / codecov/patch

cloud/scope/firewall.go#L130

Added line #L130 was not covered by tests
}
Loading
Loading