Repository: crossplane-contrib/provider-terraform
Pull request #8 (2021-07-08 03:42:16)

Improve configuration of Terraform providers 
<!--
Thank you for helping to improve Crossplane!

Please read through https://git.io/fj2m9 if this is your first time opening a
Crossplane pull request. Find us in https://slack.crossplane.io/messages/dev if
you need any help contributing.
-->

### Description of your changes

<!--
Briefly describe what this pull request does. Be sure to direct your reviewers'
attention to anything that needs special consideration.

We love pull requests that resolve an open Crossplane issue. If yours does, you
can uncomment the below line to indicate which issue your PR fixes, for example
"Fixes #500":

-->
Fixes https://github.com/crossplane-contrib/provider-terraform/issues/7

This PR:

1. Makes it possible to provide a Terraform workspace with multiple credentials files, to make it easier for a workspace to use multiple Terraform providers.
1. Makes it possible to specify HCL at the provider config level that should be injected into all workspaces. This can be used to shift Terraform provider and remote state into the provider config scope, rather than needing each workspace to set them up.

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

<!--
Before reviewers can be confident in the correctness of this pull request, it
needs to tested and shown to be correct. Briefly describe the testing that has
already been done or which is planned for this change.
-->

[contribution process]: https://git.io/fj2m9

I've tested the updated examples to ensure they work.


Patch:
diff --git a/README.md b/README.md
index 7320cf6..90afaa7 100644
--- a/README.md
+++ b/README.md
@@ -16,7 +16,7 @@ metadata:
 spec:
   forProvider:
     # Use any module source supported by terraform init -from-module. You can
-    # also specify a simple main.tf inline; see examples/example-inline.
+    # also specify a simple main.tf inline; see examples/workspace-inline.yaml
     module: https://github.com/crossplane/tf
     # Variables can be specified inline.
     vars:
@@ -48,8 +48,9 @@ Known limitations:
 * You must either use remote state or ensure the provider container's `/tf`
   directory is not lost. `provider-terraform` __does not persist state__;
   consider using the [Kubernetes] remote state backend.
-* If the module takes longer than 20 minutes to apply the underlying `terraform`
-  process will be killed. You will potentially lose state and leak resources.
+* If the module takes longer than the supplied `--timeout` to apply the
+  underlying `terraform` process will be killed. You will potentially lose state
+  and leak resources.
 * The provider won't emit an event until _after_ it has successfully applied the
   Terraform module, which can take a long time.
 
