From 6f07b460a78e7fe6f7093ded3b48c8101bb441fe Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Thu, 29 May 2025 14:09:41 +0100 Subject: [PATCH] Update and add new tests for module-parsing logic (#37176) * Add comments, rename tests, reorder tests This commit doesn't change or add any logic * Add test for overriding a cloud block with a backend block * Add tests showing multiple instances of cloud or backend blocks cause an error * Add test showing that backend and cloud blocks cannot be in the same terraform block * Add test case asserting it's not valid to have both cloud and backend blocks across files in a module * Clarify comments and replace some comments with test names * Replace comments with test names --- internal/configs/module_test.go | 237 +++++++++++++----- .../backend.tf | 3 + .../cloud.tf | 8 + .../conflict-cloud-backend/main.tf | 10 + .../invalid-modules/multiple-backends/a.tf | 3 + .../invalid-modules/multiple-backends/b.tf | 3 + .../invalid-modules/multiple-cloud/a.tf | 5 + .../invalid-modules/multiple-cloud/b.tf | 5 + .../override-cloud-with-backend/main.tf | 13 + .../override-cloud-with-backend/override.tf | 5 + 10 files changed, 223 insertions(+), 69 deletions(-) create mode 100644 internal/configs/testdata/invalid-modules/conflict-cloud-backend-separate-files/backend.tf create mode 100644 internal/configs/testdata/invalid-modules/conflict-cloud-backend-separate-files/cloud.tf create mode 100644 internal/configs/testdata/invalid-modules/conflict-cloud-backend/main.tf create mode 100644 internal/configs/testdata/invalid-modules/multiple-backends/a.tf create mode 100644 internal/configs/testdata/invalid-modules/multiple-backends/b.tf create mode 100644 internal/configs/testdata/invalid-modules/multiple-cloud/a.tf create mode 100644 internal/configs/testdata/invalid-modules/multiple-cloud/b.tf create mode 100644 internal/configs/testdata/valid-modules/override-cloud-with-backend/main.tf create mode 100644 internal/configs/testdata/valid-modules/override-cloud-with-backend/override.tf diff --git a/internal/configs/module_test.go b/internal/configs/module_test.go index 13e75f6ffe..6f47afbab8 100644 --- a/internal/configs/module_test.go +++ b/internal/configs/module_test.go @@ -315,104 +315,203 @@ 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()) - } +func TestModule_backend_overrides_a_backend(t *testing.T) { + t.Run("it can override a backend block with a different backend block", func(t *testing.T) { + mod, diags := testModuleFromDir("testdata/valid-modules/override-backend") + if diags.HasErrors() { + t.Fatal(diags.Error()) + } - gotType := mod.Backend.Type - wantType := "bar" + gotType := mod.Backend.Type + wantType := "bar" - if gotType != wantType { - t.Errorf("wrong result for backend type: got %#v, want %#v\n", gotType, wantType) - } + if gotType != wantType { + t.Errorf("wrong result for backend type: got %#v, want %#v\n", gotType, wantType) + } - attrs, _ := mod.Backend.Config.JustAttributes() + attrs, _ := mod.Backend.Config.JustAttributes() - gotAttr, diags := attrs["path"].Expr.Value(nil) - if diags.HasErrors() { - t.Fatal(diags.Error()) - } + gotAttr, diags := attrs["path"].Expr.Value(nil) + if diags.HasErrors() { + t.Fatal(diags.Error()) + } - wantAttr := cty.StringVal("CHANGED/relative/path/to/terraform.tfstate") + 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) - } + 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()) - } +func TestModule_backend_overrides_no_base(t *testing.T) { + t.Run("it can introduce a backend block via overrides when the base config has has no cloud or backend blocks", func(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") - } + 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()) - } +func TestModule_cloud_overrides_a_backend(t *testing.T) { + t.Run("it can override a backend block with a cloud block", func(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.Backend != nil { + t.Errorf("expected module Backend to be nil") + } - if mod.CloudConfig == nil { - t.Errorf("expected module CloudConfig not to be nil") - } + if mod.CloudConfig == nil { + t.Errorf("expected module CloudConfig not to be nil") + } + }) +} + +func TestModule_cloud_overrides_cloud(t *testing.T) { + t.Run("it can override a cloud block with a different cloud block", func(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") + } + }) } // 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()) - } +func TestModule_cloud_overrides_no_base(t *testing.T) { + t.Run("it can introduce a cloud block via overrides when the base config has no cloud or backend blocks", func(t *testing.T) { - if mod.CloudConfig == nil { - t.Errorf("expected module CloudConfig not to be nil") - } + 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()) - } +func TestModule_backend_overrides_cloud(t *testing.T) { + t.Run("it can override a cloud block with a backend block", func(t *testing.T) { + mod, diags := testModuleFromDir("testdata/valid-modules/override-cloud-with-backend") + if diags.HasErrors() { + t.Fatal(diags.Error()) + } - attrs, _ := mod.CloudConfig.Config.JustAttributes() + gotType := mod.Backend.Type + wantType := "override" - gotAttr, diags := attrs["organization"].Expr.Value(nil) - if diags.HasErrors() { - t.Fatal(diags.Error()) - } + if gotType != wantType { + t.Errorf("wrong result for backend type: got %#v, want %#v\n", gotType, wantType) + } - wantAttr := cty.StringVal("CHANGED") + attrs, _ := mod.Backend.Config.JustAttributes() - if !gotAttr.RawEquals(wantAttr) { - t.Errorf("wrong result for Cloud 'organization': got %#v, want %#v\n", gotAttr, wantAttr) - } + gotAttr, diags := attrs["path"].Expr.Value(nil) + if diags.HasErrors() { + t.Fatal(diags.Error()) + } - // 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") - } + wantAttr := cty.StringVal("value from override") + + if !gotAttr.RawEquals(wantAttr) { + t.Errorf("wrong result for backend 'path': got %#v, want %#v\n", gotAttr, wantAttr) + } + }) } func TestModule_cloud_duplicate_overrides(t *testing.T) { - _, diags := testModuleFromDir("testdata/invalid-modules/override-cloud-duplicates") - want := `Duplicate HCP Terraform configurations` - if got := diags.Error(); !strings.Contains(got, want) { - t.Fatalf("expected module error to contain %q\nerror was:\n%s", want, got) - } + t.Run("it raises an error when a override file contains multiple cloud blocks", func(t *testing.T) { + _, diags := testModuleFromDir("testdata/invalid-modules/override-cloud-duplicates") + want := `Duplicate HCP Terraform configurations` + if got := diags.Error(); !strings.Contains(got, want) { + t.Fatalf("expected module error to contain %q\nerror was:\n%s", want, got) + } + }) +} + +func TestModule_backend_multiple(t *testing.T) { + t.Run("it detects when two backend blocks are present within the same module in separate files", func(t *testing.T) { + _, diags := testModuleFromDir("testdata/invalid-modules/multiple-backends") + if !diags.HasErrors() { + t.Fatal("module should have error diags, but does not") + } + + want := `Duplicate backend configuration` + if got := diags.Error(); !strings.Contains(got, want) { + t.Fatalf("expected error to contain %q\nerror was:\n%s", want, got) + } + }) +} + +func TestModule_cloud_multiple(t *testing.T) { + t.Run("it detects when two cloud blocks are present within the same module in separate files", func(t *testing.T) { + + _, diags := testModuleFromDir("testdata/invalid-modules/multiple-cloud") + if !diags.HasErrors() { + t.Fatal("module should have error diags, but does not") + } + + want := `Duplicate HCP Terraform configurations` + if got := diags.Error(); !strings.Contains(got, want) { + t.Fatalf("expected error to contain %q\nerror was:\n%s", want, got) + } + }) +} + +// Cannot combine use of backend, cloud blocks. +func TestModule_conflicting_backend_cloud(t *testing.T) { + + t.Run("it detects when both cloud and backend blocks are in the same terraform block", func(t *testing.T) { + _, diags := testModuleFromDir("testdata/invalid-modules/conflict-cloud-backend") + if !diags.HasErrors() { + t.Fatal("module should have error diags, but does not") + } + + want := `Both a backend and cloud configuration are present` + if got := diags.Error(); !strings.Contains(got, want) { + t.Fatalf("expected error to contain %q\nerror was:\n%s", want, got) + } + }) + + t.Run("it detects when both cloud and backend blocks are present within the same module in separate files", func(t *testing.T) { + _, diags := testModuleFromDir("testdata/invalid-modules/conflict-cloud-backend-separate-files") + if !diags.HasErrors() { + t.Fatal("module should have error diags, but does not") + } + + want := `Both a backend and cloud configuration are present` + if got := diags.Error(); !strings.Contains(got, want) { + t.Fatalf("expected error to contain %q\nerror was:\n%s", want, got) + } + }) } diff --git a/internal/configs/testdata/invalid-modules/conflict-cloud-backend-separate-files/backend.tf b/internal/configs/testdata/invalid-modules/conflict-cloud-backend-separate-files/backend.tf new file mode 100644 index 0000000000..bee4ccf4a1 --- /dev/null +++ b/internal/configs/testdata/invalid-modules/conflict-cloud-backend-separate-files/backend.tf @@ -0,0 +1,3 @@ +terraform { + backend "foo" {} +} diff --git a/internal/configs/testdata/invalid-modules/conflict-cloud-backend-separate-files/cloud.tf b/internal/configs/testdata/invalid-modules/conflict-cloud-backend-separate-files/cloud.tf new file mode 100644 index 0000000000..065115d8df --- /dev/null +++ b/internal/configs/testdata/invalid-modules/conflict-cloud-backend-separate-files/cloud.tf @@ -0,0 +1,8 @@ +terraform { + cloud { + organization = "sarahfrench" + workspaces { + name = "test-cloud-backend" + } + } +} diff --git a/internal/configs/testdata/invalid-modules/conflict-cloud-backend/main.tf b/internal/configs/testdata/invalid-modules/conflict-cloud-backend/main.tf new file mode 100644 index 0000000000..a8f45377da --- /dev/null +++ b/internal/configs/testdata/invalid-modules/conflict-cloud-backend/main.tf @@ -0,0 +1,10 @@ +terraform { + backend "foo" {} + + cloud { + organization = "sarahfrench" + workspaces { + name = "test-cloud-backend" + } + } +} diff --git a/internal/configs/testdata/invalid-modules/multiple-backends/a.tf b/internal/configs/testdata/invalid-modules/multiple-backends/a.tf new file mode 100644 index 0000000000..bee4ccf4a1 --- /dev/null +++ b/internal/configs/testdata/invalid-modules/multiple-backends/a.tf @@ -0,0 +1,3 @@ +terraform { + backend "foo" {} +} diff --git a/internal/configs/testdata/invalid-modules/multiple-backends/b.tf b/internal/configs/testdata/invalid-modules/multiple-backends/b.tf new file mode 100644 index 0000000000..9faa4f475b --- /dev/null +++ b/internal/configs/testdata/invalid-modules/multiple-backends/b.tf @@ -0,0 +1,3 @@ +terraform { + backend "bar" {} +} diff --git a/internal/configs/testdata/invalid-modules/multiple-cloud/a.tf b/internal/configs/testdata/invalid-modules/multiple-cloud/a.tf new file mode 100644 index 0000000000..51ae925fb9 --- /dev/null +++ b/internal/configs/testdata/invalid-modules/multiple-cloud/a.tf @@ -0,0 +1,5 @@ +terraform { + cloud { + organization = "foo" + } +} diff --git a/internal/configs/testdata/invalid-modules/multiple-cloud/b.tf b/internal/configs/testdata/invalid-modules/multiple-cloud/b.tf new file mode 100644 index 0000000000..fd5e977a69 --- /dev/null +++ b/internal/configs/testdata/invalid-modules/multiple-cloud/b.tf @@ -0,0 +1,5 @@ +terraform { + cloud { + organization = "bar" + } +} diff --git a/internal/configs/testdata/valid-modules/override-cloud-with-backend/main.tf b/internal/configs/testdata/valid-modules/override-cloud-with-backend/main.tf new file mode 100644 index 0000000000..a96740450f --- /dev/null +++ b/internal/configs/testdata/valid-modules/override-cloud-with-backend/main.tf @@ -0,0 +1,13 @@ +terraform { + cloud { + organization = "foo" + } +} + +resource "aws_instance" "web" { + ami = "ami-1234" + security_groups = [ + "foo", + "bar", + ] +} diff --git a/internal/configs/testdata/valid-modules/override-cloud-with-backend/override.tf b/internal/configs/testdata/valid-modules/override-cloud-with-backend/override.tf new file mode 100644 index 0000000000..e603cf89ad --- /dev/null +++ b/internal/configs/testdata/valid-modules/override-cloud-with-backend/override.tf @@ -0,0 +1,5 @@ +terraform { + backend "override" { + path = "value from override" + } +}