Merge pull request #8 from negz/pro

Improve configuration of Terraform providers

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.
 

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"`

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
+      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"
+        }
+      }

examples/workspace-inline.yaml

@@ -8,37 +8,22 @@ 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.
       output "url" {
         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}"
       }

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:

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)}

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()},