diff --git a/api/types/state.go b/api/types/state.go index 7b531ae5..50fe1ae7 100644 --- a/api/types/state.go +++ b/api/types/state.go @@ -21,6 +21,8 @@ type ConfigurationState string // Reasons a resource is or is not ready. const ( + Authorizing ConfigurationState = "Authorizing" + ProviderNotFound ConfigurationState = "ProviderNotFound" ProviderNotReady ConfigurationState = "ProviderNotReady" ConfigurationStaticCheckFailed ConfigurationState = "ConfigurationSpecNotValid" Available ConfigurationState = "Available" @@ -46,6 +48,8 @@ const ( MessageCloudResourceDeployed = "Cloud resources are deployed and ready to use" // MessageCloudResourceDestroying is the message when cloud resource is being destroyed MessageCloudResourceDestroying = "Cloud resources is being destroyed..." + // ErrProviderNotFound means provider not found + ErrProviderNotFound = "provider not found" // ErrProviderNotReady means provider object is not ready ErrProviderNotReady = "Provider is not ready" // ConfigurationReloadingAsHCLChanged means Configuration changed and needs reloading diff --git a/controllers/configuration/configuration.go b/controllers/configuration/configuration.go index 0f8af794..bb65af7d 100644 --- a/controllers/configuration/configuration.go +++ b/controllers/configuration/configuration.go @@ -2,6 +2,7 @@ package configuration import ( "context" + "fmt" "strconv" "strings" @@ -15,7 +16,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/oam-dev/terraform-controller/api/types" + crossplane "github.com/oam-dev/terraform-controller/api/types/crossplane-runtime" "github.com/oam-dev/terraform-controller/api/v1beta1" + "github.com/oam-dev/terraform-controller/controllers/provider" ) const ( @@ -122,6 +125,29 @@ func Get(ctx context.Context, k8sClient client.Client, namespacedName apitypes.N return *configuration, nil } +// IsDeletable will check whether the Configuration can be deleted immediately +// If deletable, it means no external cloud resources are provisioned +func IsDeletable(ctx context.Context, k8sClient client.Client, configuration *v1beta1.Configuration) (bool, error) { + providerRef := GetProviderNamespacedName(*configuration) + providerObj, err := provider.GetProviderFromConfiguration(ctx, k8sClient, providerRef.Namespace, providerRef.Name) + if err != nil { + return false, err + } + // allow Configuration to delete when the Provider doesn't exist or is not ready, which means external cloud resources are + // not provisioned at all + if providerObj == nil || providerObj.Status.State == types.ProviderIsNotReady { + return true, nil + } + + if configuration.Status.Apply.State == types.ConfigurationProvisioningAndChecking { + warning := fmt.Sprintf("Destroy could not complete and needs to wait for Provision to complete first: %s", types.MessageCloudResourceProvisioningAndChecking) + klog.Warning(warning) + return false, errors.New(warning) + } + + return false, nil +} + // ReplaceTerraformSource will replace the Terraform source from GitHub to Gitee func ReplaceTerraformSource(remote string, githubBlockedStr string) string { klog.InfoS("Whether GitHub is blocked", "githubBlocked", githubBlockedStr) @@ -154,3 +180,14 @@ func ReplaceTerraformSource(remote string, githubBlockedStr string) string { } return remote } + +// GetProviderNamespacedName will get the provider namespaced name +func GetProviderNamespacedName(configuration v1beta1.Configuration) *crossplane.Reference { + if configuration.Spec.ProviderReference != nil { + return configuration.Spec.ProviderReference + } + return &crossplane.Reference{ + Name: provider.DefaultName, + Namespace: provider.DefaultNamespace, + } +} diff --git a/controllers/configuration/configuration_test.go b/controllers/configuration/configuration_test.go index d7a9605c..0bd50003 100644 --- a/controllers/configuration/configuration_test.go +++ b/controllers/configuration/configuration_test.go @@ -1,7 +1,18 @@ package configuration import ( + "context" + "strings" "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/oam-dev/terraform-controller/api/types" + crossplane "github.com/oam-dev/terraform-controller/api/types/crossplane-runtime" + "github.com/oam-dev/terraform-controller/api/v1beta1" ) func TestReplaceTerraformSource(t *testing.T) { @@ -51,3 +62,140 @@ func TestReplaceTerraformSource(t *testing.T) { }) } } + +func TestIsDeletable(t *testing.T) { + ctx := context.Background() + s := runtime.NewScheme() + v1beta1.AddToScheme(s) + provider2 := &v1beta1.Provider{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: "default", + }, + Status: v1beta1.ProviderStatus{ + State: types.ProviderIsNotReady, + }, + } + provider3 := &v1beta1.Provider{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: "default", + }, + Status: v1beta1.ProviderStatus{ + State: types.ProviderIsReady, + }, + } + k8sClient1 := fake.NewClientBuilder().WithScheme(s).Build() + k8sClient2 := fake.NewClientBuilder().WithScheme(s).WithObjects(provider2).Build() + k8sClient3 := fake.NewClientBuilder().WithScheme(s).WithObjects(provider3).Build() + k8sClient4 := fake.NewClientBuilder().Build() + + configuration := &v1beta1.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "abc", + }, + } + configuration.Spec.ProviderReference = &crossplane.Reference{ + Name: "default", + Namespace: "default", + } + + type args struct { + configuration *v1beta1.Configuration + k8sClient client.Client + } + type want struct { + deletable bool + errMsg string + } + testcases := []struct { + name string + args args + want want + }{ + { + name: "provider is not found", + args: args{ + k8sClient: k8sClient1, + configuration: &v1beta1.Configuration{}, + }, + want: want{ + deletable: true, + }, + }, + { + name: "provider is not ready, use default providerRef", + args: args{ + k8sClient: k8sClient2, + configuration: &v1beta1.Configuration{}, + }, + want: want{ + deletable: true, + }, + }, + { + name: "provider is not ready, providerRef is set in configuration spec", + args: args{ + k8sClient: k8sClient2, + configuration: configuration, + }, + want: want{ + deletable: true, + }, + }, + { + name: "configuration is provisioning", + args: args{ + k8sClient: k8sClient3, + configuration: &v1beta1.Configuration{ + Status: v1beta1.ConfigurationStatus{ + Apply: v1beta1.ConfigurationApplyStatus{ + State: types.ConfigurationProvisioningAndChecking, + }, + }, + }, + }, + want: want{ + errMsg: "Destroy could not complete and needs to wait for Provision to complete first", + }, + }, + { + name: "configuration is ready", + args: args{ + k8sClient: k8sClient3, + configuration: &v1beta1.Configuration{ + Status: v1beta1.ConfigurationStatus{ + Apply: v1beta1.ConfigurationApplyStatus{ + State: types.Available, + }, + }, + }, + }, + want: want{}, + }, + { + name: "failed to get provider", + args: args{ + k8sClient: k8sClient4, + configuration: &v1beta1.Configuration{}, + }, + want: want{ + errMsg: "failed to get Provider object", + }, + }, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + got, err := IsDeletable(ctx, tc.args.k8sClient, tc.args.configuration) + if err != nil { + if !strings.Contains(err.Error(), tc.want.errMsg) { + t.Errorf("IsDeletable() error = %v, wantErr %v", err, tc.want.errMsg) + return + } + } + if got != tc.want.deletable { + t.Errorf("IsDeletable() = %v, want %v", got, tc.want.deletable) + } + }) + } +} diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 1e8d934d..353da03b 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -99,33 +99,6 @@ type ConfigurationReconciler struct { ProviderName string } -// TFConfigurationMeta is all the metadata of a Configuration -type TFConfigurationMeta struct { - Name string - Namespace string - ConfigurationType types.ConfigurationType - CompleteConfiguration string - RemoteGit string - RemoteGitPath string - ConfigurationChanged bool - ConfigurationCMName string - BackendSecretName string - ApplyJobName string - DestroyJobName string - Envs []v1.EnvVar - ProviderReference *crossplane.Reference - VariableSecretName string - VariableSecretData map[string][]byte - DeleteResource bool - Credentials map[string]string - - // TerraformImage is the Terraform image which can run `terraform init/plan/apply` - TerraformImage string - TerraformBackendNamespace string - BusyboxImage string - GitImage string -} - // +kubebuilder:rbac:groups=terraform.core.oam.dev,resources=configurations,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=terraform.core.oam.dev,resources=configurations/status,verbs=get;update;patch @@ -141,7 +114,8 @@ func (r *ConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Reques meta := initTFConfigurationMeta(req, configuration) // add finalizer - if configuration.ObjectMeta.DeletionTimestamp.IsZero() { + var isDeleting = !configuration.ObjectMeta.DeletionTimestamp.IsZero() + if !isDeleting { if !controllerutil.ContainsFinalizer(&configuration, configurationFinalizer) { controllerutil.AddFinalizer(&configuration, configurationFinalizer) if err := r.Update(ctx, &configuration); err != nil { @@ -151,7 +125,7 @@ func (r *ConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Reques } // pre-check Configuration - if err := r.preCheck(ctx, &configuration, meta); err != nil { + if err := r.preCheck(ctx, &configuration, meta); err != nil && !isDeleting { return ctrl.Result{}, err } @@ -164,7 +138,7 @@ func (r *ConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Reques } } - if !configuration.ObjectMeta.DeletionTimestamp.IsZero() { + if isDeleting { // terraform destroy klog.InfoS("performing Configuration Destroy", "Namespace", req.Namespace, "Name", req.Name, "JobName", meta.DestroyJobName) @@ -210,6 +184,33 @@ func (r *ConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, nil } +// TFConfigurationMeta is all the metadata of a Configuration +type TFConfigurationMeta struct { + Name string + Namespace string + ConfigurationType types.ConfigurationType + CompleteConfiguration string + RemoteGit string + RemoteGitPath string + ConfigurationChanged bool + ConfigurationCMName string + BackendSecretName string + ApplyJobName string + DestroyJobName string + Envs []v1.EnvVar + ProviderReference *crossplane.Reference + VariableSecretName string + VariableSecretData map[string][]byte + DeleteResource bool + Credentials map[string]string + + // TerraformImage is the Terraform image which can run `terraform init/plan/apply` + TerraformImage string + TerraformBackendNamespace string + BusyboxImage string + GitImage string +} + func initTFConfigurationMeta(req ctrl.Request, configuration v1beta1.Configuration) *TFConfigurationMeta { var meta = &TFConfigurationMeta{ Namespace: req.Namespace, @@ -223,7 +224,7 @@ func initTFConfigurationMeta(req ctrl.Request, configuration v1beta1.Configurati // githubBlocked mark whether GitHub is blocked in the cluster githubBlockedStr := os.Getenv("GITHUB_BLOCKED") if githubBlockedStr == "" { - githubBlockedStr = "'false'" + githubBlockedStr = "false" } meta.RemoteGit = tfcfg.ReplaceTerraformSource(configuration.Spec.Remote, githubBlockedStr) @@ -234,14 +235,7 @@ func initTFConfigurationMeta(req ctrl.Request, configuration v1beta1.Configurati meta.RemoteGitPath = configuration.Spec.Path } - if configuration.Spec.ProviderReference != nil { - meta.ProviderReference = configuration.Spec.ProviderReference - } else { - meta.ProviderReference = &crossplane.Reference{ - Name: provider.DefaultName, - Namespace: provider.DefaultNamespace, - } - } + meta.ProviderReference = tfcfg.GetProviderNamespacedName(configuration) // Check the existence of Terraform state secret which is used to store TF state file. For detailed information, // please refer to https://www.terraform.io/docs/language/settings/backends/kubernetes.html#configuration-variables @@ -298,35 +292,28 @@ func (r *ConfigurationReconciler) terraformDestroy(ctx context.Context, namespac destroyJob batchv1.Job k8sClient = r.Client ) - if configuration.Status.Apply.State == types.ConfigurationProvisioningAndChecking { - warning := fmt.Sprintf("Destroy could not complete and needs to wait for Provision to complet first: %s", types.MessageCloudResourceProvisioningAndChecking) - klog.Warning(warning) - return errors.New(warning) - } - if !meta.DeleteResource { - // 1. delete Terraform input Configuration ConfigMap - if err := meta.deleteConfigMap(ctx, k8sClient); err != nil { - return err - } - - // 2. delete apply job - var applyJob batchv1.Job - if err := k8sClient.Get(ctx, client.ObjectKey{Name: meta.ApplyJobName, Namespace: namespace}, &applyJob); err == nil { - if err := k8sClient.Delete(ctx, &applyJob, client.PropagationPolicy(metav1.DeletePropagationBackground)); err != nil { - return err - } - } + deletable, err := tfcfg.IsDeletable(ctx, k8sClient, &configuration) + if err != nil { + return err } - if err := k8sClient.Get(ctx, client.ObjectKey{Name: meta.DestroyJobName, Namespace: meta.Namespace}, &destroyJob); err != nil { - if kerrors.IsNotFound(err) { - if err := r.Client.Get(ctx, client.ObjectKey{Name: configuration.Name, Namespace: configuration.Namespace}, &v1beta1.Configuration{}); err == nil { - if err = meta.assembleAndTriggerJob(ctx, k8sClient, &configuration, TerraformDestroy); err != nil { - return err + deleteConfigurationDirectly := deletable || !meta.DeleteResource + + if !deleteConfigurationDirectly { + if err := k8sClient.Get(ctx, client.ObjectKey{Name: meta.DestroyJobName, Namespace: meta.Namespace}, &destroyJob); err != nil { + if kerrors.IsNotFound(err) { + if err := r.Client.Get(ctx, client.ObjectKey{Name: configuration.Name, Namespace: configuration.Namespace}, &v1beta1.Configuration{}); err == nil { + if err = meta.assembleAndTriggerJob(ctx, k8sClient, &configuration, TerraformDestroy); err != nil { + return err + } } } } + if err := meta.updateTerraformJobIfNeeded(ctx, k8sClient, configuration, destroyJob); err != nil { + klog.ErrorS(err, types.ErrUpdateTerraformApplyJob, "Name", meta.ApplyJobName) + return errors.Wrap(err, types.ErrUpdateTerraformApplyJob) + } } // destroying @@ -334,13 +321,8 @@ func (r *ConfigurationReconciler) terraformDestroy(ctx context.Context, namespac return err } - if err := meta.updateTerraformJobIfNeeded(ctx, k8sClient, configuration, destroyJob); err != nil { - klog.ErrorS(err, types.ErrUpdateTerraformApplyJob, "Name", meta.ApplyJobName) - return errors.Wrap(err, types.ErrUpdateTerraformApplyJob) - } - // When the deletion Job process succeeded, clean up work is starting. - if destroyJob.Status.Succeeded == int32(1) { + if destroyJob.Status.Succeeded == int32(1) || deleteConfigurationDirectly { // 1. delete Terraform input Configuration ConfigMap if err := meta.deleteConfigMap(ctx, k8sClient); err != nil { return err @@ -453,13 +435,20 @@ func (r *ConfigurationReconciler) preCheck(ctx context.Context, configuration *v } // Check provider - if err := meta.checkProvider(ctx, k8sClient); err != nil { - if configuration.Status.Apply.State != types.ProviderNotReady { - if updateStatusErr := meta.updateApplyStatus(ctx, k8sClient, types.ProviderNotReady, types.ErrProviderNotReady); updateStatusErr != nil { - return errors.Wrap(updateStatusErr, errSettingStatus) - } + provider, err := provider.GetProviderFromConfiguration(ctx, k8sClient, meta.ProviderReference.Namespace, meta.ProviderReference.Name) + if provider == nil { + msg := types.ErrProviderNotFound + if err != nil { + msg = err.Error() + } + if updateStatusErr := meta.updateApplyStatus(ctx, k8sClient, types.Authorizing, msg); updateStatusErr != nil { + return errors.Wrap(updateStatusErr, msg) } - return errors.Wrap(err, types.ErrProviderNotReady) + return errors.New(msg) + } + + if err := meta.getCredentials(ctx, k8sClient, provider); err != nil { + return err } // Apply ClusterRole @@ -938,12 +927,8 @@ func (meta *TFConfigurationMeta) CheckWhetherConfigurationChanges(ctx context.Co return errors.New("unknown issue") } -// checkProver will check the Provider and get credentials from secret of the Provider -func (meta *TFConfigurationMeta) checkProvider(ctx context.Context, k8sClient client.Client) error { - providerObj, err := provider.GetProviderFromConfiguration(ctx, k8sClient, meta.ProviderReference.Namespace, meta.ProviderReference.Name) - if err != nil { - return errors.Wrap(err, "failed to get Provider from Configuration") - } +// getCredentials will get credentials from secret of the Provider +func (meta *TFConfigurationMeta) getCredentials(ctx context.Context, k8sClient client.Client, providerObj *v1beta1.Provider) error { region, err := tfcfg.SetRegion(ctx, k8sClient, meta.Namespace, meta.Name, providerObj) if err != nil { return err diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 123ee03a..5e0c4c5f 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -2,22 +2,30 @@ package controllers import ( "context" + "encoding/json" "reflect" "strings" "testing" - "gotest.tools/assert" + "github.com/agiledragon/gomonkey/v2" + "github.com/aliyun/alibaba-cloud-sdk-go/services/sts" + "github.com/stretchr/testify/assert" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" k8stypes "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/oam-dev/terraform-controller/api/types" crossplane "github.com/oam-dev/terraform-controller/api/types/crossplane-runtime" "github.com/oam-dev/terraform-controller/api/v1beta1" + "github.com/oam-dev/terraform-controller/controllers/provider" ) func TestInitTFConfigurationMeta(t *testing.T) { @@ -105,16 +113,7 @@ func TestCheckProvider(t *testing.T) { scheme := runtime.NewScheme() v1beta1.AddToScheme(scheme) - provider := &v1beta1.Provider{ - ObjectMeta: v1.ObjectMeta{ - Name: "default", - Namespace: "default", - }, - Status: v1beta1.ProviderStatus{ - State: types.ProviderIsNotReady, - }, - } - k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(provider).Build() + k8sClient1 := fake.NewClientBuilder().WithScheme(scheme).Build() meta := &TFConfigurationMeta{ ProviderReference: &crossplane.Reference{ @@ -123,21 +122,44 @@ func TestCheckProvider(t *testing.T) { }, } + type args struct { + k8sClient client.Client + provider *v1beta1.Provider + } + testcases := []struct { name string + args args want string }{ + { + name: "provider exists, and is not ready", + args: args{ + k8sClient: k8sClient1, + provider: &v1beta1.Provider{ + ObjectMeta: v1.ObjectMeta{ + Name: "default", + Namespace: "default", + }, + Status: v1beta1.ProviderStatus{ + State: types.ProviderIsNotReady, + }, + }, + }, + }, { name: "provider doesn't not exist", - want: "failed to get Provider from Configuration", + args: args{ + k8sClient: fake.NewClientBuilder().WithScheme(scheme).Build(), + }, }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - if err := meta.checkProvider(ctx, k8sClient); err != nil && + if err := meta.getCredentials(ctx, tc.args.k8sClient, tc.args.provider); tc.want != "" && !strings.Contains(err.Error(), tc.want) { - t.Errorf("checkProvider = %v, want %v", err.Error(), tc.want) + t.Errorf("getCredentials = %v, want %v", err.Error(), tc.want) } }) } @@ -148,22 +170,89 @@ func TestConfigurationReconcile(t *testing.T) { ctx := context.Background() s := runtime.NewScheme() v1beta1.AddToScheme(s) + corev1.AddToScheme(s) + batchv1.AddToScheme(s) r1.Client = fake.NewClientBuilder().WithScheme(s).Build() + ak := provider.AlibabaCloudCredentials{ + AccessKeyID: "aaaa", + AccessKeySecret: "bbbbb", + } + credentials, err := json.Marshal(&ak) + assert.Nil(t, err) + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: "default", + }, + Data: map[string][]byte{ + "credentials": credentials, + }, + Type: corev1.SecretTypeOpaque, + } + + provider := &v1beta1.Provider{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "terraform.core.oam.dev/v1beta1", + Kind: "Provider", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: "default", + }, + Spec: v1beta1.ProviderSpec{ + Provider: "alibaba", + Credentials: v1beta1.ProviderCredentials{ + Source: "Secret", + SecretRef: &crossplane.SecretKeySelector{ + SecretReference: crossplane.SecretReference{ + Name: "default", + Namespace: "default", + }, + Key: "credentials", + }, + }, + Region: "xxx", + }, + } + + configuration := &v1beta1.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "a", + Namespace: "b", + }, + Spec: v1beta1.ConfigurationSpec{ + HCL: "c", + }, + } + configuration.Spec.ProviderReference = &crossplane.Reference{ + Name: "default", + Namespace: "default", + } + + patches := gomonkey.ApplyMethod(reflect.TypeOf(&sts.Client{}), "GetCallerIdentity", func(_ *sts.Client, request *sts.GetCallerIdentityRequest) (response *sts.GetCallerIdentityResponse, err error) { + response = nil + err = nil + return + }) + defer patches.Reset() + + r2 := &ConfigurationReconciler{} + r2.Client = fake.NewClientBuilder().WithScheme(s).WithObjects(secret, provider, configuration).Build() + type args struct { req reconcile.Request r *ConfigurationReconciler } type want struct { - err error errMsg string } req := ctrl.Request{} req.NamespacedName = k8stypes.NamespacedName{ - Name: "abc", - Namespace: "default", + Name: "a", + Namespace: "b", } testcases := []struct { @@ -172,19 +261,28 @@ func TestConfigurationReconcile(t *testing.T) { want want }{ { - name: "Provider is not found", + name: "Configuration is not found", args: args{ req: req, r: r1, }, }, + { + name: "Configuration exists", + args: args{ + req: req, + r: r2, + }, + }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - if _, err := tc.args.r.Reconcile(ctx, tc.args.req); (err != nil) && - !strings.Contains(err.Error(), tc.want.errMsg) { - t.Errorf("Reconcile() error = %v, wantErr %v", err, tc.want.err) + for i := 0; i < 5; i++ { + if _, err := tc.args.r.Reconcile(ctx, tc.args.req); tc.want.errMsg != "" && + !strings.Contains(err.Error(), tc.want.errMsg) { + t.Errorf("Reconcile() error = %v, wantErr %v", err, tc.want.errMsg) + } } }) } @@ -214,7 +312,6 @@ func TestPreCheck(t *testing.T) { } type want struct { - err error errMsg string } @@ -262,17 +359,106 @@ func TestPreCheck(t *testing.T) { }, }, }, - want: want{ - errMsg: types.ErrProviderNotReady, - }, + want: want{}, }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - if err := tc.args.r.preCheck(ctx, tc.args.configuration, tc.args.meta); (err != nil) && + if err := tc.args.r.preCheck(ctx, tc.args.configuration, tc.args.meta); (tc.want.errMsg != "") && !strings.Contains(err.Error(), tc.want.errMsg) { - t.Errorf("preCheck() error = %v, wantErr %v", err, tc.want.err) + t.Errorf("preCheck() error = %v, wantErr %v", err, tc.want.errMsg) + } + }) + } +} + +func TestTerraformDestroy(t *testing.T) { + r1 := &ConfigurationReconciler{} + ctx := context.Background() + s := runtime.NewScheme() + v1beta1.AddToScheme(s) + corev1.AddToScheme(s) + batchv1.AddToScheme(s) + rbacv1.AddToScheme(s) + provider1 := &v1beta1.Provider{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: "default", + }, + Status: v1beta1.ProviderStatus{ + State: types.ProviderIsNotReady, + }, + } + k8sClient1 := fake.NewClientBuilder().WithScheme(s).WithObjects(provider1).Build() + r1.Client = k8sClient1 + + r2 := &ConfigurationReconciler{} + provider1.Status.State = types.ProviderIsReady + configuration := &v1beta1.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "b", + }, + } + k8sClient2 := fake.NewClientBuilder().WithScheme(s).WithObjects(provider1, configuration).Build() + r2.Client = k8sClient2 + + type args struct { + r *ConfigurationReconciler + namespace string + configuration *v1beta1.Configuration + k8sClient client.Client + meta *TFConfigurationMeta + } + type want struct { + errMsg string + } + testcases := []struct { + name string + args args + want want + }{ + { + name: "provider is not ready", + args: args{ + r: r1, + k8sClient: k8sClient1, + configuration: &v1beta1.Configuration{}, + meta: &TFConfigurationMeta{ + ConfigurationCMName: "tf-abc", + Namespace: "default", + }, + }, + want: want{ + errMsg: "The referenced provider could not be retrieved", + }, + }, + { + name: "provider is ready", + args: args{ + r: r2, + k8sClient: k8sClient2, + configuration: configuration, + meta: &TFConfigurationMeta{ + ConfigurationCMName: "tf-abc", + Namespace: "default", + DeleteResource: true, + }, + }, + want: want{ + errMsg: "The referenced provider could not be retrieved", + }, + }, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + err := tc.args.r.terraformDestroy(ctx, tc.args.namespace, *tc.args.configuration, tc.args.meta) + if err != nil { + if !strings.Contains(err.Error(), tc.want.errMsg) { + t.Errorf("terraformDestroy() error = %v, wantErr %v", err, tc.want.errMsg) + return + } } }) } diff --git a/controllers/provider/credentials.go b/controllers/provider/credentials.go index 80be604a..df6cc9f6 100644 --- a/controllers/provider/credentials.go +++ b/controllers/provider/credentials.go @@ -2,16 +2,15 @@ package provider import ( "context" - "fmt" "github.com/aliyun/alibaba-cloud-sdk-go/services/sts" "github.com/ghodss/yaml" "github.com/pkg/errors" v1 "k8s.io/api/core/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/oam-dev/terraform-controller/api/types" "github.com/oam-dev/terraform-controller/api/v1beta1" ) @@ -241,18 +240,20 @@ func GetProviderCredentials(ctx context.Context, k8sClient client.Client, provid } // GetProviderFromConfiguration gets provider object from Configuration +// Returns: +// 1) (nil, err): hit an issue to find the provider +// 2) (nil, nil): provider not found +// 3) (provider, nil): provider found func GetProviderFromConfiguration(ctx context.Context, k8sClient client.Client, namespace, name string) (*v1beta1.Provider, error) { var provider = &v1beta1.Provider{} if err := k8sClient.Get(ctx, client.ObjectKey{Name: name, Namespace: namespace}, provider); err != nil { + if kerrors.IsNotFound(err) { + return nil, nil + } errMsg := "failed to get Provider object" klog.ErrorS(err, errMsg, "Name", name) return nil, errors.Wrap(err, errMsg) } - if provider.Status.State != types.ProviderIsReady { - err := fmt.Errorf("provider is not ready: %s/%s", provider.Namespace, provider.Name) - klog.ErrorS(err, "failed to get credential") - return nil, err - } return provider, nil } diff --git a/controllers/provider/credentials_test.go b/controllers/provider/credentials_test.go index c2d4c3f4..6b4f7286 100644 --- a/controllers/provider/credentials_test.go +++ b/controllers/provider/credentials_test.go @@ -3,7 +3,9 @@ package provider import ( "context" "encoding/json" + "github.com/google/go-cmp/cmp" "reflect" + "strings" "testing" . "github.com/agiledragon/gomonkey/v2" @@ -223,3 +225,83 @@ func TestGetProviderCredentials(t *testing.T) { }) } } + +func TestGetProviderFromConfiguration(t *testing.T) { + ctx := context.Background() + k8sClient1 := fake.NewClientBuilder().Build() + + s := runtime.NewScheme() + v1beta1.AddToScheme(s) + provider := &v1beta1.Provider{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "terraform.core.oam.dev/v1beta1", + Kind: "Provider", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "a", + Namespace: "a", + }, + } + k8sClient2 := fake.NewClientBuilder().WithScheme(s).WithObjects(provider).Build() + + type args struct { + k8sClient client.Client + namespace string + name string + } + type want struct { + provider *v1beta1.Provider + errMsg string + } + testcases := []struct { + name string + args args + want want + }{ + { + name: "failed to get provider", + args: args{ + k8sClient: k8sClient1, + namespace: "a", + name: "b", + }, + want: want{ + errMsg: "failed to get Provider object", + }, + }, + { + name: "provider is not found", + args: args{ + k8sClient: k8sClient2, + namespace: "a", + name: "b", + }, + want: want{}, + }, + { + name: "provider is found", + args: args{ + k8sClient: k8sClient2, + namespace: "a", + name: "a", + }, + want: want{ + provider: provider, + }, + }, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + got, err := GetProviderFromConfiguration(ctx, tc.args.k8sClient, tc.args.namespace, tc.args.name) + if err != nil { + if !strings.Contains(err.Error(), tc.want.errMsg) { + t.Errorf("IsDeletable() error = %v, wantErr %v", err, tc.want.errMsg) + return + } + } + if tc.want.provider != nil && !reflect.DeepEqual(got, tc.want.provider) { + t.Errorf("IsDeletable() differs between got and want: %s", cmp.Diff(got, tc.want.provider)) + } + }) + } +} diff --git a/controllers/provider_controller_test.go b/controllers/provider_controller_test.go index 1d5431c9..e89b7e46 100644 --- a/controllers/provider_controller_test.go +++ b/controllers/provider_controller_test.go @@ -4,6 +4,7 @@ import ( "context" crossplanetypes "github.com/oam-dev/terraform-controller/api/types/crossplane-runtime" "github.com/oam-dev/terraform-controller/api/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -22,7 +23,7 @@ func TestReconcile(t *testing.T) { r2 := &ProviderReconciler{} provider := &v1beta1.Provider{ - ObjectMeta: ctrl.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "abc", Namespace: "default", }, @@ -46,7 +47,6 @@ func TestReconcile(t *testing.T) { } type want struct { - err error errMsg string } @@ -82,9 +82,9 @@ func TestReconcile(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - if _, err := tc.args.r.Reconcile(ctx, tc.args.req); (err != nil) && + if _, err := tc.args.r.Reconcile(ctx, tc.args.req); (tc.want.errMsg != "") && !strings.Contains(err.Error(), tc.want.errMsg) { - t.Errorf("Reconcile() error = %v, wantErr %v", err, tc.want.err) + t.Errorf("Reconcile() error = %v, wantErr %v", err, tc.want.errMsg) } }) }