Migrate to TF 0.12.3 #33

xescugc · 2019-07-09T09:36:15Z

Change all the workflow to work with the new 0.12 implementation of TF. This implementation uses the Plugins logic without using Plugins directly.

The HCL and State have been changed to work with the new format.

Closes #7

xlr-8

Couple typo/question to solve/answer

xlr-8 · 2019-07-16T13:04:48Z

CHANGELOG.md

+### Changed
+
+- The Terraform version from 0.11 to 0.12 with all the implications (file formats)
+  ([PR #33](https://github.com/cycloidio/terracognita/pull/33))


Could make sense to drop a link to https://www.terraform.io/upgrade-guides/0-12.html

xlr-8 · 2019-07-16T13:17:26Z

hcl/format_hcl.go

+		},
+		{
+			// Add new lines before blocks
+			//match: regexp.MustCompile("\n(\t*)([\\w\\-_\\.]+\\s{)"),


Unecessary commented code

xlr-8 · 2019-07-16T13:17:44Z

hcl/format_hcl.go

@@ -45,11 +56,15 @@ var (
 	}
 )

-// FormatHCL format the hcl to have a better formatter thatn the default one
+// Format format the hcl to have a better formatter thatn the default one


than*
returned*

xlr-8 · 2019-07-16T13:18:39Z

hcl/writer.go


 	return nil
 }

+// Has checks if the given key it's already present or not


is already*

xlr-8 · 2019-07-16T13:19:22Z

hcl/writer.go

