From a117f86b983476953ed4a4c3b8dedc30b832507e Mon Sep 17 00:00:00 2001 From: Oleksandr Levchenkov Date: Thu, 28 Nov 2024 14:22:42 +0200 Subject: [PATCH] Fix: Change warning to error when incorrect type is used for mocking a resource (#2220) Signed-off-by: pooriaghaedi Signed-off-by: Pooria Ghaedi <36617391+pooriaghaedi@users.noreply.github.com> Signed-off-by: ollevche Co-authored-by: pooriaghaedi Co-authored-by: Pooria Ghaedi <36617391+pooriaghaedi@users.noreply.github.com> --- CHANGELOG.md | 1 + .../configs/hcl2shim/mock_value_composer.go | 48 ++++++++++--------- .../hcl2shim/mock_value_composer_test.go | 38 +++++++++------ 3 files changed, 50 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 357c06a4b6..c419ae4d2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ ENHANCEMENTS: * Improved performance for large graphs when debug logs are not enabled. ([#1810](https://github.com/opentofu/opentofu/pull/1810)) * Improved performance for large graphs with many submodules. ([#1809](https://github.com/opentofu/opentofu/pull/1809)) * Extended trace logging for HTTP backend, including request and response bodies. ([#2120](https://github.com/opentofu/opentofu/pull/2120)) +* `tofu test` now throws errors instead of warnings for invalid override and mock fields. ([#2220](https://github.com/opentofu/opentofu/pull/2220)) BUG FIXES: * `templatefile` no longer crashes if the given filename is derived from a sensitive value. ([#1801](https://github.com/opentofu/opentofu/issues/1801)) diff --git a/internal/configs/hcl2shim/mock_value_composer.go b/internal/configs/hcl2shim/mock_value_composer.go index fd0e32b750..7ad6eabcf7 100644 --- a/internal/configs/hcl2shim/mock_value_composer.go +++ b/internal/configs/hcl2shim/mock_value_composer.go @@ -63,8 +63,8 @@ func (mvc MockValueComposer) ComposeBySchema(schema *configschema.Block, config for k := range defaults { if _, ok := impliedTypes[k]; !ok { diags = diags.Append(tfdiags.WholeContainingBody( - tfdiags.Warning, - fmt.Sprintf("Ignored mock/override field `%v`", k), + tfdiags.Error, + fmt.Sprintf("Invalid override for block field `%v`", k), "The field is unknown. Please, ensure it is a part of resource definition.", )) } @@ -76,16 +76,6 @@ func (mvc MockValueComposer) ComposeBySchema(schema *configschema.Block, config func (mvc MockValueComposer) composeMockValueForAttributes(schema *configschema.Block, configMap map[string]cty.Value, defaults map[string]cty.Value) (map[string]cty.Value, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics - addPotentialDefaultsWarning := func(key, description string) { - if _, ok := defaults[key]; ok { - diags = diags.Append(tfdiags.WholeContainingBody( - tfdiags.Warning, - fmt.Sprintf("Ignored mock/override field `%v`", key), - description, - )) - } - } - mockAttrs := make(map[string]cty.Value) impliedTypes := schema.ImpliedType().AttributeTypes() @@ -96,8 +86,15 @@ func (mvc MockValueComposer) composeMockValueForAttributes(schema *configschema. // If the value present in configuration - just use it. if cv, ok := configMap[k]; ok && !cv.IsNull() { + if _, ok := defaults[k]; ok { + diags = diags.Append(tfdiags.WholeContainingBody( + tfdiags.Error, + fmt.Sprintf("Invalid mock/override field `%v`", k), + "The field is ignored since overriding configuration values is not allowed.", + )) + continue + } mockAttrs[k] = cv - addPotentialDefaultsWarning(k, "The field is ignored since overriding configuration values is not allowed.") continue } @@ -105,7 +102,13 @@ func (mvc MockValueComposer) composeMockValueForAttributes(schema *configschema. // so we set them from configuration only. if !attr.Computed { mockAttrs[k] = cty.NullVal(attr.Type) - addPotentialDefaultsWarning(k, "The field is ignored since overriding non-computed fields is not allowed.") + if _, ok := defaults[k]; ok { + diags = diags.Append(tfdiags.WholeContainingBody( + tfdiags.Error, + fmt.Sprintf("Non-computed field `%v` is not allowed to be overridden", k), + "Overriding non-computed fields is not allowed, so this field cannot be processed.", + )) + } continue } @@ -115,8 +118,8 @@ func (mvc MockValueComposer) composeMockValueForAttributes(schema *configschema. converted, err := convert.Convert(ov, attr.Type) if err != nil { diags = diags.Append(tfdiags.WholeContainingBody( - tfdiags.Warning, - fmt.Sprintf("Ignored mock/override field `%v`", k), + tfdiags.Error, + fmt.Sprintf("Invalid mock/override field `%v`", k), fmt.Sprintf("Values provided for override / mock must match resource fields types: %v.", tfdiags.FormatError(err)), )) continue @@ -172,12 +175,12 @@ func (mvc MockValueComposer) composeMockValueForBlocks(schema *configschema.Bloc defaultVal, hasDefaultVal := defaults[k] if hasDefaultVal && !defaultVal.Type().IsObjectType() { - hasDefaultVal = false diags = diags.Append(tfdiags.WholeContainingBody( - tfdiags.Warning, - fmt.Sprintf("Ignored mock/override field `%v`", k), + tfdiags.Error, + fmt.Sprintf("Invalid override for block field `%v`", k), fmt.Sprintf("Blocks can be overridden only by objects, got `%s`", defaultVal.Type().FriendlyName()), )) + continue } // We must keep blocks the same as it defined in configuration, @@ -187,10 +190,11 @@ func (mvc MockValueComposer) composeMockValueForBlocks(schema *configschema.Bloc if hasDefaultVal { diags = diags.Append(tfdiags.WholeContainingBody( - tfdiags.Warning, - fmt.Sprintf("Ignored mock/override field `%v`", k), - "Cannot override block value, because it's not present in configuration.", + tfdiags.Error, + fmt.Sprintf("Invalid override for block field `%v`", k), + "Cannot overridde block value, because it's not present in configuration.", )) + continue } continue diff --git a/internal/configs/hcl2shim/mock_value_composer_test.go b/internal/configs/hcl2shim/mock_value_composer_test.go index a3725030db..00db0a66b7 100644 --- a/internal/configs/hcl2shim/mock_value_composer_test.go +++ b/internal/configs/hcl2shim/mock_value_composer_test.go @@ -14,12 +14,11 @@ func TestComposeMockValueBySchema(t *testing.T) { t.Parallel() tests := map[string]struct { - schema *configschema.Block - config cty.Value - defaults map[string]cty.Value - wantVal cty.Value - wantWarning bool - wantError bool + schema *configschema.Block + config cty.Value + defaults map[string]cty.Value + wantVal cty.Value + wantError bool }{ "diff-props-in-root-attributes": { schema: &configschema.Block{ @@ -473,10 +472,8 @@ func TestComposeMockValueBySchema(t *testing.T) { }), }), defaults: map[string]cty.Value{ - "useConfigValue": cty.StringVal("iAmFromDefaults"), "useDefaultsValue": cty.StringVal("iAmFromDefaults"), "nested": cty.ObjectVal(map[string]cty.Value{ - "useConfigValue": cty.StringVal("iAmFromDefaults"), "useDefaultsValue": cty.StringVal("iAmFromDefaults"), }), }, @@ -492,7 +489,6 @@ func TestComposeMockValueBySchema(t *testing.T) { }), }), }), - wantWarning: true, // ignored value in defaults }, "type-conversion": { schema: &configschema.Block{ @@ -514,6 +510,24 @@ func TestComposeMockValueBySchema(t *testing.T) { }), }), }, + "config-override": { + schema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "config-field": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + config: cty.ObjectVal(map[string]cty.Value{ + "config-field": cty.StringVal("iAmFromConfig"), + }), + defaults: map[string]cty.Value{ + "config-field": cty.StringVal("str"), + }, + wantError: true, + }, } for name, test := range tests { @@ -530,12 +544,6 @@ func TestComposeMockValueBySchema(t *testing.T) { case !test.wantError && gotDiags.HasErrors(): t.Fatalf("Got unexpected error diags: %v", gotDiags.ErrWithWarnings()) - case test.wantWarning && len(gotDiags) == 0: - t.Fatalf("Expected warning in diags, but none returned") - - case !test.wantWarning && len(gotDiags) != 0: - t.Fatalf("Got unexpected diags: %v", gotDiags.ErrWithWarnings()) - case !test.wantVal.RawEquals(gotVal): t.Fatalf("Got unexpected value: %v", gotVal.GoString()) }