From 99db18388a21082bcc09d3a66ee973d001e68a41 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 4 Nov 2020 11:14:53 -0500 Subject: [PATCH] return diagnostics from resource validation Return diagnostics rather than an error, since the EvalNodes have been removed and the caller has been updated. IgnoreWarnings was no longer used. --- terraform/eval_validate.go | 34 +++++++------------- terraform/eval_validate_test.go | 57 ++++++--------------------------- 2 files changed, 20 insertions(+), 71 deletions(-) diff --git a/terraform/eval_validate.go b/terraform/eval_validate.go index 182f4d1f96..a84aa3ac9f 100644 --- a/terraform/eval_validate.go +++ b/terraform/eval_validate.go @@ -252,10 +252,6 @@ type EvalValidateResource struct { Config *configs.Resource ProviderMetas map[addrs.Provider]*configs.ProviderMeta - // IgnoreWarnings means that warnings will not be passed through. This allows - // "just-in-time" passes of validation to continue execution through warnings. - IgnoreWarnings bool - // ConfigVal, if non-nil, will be updated with the value resulting from // evaluating the given configuration body. Since validation is performed // very early, this value is likely to contain lots of unknown values, @@ -264,12 +260,14 @@ type EvalValidateResource struct { ConfigVal *cty.Value } -func (n *EvalValidateResource) Validate(ctx EvalContext) error { - if n.ProviderSchema == nil || *n.ProviderSchema == nil { - return fmt.Errorf("EvalValidateResource has nil schema for %s", n.Addr) +func (n *EvalValidateResource) Validate(ctx EvalContext) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + if *n.ProviderSchema == nil { + diags = diags.Append(fmt.Errorf("EvalValidateResource has nil schema for %s", n.Addr)) + return diags } - var diags tfdiags.Diagnostics provider := *n.Provider cfg := *n.Config schema := *n.ProviderSchema @@ -351,13 +349,13 @@ func (n *EvalValidateResource) Validate(ctx EvalContext) error { Detail: fmt.Sprintf("The provider %s does not support resource type %q.", cfg.ProviderConfigAddr(), cfg.Type), Subject: &cfg.TypeRange, }) - return diags.Err() + return diags } configVal, _, valDiags := ctx.EvaluateBlock(cfg.Config, schema, nil, keyData) diags = diags.Append(valDiags) if valDiags.HasErrors() { - return diags.Err() + return diags } if cfg.Managed != nil { // can be nil only in tests with poorly-configured mocks @@ -394,13 +392,13 @@ func (n *EvalValidateResource) Validate(ctx EvalContext) error { Detail: fmt.Sprintf("The provider %s does not support data source %q.", cfg.ProviderConfigAddr(), cfg.Type), Subject: &cfg.TypeRange, }) - return diags.Err() + return diags } configVal, _, valDiags := ctx.EvaluateBlock(cfg.Config, schema, nil, keyData) diags = diags.Append(valDiags) if valDiags.HasErrors() { - return diags.Err() + return diags } req := providers.ValidateDataSourceConfigRequest{ @@ -412,17 +410,7 @@ func (n *EvalValidateResource) Validate(ctx EvalContext) error { diags = diags.Append(resp.Diagnostics.InConfigBody(cfg.Config)) } - if n.IgnoreWarnings { - // If we _only_ have warnings then we'll return nil. - if diags.HasErrors() { - return diags.NonFatalErr() - } - return nil - } else { - // We'll return an error if there are any diagnostics at all, even if - // some of them are warnings. - return diags.NonFatalErr() - } + return diags } func (n *EvalValidateResource) validateCount(ctx EvalContext, expr hcl.Expression) tfdiags.Diagnostics { diff --git a/terraform/eval_validate_test.go b/terraform/eval_validate_test.go index ce23b35964..5ff84fee1c 100644 --- a/terraform/eval_validate_test.go +++ b/terraform/eval_validate_test.go @@ -238,45 +238,6 @@ func TestEvalValidateResource_warningsAndErrorsPassedThrough(t *testing.T) { } } -func TestEvalValidateResource_ignoreWarnings(t *testing.T) { - mp := simpleMockProvider() - mp.ValidateResourceTypeConfigFn = func(req providers.ValidateResourceTypeConfigRequest) providers.ValidateResourceTypeConfigResponse { - var diags tfdiags.Diagnostics - diags = diags.Append(tfdiags.SimpleWarning("warn")) - return providers.ValidateResourceTypeConfigResponse{ - Diagnostics: diags, - } - } - - p := providers.Interface(mp) - rc := &configs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "test_object", - Name: "foo", - Config: configs.SynthBody("", map[string]cty.Value{}), - } - node := &EvalValidateResource{ - Addr: addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "test-object", - Name: "foo", - }, - Provider: &p, - Config: rc, - ProviderSchema: &mp.GetSchemaReturn, - - IgnoreWarnings: true, - } - - ctx := &MockEvalContext{} - ctx.installSimpleEval() - - err := node.Validate(ctx) - if err != nil { - t.Fatalf("Expected no error, got: %s", err) - } -} - func TestEvalValidateResource_invalidDependsOn(t *testing.T) { mp := simpleMockProvider() mp.ValidateResourceTypeConfigFn = func(req providers.ValidateResourceTypeConfigRequest) providers.ValidateResourceTypeConfigResponse { @@ -323,9 +284,9 @@ func TestEvalValidateResource_invalidDependsOn(t *testing.T) { ctx := &MockEvalContext{} ctx.installSimpleEval() - err := node.Validate(ctx) - if err != nil { - t.Fatalf("error for supposedly-valid config: %s", err) + diags := node.Validate(ctx) + if diags.HasErrors() { + t.Fatalf("error for supposedly-valid config: %s", diags.ErrWithWarnings()) } // Now we'll make it invalid by adding additional traversal steps at @@ -344,11 +305,11 @@ func TestEvalValidateResource_invalidDependsOn(t *testing.T) { }, }) - err = node.Validate(ctx) - if err == nil { + diags = node.Validate(ctx) + if !diags.HasErrors() { t.Fatal("no error for invalid depends_on") } - if got, want := err.Error(), "Invalid depends_on reference"; !strings.Contains(got, want) { + if got, want := diags.Err().Error(), "Invalid depends_on reference"; !strings.Contains(got, want) { t.Fatalf("wrong error\ngot: %s\nwant: Message containing %q", got, want) } @@ -360,11 +321,11 @@ func TestEvalValidateResource_invalidDependsOn(t *testing.T) { }, }) - err = node.Validate(ctx) - if err == nil { + diags = node.Validate(ctx) + if !diags.HasErrors() { t.Fatal("no error for invalid depends_on") } - if got, want := err.Error(), "Invalid depends_on reference"; !strings.Contains(got, want) { + if got, want := diags.Err().Error(), "Invalid depends_on reference"; !strings.Contains(got, want) { t.Fatalf("wrong error\ngot: %s\nwant: Message containing %q", got, want) } }