Skip to content

Commit 4fae5a7

Browse files
committed
chore: Adding defensive tests
1 parent b459367 commit 4fae5a7

3 files changed

Lines changed: 116 additions & 0 deletions

File tree

internal/getter/filecopy_test.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
package getter_test
2+
3+
import (
4+
"context"
5+
"os"
6+
"path/filepath"
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
12+
tggetter "github.com/gruntwork-io/terragrunt/internal/getter"
13+
"github.com/gruntwork-io/terragrunt/internal/vfs"
14+
getter "github.com/hashicorp/go-getter/v2"
15+
)
16+
17+
// TestFileCopyGetterHandlesSpacesInLocalPaths pins that a local source whose
18+
// path contains spaces still resolves. The URL reaching the getter carries
19+
// that path percent-encoded (`file:///projects/my%20local%20module`), so
20+
// recovering the filesystem path from it is the getter's job, and it has to
21+
// arrive at the same answer go-getter's own FileGetter would.
22+
func TestFileCopyGetterHandlesSpacesInLocalPaths(t *testing.T) {
23+
t.Parallel()
24+
25+
base := t.TempDir()
26+
srcDir := filepath.Join(base, "my local module")
27+
28+
require.NoError(t, os.MkdirAll(srcDir, 0o755))
29+
require.NoError(t, os.WriteFile(filepath.Join(srcDir, "main.tf"), []byte("# hi\n"), 0o644))
30+
31+
client := &getter.Client{
32+
Getters: []getter.Getter{tggetter.NewFileCopyGetter(vfs.NewOSFS())},
33+
}
34+
35+
testCases := []struct {
36+
name string
37+
src string
38+
dst string
39+
want string
40+
mode getter.Mode
41+
}{
42+
{
43+
name: "single file",
44+
src: "file://" + filepath.Join(srcDir, "main.tf"),
45+
dst: filepath.Join(base, "out-file", "main.tf"),
46+
mode: getter.ModeFile,
47+
want: filepath.Join(base, "out-file", "main.tf"),
48+
},
49+
{
50+
name: "directory",
51+
src: "file://" + srcDir,
52+
dst: filepath.Join(base, "out-dir"),
53+
mode: getter.ModeDir,
54+
want: filepath.Join(base, "out-dir", "main.tf"),
55+
},
56+
{
57+
name: "mode decided by the getter",
58+
src: "file://" + srcDir,
59+
dst: filepath.Join(base, "out-any"),
60+
mode: getter.ModeAny,
61+
want: filepath.Join(base, "out-any", "main.tf"),
62+
},
63+
}
64+
65+
for _, tc := range testCases {
66+
t.Run(tc.name, func(t *testing.T) {
67+
t.Parallel()
68+
69+
_, err := client.Get(context.Background(), &getter.Request{
70+
Src: tc.src,
71+
Dst: tc.dst,
72+
GetMode: tc.mode,
73+
})
74+
require.NoError(t, err)
75+
76+
contents, err := os.ReadFile(tc.want)
77+
require.NoError(t, err)
78+
assert.Equal(t, "# hi\n", string(contents))
79+
})
80+
}
81+
}

internal/runner/run/download_source.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ import (
3131
// that bypasses the venv filesystem. See [requireOSFilesystemForSource].
3232
var ErrNonOSFilesystem = errors.New("download requires an OS-backed filesystem")
3333

34+
// ErrNilSource is the panic value [requireOSFilesystemForSource] raises when
35+
// handed a nil source. Sources are built by [tf.NewSource], whose error every
36+
// caller checks first, so a nil arriving here is a programming mistake rather
37+
// than a condition to recover from.
38+
var ErrNilSource = errors.New("terraform source is required but nil")
39+
3440
// ModuleManifestName is the manifest for files copied from terragrunt module folder (i.e., the folder that contains the current terragrunt.hcl).
3541
const (
3642
ModuleManifestName = ".terragrunt-module-manifest"
@@ -165,7 +171,14 @@ func DownloadTerraformSource(
165171
// the filesystem they are handed; every other protocol either shells out
166172
// (git, hg, smb) or writes through os, so on a virtual filesystem it would
167173
// silently touch the real disk.
174+
//
175+
// It panics with [ErrNilSource] on a nil source, so the contract fails where
176+
// it is broken rather than at whichever field is read first.
168177
func requireOSFilesystemForSource(fsys vfs.FS, src *tf.Source) error {
178+
if src == nil {
179+
panic(ErrNilSource)
180+
}
181+
169182
if vfs.IsOSFS(fsys) {
170183
return nil
171184
}

internal/runner/run/download_source_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1558,6 +1558,28 @@ func TestDownloadTerraformSourceRejectsNonOSFilesystemPerSource(t *testing.T) {
15581558
}
15591559
}
15601560

1561+
// TestDownloadTerraformSourceIfNecessaryPanicsOnNilSource pins the contract on
1562+
// the exported helper. Every source is built by tf.NewSource, whose error the
1563+
// caller checks, so a nil one is a mistake in the calling code.
1564+
func TestDownloadTerraformSourceIfNecessaryPanicsOnNilSource(t *testing.T) {
1565+
t.Parallel()
1566+
1567+
opts, err := options.NewTerragruntOptionsForTest("./test")
1568+
require.NoError(t, err)
1569+
1570+
require.PanicsWithValue(t, run.ErrNilSource, func() {
1571+
run.DownloadTerraformSourceIfNecessary(
1572+
t.Context(),
1573+
logger.CreateLogger(),
1574+
venv.OSVenv(),
1575+
nil,
1576+
configbridge.NewRunOptions(opts),
1577+
&runcfg.RunConfig{Terraform: runcfg.TerraformConfig{}},
1578+
report.NewReport(),
1579+
)
1580+
})
1581+
}
1582+
15611583
// TestDownloadTerraformSourceIfNecessaryRejectsNonOSFilesystem pins the gate
15621584
// on the exported helper so external callers cannot bypass it.
15631585
func TestDownloadTerraformSourceIfNecessaryRejectsNonOSFilesystem(t *testing.T) {

0 commit comments

Comments
 (0)