Repository: crossplane-contrib/provider-terraform (go)

Issue:
Workspace object hangs when terraform plan fails for destroy
### What happened?
The Workspace object is hung after delete was requested because the Observe is failing.  The terraform plan command fails because the AWS VPC object that the terraform is referencing has already been deleted:

```
Status:
  At Provider:
  Conditions:
    Last Transition Time:  2022-04-08T16:37:35Z
    Reason:                Deleting
    Status:                False
    Type:                  Ready
    Last Transition Time:  2022-04-08T16:38:06Z
    Message:               observe failed: cannot diff (i.e. plan) Terraform configuration: no matching ec2 vpc found
    Reason:                ReconcileError
    Status:                False
    Type:                  Synced
Events:
  Type     Reason                         Age                      From                                Message
  ----     ------                         ----                     ----                                -------
  Warning  CannotObserveExternalResource  3m17s (x4536 over 5d5h)  managed/workspace.tf.crossplane.io  cannot diff (i.e. plan) Terraform configuration: no matching ec2 vpc found
```

Removing the finalizer finishes the delete process on the Workspace but there needs to be a way to destroy the workspace when plan fails on destroy.

### How can we reproduce it?
Create a composition that creates a VPC.ec2.aws resource and references that resource in a Workspace, using data "aws_vpc".

When the claim gets deleted, if the VPC object is deleted from AWS before the Workspace delete executes, the problem occurs.

### What environment did it happen in?
Crossplane version: 1.7

* Cloud provider or hardware configuration AWS


Pull request description:
Handle terraform plan errors on deleted resources
Signed-off-by: Bob Haddleton <bob.haddleton@nokia.com>

### Description of your changes
Update the Observe routine to check for deleted timestamp when terraform plan fails.  If the workspace depends on other resources that have already been deleted, the terraform plan will always fail and the workspace will never get deleted.

Fixes #47 

I have:

- [X] Read and followed Crossplane's [contribution process].
- [X] Run `make reviewable test` to ensure this PR is ready for review.

### How has this code been tested
Added a unit test to cover the scenario and tested locally with Kind.

[contribution process]: https://git.io/fj2m9


Patch:
diff --git a/.golangci.yml b/.golangci.yml
index 3412bb7..761867c 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -16,7 +16,7 @@ linters-settings:
 
     # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
     # default is false: such cases aren't reported by default.
-    check-blank: false
+    check-blank: true
 
     # [deprecated] comma-separated list of pairs of the form pkg:regex
     # the regex is used to ignore names within pkg. (default "fmt:.*").
diff --git a/README.md b/README.md
index ac98de5..75bd8f3 100644
--- a/README.md
+++ b/README.md
@@ -71,6 +71,16 @@ spec:
     name: terraform-workspace-example-inline
 ```
 
+## Installation
+
+We highly encourage to use a declarative way of provider installation:
+
+```
+kubectl apply -f examples/install.yaml
+```
+
+Notice that in this example Provider resource is referencing ControllerConfig with debug enabled.
+
 ## Private Git repository support
 
 To securely propagate git credentials create a `git-credentials` secret in [git credentials store] format.
@@ -159,6 +169,49 @@ Appear in the corresponding outputs section as:
 ```
 Note that the "sensitive" output is not included in status.atProvider.outputs
 
+## Terraform CLI Command Arguments
+Additional arguments can be passed to the Terraform plan, apply, and destroy commands by specifying
+the planArgs, applyArgs and destroyArgs options.
+
+For example:
+```yaml
+apiVersion: tf.crossplane.io/v1alpha1
+kind: Workspace
+metadata:
+  name: example-args
+spec:
+  forProvider:
+    # Run the terraform init command with -upgrade=true to upgrade any stored providers
+    initArgs:
+      - -upgrade=true
+    # Run the terraform plan command with the -parallelism=2 argument
+    planArgs:
+      - -parallelism=2
+    # Run the terraform apply command with the -target=specificresource argument
+    applyArgs:
+      - -target=specificresource
+    # Run the terraform destroy command with the -refresh=false argument
+    destroyArgs:
+      - -refresh=false
+    # Use any module source supported by terraform init -from-module.
+    source: Remote
+    module: https://github.com/crossplane/tf
+  # All Terraform outputs are written to the connection secret.
+  writeConnectionSecretToRef:
+    namespace: default
+    name: terraform-workspace-example-inline
+```
+This will cause the _terraform init_ command to be run with the "-upgrade=true" argument,
+the _terraform plan_ command to be run with the -parallelism=2 argument,
+the _terraform apply_ command to be run with the -target=specificresource argument,
+and the _terraform destroy_ command to be run with the -refresh=false argument.
+
+Note that by default the terraform _init_ command is run with the "-input=false", and "-no-color" arguments,
+the terraform _apply_ and _destroy_ commands are run with the
+"-no-color", "-auto-approve", and "-input=false" arguments, and the terraform _plan_ command is
+run with the "-no-color", "-input=false", and "-detailed-exitcode" arguments.  Arguments specified in
+applyArgs, destroyArgs and planArgs will be added to these default arguments.
+
 ## Known limitations:
 
 * You must either use remote state or ensure the provider container's `/tf`
