Support custom Terraform backends #291

loheagn · 2022-04-12T12:57:33Z

This pr is supposed to implement the proposal which is the concluation of the issuse #288 .

Currently, the changes only add some fields to the spec.backend to enable the end-users to custom the Terraform backend configuration.

Signed-off-by: loheagn [email protected]

codecov · 2022-04-12T13:00:42Z

Codecov Report

Merging #291 (796aa9b) into master (16bd07e) will not change coverage.
The diff coverage is n/a.

❗ Current head 796aa9b differs from pull request most recent head aaad840. Consider uploading reports for the commit aaad840 to get more accurate results

@@          Coverage Diff           @@
##           master    #291   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files           1       1           
  Lines          72      72           
======================================
  Misses         72      72

Flag	Coverage Δ
e2e	`0.00% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16bd07e...aaad840. Read the comment docs.

zzxwill · 2022-04-14T04:00:51Z

api/v1beta2/configuration_types.go

 	InClusterConfig bool `json:"inClusterConfig,omitempty"`
+
+	// HCL allows users to use raw hcl code to specify their Terraform backend configuration
+	HCL string `json:"hcl,omitempty"`


We already have HCL https://github.com/oam-dev/terraform-controller/pull/291/files#diff-56b818bb103c20f38071e987a12aebf0914429db0e363f0ec2cd58a817967b4eR30. It might get mixed.

Should we change HCL to something else, like raw?

Yes. Maybe you can use inline.

zzxwill · 2022-04-14T04:02:01Z

api/v1beta2/configuration_types.go

+	// Type specifies the Terraform backend type, for example, "kubernetes", "s3", etc
+	Type string `json:"type,omitempty"`
+	// Config is the detail configurations of the Terraform backend,
+	// and it represents the key-value pairs inside the terraform backend block in the hcl files


Users don't know how to set such fields. As Terraform backends are limited, we can support them one by one.

Do you mean that the users can write the spec.backend like this?

spec: backend: s3: bucket: "mybucket" key: "path/to/my/key" region: "my-region"

On the one hand, we need to use have a clear guide on how to set the backend just per the API, on the other hand, the spec.backend should be as simple as possible. Let's set aside some time on the design to check whether it's possible.

Maybe we can refer a Backend object, like we referring to a Provider object in Configuration.

Maybe we can refer a Backend object, like we referring to a Provider object in Configuration.

I don't think it's necessary to maintain an additional Backend object, if only to provide custom backend configuration in the Configuration object.

But if we can add backup and restore configuration and backup life cycle management to Backend object, I think it will make sense.

zzxwill · 2022-04-24T08:30:29Z

api/v1beta2/backend_types.go

+}
+
+// BackendSpecInnerMain defines all options supported by the Terraform backend configuration.
+type BackendSpecInnerMain struct {


What are the differences between creating a new Backend CRD and expanding spec.Backend of Configuration?

Creating a new Backend CRD will make the Configuration more simple and we can also add more fields like options describing backup and resotre to the new CRD.

As one Configuration will match one Backend, so you can also describe backup in Configuration. The CRD yamls will get longer, but for end-users, it won't as they will only choose one backend.
Besides, maintaining another Object will need to take care of its deletion.

Okay. I will move the spec into the Configuratoin.

zzxwill · 2022-04-26T01:27:58Z

api/v1beta2/configuration_types.go


 	// Artifactory is needed for the Terraform `artifactory` backend type.
-	Artifactory ArtifactoryBackendConf `json:"artifactory,omitempty"`
+	Artifactory *ArtifactoryBackendConf `json:"artifactory,omitempty"`


Are these backend are belong to Remote?

No, these backends don't belong to Remote. I think users can also use these backends if they use inline HCL configuration.

Are these backend are belong to Remote?

The meaning of "Remote" is the spec.Remote?

You are right.

api/v1beta2/configuration_types.go

zzxwill · 2022-04-26T07:53:13Z

I would suggest you support Kubernetes first, then S3, and other backends in other PRs. Or you might get desperate with tons of unit-test lines:)

loheagn · 2022-04-26T10:12:15Z

I would suggest you support Kubernetes first, then S3, and other backends in other PRs. Or you might get desperate with tons of unit-test lines:)

Do we need to write unit-test for each backend type as different types of backend will be uniformly converted to backend TF string using reflection in the rendering stage? I think we just need to write unit-tests for some special types, like which has secretRef fields.

zzxwill · 2022-05-05T00:56:12Z

Okay.

api/v1beta2/configuration_types.go

zzxwill · 2022-05-17T07:59:21Z

api/v1beta2/configuration_types.go

+	Inline string `json:"inline,omitempty"`
+
+	// BackendType indicates which backend type to use. This field is needed for custom backend configuration.
+	BackendType string `json:"backend_type,omitempty"`


Use enum to specify all its accepted values, please.

zzxwill · 2022-05-17T08:00:34Z

api/v1beta2/configuration_types.go

+// ConsulBackendConf defines all options supported by the Terraform `consul` backend type.
+// You can refer to https://www.terraform.io/language/settings/backends/consul for the usage of each option.
+type ConsulBackendConf struct {
+	Path        string  `json:"path" hcl:"path"`


What hcl is used for?

The hcl tag is used when encode the struct to hcl or decode the hcl into go struct.

api/v1beta2/configuration_types.go

controllers/configuration/backend/backend.go

zzxwill · 2022-05-17T08:12:24Z

controllers/configuration/backend/backend.go

+
+}
+
+func handleInlineBackendHCL(code string) (string, string, error) {


Why do we need to handle inlined backend HCL? How about transparently storing it in backend.tf?

This function is used to check for illegal properties in the inline hcl code.

zzxwill · 2022-05-17T08:18:29Z

controllers/configuration/backend/util/conf_data.go

+func (d ConfData) Get(key string) interface{} {
+	x, ok := d[key]
+	if !ok || x == nil {
+		klog.Errorf("fetch %s from the custom backend conf error", key)


Please print the backend information, or it will do any help for debugging.

zzxwill · 2022-05-23T06:21:44Z

api/v1beta2/configuration_types.go

+}
+
+// CurrentNSSecretSelector is used to specify the key in a secret in the current namespace.
+type CurrentNSSecretSelector struct {


What's the current namespace?

The same namespace as the configuration.

zzxwill · 2022-05-23T06:32:54Z

controllers/configuration/backend/backend.go

+
+	switch {
+
+	case backend != nil && len(backend.Inline) > 0:


What if Inline is not empty and BackType is not empty?

Then we will use the inline and ignore the backendType.

Signed-off-by: loheagn <[email protected]>

…Backend configuration Signed-off-by: loheagn <[email protected]>

Signed-off-by: loheagn <[email protected]>

codecov · 2022-05-25T01:18:14Z

Codecov Report

❗ No coverage uploaded for pull request base (master@8601f66). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #291   +/-   ##
=========================================
  Coverage          ?   78.18%           
=========================================
  Files             ?       23           
  Lines             ?     1595           
  Branches          ?        0           
=========================================
  Hits              ?     1247           
  Misses            ?      268           
  Partials          ?       80

Flag	Coverage Δ
e2e	`0.00% <0.00%> (?)`
unit	`81.87% <0.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8601f66...6ab7aa0. Read the comment docs.

