From 9510bb86ffdd303fea997075d125231e591d447a Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Sun, 1 Mar 2026 09:33:52 -0800 Subject: [PATCH 1/4] feat: support Terraform override files in template preview Implement Terraform's override file semantics (override.tf, *_override.tf) by merging override blocks into primary files before evaluation. Related to: https://github.com/coder/coder/issues/21991 --- override.go | 254 ++++++++++++++++++++++ override_test.go | 367 ++++++++++++++++++++++++++++++++ overridefs.go | 94 ++++++++ preview.go | 16 ++ preview_test.go | 17 ++ testdata/override/a_override.tf | 46 ++++ testdata/override/main.tf | 57 +++++ testdata/override/override.tf | 17 ++ 8 files changed, 868 insertions(+) create mode 100644 override.go create mode 100644 override_test.go create mode 100644 overridefs.go create mode 100644 testdata/override/a_override.tf create mode 100644 testdata/override/main.tf create mode 100644 testdata/override/override.tf diff --git a/override.go b/override.go new file mode 100644 index 0000000..8b75fd8 --- /dev/null +++ b/override.go @@ -0,0 +1,254 @@ +package preview + +import ( + "fmt" + "io/fs" + "path" + "strings" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclwrite" +) + +// mergeOverrideFiles scans the filesystem for Terraform override files and +// returns a new FS where override content has been merged into primary files +// using Terraform's override semantics. If no override files exist, the +// original FS is returned unchanged. +// +// Override files are identified by Terraform's naming convention: +// "override.tf", "*_override.tf", and their .tf.json variants. +// +// https://developer.hashicorp.com/terraform/language/files/override +// +// Note: Terraform validates primary blocks before merging overrides, so it +// rejects a primary block that is missing a required attribute even if the +// override would supply it. We merge first, so this edge case passes +// validation. This is immaterial in practice: Coder runs terraform plan +// during template import, which would catch it before the template is saved. +func mergeOverrideFiles(original fs.FS) (fs.FS, error) { + // Group files by directory, separating primary from override files. + // Walk the entire tree, not just the root directory, because Trivy's + // EvaluateAll processes all modules, so we need to pre-merge overrides at + // every level before Trivy sees the FS. + type dirFiles struct { + primary []string + override []string + } + dirs := make(map[string]*dirFiles) + + err := fs.WalkDir(original, ".", func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + // Skip dirs; we deal with them by acting on their files. + if d.IsDir() { + return nil + } + if tfFileExt(d.Name()) == "" { + return nil + } + + dir := path.Dir(p) + if dirs[dir] == nil { + dirs[dir] = &dirFiles{} + } + + if isOverrideFile(d.Name()) { + dirs[dir].override = append(dirs[dir].override, p) + } else { + dirs[dir].primary = append(dirs[dir].primary, p) + } + return nil + }) + if err != nil { + return nil, fmt.Errorf("walk filesystem: %w", err) + } + + // We are a no-op if there are no override files at all. + hasOverrides := false + for _, dir := range dirs { + if len(dir.override) > 0 { + hasOverrides = true + break + } + } + if !hasOverrides { + return original, nil + } + + replaced := make(map[string][]byte) + hidden := make(map[string]bool) + + for _, dir := range dirs { + if len(dir.override) == 0 { + continue + } + + // Parse all primary files upfront so override files can be applied + // sequentially, each merging into the already-merged result. + type primaryState struct { + path string + file *hclwrite.File + modified bool + } + primaries := make([]*primaryState, 0, len(dir.primary)) + for _, path := range dir.primary { + content, err := fs.ReadFile(original, path) + if err != nil { + return nil, fmt.Errorf("read file %s: %w", path, err) + } + f, diags := hclwrite.ParseConfig(content, path, hcl.Pos{Line: 1, Column: 1}) + if diags.HasErrors() { + return nil, fmt.Errorf("parse file %s: %s", path, diags.Error()) + } + primaries = append(primaries, &primaryState{path: path, file: f}) + } + + // Process each override file sequentially. If multiple override files + // define the same block, each merges into the already-merged primary, + // matching Terraform's behavior. + for _, path := range dir.override { + content, err := fs.ReadFile(original, path) + if err != nil { + return nil, fmt.Errorf("read file %s: %w", path, err) + } + + f, diags := hclwrite.ParseConfig(content, path, hcl.Pos{Line: 1, Column: 1}) + if diags.HasErrors() { + return nil, fmt.Errorf("parse file %s: %s", path, diags.Error()) + } + + for _, oblock := range f.Body().Blocks() { + key := blockKey(oblock.Type(), oblock.Labels()) + matched := false + for _, primary := range primaries { + for _, pblock := range primary.file.Body().Blocks() { + if blockKey(pblock.Type(), pblock.Labels()) == key { + mergeBlock(pblock, oblock) + primary.modified = true + matched = true + break + } + } + if matched { + break + } + } + if !matched { + return nil, fmt.Errorf("override block %s in %s has no matching block in a primary configuration file", key, path) + } + } + + hidden[path] = true + } + + // Collect modified primary files. + for _, p := range primaries { + if p.modified { + replaced[p.path] = p.file.Bytes() + } + } + } + + return &overrideFS{ + base: original, + replaced: replaced, + hidden: hidden, + }, nil +} + +// mergeBlock applies override attributes and child blocks to a primary block +// using Terraform's prepareContent semantics. +// +// - Attributes: each override attribute replaces the corresponding primary +// attribute, or is inserted if it does not exist in the primary block. +// +// - Child blocks: if override has any block of type X (including dynamic "X"), +// all blocks of type X and dynamic "X" are removed from primary. Then all +// override child blocks are appended — both replacing suppressed types and +// introducing entirely new block types not present in the primary. +// +// Ref: https://github.com/hashicorp/terraform/blob/7960f60d2147d43f5cf675a898438f6a6693da1b/internal/configs/module_merge_body.go#L76-L121 +// +// Note: Terraform re-validates type/default compatibility after variable block +// merge. We rely on Trivy's evaluator to do that during evaluation of the +// merged HCL. +func mergeBlock(primary, override *hclwrite.Block) { + // Merge attributes: override clobbers base. + for name, attr := range override.Body().Attributes() { + primary.Body().SetAttributeRaw(name, attr.Expr().BuildTokens(nil)) + } + + // Determine which child (nested) block types are overridden. + overriddenBlockTypes := make(map[string]bool) + for _, child := range override.Body().Blocks() { + if child.Type() == "dynamic" && len(child.Labels()) > 0 { + overriddenBlockTypes[child.Labels()[0]] = true + } else { + overriddenBlockTypes[child.Type()] = true + } + } + + if len(overriddenBlockTypes) == 0 { + return + } + + // Remove overridden block types from primary. + // Collect blocks to remove first to avoid modifying during iteration. + var toRemove []*hclwrite.Block + for _, child := range primary.Body().Blocks() { + shouldRemove := false + if child.Type() == "dynamic" && len(child.Labels()) > 0 { + shouldRemove = overriddenBlockTypes[child.Labels()[0]] + } else { + shouldRemove = overriddenBlockTypes[child.Type()] + } + if shouldRemove { + toRemove = append(toRemove, child) + } + } + for _, block := range toRemove { + primary.Body().RemoveBlock(block) + } + + // Append all override child blocks. + for _, child := range override.Body().Blocks() { + primary.Body().AppendBlock(child) + } +} + +// isOverrideFile returns true if the filename matches Terraform's override +// file naming convention: "override.tf", "*_override.tf", and .tf.json variants. +// +// Ref: https://github.com/hashicorp/terraform/blob/7960f60d2147d43f5cf675a898438f6a6693da1b/internal/configs/parser_file_matcher.go#L161-L170 +func isOverrideFile(filename string) bool { + name := path.Base(filename) + ext := tfFileExt(name) + if ext == "" { + return false + } + baseName := name[:len(name)-len(ext)] + return baseName == "override" || strings.HasSuffix(baseName, "_override") +} + +// tfFileExt returns the Terraform file extension (".tf" or ".tf.json") if +// present, or "" otherwise. +func tfFileExt(name string) string { + if strings.HasSuffix(name, ".tf.json") { + return ".tf.json" + } + if strings.HasSuffix(name, ".tf") { + return ".tf" + } + return "" +} + +// blockKey returns a string that uniquely identifies a block for override +// matching purposes. Two blocks with the same key represent the same logical +// entity (one primary, one override). +func blockKey(blockType string, labels []string) string { + if len(labels) == 0 { + return blockType + } + return blockType + "." + strings.Join(labels, ".") +} diff --git a/override_test.go b/override_test.go new file mode 100644 index 0000000..82e87d7 --- /dev/null +++ b/override_test.go @@ -0,0 +1,367 @@ +package preview + +import ( + "io/fs" + "strings" + "testing" + "testing/fstest" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclwrite" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIsOverrideFile(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + filename string + expected bool + }{ + {"override.tf", "override.tf", true}, + {"foo_override.tf", "foo_override.tf", true}, + {"override.tf.json", "override.tf.json", true}, + {"foo_override.tf.json", "foo_override.tf.json", true}, + {"main.tf", "main.tf", false}, + {"overrides.tf", "overrides.tf", false}, + {"my_override_file.tf", "my_override_file.tf", false}, + {"no extension", "override", false}, + {"go file", "override.go", false}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.expected, isOverrideFile(tc.filename)) + }) + } +} + +func TestBlockKey(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + blockType string + labels []string + expected string + }{ + {"no labels", "terraform", nil, "terraform"}, + {"one label", "variable", []string{"env"}, "variable.env"}, + {"two labels", "data", []string{"coder_parameter", "region"}, "data.coder_parameter.region"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.expected, blockKey(tc.blockType, tc.labels)) + }) + } +} + +func TestMergeBlock(t *testing.T) { + t.Parallel() + + // attrValue returns the text representation of an attribute's value. + attrValue := func(t *testing.T, attrs map[string]*hclwrite.Attribute, name string) string { + t.Helper() + a, ok := attrs[name] + require.True(t, ok, "attribute %q not found", name) + // trim because BuilTokens preserves the leading whitespace. + return strings.TrimSpace(string(a.Expr().BuildTokens(nil).Bytes())) + } + + // parseBlock parses HCL source and returns the first block. + parseBlock := func(t *testing.T, src string) *hclwrite.Block { + t.Helper() + f, diags := hclwrite.ParseConfig([]byte(src), "test.tf", hcl.Pos{Line: 1, Column: 1}) + require.False(t, diags.HasErrors(), diags.Error()) + return f.Body().Blocks()[0] + } + + t.Run("AttributeClobber", func(t *testing.T) { + t.Parallel() + primary := parseBlock(t, `resource "a" "b" { + x = 1 + y = 2 +}`) + override := parseBlock(t, `resource "a" "b" { + y = 3 +}`) + mergeBlock(primary, override) + + attrs := primary.Body().Attributes() + assert.Equal(t, "1", attrValue(t, attrs, "x")) + assert.Equal(t, "3", attrValue(t, attrs, "y")) + }) + + t.Run("AttributeInsertion", func(t *testing.T) { + t.Parallel() + primary := parseBlock(t, `resource "a" "b" { + x = 1 +}`) + override := parseBlock(t, `resource "a" "b" { + z = "new" +}`) + mergeBlock(primary, override) + + attrs := primary.Body().Attributes() + require.Contains(t, attrs, "x") + require.Contains(t, attrs, "z") + }) + + t.Run("NestedBlockSuppression", func(t *testing.T) { + t.Parallel() + primary := parseBlock(t, `data "coder_parameter" "disk" { + name = "disk" + option { + name = "10GB" + value = 10 + } + option { + name = "20GB" + value = 20 + } +}`) + override := parseBlock(t, `data "coder_parameter" "disk" { + option { + name = "30GB" + value = 30 + } +}`) + mergeBlock(primary, override) + + // Primary's two options should be replaced by override's single option. + blocks := primary.Body().Blocks() + require.Len(t, blocks, 1) + assert.Equal(t, "option", blocks[0].Type()) + assert.Equal(t, "30", attrValue(t, blocks[0].Body().Attributes(), "value")) + }) + + t.Run("DynamicStaticSuppression", func(t *testing.T) { + t.Parallel() + primary := parseBlock(t, `resource "a" "b" { + option { + name = "static" + } +}`) + override := parseBlock(t, `resource "a" "b" { + dynamic "option" { + for_each = var.options + content { + name = option.value + } + } +}`) + mergeBlock(primary, override) + + // Static "option" should be removed and replaced by dynamic "option". + blocks := primary.Body().Blocks() + require.Len(t, blocks, 1) + assert.Equal(t, "dynamic", blocks[0].Type()) + require.Len(t, blocks[0].Labels(), 1) + assert.Equal(t, "option", blocks[0].Labels()[0]) + }) + + t.Run("StaticDynamicSuppression", func(t *testing.T) { + t.Parallel() + primary := parseBlock(t, `resource "a" "b" { + dynamic "option" { + for_each = var.options + content { + name = option.value + } + } +}`) + override := parseBlock(t, `resource "a" "b" { + option { + name = "static" + } +}`) + mergeBlock(primary, override) + + // Dynamic "option" should be removed and replaced by static "option". + blocks := primary.Body().Blocks() + require.Len(t, blocks, 1) + assert.Equal(t, "option", blocks[0].Type()) + assert.Empty(t, blocks[0].Labels()) + }) + + t.Run("NoNestedBlocksInOverride", func(t *testing.T) { + t.Parallel() + primary := parseBlock(t, `resource "a" "b" { + x = 1 + nested { + y = 2 + } +}`) + override := parseBlock(t, `resource "a" "b" { + x = 99 +}`) + mergeBlock(primary, override) + + // Primary's nested blocks should be preserved when override has none. + blocks := primary.Body().Blocks() + require.Len(t, blocks, 1) + assert.Equal(t, "nested", blocks[0].Type()) + // Attribute should still be overridden. + assert.Equal(t, "99", attrValue(t, primary.Body().Attributes(), "x")) + }) +} + +// readFile reads a file from an fs.FS using Open+Read (since overrideFS +// doesn't implement fs.ReadFileFS). +func readFile(t *testing.T, fsys fs.FS, name string) []byte { + t.Helper() + f, err := fsys.Open(name) + require.NoError(t, err) + defer f.Close() + info, err := f.Stat() + require.NoError(t, err) + buf := make([]byte, info.Size()) + _, err = f.Read(buf) + require.NoError(t, err) + return buf +} + +func TestMergeOverrideFiles(t *testing.T) { + t.Parallel() + + t.Run("NoOverrideFiles", func(t *testing.T) { + t.Parallel() + original := fstest.MapFS{ + "main.tf": &fstest.MapFile{Data: []byte(`resource "a" "b" { x = 1 }`)}, + } + result, err := mergeOverrideFiles(original) + require.NoError(t, err) + // Should return the exact same FS when there are no overrides. + assert.Equal(t, original, result) + }) + + t.Run("UnmatchedOverrideBlock", func(t *testing.T) { + t.Parallel() + fsys := fstest.MapFS{ + "main.tf": &fstest.MapFile{Data: []byte(`resource "a" "b" { x = 1 }`)}, + "override.tf": &fstest.MapFile{Data: []byte(`resource "c" "d" { + y = 2 +}`)}, + } + _, err := mergeOverrideFiles(fsys) + require.Error(t, err) + assert.Contains(t, err.Error(), "no matching block") + }) + + t.Run("BasicAttributeMerge", func(t *testing.T) { + t.Parallel() + fsys := fstest.MapFS{ + "main.tf": &fstest.MapFile{Data: []byte(`resource "a" "b" { + x = 1 + y = 2 +}`)}, + "override.tf": &fstest.MapFile{Data: []byte(`resource "a" "b" { + y = 99 +}`)}, + } + result, err := mergeOverrideFiles(fsys) + require.NoError(t, err) + + // Read the merged primary file. + content := readFile(t, result, "main.tf") + assert.Contains(t, string(content), "99") + }) + + t.Run("OverrideFileHidden", func(t *testing.T) { + t.Parallel() + fsys := fstest.MapFS{ + "main.tf": &fstest.MapFile{Data: []byte(`resource "a" "b" { x = 1 }`)}, + "override.tf": &fstest.MapFile{Data: []byte(`resource "a" "b" { x = 2 }`)}, + } + result, err := mergeOverrideFiles(fsys) + require.NoError(t, err) + + _, err = result.Open("override.tf") + require.Error(t, err) + assert.ErrorIs(t, err, fs.ErrNotExist) + }) + + t.Run("DirectoryListingFiltersHidden", func(t *testing.T) { + t.Parallel() + fsys := fstest.MapFS{ + "main.tf": &fstest.MapFile{Data: []byte(`resource "a" "b" { x = 1 }`)}, + "foo_override.tf": &fstest.MapFile{Data: []byte(`resource "a" "b" { x = 2 }`)}, + "other.txt": &fstest.MapFile{Data: []byte("hello")}, + } + result, err := mergeOverrideFiles(fsys) + require.NoError(t, err) + + f, err := result.Open(".") + require.NoError(t, err) + defer f.Close() + + rdf, ok := f.(fs.ReadDirFile) + require.True(t, ok) + + entries, err := rdf.ReadDir(-1) + require.NoError(t, err) + + names := make([]string, 0, len(entries)) + for _, e := range entries { + names = append(names, e.Name()) + } + assert.Contains(t, names, "main.tf") + assert.Contains(t, names, "other.txt") + assert.NotContains(t, names, "foo_override.tf") + }) + + t.Run("SequentialOverrideMerge", func(t *testing.T) { + t.Parallel() + // Two override files modify the same block. Because WalkDir processes + // them in lexical order (a_ before b_), both attributes should be + // present in the merged result. + fsys := fstest.MapFS{ + "main.tf": &fstest.MapFile{Data: []byte(`resource "a" "b" { + original = "yes" +}`)}, + "a_override.tf": &fstest.MapFile{Data: []byte(`resource "a" "b" { + from_a = "aaa" +}`)}, + "b_override.tf": &fstest.MapFile{Data: []byte(`resource "a" "b" { + from_b = "bbb" +}`)}, + } + result, err := mergeOverrideFiles(fsys) + require.NoError(t, err) + + content := readFile(t, result, "main.tf") + merged := string(content) + assert.Contains(t, merged, "original") + assert.Contains(t, merged, "from_a") + assert.Contains(t, merged, "from_b") + }) + + t.Run("SubdirectoryOverrides", func(t *testing.T) { + t.Parallel() + fsys := fstest.MapFS{ + "main.tf": &fstest.MapFile{Data: []byte(`resource "root" "r" { x = 1 }`)}, + "modules/child/main.tf": &fstest.MapFile{Data: []byte(`resource "child" "c" { + y = 1 +}`)}, + "modules/child/override.tf": &fstest.MapFile{Data: []byte(`resource "child" "c" { + y = 42 +}`)}, + } + result, err := mergeOverrideFiles(fsys) + require.NoError(t, err) + + // Root file should be unchanged (no overrides in root). + rootContent := readFile(t, result, "main.tf") + assert.Contains(t, string(rootContent), "x = 1") + + // Child module should have merged content. + childContent := readFile(t, result, "modules/child/main.tf") + assert.Contains(t, string(childContent), "y = 42") + + // Override file in subdirectory should be hidden. + _, err = result.Open("modules/child/override.tf") + require.Error(t, err) + assert.ErrorIs(t, err, fs.ErrNotExist) + }) +} diff --git a/overridefs.go b/overridefs.go new file mode 100644 index 0000000..6a33b31 --- /dev/null +++ b/overridefs.go @@ -0,0 +1,94 @@ +package preview + +import ( + "bytes" + "io/fs" + "path" + "time" +) + +// overrideFS wraps a base fs.FS, serving modified content for merged files +// and hiding consumed override files. +type overrideFS struct { + base fs.FS + replaced map[string][]byte // path -> merged content + hidden map[string]bool // paths to exclude +} + +func (o *overrideFS) Open(name string) (fs.File, error) { + if o.hidden[name] { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} + } + + if content, ok := o.replaced[name]; ok { + return &memFile{ + name: path.Base(name), // Stat().Name() returns base name + content: bytes.NewReader(content), + size: int64(len(content)), + }, nil + } + + f, err := o.base.Open(name) + if err != nil { + return nil, err + } + + // If this is a directory, wrap it to filter hidden entries. + if info, err := f.Stat(); err == nil && info.IsDir() { + if rdf, ok := f.(fs.ReadDirFile); ok { + return &filteredDir{ReadDirFile: rdf, hidden: o.hidden, dirPath: name}, nil + } + } + + return f, nil +} + +// memFile is an in-memory file that implements fs.File. +type memFile struct { + name string + content *bytes.Reader + size int64 +} + +func (f *memFile) Stat() (fs.FileInfo, error) { + return &memFileInfo{name: f.name, size: f.size}, nil +} +func (f *memFile) Read(b []byte) (int, error) { return f.content.Read(b) } +func (f *memFile) Close() error { return nil } + +// memFileInfo implements fs.FileInfo for in-memory files. +type memFileInfo struct { + name string + size int64 +} + +func (fi *memFileInfo) Name() string { return fi.name } +func (fi *memFileInfo) Size() int64 { return fi.size } +func (fi *memFileInfo) Mode() fs.FileMode { return 0o444 } +func (fi *memFileInfo) ModTime() time.Time { return time.Time{} } +func (fi *memFileInfo) IsDir() bool { return false } +func (fi *memFileInfo) Sys() any { return nil } + +// filteredDir wraps a ReadDirFile to filter out hidden entries. +type filteredDir struct { + fs.ReadDirFile + hidden map[string]bool + dirPath string +} + +func (d *filteredDir) ReadDir(n int) ([]fs.DirEntry, error) { + entries, err := d.ReadDirFile.ReadDir(n) + if err != nil { + return nil, err + } + + filtered := make([]fs.DirEntry, 0, len(entries)) + for _, entry := range entries { + fullPath := path.Join(d.dirPath, entry.Name()) + if !d.hidden[fullPath] { + filtered = append(filtered, entry) + } + } + + return filtered, nil +} diff --git a/preview.go b/preview.go index f157509..55a8e92 100644 --- a/preview.go +++ b/preview.go @@ -210,6 +210,22 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn variableValues[k] = v } + // Merge override files into primary files before parsing, so Trivy + // sees pre-merged content with no duplicate blocks. This replicates + // Terraform's override file semantics. + // + // TODO: I'd be nice if Trivy did it for us. + dir, err = mergeOverrideFiles(dir) + if err != nil { + return nil, hcl.Diagnostics{ + { + Severity: hcl.DiagError, + Summary: "Process terraform override files", + Detail: err.Error(), + }, + } + } + // moduleSource is "" for a local module p := parser.New(dir, "", parser.OptionWithLogger(logger), diff --git a/preview_test.go b/preview_test.go index 5bd4821..561f920 100644 --- a/preview_test.go +++ b/preview_test.go @@ -608,6 +608,23 @@ func Test_Extract(t *testing.T) { "unknown": av().def(cty.NilVal), }, }, + { + name: "override", + dir: "override", + params: map[string]assertParam{ + "region": ap().value("ap").def("ap").optVals("ap"), + "size": ap().value("100").def("100").optVals("10", "50", "100"), + }, + presets: map[string]assertPreset{ + "dev-override": aPre().value("region", "ap"), + }, + expTags: map[string]string{ + "env": "production", + }, + variables: map[string]assertVariable{ + "string_to_number": av().def(cty.NumberIntVal(40)).typeEq(cty.Number), + }, + }, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() diff --git a/testdata/override/a_override.tf b/testdata/override/a_override.tf new file mode 100644 index 0000000..794d65f --- /dev/null +++ b/testdata/override/a_override.tf @@ -0,0 +1,46 @@ +# Override region's default (will be overridden again by b_override). +data "coder_parameter" "region" { + default = "eu" +} + +# Override size's options. +data "coder_parameter" "size" { + option { + name = "10GB" + value = 10 + } + option { + name = "50GB" + value = 50 + } +} + +# Override size again in the same file — adds 100GB option that makes the +# default valid. +data "coder_parameter" "size" { + option { + name = "10GB" + value = 10 + } + option { + name = "50GB" + value = 50 + } + option { + name = "100GB" + value = 100 + } +} + +# Override tags. +data "coder_workspace_tags" "tags" { + tags = { + "env" = "production" + } +} + +# Override variable. +variable "string_to_number" { + type = number + default = 40 +} diff --git a/testdata/override/main.tf b/testdata/override/main.tf new file mode 100644 index 0000000..58f6fb8 --- /dev/null +++ b/testdata/override/main.tf @@ -0,0 +1,57 @@ +terraform { + required_providers { + coder = { + source = "coder/coder" + version = "2.4.0-pre0" + } + } +} + +data "coder_parameter" "region" { + name = "region" + type = "string" + default = "us" + + option { + name = "US" + value = "us" + } + option { + name = "EU" + value = "eu" + } +} + +data "coder_parameter" "size" { + name = "size" + type = "number" + # Invalid value should become valid once the options are overridden. + default = 100 + + option { + name = "10GB" + value = 10 + } + option { + name = "20GB" + value = 20 + } +} + +data "coder_workspace_preset" "dev" { + name = "dev" + parameters = { + region = "us" + } +} + +data "coder_workspace_tags" "tags" { + tags = { + "env" = "staging" + } +} + +variable "string_to_number" { + type = string + default = "foo" +} diff --git a/testdata/override/override.tf b/testdata/override/override.tf new file mode 100644 index 0000000..1885d71 --- /dev/null +++ b/testdata/override/override.tf @@ -0,0 +1,17 @@ +# Override region's default again (sequential: us -> eu -> ap). +data "coder_parameter" "region" { + default = "ap" + + option { + name = "AP" + value = "ap" + } +} + +# Override preset. +data "coder_workspace_preset" "dev" { + name = "dev-override" + parameters = { + region = "ap" + } +} From 1c41920104a9f65740766d911ab8716d669435f8 Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Tue, 3 Mar 2026 18:36:00 -0800 Subject: [PATCH 2/4] Update/add comments --- override.go | 2 ++ preview.go | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/override.go b/override.go index 8b75fd8..3d54694 100644 --- a/override.go +++ b/override.go @@ -135,6 +135,8 @@ func mergeOverrideFiles(original fs.FS) (fs.FS, error) { } } if !matched { + // Terraform requires every override block to have a corresponding + // primary block — override files can only modify, not create. return nil, fmt.Errorf("override block %s in %s has no matching block in a primary configuration file", key, path) } } diff --git a/preview.go b/preview.go index 55a8e92..44847e9 100644 --- a/preview.go +++ b/preview.go @@ -211,10 +211,10 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn } // Merge override files into primary files before parsing, so Trivy - // sees pre-merged content with no duplicate blocks. This replicates + // sees post-merge content with no duplicate blocks. This replicates // Terraform's override file semantics. // - // TODO: I'd be nice if Trivy did it for us. + // TODO: It'd be nice if Trivy did this for us. dir, err = mergeOverrideFiles(dir) if err != nil { return nil, hcl.Diagnostics{ From 372a892f3820cb23adcc20b2cdfef1e97dded2ad Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Tue, 3 Mar 2026 19:57:02 -0800 Subject: [PATCH 3/4] add tests for dynamic<->static block overrides --- override_test.go | 83 +++++++++++++++++++++++++++++++++ preview_test.go | 7 ++- testdata/override/a_override.tf | 23 +++++++++ testdata/override/main.tf | 36 ++++++++++++++ 4 files changed, 147 insertions(+), 2 deletions(-) diff --git a/override_test.go b/override_test.go index 82e87d7..49d5c51 100644 --- a/override_test.go +++ b/override_test.go @@ -185,6 +185,89 @@ func TestMergeBlock(t *testing.T) { assert.Empty(t, blocks[0].Labels()) }) + t.Run("MixedStaticDynamicSuppression", func(t *testing.T) { + t.Parallel() + primary := parseBlock(t, `resource "a" "b" { + option { + name = "static" + } + dynamic "option" { + for_each = var.list + content { + name = option.value + } + } +}`) + override := parseBlock(t, `resource "a" "b" { + option { + name = "replaced" + } +}`) + mergeBlock(primary, override) + + blocks := primary.Body().Blocks() + require.Len(t, blocks, 1) + assert.Equal(t, "option", blocks[0].Type()) + assert.Empty(t, blocks[0].Labels()) + }) + + t.Run("MixedStaticDynamicSuppressionByDynamic", func(t *testing.T) { + t.Parallel() + primary := parseBlock(t, `resource "a" "b" { + option { + name = "static" + } + dynamic "option" { + for_each = var.list + content { + name = option.value + } + } +}`) + override := parseBlock(t, `resource "a" "b" { + dynamic "option" { + for_each = var.other + content { + name = option.value + } + } +}`) + mergeBlock(primary, override) + + blocks := primary.Body().Blocks() + require.Len(t, blocks, 1) + assert.Equal(t, "dynamic", blocks[0].Type()) + require.Len(t, blocks[0].Labels(), 1) + assert.Equal(t, "option", blocks[0].Labels()[0]) + }) + + t.Run("StaticSuppressionByMixedOverride", func(t *testing.T) { + t.Parallel() + primary := parseBlock(t, `resource "a" "b" { + option { + name = "old" + } +}`) + override := parseBlock(t, `resource "a" "b" { + option { + name = "static" + } + dynamic "option" { + for_each = var.list + content { + name = option.value + } + } +}`) + mergeBlock(primary, override) + + blocks := primary.Body().Blocks() + require.Len(t, blocks, 2) + assert.Equal(t, "option", blocks[0].Type()) + assert.Equal(t, "dynamic", blocks[1].Type()) + assert.Equal(t, "option", blocks[1].Labels()[0]) + }) + t.Run("NoNestedBlocksInOverride", func(t *testing.T) { t.Parallel() primary := parseBlock(t, `resource "a" "b" { diff --git a/preview_test.go b/preview_test.go index 561f920..d6029b2 100644 --- a/preview_test.go +++ b/preview_test.go @@ -612,8 +612,10 @@ func Test_Extract(t *testing.T) { name: "override", dir: "override", params: map[string]assertParam{ - "region": ap().value("ap").def("ap").optVals("ap"), - "size": ap().value("100").def("100").optVals("10", "50", "100"), + "region": ap().value("ap").def("ap").optVals("ap"), + "size": ap().value("100").def("100").optVals("10", "50", "100"), + "static_to_dynamic": ap().value("a").def("a").optVals("a", "b", "c"), + "dynamic_to_static": ap().value("x").def("x").optVals("x", "y"), }, presets: map[string]assertPreset{ "dev-override": aPre().value("region", "ap"), @@ -623,6 +625,7 @@ func Test_Extract(t *testing.T) { }, variables: map[string]assertVariable{ "string_to_number": av().def(cty.NumberIntVal(40)).typeEq(cty.Number), + "zones": av().def(cty.SetVal([]cty.Value{cty.StringVal("a"), cty.StringVal("b"), cty.StringVal("c")})).typeEq(cty.Set(cty.String)), }, }, } { diff --git a/testdata/override/a_override.tf b/testdata/override/a_override.tf index 794d65f..13123e7 100644 --- a/testdata/override/a_override.tf +++ b/testdata/override/a_override.tf @@ -39,6 +39,29 @@ data "coder_workspace_tags" "tags" { } } +# Override static options with dynamic. +data "coder_parameter" "static_to_dynamic" { + dynamic "option" { + for_each = var.zones + content { + name = option.value + value = option.value + } + } +} + +# Override dynamic options with static. +data "coder_parameter" "dynamic_to_static" { + option { + name = "X" + value = "x" + } + option { + name = "Y" + value = "y" + } +} + # Override variable. variable "string_to_number" { type = number diff --git a/testdata/override/main.tf b/testdata/override/main.tf index 58f6fb8..c84104d 100644 --- a/testdata/override/main.tf +++ b/testdata/override/main.tf @@ -51,6 +51,42 @@ data "coder_workspace_tags" "tags" { } } +variable "zones" { + type = set(string) + default = ["a", "b", "c"] +} + +# Static options, will be overridden by dynamic "option" in a_override. +data "coder_parameter" "static_to_dynamic" { + name = "static_to_dynamic" + type = "string" + default = "a" + + option { + name = "A" + value = "a" + } + option { + name = "B" + value = "b" + } +} + +# Dynamic options, will be overridden by static option blocks in a_override. +data "coder_parameter" "dynamic_to_static" { + name = "dynamic_to_static" + type = "string" + default = "x" + + dynamic "option" { + for_each = var.zones + content { + name = option.value + value = option.value + } + } +} + variable "string_to_number" { type = string default = "foo" From 121a8bc7df8dacd06fcaa884071d78691c5f2cc2 Mon Sep 17 00:00:00 2001 From: George Katsitadze Date: Wed, 4 Mar 2026 12:10:29 -0800 Subject: [PATCH 4/4] Add an empty override block unit test --- override_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/override_test.go b/override_test.go index 49d5c51..530d4b4 100644 --- a/override_test.go +++ b/override_test.go @@ -288,6 +288,24 @@ func TestMergeBlock(t *testing.T) { // Attribute should still be overridden. assert.Equal(t, "99", attrValue(t, primary.Body().Attributes(), "x")) }) + + t.Run("EmptyOverride", func(t *testing.T) { + t.Parallel() + primary := parseBlock(t, `resource "a" "b" { + x = 1 + nested { + y = 2 + } +}`) + override := parseBlock(t, `resource "a" "b" {}`) + mergeBlock(primary, override) + + // Nothing should change — attributes and blocks preserved. + assert.Equal(t, "1", attrValue(t, primary.Body().Attributes(), "x")) + blocks := primary.Body().Blocks() + require.Len(t, blocks, 1) + assert.Equal(t, "nested", blocks[0].Type()) + }) } // readFile reads a file from an fs.FS using Open+Read (since overrideFS