Merge branch 'fix-244' of https://github.com/redsnapper2006/terraform…

…-controller into fix-244

CONTRIBUTING.md

@@ -20,13 +20,17 @@ Refer to [Helm official Doc](https://helm.sh/docs/intro/install/) to install `he
 $ make install
 go: creating new go.mod: module tmp
 ...
-go get: added sigs.k8s.io/controller-tools v0.6.0
-go get: added sigs.k8s.io/structured-merge-diff/v4 v4.1.0
-go get: added sigs.k8s.io/yaml v1.2.0
+go: downloading sigs.k8s.io/controller-tools v0.6.0
+go: downloading k8s.io/apiextensions-apiserver v0.21.1
+go: downloading k8s.io/apimachinery v0.21.1
+go: downloading k8s.io/api v0.21.1
+go: downloading k8s.io/utils v0.0.0-20201110183641-67b214c5f920
+go: downloading k8s.io/klog/v2 v2.8.0
+go: downloading sigs.k8s.io/structured-merge-diff/v4 v4.1.0
 /Users/zhouzhengxi/go/bin/controller-gen "crd:trivialVersions=true" webhook paths="./..." output:crd:artifacts:config=chart/crds
 kubectl apply -f chart/crds
-customresourcedefinition.apiextensions.k8s.io/configurations.terraform.core.oam.dev configured
-customresourcedefinition.apiextensions.k8s.io/providers.terraform.core.oam.dev configured
+customresourcedefinition.apiextensions.k8s.io/configurations.terraform.core.oam.dev created
+customresourcedefinition.apiextensions.k8s.io/providers.terraform.core.oam.dev created
 ```
 
 - Run Terraform Controller
@@ -35,7 +39,7 @@ customresourcedefinition.apiextensions.k8s.io/providers.terraform.core.oam.dev c
 $ make run
 go: creating new go.mod: module tmp
 ...
-go get: added sigs.k8s.io/yaml v1.2.0
+go: downloading sigs.k8s.io/yaml v1.2.0
 /Users/zhouzhengxi/go/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
 go fmt ./...
 go vet ./...

Makefile

@@ -82,7 +82,7 @@ ifeq (, $(shell which controller-gen))
 	CONTROLLER_GEN_TMP_DIR=$$(mktemp -d) ;\
 	cd $$CONTROLLER_GEN_TMP_DIR ;\
 	go mod init tmp ;\
-	go get sigs.k8s.io/controller-tools/cmd/[email protected] ;\
+	go install sigs.k8s.io/controller-tools/cmd/[email protected] ;\
 	rm -rf $$CONTROLLER_GEN_TMP_DIR ;\
 	}
 CONTROLLER_GEN=$(GOBIN)/controller-gen

README.md

@@ -28,7 +28,7 @@ Terraform Controller is a Kubernetes Controller for Terraform.
 ## Supported Terraform Configuration
 
 - HCL
-- JSON (Deprecated in v0.3.1)
+- JSON (Deprecated in v0.3.1, removed in v0.4.6)
 
 # Get started
 

api/types/state.go

@@ -33,6 +33,15 @@ const (
 	ConfigurationReloading               ConfigurationState = "ConfigurationReloading"
 	GeneratingOutputs                    ConfigurationState = "GeneratingTerraformOutputs"
 	InvalidRegion                        ConfigurationState = "InvalidRegion"
+	TerraformInitError                   ConfigurationState = "TerraformInitError"
+)
+
+// Stage is the Terraform stage
+type Stage string
+
+const (
+	TerraformInit  Stage = "TerraformInit"
+	TerraformApply Stage = "TerraformApply"
 )
 
 const (

api/types/terraform.go

@@ -1,8 +1,6 @@
 package types
 
 const (
-	// TerraformJSONConfigurationName is the file name for Terraform json Configuration
-	TerraformJSONConfigurationName = "main.tf.json"
 	// TerraformHCLConfigurationName is the file name for Terraform hcl Configuration
 	TerraformHCLConfigurationName = "main.tf"
 )
@@ -11,8 +9,6 @@ const (
 type ConfigurationType string
 
 const (
-	// ConfigurationJSON is the json type Configuration
-	ConfigurationJSON ConfigurationType = "JSON"
 	// ConfigurationHCL is the HCL type Configuration
 	ConfigurationHCL ConfigurationType = "HCL"
 	// ConfigurationRemote means HCL stores in a remote git repository

api/v1beta2/configuration_types.go

@@ -26,9 +26,6 @@ import (
 
 // ConfigurationSpec defines the desired state of Configuration
 type ConfigurationSpec struct {
-	// JSON is the Terraform JSON syntax configuration.
-	// Deprecated: after v0.3.1, use HCL instead.
-	JSON string `json:"JSON,omitempty"`
 	// HCL is the Terraform HCL type configuration
 	HCL string `json:"hcl,omitempty"`
 

chart/crds/terraform.core.oam.dev_configurations.yaml

@@ -187,10 +187,6 @@ spec:
           spec:
             description: ConfigurationSpec defines the desired state of Configuration
             properties:
-              JSON:
-                description: 'JSON is the Terraform JSON syntax configuration. Deprecated:
-                  after v0.3.1, use HCL instead.'
-                type: string
               backend:
                 description: Backend stores the state in a Kubernetes secret with
                   locking done using a Lease resource. TODO(zzxwill) If a backend

controllers/configuration/configuration.go

@@ -34,16 +34,13 @@ const errGitHubBlockedNotBoolean = "the value of githubBlocked is not a boolean"
 
 // ValidConfigurationObject will validate a Configuration
 func ValidConfigurationObject(configuration *v1beta2.Configuration) (types.ConfigurationType, error) {
-	json := configuration.Spec.JSON
 	hcl := configuration.Spec.HCL
 	remote := configuration.Spec.Remote
 	switch {
-	case json == "" && hcl == "" && remote == "":
-		return "", errors.New("spec.JSON, spec.HCL or spec.Remote should be set")
-	case json != "" && hcl != "", json != "" && remote != "", hcl != "" && remote != "":
-		return "", errors.New("spec.JSON, spec.HCL and/or spec.Remote cloud not be set at the same time")
-	case json != "":
-		return types.ConfigurationJSON, nil
+	case hcl == "" && remote == "":
+		return "", errors.New("spec.HCL or spec.Remote should be set")
+	case hcl != "" && remote != "":
+		return "", errors.New("spec.HCL and spec.Remote cloud not be set at the same time")
 	case hcl != "":
 		return types.ConfigurationHCL, nil
 	case remote != "":
@@ -71,8 +68,6 @@ func RenderConfiguration(configuration *v1beta2.Configuration, terraformBackendN
 	}
 
 	switch configurationType {
-	case types.ConfigurationJSON:
-		return configuration.Spec.JSON, nil
 	case types.ConfigurationHCL:
 		completedConfiguration := configuration.Spec.HCL
 		completedConfiguration += "\n" + backendTF
@@ -125,7 +120,7 @@ func IsDeletable(ctx context.Context, k8sClient client.Client, configuration *v1
 	}
 	// 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 {
+	if providerObj == nil || providerObj.Status.State == types.ProviderIsNotReady || configuration.Status.Apply.State == types.TerraformInitError {
 		return true, nil
 	}
 

controllers/configuration/configuration_test.go

@@ -57,19 +57,6 @@ func TestValidConfigurationObject(t *testing.T) {
 				configurationType: types.ConfigurationRemote,
 			},
 		},
-		{
-			name: "json",
-			args: args{
-				configuration: &v1beta2.Configuration{
-					Spec: v1beta2.ConfigurationSpec{
-						JSON: "abc",
-					},
-				},
-			},
-			want: want{
-				configurationType: types.ConfigurationJSON,
-			},
-		},
 		{
 			name: "remote and hcl are set",
 			args: args{
@@ -82,7 +69,7 @@ func TestValidConfigurationObject(t *testing.T) {
 			},
 			want: want{
 				configurationType: "",
-				errMsg:            "spec.JSON, spec.HCL and/or spec.Remote cloud not be set at the same time",
+				errMsg:            "spec.HCL and spec.Remote cloud not be set at the same time",
 			},
 		},
 		{
@@ -94,7 +81,7 @@ func TestValidConfigurationObject(t *testing.T) {
 			},
 			want: want{
 				configurationType: "",
-				errMsg:            "spec.JSON, spec.HCL or spec.Remote should be set",
+				errMsg:            "spec.HCL or spec.Remote should be set",
 			},
 		},
 	}

controllers/configuration_controller.go

@@ -63,7 +63,8 @@ const (
 	// BackendVolumeMountPath is the volume mount path for Terraform backend
 	BackendVolumeMountPath = "/opt/tf-backend"
 	// terraformContainerName is the name of the container that executes the terraform in the pod
-	terraformContainerName = "terraform-executor"
+	terraformContainerName     = "terraform-executor"
+	terraformInitContainerName = "terraform-init"
 )
 
 const (
@@ -146,7 +147,7 @@ func (r *ConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		// terraform destroy
 		klog.InfoS("performing Configuration Destroy", "Namespace", req.Namespace, "Name", req.Name, "JobName", meta.DestroyJobName)
 
-		_, err := terraform.GetTerraformStatus(ctx, meta.Namespace, meta.DestroyJobName, terraformContainerName)
+		_, err := terraform.GetTerraformStatus(ctx, meta.Namespace, meta.DestroyJobName, terraformContainerName, terraformInitContainerName)
 		if err != nil {
 			klog.ErrorS(err, "Terraform destroy failed")
 			if updateErr := meta.updateDestroyStatus(ctx, r.Client, types.ConfigurationDestroyFailed, err.Error()); updateErr != nil {
@@ -182,7 +183,7 @@ func (r *ConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		}
 		return ctrl.Result{RequeueAfter: 3 * time.Second}, errors.Wrap(err, "failed to create/update cloud resource")
 	}
-	state, err := terraform.GetTerraformStatus(ctx, meta.Namespace, meta.ApplyJobName, terraformContainerName)
+	state, err := terraform.GetTerraformStatus(ctx, meta.Namespace, meta.ApplyJobName, terraformContainerName, terraformInitContainerName)
 	if err != nil {
 		klog.ErrorS(err, "Terraform apply failed")
 		if updateErr := meta.updateApplyStatus(ctx, r.Client, state, err.Error()); updateErr != nil {
@@ -601,7 +602,6 @@ func (meta *TFConfigurationMeta) updateDestroyStatus(ctx context.Context, k8sCli
 }
 
 func (meta *TFConfigurationMeta) assembleAndTriggerJob(ctx context.Context, k8sClient client.Client, executionType TerraformExecutionType) error {
-
 	// apply rbac
 	if err := createTerraformExecutorServiceAccount(ctx, k8sClient, meta.Namespace, ServiceAccountName); err != nil {
 		return err
@@ -638,11 +638,12 @@ func (meta *TFConfigurationMeta) updateTerraformJobIfNeeded(ctx context.Context,
 
 func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExecutionType) *batchv1.Job {
 	var (
-		initContainer  v1.Container
-		initContainers []v1.Container
-		parallelism    int32 = 1
-		completions    int32 = 1
-		backoffLimit   int32 = math.MaxInt32
+		initContainer           v1.Container
+		tfPreApplyInitContainer v1.Container
+		initContainers          []v1.Container
+		parallelism             int32 = 1
+		completions             int32 = 1
+		backoffLimit            int32 = math.MaxInt32
 	)
 
 	executorVolumes := meta.assembleExecutorVolumes()
@@ -661,6 +662,7 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe
 		},
 	}
 
+	// prepare local Terraform .tf files
 	initContainer = v1.Container{
 		Name:            "prepare-input-terraform-configurations",
 		Image:           meta.BusyboxImage,
@@ -672,6 +674,7 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe
 		},
 		VolumeMounts: initContainerVolumeMounts,
 	}
+
 	initContainers = append(initContainers, initContainer)
 
 	hclPath := filepath.Join(BackendVolumeMountPath, meta.RemoteGitPath)
@@ -692,14 +695,28 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe
 			})
 	}
 
+	// run `terraform init`
+	tfPreApplyInitContainer = v1.Container{
+		Name:            terraformInitContainerName,
+		Image:           meta.TerraformImage,
+		ImagePullPolicy: v1.PullIfNotPresent,
+		Command: []string{
+			"sh",
+			"-c",
+			"terraform init",
+		},
+		VolumeMounts: initContainerVolumeMounts,
+	}
+	initContainers = append(initContainers, tfPreApplyInitContainer)
+
 	container := v1.Container{
 		Name:            terraformContainerName,
 		Image:           meta.TerraformImage,
 		ImagePullPolicy: v1.PullIfNotPresent,
 		Command: []string{
 			"bash",
 			"-c",
-			fmt.Sprintf("terraform init && terraform %s -lock=false -auto-approve", executionType),
+			fmt.Sprintf("terraform %s -lock=false -auto-approve", executionType),
 		},
 		VolumeMounts: []v1.VolumeMount{
 			{
@@ -869,24 +886,44 @@ func (meta *TFConfigurationMeta) getTFOutputs(ctx context.Context, k8sClient cli
 		data[k] = []byte(v.Value)
 	}
 	var gotSecret v1.Secret
+	configurationName := configuration.ObjectMeta.Name
 	if err := k8sClient.Get(ctx, client.ObjectKey{Name: name, Namespace: ns}, &gotSecret); err != nil {
 		if kerrors.IsNotFound(err) {
 			var secret = v1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      name,
 					Namespace: ns,
 					Labels: map[string]string{
-						"created-by": "terraform-controller",
+						"terraform.core.oam.dev/created-by":      "terraform-controller",
+						"terraform.core.oam.dev/owned-by":        configurationName,
+						"terraform.core.oam.dev/owned-namespace": configuration.Namespace,
 					},
 				},
 				TypeMeta: metav1.TypeMeta{Kind: "Secret"},
 				Data:     data,
 			}
-			if err := k8sClient.Create(ctx, &secret); err != nil {
+			err = k8sClient.Create(ctx, &secret)
+			if kerrors.IsAlreadyExists(err) {
+				return nil, fmt.Errorf("secret(%s) already exists", name)
+			} else if err != nil {
 				return nil, err
 			}
 		}
 	} else {
+		// check the owner of this secret
+		labels := gotSecret.ObjectMeta.Labels
+		ownerName := labels["terraform.core.oam.dev/owned-by"]
+		ownerNamespace := labels["terraform.core.oam.dev/owned-namespace"]
+		if (ownerName != "" && ownerName != configurationName) ||
+			(ownerNamespace != "" && ownerNamespace != configuration.Namespace) {
+			errMsg := fmt.Sprintf(
+				"configuration(namespace: %s ; name: %s) cannot update secret(namespace: %s ; name: %s) whose owner is configuration(namespace: %s ; name: %s)",
+				configuration.Namespace, configurationName,
+				gotSecret.Namespace, name,
+				ownerNamespace, ownerName,
+			)
+			return nil, errors.New(errMsg)
+		}
 		gotSecret.Data = data
 		if err := k8sClient.Update(ctx, &gotSecret); err != nil {
 			return nil, err
@@ -1014,8 +1051,6 @@ func (meta *TFConfigurationMeta) createOrUpdateConfigMap(ctx context.Context, k8
 func (meta *TFConfigurationMeta) prepareTFInputConfigurationData() map[string]string {
 	var dataName string
 	switch meta.ConfigurationType {
-	case types.ConfigurationJSON:
-		dataName = types.TerraformJSONConfigurationName
 	case types.ConfigurationHCL:
 		dataName = types.TerraformHCLConfigurationName
 	case types.ConfigurationRemote:
@@ -1040,9 +1075,6 @@ func (meta *TFConfigurationMeta) CheckWhetherConfigurationChanges(ctx context.Co
 
 	var configurationChanged bool
 	switch configurationType {
-	case types.ConfigurationJSON:
-		meta.ConfigurationChanged = true
-		return nil
 	case types.ConfigurationHCL:
 		configurationChanged = cm.Data[types.TerraformHCLConfigurationName] != meta.CompleteConfiguration
 		meta.ConfigurationChanged = configurationChanged
@@ -1055,9 +1087,9 @@ func (meta *TFConfigurationMeta) CheckWhetherConfigurationChanges(ctx context.Co
 	case types.ConfigurationRemote:
 		meta.ConfigurationChanged = false
 		return nil
+	default:
+		return errors.New("unsupported configuration type, only HCL or Remote is supported")
 	}
-
-	return errors.New("unknown issue")
 }
 
 // getCredentials will get credentials from secret of the Provider