Merge pull request #28543 from hashicorp/jbardin/provider-diagnostics

Add more diagnostic configuration context to provider methods

terraform/node_provider.go

@@ -56,13 +56,13 @@ func (n *NodeApplyableProvider) ValidateProvider(ctx EvalContext, provider provi
 		return nil
 	}
 
-	resp := provider.GetProviderSchema()
+	schemaResp := provider.GetProviderSchema()
-	diags = diags.Append(resp.Diagnostics)
+	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)
+	validateResp := provider.ValidateProviderConfig(req)
-	if prepareResp.Diagnostics.HasErrors() {
+	diags = diags.Append(validateResp.Diagnostics.InConfigBody(configBody, n.Addr.String()))
-		if config == nil {
+	if diags.HasErrors() && config == nil {
-			// If there isn't an explicit "provider" block in the configuration,
+		// If there isn't an explicit "provider" block in the configuration,
-			// this error message won't be very clear. Add some detail to the
+		// this error message won't be very clear. Add some detail to the error
-			// error message in this case.
+		// message in this case.
-			diags = diags.Append(tfdiags.Sourceless(
+		diags = diags.Append(tfdiags.Sourceless(
-				tfdiags.Error,
+			tfdiags.Error,
-				"Invalid provider configuration",
+			"Invalid provider configuration",
-				fmt.Sprintf(providerConfigErr, prepareResp.Diagnostics.Err(), n.Addr.Provider),
+			fmt.Sprintf(providerConfigErr, n.Addr.Provider),
-			))
+		))
-			return diags
+	}
-		} else {
+
-			return diags.Append(prepareResp.Diagnostics)
+	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
+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.
-
-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.
 `

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)
+		}
+	}
+
+}

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() {
+	diags = diags.Append(validateResp.Diagnostics.InConfigBody(config.Config, n.Addr.String()))
-		return newVal, 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