From 07131472140ac9d6d41f707c9a8bd11df40d10de Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 11 Sep 2026 13:53:33 -0500 Subject: [PATCH 1/3] chore: update trivy do include resource closure targets --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 353e2dd..3f057fb 100644 --- a/go.mod +++ b/go.mod @@ -159,4 +159,4 @@ require ( // Trivy has some issues that we're floating patches for, and will hopefully // be upstreamed eventually. -replace github.com/aquasecurity/trivy => github.com/coder/trivy v0.0.0-20260309164037-c413f5a2f511 +replace github.com/aquasecurity/trivy => github.com/coder/trivy v0.0.0-20260911135535-15b949537506 diff --git a/go.sum b/go.sum index 70c9552..170f6f7 100644 --- a/go.sum +++ b/go.sum @@ -115,8 +115,8 @@ github.com/coder/serpent v0.10.0 h1:ofVk9FJXSek+SmL3yVE3GoArP83M+1tX+H7S4t8BSuM= github.com/coder/serpent v0.10.0/go.mod h1:cZFW6/fP+kE9nd/oRkEHJpG6sXCtQ+AX7WMMEHv0Y3Q= github.com/coder/terraform-provider-coder/v2 v2.8.0 h1:pbWfegCPI0v8eATgE8kGwIyuaMPgMRIcdLF2GTVkgG0= github.com/coder/terraform-provider-coder/v2 v2.8.0/go.mod h1:WrdLSbihuzH1RZhwrU+qmkqEhUbdZT/sjHHdarm5b5g= -github.com/coder/trivy v0.0.0-20260309164037-c413f5a2f511 h1:wJS3Pk13VuCbV8hjrQRnOBCUwP3Islk91sMvbSdY0Vk= -github.com/coder/trivy v0.0.0-20260309164037-c413f5a2f511/go.mod h1:+zF17ZBOdhFWwD3+GkLxZ/vkmKLudoOtt+hgnc1TQpA= +github.com/coder/trivy v0.0.0-20260911135535-15b949537506 h1:A9f2UisugOgV2JlhibP0GZdzpqn+sWPIcz4q3bsgopI= +github.com/coder/trivy v0.0.0-20260911135535-15b949537506/go.mod h1:+zF17ZBOdhFWwD3+GkLxZ/vkmKLudoOtt+hgnc1TQpA= github.com/coder/websocket v1.8.13 h1:f3QZdXy7uGVz+4uCJy2nTZyM0yTBj8yANEHhqlXZ9FE= github.com/coder/websocket v1.8.13/go.mod h1:LNVeNrXQZfe5qhS9ALED3uA+l5pPqvwXg3CKoDBB2gs= github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG8PI= From addd91d0394ef1b3a8cf2ed2ab76c40e8669091e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 11 Sep 2026 19:01:01 +0000 Subject: [PATCH 2/3] feat: evaluate only the parameter/preset/tag closure, with an opt-out Preview now passes OptionWithResourceClosure to the parser by default so root resources nothing in the parameter, preset, or tag closure references are skipped. Preview takes variadic options; OptionFullEvaluation restores evaluating every resource, so a caller can turn the optimization off without a new preview release if a template misbehaves. Test_Extract runs every vector in both modes against the same expectations. Test_OptionFullEvaluation observes the difference through Output.ModuleOutput, which is the one output the closure changes. --- options.go | 45 +++++++++ preview.go | 13 ++- preview_test.go | 170 ++++++++++++++++++++------------ testdata/fullevaluation/main.tf | 30 ++++++ testdata/fullevaluation/skipe2e | 1 + 5 files changed, 192 insertions(+), 67 deletions(-) create mode 100644 options.go create mode 100644 testdata/fullevaluation/main.tf create mode 100644 testdata/fullevaluation/skipe2e diff --git a/options.go b/options.go new file mode 100644 index 0000000..4803519 --- /dev/null +++ b/options.go @@ -0,0 +1,45 @@ +package preview + +// Option adjusts how Preview evaluates a template. Options exist so a caller +// can change evaluation behavior without a new preview release, for example +// to turn off an optimization that misbehaves on a particular template. +type Option func(*options) + +type options struct { + // fullEvaluation disables resource closure pruning so every resource in + // the root module is evaluated. + fullEvaluation bool +} + +// resourceClosureTargets are the block types whose values Preview renders. +// Only the parameter/preset/tag blocks and what they reference need to be +// evaluated to render a workspace form. The resources a workspace would create +// cannot feed those blocks, so root resources nothing in this closure +// references are skipped by default. +var resourceClosureTargets = []string{ + "coder_parameter", + "coder_workspace_preset", + "coder_workspace_tags", +} + +// OptionFullEvaluation evaluates every resource in the root module instead of +// only those reachable from parameter, preset, and tag blocks. It is the escape +// hatch for the resource closure optimization: parameters, presets, and tags +// are unchanged either way, so this only matters when that optimization has a +// bug, or when Output.ModuleOutput must include outputs that read resources +// outside the closure. +func OptionFullEvaluation() Option { + return func(o *options) { + o.fullEvaluation = true + } +} + +func applyOptions(opts []Option) options { + var o options + for _, opt := range opts { + if opt != nil { + opt(&o) + } + } + return o +} diff --git a/preview.go b/preview.go index 32ac43f..8f76bbe 100644 --- a/preview.go +++ b/preview.go @@ -139,7 +139,8 @@ func ValidatePrebuilds(ctx context.Context, input Input, preValid []types.Preset } } -func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagnostics hcl.Diagnostics) { +func Preview(ctx context.Context, input Input, dir fs.FS, opts ...Option) (output *Output, diagnostics hcl.Diagnostics) { + settings := applyOptions(opts) // The trivy package works with `github.com/zclconf/go-cty`. This package is // similar to `reflect` in its usage. This package can panic if types are // misused. To protect the caller, a general `recover` is used to catch any @@ -234,7 +235,7 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn } // moduleSource is "" for a local module - p := parser.New(dir, "", + parserOpts := []parser.Option{ parser.OptionWithLogger(logger), parser.OptionStopOnHCLError(false), parser.OptionWithDownloads(false), @@ -246,7 +247,13 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn // 'OptionsWithTfVars' cannot be set with 'OptionWithTFVarsPaths'. So load the // tfvars from the files ourselves and merge with the user-supplied tf vars. parser.OptionsWithTfVars(variableValues), - ) + } + if !settings.fullEvaluation { + // Skip root resources that nothing in the parameter/preset/tag closure + // references. See resourceClosureTargets and OptionFullEvaluation. + parserOpts = append(parserOpts, parser.OptionWithResourceClosure(resourceClosureTargets)) + } + p := parser.New(dir, "", parserOpts...) err = p.ParseFS(ctx, ".") if err != nil { diff --git a/preview_test.go b/preview_test.go index 5f360e4..bc23b2b 100644 --- a/preview_test.go +++ b/preview_test.go @@ -867,85 +867,127 @@ func Test_Extract(t *testing.T) { }, }, } { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - if tc.skip != "" { - t.Skip(tc.skip) - return - } + // Every vector runs twice: with the default resource closure + // optimization, and with OptionFullEvaluation. The expectations are + // shared, which pins that the optimization is output-neutral for + // parameters, presets, tags, and variables. + for _, mode := range []struct { + name string + opts []preview.Option + }{ + {name: "closure"}, + {name: "full", opts: []preview.Option{preview.OptionFullEvaluation()}}, + } { + t.Run(tc.name+"/"+mode.name, func(t *testing.T) { + t.Parallel() + if tc.skip != "" { + t.Skip(tc.skip) + return + } - if tc.unknownTags == nil { - tc.unknownTags = []string{} - } - if tc.expTags == nil { - tc.expTags = map[string]string{} - } + if tc.unknownTags == nil { + tc.unknownTags = []string{} + } + if tc.expTags == nil { + tc.expTags = map[string]string{} + } - dirFs := os.DirFS(filepath.Join("testdata", tc.dir)) + dirFs := os.DirFS(filepath.Join("testdata", tc.dir)) - output, diags := preview.Preview(context.Background(), tc.input, dirFs) - if tc.failPreview { - require.True(t, diags.HasErrors()) - return - } - if diags.HasErrors() { - t.Logf("diags: %s", diags) - } - require.False(t, diags.HasErrors()) + output, diags := preview.Preview(context.Background(), tc.input, dirFs, mode.opts...) + if tc.failPreview { + require.True(t, diags.HasErrors()) + return + } + if diags.HasErrors() { + t.Logf("diags: %s", diags) + } + require.False(t, diags.HasErrors()) - // Validate prebuilds too - preview.ValidatePrebuilds(context.Background(), tc.input, output.Presets, dirFs) + // Validate prebuilds too + preview.ValidatePrebuilds(context.Background(), tc.input, output.Presets, dirFs) - if len(tc.warnings) > 0 { - for _, w := range tc.warnings { - idx := slices.IndexFunc(diags, func(diagnostic *hcl.Diagnostic) bool { - return w.MatchString(diagnostic.Error()) + if len(tc.warnings) > 0 { + for _, w := range tc.warnings { + idx := slices.IndexFunc(diags, func(diagnostic *hcl.Diagnostic) bool { + return w.MatchString(diagnostic.Error()) - }) - require.Greater(t, idx, -1, "expected warning %q to be present in diags", w.String()) + }) + require.Greater(t, idx, -1, "expected warning %q to be present in diags", w.String()) + } } - } - - // Assert tags - validTags := output.WorkspaceTags.Tags() - for k, expected := range tc.expTags { - tag, ok := validTags[k] - if !ok { - t.Errorf("expected tag %q to be present in output, but it was not", k) - continue + // Assert tags + validTags := output.WorkspaceTags.Tags() + + for k, expected := range tc.expTags { + tag, ok := validTags[k] + if !ok { + t.Errorf("expected tag %q to be present in output, but it was not", k) + continue + } + if tag != expected { + assert.JSONEqf(t, expected, tag, "tag %q does not match expected, nor is it a json equivalent", k) + } } - if tag != expected { - assert.JSONEqf(t, expected, tag, "tag %q does not match expected, nor is it a json equivalent", k) + assert.Equal(t, len(tc.expTags), len(output.WorkspaceTags.Tags()), "unexpected number of tags in output") + + assert.ElementsMatch(t, tc.unknownTags, output.WorkspaceTags.UnusableTags().SafeNames()) + + // Assert params + require.Len(t, output.Parameters, len(tc.params), "wrong number of parameters expected") + for _, param := range output.Parameters { + check, ok := tc.params[param.Name] + require.True(t, ok, "unknown parameter %s", param.Name) + check(t, param) } - } - assert.Equal(t, len(tc.expTags), len(output.WorkspaceTags.Tags()), "unexpected number of tags in output") - assert.ElementsMatch(t, tc.unknownTags, output.WorkspaceTags.UnusableTags().SafeNames()) + for _, preset := range output.Presets { + check, ok := tc.presets[preset.Name] + require.True(t, ok, "unknown preset %s", preset.Name) + check(t, preset) + } - // Assert params - require.Len(t, output.Parameters, len(tc.params), "wrong number of parameters expected") - for _, param := range output.Parameters { - check, ok := tc.params[param.Name] - require.True(t, ok, "unknown parameter %s", param.Name) - check(t, param) - } + // Assert variables + require.Len(t, output.Variables, len(tc.variables), "wrong number of variables expected") + for _, variable := range output.Variables { + check, ok := tc.variables[variable.Name] + require.True(t, ok, "unknown variable %s", variable.Name) + check(t, variable) + } + }) + } + } +} - for _, preset := range output.Presets { - check, ok := tc.presets[preset.Name] - require.True(t, ok, "unknown preset %s", preset.Name) - check(t, preset) - } +// Test_OptionFullEvaluation proves the option changes evaluation, not just +// that both modes agree on parameters. Output.ModuleOutput is the one place a +// resource outside the closure is observable. +func Test_OptionFullEvaluation(t *testing.T) { + t.Parallel() - // Assert variables - require.Len(t, output.Variables, len(tc.variables), "wrong number of variables expected") - for _, variable := range output.Variables { - check, ok := tc.variables[variable.Name] - require.True(t, ok, "unknown variable %s", variable.Name) - check(t, variable) - } - }) + dirFs := os.DirFS(filepath.Join("testdata", "fullevaluation")) + outputName := func(t *testing.T, opts ...preview.Option) cty.Value { + t.Helper() + output, diags := preview.Preview(context.Background(), preview.Input{}, dirFs, opts...) + require.False(t, diags.HasErrors(), diags.Error()) + require.Len(t, output.Parameters, 1) + assert.Equal(t, "large", output.Parameters[0].Value.AsString()) + return output.ModuleOutput.GetAttr("unreferenced_name") } + + t.Run("Closure", func(t *testing.T) { + t.Parallel() + v := outputName(t) + assert.False(t, v.IsKnown(), "resource outside the closure should not be evaluated by default, got %s", v.GoString()) + }) + + t.Run("Full", func(t *testing.T) { + t.Parallel() + v := outputName(t, preview.OptionFullEvaluation()) + require.True(t, v.IsKnown(), "OptionFullEvaluation must evaluate every resource") + assert.Equal(t, "outside-closure", v.AsString()) + }) } func TestPresetValidation(t *testing.T) { diff --git a/testdata/fullevaluation/main.tf b/testdata/fullevaluation/main.tf new file mode 100644 index 0000000..5b7a4a3 --- /dev/null +++ b/testdata/fullevaluation/main.tf @@ -0,0 +1,30 @@ +// Observes whether resources outside the parameter/preset/tag closure were +// evaluated. Nothing a parameter reads references the resource, so the default +// closure skips it and the output that reads it is unknown. With +// OptionFullEvaluation the resource is evaluated and the output resolves. +terraform { + required_providers { + coder = { + source = "coder/coder" + version = "2.4.0-pre0" + } + docker = { + source = "kreuzwerker/docker" + version = "3.0.2" + } + } +} + +data "coder_parameter" "flavor" { + name = "flavor" + type = "string" + default = "large" +} + +resource "docker_image" "unreferenced" { + name = "outside-closure" +} + +output "unreferenced_name" { + value = docker_image.unreferenced.name +} diff --git a/testdata/fullevaluation/skipe2e b/testdata/fullevaluation/skipe2e new file mode 100644 index 0000000..6839a83 --- /dev/null +++ b/testdata/fullevaluation/skipe2e @@ -0,0 +1 @@ +evaluation mode is exercised by Test_OptionFullEvaluation (static preview eval); real terraform apply is out of scope here From 8cf05d1febc3ca3a003d4539f70c8816fb006e4f Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 11 Sep 2026 19:52:29 +0000 Subject: [PATCH 3/3] refactor: inline the resource closure target list --- options.go | 11 ----------- preview.go | 12 +++++++++--- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/options.go b/options.go index 4803519..577c39f 100644 --- a/options.go +++ b/options.go @@ -11,17 +11,6 @@ type options struct { fullEvaluation bool } -// resourceClosureTargets are the block types whose values Preview renders. -// Only the parameter/preset/tag blocks and what they reference need to be -// evaluated to render a workspace form. The resources a workspace would create -// cannot feed those blocks, so root resources nothing in this closure -// references are skipped by default. -var resourceClosureTargets = []string{ - "coder_parameter", - "coder_workspace_preset", - "coder_workspace_tags", -} - // OptionFullEvaluation evaluates every resource in the root module instead of // only those reachable from parameter, preset, and tag blocks. It is the escape // hatch for the resource closure optimization: parameters, presets, and tags diff --git a/preview.go b/preview.go index 8f76bbe..51f490a 100644 --- a/preview.go +++ b/preview.go @@ -249,9 +249,15 @@ func Preview(ctx context.Context, input Input, dir fs.FS, opts ...Option) (outpu parser.OptionsWithTfVars(variableValues), } if !settings.fullEvaluation { - // Skip root resources that nothing in the parameter/preset/tag closure - // references. See resourceClosureTargets and OptionFullEvaluation. - parserOpts = append(parserOpts, parser.OptionWithResourceClosure(resourceClosureTargets)) + // Only the parameter/preset/tag blocks and what they reference need to + // be evaluated to render a workspace form. The resources a workspace + // would create cannot feed those blocks, so root resources nothing in + // this closure references are skipped. See OptionFullEvaluation. + parserOpts = append(parserOpts, parser.OptionWithResourceClosure([]string{ + "coder_parameter", + "coder_workspace_preset", + "coder_workspace_tags", + })) } p := parser.New(dir, "", parserOpts...)