From 6f656f7475a4b84787e9a9a80fc9698f438e5691 Mon Sep 17 00:00:00 2001 From: Masayuki Morita Date: Mon, 2 Oct 2023 18:47:34 +0900 Subject: [PATCH 1/5] Add support for Terraform v1.6 --- .github/workflows/test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 05bab3e..19e1c56 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -39,9 +39,9 @@ jobs: fail-fast: false matrix: terraform: + - 1.6.0 - 1.5.7 - 1.4.7 - - 1.3.10 - 0.12.31 env: TERRAFORM_VERSION: ${{ matrix.terraform }} From 7591205b3f296e07e903a10d5347467a4ad0b868 Mon Sep 17 00:00:00 2001 From: Masayuki Morita Date: Tue, 3 Oct 2023 15:40:08 +0900 Subject: [PATCH 2/5] Mock iam_endpoint explicitly for testing with localstack Starting from Terraform v1.6, the s3 backend configuration has changed, and the tfmigrate acceptance test fails. I suspect that the authentication mechanism may have been changed with the migration to aws-sdk-go-v2; I have tried a few things and found that I can authenticate by specifying the `iam_endpoint`. The `iam_endpoint` argument has been deprecated since Terraformv1.6. It is recommended to use the newly added `endpoints` argument, though, for running tests on multiple Terraform versions, the test settings must remain compatible with the older versions. This is a test-only issue, so I'll use the deprecated `iam_endpoint` for now. In the future, we should change the backend configuration depending on the Terraform version. --- test-fixtures/backend_s3/config.tf | 3 ++- test-fixtures/storage_gcs/config.tf | 3 ++- test-fixtures/storage_s3/config.tf | 3 ++- tfexec/terraform_test.go | 5 +++-- tfexec/test_helper.go | 5 +++-- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/test-fixtures/backend_s3/config.tf b/test-fixtures/backend_s3/config.tf index 6de089f..840a31d 100644 --- a/test-fixtures/backend_s3/config.tf +++ b/test-fixtures/backend_s3/config.tf @@ -5,8 +5,9 @@ terraform { bucket = "tfstate-test" key = "test/terraform.tfstate" - # mock s3 endpoint with localstack + # mock s3/iam endpoint with localstack endpoint = "http://localstack:4566" + iam_endpoint = "http://localstack:4566" access_key = "dummy" secret_key = "dummy" skip_credentials_validation = true diff --git a/test-fixtures/storage_gcs/config.tf b/test-fixtures/storage_gcs/config.tf index 6de089f..840a31d 100644 --- a/test-fixtures/storage_gcs/config.tf +++ b/test-fixtures/storage_gcs/config.tf @@ -5,8 +5,9 @@ terraform { bucket = "tfstate-test" key = "test/terraform.tfstate" - # mock s3 endpoint with localstack + # mock s3/iam endpoint with localstack endpoint = "http://localstack:4566" + iam_endpoint = "http://localstack:4566" access_key = "dummy" secret_key = "dummy" skip_credentials_validation = true diff --git a/test-fixtures/storage_s3/config.tf b/test-fixtures/storage_s3/config.tf index 6de089f..840a31d 100644 --- a/test-fixtures/storage_s3/config.tf +++ b/test-fixtures/storage_s3/config.tf @@ -5,8 +5,9 @@ terraform { bucket = "tfstate-test" key = "test/terraform.tfstate" - # mock s3 endpoint with localstack + # mock s3/iam endpoint with localstack endpoint = "http://localstack:4566" + iam_endpoint = "http://localstack:4566" access_key = "dummy" secret_key = "dummy" skip_credentials_validation = true diff --git a/tfexec/terraform_test.go b/tfexec/terraform_test.go index 84d9f41..21f6b6b 100644 --- a/tfexec/terraform_test.go +++ b/tfexec/terraform_test.go @@ -188,8 +188,9 @@ terraform { # bucket = "tfstate-test" key = "%s/terraform.tfstate" - # mock s3 endpoint with localstack + # mock s3/iam endpoint with localstack endpoint = "%s" + iam_endpoint = "%s" access_key = "dummy" secret_key = "dummy" skip_credentials_validation = true @@ -197,7 +198,7 @@ terraform { force_path_style = true } } -`, t.Name(), endpoint) +`, t.Name(), endpoint, endpoint) source := ` resource "null_resource" "foo" {} resource "null_resource" "bar" {} diff --git a/tfexec/test_helper.go b/tfexec/test_helper.go index 72818b8..c8d0e1c 100644 --- a/tfexec/test_helper.go +++ b/tfexec/test_helper.go @@ -271,8 +271,9 @@ terraform { bucket = "%s" key = "%s" - # mock s3 endpoint with localstack + # mock s3/iam endpoint with localstack endpoint = "%s" + iam_endpoint = "%s" access_key = "%s" secret_key = "%s" skip_credentials_validation = true @@ -280,7 +281,7 @@ terraform { force_path_style = true } } -`, TestS3Region, TestS3Bucket, key, endpoint, TestS3AccessKey, TestS3SecretKey) +`, TestS3Region, TestS3Bucket, key, endpoint, endpoint, TestS3AccessKey, TestS3SecretKey) return backendConfig } From ae31322fb60fef4185437f604a6a257a13bc6b41 Mon Sep 17 00:00:00 2001 From: Masayuki Morita Date: Tue, 3 Oct 2023 18:01:48 +0900 Subject: [PATCH 3/5] Fix issue in switch back test error messages changed in Terraform v1.6 SwitchBackToRemoteFuncError tests verify error messages, but the error message for missing bucket key in the s3 backend differs depending on the Terraform version. Define a helper function to hide the difference. --- tfmigrate/multi_state_migrator_test.go | 10 +++---- tfmigrate/state_migrator_test.go | 10 +++---- tfmigrate/test_helper.go | 38 ++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 12 deletions(-) create mode 100644 tfmigrate/test_helper.go diff --git a/tfmigrate/multi_state_migrator_test.go b/tfmigrate/multi_state_migrator_test.go index 745e6c2..2dda6b5 100644 --- a/tfmigrate/multi_state_migrator_test.go +++ b/tfmigrate/multi_state_migrator_test.go @@ -1010,9 +1010,8 @@ resource "null_resource" "qux" {} t.Fatalf("expected migrator plan error") } - expected := "Error: \"bucket\": required field is not set" - if !strings.Contains(err.Error(), expected) { - t.Fatalf("expected migrator plan error to contain %s, got: %s", expected, err.Error()) + if !containsBucketRequiredError(err) { + t.Fatalf("expected migrator plan error to contain bucket required error: %s", err.Error()) } } @@ -1130,8 +1129,7 @@ resource "null_resource" "qux" {} t.Fatalf("expected migrator plan error to contain %s, got: %s", expected, err.Error()) } - expected = "Error: \"bucket\": required field is not set" - if !strings.Contains(err.Error(), expected) { - t.Fatalf("expected migrator plan error to contain %s, got: %s", expected, err.Error()) + if !containsBucketRequiredError(err) { + t.Fatalf("expected migrator plan error to contain bucket required error: %s", err.Error()) } } diff --git a/tfmigrate/state_migrator_test.go b/tfmigrate/state_migrator_test.go index d0a7173..e2fe828 100644 --- a/tfmigrate/state_migrator_test.go +++ b/tfmigrate/state_migrator_test.go @@ -529,9 +529,8 @@ resource "null_resource" "bar" {} t.Fatalf("expected migrator plan error") } - expected := "Error: \"bucket\": required field is not set" - if !strings.Contains(err.Error(), expected) { - t.Fatalf("expected migrator plan error to contain %s, got: %s", expected, err.Error()) + if !containsBucketRequiredError(err) { + t.Fatalf("expected migrator plan error to contain bucket required error: %s", err.Error()) } } @@ -618,8 +617,7 @@ resource "null_resource" "bar" {} t.Fatalf("expected migrator plan error to contain %s, got: %s", expected, err.Error()) } - expected = "Error: \"bucket\": required field is not set" - if !strings.Contains(err.Error(), expected) { - t.Fatalf("expected migrator plan error to contain %s, got: %s", expected, err.Error()) + if !containsBucketRequiredError(err) { + t.Fatalf("expected migrator plan error to contain bucket required error: %s", err.Error()) } } diff --git a/tfmigrate/test_helper.go b/tfmigrate/test_helper.go new file mode 100644 index 0000000..92a0de0 --- /dev/null +++ b/tfmigrate/test_helper.go @@ -0,0 +1,38 @@ +package tfmigrate + +import "regexp" + +// SwitchBackToRemoteFuncError tests verify error messages, but the +// error message for missing bucket key in the s3 backend differs +// depending on the Terraform version. +// Define a helper function to hide the difference. +// +// # Terraform v1.5 +// +// ``` +// Error: "bucket": required field is not set +// ``` +// +// # Terraform v1.6 +// +// ``` +// +// Error: Missing Required Value +// +// on main.tf line 4, in terraform: +// 4: backend "s3" { +// +// The attribute "bucket" is required by the backend. +// +// Refer to the backend documentation for additional information which +// attributes are required. +// +// ``` +const testBucketRequiredErrorLegacyTF = `Error: "bucket": required field is not set` +const testBucketRequiredErrorTF16 = `The attribute "bucket" is required by the backend` + +var testBucketRequiredErrorRE = regexp.MustCompile(testBucketRequiredErrorLegacyTF + `|` + testBucketRequiredErrorTF16) + +func containsBucketRequiredError(err error) bool { + return testBucketRequiredErrorRE.MatchString(err.Error()) +} From bb079fd0cf75ef5674d299d9d889724c96efa855 Mon Sep 17 00:00:00 2001 From: Masayuki Morita Date: Tue, 3 Oct 2023 22:52:01 +0900 Subject: [PATCH 4/5] Truncate pre-release version before comparing The hashicorp/go-version returns false when comparing pre-releases, for example 1.6.0-rc1 >= 0.13. This is counter-intuitive for determining the presence or absence of a feature, so remove the pre-release information before comparing. --- tfexec/terraform_providers_test.go | 14 +++----------- tfexec/terraform_state_replace_provider.go | 7 ++++++- tfexec/terraform_version.go | 22 ++++++++++++++++++++++ 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/tfexec/terraform_providers_test.go b/tfexec/terraform_providers_test.go index 90da214..924e4d4 100644 --- a/tfexec/terraform_providers_test.go +++ b/tfexec/terraform_providers_test.go @@ -2,11 +2,8 @@ package tfexec import ( "context" - "fmt" "reflect" "testing" - - "github.com/hashicorp/go-version" ) var providersStdout = ` @@ -100,18 +97,13 @@ resource "null_resource" "bar" {} t.Fatalf("failed to run terraform providers: %s", err) } - v, err := terraformCLI.Version(context.Background()) - if err != nil { - t.Fatalf("unexpected version error: %s", err) - } - - constraints, err := version.NewConstraint(fmt.Sprintf(">= %s", MinimumTerraformVersionForStateReplaceProvider)) + supportsStateReplaceProvider, _, err := terraformCLI.SupportsStateReplaceProvider(context.Background()) if err != nil { - t.Fatalf("unexpected version constraint error: %s", err) + t.Fatalf("failed to determine if Terraform version supports state replace-provider: %s", err) } want := providersStdout - if !constraints.Check(v) { + if !supportsStateReplaceProvider { want = legacyProvidersStdout } diff --git a/tfexec/terraform_state_replace_provider.go b/tfexec/terraform_state_replace_provider.go index 0c424f4..40c4eae 100644 --- a/tfexec/terraform_state_replace_provider.go +++ b/tfexec/terraform_state_replace_provider.go @@ -34,7 +34,12 @@ func (c *terraformCLI) SupportsStateReplaceProvider(ctx context.Context) (bool, return false, constraints, err } - if !constraints.Check(v) { + ver, err := truncatePreReleaseVersion(v) + if err != nil { + return false, constraints, err + } + + if !constraints.Check(ver) { return false, constraints, nil } diff --git a/tfexec/terraform_version.go b/tfexec/terraform_version.go index ea21b87..5fab486 100644 --- a/tfexec/terraform_version.go +++ b/tfexec/terraform_version.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "regexp" + "strings" "github.com/hashicorp/go-version" ) @@ -29,3 +30,24 @@ func (c *terraformCLI) Version(ctx context.Context) (*version.Version, error) { return version, nil } + +// truncatePreReleaseVersion is a helper function that removes +// pre-release information. +// The hashicorp/go-version returns false when comparing pre-releases, for +// example 1.6.0-rc1 >= 0.13. This is counter-intuitive for determining the +// presence or absence of a feature, so remove the pre-release information +// before comparing. +func truncatePreReleaseVersion(v *version.Version) (*version.Version, error) { + if v.Prerelease() == "" { + return v, nil + } + + vs, _, _ := strings.Cut(v.String(), "-") + + ver, err := version.NewVersion(vs) + if err != nil { + return nil, err + } + + return ver, nil +} From 72284a7b2cca8de88e472f92d389abcdce671287 Mon Sep 17 00:00:00 2001 From: Masayuki Morita Date: Thu, 5 Oct 2023 10:34:19 +0900 Subject: [PATCH 5/5] Add tests for truncatePreReleaseVersion, containsBucketRequiredError --- tfexec/terraform_version_test.go | 43 +++++++++++++++++++++++++++ tfmigrate/test_helper.go | 22 -------------- tfmigrate/test_helper_test.go | 50 ++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 22 deletions(-) create mode 100644 tfmigrate/test_helper_test.go diff --git a/tfexec/terraform_version_test.go b/tfexec/terraform_version_test.go index 61adddb..5aca27f 100644 --- a/tfexec/terraform_version_test.go +++ b/tfexec/terraform_version_test.go @@ -5,6 +5,8 @@ import ( "fmt" "os" "testing" + + "github.com/hashicorp/go-version" ) func TestTerraformCLIVersion(t *testing.T) { @@ -87,3 +89,44 @@ func TestAccTerraformCLIVersion(t *testing.T) { } fmt.Printf("got = %s\n", got) } + +func TestTruncatePreReleaseVersion(t *testing.T) { + cases := []struct { + desc string + v string + want string + ok bool + }{ + { + desc: "pre-release", + v: "1.6.0-rc1", + want: "1.6.0", + ok: true, + }, + { + desc: "not pre-release", + v: "1.6.0", + want: "1.6.0", + ok: true, + }, + } + + for _, tc := range cases { + t.Run(tc.desc, func(t *testing.T) { + v, err := version.NewVersion(tc.v) + if err != nil { + t.Fatalf("failed to parse version: %s", err) + } + got, err := truncatePreReleaseVersion(v) + if tc.ok && err != nil { + t.Fatalf("unexpected err: %s", err) + } + if !tc.ok && err == nil { + t.Fatalf("expected to return an error, but no error, got = %s", got) + } + if tc.ok && got.String() != tc.want { + t.Errorf("got: %s, want: %s", got, tc.want) + } + }) + } +} diff --git a/tfmigrate/test_helper.go b/tfmigrate/test_helper.go index 92a0de0..1660adc 100644 --- a/tfmigrate/test_helper.go +++ b/tfmigrate/test_helper.go @@ -6,28 +6,6 @@ import "regexp" // error message for missing bucket key in the s3 backend differs // depending on the Terraform version. // Define a helper function to hide the difference. -// -// # Terraform v1.5 -// -// ``` -// Error: "bucket": required field is not set -// ``` -// -// # Terraform v1.6 -// -// ``` -// -// Error: Missing Required Value -// -// on main.tf line 4, in terraform: -// 4: backend "s3" { -// -// The attribute "bucket" is required by the backend. -// -// Refer to the backend documentation for additional information which -// attributes are required. -// -// ``` const testBucketRequiredErrorLegacyTF = `Error: "bucket": required field is not set` const testBucketRequiredErrorTF16 = `The attribute "bucket" is required by the backend` diff --git a/tfmigrate/test_helper_test.go b/tfmigrate/test_helper_test.go new file mode 100644 index 0000000..3c50c59 --- /dev/null +++ b/tfmigrate/test_helper_test.go @@ -0,0 +1,50 @@ +package tfmigrate + +import ( + "errors" + "testing" +) + +func TestContainsBucketRequiredError(t *testing.T) { + cases := []struct { + desc string + msg string + want bool + }{ + { + desc: "terraform v1.5", + msg: `Error: "bucket": required field is not set`, + want: true, + }, + { + desc: "terraform v1.6", + msg: ` +Error: Missing Required Value + + on main.tf line 4, in terraform: + 4: backend "s3" { + +The attribute "bucket" is required by the backend. + +Refer to the backend documentation for additional information which +attributes are required. + +`, + want: true, + }, + { + desc: "unknown", + msg: `Error: unknown`, + want: false, + }, + } + + for _, tc := range cases { + t.Run(tc.desc, func(t *testing.T) { + got := containsBucketRequiredError(errors.New(tc.msg)) + if got != tc.want { + t.Errorf("got: %t, want: %t", got, tc.want) + } + }) + } +}