diff --git a/configs/module_call.go b/configs/module_call.go index befccb28a..86f840216 100644 --- a/configs/module_call.go +++ b/configs/module_call.go @@ -12,6 +12,7 @@ type ModuleCall struct { SourceAddr string SourceAddrRange hcl.Range + SourceSet bool Config hcl.Body @@ -25,13 +26,18 @@ type ModuleCall struct { DeclRange hcl.Range } -func decodeModuleBlock(block *hcl.Block) (*ModuleCall, hcl.Diagnostics) { +func decodeModuleBlock(block *hcl.Block, override bool) (*ModuleCall, hcl.Diagnostics) { mc := &ModuleCall{ Name: block.Labels[0], DeclRange: block.DefRange, } - content, remain, diags := block.Body.PartialContent(moduleBlockSchema) + schema := moduleBlockSchema + if override { + schema = schemaForOverrides(schema) + } + + content, remain, diags := block.Body.PartialContent(schema) mc.Config = remain if !hclsyntax.ValidIdentifier(mc.Name) { @@ -47,6 +53,7 @@ func decodeModuleBlock(block *hcl.Block) (*ModuleCall, hcl.Diagnostics) { valDiags := gohcl.DecodeExpression(attr.Expr, nil, &mc.SourceAddr) diags = append(diags, valDiags...) mc.SourceAddrRange = attr.Expr.Range() + mc.SourceSet = true } if attr, exists := content.Attributes["version"]; exists { diff --git a/configs/module_merge.go b/configs/module_merge.go index d6adc5442..2e5ed3249 100644 --- a/configs/module_merge.go +++ b/configs/module_merge.go @@ -45,8 +45,9 @@ func mergeProviderVersionConstraints(recv map[string][]VersionConstraint, ovrd [ func (v *Variable) merge(ov *Variable) hcl.Diagnostics { var diags hcl.Diagnostics - if ov.Description != "" { + if ov.DescriptionSet { v.Description = ov.Description + v.DescriptionSet = ov.DescriptionSet } if ov.Default != cty.NilVal { v.Default = ov.Default @@ -80,10 +81,9 @@ func (o *Output) merge(oo *Output) hcl.Diagnostics { if oo.Expr != nil { o.Expr = oo.Expr } - if oo.Sensitive { - // Since this is just a bool, we can't distinguish false from unset - // and so the override can only make the output _more_ sensitive. + if oo.SensitiveSet { o.Sensitive = oo.Sensitive + o.SensitiveSet = oo.SensitiveSet } // We don't allow depends_on to be overridden because that is likely to @@ -103,9 +103,10 @@ func (o *Output) merge(oo *Output) hcl.Diagnostics { func (mc *ModuleCall) merge(omc *ModuleCall) hcl.Diagnostics { var diags hcl.Diagnostics - if omc.SourceAddr != "" { + if omc.SourceSet { mc.SourceAddr = omc.SourceAddr mc.SourceAddrRange = omc.SourceAddrRange + mc.SourceSet = omc.SourceSet } if omc.Count != nil { @@ -145,9 +146,9 @@ func (r *ManagedResource) merge(or *ManagedResource) hcl.Diagnostics { if or.Count != nil { r.Count = or.Count } - if or.CreateBeforeDestroy { - // We can't distinguish false from unset here + if or.CreateBeforeDestroySet { r.CreateBeforeDestroy = or.CreateBeforeDestroy + r.CreateBeforeDestroySet = or.CreateBeforeDestroySet } if or.ForEach != nil { r.ForEach = or.ForEach @@ -155,9 +156,9 @@ func (r *ManagedResource) merge(or *ManagedResource) hcl.Diagnostics { if len(or.IgnoreChanges) != 0 { r.IgnoreChanges = or.IgnoreChanges } - if or.PreventDestroy { - // We can't distinguish false from unset here + if or.PreventDestroySet { r.PreventDestroy = or.PreventDestroy + r.PreventDestroySet = or.PreventDestroySet } if or.ProviderConfigRef != nil { r.ProviderConfigRef = or.ProviderConfigRef diff --git a/configs/module_merge_test.go b/configs/module_merge_test.go new file mode 100644 index 000000000..6e74fd402 --- /dev/null +++ b/configs/module_merge_test.go @@ -0,0 +1,61 @@ +package configs + +import ( + "testing" + + "github.com/hashicorp/hcl2/hcl" + "github.com/zclconf/go-cty/cty" +) + +func TestModuleOverrideVariable(t *testing.T) { + mod, diags := testModuleFromDir("test-fixtures/valid-modules/override-variable") + assertNoDiagnostics(t, diags) + if mod == nil { + t.Fatalf("module is nil") + } + + got := mod.Variables + want := map[string]*Variable{ + "fully_overridden": { + Name: "fully_overridden", + Description: "b_override description", + DescriptionSet: true, + Default: cty.StringVal("b_override"), + TypeHint: TypeHintString, + DeclRange: hcl.Range{ + Filename: "test-fixtures/valid-modules/override-variable/primary.tf", + Start: hcl.Pos{ + Line: 1, + Column: 1, + Byte: 0, + }, + End: hcl.Pos{ + Line: 1, + Column: 28, + Byte: 27, + }, + }, + }, + "partially_overridden": { + Name: "partially_overridden", + Description: "base description", + DescriptionSet: true, + Default: cty.StringVal("b_override partial"), + TypeHint: TypeHintString, + DeclRange: hcl.Range{ + Filename: "test-fixtures/valid-modules/override-variable/primary.tf", + Start: hcl.Pos{ + Line: 7, + Column: 1, + Byte: 103, + }, + End: hcl.Pos{ + Line: 7, + Column: 32, + Byte: 134, + }, + }, + }, + } + assertResultDeepEqual(t, got, want) +} diff --git a/configs/named_values.go b/configs/named_values.go index 052b04017..42ae4750d 100644 --- a/configs/named_values.go +++ b/configs/named_values.go @@ -19,6 +19,8 @@ type Variable struct { Default cty.Value TypeHint VariableTypeHint + DescriptionSet bool + DeclRange hcl.Range } @@ -56,6 +58,7 @@ func decodeVariableBlock(block *hcl.Block) (*Variable, hcl.Diagnostics) { if attr, exists := content.Attributes["description"]; exists { valDiags := gohcl.DecodeExpression(attr.Expr, nil, &v.Description) diags = append(diags, valDiags...) + v.DescriptionSet = true } if attr, exists := content.Attributes["default"]; exists { @@ -106,16 +109,24 @@ type Output struct { DependsOn []hcl.Traversal Sensitive bool + DescriptionSet bool + SensitiveSet bool + DeclRange hcl.Range } -func decodeOutputBlock(block *hcl.Block) (*Output, hcl.Diagnostics) { +func decodeOutputBlock(block *hcl.Block, override bool) (*Output, hcl.Diagnostics) { o := &Output{ Name: block.Labels[0], DeclRange: block.DefRange, } - content, diags := block.Body.Content(outputBlockSchema) + schema := outputBlockSchema + if override { + schema = schemaForOverrides(schema) + } + + content, diags := block.Body.Content(schema) if !hclsyntax.ValidIdentifier(o.Name) { diags = append(diags, &hcl.Diagnostic{ @@ -129,6 +140,7 @@ func decodeOutputBlock(block *hcl.Block) (*Output, hcl.Diagnostics) { if attr, exists := content.Attributes["description"]; exists { valDiags := gohcl.DecodeExpression(attr.Expr, nil, &o.Description) diags = append(diags, valDiags...) + o.DescriptionSet = true } if attr, exists := content.Attributes["value"]; exists { @@ -138,6 +150,7 @@ func decodeOutputBlock(block *hcl.Block) (*Output, hcl.Diagnostics) { if attr, exists := content.Attributes["sensitive"]; exists { valDiags := gohcl.DecodeExpression(attr.Expr, nil, &o.Sensitive) diags = append(diags, valDiags...) + o.SensitiveSet = true } if attr, exists := content.Attributes["depends_on"]; exists { diff --git a/configs/parser_config.go b/configs/parser_config.go index 16eb77c34..c7fc33fd9 100644 --- a/configs/parser_config.go +++ b/configs/parser_config.go @@ -19,6 +19,18 @@ import ( // This method wraps LoadHCLFile, and so it inherits the syntax selection // behaviors documented for that method. func (p *Parser) LoadConfigFile(path string) (*File, hcl.Diagnostics) { + return p.loadConfigFile(path, false) +} + +// LoadConfigFileOverride is the same as LoadConfigFile except that it relaxes +// certain required attribute constraints in order to interpret the given +// file as an overrides file. +func (p *Parser) LoadConfigFileOverride(path string) (*File, hcl.Diagnostics) { + return p.loadConfigFile(path, true) +} + +func (p *Parser) loadConfigFile(path string, override bool) (*File, hcl.Diagnostics) { + body, diags := p.LoadHCLFile(path) if body == nil { return nil, diags @@ -86,14 +98,14 @@ func (p *Parser) LoadConfigFile(path string) (*File, hcl.Diagnostics) { file.Locals = append(file.Locals, defs...) case "output": - cfg, cfgDiags := decodeOutputBlock(block) + cfg, cfgDiags := decodeOutputBlock(block, override) diags = append(diags, cfgDiags...) if cfg != nil { file.Outputs = append(file.Outputs, cfg) } case "module": - cfg, cfgDiags := decodeModuleBlock(block) + cfg, cfgDiags := decodeModuleBlock(block, override) diags = append(diags, cfgDiags...) if cfg != nil { file.ModuleCalls = append(file.ModuleCalls, cfg) diff --git a/configs/parser_config_dir.go b/configs/parser_config_dir.go index 57c72b604..fad3a9837 100644 --- a/configs/parser_config_dir.go +++ b/configs/parser_config_dir.go @@ -33,9 +33,9 @@ func (p *Parser) LoadConfigDir(path string) (*Module, hcl.Diagnostics) { return nil, diags } - primary, fDiags := p.loadFiles(primaryPaths) + primary, fDiags := p.loadFiles(primaryPaths, false) diags = append(diags, fDiags...) - override, fDiags := p.loadFiles(overridePaths) + override, fDiags := p.loadFiles(overridePaths, true) diags = append(diags, fDiags...) mod, modDiags := NewModule(primary, override) @@ -52,12 +52,18 @@ func (p *Parser) IsConfigDir(path string) bool { return (len(primaryPaths) + len(overridePaths)) > 0 } -func (p *Parser) loadFiles(paths []string) ([]*File, hcl.Diagnostics) { +func (p *Parser) loadFiles(paths []string, override bool) ([]*File, hcl.Diagnostics) { var files []*File var diags hcl.Diagnostics for _, path := range paths { - f, fDiags := p.LoadConfigFile(path) + var f *File + var fDiags hcl.Diagnostics + if override { + f, fDiags = p.LoadConfigFileOverride(path) + } else { + f, fDiags = p.LoadConfigFile(path) + } diags = append(diags, fDiags...) if f != nil { files = append(files, f) diff --git a/configs/parser_test.go b/configs/parser_test.go index 316a09e5d..f5f1edba2 100644 --- a/configs/parser_test.go +++ b/configs/parser_test.go @@ -3,7 +3,12 @@ package configs import ( "os" "path" + "reflect" + "testing" + "github.com/davecgh/go-spew/spew" + + "github.com/hashicorp/hcl2/hcl" "github.com/spf13/afero" ) @@ -29,3 +34,62 @@ func testParser(files map[string]string) *Parser { return NewParser(fs) } + +// testModuleFromFile reads a single file, wraps it in a module, and returns +// it. This is a helper for use in unit tests. +func testModuleFromFile(filename string) (*Module, hcl.Diagnostics) { + parser := NewParser(nil) + f, diags := parser.LoadConfigFile(filename) + mod, modDiags := NewModule([]*File{f}, nil) + diags = append(diags, modDiags...) + return mod, modDiags +} + +// testModuleFromDir reads configuration from the given directory path as +// a module and returns it. This is a helper for use in unit tests. +func testModuleFromDir(path string) (*Module, hcl.Diagnostics) { + parser := NewParser(nil) + return parser.LoadConfigDir(path) +} + +func assertNoDiagnostics(t *testing.T, diags hcl.Diagnostics) bool { + t.Helper() + return assertDiagnosticCount(t, diags, 0) +} + +func assertDiagnosticCount(t *testing.T, diags hcl.Diagnostics, want int) bool { + t.Helper() + if len(diags) != 0 { + t.Errorf("wrong number of diagnostics %d; want %d", len(diags), want) + for _, diag := range diags { + t.Logf("- %s", diag) + } + return true + } + return false +} + +func assertDiagnosticSummary(t *testing.T, diags hcl.Diagnostics, want string) bool { + t.Helper() + + for _, diag := range diags { + if diag.Summary == want { + return false + } + } + + t.Errorf("missing diagnostic summary %q", want) + for _, diag := range diags { + t.Logf("- %s", diag) + } + return true +} + +func assertResultDeepEqual(t *testing.T, got, want interface{}) bool { + t.Helper() + if !reflect.DeepEqual(got, want) { + t.Errorf("wrong result\ngot: %swant: %s", spew.Sdump(got), spew.Sdump(want)) + return true + } + return false +} diff --git a/configs/resource.go b/configs/resource.go index b66d12e38..3892c86b9 100644 --- a/configs/resource.go +++ b/configs/resource.go @@ -27,6 +27,9 @@ type ManagedResource struct { PreventDestroy bool IgnoreChanges []hcl.Traversal + CreateBeforeDestroySet bool + PreventDestroySet bool + DeclRange hcl.Range TypeRange hcl.Range } @@ -105,11 +108,13 @@ func decodeResourceBlock(block *hcl.Block) (*ManagedResource, hcl.Diagnostics) { if attr, exists := lcContent.Attributes["create_before_destroy"]; exists { valDiags := gohcl.DecodeExpression(attr.Expr, nil, &r.CreateBeforeDestroy) diags = append(diags, valDiags...) + r.CreateBeforeDestroySet = true } if attr, exists := lcContent.Attributes["prevent_destroy"]; exists { valDiags := gohcl.DecodeExpression(attr.Expr, nil, &r.PreventDestroy) diags = append(diags, valDiags...) + r.PreventDestroySet = true } if attr, exists := lcContent.Attributes["ignore_changes"]; exists { diff --git a/configs/test-fixtures/valid-modules/override-module/a_override.tf b/configs/test-fixtures/valid-modules/override-module/a_override.tf new file mode 100644 index 000000000..9e0403ff8 --- /dev/null +++ b/configs/test-fixtures/valid-modules/override-module/a_override.tf @@ -0,0 +1,5 @@ + +module "example" { + foo = "a_override foo" + new = "a_override new" +} diff --git a/configs/test-fixtures/valid-modules/override-module/b_override.tf b/configs/test-fixtures/valid-modules/override-module/b_override.tf new file mode 100644 index 000000000..98d5a3101 --- /dev/null +++ b/configs/test-fixtures/valid-modules/override-module/b_override.tf @@ -0,0 +1,5 @@ + +module "example" { + new = "b_override new" + newer = "b_override newer" +} diff --git a/configs/test-fixtures/valid-modules/override-module/primary.tf b/configs/test-fixtures/valid-modules/override-module/primary.tf new file mode 100644 index 000000000..0567e7867 --- /dev/null +++ b/configs/test-fixtures/valid-modules/override-module/primary.tf @@ -0,0 +1,7 @@ + +module "example" { + source = "./example2" + + kept = "primary kept" + foo = "primary foo" +} diff --git a/configs/test-fixtures/valid-modules/override-output-sensitive/override.tf b/configs/test-fixtures/valid-modules/override-output-sensitive/override.tf new file mode 100644 index 000000000..b965fc124 --- /dev/null +++ b/configs/test-fixtures/valid-modules/override-output-sensitive/override.tf @@ -0,0 +1,3 @@ +output "foo" { + sensitive = true +} diff --git a/configs/test-fixtures/valid-modules/override-output-sensitive/primary.tf b/configs/test-fixtures/valid-modules/override-output-sensitive/primary.tf new file mode 100644 index 000000000..13bd3a99b --- /dev/null +++ b/configs/test-fixtures/valid-modules/override-output-sensitive/primary.tf @@ -0,0 +1,3 @@ +output "foo" { + value = "Hello World" +} diff --git a/configs/util.go b/configs/util.go index 9594cbeee..002bb8cb8 100644 --- a/configs/util.go +++ b/configs/util.go @@ -16,3 +16,30 @@ func exprIsNativeQuotedString(expr hcl.Expression) bool { _, ok := expr.(*hclsyntax.TemplateExpr) return ok } + +// schemaForOverrides takes a *hcl.BodySchema and produces a new one that is +// equivalent except that any required attributes are forced to not be required. +// +// This is useful for dealing with "override" config files, which are allowed +// to omit things that they don't wish to override from the main configuration. +// +// The returned schema may have some pointers in common with the given schema, +// so neither the given schema nor the returned schema should be modified after +// using this function in order to avoid confusion. +// +// Overrides are rarely used, so it's recommended to just create the override +// schema on the fly only when it's needed, rather than storing it in a global +// variable as we tend to do for a primary schema. +func schemaForOverrides(schema *hcl.BodySchema) *hcl.BodySchema { + ret := &hcl.BodySchema{ + Attributes: make([]hcl.AttributeSchema, len(schema.Attributes)), + Blocks: schema.Blocks, + } + + for i, attrS := range schema.Attributes { + ret.Attributes[i] = attrS + ret.Attributes[i].Required = false + } + + return ret +}