+// Has checks if the given key it's already present or not
+func (w *Writer) Has(key string) (bool, error) {
+	keys := strings.Split(key, ".")
+	if len(keys) != 2 || (keys[0] == "" || keys[1] == "") {


Unecessary parenthesis

xlr-8 · 2019-07-16T13:20:24Z

provider/import.go

 				if err != nil {
-					return errors.Wrapf(err, "error while calculating the Config of resource %q", t)
+					cause := errors.Cause(err)
+					// All those ignored errors are types of errors to identify that something happend, but not that something whent wrong


xlr-8 · 2019-07-16T13:20:57Z

provider/import.go

+				if tfstate != nil {
+					err = r.State(tfstate)
+					if err != nil {
+						return errors.Wrapf(err, "error while calculating the Satate of resource %q", t)


xlr-8 · 2019-07-16T13:25:44Z

state/writer.go

+func (w *Writer) Has(key string) (bool, error) {
+	_, ok := w.Config[key]
+	return ok, nil
+}


I'm not sure this Has method or the other one are used anywhere

https://github.com/cycloidio/terracognita/pull/33/files#diff-9a4396fc8b469f87eb63a29771b3380fR330 also on the HCL.

The resource.go was collapsed on my diff maybe you forgot to review it.

xescugc · 2019-07-16T16:33:20Z

Did the changes.

xlr-8 · 2019-07-19T12:20:45Z

provider/resource.go

+	// to the Resource and could return []Resource if
+	// it imported more than one state, this list does not
+	// include the actual Resource on parameters, so if
+	// len([]Resource) == 0 menas only the Resource it's imported


means*
is* imported

xlr-8 · 2019-07-22T08:43:02Z

provider/resource.go

-			}
-			gpsKey = fmt.Sprintf("%s/%s", gpsKey, gp)
+	resources := make([]Resource, 0, len(newInstanceStates)-1)
+	// We assume that the first state it's the one for this Resource


is the one*

xlr-8 · 2019-07-22T08:43:52Z

provider/resource.go

-	})
+func (r *resource) ImportState() ([]Resource, error) {
+	// If it does not support import do not try
+	if r.TFResource().Importer == nil {


Could it make sense to log that as a Warning/Debug info?

xlr-8 · 2019-07-29T07:33:13Z

The code generated doesn't quite seem to be valid for terraform, some blocks/quote and optional params are generating errors.

The resources that are imported do not require sets, with the ID it's enough

xescugc · 2019-10-28T11:24:05Z

Ok done, some of the issues we are having now are related to TF not working on some form and we have to patch it on top normally. I would say to merge this 0.11 -> 0.12 and then fix the other issues in separated PRs.

xlr-8 · 2019-10-29T10:33:22Z

aws/resources.go

-			return nil, err
-		}
-		err = r.Data().Set("groups", groupSet)
+		r, err := initializeResource(a, strings.Join(append([]string{un}, groupNames...), "/"), resourceType)


I don't know if adding a small comment regarding each format could help

each format could help

Not only this one

xlr-8 · 2019-10-29T10:34:37Z

hcl/format.go

+)
+
+var (
+	// transformers are all the transformmations


transformations*

xlr-8 · 2019-10-29T10:35:36Z

hcl/format.go

+		replaceFn func([]byte) []byte
+	}{
+		{
+			// Replace all the `"key" = "value"` for `key = "value"`


I believe this is excluding key with special characters such as: cycloid.io, might be worth mentioning, as this is a "new element" from TC 0.12

xlr-8 · 2019-10-29T10:36:08Z

hcl/format.go

+			replace: []byte("}\n\n"),
+		},
+		{
+			// Remove "" from resources definition


More info on this?

xlr-8 · 2019-10-29T10:36:56Z

hcl/format.go

+	}
+)
+
+// Format format the hcl to have a better formatter that the default one


xlr-8 · 2019-10-29T10:42:27Z

hcl/format.go

+			replace: []byte(`$1 =`),
+		},
+		{
+			// Replace all the `key = {` for `key {`


Do you think that works for everything?
Because you can also have other ones such as: https://www.terraform.io/docs/providers/aws/r/cloudwatch_metric_alarm.html

Checked how the other handle it?
Could there be a way to generate many (all?) AWS resource and try to run our code against it to make sure it imports them properly?

xlr-8 · 2019-10-29T11:08:43Z

util/retry.go

@@ -25,6 +26,13 @@ func Retry(rfn RetryFn, times int, interval time.Duration) error {
 		if times == 0 {
 			return err
 		}
+		// If the error it's from the stdlib we just continue with them


xescugc · 2019-10-29T16:16:37Z

Done

xlr-8

Some of the typos previously mentioned weren't fixed

xlr-8 · 2019-10-29T16:44:23Z

aws/resources.go

-			return nil, err
-		}
-		err = r.Data().Set("groups", groupSet)
+		r, err := initializeResource(a, strings.Join(append([]string{un}, groupNames...), "/"), resourceType)


each format could help

Not only this one

xlr-8 · 2019-10-29T16:50:22Z

provider/resource.go

@@ -512,6 +513,12 @@ func mergeFullConfig(cfgr *schema.ResourceData, sch map[string]*schema.Schema, k
 			}
 		}

+		// If it's a type map, we'll prefix the key with '=tc=' to know
+		// that it's an attribute so we have to keep the '=' when formatting
+		// the HCL


Good idea :)

Using all the plugin methods without directly using the Plugins. Some of this methods had to be 'copied' from TF itself as those where private so not beeing able to use them directly, the ones that match this caracteristics have comments that mention it.

Also refactored the resources to use the new 'NewResource'

One for the HCL logic 'hcl/' and one for the TF State logic 'state/'. This way the implementation of the interfaces it's not on the same package as the definition of the interface and causes some cycle imports on other packages

Changed the 'New' statement of the Writers to use the new packages

This way if we removed some mock now it'll also be removed, or if we accidentally remove a generate frome some 'interface' now it'll be detected. It has happend with the generate from the 'writer.Writer', the '//go:generate' was removed by mistake and no error was triggered anywhere, this will trigger an error now

Update the TF version to 0.12.3 and other dependencies

To have the right ID on the field, it's expected to be an specific value

So the users know which ones they have to have and we use

The AWS 'IsErrorRetryable' retries all std errors, so to prevent that (to prevent our own errors to be retried) we'll ignore std errors so only errors that internally have an strucure will be passed

xescugc added the Status: Need review label Jul 9, 2019

xescugc force-pushed the fg-tf-0.12 branch 2 times, most recently from 60bf400 to b140975 Compare July 10, 2019 13:56

xlr-8 suggested changes Jul 16, 2019

View reviewed changes

xescugc mentioned this pull request Jul 18, 2019

panic during parsing #36

Closed

xlr-8 reviewed Jul 22, 2019

