Skip to content

Commit 96e50f6

Browse files
liamcervanteliamcervante
authored
stacks: ensure consistent sources between removed blocks (#36915)
1 parent dcff9e4 commit 96e50f6

File tree

6 files changed

+195
-4
lines changed

6 files changed

+195
-4
lines changed

‎internal/stacks/stackconfig/config.go

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,11 @@ type ConfigNode struct {
7676
// Stack is the definition of this node in the stack tree.
7777
Stack *Stack
7878

79+
// Source is the source address of this stack. This is mainly used to
80+
// ensure consistency in places where a stack might be initialised in
81+
// multiple places (like in different source blocks).
82+
Source sourceaddrs.FinalSource
83+
7984
// Children describes all of the embedded stacks nested directly beneath
8085
// this node in the stack tree. The keys match the labels on the "stack"
8186
// blocks in the configuration that [Config.Stack] was built from, and
@@ -150,6 +155,7 @@ func loadConfigDir(sourceAddr sourceaddrs.FinalSource, sources *sourcebundle.Bun
150155

151156
ret := &ConfigNode{
152157
Stack: stack,
158+
Source: sourceAddr,
153159
Children: make(map[string]*ConfigNode),
154160
}
155161
for _, call := range stack.EmbeddedStacks {
@@ -236,17 +242,29 @@ func loadConfigDir(sourceAddr sourceaddrs.FinalSource, sources *sourcebundle.Bun
236242
Severity: hcl.DiagError,
237243
Summary: "Invalid removed block",
238244
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.",
239-
Subject: block.SourceAddrRange.ToHCL().Ptr(),
245+
Subject: block.DeclRange.ToHCL().Ptr(),
240246
})
241247
break
242248
}
243249
}
244250

245251
if current != nil {
246252
next := target.Item.Name
247-
if _, ok := current.Children[next]; ok {
253+
if childNode, ok := current.Children[next]; ok {
248254
// Then we've already loaded the configuration for this
249-
// stack in the direct stack call.
255+
// stack in the direct stack call or in another removed
256+
// block.
257+
258+
if childNode.Source != block.FinalSourceAddr {
259+
// but apparently the blocks don't agree on what the
260+
// source should be here, so that is an error
261+
diags = diags.Append(&hcl.Diagnostic{
262+
Severity: hcl.DiagError,
263+
Summary: "Invalid source address",
264+
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),
265+
Subject: block.SourceAddrRange.ToHCL().Ptr(),
266+
})
267+
}
250268
continue
251269
}
252270

@@ -281,7 +299,15 @@ func loadConfigDir(sourceAddr sourceaddrs.FinalSource, sources *sourcebundle.Bun
281299
cmpn.FinalSourceAddr = effectiveSourceAddr
282300
}
283301

284-
for _, blocks := range stack.RemovedComponents.All() {
302+
for addr, blocks := range stack.RemovedComponents.All() {
303+
304+
var source sourceaddrs.FinalSource
305+
if len(addr.Stack) == 0 {
306+
if cmpn, ok := stack.Components[addr.Item.Name]; ok {
307+
source = cmpn.FinalSourceAddr
308+
}
309+
}
310+
285311
for _, rmvd := range blocks {
286312
effectiveSourceAddr, err := resolveFinalSourceAddr(sourceAddr, rmvd.SourceAddr, rmvd.VersionConstraints, sources)
287313
if err != nil {
@@ -297,6 +323,17 @@ func loadConfigDir(sourceAddr sourceaddrs.FinalSource, sources *sourcebundle.Bun
297323
continue
298324
}
299325

326+
if source == nil {
327+
source = effectiveSourceAddr
328+
} else if source != effectiveSourceAddr {
329+
diags = diags.Append(&hcl.Diagnostic{
330+
Severity: hcl.DiagError,
331+
Summary: "Invalid source address",
332+
Detail: fmt.Sprintf("Cannot use %q as a source address here: the target stack is already initialised with another source %q.", effectiveSourceAddr, source),
333+
Subject: rmvd.SourceAddrRange.ToHCL().Ptr(),
334+
})
335+
}
336+
300337
rmvd.FinalSourceAddr = effectiveSourceAddr
301338
}
302339
}

‎internal/stacks/stackconfig/config_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,65 @@ func TestLoadConfigDirErrors(t *testing.T) {
7373
}
7474
}
7575

76+
func TestLoadConfigDirSourceErrors(t *testing.T) {
77+
bundle, err := sourcebundle.OpenDir("testdata/basics-bundle")
78+
if err != nil {
79+
t.Fatal(err)
80+
}
81+
82+
rootAddr := sourceaddrs.MustParseSource("git::https://example.com/errored-sources.git").(sourceaddrs.RemoteSource)
83+
_, gotDiags := LoadConfigDir(rootAddr, bundle)
84+
85+
sort.SliceStable(gotDiags, func(i, j int) bool {
86+
if gotDiags[i].Severity() != gotDiags[j].Severity() {
87+
return gotDiags[i].Severity() < gotDiags[j].Severity()
88+
}
89+
90+
if gotDiags[i].Description().Summary != gotDiags[j].Description().Summary {
91+
return gotDiags[i].Description().Summary < gotDiags[j].Description().Summary
92+
}
93+
94+
return gotDiags[i].Description().Detail < gotDiags[j].Description().Detail
95+
})
96+
97+
wantDiags := tfdiags.Diagnostics{
98+
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."),
99+
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\"."),
100+
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\"."),
101+
}
102+
103+
count := len(wantDiags)
104+
if len(gotDiags) > count {
105+
count = len(gotDiags)
106+
}
107+
108+
for i := 0; i < count; i++ {
109+
if i >= len(wantDiags) {
110+
t.Errorf("unexpected diagnostic:\n%s", gotDiags[i])
111+
continue
112+
}
113+
114+
if i >= len(gotDiags) {
115+
t.Errorf("missing diagnostic:\n%s", wantDiags[i])
116+
continue
117+
}
118+
119+
got, want := gotDiags[i], wantDiags[i]
120+
121+
if got, want := got.Severity(), want.Severity(); got != want {
122+
t.Errorf("diagnostics[%d] severity\ngot: %s\nwant: %s", i, got, want)
123+
}
124+
125+
if got, want := got.Description().Summary, want.Description().Summary; got != want {
126+
t.Errorf("diagnostics[%d] summary\ngot: %s\nwant: %s", i, got, want)
127+
}
128+
129+
if got, want := got.Description().Detail, want.Description().Detail; got != want {
130+
t.Errorf("diagnostics[%d] detail\ngot: %s\nwant: %s", i, got, want)
131+
}
132+
}
133+
}
134+
76135
func TestLoadConfigDirBasics(t *testing.T) {
77136
bundle, err := sourcebundle.OpenDir("testdata/basics-bundle")
78137
if err != nil {

‎internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/main.tf

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
variable "name" {
2+
type = string
3+
}
4+
5+
resource "null_resource" "example" {
6+
triggers = {
7+
name = var.name
8+
}
9+
}
10+
11+
output "greeting" {
12+
value = "Hello, ${var.name}!"
13+
}
14+
15+
output "resource_id" {
16+
value = null_resource.example.id
17+
}

‎internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/main.tfstack.hcl

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
required_providers {
2+
null = {
3+
source = "hashicorp/null"
4+
version = "3.2.1"
5+
}
6+
}
7+
8+
provider "null" "a" {}
9+
10+
removed {
11+
from = stack.a.component.a // bad, stack.a is undefined so this is orphaned
12+
13+
source = "./"
14+
15+
providers = {
16+
null = provider.null.a
17+
}
18+
}
19+
20+
removed {
21+
from = stack.a.stack.b // bad, stack.a is undefined so this is orphaned
22+
source = "./subdir"
23+
}
24+
25+
removed {
26+
from = stack.b["a"]
27+
source = "./subdir"
28+
}
29+
30+
removed {
31+
from = stack.b["b"]
32+
source = "./" // bad, the sources should be the same for stack.b
33+
}
34+
35+
removed {
36+
from = stack.a.component.b["a"]
37+
38+
source = "./"
39+
40+
providers = {
41+
null = provider.null.a
42+
}
43+
}
44+
45+
removed {
46+
from = stack.a.component.b["b"] // bad, the sources should be the same for component.b
47+
48+
source = "./subdir"
49+
50+
providers = {
51+
null = provider.null.a
52+
}
53+
}

‎internal/stacks/stackconfig/testdata/basics-bundle/errored-sources/subdir/main.tfstack.hcl

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
required_providers {
2+
null = {
3+
source = "hashicorp/null"
4+
version = "3.2.1"
5+
}
6+
}
7+
8+
provider "null" "a" {}
9+
10+
component "a" {
11+
source = "../"
12+
13+
inputs = {
14+
name = var.name
15+
}
16+
17+
providers = {
18+
null = provider.null.a
19+
}
20+
}

‎internal/stacks/stackconfig/testdata/basics-bundle/terraform-sources.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@
1515
"source": "git::https://example.com/errored.git",
1616
"local": "errored",
1717
"meta": {}
18+
},
19+
{
20+
"source": "git::https://example.com/errored-sources.git",
21+
"local": "errored-sources",
22+
"meta": {}
1823
}
1924
],
2025
"registry": [

0 commit comments

Comments
 (0)