diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index 1a9de0add..b550ecd12 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -23,8 +23,6 @@ import ( "github.com/linode/linodego" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/util/patch" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" @@ -37,10 +35,6 @@ type ClusterScopeParams struct { LinodeCluster *infrav1alpha1.LinodeCluster } -// var patchNewHelper = patch.NewHelper - -type patchHelper func(obj client.Object, crClient client.Client) (*patch.Helper, error) - func validateClusterScopeParams(params ClusterScopeParams) error { if params.Cluster == nil { return errors.New("cluster is required when creating a ClusterScope") @@ -54,7 +48,7 @@ func validateClusterScopeParams(params ClusterScopeParams) error { // NewClusterScope creates a new Scope from the supplied parameters. // This is meant to be called for each reconcile iteration. -func NewClusterScope(ctx context.Context, apiKey string, params ClusterScopeParams, patchNewHelper patchHelper) (*ClusterScope, error) { +func NewClusterScope(ctx context.Context, apiKey string, params ClusterScopeParams, patchNewHelper patchNewHelper) (*ClusterScope, error) { if err := validateClusterScopeParams(params); err != nil { return nil, err } diff --git a/cloud/scope/common.go b/cloud/scope/common.go index 8c3a83c37..f75cd58b9 100644 --- a/cloud/scope/common.go +++ b/cloud/scope/common.go @@ -9,11 +9,14 @@ import ( "github.com/linode/linodego" "golang.org/x/oauth2" corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/linode/cluster-api-provider-linode/version" ) +type patchNewHelper func(obj client.Object, crClient client.Client) (*patch.Helper, error) + func createLinodeClient(apiKey string) (*linodego.Client, error) { if apiKey == "" { return nil, errors.New("missing Linode API key") diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index 9ea0d6898..5ff257c62 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -9,7 +9,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -25,9 +24,9 @@ type MachineScopeParams struct { } type MachineScope struct { - client client.Client + client k8sClient - PatchHelper *patch.Helper + PatchHelper PatchHelper Cluster *clusterv1.Cluster Machine *clusterv1.Machine LinodeClient *linodego.Client @@ -52,7 +51,7 @@ func validateMachineScopeParams(params MachineScopeParams) error { return nil } -func NewMachineScope(ctx context.Context, apiKey string, params MachineScopeParams) (*MachineScope, error) { +func NewMachineScope(ctx context.Context, apiKey string, params MachineScopeParams, patchNewHelper patchNewHelper) (*MachineScope, error) { if err := validateMachineScopeParams(params); err != nil { return nil, err } @@ -89,7 +88,7 @@ func NewMachineScope(ctx context.Context, apiKey string, params MachineScopePara return nil, fmt.Errorf("failed to create linode client: %w", err) } - helper, err := patch.NewHelper(params.LinodeMachine, params.Client) + helper, err := patchNewHelper(params.LinodeMachine, params.Client) if err != nil { return nil, fmt.Errorf("failed to init patch helper: %w", err) } diff --git a/cloud/scope/machine_test.go b/cloud/scope/machine_test.go new file mode 100644 index 000000000..5f22a6397 --- /dev/null +++ b/cloud/scope/machine_test.go @@ -0,0 +1,549 @@ +package scope + +import ( + "context" + "errors" + "testing" + + "github.com/linode/linodego" + "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/types" + "k8s.io/utils/ptr" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/util/patch" + "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" +) + +func TestValidateMachineScopeParams(t *testing.T) { + t.Parallel() + type args struct { + params MachineScopeParams + } + tests := []struct { + name string + args args + wantErr bool + }{ + // TODO: Add test cases. + { + "Valid MachineScopeParams", + args{ + params: MachineScopeParams{ + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + LinodeMachine: &infrav1alpha1.LinodeMachine{}, + }, + }, + false, + }, + { + "Invalid MachineScopeParams - empty MachineScopeParams", + args{ + params: MachineScopeParams{}, + }, + true, + }, + { + "Invalid MachineScopeParams - no LinodeCluster in MachineScopeParams", + args{ + params: MachineScopeParams{ + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{}, + LinodeMachine: &infrav1alpha1.LinodeMachine{}, + }, + }, + true, + }, + { + "Invalid MachineScopeParams - no LinodeMachine in MachineScopeParams", + args{ + params: MachineScopeParams{ + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + }, + }, + true, + }, + { + "Invalid MachineScopeParams - no Cluster in MachineScopeParams", + args{ + params: MachineScopeParams{ + Machine: &clusterv1.Machine{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + LinodeMachine: &infrav1alpha1.LinodeMachine{}, + }, + }, + true, + }, + { + "Invalid MachineScopeParams - no Machine in MachineScopeParams", + args{ + params: MachineScopeParams{ + Cluster: &clusterv1.Cluster{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + LinodeMachine: &infrav1alpha1.LinodeMachine{}, + }, + }, + true, + }, + } + for _, tt := range tests { + testcase := tt + t.Run(testcase.name, func(t *testing.T) { + t.Parallel() + if err := validateMachineScopeParams(testcase.args.params); (err != nil) != testcase.wantErr { + t.Errorf("validateMachineScopeParams() error = %v, wantErr %v", err, testcase.wantErr) + } + }) + } +} + +func TestMachineScopeAddFinalizer(t *testing.T) { + t.Parallel() + type fields struct { + LinodeMachine *infrav1alpha1.LinodeMachine + } + tests := []struct { + name string + fields fields + wantErr bool + }{ + // TODO: Add test cases. + { + "Success - finalizer should be added to the Linode Machine object", + fields{ + LinodeMachine: &infrav1alpha1.LinodeMachine{}, + }, + false, + }, + { + "AddFinalizer error - finalizer should not be added to the Linode Machine object. Function returns nil since it was already present", + fields{ + LinodeMachine: &infrav1alpha1.LinodeMachine{ + ObjectMeta: metav1.ObjectMeta{ + Finalizers: []string{infrav1alpha1.GroupVersion.String()}, + }, + }, + }, + false, + }, + } + for _, tt := range tests { + testcase := tt + t.Run(testcase.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockPatchHelper := mock.NewMockPatchHelper(ctrl) + mockK8sClient := mock.NewMockk8sClient(ctrl) + + mScope := &MachineScope{ + client: mockK8sClient, + PatchHelper: mockPatchHelper, + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{}, + LinodeClient: &linodego.Client{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + LinodeMachine: testcase.fields.LinodeMachine, + } + + if mScope.LinodeMachine.Finalizers == nil { + mockPatchHelper.EXPECT().Patch(gomock.Any(), gomock.Any()).Return(nil) + } + + if err := mScope.AddFinalizer(context.Background()); (err != nil) != testcase.wantErr { + t.Errorf("MachineScope.AddFinalizer() error = %v, wantErr %v", err, testcase.wantErr) + } + + if mScope.LinodeMachine.Finalizers[0] != infrav1alpha1.GroupVersion.String() { + t.Errorf("Not able to add finalizer") + } + }) + } +} + +func TestNewMachineScope(t *testing.T) { + t.Parallel() + type args struct { + apiKey string + params MachineScopeParams + } + tests := []struct { + name string + args args + want *MachineScope + expectedErr error + patchFunc func(obj client.Object, crClient client.Client) (*patch.Helper, error) + getFunc func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error + }{ + // TODO: Add test cases. + { + name: "Success - Pass in valid args and get a valid MachineScope", + args: args{ + apiKey: "test-key", + params: MachineScopeParams{ + Client: nil, + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + LinodeMachine: &infrav1alpha1.LinodeMachine{}, + }, + }, + want: &MachineScope{}, + expectedErr: nil, + patchFunc: func(obj client.Object, crClient client.Client) (*patch.Helper, error) { + return &patch.Helper{}, nil + }, + getFunc: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + return nil + }, + }, + { + name: "Success - Pass in credential ref through MachineScopeParams.LinodeMachine and get a valid MachineScope", + args: args{ + apiKey: "test-key", + params: MachineScopeParams{ + Client: nil, + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + LinodeMachine: &infrav1alpha1.LinodeMachine{ + Spec: infrav1alpha1.LinodeMachineSpec{ + CredentialsRef: &corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + }, + }, + }, + }, + want: &MachineScope{}, + expectedErr: nil, + patchFunc: func(obj client.Object, crClient client.Client) (*patch.Helper, error) { + return &patch.Helper{}, nil + }, + getFunc: 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 + }, + }, + { + name: "Success - Pass in credential ref through MachineScopeParams.LinodeCluster and get a valid MachineScope", + args: args{ + apiKey: "test-key", + params: MachineScopeParams{ + Client: nil, + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{ + Spec: infrav1alpha1.LinodeClusterSpec{ + CredentialsRef: &corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + }, + }, + LinodeMachine: &infrav1alpha1.LinodeMachine{}, + }, + }, + want: &MachineScope{}, + expectedErr: nil, + patchFunc: func(obj client.Object, crClient client.Client) (*patch.Helper, error) { + return &patch.Helper{}, nil + }, + getFunc: 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 + }, + }, + { + name: "Error - Pass in credential ref through MachineScopeParams.LinodeCluster and getCredentialDataFromRef() returns error", + args: args{ + apiKey: "test-key", + params: MachineScopeParams{ + Client: nil, + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{ + Spec: infrav1alpha1.LinodeClusterSpec{ + CredentialsRef: &corev1.SecretReference{ + Name: "example", + Namespace: "test", + }, + }, + }, + LinodeMachine: &infrav1alpha1.LinodeMachine{}, + }, + }, + want: &MachineScope{}, + expectedErr: errors.New("credentials from cluster secret ref: get credentials secret test/example: Creds not found"), + patchFunc: func(obj client.Object, crClient client.Client) (*patch.Helper, error) { + return &patch.Helper{}, nil + }, + getFunc: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + return errors.New("Creds not found") + }, + }, + { + name: "Error - Pass in invalid args and get an error. Set ClusterScopeParams.Cluster to nil", + args: args{ + apiKey: "test-key", + params: MachineScopeParams{ + Client: nil, + Cluster: nil, + Machine: &clusterv1.Machine{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + LinodeMachine: &infrav1alpha1.LinodeMachine{}, + }, + }, + want: nil, + expectedErr: errors.New("custer is required when creating a MachineScope"), + patchFunc: func(obj client.Object, crClient client.Client) (*patch.Helper, error) { + return &patch.Helper{}, nil + }, + getFunc: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + return nil + }, + }, + { + name: "Error - Pass in valid args but couldn't get patch helper", + args: args{ + apiKey: "test-key", + params: MachineScopeParams{ + Client: nil, + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + LinodeMachine: &infrav1alpha1.LinodeMachine{}, + }, + }, + want: &MachineScope{}, + expectedErr: errors.New("failed to init patch helper: failed to create patch helper"), + patchFunc: func(obj client.Object, crClient client.Client) (*patch.Helper, error) { + return nil, errors.New("failed to create patch helper") + }, + getFunc: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + return nil + }, + }, + } + + 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) + + linodeMachine := testcase.args.params.LinodeMachine + linodeCluster := testcase.args.params.LinodeCluster + + if (linodeMachine != nil && linodeMachine.Spec.CredentialsRef != nil) || + (linodeCluster != nil && linodeCluster.Spec.CredentialsRef != nil) { + mockK8sClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(testcase.getFunc).Times(1) + } + + testcase.args.params.Client = mockK8sClient + + got, err := NewMachineScope(context.Background(), testcase.args.apiKey, testcase.args.params, testcase.patchFunc) + + if testcase.expectedErr != nil { + assert.EqualError(t, err, testcase.expectedErr.Error()) + } else { + assert.NotEmpty(t, got) + } + }) + } +} + +func TestMachineScopeGetBootstrapData(t *testing.T) { + t.Parallel() + type fields struct { + Cluster *clusterv1.Cluster + Machine *clusterv1.Machine + LinodeClient *linodego.Client + LinodeCluster *infrav1alpha1.LinodeCluster + LinodeMachine *infrav1alpha1.LinodeMachine + } + tests := []struct { + name string + fields fields + want []byte + expectedErr error + getFunc func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error + }{ + // TODO: Add test cases. + { + name: "Success - Using a valid MachineScope. Get bootstrap data", + fields: fields{ + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{ + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + DataSecretName: ptr.To("test-data"), + }, + }, + }, + LinodeClient: &linodego.Client{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + LinodeMachine: &infrav1alpha1.LinodeMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-linode-machine", + Namespace: "test-namespace", + }, + }, + }, + want: []byte("test-data"), + expectedErr: nil, + getFunc: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + cred := corev1.Secret{ + Data: map[string][]byte{ + "value": []byte("test-data"), + }, + } + *obj = cred + return nil + }, + }, + { + name: "Error - Set MachineScope.Machine.Spec.Bootstrap.DataSecretName to nil. Returns an error", + fields: fields{ + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{ + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + DataSecretName: nil, + }, + }, + }, + LinodeClient: &linodego.Client{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + LinodeMachine: &infrav1alpha1.LinodeMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-linode-machine", + Namespace: "test-namespace", + }, + }, + }, + want: nil, + expectedErr: errors.New("bootstrap data secret is nil for LinodeMachine test-namespace/test-linode-machine"), + getFunc: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + return nil + }, + }, + { + name: "Error - client.Get return an error while retrieving bootstrap data secret", + fields: fields{ + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{ + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + DataSecretName: ptr.To("test-data"), + }, + }, + }, + LinodeClient: &linodego.Client{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + LinodeMachine: &infrav1alpha1.LinodeMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-linode-machine", + Namespace: "test-namespace", + }, + }, + }, + want: nil, + expectedErr: errors.New("failed to retrieve bootstrap data secret for LinodeMachine test-namespace/test-linode-machine"), + getFunc: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + return errors.New("test-error") + }, + }, + { + name: "Error - client.Get return some data but it doesn't contain the bootstrap data secret and secret key 'value' is missing", + fields: fields{ + Cluster: &clusterv1.Cluster{}, + Machine: &clusterv1.Machine{ + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + DataSecretName: ptr.To("test-data"), + }, + }, + }, + LinodeClient: &linodego.Client{}, + LinodeCluster: &infrav1alpha1.LinodeCluster{}, + LinodeMachine: &infrav1alpha1.LinodeMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-linode-machine", + Namespace: "test-namespace", + }, + }, + }, + want: nil, + expectedErr: errors.New("bootstrap data secret value key is missing for LinodeMachine test-namespace/test-linode-machine"), + getFunc: func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error { + cred := corev1.Secret{ + Data: map[string][]byte{}, + } + *obj = cred + return nil + }, + }, + } + 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) + + if testcase.fields.Machine.Spec.Bootstrap.DataSecretName != nil { + mockK8sClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(testcase.getFunc).Times(1) + } + + mScope := &MachineScope{ + client: mockK8sClient, + PatchHelper: &patch.Helper{}, // empty patch helper + Cluster: testcase.fields.Cluster, + Machine: testcase.fields.Machine, + LinodeClient: testcase.fields.LinodeClient, + LinodeCluster: testcase.fields.LinodeCluster, + LinodeMachine: testcase.fields.LinodeMachine, + } + + got, err := mScope.GetBootstrapData(context.Background()) + + if testcase.expectedErr != nil { + assert.EqualError(t, err, testcase.expectedErr.Error()) + } else { + assert.Equal(t, testcase.want, got) + } + }) + } +} diff --git a/controller/linodemachine_controller.go b/controller/linodemachine_controller.go index 74fa36b6d..9b2d18d33 100644 --- a/controller/linodemachine_controller.go +++ b/controller/linodemachine_controller.go @@ -130,6 +130,7 @@ func (r *LinodeMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques LinodeCluster: &infrav1alpha1.LinodeCluster{}, LinodeMachine: linodeMachine, }, + util.NewPatchHelper, ) if err != nil { log.Error(err, "Failed to create machine scope")