From ba0514106a63f5c45aecb0b944429e80a9d4ac42 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 21 Nov 2017 15:08:00 -0800 Subject: [PATCH] return tfdiags.Diagnostics from validation methods Validation is the best time to return detailed diagnostics to the user since we're much more likely to have source location information, etc than we are in later operations. This change doesn't actually add any detail to the messages yet, but it changes the interface so that we can gradually introduce more detailed diagnostics over time. While here there are some minor adjustments to some of the messages to improve their consistency with terminology we use elsewhere. --- backend/local/backend_local.go | 49 +++-- command/command.go | 36 +--- command/init.go | 3 +- command/providers.go | 9 +- command/validate.go | 11 +- command/validate_test.go | 12 +- config/config.go | 270 ++++++++++++++---------- config/config_test.go | 26 +-- config/module/tree.go | 72 +++---- config/module/tree_test.go | 18 +- helper/resource/testing.go | 15 +- helper/resource/testing_config.go | 16 +- terraform/context.go | 39 ++-- terraform/context_apply_test.go | 18 +- terraform/context_plan_test.go | 27 +-- terraform/context_refresh_test.go | 9 +- terraform/context_validate_test.go | 326 ++++++++++------------------- 17 files changed, 426 insertions(+), 530 deletions(-) diff --git a/backend/local/backend_local.go b/backend/local/backend_local.go index 2c121d2e6..aa056a1a1 100644 --- a/backend/local/backend_local.go +++ b/backend/local/backend_local.go @@ -2,12 +2,13 @@ package local import ( "errors" - "fmt" "log" - "strings" + + "github.com/hashicorp/terraform/command/format" + + "github.com/hashicorp/terraform/tfdiags" "github.com/hashicorp/errwrap" - "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" @@ -91,27 +92,31 @@ func (b *Local) context(op *backend.Operation) (*terraform.Context, state.State, // If validation is enabled, validate if b.OpValidation { - // We ignore warnings here on purpose. We expect users to be listening - // to the terraform.Hook called after a validation. - ws, es := tfCtx.Validate() - if len(ws) > 0 { - // Log just in case the CLI isn't enabled - log.Printf("[WARN] backend/local: %d warnings: %v", len(ws), ws) - - // If we have a CLI, output the warnings - if b.CLI != nil { - b.CLI.Warn(strings.TrimSpace(validateWarnHeader) + "\n") - for _, w := range ws { - b.CLI.Warn(fmt.Sprintf(" * %s", w)) - } - - // Make a newline before continuing - b.CLI.Output("") + diags := tfCtx.Validate() + if len(diags) > 0 { + if diags.HasErrors() { + // If there are warnings _and_ errors then we'll take this + // path and return them all together in this error. + return nil, nil, diags.Err() } - } - if len(es) > 0 { - return nil, nil, multierror.Append(nil, es...) + // For now we can't propagate warnings any further without + // printing them directly to the UI, so we'll need to + // format them here ourselves. + for _, diag := range diags { + if diag.Severity() != tfdiags.Warning { + continue + } + if b.CLI != nil { + b.CLI.Warn(format.Diagnostic(diag, b.Colorize(), 72)) + } else { + desc := diag.Description() + log.Printf("[WARN] backend/local: %s", desc.Summary) + } + } + + // Make a newline before continuing + b.CLI.Output("") } } } diff --git a/command/command.go b/command/command.go index 0fbb44df3..0cd11da08 100644 --- a/command/command.go +++ b/command/command.go @@ -7,7 +7,6 @@ import ( "runtime" "github.com/hashicorp/terraform/terraform" - "github.com/mitchellh/cli" ) // Set to true when we're testing @@ -82,39 +81,18 @@ func ModulePath(args []string) (string, error) { return args[0], nil } -func validateContext(ctx *terraform.Context, ui cli.Ui) bool { +func (m *Meta) validateContext(ctx *terraform.Context) bool { log.Println("[INFO] Validating the context...") - ws, es := ctx.Validate() - log.Printf("[INFO] Validation result: %d warnings, %d errors", len(ws), len(es)) + diags := ctx.Validate() + log.Printf("[INFO] Validation result: %d diagnostics", len(diags)) - if len(ws) > 0 || len(es) > 0 { - ui.Output( + if len(diags) > 0 { + m.Ui.Output( "There are warnings and/or errors related to your configuration. Please\n" + "fix these before continuing.\n") - if len(ws) > 0 { - ui.Warn("Warnings:\n") - for _, w := range ws { - ui.Warn(fmt.Sprintf(" * %s", w)) - } - - if len(es) > 0 { - ui.Output("") - } - } - - if len(es) > 0 { - ui.Error("Errors:\n") - for _, e := range es { - ui.Error(fmt.Sprintf(" * %s", e)) - } - return false - } else { - ui.Warn(fmt.Sprintf("\n"+ - "No errors found. Continuing with %d warning(s).\n", len(ws))) - return true - } + m.showDiagnostics(diags) } - return true + return !diags.HasErrors() } diff --git a/command/init.go b/command/init.go index 1deb20889..57e5b387d 100644 --- a/command/init.go +++ b/command/init.go @@ -280,7 +280,8 @@ func (c *InitCommand) getProviders(path string, state *terraform.State, upgrade return err } - if err := mod.Validate(); err != nil { + if diags := mod.Validate(); diags.HasErrors() { + err := diags.Err() c.Ui.Error(fmt.Sprintf("Error getting plugins: %s", err)) return err } diff --git a/command/providers.go b/command/providers.go index 2a755f126..a89136a04 100644 --- a/command/providers.go +++ b/command/providers.go @@ -53,10 +53,11 @@ func (c *ProvidersCommand) Run(args []string) int { } // Validate the config (to ensure the version constraints are valid) - err = root.Validate() - if err != nil { - c.Ui.Error(err.Error()) - return 1 + if diags := root.Validate(); len(diags) != 0 { + c.showDiagnostics(diags) + if diags.HasErrors() { + return 1 + } } // Load the backend diff --git a/command/validate.go b/command/validate.go index b6aff5a95..335ebe618 100644 --- a/command/validate.go +++ b/command/validate.go @@ -100,10 +100,11 @@ func (c *ValidateCommand) validate(dir string, checkVars bool) int { c.showDiagnostics(err) return 1 } - err = cfg.Validate() - if err != nil { - c.showDiagnostics(err) - return 1 + if diags := cfg.Validate(); len(diags) != 0 { + c.showDiagnostics(diags) + if diags.HasErrors() { + return 1 + } } if checkVars { @@ -122,7 +123,7 @@ func (c *ValidateCommand) validate(dir string, checkVars bool) int { return 1 } - if !validateContext(tfCtx, c.Ui) { + if !c.validateContext(tfCtx) { return 1 } } diff --git a/command/validate_test.go b/command/validate_test.go index a52b956ff..18243e343 100644 --- a/command/validate_test.go +++ b/command/validate_test.go @@ -74,7 +74,7 @@ func TestValidateFailingCommandMissingVariable(t *testing.T) { if code != 1 { t.Fatalf("Should have failed: %d\n\n%s", code, ui.ErrorWriter.String()) } - if !strings.HasSuffix(strings.TrimSpace(ui.ErrorWriter.String()), "config: unknown variable referenced: 'description'. define it with 'variable' blocks") { + if !strings.HasSuffix(strings.TrimSpace(ui.ErrorWriter.String()), "config: unknown variable referenced: 'description'; define it with a 'variable' block") { t.Fatalf("Should have failed: %d\n\n'%s'", code, ui.ErrorWriter.String()) } } @@ -84,7 +84,7 @@ func TestSameProviderMutipleTimesShouldFail(t *testing.T) { if code != 1 { t.Fatalf("Should have failed: %d\n\n%s", code, ui.ErrorWriter.String()) } - if !strings.HasSuffix(strings.TrimSpace(ui.ErrorWriter.String()), "provider.aws: declared multiple times, you can only declare a provider once") { + if !strings.HasSuffix(strings.TrimSpace(ui.ErrorWriter.String()), "provider.aws: multiple configurations present; only one configuration is allowed per provider") { t.Fatalf("Should have failed: %d\n\n'%s'", code, ui.ErrorWriter.String()) } } @@ -94,7 +94,7 @@ func TestSameModuleMultipleTimesShouldFail(t *testing.T) { if code != 1 { t.Fatalf("Should have failed: %d\n\n%s", code, ui.ErrorWriter.String()) } - if !strings.HasSuffix(strings.TrimSpace(ui.ErrorWriter.String()), "multi_module: module repeated multiple times") { + if !strings.HasSuffix(strings.TrimSpace(ui.ErrorWriter.String()), "module \"multi_module\": module repeated multiple times") { t.Fatalf("Should have failed: %d\n\n'%s'", code, ui.ErrorWriter.String()) } } @@ -114,7 +114,7 @@ func TestOutputWithoutValueShouldFail(t *testing.T) { if code != 1 { t.Fatalf("Should have failed: %d\n\n%s", code, ui.ErrorWriter.String()) } - if !strings.HasSuffix(strings.TrimSpace(ui.ErrorWriter.String()), "output is missing required 'value' key") { + if !strings.HasSuffix(strings.TrimSpace(ui.ErrorWriter.String()), "output \"myvalue\": missing required 'value' argument") { t.Fatalf("Should have failed: %d\n\n'%s'", code, ui.ErrorWriter.String()) } } @@ -125,7 +125,7 @@ func TestModuleWithIncorrectNameShouldFail(t *testing.T) { t.Fatalf("Should have failed: %d\n\n%s", code, ui.ErrorWriter.String()) } - if !strings.Contains(ui.ErrorWriter.String(), "module name can only contain letters, numbers, dashes, and underscores") { + if !strings.Contains(ui.ErrorWriter.String(), "module name must be a letter or underscore followed by only letters, numbers, dashes, and underscores") { t.Fatalf("Should have failed: %d\n\n'%s'", code, ui.ErrorWriter.String()) } if !strings.Contains(ui.ErrorWriter.String(), "module source cannot contain interpolations") { @@ -142,7 +142,7 @@ func TestWronglyUsedInterpolationShouldFail(t *testing.T) { if !strings.Contains(ui.ErrorWriter.String(), "depends on value cannot contain interpolations") { t.Fatalf("Should have failed: %d\n\n'%s'", code, ui.ErrorWriter.String()) } - if !strings.Contains(ui.ErrorWriter.String(), "Variable 'vairable_with_interpolation': cannot contain interpolations") { + if !strings.Contains(ui.ErrorWriter.String(), "variable \"vairable_with_interpolation\": default may not contain interpolations") { t.Fatalf("Should have failed: %d\n\n'%s'", code, ui.ErrorWriter.String()) } } diff --git a/config/config.go b/config/config.go index 3fb53cbd8..f77ca475c 100644 --- a/config/config.go +++ b/config/config.go @@ -8,10 +8,10 @@ import ( "strconv" "strings" - "github.com/hashicorp/go-multierror" "github.com/hashicorp/hil/ast" "github.com/hashicorp/terraform/helper/hilmapstructure" "github.com/hashicorp/terraform/plugin/discovery" + "github.com/hashicorp/terraform/tfdiags" "github.com/mitchellh/reflectwalk" ) @@ -278,30 +278,35 @@ func ResourceProviderFullName(resourceType, explicitProvider string) string { } // Validate does some basic semantic checking of the configuration. -func (c *Config) Validate() error { +func (c *Config) Validate() tfdiags.Diagnostics { if c == nil { return nil } - var errs []error + var diags tfdiags.Diagnostics for _, k := range c.unknownKeys { - errs = append(errs, fmt.Errorf( - "Unknown root level key: %s", k)) + diags = diags.Append( + fmt.Errorf("Unknown root level key: %s", k), + ) } // Validate the Terraform config if tf := c.Terraform; tf != nil { - errs = append(errs, c.Terraform.Validate()...) + errs := c.Terraform.Validate() + for _, err := range errs { + diags = diags.Append(err) + } } vars := c.InterpolatedVariables() varMap := make(map[string]*Variable) for _, v := range c.Variables { if _, ok := varMap[v.Name]; ok { - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "Variable '%s': duplicate found. Variable names must be unique.", - v.Name)) + v.Name, + )) } varMap[v.Name] = v @@ -309,17 +314,19 @@ func (c *Config) Validate() error { for k, _ := range varMap { if !NameRegexp.MatchString(k) { - errs = append(errs, fmt.Errorf( - "variable %q: variable name must match regular expresion %s", - k, NameRegexp)) + diags = diags.Append(fmt.Errorf( + "variable %q: variable name must match regular expression %s", + k, NameRegexp, + )) } } for _, v := range c.Variables { if v.Type() == VariableTypeUnknown { - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "Variable '%s': must be a string or a map", - v.Name)) + v.Name, + )) continue } @@ -340,9 +347,10 @@ func (c *Config) Validate() error { if v.Default != nil { if err := reflectwalk.Walk(v.Default, w); err == nil { if interp { - errs = append(errs, fmt.Errorf( - "Variable '%s': cannot contain interpolations", - v.Name)) + diags = diags.Append(fmt.Errorf( + "variable %q: default may not contain interpolations", + v.Name, + )) } } } @@ -358,10 +366,11 @@ func (c *Config) Validate() error { } if _, ok := varMap[uv.Name]; !ok { - errs = append(errs, fmt.Errorf( - "%s: unknown variable referenced: '%s'. define it with 'variable' blocks", + diags = diags.Append(fmt.Errorf( + "%s: unknown variable referenced: '%s'; define it with a 'variable' block", source, - uv.Name)) + uv.Name, + )) } } } @@ -372,17 +381,19 @@ func (c *Config) Validate() error { switch v := rawV.(type) { case *CountVariable: if v.Type == CountValueInvalid { - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "%s: invalid count variable: %s", source, - v.FullKey())) + v.FullKey(), + )) } case *PathVariable: if v.Type == PathValueInvalid { - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "%s: invalid path variable: %s", source, - v.FullKey())) + v.FullKey(), + )) } } } @@ -394,16 +405,17 @@ func (c *Config) Validate() error { for _, p := range c.ProviderConfigs { name := p.FullName() if _, ok := providerSet[name]; ok { - errs = append(errs, fmt.Errorf( - "provider.%s: declared multiple times, you can only declare a provider once", - name)) + diags = diags.Append(fmt.Errorf( + "provider.%s: multiple configurations present; only one configuration is allowed per provider", + name, + )) continue } if p.Version != "" { _, err := discovery.ConstraintStr(p.Version).Parse() if err != nil { - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "provider.%s: invalid version constraint %q: %s", name, p.Version, err, )) @@ -422,9 +434,10 @@ func (c *Config) Validate() error { if _, ok := dupped[m.Id()]; !ok { dupped[m.Id()] = struct{}{} - errs = append(errs, fmt.Errorf( - "%s: module repeated multiple times", - m.Id())) + diags = diags.Append(fmt.Errorf( + "module %q: module repeated multiple times", + m.Id(), + )) } // Already seen this module, just skip it @@ -438,21 +451,23 @@ func (c *Config) Validate() error { "root": m.Source, }) if err != nil { - errs = append(errs, fmt.Errorf( - "%s: module source error: %s", - m.Id(), err)) + diags = diags.Append(fmt.Errorf( + "module %q: module source error: %s", + m.Id(), err, + )) } else if len(rc.Interpolations) > 0 { - errs = append(errs, fmt.Errorf( - "%s: module source cannot contain interpolations", - m.Id())) + diags = diags.Append(fmt.Errorf( + "module %q: module source cannot contain interpolations", + m.Id(), + )) } // Check that the name matches our regexp if !NameRegexp.Match([]byte(m.Name)) { - errs = append(errs, fmt.Errorf( - "%s: module name can only contain letters, numbers, "+ - "dashes, and underscores", - m.Id())) + diags = diags.Append(fmt.Errorf( + "module %q: module name must be a letter or underscore followed by only letters, numbers, dashes, and underscores", + m.Id(), + )) } // Check that the configuration can all be strings, lists or maps @@ -476,36 +491,44 @@ func (c *Config) Validate() error { continue } - errs = append(errs, fmt.Errorf( - "%s: variable %s must be a string, list or map value", - m.Id(), k)) + diags = diags.Append(fmt.Errorf( + "module %q: argument %s must have a string, list, or map value", + m.Id(), k, + )) } // Check for invalid count variables for _, v := range m.RawConfig.Variables { switch v.(type) { case *CountVariable: - errs = append(errs, fmt.Errorf( - "%s: count variables are only valid within resources", m.Name)) + diags = diags.Append(fmt.Errorf( + "module %q: count variables are only valid within resources", + m.Name, + )) case *SelfVariable: - errs = append(errs, fmt.Errorf( - "%s: self variables are only valid within resources", m.Name)) + diags = diags.Append(fmt.Errorf( + "module %q: self variables are only valid within resources", + m.Name, + )) } } // Update the raw configuration to only contain the string values m.RawConfig, err = NewRawConfig(raw) if err != nil { - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "%s: can't initialize configuration: %s", - m.Id(), err)) + m.Id(), err, + )) } // check that all named providers actually exist for _, p := range m.Providers { if !providerSet[p] { - errs = append(errs, fmt.Errorf( - "provider %q named in module %q does not exist", p, m.Name)) + diags = diags.Append(fmt.Errorf( + "module %q: cannot pass non-existent provider %q", + m.Name, p, + )) } } @@ -522,10 +545,10 @@ func (c *Config) Validate() error { } if _, ok := modules[mv.Name]; !ok { - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "%s: unknown module referenced: %s", - source, - mv.Name)) + source, mv.Name, + )) } } } @@ -538,9 +561,10 @@ func (c *Config) Validate() error { if _, ok := dupped[r.Id()]; !ok { dupped[r.Id()] = struct{}{} - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "%s: resource repeated multiple times", - r.Id())) + r.Id(), + )) } } @@ -554,15 +578,15 @@ func (c *Config) Validate() error { for _, v := range r.RawCount.Variables { switch v.(type) { case *CountVariable: - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "%s: resource count can't reference count variable: %s", - n, - v.FullKey())) + n, v.FullKey(), + )) case *SimpleVariable: - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "%s: resource count can't reference variable: %s", - n, - v.FullKey())) + n, v.FullKey(), + )) // Good case *ModuleVariable: @@ -572,21 +596,24 @@ func (c *Config) Validate() error { case *LocalVariable: default: - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "Internal error. Unknown type in count var in %s: %T", - n, v)) + n, v, + )) } } if !r.RawCount.couldBeInteger() { - errs = append(errs, fmt.Errorf( - "%s: resource count must be an integer", - n)) + diags = diags.Append(fmt.Errorf( + "%s: resource count must be an integer", n, + )) } r.RawCount.init() // Validate DependsOn - errs = append(errs, c.validateDependsOn(n, r.DependsOn, resources, modules)...) + for _, err := range c.validateDependsOn(n, r.DependsOn, resources, modules) { + diags = diags.Append(err) + } // Verify provisioners for _, p := range r.Provisioners { @@ -600,9 +627,10 @@ func (c *Config) Validate() error { } if rv.Multi && rv.Index == -1 && rv.Type == r.Type && rv.Name == r.Name { - errs = append(errs, fmt.Errorf( - "%s: connection info cannot contain splat variable "+ - "referencing itself", n)) + diags = diags.Append(fmt.Errorf( + "%s: connection info cannot contain splat variable referencing itself", + n, + )) break } } @@ -614,9 +642,10 @@ func (c *Config) Validate() error { } if rv.Multi && rv.Index == -1 && rv.Type == r.Type && rv.Name == r.Name { - errs = append(errs, fmt.Errorf( - "%s: connection info cannot contain splat variable "+ - "referencing itself", n)) + diags = diags.Append(fmt.Errorf( + "%s: connection info cannot contain splat variable referencing itself", + n, + )) break } } @@ -624,21 +653,24 @@ func (c *Config) Validate() error { // Check for invalid when/onFailure values, though this should be // picked up by the loader we check here just in case. if p.When == ProvisionerWhenInvalid { - errs = append(errs, fmt.Errorf( - "%s: provisioner 'when' value is invalid", n)) + diags = diags.Append(fmt.Errorf( + "%s: provisioner 'when' value is invalid", n, + )) } if p.OnFailure == ProvisionerOnFailureInvalid { - errs = append(errs, fmt.Errorf( - "%s: provisioner 'on_failure' value is invalid", n)) + diags = diags.Append(fmt.Errorf( + "%s: provisioner 'on_failure' value is invalid", n, + )) } } // Verify ignore_changes contains valid entries for _, v := range r.Lifecycle.IgnoreChanges { if strings.Contains(v, "*") && v != "*" { - errs = append(errs, fmt.Errorf( - "%s: ignore_changes does not support using a partial string "+ - "together with a wildcard: %s", n, v)) + diags = diags.Append(fmt.Errorf( + "%s: ignore_changes does not support using a partial string together with a wildcard: %s", + n, v, + )) } } @@ -647,21 +679,24 @@ func (c *Config) Validate() error { "root": r.Lifecycle.IgnoreChanges, }) if err != nil { - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "%s: lifecycle ignore_changes error: %s", - n, err)) + n, err, + )) } else if len(rc.Interpolations) > 0 { - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "%s: lifecycle ignore_changes cannot contain interpolations", - n)) + n, + )) } // If it is a data source then it can't have provisioners if r.Mode == DataResourceMode { if _, ok := r.RawConfig.Raw["provisioner"]; ok { - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "%s: data sources cannot have provisioners", - n)) + n, + )) } } } @@ -675,11 +710,12 @@ func (c *Config) Validate() error { id := rv.ResourceId() if _, ok := resources[id]; !ok { - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "%s: unknown resource '%s' referenced in variable %s", source, id, - rv.FullKey())) + rv.FullKey(), + )) continue } } @@ -690,7 +726,7 @@ func (c *Config) Validate() error { found := make(map[string]struct{}) for _, l := range c.Locals { if _, ok := found[l.Name]; ok { - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "%s: duplicate local. local value names must be unique", l.Name, )) @@ -700,7 +736,7 @@ func (c *Config) Validate() error { for _, v := range l.RawConfig.Variables { if _, ok := v.(*CountVariable); ok { - errs = append(errs, fmt.Errorf( + diags = diags.Append(fmt.Errorf( "local %s: count variables are only valid within resources", l.Name, )) } @@ -714,9 +750,10 @@ func (c *Config) Validate() error { for _, o := range c.Outputs { // Verify the output is new if _, ok := found[o.Name]; ok { - errs = append(errs, fmt.Errorf( - "%s: duplicate output. output names must be unique.", - o.Name)) + diags = diags.Append(fmt.Errorf( + "output %q: an output of this name was already defined", + o.Name, + )) continue } found[o.Name] = struct{}{} @@ -736,9 +773,10 @@ func (c *Config) Validate() error { continue } - errs = append(errs, fmt.Errorf( - "%s: value for 'sensitive' must be boolean", - o.Name)) + diags = diags.Append(fmt.Errorf( + "output %q: value for 'sensitive' must be boolean", + o.Name, + )) continue } if k == "description" { @@ -747,29 +785,35 @@ func (c *Config) Validate() error { continue } - errs = append(errs, fmt.Errorf( - "%s: value for 'description' must be string", - o.Name)) + diags = diags.Append(fmt.Errorf( + "output %q: value for 'description' must be string", + o.Name, + )) continue } invalidKeys = append(invalidKeys, k) } if len(invalidKeys) > 0 { - errs = append(errs, fmt.Errorf( - "%s: output has invalid keys: %s", - o.Name, strings.Join(invalidKeys, ", "))) + diags = diags.Append(fmt.Errorf( + "output %q: invalid keys: %s", + o.Name, strings.Join(invalidKeys, ", "), + )) } if !valueKeyFound { - errs = append(errs, fmt.Errorf( - "%s: output is missing required 'value' key", o.Name)) + diags = diags.Append(fmt.Errorf( + "output %q: missing required 'value' argument", o.Name, + )) } for _, v := range o.RawConfig.Variables { if _, ok := v.(*CountVariable); ok { - errs = append(errs, fmt.Errorf( - "%s: count variables are only valid within resources", o.Name)) + diags = diags.Append(fmt.Errorf( + "output %q: count variables are only valid within resources", + o.Name, + )) } } + } } @@ -783,17 +827,15 @@ func (c *Config) Validate() error { for _, v := range rc.Variables { if _, ok := v.(*SelfVariable); ok { - errs = append(errs, fmt.Errorf( - "%s: cannot contain self-reference %s", source, v.FullKey())) + diags = diags.Append(fmt.Errorf( + "%s: cannot contain self-reference %s", + source, v.FullKey(), + )) } } } - if len(errs) > 0 { - return &multierror.Error{Errors: errs} - } - - return nil + return diags } // InterpolatedVariables is a helper that returns a mapping of all the interpolated diff --git a/config/config_test.go b/config/config_test.go index ab5dda75b..7f4835c2c 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -223,20 +223,21 @@ func TestConfigValidate_table(t *testing.T) { "invalid provider name in module block", "validate-missing-provider", true, - "does not exist", + "cannot pass non-existent provider", }, } for i, tc := range cases { t.Run(fmt.Sprintf("%d-%s", i, tc.Name), func(t *testing.T) { c := testConfig(t, tc.Fixture) - err := c.Validate() - if (err != nil) != tc.Err { - t.Fatalf("err: %s", err) + diags := c.Validate() + if diags.HasErrors() != tc.Err { + t.Fatalf("err: %s", diags.Err().Error()) } - if err != nil { - if tc.ErrString != "" && !strings.Contains(err.Error(), tc.ErrString) { - t.Fatalf("expected err to contain: %s\n\ngot: %s", tc.ErrString, err) + if diags.HasErrors() { + gotErr := diags.Err().Error() + if tc.ErrString != "" && !strings.Contains(gotErr, tc.ErrString) { + t.Fatalf("expected err to contain: %s\n\ngot: %s", tc.ErrString, gotErr) } return @@ -291,15 +292,16 @@ func TestConfigValidate_countInt_HCL2(t *testing.T) { func TestConfigValidate_countBadContext(t *testing.T) { c := testConfig(t, "validate-count-bad-context") - err := c.Validate() + diags := c.Validate() expected := []string{ - "no_count_in_output: count variables are only valid within resources", - "no_count_in_module: count variables are only valid within resources", + "output \"no_count_in_output\": count variables are only valid within resources", + "module \"no_count_in_module\": count variables are only valid within resources", } for _, exp := range expected { - if !strings.Contains(err.Error(), exp) { - t.Fatalf("expected: %q,\nto contain: %q", err, exp) + errStr := diags.Err().Error() + if !strings.Contains(errStr, exp) { + t.Errorf("expected: %q,\nto contain: %q", errStr, exp) } } } diff --git a/config/module/tree.go b/config/module/tree.go index 0bf6b93f7..f56d69b70 100644 --- a/config/module/tree.go +++ b/config/module/tree.go @@ -9,6 +9,8 @@ import ( "strings" "sync" + "github.com/hashicorp/terraform/tfdiags" + getter "github.com/hashicorp/go-getter" "github.com/hashicorp/terraform/config" ) @@ -383,32 +385,35 @@ func (t *Tree) String() string { // as verifying things such as parameters/outputs between the various modules. // // Load must be called prior to calling Validate or an error will be returned. -func (t *Tree) Validate() error { - if !t.Loaded() { - return fmt.Errorf("tree must be loaded before calling Validate") - } +func (t *Tree) Validate() tfdiags.Diagnostics { + var diags tfdiags.Diagnostics - // If something goes wrong, here is our error template - newErr := &treeError{Name: []string{t.Name()}} + if !t.Loaded() { + diags = diags.Append(fmt.Errorf( + "tree must be loaded before calling Validate", + )) + return diags + } // Terraform core does not handle root module children named "root". // We plan to fix this in the future but this bug was brought up in // the middle of a release and we don't want to introduce wide-sweeping // changes at that time. if len(t.path) == 1 && t.name == "root" { - return fmt.Errorf("root module cannot contain module named 'root'") + diags = diags.Append(fmt.Errorf( + "root module cannot contain module named 'root'", + )) + return diags } // Validate our configuration first. - if err := t.config.Validate(); err != nil { - newErr.Add(err) - } + diags = diags.Append(t.config.Validate()) // If we're the root, we do extra validation. This validation usually // requires the entire tree (since children don't have parent pointers). if len(t.path) == 0 { if err := t.validateProviderAlias(); err != nil { - newErr.Add(err) + diags = diags.Append(err) } } @@ -417,20 +422,11 @@ func (t *Tree) Validate() error { // Validate all our children for _, c := range children { - err := c.Validate() - if err == nil { + childDiags := c.Validate() + diags = diags.Append(childDiags) + if diags.HasErrors() { continue } - - verr, ok := err.(*treeError) - if !ok { - // Unknown error, just return... - return err - } - - // Append ourselves to the error and then return - verr.Name = append(verr.Name, t.Name()) - newErr.AddChild(verr) } // Go over all the modules and verify that any parameters are valid @@ -456,9 +452,10 @@ func (t *Tree) Validate() error { // Compare to the keys in our raw config for the module for k, _ := range m.RawConfig.Raw { if _, ok := varMap[k]; !ok { - newErr.Add(fmt.Errorf( - "module %s: %s is not a valid parameter", - m.Name, k)) + diags = diags.Append(fmt.Errorf( + "module %q: %q is not a valid argument", + m.Name, k, + )) } // Remove the required @@ -467,9 +464,10 @@ func (t *Tree) Validate() error { // If we have any required left over, they aren't set. for k, _ := range requiredMap { - newErr.Add(fmt.Errorf( - "module %s: required variable %q not set", - m.Name, k)) + diags = diags.Append(fmt.Errorf( + "module %q: missing required argument %q", + m.Name, k, + )) } } @@ -484,9 +482,10 @@ func (t *Tree) Validate() error { tree, ok := children[mv.Name] if !ok { - newErr.Add(fmt.Errorf( - "%s: undefined module referenced %s", - source, mv.Name)) + diags = diags.Append(fmt.Errorf( + "%s: reference to undefined module %q", + source, mv.Name, + )) continue } @@ -498,14 +497,15 @@ func (t *Tree) Validate() error { } } if !found { - newErr.Add(fmt.Errorf( - "%s: %s is not a valid output for module %s", - source, mv.Field, mv.Name)) + diags = diags.Append(fmt.Errorf( + "%s: %q is not a valid output for module %q", + source, mv.Field, mv.Name, + )) } } } - return newErr.ErrOrNil() + return diags } // versionedPathKey returns a path string with every levels full name, version diff --git a/config/module/tree_test.go b/config/module/tree_test.go index 5e9abba4b..5c642962f 100644 --- a/config/module/tree_test.go +++ b/config/module/tree_test.go @@ -404,15 +404,15 @@ func TestTreeValidate_table(t *testing.T) { t.Fatalf("err: %s", err) } - err := tree.Validate() - if (err != nil) != (tc.Err != "") { - t.Fatalf("err: %s", err) + diags := tree.Validate() + if (diags.HasErrors()) != (tc.Err != "") { + t.Fatalf("err: %s", diags.Err()) } - if err == nil { + if len(diags) == 0 { return } - if !strings.Contains(err.Error(), tc.Err) { - t.Fatalf("err should contain %q: %s", tc.Err, err) + if !strings.Contains(diags.Err().Error(), tc.Err) { + t.Fatalf("err should contain %q: %s", tc.Err, diags.Err().Error()) } }) } @@ -519,13 +519,13 @@ func TestTreeValidate_requiredChildVar(t *testing.T) { t.Fatalf("err: %s", err) } - err := tree.Validate() - if err == nil { + diags := tree.Validate() + if !diags.HasErrors() { t.Fatal("should error") } // ensure both variables are mentioned in the output - errMsg := err.Error() + errMsg := diags.Err().Error() for _, v := range []string{"feature", "memory"} { if !strings.Contains(errMsg, v) { t.Fatalf("no mention of missing variable %q", v) diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 05e7c0feb..64f5c899b 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -15,6 +15,7 @@ import ( "testing" "github.com/davecgh/go-spew/spew" + "github.com/hashicorp/errwrap" "github.com/hashicorp/go-multierror" "github.com/hashicorp/logutils" "github.com/hashicorp/terraform/config/module" @@ -662,18 +663,12 @@ func testIDOnlyRefresh(c TestCase, opts terraform.ContextOpts, step TestStep, r if err != nil { return err } - if ws, es := ctx.Validate(); len(ws) > 0 || len(es) > 0 { - if len(es) > 0 { - estrs := make([]string, len(es)) - for i, e := range es { - estrs[i] = e.Error() - } - return fmt.Errorf( - "Configuration is invalid.\n\nWarnings: %#v\n\nErrors: %#v", - ws, estrs) + if diags := ctx.Validate(); len(diags) > 0 { + if diags.HasErrors() { + return errwrap.Wrapf("config is invalid: {{err}}", diags.Err()) } - log.Printf("[WARN] Config warnings: %#v", ws) + log.Printf("[WARN] Config warnings:\n%s", diags.Err().Error()) } // Refresh! diff --git a/helper/resource/testing_config.go b/helper/resource/testing_config.go index 537a11c34..300a9ea6e 100644 --- a/helper/resource/testing_config.go +++ b/helper/resource/testing_config.go @@ -5,6 +5,7 @@ import ( "log" "strings" + "github.com/hashicorp/errwrap" "github.com/hashicorp/terraform/terraform" ) @@ -33,17 +34,12 @@ func testStep( if err != nil { return state, fmt.Errorf("Error initializing context: %s", err) } - if ws, es := ctx.Validate(); len(ws) > 0 || len(es) > 0 { - if len(es) > 0 { - estrs := make([]string, len(es)) - for i, e := range es { - estrs[i] = e.Error() - } - return state, fmt.Errorf( - "Configuration is invalid.\n\nWarnings: %#v\n\nErrors: %#v", - ws, estrs) + if diags := ctx.Validate(); len(diags) > 0 { + if diags.HasErrors() { + return nil, errwrap.Wrapf("config is invalid: {{err}}", diags.Err()) } - log.Printf("[WARN] Config warnings: %#v", ws) + + log.Printf("[WARN] Config warnings:\n%s", diags) } // Refresh! diff --git a/terraform/context.go b/terraform/context.go index ed756c88d..cede4f817 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -8,6 +8,8 @@ import ( "strings" "sync" + "github.com/hashicorp/terraform/tfdiags" + "github.com/hashicorp/go-multierror" "github.com/hashicorp/hcl" "github.com/hashicorp/terraform/config" @@ -671,29 +673,27 @@ func (c *Context) Stop() { } // Validate validates the configuration and returns any warnings or errors. -func (c *Context) Validate() ([]string, []error) { +func (c *Context) Validate() tfdiags.Diagnostics { defer c.acquireRun("validate")() - var errs error + var diags tfdiags.Diagnostics // Validate the configuration itself - if err := c.module.Validate(); err != nil { - errs = multierror.Append(errs, err) - } + diags = diags.Append(c.module.Validate()) // This only needs to be done for the root module, since inter-module // variables are validated in the module tree. if config := c.module.Config(); config != nil { // Validate the user variables - if err := smcUserVariables(config, c.variables); len(err) > 0 { - errs = multierror.Append(errs, err...) + for _, err := range smcUserVariables(config, c.variables) { + diags = diags.Append(err) } } // If we have errors at this point, the graphing has no chance, // so just bail early. - if errs != nil { - return nil, []error{errs} + if diags.HasErrors() { + return diags } // Build the graph so we can walk it and run Validate on nodes. @@ -702,24 +702,29 @@ func (c *Context) Validate() ([]string, []error) { // graph again later after Planning. graph, err := c.Graph(GraphTypeValidate, nil) if err != nil { - return nil, []error{err} + diags = diags.Append(err) + return diags } // Walk walker, err := c.walk(graph, walkValidate) if err != nil { - return nil, multierror.Append(errs, err).Errors + diags = diags.Append(err) } - // Return the result - rerrs := multierror.Append(errs, walker.ValidationErrors...) - sort.Strings(walker.ValidationWarnings) - sort.Slice(rerrs.Errors, func(i, j int) bool { - return rerrs.Errors[i].Error() < rerrs.Errors[j].Error() + sort.Slice(walker.ValidationErrors, func(i, j int) bool { + return walker.ValidationErrors[i].Error() < walker.ValidationErrors[j].Error() }) - return walker.ValidationWarnings, rerrs.Errors + for _, warn := range walker.ValidationWarnings { + diags = diags.Append(tfdiags.SimpleWarning(warn)) + } + for _, err := range walker.ValidationErrors { + diags = diags.Append(err) + } + + return diags } // Module returns the module tree associated with this context. diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 656ac9438..7bc28f3bd 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -7966,12 +7966,9 @@ func TestContext2Apply_vars(t *testing.T) { }, }) - w, e := ctx.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %s", e) + diags := ctx.Validate() + if len(diags) != 0 { + t.Fatalf("bad: %#v", diags) } if _, err := ctx.Plan(); err != nil { @@ -8009,12 +8006,9 @@ func TestContext2Apply_varsEnv(t *testing.T) { ), }) - w, e := ctx.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %s", e) + diags := ctx.Validate() + if len(diags) != 0 { + t.Fatalf("bad: %#v", diags) } if _, err := ctx.Plan(); err != nil { diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 8240e435f..00d815901 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -3499,12 +3499,9 @@ func TestContext2Plan_resourceNestedCount(t *testing.T) { State: s, }) - w, e := ctx.Validate() - if len(w) > 0 { - t.Fatalf("warnings generated on validate: %#v", w) - } - if len(e) > 0 { - t.Fatalf("errors generated on validate: %#v", e) + diags := ctx.Validate() + if len(diags) != 0 { + t.Fatalf("bad: %#v", diags) } _, err := ctx.Refresh() @@ -3582,12 +3579,9 @@ output "out" { }) // if this ever fails to pass validate, add a resource to reference in the config - w, e := ctx.Validate() - if len(w) > 0 { - t.Fatalf("warnings generated on validate: %#v", w) - } - if len(e) > 0 { - t.Fatalf("errors generated on validate: %v", e) + diags := ctx.Validate() + if len(diags) != 0 { + t.Fatalf("bad: %#v", diags) } _, err := ctx.Refresh() @@ -3630,12 +3624,9 @@ resource "aws_instance" "foo" { }) // if this ever fails to pass validate, add a resource to reference in the config - w, e := ctx.Validate() - if len(w) > 0 { - t.Fatalf("warnings generated on validate: %#v", w) - } - if len(e) > 0 { - t.Fatalf("errors generated on validate: %v", e) + diags := ctx.Validate() + if len(diags) != 0 { + t.Fatalf("bad: %#v", diags) } _, err := ctx.Refresh() diff --git a/terraform/context_refresh_test.go b/terraform/context_refresh_test.go index 53e5534cd..9c4fad9a5 100644 --- a/terraform/context_refresh_test.go +++ b/terraform/context_refresh_test.go @@ -1042,12 +1042,9 @@ func TestContext2Validate(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %s", e) + diags := c.Validate() + if len(diags) != 0 { + t.Fatalf("bad: %#v", diags) } } diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go index 8cf847e6d..04dc7cb1a 100644 --- a/terraform/context_validate_test.go +++ b/terraform/context_validate_test.go @@ -18,12 +18,9 @@ func TestContext2Validate_badCount(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %#v", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -39,12 +36,9 @@ func TestContext2Validate_badVar(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %#v", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -63,12 +57,9 @@ func TestContext2Validate_varMapOverrideOld(t *testing.T) { }, }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %s", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -78,12 +69,9 @@ func TestContext2Validate_varNoDefaultExplicitType(t *testing.T) { Module: m, }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %#v", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -112,14 +100,9 @@ func TestContext2Validate_computedVar(t *testing.T) { return fmt.Errorf("Configure should not be called for provider") } - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - for _, err := range e { - t.Errorf("bad: %s", err) - } + diags := c.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -138,12 +121,9 @@ func TestContext2Validate_countComputed(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %s", e) + diags := c.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -159,12 +139,9 @@ func TestContext2Validate_countNegative(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %#v", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -180,12 +157,9 @@ func TestContext2Validate_countVariable(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %s", e) + diags := c.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -201,12 +175,9 @@ func TestContext2Validate_countVariableNoDefault(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) != 1 { - t.Fatalf("bad: %s", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -224,12 +195,9 @@ func TestContext2Validate_cycle(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("expected no warns, got: %#v", w) - } - if len(e) != 1 { - t.Fatalf("expected 1 err, got: %s", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } */ @@ -246,12 +214,9 @@ func TestContext2Validate_moduleBadOutput(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %s", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -267,12 +232,9 @@ func TestContext2Validate_moduleGood(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %#v", e) + diags := c.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -290,12 +252,9 @@ func TestContext2Validate_moduleBadResource(t *testing.T) { p.ValidateResourceReturnErrors = []error{fmt.Errorf("bad")} - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %#v", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -311,13 +270,9 @@ func TestContext2Validate_moduleDepsShouldNotCycle(t *testing.T) { ), }) - w, e := ctx.Validate() - - if len(w) > 0 { - t.Fatalf("expected no warnings, got: %s", w) - } - if len(e) > 0 { - t.Fatalf("expected no errors, got: %s", e) + diags := ctx.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -360,12 +315,9 @@ func TestContext2Validate_moduleProviderInheritOrphan(t *testing.T) { return nil, nil } - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %s", e) + diags := c.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -388,12 +340,9 @@ func TestContext2Validate_moduleProviderVar(t *testing.T) { return nil, c.CheckSet([]string{"foo"}) } - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %s", e) + diags := c.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -413,12 +362,9 @@ func TestContext2Validate_moduleProviderInheritUnused(t *testing.T) { return nil, c.CheckSet([]string{"foo"}) } - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %s", e) + diags := c.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -455,12 +401,9 @@ func TestContext2Validate_orphans(t *testing.T) { return nil, c.CheckSet([]string{"foo"}) } - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %s", e) + diags := c.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -478,15 +421,12 @@ func TestContext2Validate_providerConfig_bad(t *testing.T) { p.ValidateReturnErrors = []error{fmt.Errorf("bad")} - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) + diags := c.Validate() + if len(diags) != 1 { + t.Fatalf("wrong number of diagnostics %d; want %d", len(diags), 1) } - if len(e) == 0 { - t.Fatalf("bad: %s", e) - } - if !strings.Contains(fmt.Sprintf("%s", e), "bad") { - t.Fatalf("bad: %s", e) + if !strings.Contains(diags.Err().Error(), "bad") { + t.Fatalf("bad: %s", diags.Err().Error()) } } @@ -504,12 +444,9 @@ func TestContext2Validate_providerConfig_badEmpty(t *testing.T) { p.ValidateReturnErrors = []error{fmt.Errorf("bad")} - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %#v", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -525,12 +462,9 @@ func TestContext2Validate_providerConfig_good(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %#v", e) + diags := c.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -552,12 +486,9 @@ func TestContext2Validate_provisionerConfig_bad(t *testing.T) { pr.ValidateReturnErrors = []error{fmt.Errorf("bad")} - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %#v", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -583,12 +514,9 @@ func TestContext2Validate_provisionerConfig_good(t *testing.T) { }, }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %#v", e) + diags := c.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -604,12 +532,9 @@ func TestContext2Validate_requiredVar(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %s", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -627,12 +552,9 @@ func TestContext2Validate_resourceConfig_bad(t *testing.T) { p.ValidateResourceReturnErrors = []error{fmt.Errorf("bad")} - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %s", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -648,12 +570,9 @@ func TestContext2Validate_resourceConfig_good(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %#v", e) + diags := c.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -669,12 +588,9 @@ func TestContext2Validate_resourceNameSymbol(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %s", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -690,12 +606,9 @@ func TestContext2Validate_selfRef(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %s", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -711,12 +624,9 @@ func TestContext2Validate_selfRefMulti(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %#v", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -732,12 +642,9 @@ func TestContext2Validate_selfRefMultiAll(t *testing.T) { ), }) - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) == 0 { - t.Fatalf("bad: %#v", e) + diags := c.Validate() + if !diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -775,12 +682,9 @@ func TestContext2Validate_tainted(t *testing.T) { return nil, c.CheckSet([]string{"foo"}) } - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %#v", e) + diags := c.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -815,16 +719,9 @@ func TestContext2Validate_targetedDestroy(t *testing.T) { Destroy: true, }) - w, e := ctx.Validate() - if len(w) > 0 { - warnStr := "" - for _, v := range w { - warnStr = warnStr + " " + v - } - t.Fatalf("bad: %s", warnStr) - } - if len(e) > 0 { - t.Fatalf("bad: %s", e) + diags := ctx.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -875,12 +772,9 @@ func TestContext2Validate_interpolateVar(t *testing.T) { UIInput: input, }) - w, e := ctx.Validate() - if w != nil { - t.Log("warnings:", w) - } - if e != nil { - t.Fatal("err:", e) + diags := ctx.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -904,12 +798,9 @@ func TestContext2Validate_interpolateComputedModuleVarDef(t *testing.T) { UIInput: input, }) - w, e := ctx.Validate() - if w != nil { - t.Log("warnings:", w) - } - if e != nil { - t.Fatal("err:", e) + diags := ctx.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } } @@ -932,12 +823,9 @@ func TestContext2Validate_interpolateMap(t *testing.T) { UIInput: input, }) - w, e := ctx.Validate() - if w != nil { - t.Log("warnings:", w) - } - if e != nil { - t.Fatal("err:", e) + diags := ctx.Validate() + if diags.HasErrors() { + t.Fatalf("bad: %#v", diags) } }