diff --git a/apis/v1alpha1/workspace_types.go b/apis/v1alpha1/workspace_types.go
index 0629777..0422d2e 100644
--- a/apis/v1alpha1/workspace_types.go
+++ b/apis/v1alpha1/workspace_types.go
@@ -108,6 +108,18 @@ type WorkspaceParameters struct {
 	// precedence.
 	// +optional
 	VarFiles []VarFile `json:"varFiles,omitempty"`
+
+	// Arguments to be included in the terraform init CLI command
+	InitArgs []string `json:"initArgs,omitempty"`
+
+	// Arguments to be included in the terraform plan CLI command
+	PlanArgs []string `json:"planArgs,omitempty"`
+
+	// Arguments to be included in the terraform apply CLI command
+	ApplyArgs []string `json:"applyArgs,omitempty"`
+
+	// Arguments to be included in the terraform destroy CLI command
+	DestroyArgs []string `json:"destroyArgs,omitempty"`
 }
 
 // WorkspaceObservation are the observable fields of a Workspace.
diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go
index 8ebf1d6..2d2362c 100644
--- a/apis/v1alpha1/zz_generated.deepcopy.go
+++ b/apis/v1alpha1/zz_generated.deepcopy.go
@@ -357,6 +357,26 @@ func (in *WorkspaceParameters) DeepCopyInto(out *WorkspaceParameters) {
 			(*in)[i].DeepCopyInto(&(*out)[i])
 		}
 	}
