diff --git a/terraform/node_provider.go b/terraform/node_provider.go index 3e70cee08..5b06a25a6 100644 --- a/terraform/node_provider.go +++ b/terraform/node_provider.go @@ -56,13 +56,13 @@ func (n *NodeApplyableProvider) ValidateProvider(ctx EvalContext, provider provi return nil } - resp := provider.GetProviderSchema() - diags = diags.Append(resp.Diagnostics) + schemaResp := provider.GetProviderSchema() + diags = diags.Append(schemaResp.Diagnostics.InConfigBody(configBody, n.Addr.String())) if diags.HasErrors() { return diags } - configSchema := resp.Provider.Block + configSchema := schemaResp.Provider.Block if configSchema == nil { // Should never happen in real code, but often comes up in tests where // mock schemas are being used that tend to be incomplete. @@ -85,7 +85,7 @@ func (n *NodeApplyableProvider) ValidateProvider(ctx EvalContext, provider provi } validateResp := provider.ValidateProviderConfig(req) - diags = diags.Append(validateResp.Diagnostics) + diags = diags.Append(validateResp.Diagnostics.InConfigBody(configBody, n.Addr.String())) return diags } @@ -99,7 +99,7 @@ func (n *NodeApplyableProvider) ConfigureProvider(ctx EvalContext, provider prov configBody := buildProviderConfig(ctx, n.Addr, config) resp := provider.GetProviderSchema() - diags = diags.Append(resp.Diagnostics) + diags = diags.Append(resp.Diagnostics.InConfigBody(configBody, n.Addr.String())) if diags.HasErrors() { return diags } @@ -133,53 +133,44 @@ func (n *NodeApplyableProvider) ConfigureProvider(ctx EvalContext, provider prov // ValidateProviderConfig is only used for validation. We are intentionally // ignoring the PreparedConfig field to maintain existing behavior. - prepareResp := provider.ValidateProviderConfig(req) - if prepareResp.Diagnostics.HasErrors() { - if config == nil { - // If there isn't an explicit "provider" block in the configuration, - // this error message won't be very clear. Add some detail to the - // error message in this case. - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Invalid provider configuration", - fmt.Sprintf(providerConfigErr, prepareResp.Diagnostics.Err(), n.Addr.Provider), - )) - return diags - } else { - return diags.Append(prepareResp.Diagnostics) - } + validateResp := provider.ValidateProviderConfig(req) + diags = diags.Append(validateResp.Diagnostics.InConfigBody(configBody, n.Addr.String())) + if diags.HasErrors() && config == nil { + // If there isn't an explicit "provider" block in the configuration, + // this error message won't be very clear. Add some detail to the error + // message in this case. + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid provider configuration", + fmt.Sprintf(providerConfigErr, n.Addr.Provider), + )) + } + + if diags.HasErrors() { + return diags } - diags = diags.Append(prepareResp.Diagnostics) // If the provider returns something different, log a warning to help // indicate to provider developers that the value is not used. - preparedCfg := prepareResp.PreparedConfig + preparedCfg := validateResp.PreparedConfig if preparedCfg != cty.NilVal && !preparedCfg.IsNull() && !preparedCfg.RawEquals(unmarkedConfigVal) { log.Printf("[WARN] ValidateProviderConfig from %q changed the config value, but that value is unused", n.Addr) } configDiags := ctx.ConfigureProvider(n.Addr, unmarkedConfigVal) - if configDiags.HasErrors() { - if config == nil { - // If there isn't an explicit "provider" block in the configuration, - // this error message won't be very clear. Add some detail to the - // error message in this case. - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Invalid provider configuration", - fmt.Sprintf(providerConfigErr, configDiags.InConfigBody(configBody, n.Addr.String()).Err(), n.Addr.Provider), - )) - return diags - } else { - return diags.Append(configDiags.InConfigBody(configBody, n.Addr.String())) - } - } diags = diags.Append(configDiags.InConfigBody(configBody, n.Addr.String())) - + if diags.HasErrors() && config == nil { + // If there isn't an explicit "provider" block in the configuration, + // this error message won't be very clear. Add some detail to the error + // message in this case. + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid provider configuration", + fmt.Sprintf(providerConfigErr, n.Addr.Provider), + )) + } return diags } -const providerConfigErr = `%s - -Provider %q requires explicit configuration. Add a provider block to the root module and configure the provider's required arguments as described in the provider documentation. +const providerConfigErr = `Provider %q requires explicit configuration. Add a provider block to the root module and configure the provider's required arguments as described in the provider documentation. ` diff --git a/terraform/node_provider_test.go b/terraform/node_provider_test.go index f52f3b282..4eafe49fa 100644 --- a/terraform/node_provider_test.go +++ b/terraform/node_provider_test.go @@ -316,7 +316,8 @@ func TestNodeApplyableProvider_ConfigProvider(t *testing.T) { provider.ValidateProviderConfigFn = func(req providers.ValidateProviderConfigRequest) (resp providers.ValidateProviderConfigResponse) { region := req.Config.GetAttr("region") if region.IsNull() { - resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("value is not found")) + resp.Diagnostics = resp.Diagnostics.Append( + tfdiags.WholeContainingBody(tfdiags.Error, "value is not found", "you did not supply a required value")) } return } @@ -376,7 +377,7 @@ func TestNodeApplyableProvider_ConfigProvider(t *testing.T) { if !diags.HasErrors() { t.Fatal("missing expected error with invalid config") } - if diags.Err().Error() != "value is not found" { + if !strings.Contains(diags.Err().Error(), "value is not found") { t.Errorf("wrong diagnostic: %s", diags.Err()) } }) @@ -470,3 +471,29 @@ func TestNodeApplyableProvider_ConfigProvider_config_fn_err(t *testing.T) { } }) } + +func TestGetSchemaError(t *testing.T) { + provider := &MockProvider{ + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + Diagnostics: tfdiags.Diagnostics.Append(nil, tfdiags.WholeContainingBody(tfdiags.Error, "oops", "error")), + }, + } + + providerAddr := mustProviderConfig(`provider["terraform.io/some/provider"]`) + ctx := &MockEvalContext{ProviderProvider: provider} + ctx.installSimpleEval() + node := NodeApplyableProvider{ + NodeAbstractProvider: &NodeAbstractProvider{ + Addr: providerAddr, + }, + } + + diags := node.ConfigureProvider(ctx, provider, false) + for _, d := range diags { + desc := d.Description() + if desc.Address != providerAddr.String() { + t.Fatalf("missing provider address from diagnostics: %#v", desc) + } + } + +} diff --git a/terraform/node_resource_abstract_instance.go b/terraform/node_resource_abstract_instance.go index 59227b55a..e27f84e67 100644 --- a/terraform/node_resource_abstract_instance.go +++ b/terraform/node_resource_abstract_instance.go @@ -478,6 +478,10 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, state *states.Re } resp := provider.ReadResource(providerReq) + if n.Config != nil { + resp.Diagnostics = resp.Diagnostics.InConfigBody(n.Config.Config, n.Addr.String()) + } + diags = diags.Append(resp.Diagnostics) if diags.HasErrors() { return state, diags @@ -626,8 +630,8 @@ func (n *NodeAbstractResourceInstance) plan( }, ) - if validateResp.Diagnostics.HasErrors() { - diags = diags.Append(validateResp.Diagnostics.InConfigBody(config.Config, n.Addr.String())) + diags = diags.Append(validateResp.Diagnostics.InConfigBody(config.Config, n.Addr.String())) + if diags.HasErrors() { return plan, state, diags } @@ -1195,8 +1199,9 @@ func (n *NodeAbstractResourceInstance) readDataSource(ctx EvalContext, configVal Config: configVal, }, ) - if validateResp.Diagnostics.HasErrors() { - return newVal, validateResp.Diagnostics.InConfigBody(config.Config, n.Addr.String()) + diags = diags.Append(validateResp.Diagnostics.InConfigBody(config.Config, n.Addr.String())) + if diags.HasErrors() { + return newVal, diags } // If we get down here then our configuration is complete and we're read