Signed-off-by: loheagn <[email protected]>

lgtm-com · 2022-05-25T02:07:11Z

This pull request introduces 3 alerts when merging 6ab7aa0 into 8601f66 - view on LGTM.com

new alerts:

2 for Missing error check
1 for Useless assignment to local variable

zzxwill reviewed Apr 14, 2022

View reviewed changes

zzxwill reviewed Apr 24, 2022

View reviewed changes

loheagn marked this pull request as draft April 24, 2022 08:52

loheagn changed the title ~~WIP: Avoid overriding the specified backend configuration in the user's input~~ Avoid overriding the specified backend configuration in the user's input Apr 24, 2022

loheagn force-pushed the backend_specification branch from 564db97 to 94f42f4 Compare April 24, 2022 10:03

zzxwill reviewed Apr 26, 2022

View reviewed changes

zzxwill changed the title ~~Avoid overriding the specified backend configuration in the user's input~~ Support custom Terraform backends Apr 26, 2022

zzxwill reviewed Apr 26, 2022

View reviewed changes

api/v1beta2/configuration_types.go Outdated Show resolved Hide resolved

zzxwill reviewed Apr 26, 2022

View reviewed changes

api/v1beta2/configuration_types.go Outdated Show resolved Hide resolved

zzxwill reviewed Apr 26, 2022

View reviewed changes

api/v1beta2/configuration_types.go Show resolved Hide resolved

loheagn force-pushed the backend_specification branch 2 times, most recently from c759a59 to 76795dc Compare May 14, 2022 13:30

loheagn marked this pull request as ready for review May 14, 2022 14:45

loheagn force-pushed the backend_specification branch from 5afacce to 2eac213 Compare May 17, 2022 04:27