View reviewed changes

xescugc force-pushed the fg-tf-0.12 branch from 4d22eac to 46a5177 Compare July 22, 2019 11:32

xlr-8 added Status: Blocked and removed Status: Need review labels Jul 29, 2019

xescugc force-pushed the fg-tf-0.12 branch 2 times, most recently from bd5f27e to 5f9328e Compare October 17, 2019 11:51

aws: Remove the not needed code as

61ae826

The resources that are imported do not require sets, with the ID it's enough

xescugc force-pushed the fg-tf-0.12 branch from 1fd83f7 to f21dcd0 Compare October 24, 2019 08:44

xescugc removed the Status: Blocked label Oct 28, 2019

xlr-8 reviewed Oct 29, 2019

View reviewed changes

xlr-8 suggested changes Oct 29, 2019

View reviewed changes

xlr-8 approved these changes Oct 29, 2019

View reviewed changes

xescugc added 9 commits October 29, 2019 18:18

aws: Add the TFProvider function to it

412d595

Also refactored the resources to use the new 'NewResource'

writer: Split the logic into different packages

2ebcecd

One for the HCL logic 'hcl/' and one for the TF State logic 'state/'. This way the implementation of the interfaces it's not on the same package as the definition of the interface and causes some cycle imports on other packages

cache: Use the new 'prvider.NewResource'

ad83d2f

cmd: Update to use the new Writer (hcl and state)

0c365a4

Changed the 'New' statement of the Writers to use the new packages

mock: Update the mocks

8de1cf8

hcl: Implement the changes for the HCL format on TF 0.12

f9b8a15

mod: Update TF to 0.12.3 version

dadd99e

Update the TF version to 0.12.3 and other dependencies

xescugc added 5 commits October 29, 2019 18:21

CHANGELOG: Add a new entry

f27a848

aws: Update import for type 'iam_account_password_policy'

ba36af4

To have the right ID on the field, it's expected to be an specific value

Rebase: Conflicts solved aftert the rebase from master

2103fa5

README: Update the version we are using of Providers and TF

eb0e57e

So the users know which ones they have to have and we use

util: Fix retry that always retries all errors

c077327

The AWS 'IsErrorRetryable' retries all std errors, so to prevent that (to prevent our own errors to be retried) we'll ignore std errors so only errors that internally have an strucure will be passed

xescugc force-pushed the fg-tf-0.12 branch from 31cbb31 to c077327 Compare October 29, 2019 17:23

xescugc merged commit 79e0348 into master Oct 29, 2019

xescugc deleted the fg-tf-0.12 branch October 29, 2019 17:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to TF 0.12.3 #33

Migrate to TF 0.12.3 #33

xescugc commented Jul 9, 2019

xlr-8 left a comment

xlr-8 Jul 16, 2019

xlr-8 Jul 16, 2019

xlr-8 Jul 16, 2019

xlr-8 Jul 16, 2019

xlr-8 Jul 16, 2019

xlr-8 Jul 16, 2019

xlr-8 Jul 16, 2019

xlr-8 Jul 16, 2019

xescugc Jul 16, 2019

xescugc commented Jul 16, 2019

xlr-8 Jul 19, 2019

xlr-8 Jul 22, 2019

xlr-8 Jul 22, 2019

xlr-8 commented Jul 29, 2019

xescugc commented Oct 28, 2019

xlr-8 Oct 29, 2019

xlr-8 Oct 29, 2019

xlr-8 Oct 29, 2019

xlr-8 Oct 29, 2019

xlr-8 Oct 29, 2019

xlr-8 Oct 29, 2019

xlr-8 Oct 29, 2019

xlr-8 Oct 29, 2019

xescugc commented Oct 29, 2019

xlr-8 left a comment

xlr-8 Oct 29, 2019

xlr-8 Oct 29, 2019

Migrate to TF 0.12.3 #33

Migrate to TF 0.12.3 #33

Conversation

xescugc commented Jul 9, 2019

xlr-8 left a 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

xescugc commented Jul 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xlr-8 commented Jul 29, 2019

xescugc commented Oct 28, 2019

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

xescugc commented Oct 29, 2019

xlr-8 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment