Skip to content

Commit 56ca70a

Browse files
committed
chore: Completing venv abstraction
🤞🏽 Should complete the venv abstraction 🤞🏽. Closes some gaps related to GOOS and GOARCH usage. Also closes direct homedir access. Closes more gaps from direct filesystem and exec usage. Also adds some optimization for listing files in a directory with a particular suffix... Then I added one more optimization in `getTerraformInputNamesFromAutomaticVarFiles`. Addressing `EOF` in input stream. Dropping `iofs` aliases. Using TLS server for engine download tests. As a consequence, we can do a bit more parallelization in tests.
1 parent 6e06adb commit 56ca70a

202 files changed

Lines changed: 1902 additions & 963 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.golangci.yml

Lines changed: 45 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,15 @@ linters:
116116
- pattern: ^os\.(UserHomeDir|UserCacheDir|UserConfigDir)$
117117
pkg: ^os$
118118
msg: use the venv's Platform handles instead of reading the invoking user's directories
119+
- pattern: ^os\.(Getpid|Executable|Hostname)$
120+
pkg: ^os$
121+
msg: use the venv's Platform handles instead of reading the running process
122+
- pattern: ^os\.Args$
123+
pkg: ^os$
124+
msg: take the arguments the CLI was handed rather than reading the process arguments
125+
- pattern: ^runtime\.(GOOS|GOARCH)$
126+
pkg: ^runtime$
127+
msg: take the platform from the venv's Platform.GOOS and Platform.GOARCH instead of the compiled-in constants
119128
- pattern: ^os\.(Stdin|Stdout|Stderr)$
120129
pkg: ^os$
121130
msg: read and write the venv's Reader and Writers, and ask its Terminal whether a stream is a tty
@@ -128,11 +137,6 @@ linters:
128137
- pattern: ^http\.Default(Client|Transport)$
129138
pkg: ^net/http$
130139
msg: send through the venv's vhttp.Client instead of the shared default client
131-
# A composite literal builds a client without calling anything the rules
132-
# above match, so it is the remaining way to reach the network past the
133-
# venv. The two production sites that survive it are the GCS client
134-
# constructors, whose nolint directives cover this rule as well; both
135-
# wrap a transport derived from the venv's own client.
136140
- pattern: ^http\.Client$
137141
pkg: ^net/http$
138142
msg: take the client from the venv's vhttp.Client rather than building one
@@ -147,9 +151,6 @@ linters:
147151
msg: resolve links on the venv's vfs.FS via vfs.EvalSymlinks, or vfs.ResolveForCompare when the result is to be compared against another path
148152
- pattern: ^(vfs\.NewOSFS|vexec\.NewOSExec|vhttp\.NewOSClient|vsops\.NewOSDecrypter)$
149153
msg: take the handle from the venv already in scope rather than building a fresh OS-backed one
150-
# Cloud SDKs build their own transport when handed no client, so these
151-
# entry points leak past the venv without any net/http call of our own
152-
# for the rules above to catch.
153154
- pattern: ^config\.LoadDefaultConfig$
154155
pkg: ^github.com/aws/aws-sdk-go-v2/config$
155156
msg: build AWS config via awshelper.NewAWSConfigBuilder so requests ride the venv's HTTP client
@@ -221,33 +222,39 @@ linters:
221222
- linters:
222223
- forbidigo
223224
path: ^(internal/view/tui/tty\.go|internal/tf/run_cmd\.go)$
224-
225-
# Everything below are the call sites that predate the forbidigo
226-
# rules above. They reach the real OS directly and still need porting to
227-
# venv. Delete entries as they are ported.
228-
229-
- linters:
230-
- forbidigo
231-
path: ^(internal/cli/commands/browse/tui/model\.go|internal/cli/commands/catalog/cli\.go|internal/view/tui/form/form\.go|internal/cli/commands/catalog/tui/tags_layout\.go|internal/cli/commands/commands\.go|internal/cli/commands/hcl/validate/validate\.go|internal/cli/commands/help/cli\.go|internal/cli/commands/render/render\.go|internal/cli/commands/scaffold/cli\.go|internal/cli/commands/scaffold/scaffold\.go)$
232-
- linters:
233-
- forbidigo
234-
path: ^(internal/tf/cache/handlers/filesystem_mirror_provider\.go|internal/tf/cache/helpers/http\.go|internal/tf/cliconfig/credentials\.go|internal/tf/getproviders/constraints\.go|internal/tf/getproviders/hash\.go|internal/tf/getproviders/lock\.go|internal/tf/getproviders/package_authentication\.go|internal/tf/source\.go|internal/tf/tf\.go)$
225+
# main is where the process arguments enter the program and become the
226+
# arguments every layer below is handed, so it is the one place that has
227+
# nothing to read them from.
235228
- linters:
236229
- forbidigo
237-
path: ^(internal/runner/common/unit_runner\.go|internal/runner/run/debug\.go|internal/runner/run/run\.go|internal/runner/runnerpool/runner\.go)$
238-
- linters:
239-
- forbidigo
240-
path: ^(pkg/config/catalog\.go|pkg/config/config_helpers\.go|pkg/config/config_partial\.go|pkg/config/config\.go|pkg/config/dependency\.go|pkg/config/stack\.go)$
241-
# DefaultWrappedPath is a package-level var, so it probes for `tofu` at
242-
# init time, before any venv exists.
243-
#
244-
# TODO: Make the lookup lazy with a once so we can inject venv into this.
230+
text: use of `os.Args`
231+
path: ^main\.go$
232+
233+
# The vendored urfave/cli help renderer reads CLI_TEMPLATE_ERROR_DEBUG
234+
# from the process with os.Getenv, so the only way to turn its template
235+
# diagnostics on is to put the variable where it will look.
245236
- linters:
246237
- forbidigo
247-
path: ^pkg/options/options\.go$
238+
text: use of `os.Setenv`
239+
path: ^internal/cli/commands/help/cli\.go$
240+
241+
# Pre-existing style debt in two paths, held back so it can be cleaned
242+
# up on its own. The venv rules are deliberately absent from this list,
243+
# so they still apply to both paths.
248244
- linters:
249-
- forbidigo
250-
path: ^(internal/discovery/phase_worktree\.go|internal/engine/engine\.go|internal/engine/verification\.go|internal/getter/tfrhelpers\.go|internal/git/git\.go|internal/remotestate/remote_state\.go|internal/remotestate/terraform_state_file\.go|internal/report/writer\.go|internal/services/catalog/module/doc\.go|internal/services/catalog/module/module\.go|internal/shell/run_cmd\.go|internal/stacks/clean/clean\.go|internal/tfimpl/tfimpl\.go|internal/util/lockfile\.go|internal/worktrees/worktrees\.go)$
245+
- errcheck
246+
- govet
247+
- mnd
248+
- paralleltest
249+
- perfsprint
250+
- staticcheck
251+
- testifylint
252+
- thelper
253+
- tparallel
254+
- unused
255+
- wsl_v5
256+
path: ^(internal/github/|test/integration_docs_test\.go$)
257+
251258
# We end up with duplicated content in this package to save us from duplicating code in other packages.
252259
- linters:
253260
- dupl
@@ -266,10 +273,10 @@ linters:
266273
- lll
267274
path-except: "^(internal/awshelper/|internal/cas/|internal/cli/commands/(backend/(delete|migrate)|browse|catalog/tui/command|dag/graph|exec|find|help|list|scaffold|stack)/|internal/cloner/|internal/codegen/|internal/configbridge/|internal/discovery/|internal/engine/|internal/errorconfig/|internal/errors/|internal/experiment/|internal/filter/|internal/gcphelper/|internal/getter/|internal/git/|internal/os/exec/|internal/prepare/|internal/queue/|internal/report/|internal/retry/|internal/runner/(common|graph|run/creds|runall|runcfg)/|internal/stacks/(generate|output)/|internal/telemetry/|internal/tf/cache/(controllers|middleware)/|internal/tflint/|internal/tips/|internal/vfs/|internal/worktrees/|pkg/log/(format/(options|placeholders)|writer)/|pkg/options/)"
268275
paths:
269-
- docs
270-
- _ci
271-
- .github
272-
- .circleci
276+
- ^docs/
277+
- ^_ci/
278+
- ^\.github/
279+
- ^\.circleci/
273280
- third_party$
274281
- builtin$
275282
- examples$
@@ -285,10 +292,10 @@ formatters:
285292
exclusions:
286293
generated: lax
287294
paths:
288-
- docs
289-
- _ci
290-
- .github
291-
- .circleci
295+
- ^docs/
296+
- ^_ci/
297+
- ^\.github/
298+
- ^\.circleci/
292299
- third_party$
293300
- builtin$
294301
- examples$
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
version: "v1.1.4"
3+
category: "bug-fixes"
4+
---
5+
6+
#### Prompts accept a piped answer that has no trailing newline
7+
8+
Piping an answer to a confirmation prompt, as in `printf yes | terragrunt run --all destroy`, failed with an `EOF` error because Terragrunt discarded a final answer that ended without a newline. The answer is now read as typed, and only a prompt that gets no input at all reports `EOF`.

