Skip to content

Commit ae8ed13

Browse files
committed
chore: simplified auto include errors
1 parent e08b089 commit ae8ed13

10 files changed

Lines changed: 137 additions & 28 deletions

File tree

internal/discovery/helpers.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,12 @@ func stackDependencyPaths(fs vfs.FS, depPaths []string, c component.Component) (
290290
expanded := make([]string, 0, len(depPaths))
291291

292292
for _, depPath := range depPaths {
293+
// Only expand directories: dependency paths can also point at non-default-named config files (e.g. another-name.hcl), which the stack-file parser would otherwise reject as "not a directory".
294+
if info, statErr := os.Stat(depPath); statErr != nil || !info.IsDir() {
295+
expanded = append(expanded, depPath)
296+
continue
297+
}
298+
293299
unitPaths, err := inthclparse.UnitPathsFromStackDir(fs, depPath)
294300
if err != nil {
295301
return nil, NewStackDependencyExpansionError(depPath, err)
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package discovery_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/gruntwork-io/terragrunt/internal/discovery"
7+
"github.com/gruntwork-io/terragrunt/internal/hclparse"
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
// Direct typed-error contract test: StackDependencyExpansionError must carry the depPath and unwrap cleanly to the original parser error.
13+
func TestStackDependencyExpansionError_Unwrap(t *testing.T) {
14+
t.Parallel()
15+
16+
innerErr := hclparse.MalformedDependencyError{
17+
FilePath: "/some/path/terragrunt.autoinclude.hcl",
18+
Name: "vpc",
19+
Reason: "missing config_path attribute",
20+
}
21+
22+
wrapped := discovery.NewStackDependencyExpansionError("/path/to/dep", innerErr)
23+
require.Error(t, wrapped)
24+
assert.Contains(t, wrapped.Error(), "/path/to/dep")
25+
assert.Contains(t, wrapped.Error(), "missing config_path")
26+
27+
// errors.As must reach both the wrapper and the underlying typed error.
28+
var expansion discovery.StackDependencyExpansionError
29+
require.ErrorAs(t, wrapped, &expansion)
30+
assert.Equal(t, "/path/to/dep", expansion.DepPath)
31+
32+
var malformed hclparse.MalformedDependencyError
33+
require.ErrorAs(t, wrapped, &malformed)
34+
assert.Equal(t, "vpc", malformed.Name)
35+
36+
// errors.Is must reach the leaf via Unwrap chain.
37+
require.ErrorIs(t, wrapped, innerErr)
38+
}

internal/hclparse/autoinclude.go

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,11 @@ func AutoIncludeDependencyPaths(fs vfs.FS, unitDir string) ([]string, error) {
234234
paths = append(paths, depPath)
235235
}
236236

237-
// Return whatever paths we discovered alongside any errors so callers can decide whether partial DAG enrichment is useful.
238-
return paths, errors.Join(errs...)
237+
if len(errs) > 0 {
238+
return nil, errors.Join(errs...)
239+
}
240+
241+
return paths, nil
239242
}
240243

241244
// readAutoIncludeBody reads and parses an autoinclude file, returning (nil, nil) when the file does not exist.
@@ -262,10 +265,10 @@ func readAutoIncludeBody(fs vfs.FS, autoIncludePath string) (*hclsyntax.Body, er
262265
return body, nil
263266
}
264267

265-
// blockLabelsString joins a block's labels for error messages; returns "<unlabeled>" when there are none.
268+
// blockLabelsString joins a block's labels for error messages; returns "(unlabeled)" when there are none.
266269
func blockLabelsString(block *hclsyntax.Block) string {
267270
if len(block.Labels) == 0 {
268-
return "<unlabeled>"
271+
return "(unlabeled)"
269272
}
270273

271274
return strings.Join(block.Labels, " ")
@@ -282,11 +285,19 @@ func extractDepPath(block *hclsyntax.Block, autoIncludePath, unitDir string) (st
282285

283286
val, valDiags := configPathAttr.Expr.Value(nil)
284287
if valDiags.HasErrors() {
285-
return "", MalformedDependencyError{FilePath: autoIncludePath, Name: name, Reason: "config_path: " + valDiags.Error(), Wrapped: valDiags}
288+
return "", MalformedDependencyError{FilePath: autoIncludePath, Name: name, Reason: "config_path: " + valDiags.Error(), Err: valDiags}
289+
}
290+
291+
if !val.IsKnown() {
292+
return "", MalformedDependencyError{FilePath: autoIncludePath, Name: name, Reason: "config_path is unknown"}
286293
}
287294

288-
if !val.IsKnown() || val.IsNull() || val.Type() != cty.String {
289-
return "", MalformedDependencyError{FilePath: autoIncludePath, Name: name, Reason: "config_path must be a known string literal"}
295+
if val.IsNull() {
296+
return "", MalformedDependencyError{FilePath: autoIncludePath, Name: name, Reason: "config_path is null"}
297+
}
298+
299+
if val.Type() != cty.String {
300+
return "", MalformedDependencyError{FilePath: autoIncludePath, Name: name, Reason: "config_path must be a string, got " + val.Type().FriendlyName()}
290301
}
291302

292303
depPath := val.AsString()

internal/hclparse/autoinclude_test.go

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,42 @@ dependency "vpc" {
6161
assert.NotNil(t, result.RawBody)
6262
}
6363

64+
// dependency block with zero labels must be reported as a diagnostic (not silently skipped) so users learn the labelling convention.
65+
func TestAutoIncludeHCL_Resolve_RejectsZeroLabels(t *testing.T) {
66+
t.Parallel()
67+
68+
body := parseHCLBody(t, `
69+
dependency {
70+
config_path = "../vpc"
71+
}
72+
`)
73+
74+
autoInclude := &hclparse.AutoIncludeHCL{Remain: body}
75+
76+
result, diags := autoInclude.Resolve(&hcl.EvalContext{Variables: map[string]cty.Value{}})
77+
assert.Nil(t, result)
78+
require.True(t, diags.HasErrors())
79+
assert.Contains(t, diags.Error(), "exactly one label, got 0")
80+
}
81+
82+
// dependency block with two or more labels likewise produces a diagnostic instead of silently using only the first label.
83+
func TestAutoIncludeHCL_Resolve_RejectsTwoLabels(t *testing.T) {
84+
t.Parallel()
85+
86+
body := parseHCLBody(t, `
87+
dependency "vpc" "extra" {
88+
config_path = "../vpc"
89+
}
90+
`)
91+
92+
autoInclude := &hclparse.AutoIncludeHCL{Remain: body}
93+
94+
result, diags := autoInclude.Resolve(&hcl.EvalContext{Variables: map[string]cty.Value{}})
95+
assert.Nil(t, result)
96+
require.True(t, diags.HasErrors())
97+
assert.Contains(t, diags.Error(), "exactly one label, got 2")
98+
}
99+
64100
func TestAutoIncludeHCL_Resolve_MultipleDependencies(t *testing.T) {
65101
t.Parallel()
66102

@@ -303,7 +339,7 @@ func TestAutoIncludeDependencyPaths_MalformedReturnsTypedError(t *testing.T) {
303339
name: "non-string config_path",
304340
content: `dependency "x" { config_path = 42 }`,
305341
wantDepName: "x",
306-
wantReasonPart: "config_path must be a known string literal",
342+
wantReasonPart: "config_path must be a string, got number",
307343
},
308344
{
309345
name: "unevaluable config_path",
@@ -314,7 +350,7 @@ func TestAutoIncludeDependencyPaths_MalformedReturnsTypedError(t *testing.T) {
314350
{
315351
name: "no labels",
316352
content: `dependency { config_path = "../vpc" }`,
317-
wantDepName: "<unlabeled>",
353+
wantDepName: "(unlabeled)",
318354
wantReasonPart: "exactly one label, got 0",
319355
},
320356
{
@@ -334,7 +370,7 @@ func TestAutoIncludeDependencyPaths_MalformedReturnsTypedError(t *testing.T) {
334370

335371
paths, err := hclparse.AutoIncludeDependencyPaths(fs, "/test")
336372
require.Error(t, err)
337-
assert.Empty(t, paths, "no valid dependency paths in this fixture")
373+
assert.Nil(t, paths)
338374

339375
var malformedErr hclparse.MalformedDependencyError
340376
require.ErrorAs(t, err, &malformedErr)
@@ -345,6 +381,28 @@ func TestAutoIncludeDependencyPaths_MalformedReturnsTypedError(t *testing.T) {
345381
}
346382
}
347383

384+
// Mixed-case fixture: one valid dependency + one malformed. Strict contract: producer returns (nil, err) on any malformed block — no partial paths.
385+
func TestAutoIncludeDependencyPaths_MixedValidAndMalformed(t *testing.T) {
386+
t.Parallel()
387+
388+
fs := vfs.NewMemMapFS()
389+
require.NoError(t, vfs.WriteFile(fs, filepath.Join("/test", hclparse.AutoIncludeFile), []byte(`
390+
dependency "vpc" {
391+
config_path = "../vpc"
392+
}
393+
394+
dependency "broken" {}
395+
`), 0644))
396+
397+
paths, err := hclparse.AutoIncludeDependencyPaths(fs, "/test")
398+
require.Error(t, err)
399+
assert.Nil(t, paths, "strict fail-fast: no partial paths when any block is malformed")
400+
401+
var malformedErr hclparse.MalformedDependencyError
402+
require.ErrorAs(t, err, &malformedErr)
403+
assert.Equal(t, "broken", malformedErr.Name)
404+
}
405+
348406
// parseHCLBody is a test helper that parses an HCL string and returns the body.
349407
func parseHCLBody(t *testing.T, src string) hcl.Body {
350408
t.Helper()

internal/hclparse/errors.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,9 @@ func (e LocalsMaxIterError) Error() string {
134134
return fmt.Sprintf("locals evaluation exceeded %d iterations with %d unresolved locals", e.MaxIterations, e.Remaining)
135135
}
136136

137-
// MalformedDependencyError indicates a dependency block in an autoinclude file is malformed. Wrapped optionally carries the original HCL diagnostics so callers can extract position info via errors.As/Is.
137+
// MalformedDependencyError indicates a dependency block in an autoinclude file is malformed. Err optionally carries the original HCL diagnostics so callers can extract position info via errors.As/Is.
138138
type MalformedDependencyError struct {
139-
Wrapped error
139+
Err error
140140
FilePath string
141141
Name string
142142
Reason string
@@ -147,5 +147,5 @@ func (e MalformedDependencyError) Error() string {
147147
}
148148

149149
func (e MalformedDependencyError) Unwrap() error {
150-
return e.Wrapped
150+
return e.Err
151151
}

internal/hclparse/parse.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,9 @@ func mergeOneInclude(fs vfs.FS, stackFile *StackFileHCL, inc *StackIncludeHCL, s
334334
return nil
335335
}
336336

337-
// recordAutoIncludeSources adds entries to srcByAutoInclude mapping each AutoInclude pointer in stackFile to src. Called once for the root stack file and once per included file so each block knows which file it was parsed from.
337+
// recordAutoIncludeSources maps each AutoInclude pointer in stackFile to the bytes of the file it was parsed from.
338+
//
339+
// Pointer-keying invariant: every *AutoIncludeHCL in the slice trees must be a unique allocation that is never copied into a value receiver, since the lookup at resolution time uses pointer identity. gohcl.DecodeBody satisfies this because it allocates a fresh struct for each block; only direct construction by callers would risk duplicate keys.
338340
func recordAutoIncludeSources(srcByAutoInclude map[*AutoIncludeHCL][]byte, stackFile *StackFileHCL, src []byte) {
339341
for _, u := range stackFile.Units {
340342
if u != nil && u.AutoInclude != nil {

internal/hclparse/stack.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package hclparse
33
import (
44
iofs "io/fs"
55
"path/filepath"
6-
"syscall"
76

87
"github.com/gruntwork-io/terragrunt/internal/errors"
98
"github.com/gruntwork-io/terragrunt/internal/util"
@@ -154,14 +153,14 @@ func ExtractStackRefs(stacks []*StackBlockHCL) []ComponentRef {
154153
}
155154

156155
// ParseStackFileFromPath reads stackDir/terragrunt.stack.hcl from disk and performs a two-pass parse.
157-
// Returns (nil, nil) when no stack file is reachable: file missing, or stackDir is itself a regular file (ENOTDIR).
156+
// Returns (nil, nil) only when the stack file does not exist. Callers that may pass non-directory paths must filter those before calling.
158157
func ParseStackFileFromPath(fs vfs.FS, stackDir string) (*ParseResult, error) {
159158
stackDir = util.ResolvePath(stackDir)
160159
stackFile := filepath.Join(stackDir, "terragrunt.stack.hcl")
161160

162161
data, err := vfs.ReadFile(fs, stackFile)
163162
if err != nil {
164-
if errors.Is(err, iofs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
163+
if errors.Is(err, iofs.ErrNotExist) {
165164
return nil, nil
166165
}
167166

internal/hclparse/stack_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -339,16 +339,16 @@ func TestParseStackFileFromPath_NoFile(t *testing.T) {
339339
assert.Nil(t, result)
340340
}
341341

342-
// Discovery callers may pass an arbitrary dependency path that is itself a regular file (e.g. another-name.hcl); ENOTDIR must be treated like a missing stack file, not a hard error.
343-
func TestParseStackFileFromPath_StackDirIsFile(t *testing.T) {
342+
// ParseStackFileFromPath is strict: passing a regular file produces an error. Callers that may receive non-directory paths (e.g. discovery) must filter them upstream.
343+
func TestParseStackFileFromPath_StackDirIsFileReturnsError(t *testing.T) {
344344
t.Parallel()
345345

346346
tmpDir := t.TempDir()
347347
filePath := filepath.Join(tmpDir, "another-name.hcl")
348348
require.NoError(t, os.WriteFile(filePath, []byte(`# regular file, not a directory`), 0644))
349349

350350
result, err := hclparse.ParseStackFileFromPath(vfs.NewOSFS(), filePath)
351-
require.NoError(t, err)
351+
require.Error(t, err)
352352
assert.Nil(t, result)
353353
}
354354

pkg/config/stack.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -396,13 +396,7 @@ func generateAutoInclude(l log.Logger, opts *generateOpts, cmp *componentToGener
396396

397397
l.Infof("Generating %s for %s %s", inthclparse.AutoIncludeFile, kindStr, cmp.name)
398398

399-
// Use the resolved autoinclude's own source bytes; falling back to root stack bytes when unset preserves prior behavior for non-include flows.
400-
srcBytes := resolved.SourceBytes
401-
if srcBytes == nil {
402-
srcBytes = opts.stackSrcBytes
403-
}
404-
405-
if err := inthclparse.GenerateAutoIncludeFile(vfs.NewOSFS(), resolved, dest, srcBytes, resolved.EvalCtx); err != nil {
399+
if err := inthclparse.GenerateAutoIncludeFile(vfs.NewOSFS(), resolved, dest, resolved.SourceBytes, resolved.EvalCtx); err != nil {
406400
return errors.Errorf("failed to write autoinclude for %s %s: %w", kindStr, cmp.name, err)
407401
}
408402

pkg/config/stack_autoinclude_internal_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"github.com/hashicorp/hcl/v2"
77
"github.com/hashicorp/hcl/v2/hclsyntax"
88
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
910
)
1011

1112
func TestStackConfigHasAutoInclude(t *testing.T) {
@@ -69,7 +70,7 @@ func parseSyntaxBody(t *testing.T, src string) hcl.Body {
6970
t.Helper()
7071

7172
file, diags := hclsyntax.ParseConfig([]byte(src), "test.hcl", hcl.Pos{Line: 1, Column: 1})
72-
assert.False(t, diags.HasErrors(), "parse: %s", diags)
73+
require.False(t, diags.HasErrors(), "parse: %s", diags)
7374

7475
return file.Body
7576
}

0 commit comments

Comments
 (0)