+	if in.InitArgs != nil {
+		in, out := &in.InitArgs, &out.InitArgs
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
+	if in.PlanArgs != nil {
+		in, out := &in.PlanArgs, &out.PlanArgs
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
+	if in.ApplyArgs != nil {
+		in, out := &in.ApplyArgs, &out.ApplyArgs
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
+	if in.DestroyArgs != nil {
+		in, out := &in.DestroyArgs, &out.DestroyArgs
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WorkspaceParameters.
diff --git a/examples/install.yaml b/examples/install.yaml
new file mode 100644
index 0000000..fdc1673
--- /dev/null
+++ b/examples/install.yaml
@@ -0,0 +1,19 @@
+---
+apiVersion: pkg.crossplane.io/v1alpha1
+kind: ControllerConfig
+metadata:
+  name: terraform-config
+  labels:
+    app: crossplane-provider-terraform
+spec:
+  image: crossplane/provider-terraform-controller:v0.2.0
+  args: ["-d"]
+---
+apiVersion: pkg.crossplane.io/v1
+kind: Provider
+metadata:
+  name: crossplane-provider-terraform
+spec:
+  package: crossplane/provider-terraform:v0.2.0
+  controllerConfigRef:
+    name: terraform-config
diff --git a/internal/controller/workspace/workspace.go b/internal/controller/workspace/workspace.go
index 518880a..713b171 100644
--- a/internal/controller/workspace/workspace.go
+++ b/internal/controller/workspace/workspace.go
@@ -24,6 +24,7 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/spf13/afero"
+	coordv1 "k8s.io/api/coordination/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/workqueue"
@@ -39,10 +40,11 @@ import (
 	"github.com/crossplane/crossplane-runtime/pkg/reconciler/managed"
 	"github.com/crossplane/crossplane-runtime/pkg/resource"
 
+	"github.com/hashicorp/go-getter"
+
 	"github.com/crossplane-contrib/provider-terraform/apis/v1alpha1"
 	"github.com/crossplane-contrib/provider-terraform/internal/terraform"
 	"github.com/crossplane-contrib/provider-terraform/internal/workdir"
-	getter "github.com/hashicorp/go-getter"
 )
 
 const (
@@ -64,8 +66,12 @@ const (
 	errOutputs       = "cannot list Terraform outputs"
 	errOptions       = "cannot determine Terraform options"
 	errApply         = "cannot apply Terraform configuration"
-	errDestroy       = "cannot apply Terraform configuration"
+	errDestroy       = "cannot destroy Terraform configuration"
 	errVarFile       = "cannot get tfvars"
+	errListLeases    = "cannot get list of Lease objects"
+	errListSecrets   = "cannot get list of Secret objects"
+	errDeleteSecret  = "cannot delete Secret for Workspace"
+	errDeleteLease   = "cannot delete Least for Workspace"
 
 	gitCredentialsFilename = ".git-credentials"
 )
@@ -228,7 +234,9 @@ func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.E
 	}
 
 	tf := c.terraform(dir)
-	if err := tf.Init(ctx); err != nil {
+	o := make([]terraform.InitOption, 0, len(cr.Spec.ForProvider.InitArgs))
+	o = append(o, terraform.WithInitArgs(cr.Spec.ForProvider.InitArgs))
+	if err := tf.Init(ctx, o...); err != nil {
 		return nil, errors.Wrap(err, errInit)
 	}
 	// TODO(ytsarev): cache .terraform in /tmp to speed up `terraform init` on next reconcile
@@ -238,7 +246,7 @@ func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.E
 
 type external struct {
 	tf   tfclient
-	kube client.Reader
+	kube client.Client
 }
 
 func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.ExternalObservation, error) {
@@ -252,8 +260,15 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex
 		return managed.ExternalObservation{}, errors.Wrap(err, errOptions)
 	}
 
+	o = append(o, terraform.WithArgs(cr.Spec.ForProvider.PlanArgs))
 	differs, err := c.tf.Diff(ctx, o...)
 	if err != nil {
+		if meta.WasDeleted(mg) {
+			if derr := c.Delete(ctx, mg); derr != nil {
+				return managed.ExternalObservation{}, errors.Wrap(derr, errDiff)
+			}
+			return managed.ExternalObservation{ResourceExists: false}, nil
+		}
 		return managed.ExternalObservation{}, errors.Wrap(err, errDiff)
 	}
 
@@ -295,6 +310,7 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
 		return managed.ExternalUpdate{}, errors.Wrap(err, errOptions)
 	}
 
+	o = append(o, terraform.WithArgs(cr.Spec.ForProvider.ApplyArgs))
 	if err := c.tf.Apply(ctx, o...); err != nil {
 		return managed.ExternalUpdate{}, errors.Wrap(err, errApply)
 	}
@@ -322,11 +338,36 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) error {
 		return errors.Wrap(err, errOptions)
 	}
 
-	return errors.Wrap(c.tf.Destroy(ctx, o...), errDestroy)
+	o = append(o, terraform.WithArgs(cr.Spec.ForProvider.DestroyArgs))
+	if err := c.tf.Destroy(ctx, o...); err != nil {
+		return errors.Wrap(err, errDestroy)
+	}
+	labels := map[string]string{"tfstate": "true", "tfstateWorkspace": cr.Name}
+	sl := &corev1.SecretList{}
+	if err := c.kube.List(ctx, sl, client.MatchingLabels(labels)); err != nil {
+		return errors.Wrap(err, errListSecrets)
+	}
+	for s := range sl.Items {
+		sec := sl.Items[s]
+		if err := c.kube.Delete(ctx, &sec); err != nil {
+			return errors.Wrap(err, errDeleteSecret)
+		}
+	}
+	ll := &coordv1.LeaseList{}
+	if err := c.kube.List(ctx, ll, client.MatchingLabels(labels)); err != nil {
+		return errors.Wrap(err, errListLeases)
+	}
+	for l := range ll.Items {
+		ls := ll.Items[l]
+		if err := c.kube.Delete(ctx, &ls); err != nil {
+			return errors.Wrap(err, errDeleteLease)
+		}
+	}
+	return nil
 }
 
 func (c *external) options(ctx context.Context, p v1alpha1.WorkspaceParameters) ([]terraform.Option, error) {
-	o := make([]terraform.Option, 0, len(p.Vars)+len(p.VarFiles))
+	o := make([]terraform.Option, 0, len(p.Vars)+len(p.VarFiles)+len(p.DestroyArgs)+len(p.ApplyArgs)+len(p.PlanArgs))
 
 	for _, v := range p.Vars {
 		o = append(o, terraform.WithVar(v.Key, v.Value))
diff --git a/internal/terraform/terraform.go b/internal/terraform/terraform.go
index 8133437..fe2a1eb 100644
--- a/internal/terraform/terraform.go
+++ b/internal/terraform/terraform.go
@@ -108,6 +108,13 @@ func FromModule(module string) InitOption {
 	}
 }
 
+// WithInitArgs supplies a list of Terraform argument.
+func WithInitArgs(v []string) InitOption {
+	return func(o *initOptions) {
+		o.args = append(o.args, v...)
+	}
+}
+
 // Init initializes a Terraform configuration.
 func (h Harness) Init(ctx context.Context, o ...InitOption) error {
 	io := &initOptions{}
@@ -333,6 +340,13 @@ type options struct {
 // An Option affects how a Terraform is invoked.
 type Option func(o *options)
 
+// WithArgs supplies a list of Terraform argument.
+func WithArgs(v []string) Option {
+	return func(o *options) {
+		o.args = append(o.args, v...)
+	}
+}
+
 // WithVar supplies a Terraform variable.
 func WithVar(k, v string) Option {
 	return func(o *options) {
diff --git a/package/crds/tf.crossplane.io_workspaces.yaml b/package/crds/tf.crossplane.io_workspaces.yaml
index 26a5078..590d1c1 100644
--- a/package/crds/tf.crossplane.io_workspaces.yaml
+++ b/package/crds/tf.crossplane.io_workspaces.yaml
@@ -50,9 +50,29 @@ spec:
               forProvider:
                 description: WorkspaceParameters are the configurable fields of a Workspace.
                 properties:
+                  applyArgs:
+                    description: Arguments to be included in the terraform apply CLI command
+                    items:
+                      type: string
+                    type: array
+                  destroyArgs:
+                    description: Arguments to be included in the terraform destroy CLI command
+                    items:
+                      type: string
+                    type: array
+                  initArgs:
+                    description: Arguments to be included in the terraform init CLI command
+                    items:
+                      type: string
+                    type: array
                   module:
                     description: The root module of this workspace; i.e. the module containing its main.tf file. When the workspace's source is 'Remote' (the default) this can be any address supported by terraform init -from-module, for example a git repository or an S3 bucket. When the workspace's source is 'Inline' the content of a simple main.tf file may be written inline.
                     type: string
+                  planArgs:
+                    description: Arguments to be included in the terraform plan CLI command
+                    items:
+                      type: string
+                    type: array
                   source:
                     description: Source of the root module of this workspace.
                     enum:


Test patch:
diff --git a/internal/controller/workspace/workspace_test.go b/internal/controller/workspace/workspace_test.go
index 6029038..7fea723 100644
--- a/internal/controller/workspace/workspace_test.go
+++ b/internal/controller/workspace/workspace_test.go
@@ -445,6 +445,9 @@ func TestConnect(t *testing.T) {
 				mg: &v1alpha1.Workspace{
 					ObjectMeta: metav1.ObjectMeta{UID: uid},
 					Spec: v1alpha1.WorkspaceSpec{
+						ForProvider: v1alpha1.WorkspaceParameters{
+							InitArgs: []string{"-upgrade=true"},
+						},
 						ResourceSpec: xpv1.ResourceSpec{
 							ProviderConfigReference: &xpv1.Reference{},
 						},
@@ -473,10 +476,11 @@ func TestConnect(t *testing.T) {
 
 func TestObserve(t *testing.T) {
 	errBoom := errors.New("boom")
+	now := metav1.Now()
 
 	type fields struct {
 		tf   tfclient
-		kube client.Reader
+		kube client.Client
 	}
 
 	type args struct {
@@ -579,6 +583,50 @@ func TestObserve(t *testing.T) {
 				err: errors.Wrap(errBoom, errDiff),
 			},
 		},
+		"DiffErrorDeleted": {
+			reason: "We should call the Delete function when an error is encountered while diffing the Terraform configuration on a deleted Workspace",
+			fields: fields{
+				tf: &MockTf{
+					MockDestroy: func(_ context.Context, _ ...terraform.Option) error { return nil },
+					MockDiff:    func(ctx context.Context, o ...terraform.Option) (bool, error) { return false, errBoom },
+				},
+				kube: &test.MockClient{
+					MockDelete: test.NewMockDeleteFn(nil),
+					MockGet:    test.NewMockGetFn(nil),
+					MockList:   test.NewMockListFn(nil),
+				},
+			},
+			args: args{
+				mg: &v1alpha1.Workspace{
+					ObjectMeta: metav1.ObjectMeta{
+						DeletionTimestamp: &now,
+					},
+				},
+			},
+			want: want{
+				o: managed.ExternalObservation{ResourceExists: false},
+			},
+		},
+		"DiffErrorDeletedDestroyError": {
+			reason: "We should raise an error when the Delete function fails after Diff fails on a deleted resource",
+			fields: fields{
+				tf: &MockTf{
+					MockDestroy: func(_ context.Context, _ ...terraform.Option) error { return errBoom },
+					MockDiff:    func(ctx context.Context, o ...terraform.Option) (bool, error) { return false, errBoom },
+				},
+			},
+			args: args{
+				mg: &v1alpha1.Workspace{
+					ObjectMeta: metav1.ObjectMeta{
+						DeletionTimestamp: &now,
+					},
+				},
+			},
+			want: want{
+				o:   managed.ExternalObservation{},
+				err: errors.Wrap(errors.Wrap(errBoom, errDestroy), errDiff),
+			},
+		},
 		"ResourcesError": {
 			reason: "We should return any error encountered while listing extant Terraform resources",
 			fields: fields{
@@ -650,7 +698,13 @@ func TestObserve(t *testing.T) {
 				},
 			},
 			args: args{
-				mg: &v1alpha1.Workspace{},
+				mg: &v1alpha1.Workspace{
+					Spec: v1alpha1.WorkspaceSpec{
+						ForProvider: v1alpha1.WorkspaceParameters{
+							PlanArgs: []string{"-refresh=false"},
+						},
+					},
+				},
 			},
 			want: want{
 				o: managed.ExternalObservation{
@@ -694,7 +748,7 @@ func TestCreate(t *testing.T) {
 
 	type fields struct {
 		tf   tfclient
-		kube client.Reader
+		kube client.Client
 	}
 
 	type args struct {
@@ -832,7 +886,8 @@ func TestCreate(t *testing.T) {
 				mg: &v1alpha1.Workspace{
 					Spec: v1alpha1.WorkspaceSpec{
 						ForProvider: v1alpha1.WorkspaceParameters{
-							Vars: []v1alpha1.Var{{Key: "super", Value: "cool"}},
+							ApplyArgs: []string{"-refresh=false"},
+							Vars:      []v1alpha1.Var{{Key: "super", Value: "cool"}},
 							VarFiles: []v1alpha1.VarFile{
 								{
 									Source:                v1alpha1.VarFileSourceConfigMapKey,
@@ -888,7 +943,7 @@ func TestDelete(t *testing.T) {
 
 	type fields struct {
 		tf   tfclient
-		kube client.Reader
+		kube client.Client
 	}
 
 	type args struct {
@@ -975,7 +1030,7 @@ func TestDelete(t *testing.T) {
 			args: args{
 				mg: &v1alpha1.Workspace{},
 			},
-			want: errors.Wrap(errBoom, errApply),
+			want: errors.Wrap(errBoom, errDestroy),
 		},
 		"Success": {
 			reason: "We should not return an error if we successfully destroy the Terraform configuration",
@@ -984,14 +1039,17 @@ func TestDelete(t *testing.T) {
 					MockDestroy: func(_ context.Context, _ ...terraform.Option) error { return nil },
 				},
 				kube: &test.MockClient{
-					MockGet: test.NewMockGetFn(nil),
+					MockDelete: test.NewMockDeleteFn(nil),
+					MockGet:    test.NewMockGetFn(nil),
+					MockList:   test.NewMockListFn(nil),
 				},
 			},
 			args: args{
 				mg: &v1alpha1.Workspace{
 					Spec: v1alpha1.WorkspaceSpec{
 						ForProvider: v1alpha1.WorkspaceParameters{
-							Vars: []v1alpha1.Var{{Key: "super", Value: "cool"}},
+							DestroyArgs: []string{"-refresh=false"},
+							Vars:        []v1alpha1.Var{{Key: "super", Value: "cool"}},
 							VarFiles: []v1alpha1.VarFile{
 								{
 									Source:                v1alpha1.VarFileSourceConfigMapKey,