internal/cas/cas.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"io"
1414
"os"
1515
"path/filepath"
16-
"runtime"
1716

1817
"errors"
1918

@@ -804,6 +803,8 @@ func (c *CAS) ensureBlob(
804803
hash string,
805804
gitPerm os.FileMode,
806805
) (err error) {
806+
v.RequireGOOS()
807+
807808
needsWrite, lock, err := c.blobStore.EnsureWithWait(v, hash)
808809
if err != nil {
809810
return err
@@ -839,7 +840,7 @@ func (c *CAS) ensureBlob(
839840
return err
840841
}
841842

842-
if runtime.GOOS == "windows" {
843+
if v.Platform.GOOS == WindowsOS {
843844
if err = tmpHandle.Sync(); err != nil {
844845
return err
845846
}

internal/cas/content.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"io"
99
"os"
1010
"path/filepath"
11-
"runtime"
1211

1312
"github.com/gruntwork-io/terragrunt/internal/telemetry"
1413
"github.com/gruntwork-io/terragrunt/internal/venv"
@@ -361,6 +360,8 @@ func (c *Content) Read(v *venv.Venv, hash string) ([]byte, error) {
361360
// writeContentToFile writes data to a temporary file, sets appropriate
362361
// permissions, and performs an atomic rename.
363362
func (c *Content) writeContentToFile(l log.Logger, v *venv.Venv, hash string, data []byte) error {
363+
v.RequireGOOS()
364+
364365
path := c.getPath(hash)
365366
tempPath := path + ".tmp"
366367

@@ -411,7 +412,7 @@ func (c *Content) writeContentToFile(l log.Logger, v *venv.Venv, hash string, da
411412
return fmt.Errorf("chmod temp %s: %w", tempPath, err)
412413
}
413414

414-
if runtime.GOOS == WindowsOS {
415+
if v.Platform.GOOS == WindowsOS {
415416
if _, err := v.FS.Stat(path); err == nil {
416417
if err := v.FS.Chmod(path, RegularFilePerms); err != nil {
417418
l.Warnf("failed to make destination file writable %s: %v", path, err)
@@ -427,7 +428,7 @@ func (c *Content) writeContentToFile(l log.Logger, v *venv.Venv, hash string, da
427428
return fmt.Errorf("finalize %s: %w", path, err)
428429
}
429430

430-
if runtime.GOOS == WindowsOS {
431+
if v.Platform.GOOS == WindowsOS {
431432
if err := v.FS.Chmod(path, StoredFilePerms); err != nil {
432433
return fmt.Errorf("chmod %s: %w", path, err)
433434
}

internal/cli/app_test.go

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
awsproviderpatch "github.com/gruntwork-io/terragrunt/internal/cli/commands/aws-provider-patch"
1616
"github.com/gruntwork-io/terragrunt/internal/cli/commands/hcl"
1717
hclformat "github.com/gruntwork-io/terragrunt/internal/cli/commands/hcl/format"
18+
"github.com/gruntwork-io/terragrunt/internal/vexec"
1819

1920
"github.com/gruntwork-io/terragrunt/internal/cli/commands/run"
2021
"github.com/gruntwork-io/terragrunt/internal/cli/flags"
@@ -43,10 +44,8 @@ func TestParseTerragruntOptionsFromArgs(t *testing.T) {
4344
t.Skip("Skipping test on Windows")
4445
}
4546

46-
workingDir, err := os.Getwd()
47-
if err != nil {
48-
t.Fatal(err)
49-
}
47+
workingDir, err := venvtest.New().Platform.Getwd()
48+
require.NoError(t, err)
5049

5150
testCases := []struct {
5251
expectedErr error
@@ -450,7 +449,7 @@ func TestParseTerragruntOptionsFromArgs(t *testing.T) {
450449
t.Run(fmt.Sprintf("testCase-%d", i), func(t *testing.T) {
451450
t.Parallel()
452451

453-
opts := options.NewTerragruntOptions()
452+
opts := options.NewTerragruntOptions(vexec.NewOSExec())
454453

455454
l := log.New(
456455
log.WithOutput(os.Stderr),
@@ -725,7 +724,7 @@ func TestFilterTerragruntArgs(t *testing.T) {
725724
t.Run(fmt.Sprintf("testCase-%d", i), func(t *testing.T) {
726725
t.Parallel()
727726

728-
opts := options.NewTerragruntOptions()
727+
opts := options.NewTerragruntOptions(vexec.NewOSExec())
729728
l := log.New(
730729
log.WithOutput(os.Stderr),
731730
log.WithLevel(defaultLogLevel),
@@ -802,7 +801,7 @@ func TestParseMultiStringArg(t *testing.T) {
802801
t.Run(fmt.Sprintf("testCase-%d", i), func(t *testing.T) {
803802
t.Parallel()
804803

805-
opts := options.NewTerragruntOptions()
804+
opts := options.NewTerragruntOptions(vexec.NewOSExec())
806805
l := log.New(
807806
log.WithOutput(os.Stderr),
808807
log.WithLevel(defaultLogLevel),
@@ -879,7 +878,7 @@ func TestParseMutliStringKeyValueArg(t *testing.T) {
879878
}
880879

881880
for _, tc := range testCases {
882-
opts := options.NewTerragruntOptions()
881+
opts := options.NewTerragruntOptions(vexec.NewOSExec())
883882
opts.AwsProviderPatchOverrides = tc.defaultValue
884883
l := log.New(
885884
log.WithOutput(os.Stderr),
@@ -918,7 +917,7 @@ func TestTerragruntVersion(t *testing.T) {
918917

919918
for _, tc := range testCases {
920919
output := &bytes.Buffer{}
921-
opts := options.NewTerragruntOptions()
920+
opts := options.NewTerragruntOptions(vexec.NewOSExec())
922921

923922
testV := venvtest.New()
924923

@@ -941,7 +940,7 @@ func TestTerragruntHelp(t *testing.T) {
941940

942941
terragruntPrefix := flags.Prefix{flags.TerragruntPrefix}
943942

944-
opts := options.NewTerragruntOptions()
943+
opts := options.NewTerragruntOptions(vexec.NewOSExec())
945944
app := cli.NewApp(logger.CreateLogger(), opts, venvtest.New())
946945

947946
testCases := []struct {
@@ -980,7 +979,7 @@ func TestTerragruntHelp(t *testing.T) {
980979
t.Parallel()
981980

982981
output := &bytes.Buffer{}
983-
opts := options.NewTerragruntOptions()
982+
opts := options.NewTerragruntOptions(vexec.NewOSExec())
984983

985984
testV := venvtest.New()
986985

@@ -1006,7 +1005,7 @@ func TestTerraformHelp_wrongHelpFlag(t *testing.T) {
10061005

10071006
output := &bytes.Buffer{}
10081007

1009-
opts := options.NewTerragruntOptions()
1008+
opts := options.NewTerragruntOptions(vexec.NewOSExec())
10101009

10111010
testV := venvtest.New()
10121011

@@ -1077,7 +1076,9 @@ func (err argMissingValueError) Error() string {
10771076
return "flag needs an argument: -" + string(err)
10781077
}
10791078

1080-
func TestAutocomplete(t *testing.T) { //nolint:paralleltest
1079+
func TestAutocomplete(t *testing.T) {
1080+
t.Parallel()
1081+
10811082
testCases := []struct {
10821083
compLine string
10831084
expectedCompletes []string
@@ -1102,7 +1103,7 @@ func TestAutocomplete(t *testing.T) { //nolint:paralleltest
11021103

11031104
for _, tc := range testCases {
11041105
output := &bytes.Buffer{}
1105-
opts := options.NewTerragruntOptions()
1106+
opts := options.NewTerragruntOptions(vexec.NewOSExec())
11061107

11071108
// Autocomplete reads COMP_LINE from the venv's environment, so the
11081109
// completion request is handed over rather than exported to the process.

internal/cli/app_tf_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"testing"
99

1010
"github.com/gruntwork-io/terragrunt/internal/cli"
11+
"github.com/gruntwork-io/terragrunt/internal/vexec"
1112

1213
"github.com/gruntwork-io/terragrunt/internal/tf"
1314
"github.com/gruntwork-io/terragrunt/internal/venv"
@@ -41,7 +42,7 @@ func TestTFTerraformHelp(t *testing.T) {
4142

4243
for _, tc := range testCases {
4344
output := &bytes.Buffer{}
44-
opts := options.NewTerragruntOptions()
45+
opts := options.NewTerragruntOptions(vexec.NewOSExec())
4546

4647
testV := venv.OSVenv()
4748

internal/cli/commands/backend/bootstrap/bootstrap.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func runBootstrap(
4343
return err
4444
}
4545

46-
return remoteState.Bootstrap(ctx, l, v, configbridge.RemoteStateOptsFromOpts(opts))
46+
return remoteState.Bootstrap(ctx, l, v, configbridge.RemoteStateOptsFromOpts(v.Env, opts))
4747
})
4848
}
4949

internal/cli/commands/backend/delete/delete.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func runDelete(
4444
ctx,
4545
l,
4646
v,
47-
configbridge.RemoteStateOptsFromOpts(opts),
47+
configbridge.RemoteStateOptsFromOpts(v.Env, opts),
4848
)
4949
if err != nil && !errors.As(err, new(backend.BucketDoesNotExistError)) {
5050
return err
@@ -63,7 +63,7 @@ func runDelete(
6363
return fmt.Errorf("flag -%s is not supported yet", BucketFlagName)
6464
}
6565

66-
return remoteState.Delete(ctx, l, v, configbridge.RemoteStateOptsFromOpts(opts))
66+
return remoteState.Delete(ctx, l, v, configbridge.RemoteStateOptsFromOpts(v.Env, opts))
6767
}
6868

6969
func runAll(

internal/cli/commands/backend/migrate/migrate.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func Run(
113113
ctx,
114114
l,
115115
srcV,
116-
configbridge.RemoteStateOptsFromOpts(srcOpts),
116+
configbridge.RemoteStateOptsFromOpts(v.Env, srcOpts),
117117
)
118118
if err != nil && !errors.As(err, new(backend.BucketDoesNotExistError)) {
119119
return err
@@ -130,8 +130,8 @@ func Run(
130130
return srcRemoteState.Migrate(
131131
ctx, l,
132132
srcV, dstV,
133-
configbridge.RemoteStateOptsFromOpts(srcOpts),
134-
configbridge.RemoteStateOptsFromOpts(dstOpts),
133+
configbridge.RemoteStateOptsFromOpts(v.Env, srcOpts),
134+
configbridge.RemoteStateOptsFromOpts(v.Env, dstOpts),
135135
dstRemoteState,
136136
)
137137
}

internal/cli/commands/browse/browse.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@ func Run(ctx context.Context, l log.Logger, v *venv.Venv, opts *Options) error {
7373
color = tui.ColorEnabled
7474
}
7575

76-
err = tui.Run(ctx, l, v.FS, root, color, resultCh, warnCh)
76+
v.RequireUserHomeDir()
77+
78+
err = tui.Run(ctx, l, v.FS, v.Platform.UserHomeDir, root, color, resultCh, warnCh)
7779

7880
cancel()
7981
<-done

0 commit comments

Comments
 (0)