From cbe049f55811d2e6b64987a6054c1f99664574b1 Mon Sep 17 00:00:00 2001 From: loheagn Date: Sat, 19 Feb 2022 16:12:34 +0800 Subject: [PATCH 01/22] Avoid writing connection string to a same secret Signed-off-by: loheagn --- controllers/configuration_controller.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index d5ff21bb..88ab58a9 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -785,6 +785,7 @@ 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{ @@ -792,17 +793,24 @@ func (meta *TFConfigurationMeta) getTFOutputs(ctx context.Context, k8sClient cli Name: name, Namespace: ns, Labels: map[string]string{ - "created-by": "terraform-controller", + "created-by": "terraform-controller", + "terraform-controller-owner": configurationName, }, }, 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 { + if owner, ok := gotSecret.ObjectMeta.Labels["terraform-controller-owner"]; ok && owner != configurationName { + return nil, fmt.Errorf("configuration(%s) cannot update secret(%s) which owner is configuration(%s)", configurationName, name, owner) + } gotSecret.Data = data if err := k8sClient.Update(ctx, &gotSecret); err != nil { return nil, err From 4940e4fb5e5457b0617989817d6c6972d93fbe58 Mon Sep 17 00:00:00 2001 From: loheagn Date: Thu, 10 Mar 2022 18:48:13 +0800 Subject: [PATCH 02/22] Add test cases to configuration_controller_test.TestGetTFOutputs Signed-off-by: loheagn --- controllers/configuration_controller_test.go | 192 ++++++++++++++++++- 1 file changed, 191 insertions(+), 1 deletion(-) diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index c7a47091..048dcbfc 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -5,12 +5,13 @@ import ( "encoding/base64" "encoding/json" "fmt" - "github.com/oam-dev/terraform-controller/api/v1beta1" "reflect" "strings" "testing" "time" + "github.com/oam-dev/terraform-controller/api/v1beta1" + "github.com/agiledragon/gomonkey/v2" "github.com/aliyun/alibaba-cloud-sdk-go/services/sts" "github.com/stretchr/testify/assert" @@ -26,6 +27,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" + runtimetypes "github.com/oam-dev/terraform-controller/api/types/crossplane-runtime" + "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/v1beta2" @@ -913,6 +916,146 @@ func TestGetTFOutputs(t *testing.T) { TerraformBackendNamespace: "default", } + tfStateData, _ := base64.StdEncoding.DecodeString("H4sIAAAAAAAA/4SQzarbMBCF934KoXUdPKNf+1VKCWNp5AocO8hyaSl592KlcBd3cZfnHPHpY/52QshfXI68b3IS+tuVK5dCaS+P+8ci4TbcULb94JJplZPAFte8MS18PQrKBO8Q+xk59SHa1AMA9M4YmoN3FGJ8M/azPs96yElcCkLIsG+V8sblnqOc3uXlRuvZ0GxSSuiCRUYbw2gGHRFGPxitEgJYQDQ0a68I2ChNo1cAZJ2bR20UtW8bsv55NuJRS94W2erXe5X5QQs3A/FZ4fhJaOwUgZTVMRjto1HGpSGSQuuD955hdDDPcR6NY1ZpQJ/YwagTRAvBpsi8LXn7Pa1U+ahfWHX/zWThYz9L4Otg3390r+5fAAAA//8hmcuNuQEAAA==") + tfStateOutputs := map[string]v1beta2.Property{ + "container_id": { + Value: "e5fff27c62e26dc9504d21980543f21161225ab483a1e534a98311a677b9453a", + Type: "string", + }, + "image_id": { + Value: "sha256:d1a364dc548d5357f0da3268c888e1971bbdb957ee3f028fe7194f1d61c6fdeenginx:latest", + Type: "string", + }, + } + + secret3 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "b", + Namespace: "default", + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + TerraformStateNameInSecret: tfStateData, + }, + } + k8sClient3 := fake.NewClientBuilder().WithObjects(secret3).Build() + meta3 := &TFConfigurationMeta{ + BackendSecretName: "b", + TerraformBackendNamespace: "default", + } + + secret4 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c", + Namespace: "default", + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + TerraformStateNameInSecret: tfStateData, + }, + } + k8sClient4 := fake.NewClientBuilder().WithObjects(secret4).Build() + configuration4 := v1beta2.Configuration{ + Spec: v1beta2.ConfigurationSpec{ + BaseConfigurationSpec: v1beta2.BaseConfigurationSpec{ + WriteConnectionSecretToReference: &runtimetypes.SecretReference{ + Name: "connection-secret-c", + Namespace: "default", + }, + }, + }, + } + meta4 := &TFConfigurationMeta{ + BackendSecretName: "c", + TerraformBackendNamespace: "default", + } + + secret5 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "d", + Namespace: "default", + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + TerraformStateNameInSecret: tfStateData, + }, + } + oldConnectionSecret5 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "connection-secret-d", + Namespace: "default", + Labels: map[string]string{ + "created-by": "terraform-controller", + "terraform-controller-owner": "configuration5", + }, + }, + TypeMeta: metav1.TypeMeta{Kind: "Secret"}, + Data: map[string][]byte{ + "container_id": []byte("something"), + }, + } + k8sClient5 := fake.NewClientBuilder().WithObjects(secret5, oldConnectionSecret5).Build() + configuration5 := v1beta2.Configuration{ + ObjectMeta: v1.ObjectMeta{ + Name: "configuration5", + }, + Spec: v1beta2.ConfigurationSpec{ + BaseConfigurationSpec: v1beta2.BaseConfigurationSpec{ + WriteConnectionSecretToReference: &runtimetypes.SecretReference{ + Name: "connection-secret-d", + Namespace: "default", + }, + }, + }, + } + meta5 := &TFConfigurationMeta{ + BackendSecretName: "d", + TerraformBackendNamespace: "default", + } + + secret6 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "e", + Namespace: "default", + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + TerraformStateNameInSecret: tfStateData, + }, + } + oldConnectionSecret6 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "connection-secret-e", + Namespace: "default", + Labels: map[string]string{ + "created-by": "terraform-controller", + "terraform-controller-owner": "configuration5", + }, + }, + TypeMeta: metav1.TypeMeta{Kind: "Secret"}, + Data: map[string][]byte{ + "container_id": []byte("something"), + }, + } + k8sClient6 := fake.NewClientBuilder().WithObjects(secret6, oldConnectionSecret6).Build() + configuration6 := v1beta2.Configuration{ + ObjectMeta: v1.ObjectMeta{ + Name: "configuration6", + }, + Spec: v1beta2.ConfigurationSpec{ + BaseConfigurationSpec: v1beta2.BaseConfigurationSpec{ + WriteConnectionSecretToReference: &runtimetypes.SecretReference{ + Name: "connection-secret-e", + Namespace: "default", + }, + }, + }, + } + meta6 := &TFConfigurationMeta{ + BackendSecretName: "e", + TerraformBackendNamespace: "default", + } + testcases := map[string]struct { args args want want @@ -939,6 +1082,53 @@ func TestGetTFOutputs(t *testing.T) { errMsg: "failed to get tfstate from Terraform State secret", }, }, + "some data in a backend secret": { + args: args{ + ctx: ctx, + k8sClient: k8sClient3, + meta: meta3, + }, + want: want{ + property: tfStateOutputs, + errMsg: "", + }, + }, + "some data in a backend secret and creates a connectionSecret": { + args: args{ + ctx: ctx, + k8sClient: k8sClient4, + configuration: configuration4, + meta: meta4, + }, + want: want{ + property: tfStateOutputs, + errMsg: "", + }, + }, + "some data in a backend secret and update a connectionSecret belong to the same configuration": { + args: args{ + ctx: ctx, + k8sClient: k8sClient5, + configuration: configuration5, + meta: meta5, + }, + want: want{ + property: tfStateOutputs, + errMsg: "", + }, + }, + "some data in a backend secret and update a connectionSecret belong to another configuration": { + args: args{ + ctx: ctx, + k8sClient: k8sClient6, + configuration: configuration6, + meta: meta6, + }, + want: want{ + property: nil, + errMsg: "configuration(configuration6) cannot update secret(connection-secret-e) which owner is configuration(configuration5)", + }, + }, } for name, tc := range testcases { From 48f5f8929fff66c594e9f8d65a12fb9d8e8aa6d7 Mon Sep 17 00:00:00 2001 From: loheagn Date: Thu, 10 Mar 2022 19:01:03 +0800 Subject: [PATCH 03/22] Lint code Signed-off-by: loheagn --- controllers/configuration_controller.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 88ab58a9..5be6dbdc 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -809,7 +809,9 @@ func (meta *TFConfigurationMeta) getTFOutputs(ctx context.Context, k8sClient cli } } else { if owner, ok := gotSecret.ObjectMeta.Labels["terraform-controller-owner"]; ok && owner != configurationName { - return nil, fmt.Errorf("configuration(%s) cannot update secret(%s) which owner is configuration(%s)", configurationName, name, owner) + return nil, fmt.Errorf( + "configuration(%s) cannot update secret(%s) which owner is configuration(%s)", + configurationName, name, owner) } gotSecret.Data = data if err := k8sClient.Update(ctx, &gotSecret); err != nil { From 2e58c7b13ffbb915d19505c630bc09f0d3a8678b Mon Sep 17 00:00:00 2001 From: loheagn Date: Thu, 10 Mar 2022 19:10:09 +0800 Subject: [PATCH 04/22] Lint code Signed-off-by: loheagn --- controllers/configuration_controller.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 5be6dbdc..6c0e546c 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -808,10 +808,16 @@ func (meta *TFConfigurationMeta) getTFOutputs(ctx context.Context, k8sClient cli } } } else { - if owner, ok := gotSecret.ObjectMeta.Labels["terraform-controller-owner"]; ok && owner != configurationName { - return nil, fmt.Errorf( + // check the owner of this secret + labels := gotSecret.ObjectMeta.Labels + if owner, ok := labels["terraform-controller-owner"]; ok && owner != configurationName { + errMsg := fmt.Sprintf( "configuration(%s) cannot update secret(%s) which owner is configuration(%s)", - configurationName, name, owner) + configurationName, + name, + owner, + ) + return nil, errors.New(errMsg) } gotSecret.Data = data if err := k8sClient.Update(ctx, &gotSecret); err != nil { From 0c8186ecd0147fb784fd8ecfe28660dc145e181d Mon Sep 17 00:00:00 2001 From: loheagn Date: Fri, 11 Mar 2022 11:16:55 +0800 Subject: [PATCH 05/22] Rename "terraform-controller-owner" to "owned-by" Signed-off-by: loheagn --- controllers/configuration_controller.go | 6 +++--- controllers/configuration_controller_test.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 6c0e546c..0487093f 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -793,8 +793,8 @@ func (meta *TFConfigurationMeta) getTFOutputs(ctx context.Context, k8sClient cli Name: name, Namespace: ns, Labels: map[string]string{ - "created-by": "terraform-controller", - "terraform-controller-owner": configurationName, + "created-by": "terraform-controller", + "owned-by": configurationName, }, }, TypeMeta: metav1.TypeMeta{Kind: "Secret"}, @@ -810,7 +810,7 @@ func (meta *TFConfigurationMeta) getTFOutputs(ctx context.Context, k8sClient cli } else { // check the owner of this secret labels := gotSecret.ObjectMeta.Labels - if owner, ok := labels["terraform-controller-owner"]; ok && owner != configurationName { + if owner, ok := labels["owned-by"]; ok && owner != configurationName { errMsg := fmt.Sprintf( "configuration(%s) cannot update secret(%s) which owner is configuration(%s)", configurationName, diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 048dcbfc..abd0e9a7 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -985,8 +985,8 @@ func TestGetTFOutputs(t *testing.T) { Name: "connection-secret-d", Namespace: "default", Labels: map[string]string{ - "created-by": "terraform-controller", - "terraform-controller-owner": "configuration5", + "created-by": "terraform-controller", + "owned-by": "configuration5", }, }, TypeMeta: metav1.TypeMeta{Kind: "Secret"}, @@ -1028,8 +1028,8 @@ func TestGetTFOutputs(t *testing.T) { Name: "connection-secret-e", Namespace: "default", Labels: map[string]string{ - "created-by": "terraform-controller", - "terraform-controller-owner": "configuration5", + "created-by": "terraform-controller", + "owned-by": "configuration5", }, }, TypeMeta: metav1.TypeMeta{Kind: "Secret"}, From a6bca47291a960456cf0bf2cb00cd43d1f928fb2 Mon Sep 17 00:00:00 2001 From: loheagn Date: Fri, 11 Mar 2022 11:31:54 +0800 Subject: [PATCH 06/22] Add "owned-namespace" to the labels of connection-secret Signed-off-by: loheagn --- controllers/configuration_controller.go | 15 +++-- controllers/configuration_controller_test.go | 69 ++++++++++++++++++-- 2 files changed, 74 insertions(+), 10 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 0487093f..ece0886d 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -793,8 +793,9 @@ func (meta *TFConfigurationMeta) getTFOutputs(ctx context.Context, k8sClient cli Name: name, Namespace: ns, Labels: map[string]string{ - "created-by": "terraform-controller", - "owned-by": configurationName, + "created-by": "terraform-controller", + "owned-by": configurationName, + "owned-namespace": configuration.Namespace, }, }, TypeMeta: metav1.TypeMeta{Kind: "Secret"}, @@ -810,12 +811,14 @@ func (meta *TFConfigurationMeta) getTFOutputs(ctx context.Context, k8sClient cli } else { // check the owner of this secret labels := gotSecret.ObjectMeta.Labels - if owner, ok := labels["owned-by"]; ok && owner != configurationName { + ownerName := labels["owned-by"] + ownerNamespace := labels["owned-namespace"] + if configurationName != ownerName || configuration.Namespace != ownerNamespace { errMsg := fmt.Sprintf( - "configuration(%s) cannot update secret(%s) which owner is configuration(%s)", - configurationName, + "configuration(%s-%s) cannot update secret(%s) which owner is configuration(%s-%s)", + configuration.Namespace, configurationName, name, - owner, + ownerNamespace, ownerName, ) return nil, errors.New(errMsg) } diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index abd0e9a7..fb5d5e72 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -1028,8 +1028,9 @@ func TestGetTFOutputs(t *testing.T) { Name: "connection-secret-e", Namespace: "default", Labels: map[string]string{ - "created-by": "terraform-controller", - "owned-by": "configuration5", + "created-by": "terraform-controller", + "owned-by": "configuration5", + "owned-namespace": "default", }, }, TypeMeta: metav1.TypeMeta{Kind: "Secret"}, @@ -1040,7 +1041,8 @@ func TestGetTFOutputs(t *testing.T) { k8sClient6 := fake.NewClientBuilder().WithObjects(secret6, oldConnectionSecret6).Build() configuration6 := v1beta2.Configuration{ ObjectMeta: v1.ObjectMeta{ - Name: "configuration6", + Name: "configuration6", + Namespace: "default", }, Spec: v1beta2.ConfigurationSpec{ BaseConfigurationSpec: v1beta2.BaseConfigurationSpec{ @@ -1056,6 +1058,53 @@ func TestGetTFOutputs(t *testing.T) { TerraformBackendNamespace: "default", } + namespaceA := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "a"}} + namespaceB := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "b"}} + secret7 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "f", + Namespace: "a", + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + TerraformStateNameInSecret: tfStateData, + }, + } + oldConnectionSecret7 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "connection-secret-e", + Namespace: "default", + Labels: map[string]string{ + "created-by": "terraform-controller", + "owned-by": "configuration6", + "owned-namespace": "a", + }, + }, + TypeMeta: metav1.TypeMeta{Kind: "Secret"}, + Data: map[string][]byte{ + "container_id": []byte("something"), + }, + } + k8sClient7 := fake.NewClientBuilder().WithObjects(namespaceA, namespaceB, secret7, oldConnectionSecret7).Build() + configuration7 := v1beta2.Configuration{ + ObjectMeta: v1.ObjectMeta{ + Name: "configuration6", + Namespace: "b", + }, + Spec: v1beta2.ConfigurationSpec{ + BaseConfigurationSpec: v1beta2.BaseConfigurationSpec{ + WriteConnectionSecretToReference: &runtimetypes.SecretReference{ + Name: "connection-secret-e", + Namespace: "default", + }, + }, + }, + } + meta7 := &TFConfigurationMeta{ + BackendSecretName: "f", + TerraformBackendNamespace: "a", + } + testcases := map[string]struct { args args want want @@ -1126,7 +1175,19 @@ func TestGetTFOutputs(t *testing.T) { }, want: want{ property: nil, - errMsg: "configuration(configuration6) cannot update secret(connection-secret-e) which owner is configuration(configuration5)", + errMsg: "configuration(default-configuration6) cannot update secret(connection-secret-e) which owner is configuration(default-configuration5)", + }, + }, + "update a connectionSecret belong to another configuration(same name but different namespace": { + args: args{ + ctx: ctx, + k8sClient: k8sClient7, + configuration: configuration7, + meta: meta7, + }, + want: want{ + property: nil, + errMsg: "configuration(b-configuration6) cannot update secret(connection-secret-e) which owner is configuration(a-configuration6)", }, }, } From edf9d98658044a1e5f88592888b738482d4e2ebe Mon Sep 17 00:00:00 2001 From: loheagn Date: Fri, 11 Mar 2022 11:45:21 +0800 Subject: [PATCH 07/22] Fix typo Signed-off-by: loheagn --- controllers/configuration_controller.go | 2 +- controllers/configuration_controller_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index ece0886d..627020dd 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -815,7 +815,7 @@ func (meta *TFConfigurationMeta) getTFOutputs(ctx context.Context, k8sClient cli ownerNamespace := labels["owned-namespace"] if configurationName != ownerName || configuration.Namespace != ownerNamespace { errMsg := fmt.Sprintf( - "configuration(%s-%s) cannot update secret(%s) which owner is configuration(%s-%s)", + "configuration(%s-%s) cannot update secret(%s) whose owner is configuration(%s-%s)", configuration.Namespace, configurationName, name, ownerNamespace, ownerName, diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index fb5d5e72..95354f0c 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -1175,7 +1175,7 @@ func TestGetTFOutputs(t *testing.T) { }, want: want{ property: nil, - errMsg: "configuration(default-configuration6) cannot update secret(connection-secret-e) which owner is configuration(default-configuration5)", + errMsg: "configuration(default-configuration6) cannot update secret(connection-secret-e) whose owner is configuration(default-configuration5)", }, }, "update a connectionSecret belong to another configuration(same name but different namespace": { @@ -1187,7 +1187,7 @@ func TestGetTFOutputs(t *testing.T) { }, want: want{ property: nil, - errMsg: "configuration(b-configuration6) cannot update secret(connection-secret-e) which owner is configuration(a-configuration6)", + errMsg: "configuration(b-configuration6) cannot update secret(connection-secret-e) whose owner is configuration(a-configuration6)", }, }, } From 98befc788f1e520f98573141f5a2235f24e34d44 Mon Sep 17 00:00:00 2001 From: Yan Du Date: Sat, 12 Mar 2022 17:53:00 +0800 Subject: [PATCH 08/22] oam-dev/terraform-controller#273 go get: installing executables with 'go get' in module mode is deprecated. Signed-off-by: Yan Du --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 2f8a7e0b..182b282a 100644 --- a/Makefile +++ b/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/controller-gen@v0.6.0 ;\ + go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.6.0 ;\ rm -rf $$CONTROLLER_GEN_TMP_DIR ;\ } CONTROLLER_GEN=$(GOBIN)/controller-gen From 69baabd2b87a71abd6a26bd57875a2f174cab752 Mon Sep 17 00:00:00 2001 From: redsnapper2006 Date: Mon, 14 Mar 2022 10:11:13 +0800 Subject: [PATCH 09/22] Typo in getting-started.md & make command console output need to be updated due to changing from [go get] to [go install] in Makefile #278 (#279) Signed-off-by: Yan Du --- CONTRIBUTING.md | 16 ++++++++++------ getting-started.md | 4 ++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index be60ca3e..f106e991 100644 --- a/CONTRIBUTING.md +++ b/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 ./... diff --git a/getting-started.md b/getting-started.md index 7b8e74f4..ebb16ccf 100644 --- a/getting-started.md +++ b/getting-started.md @@ -495,7 +495,7 @@ es-connection Opaque 1 7m37s ## KubeVela Terraform Addon -Terraform Controller is well integrated in [KuberVela](https://github.com/oam-dev/kubevela) as a Terraform addon. Enabling +Terraform Controller is well integrated in [KubeVela](https://github.com/oam-dev/kubevela) as a Terraform addon. Enabling Terraform addon and Terraform provider addon is the simplest way. - Install Terraform Controller @@ -519,4 +519,4 @@ $ vela addon enable terraform-alibaba ALICLOUD_ACCESS_KEY= ALICLOUD_SECRET_ - Provision and Consume cloud resources -Try to provision and consume cloud resources by KubeVela [Cli](https://kubevela.io/docs/end-user/components/cloud-services/provider-and-consume-cloud-services) or [VelaUX](https://kubevela.io/docs/next/tutorials/consume-cloud-services). \ No newline at end of file +Try to provision and consume cloud resources by KubeVela [Cli](https://kubevela.io/docs/end-user/components/cloud-services/provider-and-consume-cloud-services) or [VelaUX](https://kubevela.io/docs/next/tutorials/consume-cloud-services). From 7810dc21a19abc39a005510e7fef021781d49583 Mon Sep 17 00:00:00 2001 From: loheagn Date: Thu, 17 Mar 2022 00:31:21 +0800 Subject: [PATCH 10/22] Add `tf_validation_backend` tool to the `hack` dir Signed-off-by: loheagn --- .../tf_validation_backend/README.md | 24 +++++ .../validation/tf_validation_backend/go.mod | 14 +++ .../validation/tf_validation_backend/go.sum | 68 ++++++++++++++ .../tf_validation_backend.go | 71 ++++++++++++++ .../tf_validation_backend_test.go | 94 +++++++++++++++++++ 5 files changed, 271 insertions(+) create mode 100644 hack/tool/validation/tf_validation_backend/README.md create mode 100644 hack/tool/validation/tf_validation_backend/go.mod create mode 100644 hack/tool/validation/tf_validation_backend/go.sum create mode 100644 hack/tool/validation/tf_validation_backend/tf_validation_backend.go create mode 100644 hack/tool/validation/tf_validation_backend/tf_validation_backend_test.go diff --git a/hack/tool/validation/tf_validation_backend/README.md b/hack/tool/validation/tf_validation_backend/README.md new file mode 100644 index 00000000..25d64b7a --- /dev/null +++ b/hack/tool/validation/tf_validation_backend/README.md @@ -0,0 +1,24 @@ +# tf_validation_backend + +This module shows how to fetch the backend block from a Terraform configuration file. + +## How to use + +You can run the `tf_validation_backend.go` to fetch the backend name from a `.tf` file. + +```bash +go run tf_validation_backend.go ${path_to_your_tf_file} +``` + +## What's more + +Actually, the function `fetchBackendName` in `tf_validation_backend.go` is just a simple demo. You can extend it to get more information about the backend configuration. As you can see, all the attributes and blocks in the backend block are parsed into the `Remain` field which type is [hcl.Body](https://pkg.go.dev/github.com/hashicorp/hcl/v2@v2.11.1?utm_source=gopls#Body). We can get all the information by accessing the fields of the [hcl.Body](https://pkg.go.dev/github.com/hashicorp/hcl/v2@v2.11.1?utm_source=gopls#Body). + +For example, the following code prints all the keys of the attributes of the backend block. + +```go +attrsMap, _ := config.Terraform.Backend.Remain.JustAttributes() +for k := range attrsMap { + fmt.Println(k) +} +``` \ No newline at end of file diff --git a/hack/tool/validation/tf_validation_backend/go.mod b/hack/tool/validation/tf_validation_backend/go.mod new file mode 100644 index 00000000..55852d7a --- /dev/null +++ b/hack/tool/validation/tf_validation_backend/go.mod @@ -0,0 +1,14 @@ +module tf_validation_backend + +go 1.17 + +require github.com/hashicorp/hcl/v2 v2.11.1 + +require ( + github.com/agext/levenshtein v1.2.3 // indirect + github.com/apparentlymart/go-textseg/v13 v13.0.0 // indirect + github.com/google/go-cmp v0.5.7 // indirect + github.com/mitchellh/go-wordwrap v1.0.1 // indirect + github.com/zclconf/go-cty v1.10.0 // indirect + golang.org/x/text v0.3.7 // indirect +) diff --git a/hack/tool/validation/tf_validation_backend/go.sum b/hack/tool/validation/tf_validation_backend/go.sum new file mode 100644 index 00000000..59e1eec2 --- /dev/null +++ b/hack/tool/validation/tf_validation_backend/go.sum @@ -0,0 +1,68 @@ +github.com/agext/levenshtein v1.2.1 h1:QmvMAjj2aEICytGiWzmxoE0x2KZvE0fvmqMOfy2tjT8= +github.com/agext/levenshtein v1.2.1/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= +github.com/agext/levenshtein v1.2.3 h1:YB2fHEn0UJagG8T1rrWknE3ZQzWM06O8AMAatNn7lmo= +github.com/agext/levenshtein v1.2.3/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= +github.com/apparentlymart/go-dump v0.0.0-20180507223929-23540a00eaa3/go.mod h1:oL81AME2rN47vu18xqj1S1jPIPuN7afo62yKTNn3XMM= +github.com/apparentlymart/go-textseg v1.0.0 h1:rRmlIsPEEhUTIKQb7T++Nz/A5Q6C9IuX2wFoYVvnCs0= +github.com/apparentlymart/go-textseg v1.0.0/go.mod h1:z96Txxhf3xSFMPmb5X/1W05FF/Nj9VFpLOpjS5yuumk= +github.com/apparentlymart/go-textseg/v13 v13.0.0 h1:Y+KvPE1NYz0xl601PVImeQfFyEy6iT90AvPUL1NNfNw= +github.com/apparentlymart/go-textseg/v13 v13.0.0/go.mod h1:ZK2fH7c4NqDTLtiYLvIkEghdlcqw7yxLeM89kiTRPUo= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/go-test/deep v1.0.3 h1:ZrJSEWsXzPOxaZnFteGEfooLba+ju3FYIbOrS+rQd68= +github.com/go-test/deep v1.0.3/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA= +github.com/golang/protobuf v1.1.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= +github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= +github.com/golang/protobuf v1.3.4/go.mod h1:vzj43D7+SQXF/4pzW/hwtAqwc6iTitCiVSaWz5lYuqw= +github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg= +github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= +github.com/google/go-cmp v0.5.7 h1:81/ik6ipDQS2aGcBfIN5dHDB36BwrStyeAQquSYCV4o= +github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE= +github.com/hashicorp/hcl/v2 v2.11.1 h1:yTyWcXcm9XB0TEkyU/JCRU6rYy4K+mgLtzn2wlrJbcc= +github.com/hashicorp/hcl/v2 v2.11.1/go.mod h1:FwWsfWEjyV/CMj8s/gqAuiviY72rJ1/oayI9WftqcKg= +github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= +github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= +github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= +github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348 h1:MtvEpTB6LX3vkb4ax0b5D2DHbNAUsen0Gx5wZoq3lV4= +github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k= +github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7 h1:DpOJ2HYzCv8LZP15IdmG+YdwD2luVPHITV96TkirNBM= +github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo= +github.com/mitchellh/go-wordwrap v1.0.1 h1:TLuKupo69TCn6TQSyGxwI1EblZZEsQ0vMlAFQflz0v0= +github.com/mitchellh/go-wordwrap v1.0.1/go.mod h1:R62XHJLzvMFRBbcrT7m7WgmE1eOyTSsCt+hzestvNj0= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ= +github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo= +github.com/spf13/pflag v1.0.2/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= +github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= +github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk= +github.com/vmihailenco/msgpack/v4 v4.3.12/go.mod h1:gborTTJjAo/GWTqqRjrLCn9pgNN+NXzzngzBKDPIqw4= +github.com/vmihailenco/tagparser v0.1.1/go.mod h1:OeAg3pn3UbLjkWt+rN9oFYB6u/cQgqMEUPoW2WPyhdI= +github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8= +github.com/zclconf/go-cty v1.8.0 h1:s4AvqaeQzJIu3ndv4gVIhplVD0krU+bgrcLSVUnaWuA= +github.com/zclconf/go-cty v1.8.0/go.mod h1:vVKLxnk3puL4qRAv72AO+W99LUD4da90g3uUAzyuvAk= +github.com/zclconf/go-cty v1.10.0 h1:mp9ZXQeIcN8kAwuqorjH+Q+njbJKjLrvB2yIh4q7U+0= +github.com/zclconf/go-cty v1.10.0/go.mod h1:vVKLxnk3puL4qRAv72AO+W99LUD4da90g3uUAzyuvAk= +github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b/go.mod h1:ZRKQfBXbGkpdV6QMzT3rU1kSTAnfu1dO8dPKjYprgj8= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/net v0.0.0-20180811021610-c39426892332/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= +golang.org/x/net v0.0.0-20200301022130-244492dfa37a/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20190502175342-a43fa875dd82/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= +golang.org/x/text v0.3.5 h1:i6eZZ+zk0SOf0xgBpEpPD18qWcJda6q1sxt3S0kzyUQ= +golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk= +golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= +google.golang.org/appengine v1.6.5/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= +gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/hack/tool/validation/tf_validation_backend/tf_validation_backend.go b/hack/tool/validation/tf_validation_backend/tf_validation_backend.go new file mode 100644 index 00000000..125e64de --- /dev/null +++ b/hack/tool/validation/tf_validation_backend/tf_validation_backend.go @@ -0,0 +1,71 @@ +/* +Copyright 2022 The KubeVela Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "errors" + "log" + "os" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/gohcl" + "github.com/hashicorp/hcl/v2/hclparse" +) + +// Config describes the schema of a whole tf file. +type Config struct { + Remain interface{} `hcl:",remain"` + Terraform struct { + Remain interface{} `hcl:",remain"` + Backend *BackendConfig `hcl:"backend,block"` + } `hcl:"terraform,block"` +} + +// BackendConfig describes the schema of the backend block. +type BackendConfig struct { + Name string `hcl:"name,label"` + Remain hcl.Body `hcl:",remain"` +} + +func fetchBackendName(body hcl.Body) (string, error) { + var config Config + diags := gohcl.DecodeBody(body, nil, &config) + if diags.HasErrors() { + return "", diags + } + backendConf := config.Terraform.Backend + if backendConf == nil { + return "", errors.New("can not find the \"backend block\"") + } + return backendConf.Name, nil +} + +func main() { + if len(os.Args) < 2 { + log.Fatalln("can not parse filename from args") + } + filename := os.Args[1] + file, diags := hclparse.NewParser().ParseHCLFile(filename) + if diags.HasErrors() { + log.Fatalln(diags.Error()) + } + backendName, err := fetchBackendName(file.Body) + if err != nil { + log.Fatalln(err.Error()) + } + log.Printf("fetch the name of backend: %s\n", backendName) +} diff --git a/hack/tool/validation/tf_validation_backend/tf_validation_backend_test.go b/hack/tool/validation/tf_validation_backend/tf_validation_backend_test.go new file mode 100644 index 00000000..cec0959b --- /dev/null +++ b/hack/tool/validation/tf_validation_backend/tf_validation_backend_test.go @@ -0,0 +1,94 @@ +/* +Copyright 2022 The KubeVela Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "testing" + + "github.com/hashicorp/hcl/v2/hclparse" +) + +func Test_fetchBackendName(t *testing.T) { + tests := []struct { + name string + hcl string + want string + wantErr bool + }{ + { + name: "valid backend block", + hcl: ` + terraform { + backend "remote" { + workspace { + prefix = "test_" + } + } + } + `, + want: "remote", + wantErr: false, + }, + { + name: "empty backend block", + hcl: ` + terraform { + backend "remote" { + } + } + `, + want: "remote", + wantErr: false, + }, + { + name: "invalid backend block", + hcl: ` + terraform { + backend { + workspace { + prefix = "test_" + } + } + } + `, + want: "", + wantErr: true, + }, + { + name: "no backend block", + hcl: ` + terraform { + } + `, + want: "", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + file, _ := hclparse.NewParser().ParseHCL([]byte(tt.hcl), "test.tf") + got, err := fetchBackendName(file.Body) + if (err != nil) != tt.wantErr { + t.Errorf("fetchBackendName() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("fetchBackendName() = %v, want %v", got, tt.want) + } + }) + } +} From 33fa1de0d2eeec7f4cde4855301041d5962a4453 Mon Sep 17 00:00:00 2001 From: loheagn Date: Thu, 17 Mar 2022 15:16:08 +0800 Subject: [PATCH 11/22] Fix the incompatibility issue that the configurations can not update the previous connection secrets which have no owner labels Signed-off-by: loheagn --- controllers/configuration_controller.go | 13 ++-- controllers/configuration_controller_test.go | 70 +++++++++++++++++--- 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 34709884..f5b62a5f 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -792,9 +792,9 @@ func (meta *TFConfigurationMeta) getTFOutputs(ctx context.Context, k8sClient cli Name: name, Namespace: ns, Labels: map[string]string{ - "created-by": "terraform-controller", - "owned-by": configurationName, - "owned-namespace": configuration.Namespace, + "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"}, @@ -810,9 +810,10 @@ func (meta *TFConfigurationMeta) getTFOutputs(ctx context.Context, k8sClient cli } else { // check the owner of this secret labels := gotSecret.ObjectMeta.Labels - ownerName := labels["owned-by"] - ownerNamespace := labels["owned-namespace"] - if configurationName != ownerName || configuration.Namespace != ownerNamespace { + 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(%s-%s) cannot update secret(%s) whose owner is configuration(%s-%s)", configuration.Namespace, configurationName, diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 95354f0c..acb54de0 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -985,8 +985,8 @@ func TestGetTFOutputs(t *testing.T) { Name: "connection-secret-d", Namespace: "default", Labels: map[string]string{ - "created-by": "terraform-controller", - "owned-by": "configuration5", + "terraform.core.oam.dev/created-by": "terraform-controller", + "terraform.core.oam.dev/owned-by": "configuration5", }, }, TypeMeta: metav1.TypeMeta{Kind: "Secret"}, @@ -1028,9 +1028,9 @@ func TestGetTFOutputs(t *testing.T) { Name: "connection-secret-e", Namespace: "default", Labels: map[string]string{ - "created-by": "terraform-controller", - "owned-by": "configuration5", - "owned-namespace": "default", + "terraform.core.oam.dev/created-by": "terraform-controller", + "terraform.core.oam.dev/owned-by": "configuration5", + "terraform.core.oam.dev/owned-namespace": "default", }, }, TypeMeta: metav1.TypeMeta{Kind: "Secret"}, @@ -1075,9 +1075,9 @@ func TestGetTFOutputs(t *testing.T) { Name: "connection-secret-e", Namespace: "default", Labels: map[string]string{ - "created-by": "terraform-controller", - "owned-by": "configuration6", - "owned-namespace": "a", + "terraform.core.oam.dev/created-by": "terraform-controller", + "terraform.core.oam.dev/owned-by": "configuration6", + "terraform.core.oam.dev/owned-namespace": "a", }, }, TypeMeta: metav1.TypeMeta{Kind: "Secret"}, @@ -1105,6 +1105,48 @@ func TestGetTFOutputs(t *testing.T) { TerraformBackendNamespace: "a", } + secret8 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "d", + Namespace: "default", + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + TerraformStateNameInSecret: tfStateData, + }, + } + oldConnectionSecret8 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "connection-secret-d", + Namespace: "default", + Labels: map[string]string{ + "terraform.core.oam.dev/created-by": "terraform-controller", + }, + }, + TypeMeta: metav1.TypeMeta{Kind: "Secret"}, + Data: map[string][]byte{ + "container_id": []byte("something"), + }, + } + k8sClient8 := fake.NewClientBuilder().WithObjects(secret8, oldConnectionSecret8).Build() + configuration8 := v1beta2.Configuration{ + ObjectMeta: v1.ObjectMeta{ + Name: "configuration5", + }, + Spec: v1beta2.ConfigurationSpec{ + BaseConfigurationSpec: v1beta2.BaseConfigurationSpec{ + WriteConnectionSecretToReference: &runtimetypes.SecretReference{ + Name: "connection-secret-d", + Namespace: "default", + }, + }, + }, + } + meta8 := &TFConfigurationMeta{ + BackendSecretName: "d", + TerraformBackendNamespace: "default", + } + testcases := map[string]struct { args args want want @@ -1190,6 +1232,18 @@ func TestGetTFOutputs(t *testing.T) { errMsg: "configuration(b-configuration6) cannot update secret(connection-secret-e) whose owner is configuration(a-configuration6)", }, }, + "update a connectionSecret without owner labels": { + args: args{ + ctx: ctx, + k8sClient: k8sClient8, + configuration: configuration8, + meta: meta8, + }, + want: want{ + property: tfStateOutputs, + errMsg: "", + }, + }, } for name, tc := range testcases { From f6ac3724384adcc82d8ed8521fc30c150cfdd613 Mon Sep 17 00:00:00 2001 From: loheagn Date: Thu, 17 Mar 2022 15:54:13 +0800 Subject: [PATCH 12/22] Fix compile errors Signed-off-by: loheagn --- controllers/configuration_controller_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index acb54de0..6112f3b6 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -920,11 +920,9 @@ func TestGetTFOutputs(t *testing.T) { tfStateOutputs := map[string]v1beta2.Property{ "container_id": { Value: "e5fff27c62e26dc9504d21980543f21161225ab483a1e534a98311a677b9453a", - Type: "string", }, "image_id": { Value: "sha256:d1a364dc548d5357f0da3268c888e1971bbdb957ee3f028fe7194f1d61c6fdeenginx:latest", - Type: "string", }, } From 7e7efc28ba624ac75d616d00115e0238fd1f0cec Mon Sep 17 00:00:00 2001 From: Zheng Xi Zhou Date: Sat, 19 Mar 2022 14:43:51 +0800 Subject: [PATCH 13/22] Remove Terraform JSON support JSON typed configuration is deprecated in v0.3.1, now removed it. Signed-off-by: Zheng Xi Zhou --- README.md | 2 +- api/types/terraform.go | 4 -- api/v1beta2/configuration_types.go | 3 -- controllers/configuration/configuration.go | 13 ++----- .../configuration/configuration_test.go | 17 +-------- controllers/configuration_controller.go | 5 --- controllers/configuration_controller_test.go | 2 +- .../oss/configuration_json_bucket.yaml | 37 ------------------- 8 files changed, 8 insertions(+), 75 deletions(-) delete mode 100644 examples/alibaba/oss/configuration_json_bucket.yaml diff --git a/README.md b/README.md index e3ebaca6..ad4d4605 100644 --- a/README.md +++ b/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 diff --git a/api/types/terraform.go b/api/types/terraform.go index dbe31b36..83e27b05 100644 --- a/api/types/terraform.go +++ b/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 diff --git a/api/v1beta2/configuration_types.go b/api/v1beta2/configuration_types.go index 65f4ab5b..2700cd79 100644 --- a/api/v1beta2/configuration_types.go +++ b/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"` diff --git a/controllers/configuration/configuration.go b/controllers/configuration/configuration.go index c3dc0fa2..0f6da89b 100644 --- a/controllers/configuration/configuration.go +++ b/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 diff --git a/controllers/configuration/configuration_test.go b/controllers/configuration/configuration_test.go index ccefa7d2..350bbac5 100644 --- a/controllers/configuration/configuration_test.go +++ b/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", }, }, } diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index f5b62a5f..61db732f 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -949,8 +949,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: @@ -975,9 +973,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 diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 6112f3b6..2482b60e 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -511,7 +511,7 @@ func TestPreCheck(t *testing.T) { meta: &TFConfigurationMeta{}, }, want: want{ - 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", }, }, { diff --git a/examples/alibaba/oss/configuration_json_bucket.yaml b/examples/alibaba/oss/configuration_json_bucket.yaml deleted file mode 100644 index a7ba01a8..00000000 --- a/examples/alibaba/oss/configuration_json_bucket.yaml +++ /dev/null @@ -1,37 +0,0 @@ -apiVersion: terraform.core.oam.dev/v1beta1 -kind: Configuration -metadata: - name: alibaba-oss-bucket-json -spec: - JSON: | - { - "resource": { - "alicloud_oss_bucket": { - "bucket-acl": { - "bucket": "${var.bucket}", - "acl": "${var.acl}" - } - } - }, - "output": { - "BUCKET_NAME": { - "value": "${alicloud_oss_bucket.bucket-acl.bucket}.${alicloud_oss_bucket.bucket-acl.extranet_endpoint}" - } - }, - "variable": { - "bucket": { - "default": "poc" - }, - "acl": { - "default": "private" - } - } - } - - variable: - bucket: "vela-website" - acl: "private" - - writeConnectionSecretToRef: - name: oss-conn - namespace: default From 36395ff7168ff06b7922297cb537e42b234ee125 Mon Sep 17 00:00:00 2001 From: Zheng Xi Zhou Date: Sun, 20 Mar 2022 11:14:33 +0800 Subject: [PATCH 14/22] fix typo Signed-off-by: Zheng Xi Zhou --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ad4d4605..033221dd 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ Terraform Controller is a Kubernetes Controller for Terraform. ## Supported Terraform Configuration - HCL -- JSON (Deprecated in v0.3.1, Removed in v0.4.6) +- JSON (Deprecated in v0.3.1, removed in v0.4.6) # Get started From d1a5282610e3e1fa50359408fa2b7546a4e9c178 Mon Sep 17 00:00:00 2001 From: Zheng Xi Zhou Date: Sun, 20 Mar 2022 16:50:32 +0800 Subject: [PATCH 15/22] Add unit testcases Added more uts Signed-off-by: Zheng Xi Zhou --- controllers/configuration_controller.go | 5 +- controllers/configuration_controller_test.go | 217 ++++++++++++++++++- 2 files changed, 211 insertions(+), 11 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 61db732f..54b99e82 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -541,7 +541,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 @@ -985,9 +984,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 diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 2482b60e..7b3b858c 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -10,27 +10,28 @@ import ( "testing" "time" - "github.com/oam-dev/terraform-controller/api/v1beta1" - "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" + kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" 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/apiutil" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" - runtimetypes "github.com/oam-dev/terraform-controller/api/types/crossplane-runtime" - "github.com/oam-dev/terraform-controller/api/types" crossplane "github.com/oam-dev/terraform-controller/api/types/crossplane-runtime" + runtimetypes "github.com/oam-dev/terraform-controller/api/types/crossplane-runtime" + "github.com/oam-dev/terraform-controller/api/v1beta1" "github.com/oam-dev/terraform-controller/api/v1beta2" "github.com/oam-dev/terraform-controller/controllers/provider" ) @@ -515,7 +516,7 @@ func TestPreCheck(t *testing.T) { }, }, { - name: "configuration is valid", + name: "HCL configuration is valid", args: args{ r: r, configuration: &v1beta2.Configuration{ @@ -536,6 +537,28 @@ func TestPreCheck(t *testing.T) { }, want: want{}, }, + { + name: "Remote configuration is valid", + args: args{ + r: r, + configuration: &v1beta2.Configuration{ + ObjectMeta: v1.ObjectMeta{ + Name: "abc", + }, + Spec: v1beta2.ConfigurationSpec{ + Remote: "https://github.com/a/b", + }, + }, + meta: &TFConfigurationMeta{ + ConfigurationCMName: "abc", + ProviderReference: &crossplane.Reference{ + Namespace: "default", + Name: "default", + }, + }, + }, + want: want{}, + }, { name: "could not find provider", args: args{ @@ -564,9 +587,11 @@ func TestPreCheck(t *testing.T) { 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); (tc.want.errMsg != "") && - !strings.Contains(err.Error(), tc.want.errMsg) { - t.Errorf("preCheck() error = %v, wantErr %v", err, tc.want.errMsg) + err := tc.args.r.preCheck(ctx, tc.args.configuration, tc.args.meta) + if tc.want.errMsg != "" || err != nil { + if !strings.Contains(err.Error(), tc.want.errMsg) { + t.Errorf("preCheck() error = %v, wantErr %v", err, tc.want.errMsg) + } } }) } @@ -1257,3 +1282,179 @@ func TestGetTFOutputs(t *testing.T) { } } + +func TestUpdateApplyStatus(t *testing.T) { + type args struct { + k8sClient client.Client + state types.ConfigurationState + message string + meta *TFConfigurationMeta + } + type want struct { + errMsg string + } + ctx := context.Background() + s := runtime.NewScheme() + v1beta2.AddToScheme(s) + + configuration := &v1beta2.Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "a", + Namespace: "b", + Generation: int64(1), + }, + Spec: v1beta2.ConfigurationSpec{ + HCL: "c", + }, + Status: v1beta2.ConfigurationStatus{ + Apply: v1beta2.ConfigurationApplyStatus{ + State: types.Available, + }, + }, + } + k8sClient = fake.NewClientBuilder().WithScheme(s).WithObjects(configuration).Build() + + testcases := map[string]struct { + args args + want want + }{ + "configuration is available": { + args: args{ + meta: &TFConfigurationMeta{ + Name: "a", + Namespace: "b", + }, + state: types.Available, + message: "xxx", + }, + }, + "configuration cloud not be found": { + args: args{ + meta: &TFConfigurationMeta{ + Name: "z", + Namespace: "b", + }, + state: types.Available, + message: "xxx", + }, + }, + } + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + err := tc.args.meta.updateApplyStatus(ctx, k8sClient, tc.args.state, tc.args.message) + if tc.want.errMsg != "" || err != nil { + if !strings.Contains(err.Error(), tc.want.errMsg) { + t.Errorf("updateApplyStatus() error = %v, wantErr %v", err, tc.want.errMsg) + } + } + }) + } +} + +func TestAssembleAndTriggerJob(t *testing.T) { + type prepare func(t *testing.T) + type args struct { + k8sClient client.Client + executionType TerraformExecutionType + prepare + } + type want struct { + errMsg string + } + ctx := context.Background() + k8sClient = fake.NewClientBuilder().Build() + meta := &TFConfigurationMeta{ + Namespace: "b", + } + + patches := gomonkey.ApplyFunc(apiutil.GVKForObject, func(obj runtime.Object, scheme *runtime.Scheme) (schema.GroupVersionKind, error) { + return schema.GroupVersionKind{}, kerrors.NewNotFound(schema.GroupResource{}, "") + }) + defer patches.Reset() + + testcases := map[string]struct { + args args + want want + }{ + "failed to create ServiceAccount": { + args: args{ + executionType: TerraformApply, + }, + want: want{ + errMsg: "failed to create ServiceAccount for Terraform executor", + }, + }, + } + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + err := meta.assembleAndTriggerJob(ctx, k8sClient, tc.args.executionType) + if tc.want.errMsg != "" || err != nil { + if !strings.Contains(err.Error(), tc.want.errMsg) { + t.Errorf("assembleAndTriggerJob() error = %v, wantErr %v", err, tc.want.errMsg) + } + } + }) + } +} + +func TestCheckWhetherConfigurationChanges(t *testing.T) { + type args struct { + k8sClient client.Client + configurationType types.ConfigurationType + meta *TFConfigurationMeta + } + type want struct { + errMsg string + } + ctx := context.Background() + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "a", + Namespace: "b", + }, + Data: map[string]string{ + "c": "d", + }, + } + k8sClient = fake.NewClientBuilder().WithObjects(cm).Build() + + testcases := map[string]struct { + args args + want want + }{ + "unknown configuration type": { + args: args{ + meta: &TFConfigurationMeta{ + ConfigurationCMName: "a", + Namespace: "b", + }, + configurationType: "xxx", + }, + want: want{ + errMsg: "unsupported configuration type, only HCL or Remote is supported", + }, + }, + "configuration map is not found": { + args: args{ + meta: &TFConfigurationMeta{ + ConfigurationCMName: "aaa", + Namespace: "b", + }, + configurationType: "xxx", + }, + want: want{ + errMsg: "not found", + }, + }, + } + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + err := tc.args.meta.CheckWhetherConfigurationChanges(ctx, k8sClient, tc.args.configurationType) + if tc.want.errMsg != "" || err != nil { + if !strings.Contains(err.Error(), tc.want.errMsg) { + t.Errorf("CheckWhetherConfigurationChanges() error = %v, wantErr %v", err, tc.want.errMsg) + } + } + }) + } +} From e8458e6e38c8e3694ebecc37a65952e2f4d25f91 Mon Sep 17 00:00:00 2001 From: loheagn Date: Wed, 23 Mar 2022 20:36:01 +0800 Subject: [PATCH 16/22] Update the error string reported when different configurations try to update the same secret Signed-off-by: loheagn --- controllers/configuration_controller.go | 4 ++-- controllers/configuration_controller_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 54b99e82..408a18a3 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -814,9 +814,9 @@ func (meta *TFConfigurationMeta) getTFOutputs(ctx context.Context, k8sClient cli if (ownerName != "" && ownerName != configurationName) || (ownerNamespace != "" && ownerNamespace != configuration.Namespace) { errMsg := fmt.Sprintf( - "configuration(%s-%s) cannot update secret(%s) whose owner is configuration(%s-%s)", + "configuration(namespace: %s ; name: %s) cannot update secret(namespace: %s ; name: %s) whose owner is configuration(namespace: %s ; name: %s)", configuration.Namespace, configurationName, - name, + gotSecret.Namespace, name, ownerNamespace, ownerName, ) return nil, errors.New(errMsg) diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 7b3b858c..9581f451 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -1240,7 +1240,7 @@ func TestGetTFOutputs(t *testing.T) { }, want: want{ property: nil, - errMsg: "configuration(default-configuration6) cannot update secret(connection-secret-e) whose owner is configuration(default-configuration5)", + errMsg: "configuration(namespace: default ; name: configuration6) cannot update secret(namespace: default ; name: connection-secret-e) whose owner is configuration(namespace: default ; name: configuration5)", }, }, "update a connectionSecret belong to another configuration(same name but different namespace": { @@ -1252,7 +1252,7 @@ func TestGetTFOutputs(t *testing.T) { }, want: want{ property: nil, - errMsg: "configuration(b-configuration6) cannot update secret(connection-secret-e) whose owner is configuration(a-configuration6)", + errMsg: "configuration(namespace: b ; name: configuration6) cannot update secret(namespace: default ; name: connection-secret-e) whose owner is configuration(namespace: a ; name: configuration6)", }, }, "update a connectionSecret without owner labels": { From ede04da45e54f260a1297f66e938e602b2063e94 Mon Sep 17 00:00:00 2001 From: Zheng Xi Zhou Date: Thu, 17 Mar 2022 18:05:53 +0800 Subject: [PATCH 17/22] Check whether there is error in Terraform init stage - check logs when `terraform init` - allow a Configuration to delete when hiting init issues Signed-off-by: Zheng Xi Zhou --- api/types/state.go | 9 +++++ controllers/configuration/configuration.go | 2 +- controllers/configuration_controller.go | 38 ++++++++++++++++------ controllers/terraform/logging.go | 35 ++++++++++++++++---- controllers/terraform/status.go | 19 +++++++---- 5 files changed, 79 insertions(+), 24 deletions(-) diff --git a/api/types/state.go b/api/types/state.go index 50fe1ae7..762ea34c 100644 --- a/api/types/state.go +++ b/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 ( diff --git a/controllers/configuration/configuration.go b/controllers/configuration/configuration.go index 0f6da89b..18000425 100644 --- a/controllers/configuration/configuration.go +++ b/controllers/configuration/configuration.go @@ -120,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 } diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 408a18a3..710b24ec 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -62,7 +62,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 ( @@ -145,7 +146,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 { @@ -181,7 +182,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 { @@ -577,11 +578,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() @@ -600,6 +602,7 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe }, } + // prepare local Terraform .tf files initContainer = v1.Container{ Name: "prepare-input-terraform-configurations", Image: meta.BusyboxImage, @@ -611,6 +614,7 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe }, VolumeMounts: initContainerVolumeMounts, } + initContainers = append(initContainers, initContainer) hclPath := filepath.Join(BackendVolumeMountPath, meta.RemoteGitPath) @@ -631,6 +635,20 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe }) } + // run `terraform init` + tfPreApplyInitContainer = v1.Container{ + Name: terraformInitContainerName, + Image: meta.TerraformImage, + ImagePullPolicy: v1.PullIfNotPresent, + Command: []string{ + "sh", + "-c", + fmt.Sprintf("terraform init"), + }, + VolumeMounts: initContainerVolumeMounts, + } + initContainers = append(initContainers, tfPreApplyInitContainer) + return &batchv1.Job{ TypeMeta: metav1.TypeMeta{ Kind: "Job", @@ -658,7 +676,7 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe // state file directory in advance InitContainers: initContainers, // Container terraform-executor will first copy predefined terraform.d to working directory, and - // then run terraform init/apply. + // then run terraform apply/destroy. Containers: []v1.Container{{ Name: terraformContainerName, Image: meta.TerraformImage, @@ -666,7 +684,7 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe 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{ { diff --git a/controllers/terraform/logging.go b/controllers/terraform/logging.go index 6d541440..d30cab37 100644 --- a/controllers/terraform/logging.go +++ b/controllers/terraform/logging.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "github.com/oam-dev/terraform-controller/api/types" "io" v1 "k8s.io/api/core/v1" @@ -12,23 +13,41 @@ import ( "k8s.io/klog/v2" ) -func getPodLog(ctx context.Context, client kubernetes.Interface, namespace, jobName, containerName string) (string, error) { +func getPods(ctx context.Context, client kubernetes.Interface, namespace, jobName string) (*v1.PodList, error) { label := fmt.Sprintf("job-name=%s", jobName) pods, err := client.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{LabelSelector: label}) - if err != nil || pods == nil || len(pods.Items) == 0 { + if err != nil { klog.InfoS("pods are not found", "Label", label, "Error", err) - return "", nil + return nil, err + } + return pods, nil +} + +func getPodLog(ctx context.Context, client kubernetes.Interface, namespace, jobName, containerName, initContainerName string) (types.Stage, string, error) { + var ( + targetContainer = containerName + stage = types.TerraformApply + ) + pods, err := getPods(ctx, client, namespace, jobName) + if err != nil || pods == nil || len(pods.Items) == 0 { + return stage, "", nil } pod := pods.Items[0] + // Here are two cases for Pending phase: 1) init container `terraform init` is not finished yet, 2) pod is not ready yet. if pod.Status.Phase == v1.PodPending { - return "", nil + for _, c := range pod.Status.InitContainerStatuses { + if c.Name == initContainerName && !c.Ready { + targetContainer = initContainerName + stage = types.TerraformInit + } + } } - req := client.CoreV1().Pods(namespace).GetLogs(pod.Name, &v1.PodLogOptions{Container: containerName}) + req := client.CoreV1().Pods(namespace).GetLogs(pod.Name, &v1.PodLogOptions{Container: targetContainer}) logs, err := req.Stream(ctx) if err != nil { - return "", err + return stage, "", err } defer func(logs io.ReadCloser) { err := logs.Close() @@ -37,7 +56,9 @@ func getPodLog(ctx context.Context, client kubernetes.Interface, namespace, jobN } }(logs) - return flushStream(logs, pod.Name) + log, err := flushStream(logs, pod.Name) + + return stage, log, err } func flushStream(rc io.ReadCloser, podName string) (string, error) { diff --git a/controllers/terraform/status.go b/controllers/terraform/status.go index 9ec38901..a60a48fc 100644 --- a/controllers/terraform/status.go +++ b/controllers/terraform/status.go @@ -12,21 +12,23 @@ import ( ) // GetTerraformStatus will get Terraform execution status -func GetTerraformStatus(ctx context.Context, namespace, jobName, containerName string) (types.ConfigurationState, error) { - klog.InfoS("checking Terraform execution status", "Namespace", namespace, "Job", jobName) +func GetTerraformStatus(ctx context.Context, namespace, jobName, containerName, initContainerName string) (types.ConfigurationState, error) { + klog.InfoS("checking Terraform init and execution status", "Namespace", namespace, "Job", jobName) clientSet, err := client.Init() if err != nil { klog.ErrorS(err, "failed to init clientSet") return types.ConfigurationProvisioningAndChecking, err } - logs, err := getPodLog(ctx, clientSet, namespace, jobName, containerName) + // check the stage of the pod + + stage, logs, err := getPodLog(ctx, clientSet, namespace, jobName, containerName, initContainerName) if err != nil { klog.ErrorS(err, "failed to get pod logs") return types.ConfigurationProvisioningAndChecking, err } - success, state, errMsg := analyzeTerraformLog(logs) + success, state, errMsg := analyzeTerraformLog(logs, stage) if success { return state, nil } @@ -34,7 +36,7 @@ func GetTerraformStatus(ctx context.Context, namespace, jobName, containerName s return state, errors.New(errMsg) } -func analyzeTerraformLog(logs string) (bool, types.ConfigurationState, string) { +func analyzeTerraformLog(logs string, stage types.Stage) (bool, types.ConfigurationState, string) { lines := strings.Split(logs, "\n") for i, line := range lines { if strings.Contains(line, "31mError:") { @@ -42,7 +44,12 @@ func analyzeTerraformLog(logs string) (bool, types.ConfigurationState, string) { if strings.Contains(errMsg, "Invalid Alibaba Cloud region") { return false, types.InvalidRegion, errMsg } - return false, types.ConfigurationApplyFailed, errMsg + switch stage { + case types.TerraformInit: + return false, types.TerraformInitError, errMsg + case types.TerraformApply: + return false, types.ConfigurationApplyFailed, errMsg + } } } return true, types.ConfigurationProvisioningAndChecking, "" From d837a0ffc3c9efe3caae9d4e7c7542c960fdcea3 Mon Sep 17 00:00:00 2001 From: Zheng Xi Zhou Date: Thu, 17 Mar 2022 19:50:50 +0800 Subject: [PATCH 18/22] add unit tests Signed-off-by: Zheng Xi Zhou --- controllers/terraform/logging.go | 3 ++- controllers/terraform/logging_test.go | 7 ++++++- controllers/terraform/status_test.go | 6 +++--- controllers/util/decompress_test.go | 26 ++++++++++++++++++++------ 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/controllers/terraform/logging.go b/controllers/terraform/logging.go index d30cab37..9f7a8cbe 100644 --- a/controllers/terraform/logging.go +++ b/controllers/terraform/logging.go @@ -4,13 +4,14 @@ import ( "bytes" "context" "fmt" - "github.com/oam-dev/terraform-controller/api/types" "io" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/klog/v2" + + "github.com/oam-dev/terraform-controller/api/types" ) func getPods(ctx context.Context, client kubernetes.Interface, namespace, jobName string) (*v1.PodList, error) { diff --git a/controllers/terraform/logging_test.go b/controllers/terraform/logging_test.go index 489b6ef7..9b18d62f 100644 --- a/controllers/terraform/logging_test.go +++ b/controllers/terraform/logging_test.go @@ -2,6 +2,7 @@ package terraform import ( "context" + "github.com/Azure/go-autorest/autorest" "io" "io/ioutil" "net/http" @@ -23,11 +24,15 @@ import ( func TestGetPodLog(t *testing.T) { ctx := context.Background() + func autorest.Prepare() { + + } type args struct { client kubernetes.Interface namespace string name string containerName string + prepare func } type want struct { log string @@ -90,7 +95,7 @@ func TestGetPodLog(t *testing.T) { } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - got, err := getPodLog(ctx, tc.args.client, tc.args.namespace, tc.args.name, tc.args.containerName) + _, got, err := getPodLog(ctx, tc.args.client, tc.args.namespace, tc.args.name, tc.args.containerName, "") if tc.want.errMsg != "" { assert.EqualError(t, err, tc.want.errMsg) } else { diff --git a/controllers/terraform/status_test.go b/controllers/terraform/status_test.go index 21b93d23..d0c5b65b 100644 --- a/controllers/terraform/status_test.go +++ b/controllers/terraform/status_test.go @@ -49,7 +49,7 @@ func TestGetTerraformStatus(t *testing.T) { } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - state, err := GetTerraformStatus(ctx, tc.args.namespace, tc.args.name, tc.args.containerName) + state, err := GetTerraformStatus(ctx, tc.args.namespace, tc.args.name, tc.args.containerName, "") if tc.want.errMsg != "" { assert.EqualError(t, err, tc.want.errMsg) } else { @@ -92,7 +92,7 @@ func TestGetTerraformStatus2(t *testing.T) { } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - state, err := GetTerraformStatus(ctx, tc.args.namespace, tc.args.name, tc.args.containerName) + state, err := GetTerraformStatus(ctx, tc.args.namespace, tc.args.name, tc.args.containerName, "") if tc.want.errMsg != "" { assert.Contains(t, err.Error(), tc.want.errMsg) } else { @@ -143,7 +143,7 @@ func TestAnalyzeTerraformLog(t *testing.T) { } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - success, state, errMsg := analyzeTerraformLog(tc.args.logs) + success, state, errMsg := analyzeTerraformLog(tc.args.logs, types.TerraformApply) if tc.want.errMsg != "" { assert.Contains(t, errMsg, tc.want.errMsg) } else { diff --git a/controllers/util/decompress_test.go b/controllers/util/decompress_test.go index d896432c..6f7c4c61 100644 --- a/controllers/util/decompress_test.go +++ b/controllers/util/decompress_test.go @@ -9,7 +9,8 @@ import ( func TestDecompressTerraformStateSecret(t *testing.T) { type args struct { - data string + data string + needDecode bool } type want struct { raw string @@ -23,7 +24,8 @@ func TestDecompressTerraformStateSecret(t *testing.T) { { name: "decompress terraform state secret", args: args{ - data: "H4sIAAAAAAAA/0SMwa7CIBBF9/0KMutH80ArDb9ijKHDYEhqMQO4afrvBly4POfc3H0QAt7EOaYNrDj/NS7E7ELi5/1XQI3/o4beM3F0K1ihO65xI/egNsLThLPRWi6agkR/CVIppaSZJrfgbBx6//1ItbxqyWDFfnTBlFNlpKaut+EYPgEAAP//xUXpvZsAAAA=", + data: "H4sIAAAAAAAA/0SMwa7CIBBF9/0KMutH80ArDb9ijKHDYEhqMQO4afrvBly4POfc3H0QAt7EOaYNrDj/NS7E7ELi5/1XQI3/o4beM3F0K1ihO65xI/egNsLThLPRWi6agkR/CVIppaSZJrfgbBx6//1ItbxqyWDFfnTBlFNlpKaut+EYPgEAAP//xUXpvZsAAAA=", + needDecode: true, }, want: want{ raw: `{ @@ -37,14 +39,26 @@ func TestDecompressTerraformStateSecret(t *testing.T) { `, }, }, + { + name: "bad data", + args: args{ + data: "abc", + }, + want: want{ + errMsg: "EOF", + }, + }, } for _, tt := range testcases { t.Run(tt.name, func(t *testing.T) { - state, err := base64.StdEncoding.DecodeString(tt.args.data) - assert.NoError(t, err) - got, err := DecompressTerraformStateSecret(string(state)) - if tt.want.errMsg != "" { + if tt.args.needDecode { + state, err := base64.StdEncoding.DecodeString(tt.args.data) + assert.NoError(t, err) + tt.args.data = string(state) + } + got, err := DecompressTerraformStateSecret(tt.args.data) + if tt.want.errMsg != "" || err != nil { assert.Contains(t, err.Error(), tt.want.errMsg) } else { assert.Equal(t, tt.want.raw, string(got)) From 787937415039c48c384565c2fdcef71c1d8d886d Mon Sep 17 00:00:00 2001 From: Zheng Xi Zhou Date: Sat, 19 Mar 2022 14:27:56 +0800 Subject: [PATCH 19/22] revise ut Signed-off-by: Zheng Xi Zhou --- controllers/terraform/logging.go | 1 + controllers/terraform/logging_test.go | 101 ++++++++++++++++---------- 2 files changed, 62 insertions(+), 40 deletions(-) diff --git a/controllers/terraform/logging.go b/controllers/terraform/logging.go index 9f7a8cbe..0f645cd5 100644 --- a/controllers/terraform/logging.go +++ b/controllers/terraform/logging.go @@ -41,6 +41,7 @@ func getPodLog(ctx context.Context, client kubernetes.Interface, namespace, jobN if c.Name == initContainerName && !c.Ready { targetContainer = initContainerName stage = types.TerraformInit + break } } } diff --git a/controllers/terraform/logging_test.go b/controllers/terraform/logging_test.go index 9b18d62f..9bc2c1df 100644 --- a/controllers/terraform/logging_test.go +++ b/controllers/terraform/logging_test.go @@ -2,77 +2,71 @@ package terraform import ( "context" - "github.com/Azure/go-autorest/autorest" + "github.com/agiledragon/gomonkey/v2" + "github.com/oam-dev/terraform-controller/api/types" "io" "io/ioutil" + "k8s.io/client-go/kubernetes/typed/core/v1/fake" + "k8s.io/client-go/rest" "net/http" "net/url" "reflect" "strings" "testing" - "github.com/agiledragon/gomonkey/v2" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" fakeclient "k8s.io/client-go/kubernetes/fake" - "k8s.io/client-go/kubernetes/typed/core/v1/fake" - "k8s.io/client-go/rest" - "k8s.io/client-go/util/flowcontrol" ) func TestGetPodLog(t *testing.T) { ctx := context.Background() - func autorest.Prepare() { - } + type prepare func(t *testing.T) + k8sClientSet := fakeclient.NewSimpleClientset() + type args struct { - client kubernetes.Interface - namespace string - name string - containerName string - prepare func + client kubernetes.Interface + namespace string + name string + containerName string + initContainerName string + prepare } type want struct { + state types.Stage log string errMsg string } - pod := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "p1", - Namespace: "default", - Labels: map[string]string{ - "job-name": "j1", - }, - }, - TypeMeta: metav1.TypeMeta{ - Kind: "Pod", - }, - Status: v1.PodStatus{ - Phase: v1.PodRunning, - }, - } - - k8sClientSet := fakeclient.NewSimpleClientset(pod) + p := gomonkey.ApplyMethod(reflect.TypeOf(&http.Client{}), "Do", + func(_ *http.Client, req *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: 200, + Body: ioutil.NopCloser(strings.NewReader("xxx")), + }, nil + }) patches := gomonkey.ApplyMethod(reflect.TypeOf(&fake.FakePods{}), "GetLogs", func(_ *fake.FakePods, _ string, _ *v1.PodLogOptions) *rest.Request { - rate := flowcontrol.NewFakeNeverRateLimiter() + // rate := flowcontrol.NewFakeNeverRateLimiter() restClient, _ := rest.NewRESTClient( &url.URL{ Scheme: "http", - Host: "", + Host: "127.0.0.1", }, "", rest.ClientContentConfig{}, - rate, + nil, http.DefaultClient) r := rest.NewRequest(restClient) r.Body([]byte("xxx")) return r }) + + defer p.Reset() defer patches.Reset() var testcases = []struct { @@ -83,24 +77,51 @@ func TestGetPodLog(t *testing.T) { { name: "Pod is available, but no logs", args: args{ - client: k8sClientSet, - namespace: "default", - name: "j1", - containerName: "terraform-executor", + client: k8sClientSet, + namespace: "default", + name: "j1", + containerName: "terraform-executor", + initContainerName: "terraform-init", + prepare: func(t *testing.T) { + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + Namespace: "default", + Labels: map[string]string{ + "job-name": "j1", + }, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "Pod", + }, + Status: v1.PodStatus{ + Phase: v1.PodPending, + InitContainerStatuses: []v1.ContainerStatus{ + { + Name: "terraform-init", + Ready: false, + }, + }, + }, + } + k8sClientSet.CoreV1().Pods("default").Create(ctx, pod, metav1.CreateOptions{}) + }, }, want: want{ - errMsg: "can not be accept", + state: types.TerraformInit, + log: "xxx", }, }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - _, got, err := getPodLog(ctx, tc.args.client, tc.args.namespace, tc.args.name, tc.args.containerName, "") - if tc.want.errMsg != "" { + tc.args.prepare(t) + state, got, err := getPodLog(ctx, tc.args.client, tc.args.namespace, tc.args.name, tc.args.containerName, tc.args.initContainerName) + if tc.want.errMsg != "" || err != nil { assert.EqualError(t, err, tc.want.errMsg) } else { - assert.NoError(t, err) assert.Equal(t, tc.want.log, got) + assert.Equal(t, tc.want.state, state) } }) } From 5bb2ef99a068f9123897135666296e73d1267de8 Mon Sep 17 00:00:00 2001 From: Zheng Xi Zhou Date: Sun, 20 Mar 2022 11:25:41 +0800 Subject: [PATCH 20/22] fix format issues Signed-off-by: Zheng Xi Zhou --- controllers/configuration_controller.go | 2 +- controllers/terraform/logging.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index 710b24ec..27b12e3c 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -643,7 +643,7 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe Command: []string{ "sh", "-c", - fmt.Sprintf("terraform init"), + "terraform init", }, VolumeMounts: initContainerVolumeMounts, } diff --git a/controllers/terraform/logging.go b/controllers/terraform/logging.go index 0f645cd5..703e628c 100644 --- a/controllers/terraform/logging.go +++ b/controllers/terraform/logging.go @@ -31,6 +31,7 @@ func getPodLog(ctx context.Context, client kubernetes.Interface, namespace, jobN ) pods, err := getPods(ctx, client, namespace, jobName) if err != nil || pods == nil || len(pods.Items) == 0 { + klog.V(4).InfoS("pods are not found", "PodName", jobName, "Namepspace", namespace, "Error", err) return stage, "", nil } pod := pods.Items[0] From 75ae1d409ff0f7314ac8dec46f5a351cc6840857 Mon Sep 17 00:00:00 2001 From: Zheng Xi Zhou Date: Wed, 23 Mar 2022 20:51:43 +0800 Subject: [PATCH 21/22] fix CI issues Signed-off-by: Zheng Xi Zhou --- ...terraform.core.oam.dev_configurations.yaml | 4 - controllers/terraform/logging_test.go | 75 +++++++------------ 2 files changed, 27 insertions(+), 52 deletions(-) diff --git a/chart/crds/terraform.core.oam.dev_configurations.yaml b/chart/crds/terraform.core.oam.dev_configurations.yaml index 52c66a34..8d4528e8 100644 --- a/chart/crds/terraform.core.oam.dev_configurations.yaml +++ b/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 diff --git a/controllers/terraform/logging_test.go b/controllers/terraform/logging_test.go index 9bc2c1df..e1dfdbb6 100644 --- a/controllers/terraform/logging_test.go +++ b/controllers/terraform/logging_test.go @@ -2,38 +2,35 @@ package terraform import ( "context" - "github.com/agiledragon/gomonkey/v2" - "github.com/oam-dev/terraform-controller/api/types" "io" "io/ioutil" - "k8s.io/client-go/kubernetes/typed/core/v1/fake" - "k8s.io/client-go/rest" "net/http" "net/url" "reflect" "strings" "testing" + "github.com/agiledragon/gomonkey/v2" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" fakeclient "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/kubernetes/typed/core/v1/fake" + "k8s.io/client-go/rest" + "k8s.io/client-go/util/flowcontrol" + + "github.com/oam-dev/terraform-controller/api/types" ) func TestGetPodLog(t *testing.T) { ctx := context.Background() - - type prepare func(t *testing.T) - k8sClientSet := fakeclient.NewSimpleClientset() - type args struct { client kubernetes.Interface namespace string name string containerName string initContainerName string - prepare } type want struct { state types.Stage @@ -41,32 +38,40 @@ func TestGetPodLog(t *testing.T) { errMsg string } - p := gomonkey.ApplyMethod(reflect.TypeOf(&http.Client{}), "Do", - func(_ *http.Client, req *http.Request) (*http.Response, error) { - return &http.Response{ - StatusCode: 200, - Body: ioutil.NopCloser(strings.NewReader("xxx")), - }, nil - }) + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + Namespace: "default", + Labels: map[string]string{ + "job-name": "j1", + }, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "Pod", + }, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + }, + } + + k8sClientSet := fakeclient.NewSimpleClientset(pod) patches := gomonkey.ApplyMethod(reflect.TypeOf(&fake.FakePods{}), "GetLogs", func(_ *fake.FakePods, _ string, _ *v1.PodLogOptions) *rest.Request { - // rate := flowcontrol.NewFakeNeverRateLimiter() + rate := flowcontrol.NewFakeNeverRateLimiter() restClient, _ := rest.NewRESTClient( &url.URL{ Scheme: "http", - Host: "127.0.0.1", + Host: "", }, "", rest.ClientContentConfig{}, - nil, + rate, http.DefaultClient) r := rest.NewRequest(restClient) r.Body([]byte("xxx")) return r }) - - defer p.Reset() defer patches.Reset() var testcases = []struct { @@ -82,40 +87,14 @@ func TestGetPodLog(t *testing.T) { name: "j1", containerName: "terraform-executor", initContainerName: "terraform-init", - prepare: func(t *testing.T) { - pod := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "p1", - Namespace: "default", - Labels: map[string]string{ - "job-name": "j1", - }, - }, - TypeMeta: metav1.TypeMeta{ - Kind: "Pod", - }, - Status: v1.PodStatus{ - Phase: v1.PodPending, - InitContainerStatuses: []v1.ContainerStatus{ - { - Name: "terraform-init", - Ready: false, - }, - }, - }, - } - k8sClientSet.CoreV1().Pods("default").Create(ctx, pod, metav1.CreateOptions{}) - }, }, want: want{ - state: types.TerraformInit, - log: "xxx", + errMsg: "can not be accept", }, }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - tc.args.prepare(t) state, got, err := getPodLog(ctx, tc.args.client, tc.args.namespace, tc.args.name, tc.args.containerName, tc.args.initContainerName) if tc.want.errMsg != "" || err != nil { assert.EqualError(t, err, tc.want.errMsg) From 6fd6744a811018f3c0424a1a6b7cf41751ce22b1 Mon Sep 17 00:00:00 2001 From: Yan Du Date: Thu, 24 Mar 2022 17:40:05 +0800 Subject: [PATCH 22/22] resolve conflict Signed-off-by: Yan Du --- controllers/configuration_controller.go | 1 - controllers/configuration_controller_test.go | 14 +++----------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/controllers/configuration_controller.go b/controllers/configuration_controller.go index ad780c87..395efdaa 100644 --- a/controllers/configuration_controller.go +++ b/controllers/configuration_controller.go @@ -782,7 +782,6 @@ func (meta *TFConfigurationMeta) assembleTerraformJob(executionType TerraformExe // state file directory in advance InitContainers: initContainers, // Container terraform-executor will first copy predefined terraform.d to working directory, and -<<<<<<< HEAD // then run terraform init/apply. Containers: []v1.Container{container}, ServiceAccountName: ServiceAccountName, diff --git a/controllers/configuration_controller_test.go b/controllers/configuration_controller_test.go index 00ad0a90..a9ab7c49 100644 --- a/controllers/configuration_controller_test.go +++ b/controllers/configuration_controller_test.go @@ -10,19 +10,16 @@ import ( "testing" "time" - "github.com/oam-dev/terraform-controller/api/v1beta1" - "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" -<<<<<<< HEAD + "k8s.io/apimachinery/pkg/api/resource" -======= kerrors "k8s.io/apimachinery/pkg/api/errors" ->>>>>>> master + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -919,20 +916,15 @@ func TestPreCheck(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { -<<<<<<< HEAD if tc.prepare != nil { tc.prepare(t) } - 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.errMsg) -======= + err := tc.args.r.preCheck(ctx, tc.args.configuration, tc.args.meta) if tc.want.errMsg != "" || err != nil { if !strings.Contains(err.Error(), tc.want.errMsg) { t.Errorf("preCheck() error = %v, wantErr %v", err, tc.want.errMsg) } ->>>>>>> master } }) }