zzxwill reviewed May 17, 2022

View reviewed changes

api/v1beta2/configuration_types.go Outdated Show resolved Hide resolved

zzxwill reviewed May 17, 2022

View reviewed changes

api/v1beta2/configuration_types.go Outdated Show resolved Hide resolved

zzxwill reviewed May 17, 2022

View reviewed changes

controllers/configuration/backend/backend.go Outdated Show resolved Hide resolved

zzxwill reviewed May 17, 2022

View reviewed changes

controllers/configuration/backend/backend.go Outdated Show resolved Hide resolved

zzxwill reviewed May 17, 2022

View reviewed changes

loheagn force-pushed the backend_specification branch from 568f271 to 336e8a4 Compare May 21, 2022 06:03

zzxwill reviewed May 23, 2022

View reviewed changes

loheagn added 23 commits May 25, 2022 09:11

Allow to get state-json from custom backend

ab7e8a7

Signed-off-by: loheagn <[email protected]>

make lint

e5b8e47

Signed-off-by: loheagn <[email protected]>

Remove unused function

161678a

Signed-off-by: loheagn <[email protected]>

Remove "log-collector" container

3b3bded

Signed-off-by: loheagn <[email protected]>

Users are not allowed to refer to Secrets in other namespaces in the …

0283f5b

…Backend configuration Signed-off-by: loheagn <[email protected]>

Feat: fetch state json from Terraform kubernetes backend

159011b

Signed-off-by: loheagn <[email protected]>

Add unit-test for meta.getStateJSON

f1f62f1

Signed-off-by: loheagn <[email protected]>

Add unit-test for backend.GetStateJSON

a67989c

Signed-off-by: loheagn <[email protected]>

Add copyright header

dd1a899

Signed-off-by: loheagn <[email protected]>

Add copyright header

cfdb4cd

Signed-off-by: loheagn <[email protected]>

Add e2e test for custom Terraform kubernetes backend

6dc3412

Signed-off-by: loheagn <[email protected]>

Add e2e test for custom Terraform kubernetes backend

ae3f69f

Signed-off-by: loheagn <[email protected]>

Add e2e test for custom Terraform kubernetes backend

71928e1

Signed-off-by: loheagn <[email protected]>

Add more info in the error logs

0355c91

Signed-off-by: loheagn <[email protected]>

Update comments

daeb766

Signed-off-by: loheagn <[email protected]>

Use enum to specify all spec.backend.backend_type accepted values

e858abf

Signed-off-by: loheagn <[email protected]>

spec.backend.backendType uses camel case

aa8d455

Signed-off-by: loheagn <[email protected]>

make generate

55761b3

Signed-off-by: loheagn <[email protected]>

Remove all others backends except kubernetes

b0e5dae

Signed-off-by: loheagn <[email protected]>

Add example for custom kubernetes backend

a2a48ff

Signed-off-by: loheagn <[email protected]>

Remove useless code

eedc332

Signed-off-by: loheagn <[email protected]>

Simplify KubernetesBackendConf

110c9d7

Signed-off-by: loheagn <[email protected]>

Fix unit-tests

97feed1

Signed-off-by: loheagn <[email protected]>

loheagn force-pushed the backend_specification branch from aaad840 to 97feed1 Compare May 25, 2022 01:11

loheagn added 2 commits May 25, 2022 09:53

Fix unit-tests in configuration_test.go

2560d24

Signed-off-by: loheagn <[email protected]>

lint code

6ab7aa0

Signed-off-by: loheagn <[email protected]>

zzxwill merged commit 971d169 into kubevela:master May 25, 2022

loheagn deleted the backend_specification branch May 25, 2022 03:06


		}

		func handleInlineBackendHCL(code string) (string, string, error) {

Support custom Terraform backends #291

Support custom Terraform backends #291

Conversation

loheagn commented Apr 12, 2022 • edited Loading

codecov bot commented Apr 12, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zzxwill Apr 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loheagn Apr 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zzxwill commented Apr 26, 2022

loheagn commented Apr 26, 2022

zzxwill commented May 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 25, 2022 • edited Loading

Codecov Report

lgtm-com bot commented May 25, 2022

loheagn commented Apr 12, 2022 •

edited

Loading

codecov bot commented Apr 12, 2022 •

edited

Loading

zzxwill Apr 24, 2022 •

edited

Loading

loheagn Apr 26, 2022 •

edited

Loading

codecov bot commented May 25, 2022 •

edited

Loading