From 96e50f680f3db7dff1657e5ffb77242584549af7 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Thu, 24 Apr 2025 10:24:20 +0200 Subject: [PATCH] stacks: ensure consistent sources between removed blocks (#36915) --- internal/stacks/stackconfig/config.go | 45 ++++++++++++-- internal/stacks/stackconfig/config_test.go | 59 +++++++++++++++++++ .../basics-bundle/errored-sources/main.tf | 17 ++++++ .../errored-sources/main.tfstack.hcl | 53 +++++++++++++++++ .../errored-sources/subdir/main.tfstack.hcl | 20 +++++++ .../basics-bundle/terraform-sources.json | 5 ++ 6 files changed, 195 insertions(+), 4 deletions(-) create mode 100644 internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/main.tf create mode 100644 internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/main.tfstack.hcl create mode 100644 internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/subdir/main.tfstack.hcl diff --git a/internal/stacks/stackconfig/config.go b/internal/stacks/stackconfig/config.go index 2c90597645..883b52f40e 100644 --- a/internal/stacks/stackconfig/config.go +++ b/internal/stacks/stackconfig/config.go @@ -76,6 +76,11 @@ type ConfigNode struct { // Stack is the definition of this node in the stack tree. Stack *Stack + // Source is the source address of this stack. This is mainly used to + // ensure consistency in places where a stack might be initialised in + // multiple places (like in different source blocks). + Source sourceaddrs.FinalSource + // Children describes all of the embedded stacks nested directly beneath // this node in the stack tree. The keys match the labels on the "stack" // blocks in the configuration that [Config.Stack] was built from, and @@ -150,6 +155,7 @@ func loadConfigDir(sourceAddr sourceaddrs.FinalSource, sources *sourcebundle.Bun ret := &ConfigNode{ Stack: stack, + Source: sourceAddr, Children: make(map[string]*ConfigNode), } for _, call := range stack.EmbeddedStacks { @@ -236,7 +242,7 @@ func loadConfigDir(sourceAddr sourceaddrs.FinalSource, sources *sourcebundle.Bun Severity: hcl.DiagError, Summary: "Invalid removed block", Detail: "The linked removed block was not executed because the `from` attribute of the removed block targets a component or embedded stack within an orphaned embedded stack.\n\nIn order to remove an entire stack, update your removed block to target the entire removed stack itself instead of the specific elements within it.", - Subject: block.SourceAddrRange.ToHCL().Ptr(), + Subject: block.DeclRange.ToHCL().Ptr(), }) break } @@ -244,9 +250,21 @@ func loadConfigDir(sourceAddr sourceaddrs.FinalSource, sources *sourcebundle.Bun if current != nil { next := target.Item.Name - if _, ok := current.Children[next]; ok { + if childNode, ok := current.Children[next]; ok { // Then we've already loaded the configuration for this - // stack in the direct stack call. + // stack in the direct stack call or in another removed + // block. + + if childNode.Source != block.FinalSourceAddr { + // but apparently the blocks don't agree on what the + // source should be here, so that is an error + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid source address", + Detail: fmt.Sprintf("Cannot use %q as a source address here: the target stack is already initialised with another source %q.", block.FinalSourceAddr, childNode.Source), + Subject: block.SourceAddrRange.ToHCL().Ptr(), + }) + } continue } @@ -281,7 +299,15 @@ func loadConfigDir(sourceAddr sourceaddrs.FinalSource, sources *sourcebundle.Bun cmpn.FinalSourceAddr = effectiveSourceAddr } - for _, blocks := range stack.RemovedComponents.All() { + for addr, blocks := range stack.RemovedComponents.All() { + + var source sourceaddrs.FinalSource + if len(addr.Stack) == 0 { + if cmpn, ok := stack.Components[addr.Item.Name]; ok { + source = cmpn.FinalSourceAddr + } + } + for _, rmvd := range blocks { effectiveSourceAddr, err := resolveFinalSourceAddr(sourceAddr, rmvd.SourceAddr, rmvd.VersionConstraints, sources) if err != nil { @@ -297,6 +323,17 @@ func loadConfigDir(sourceAddr sourceaddrs.FinalSource, sources *sourcebundle.Bun continue } + if source == nil { + source = effectiveSourceAddr + } else if source != effectiveSourceAddr { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid source address", + Detail: fmt.Sprintf("Cannot use %q as a source address here: the target stack is already initialised with another source %q.", effectiveSourceAddr, source), + Subject: rmvd.SourceAddrRange.ToHCL().Ptr(), + }) + } + rmvd.FinalSourceAddr = effectiveSourceAddr } } diff --git a/internal/stacks/stackconfig/config_test.go b/internal/stacks/stackconfig/config_test.go index 6f283b38b5..c088b5ee5c 100644 --- a/internal/stacks/stackconfig/config_test.go +++ b/internal/stacks/stackconfig/config_test.go @@ -73,6 +73,65 @@ func TestLoadConfigDirErrors(t *testing.T) { } } +func TestLoadConfigDirSourceErrors(t *testing.T) { + bundle, err := sourcebundle.OpenDir("testdata/basics-bundle") + if err != nil { + t.Fatal(err) + } + + rootAddr := sourceaddrs.MustParseSource("git::https://example.com/errored-sources.git").(sourceaddrs.RemoteSource) + _, gotDiags := LoadConfigDir(rootAddr, bundle) + + sort.SliceStable(gotDiags, func(i, j int) bool { + if gotDiags[i].Severity() != gotDiags[j].Severity() { + return gotDiags[i].Severity() < gotDiags[j].Severity() + } + + if gotDiags[i].Description().Summary != gotDiags[j].Description().Summary { + return gotDiags[i].Description().Summary < gotDiags[j].Description().Summary + } + + return gotDiags[i].Description().Detail < gotDiags[j].Description().Detail + }) + + wantDiags := tfdiags.Diagnostics{ + tfdiags.Sourceless(tfdiags.Error, "Invalid removed block", "The linked removed block was not executed because the `from` attribute of the removed block targets a component or embedded stack within an orphaned embedded stack.\n\nIn order to remove an entire stack, update your removed block to target the entire removed stack itself instead of the specific elements within it."), + tfdiags.Sourceless(tfdiags.Error, "Invalid source address", "Cannot use \"git::https://example.com/errored-sources.git\" as a source address here: the target stack is already initialised with another source \"git::https://example.com/errored-sources.git//subdir\"."), + tfdiags.Sourceless(tfdiags.Error, "Invalid source address", "Cannot use \"git::https://example.com/errored-sources.git//subdir\" as a source address here: the target stack is already initialised with another source \"git::https://example.com/errored-sources.git\"."), + } + + count := len(wantDiags) + if len(gotDiags) > count { + count = len(gotDiags) + } + + for i := 0; i < count; i++ { + if i >= len(wantDiags) { + t.Errorf("unexpected diagnostic:\n%s", gotDiags[i]) + continue + } + + if i >= len(gotDiags) { + t.Errorf("missing diagnostic:\n%s", wantDiags[i]) + continue + } + + got, want := gotDiags[i], wantDiags[i] + + if got, want := got.Severity(), want.Severity(); got != want { + t.Errorf("diagnostics[%d] severity\ngot: %s\nwant: %s", i, got, want) + } + + if got, want := got.Description().Summary, want.Description().Summary; got != want { + t.Errorf("diagnostics[%d] summary\ngot: %s\nwant: %s", i, got, want) + } + + if got, want := got.Description().Detail, want.Description().Detail; got != want { + t.Errorf("diagnostics[%d] detail\ngot: %s\nwant: %s", i, got, want) + } + } +} + func TestLoadConfigDirBasics(t *testing.T) { bundle, err := sourcebundle.OpenDir("testdata/basics-bundle") if err != nil { diff --git a/internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/main.tf b/internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/main.tf new file mode 100644 index 0000000000..5ab385503e --- /dev/null +++ b/internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/main.tf @@ -0,0 +1,17 @@ +variable "name" { + type = string +} + +resource "null_resource" "example" { + triggers = { + name = var.name + } +} + +output "greeting" { + value = "Hello, ${var.name}!" +} + +output "resource_id" { + value = null_resource.example.id +} diff --git a/internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/main.tfstack.hcl b/internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/main.tfstack.hcl new file mode 100644 index 0000000000..960c36e29f --- /dev/null +++ b/internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/main.tfstack.hcl @@ -0,0 +1,53 @@ +required_providers { + null = { + source = "hashicorp/null" + version = "3.2.1" + } +} + +provider "null" "a" {} + +removed { + from = stack.a.component.a // bad, stack.a is undefined so this is orphaned + + source = "./" + + providers = { + null = provider.null.a + } +} + +removed { + from = stack.a.stack.b // bad, stack.a is undefined so this is orphaned + source = "./subdir" +} + +removed { + from = stack.b["a"] + source = "./subdir" +} + +removed { + from = stack.b["b"] + source = "./" // bad, the sources should be the same for stack.b +} + +removed { + from = stack.a.component.b["a"] + + source = "./" + + providers = { + null = provider.null.a + } +} + +removed { + from = stack.a.component.b["b"] // bad, the sources should be the same for component.b + + source = "./subdir" + + providers = { + null = provider.null.a + } +} \ No newline at end of file diff --git a/internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/subdir/main.tfstack.hcl b/internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/subdir/main.tfstack.hcl new file mode 100644 index 0000000000..2ea566a8c4 --- /dev/null +++ b/internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/subdir/main.tfstack.hcl @@ -0,0 +1,20 @@ +required_providers { + null = { + source = "hashicorp/null" + version = "3.2.1" + } +} + +provider "null" "a" {} + +component "a" { + source = "../" + + inputs = { + name = var.name + } + + providers = { + null = provider.null.a + } +} diff --git a/internal/stacks/stackconfig/testdata/basics-bundle/terraform-sources.json b/internal/stacks/stackconfig/testdata/basics-bundle/terraform-sources.json index b40f78defd..16e1f0a660 100644 --- a/internal/stacks/stackconfig/testdata/basics-bundle/terraform-sources.json +++ b/internal/stacks/stackconfig/testdata/basics-bundle/terraform-sources.json @@ -15,6 +15,11 @@ "source": "git::https://example.com/errored.git", "local": "errored", "meta": {} + }, + { + "source": "git::https://example.com/errored-sources.git", + "local": "errored-sources", + "meta": {} } ], "registry": [