diff --git a/apis/v1alpha1/types.go b/apis/v1alpha1/types.go
index 57b1373..ac01d77 100644
--- a/apis/v1alpha1/types.go
+++ b/apis/v1alpha1/types.go
@@ -25,11 +25,21 @@ import (
 // A ProviderConfigSpec defines the desired state of a ProviderConfig.
 type ProviderConfigSpec struct {
 	// Credentials required to authenticate to this provider.
-	Credentials ProviderCredentials `json:"credentials"`
+	Credentials []ProviderCredentials `json:"credentials"`
+
+	// Configuration that should be injected into all workspaces that use
+	// this provider config, expressed as inline HCL. This can be used to
+	// automatically inject Terraform provider configuration blocks.
+	// +optional
+	Configuration *string `json:"configuration,omitempty"`
 }
 
 // ProviderCredentials required to authenticate.
 type ProviderCredentials struct {
+	// Filename (relative to main.tf) to which these provider credentials
+	// should be written.
+	Filename string `json:"filename"`
+
 	// Source of the provider credentials.
 	// +kubebuilder:validation:Enum=None;Secret;InjectedIdentity;Environment;Filesystem
 	Source xpv1.CredentialsSource `json:"source"`
diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go
index 1c5c6f6..e23e444 100644
--- a/apis/v1alpha1/zz_generated.deepcopy.go
+++ b/apis/v1alpha1/zz_generated.deepcopy.go
@@ -101,7 +101,18 @@ func (in *ProviderConfigList) DeepCopyObject() runtime.Object {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ProviderConfigSpec) DeepCopyInto(out *ProviderConfigSpec) {
 	*out = *in
-	in.Credentials.DeepCopyInto(&out.Credentials)
+	if in.Credentials != nil {
+		in, out := &in.Credentials, &out.Credentials
+		*out = make([]ProviderCredentials, len(*in))
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
+		}
+	}
+	if in.Configuration != nil {
+		in, out := &in.Configuration, &out.Configuration
+		*out = new(string)
+		**out = **in
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProviderConfigSpec.
diff --git a/examples/providerconfig.yaml b/examples/providerconfig.yaml
index 5c0a378..6f407d4 100644
--- a/examples/providerconfig.yaml
+++ b/examples/providerconfig.yaml
@@ -1,23 +1,35 @@
-apiVersion: v1
-kind: Secret
-metadata:
-  namespace: crossplane-system
-  name: default-terraform-credentials
-type: Opaque
-data:
-  # credentials: BASE64ENCODED_PROVIDER_CREDS
----
 apiVersion: tf.crossplane.io/v1alpha1
 kind: ProviderConfig
 metadata:
   name: default
 spec:
-  # These will be written to a file named 'credentials' inside each Terraform
-  # configuration workdir - i.e. next to main.tf.
+  # Note that unlike most provider configs this one supports an array of
+  # credentials. This is because each Terraform workspace uses a single
+  # Crossplane provider config, but could use multiple Terraform providers each
+  # with their own credentials.
   credentials:
+  - filename: gcp-credentials.json
     source: Secret
     secretRef:
       namespace: crossplane-system
-      # name: default-terraform-credentials
       name: gcp-credentials
-      key: credentials
\ No newline at end of file
+      key: credentials
+  # This optional configuration block can be used to inject HCL into any
+  # workspace that uses this provider config, for example to setup Terraform
+  # providers.
+  configuration: |
+      provider "google" {
+        credentials = "gcp-credentials.json"
+        project     = "crossplane-example-project"
+      }
+
+      // Modules _must_ use remote state. The provider does not persist state.
+      // Consider using Kubernetes remote state.
+      // https://www.terraform.io/docs/language/settings/backends/kubernetes.html
+      terraform {
+        backend "gcs" {
+          bucket      = "crossplane-example-tf"
+          prefix      = "provider-terraform/default"
+          credentials = "gcp-credentials.json"
+        }
+      }
diff --git a/examples/workspace-inline.yaml b/examples/workspace-inline.yaml
index f1f68ab..17db5b6 100644
--- a/examples/workspace-inline.yaml
+++ b/examples/workspace-inline.yaml
@@ -8,9 +8,9 @@ metadata:
     crossplane.io/external-name: coolbucket
 spec:
   forProvider:
-    # The default source is 'Remote', which allows you to specify any module
-    # supported by terraform init -from-module, for example a git repository
-    # like https://github.com/crossplane/tf
+    # Workspaces default to using a remote source - like workspace-remote.yaml.
+    # For simple cases you can use an inline source to specify the content of
+    # main.tf as opaque, inline HCL.
     source: Inline
     module: |
       // Outputs are written to the connection secret.
@@ -18,27 +18,12 @@ spec:
         value       = google_storage_bucket.example.self_link
       }
       
-      // Modules _must_ use remote state. The provider does not persist state.
-      // Consider using Kubernetes remote state.
-      // https://www.terraform.io/docs/language/settings/backends/kubernetes.html
-      terraform {
-        backend "gcs" {
-          bucket      = "crossplane-example-tf"
-          prefix      = "terraform/state"
-          credentials = "credentials"
-        }
-      }
-      
-      provider "google" {
-        // ProviderConfig credentials are written to the "credentials" file.
-        credentials = "credentials"
-        project     = "crossplane-example-tf"
-      }
-      
       resource "random_id" "example" {
         byte_length = 4
       }
       
+      // The google provider and remote state are configured by the provider
+      // config - see providerconfig.yaml.
       resource "google_storage_bucket" "example" {
         name = "crossplane-example-${terraform.workspace}-${random_id.example.hex}"
       }
diff --git a/examples/workspace-remote.yaml b/examples/workspace-remote.yaml
index 1973883..cc284fa 100644
--- a/examples/workspace-remote.yaml
+++ b/examples/workspace-remote.yaml
@@ -8,10 +8,9 @@ metadata:
     crossplane.io/external-name: myworkspace
 spec:
   forProvider:
-    # The supplied module MUST use remote state; provider-terraform does not
-    # persist Terraform state. Any credentials loaded from a ProviderConfig will
-    # be written to a file named './credentials' relative to main.tf.
-    # You can also specify a simple main.tf inline; see example-inline.
+    # Any module supported by terraform init -from-module, for example a git
+    # repository. You can also specify a simple main.tf inline; see
+    # workspace-inline.yaml.
     module: https://github.com/crossplane/tf
     # Variables can be specified inline.
     vars:
diff --git a/internal/controller/workspace/workspace.go b/internal/controller/workspace/workspace.go
index 6b1d75c..af40dd7 100644
--- a/internal/controller/workspace/workspace.go
+++ b/internal/controller/workspace/workspace.go
@@ -50,26 +50,27 @@ const (
 	errGetPC        = "cannot get ProviderConfig"
 	errGetCreds     = "cannot get credentials"
 
-	errMkdir      = "cannot make Terraform configuration directory"
-	errWriteCreds = "cannot write Terraform credentials"
-	errWriteMain  = "cannot write Terraform configuration " + tfMain
-	errInit       = "cannot initialize Terraform configuration"
-	errWorkspace  = "cannot select Terraform workspace"
-	errResources  = "cannot list Terraform resources"
-	errDiff       = "cannot diff (i.e. plan) Terraform configuration"
-	errOutputs    = "cannot list Terraform outputs"
-	errOptions    = "cannot determine Terraform options"
-	errApply      = "cannot apply Terraform configuration"
-	errDestroy    = "cannot apply Terraform configuration"
-	errVarFile    = "cannot get tfvars"
+	errMkdir       = "cannot make Terraform configuration directory"
+	errWriteCreds  = "cannot write Terraform credentials"
+	errWriteConfig = "cannot write Terraform configuration " + tfConfig
+	errWriteMain   = "cannot write Terraform configuration " + tfMain
+	errInit        = "cannot initialize Terraform configuration"
+	errWorkspace   = "cannot select Terraform workspace"
+	errResources   = "cannot list Terraform resources"
+	errDiff        = "cannot diff (i.e. plan) Terraform configuration"
+	errOutputs     = "cannot list Terraform outputs"
+	errOptions     = "cannot determine Terraform options"
+	errApply       = "cannot apply Terraform configuration"
+	errDestroy     = "cannot apply Terraform configuration"
+	errVarFile     = "cannot get tfvars"
 )
 
 const (
 	// TODO(negz): Make the Terraform binary path and work dir configurable.
-	tfPath  = "terraform"
-	tfDir   = "/tf"
-	tfCreds = "credentials"
-	tfMain  = "main.tf"
+	tfPath   = "terraform"
+	tfDir    = "/tf"
+	tfMain   = "main.tf"
+	tfConfig = "crossplane-provider-config.tf"
 )
 
 type tfclient interface {
@@ -124,7 +125,11 @@ type connector struct {
 	terraform func(dir string) tfclient
 }
 
-func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.ExternalClient, error) {
+func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.ExternalClient, error) { //nolint:gocyclo
+	// NOTE(negz): This method is slightly over our complexity goal, but I
+	// can't immediately think of a clean way to decompose it without
+	// affecting readability.
+
 	cr, ok := mg.(*v1alpha1.Workspace)
 	if !ok {
 		return nil, errors.New(errNotWorkspace)
@@ -146,14 +151,21 @@ func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.E
 		return nil, errors.Wrap(err, errGetPC)
 	}
 
-	cd := pc.Spec.Credentials
-	data, err := resource.CommonCredentialExtractor(ctx, cd.Source, c.kube, cd.CommonCredentialSelectors)
-	if err != nil {
-		return nil, errors.Wrap(err, errGetCreds)
+	for _, cd := range pc.Spec.Credentials {
+		data, err := resource.CommonCredentialExtractor(ctx, cd.Source, c.kube, cd.CommonCredentialSelectors)
+		if err != nil {
+			return nil, errors.Wrap(err, errGetCreds)
+		}
+		p := filepath.Clean(filepath.Join(dir, filepath.Base(cd.Filename)))
+		if err := c.fs.WriteFile(p, data, 0600); err != nil {
+			return nil, errors.Wrap(err, errWriteCreds)
+		}
 	}
 
-	if err := c.fs.WriteFile(filepath.Join(dir, tfCreds), data, 0600); err != nil {
-		return nil, errors.Wrap(err, errWriteCreds)
+	if pc.Spec.Configuration != nil {
+		if err := c.fs.WriteFile(filepath.Join(dir, tfConfig), []byte(*pc.Spec.Configuration), 0600); err != nil {
+			return nil, errors.Wrap(err, errWriteConfig)
+		}
 	}
 
 	io := []terraform.InitOption{terraform.FromModule(cr.Spec.ForProvider.Module)}
diff --git a/package/crds/tf.crossplane.io_providerconfigs.yaml b/package/crds/tf.crossplane.io_providerconfigs.yaml
index 12f9a24..4bdfcfb 100644
--- a/package/crds/tf.crossplane.io_providerconfigs.yaml
+++ b/package/crds/tf.crossplane.io_providerconfigs.yaml
@@ -38,56 +38,66 @@ spec:
           spec:
             description: A ProviderConfigSpec defines the desired state of a ProviderConfig.
             properties:
+              configuration:
+                description: Configuration that should be injected into all workspaces that use this provider config, expressed as inline HCL. This can be used to automatically inject Terraform provider configuration blocks.
+                type: string
               credentials:
                 description: Credentials required to authenticate to this provider.
-                properties:
-                  env:
-                    description: Env is a reference to an environment variable that contains credentials that must be used to connect to the provider.
-                    properties:
-                      name:
-                        description: Name is the name of an environment variable.
-                        type: string
-                    required:
-                    - name
-                    type: object
-                  fs:
-                    description: Fs is a reference to a filesystem location that contains credentials that must be used to connect to the provider.
-                    properties:
-                      path:
-                        description: Path is a filesystem path.
-                        type: string
-                    required:
-                    - path
-                    type: object
-                  secretRef:
-                    description: A SecretRef is a reference to a secret key that contains the credentials that must be used to connect to the provider.
-                    properties:
-                      key:
-                        description: The key to select.
-                        type: string
-                      name:
-                        description: Name of the secret.
-                        type: string
-                      namespace:
-                        description: Namespace of the secret.
-                        type: string
-                    required:
-                    - key
-                    - name
-                    - namespace
-                    type: object
-                  source:
-                    description: Source of the provider credentials.
-                    enum:
-                    - None
-                    - Secret
-                    - InjectedIdentity
-                    - Environment
-                    - Filesystem
-                    type: string
-                required:
-                - source
-                type: object
+                items:
+                  description: ProviderCredentials required to authenticate.
+                  properties:
+                    env:
+                      description: Env is a reference to an environment variable that contains credentials that must be used to connect to the provider.
+                      properties:
+                        name:
+                          description: Name is the name of an environment variable.
+                          type: string
+                      required:
+                      - name
+                      type: object
+                    filename:
+                      description: Filename (relative to main.tf) to which these provider credentials should be written.
+                      type: string
+                    fs:
+                      description: Fs is a reference to a filesystem location that contains credentials that must be used to connect to the provider.
+                      properties:
+                        path:
+                          description: Path is a filesystem path.
+                          type: string
+                      required:
+                      - path
+                      type: object
+                    secretRef:
+                      description: A SecretRef is a reference to a secret key that contains the credentials that must be used to connect to the provider.
+                      properties:
+                        key:
+                          description: The key to select.
+                          type: string
+                        name:
+                          description: Name of the secret.
+                          type: string
+                        namespace:
+                          description: Namespace of the secret.
+                          type: string
+                      required:
+                      - key
+                      - name
+                      - namespace
+                      type: object
+                    source:
+                      description: Source of the provider credentials.
+                      enum:
+                      - None
+                      - Secret
+                      - InjectedIdentity
+                      - Environment
+                      - Filesystem
+                      type: string
+                  required:
+                  - filename
+                  - source
+                  type: object
+                type: array
             required:
             - credentials
             type: object


Test patch:
diff --git a/internal/controller/workspace/workspace_test.go b/internal/controller/workspace/workspace_test.go
index bdcf7d2..8c4524d 100644
--- a/internal/controller/workspace/workspace_test.go
+++ b/internal/controller/workspace/workspace_test.go
@@ -101,6 +101,7 @@ func (tf *MockTf) Destroy(ctx context.Context, o ...terraform.Option) error {
 func TestConnect(t *testing.T) {
 	errBoom := errors.New("boom")
 	uid := types.UID("no-you-id")
+	tfCreds := "credentials"
 
 	type fields struct {
 		kube      client.Client
@@ -189,7 +190,9 @@ func TestConnect(t *testing.T) {
 							// here. We cause an error to be returned by asking
 							// for credentials from the environment, but not
 							// specifying an environment variable.
-							pc.Spec.Credentials.Source = xpv1.CredentialsSourceEnvironment
+							pc.Spec.Credentials = []v1alpha1.ProviderCredentials{{
+								Source: xpv1.CredentialsSourceEnvironment,
+							}}
 						}
 						return nil
 					}),
@@ -215,7 +218,10 @@ func TestConnect(t *testing.T) {
 				kube: &test.MockClient{
 					MockGet: test.NewMockGetFn(nil, func(obj client.Object) error {
 						if pc, ok := obj.(*v1alpha1.ProviderConfig); ok {
-							pc.Spec.Credentials.Source = xpv1.CredentialsSourceNone
+							pc.Spec.Credentials = []v1alpha1.ProviderCredentials{{
+								Filename: tfCreds,
+								Source:   xpv1.CredentialsSourceNone,
+							}}
 						}
 						return nil
 					}),
@@ -240,18 +246,49 @@ func TestConnect(t *testing.T) {
 			},
 			want: errors.Wrap(errBoom, errWriteCreds),
 		},
-		"WriteMainError": {
-			reason: "We should return any error encountered while writing our main.tf file",
+		"WriteConfigError": {
+			reason: "We should return any error encountered while writing our crossplane-provider-config.tf file",
 			fields: fields{
 				kube: &test.MockClient{
 					MockGet: test.NewMockGetFn(nil, func(obj client.Object) error {
 						if pc, ok := obj.(*v1alpha1.ProviderConfig); ok {
-							pc.Spec.Credentials.Source = xpv1.CredentialsSourceNone
+							cfg := "I'm HCL!"
+							pc.Spec.Configuration = &cfg
 						}
 						return nil
 					}),
 				},
 				usage: resource.TrackerFn(func(_ context.Context, _ resource.Managed) error { return nil }),
+				fs: afero.Afero{
+					Fs: &ErrFs{
+						Fs:   afero.NewMemMapFs(),
+						errs: map[string]error{filepath.Join(tfDir, string(uid), tfConfig): errBoom},
+					},
+				},
+			},
+			args: args{
+				mg: &v1alpha1.Workspace{
+					ObjectMeta: metav1.ObjectMeta{UID: uid},
+					Spec: v1alpha1.WorkspaceSpec{
+						ResourceSpec: xpv1.ResourceSpec{
+							ProviderConfigReference: &xpv1.Reference{},
+						},
+						ForProvider: v1alpha1.WorkspaceParameters{
+							Module: "I'm HCL!",
+							Source: v1alpha1.ModuleSourceInline,
+						},
+					},
+				},
+			},
+			want: errors.Wrap(errBoom, errWriteConfig),
+		},
+		"WriteMainError": {
+			reason: "We should return any error encountered while writing our main.tf file",
+			fields: fields{
+				kube: &test.MockClient{
+					MockGet: test.NewMockGetFn(nil),
+				},
+				usage: resource.TrackerFn(func(_ context.Context, _ resource.Managed) error { return nil }),
 				fs: afero.Afero{
 					Fs: &ErrFs{
 						Fs:   afero.NewMemMapFs(),
@@ -279,12 +316,7 @@ func TestConnect(t *testing.T) {
 			reason: "We should return any error encountered while initializing Terraform",
 			fields: fields{
 				kube: &test.MockClient{
-					MockGet: test.NewMockGetFn(nil, func(obj client.Object) error {
-						if pc, ok := obj.(*v1alpha1.ProviderConfig); ok {
-							pc.Spec.Credentials.Source = xpv1.CredentialsSourceNone
-						}
-						return nil
-					}),
+					MockGet: test.NewMockGetFn(nil),
 				},
 				usage: resource.TrackerFn(func(_ context.Context, _ resource.Managed) error { return nil }),
 				fs:    afero.Afero{Fs: afero.NewMemMapFs()},
@@ -308,12 +340,7 @@ func TestConnect(t *testing.T) {
 			reason: "We should return any error encountered while selecting a Terraform workspace",
 			fields: fields{
 				kube: &test.MockClient{
-					MockGet: test.NewMockGetFn(nil, func(obj client.Object) error {
-						if pc, ok := obj.(*v1alpha1.ProviderConfig); ok {
-							pc.Spec.Credentials.Source = xpv1.CredentialsSourceNone
-						}
-						return nil
-					}),
+					MockGet: test.NewMockGetFn(nil),
 				},
 				usage: resource.TrackerFn(func(_ context.Context, _ resource.Managed) error { return nil }),
 				fs:    afero.Afero{Fs: afero.NewMemMapFs()},
@@ -340,12 +367,7 @@ func TestConnect(t *testing.T) {
 			reason: "We should not return an error when we successfully 'connect' to Terraform",
 			fields: fields{
 				kube: &test.MockClient{
-					MockGet: test.NewMockGetFn(nil, func(obj client.Object) error {
-						if pc, ok := obj.(*v1alpha1.ProviderConfig); ok {
-							pc.Spec.Credentials.Source = xpv1.CredentialsSourceNone
-						}
-						return nil
-					}),
+					MockGet: test.NewMockGetFn(nil),
 				},
 				usage: resource.TrackerFn(func(_ context.Context, _ resource.Managed) error { return nil }),
 				fs:    afero.Afero{Fs: afero.NewMemMapFs()},