From 18d54c11290fc1300fe3419fd577c14068e922e6 Mon Sep 17 00:00:00 2001 From: Chris Arcand Date: Fri, 27 Aug 2021 12:13:41 -0500 Subject: [PATCH] Allow cloud block overrides These changes allow cloud blocks to be overridden by backend blocks and vice versa; the logic follows the current backend behavior of a block overriding a preceding block in full, with no merges. --- internal/configs/module.go | 26 +++-- internal/configs/module_test.go | 103 ++++++++++++++++++ .../override-cloud-duplicates/main.tf | 14 +++ .../override-cloud-duplicates/override.tf | 11 ++ .../override-backend-no-base/main.tf | 7 ++ .../override-backend-no-base/override.tf | 5 + .../override-backend-with-cloud/main.tf | 13 +++ .../override-backend-with-cloud/override.tf | 5 + .../valid-modules/override-backend/main.tf | 13 +++ .../override-backend/override.tf | 5 + .../override-cloud-no-base/main.tf | 7 ++ .../override-cloud-no-base/override.tf | 5 + .../valid-modules/override-cloud/main.tf | 14 +++ .../valid-modules/override-cloud/override.tf | 5 + 14 files changed, 223 insertions(+), 10 deletions(-) create mode 100644 internal/configs/testdata/invalid-modules/override-cloud-duplicates/main.tf create mode 100644 internal/configs/testdata/invalid-modules/override-cloud-duplicates/override.tf create mode 100644 internal/configs/testdata/valid-modules/override-backend-no-base/main.tf create mode 100644 internal/configs/testdata/valid-modules/override-backend-no-base/override.tf create mode 100644 internal/configs/testdata/valid-modules/override-backend-with-cloud/main.tf create mode 100644 internal/configs/testdata/valid-modules/override-backend-with-cloud/override.tf create mode 100644 internal/configs/testdata/valid-modules/override-backend/main.tf create mode 100644 internal/configs/testdata/valid-modules/override-backend/override.tf create mode 100644 internal/configs/testdata/valid-modules/override-cloud-no-base/main.tf create mode 100644 internal/configs/testdata/valid-modules/override-cloud-no-base/override.tf create mode 100644 internal/configs/testdata/valid-modules/override-cloud/main.tf create mode 100644 internal/configs/testdata/valid-modules/override-cloud/override.tf diff --git a/internal/configs/module.go b/internal/configs/module.go index b75cc7fd5..c2088b9fd 100644 --- a/internal/configs/module.go +++ b/internal/configs/module.go @@ -379,6 +379,7 @@ func (m *Module) mergeFile(file *File) hcl.Diagnostics { if len(file.Backends) != 0 { switch len(file.Backends) { case 1: + m.CloudConfig = nil // A backend block is mutually exclusive with a cloud one, and overwrites any cloud config m.Backend = file.Backends[0] default: // An override file with multiple backends is still invalid, even @@ -392,16 +393,21 @@ func (m *Module) mergeFile(file *File) hcl.Diagnostics { } } - // TODO: This restriction is temporary. Overrides should be allowed, but have the added - // complexity of needing to also override a 'backend' block, so this work is being deferred - // for now. - for _, m := range file.CloudConfigs { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Cannot override 'cloud' configuration", - Detail: "Terraform Cloud configuration blocks can appear only in normal files, not in override files.", - Subject: m.DeclRange.Ptr(), - }) + if len(file.CloudConfigs) != 0 { + switch len(file.CloudConfigs) { + case 1: + m.Backend = nil // A cloud block is mutually exclusive with a backend one, and overwrites any backend + m.CloudConfig = file.CloudConfigs[0] + default: + // An override file with multiple cloud blocks is still invalid, even + // though it can override cloud/backend blocks from _other_ files. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Duplicate Terraform Cloud configurations", + Detail: fmt.Sprintf("A module may have only one 'cloud' block configuring Terraform Cloud. Terraform Cloud was previously configured at %s.", file.CloudConfigs[0].DeclRange), + Subject: &file.CloudConfigs[1].DeclRange, + }) + } } for _, pc := range file.ProviderConfigs { diff --git a/internal/configs/module_test.go b/internal/configs/module_test.go index 394eaff1c..3eea93d37 100644 --- a/internal/configs/module_test.go +++ b/internal/configs/module_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/hashicorp/terraform/internal/addrs" + "github.com/zclconf/go-cty/cty" ) // TestNewModule_provider_fqns exercises module.gatherProviderLocalNames() @@ -309,3 +310,105 @@ func TestImpliedProviderForUnqualifiedType(t *testing.T) { } } } + +func TestModule_backend_override(t *testing.T) { + mod, diags := testModuleFromDir("testdata/valid-modules/override-backend") + if diags.HasErrors() { + t.Fatal(diags.Error()) + } + + gotType := mod.Backend.Type + wantType := "bar" + + if gotType != wantType { + t.Errorf("wrong result for backend type: got %#v, want %#v\n", gotType, wantType) + } + + attrs, _ := mod.Backend.Config.JustAttributes() + + gotAttr, diags := attrs["path"].Expr.Value(nil) + if diags.HasErrors() { + t.Fatal(diags.Error()) + } + + wantAttr := cty.StringVal("CHANGED/relative/path/to/terraform.tfstate") + + if !gotAttr.RawEquals(wantAttr) { + t.Errorf("wrong result for backend 'path': got %#v, want %#v\n", gotAttr, wantAttr) + } +} + +// Unlike most other overrides, backend blocks do not require a base configuration in a primary +// configuration file, as an omitted backend there implies the local backend. +func TestModule_backend_override_no_base(t *testing.T) { + mod, diags := testModuleFromDir("testdata/valid-modules/override-backend-no-base") + if diags.HasErrors() { + t.Fatal(diags.Error()) + } + + if mod.Backend == nil { + t.Errorf("expected module Backend not to be nil") + } +} + +func TestModule_cloud_override_backend(t *testing.T) { + mod, diags := testModuleFromDir("testdata/valid-modules/override-backend-with-cloud") + if diags.HasErrors() { + t.Fatal(diags.Error()) + } + + if mod.Backend != nil { + t.Errorf("expected module Backend to be nil") + } + + if mod.CloudConfig == nil { + t.Errorf("expected module CloudConfig not to be nil") + } +} + +// Unlike most other overrides, cloud blocks do not require a base configuration in a primary +// configuration file, as an omitted backend there implies the local backend and cloud blocks +// override backends. +func TestModule_cloud_override_no_base(t *testing.T) { + mod, diags := testModuleFromDir("testdata/valid-modules/override-cloud-no-base") + if diags.HasErrors() { + t.Fatal(diags.Error()) + } + + if mod.CloudConfig == nil { + t.Errorf("expected module CloudConfig not to be nil") + } +} + +func TestModule_cloud_override(t *testing.T) { + mod, diags := testModuleFromDir("testdata/valid-modules/override-cloud") + if diags.HasErrors() { + t.Fatal(diags.Error()) + } + + attrs, _ := mod.CloudConfig.Config.JustAttributes() + + gotAttr, diags := attrs["organization"].Expr.Value(nil) + if diags.HasErrors() { + t.Fatal(diags.Error()) + } + + wantAttr := cty.StringVal("CHANGED") + + if !gotAttr.RawEquals(wantAttr) { + t.Errorf("wrong result for Cloud 'organization': got %#v, want %#v\n", gotAttr, wantAttr) + } + + // The override should have completely replaced the cloud block in the primary file, no merging + if attrs["should_not_be_present_with_override"] != nil { + t.Errorf("expected 'should_not_be_present_with_override' attribute to be nil") + } +} + +func TestModule_cloud_duplicate_overrides(t *testing.T) { + _, diags := testModuleFromDir("testdata/invalid-modules/override-cloud-duplicates") + want := `Duplicate Terraform Cloud configurations` + if got := diags.Error(); !strings.Contains(got, want) { + t.Fatalf("expected module error to contain %q\nerror was:\n%s", want, got) + } +} diff --git a/internal/configs/testdata/invalid-modules/override-cloud-duplicates/main.tf b/internal/configs/testdata/invalid-modules/override-cloud-duplicates/main.tf new file mode 100644 index 000000000..2de9a58dd --- /dev/null +++ b/internal/configs/testdata/invalid-modules/override-cloud-duplicates/main.tf @@ -0,0 +1,14 @@ +terraform { + cloud { + organization = "foo" + should_not_be_present_with_override = true + } +} + +resource "aws_instance" "web" { + ami = "ami-1234" + security_groups = [ + "foo", + "bar", + ] +} diff --git a/internal/configs/testdata/invalid-modules/override-cloud-duplicates/override.tf b/internal/configs/testdata/invalid-modules/override-cloud-duplicates/override.tf new file mode 100644 index 000000000..17ef01150 --- /dev/null +++ b/internal/configs/testdata/invalid-modules/override-cloud-duplicates/override.tf @@ -0,0 +1,11 @@ +terraform { + cloud { + organization = "foo" + } +} + +terraform { + cloud { + organization = "bar" + } +} diff --git a/internal/configs/testdata/valid-modules/override-backend-no-base/main.tf b/internal/configs/testdata/valid-modules/override-backend-no-base/main.tf new file mode 100644 index 000000000..7bb1380e6 --- /dev/null +++ b/internal/configs/testdata/valid-modules/override-backend-no-base/main.tf @@ -0,0 +1,7 @@ +resource "aws_instance" "web" { + ami = "ami-1234" + security_groups = [ + "foo", + "bar", + ] +} diff --git a/internal/configs/testdata/valid-modules/override-backend-no-base/override.tf b/internal/configs/testdata/valid-modules/override-backend-no-base/override.tf new file mode 100644 index 000000000..d57fade63 --- /dev/null +++ b/internal/configs/testdata/valid-modules/override-backend-no-base/override.tf @@ -0,0 +1,5 @@ +terraform { + backend "bar" { + path = "CHANGED/relative/path/to/terraform.tfstate" + } +} diff --git a/internal/configs/testdata/valid-modules/override-backend-with-cloud/main.tf b/internal/configs/testdata/valid-modules/override-backend-with-cloud/main.tf new file mode 100644 index 000000000..56fb72f32 --- /dev/null +++ b/internal/configs/testdata/valid-modules/override-backend-with-cloud/main.tf @@ -0,0 +1,13 @@ +terraform { + backend "foo" { + path = "relative/path/to/terraform.tfstate" + } +} + +resource "aws_instance" "web" { + ami = "ami-1234" + security_groups = [ + "foo", + "bar", + ] +} diff --git a/internal/configs/testdata/valid-modules/override-backend-with-cloud/override.tf b/internal/configs/testdata/valid-modules/override-backend-with-cloud/override.tf new file mode 100644 index 000000000..51ae925fb --- /dev/null +++ b/internal/configs/testdata/valid-modules/override-backend-with-cloud/override.tf @@ -0,0 +1,5 @@ +terraform { + cloud { + organization = "foo" + } +} diff --git a/internal/configs/testdata/valid-modules/override-backend/main.tf b/internal/configs/testdata/valid-modules/override-backend/main.tf new file mode 100644 index 000000000..56fb72f32 --- /dev/null +++ b/internal/configs/testdata/valid-modules/override-backend/main.tf @@ -0,0 +1,13 @@ +terraform { + backend "foo" { + path = "relative/path/to/terraform.tfstate" + } +} + +resource "aws_instance" "web" { + ami = "ami-1234" + security_groups = [ + "foo", + "bar", + ] +} diff --git a/internal/configs/testdata/valid-modules/override-backend/override.tf b/internal/configs/testdata/valid-modules/override-backend/override.tf new file mode 100644 index 000000000..d57fade63 --- /dev/null +++ b/internal/configs/testdata/valid-modules/override-backend/override.tf @@ -0,0 +1,5 @@ +terraform { + backend "bar" { + path = "CHANGED/relative/path/to/terraform.tfstate" + } +} diff --git a/internal/configs/testdata/valid-modules/override-cloud-no-base/main.tf b/internal/configs/testdata/valid-modules/override-cloud-no-base/main.tf new file mode 100644 index 000000000..7bb1380e6 --- /dev/null +++ b/internal/configs/testdata/valid-modules/override-cloud-no-base/main.tf @@ -0,0 +1,7 @@ +resource "aws_instance" "web" { + ami = "ami-1234" + security_groups = [ + "foo", + "bar", + ] +} diff --git a/internal/configs/testdata/valid-modules/override-cloud-no-base/override.tf b/internal/configs/testdata/valid-modules/override-cloud-no-base/override.tf new file mode 100644 index 000000000..51ae925fb --- /dev/null +++ b/internal/configs/testdata/valid-modules/override-cloud-no-base/override.tf @@ -0,0 +1,5 @@ +terraform { + cloud { + organization = "foo" + } +} diff --git a/internal/configs/testdata/valid-modules/override-cloud/main.tf b/internal/configs/testdata/valid-modules/override-cloud/main.tf new file mode 100644 index 000000000..2de9a58dd --- /dev/null +++ b/internal/configs/testdata/valid-modules/override-cloud/main.tf @@ -0,0 +1,14 @@ +terraform { + cloud { + organization = "foo" + should_not_be_present_with_override = true + } +} + +resource "aws_instance" "web" { + ami = "ami-1234" + security_groups = [ + "foo", + "bar", + ] +} diff --git a/internal/configs/testdata/valid-modules/override-cloud/override.tf b/internal/configs/testdata/valid-modules/override-cloud/override.tf new file mode 100644 index 000000000..a4a7752ca --- /dev/null +++ b/internal/configs/testdata/valid-modules/override-cloud/override.tf @@ -0,0 +1,5 @@ +terraform { + cloud { + organization = "CHANGED" + } +}