diff --git a/terraform/eval_context_mock.go b/terraform/eval_context_mock.go index 11ae6941f5..ca8b84a6a2 100644 --- a/terraform/eval_context_mock.go +++ b/terraform/eval_context_mock.go @@ -55,6 +55,9 @@ type MockEvalContext struct { SetProviderInputAddr addrs.AbsProviderConfig SetProviderInputValues map[string]cty.Value + ConfigureProviderFn func( + addr addrs.AbsProviderConfig, + cfg cty.Value) tfdiags.Diagnostics // overrides the other values below, if set ConfigureProviderCalled bool ConfigureProviderAddr addrs.AbsProviderConfig ConfigureProviderConfig cty.Value @@ -183,9 +186,13 @@ func (c *MockEvalContext) CloseProvider(addr addrs.AbsProviderConfig) error { } func (c *MockEvalContext) ConfigureProvider(addr addrs.AbsProviderConfig, cfg cty.Value) tfdiags.Diagnostics { + c.ConfigureProviderCalled = true c.ConfigureProviderAddr = addr c.ConfigureProviderConfig = cfg + if c.ConfigureProviderFn != nil { + return c.ConfigureProviderFn(addr, cfg) + } return c.ConfigureProviderDiags } diff --git a/terraform/node_provider.go b/terraform/node_provider.go index 98a8e85a01..585417340f 100644 --- a/terraform/node_provider.go +++ b/terraform/node_provider.go @@ -63,10 +63,22 @@ func (n *NodeApplyableProvider) ValidateProvider(ctx EvalContext, provider provi } configVal, _, evalDiags := ctx.EvaluateBlock(configBody, configSchema, nil, EvalDataForNoInstanceKey) - diags = diags.Append(evalDiags) if evalDiags.HasErrors() { - return diags + if n.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, evalDiags.Err(), n.Addr.Provider), + )) + return diags + } else { + return diags.Append(evalDiags) + } } + diags = diags.Append(evalDiags) // If our config value contains any marked values, ensure those are // stripped out before sending this to the provider @@ -126,10 +138,22 @@ func (n *NodeApplyableProvider) ConfigureProvider(ctx EvalContext, provider prov // PrepareProviderConfig is only used for validation. We are intentionally // ignoring the PreparedConfig field to maintain existing behavior. prepareResp := provider.PrepareProviderConfig(req) - diags = diags.Append(prepareResp.Diagnostics) - if diags.HasErrors() { - return diags + 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) + } } + 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. @@ -139,7 +163,27 @@ func (n *NodeApplyableProvider) ConfigureProvider(ctx EvalContext, provider prov } 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).Err(), n.Addr.Provider), + )) + return diags + } else { + return diags.Append(configDiags.InConfigBody(configBody)) + } + } diags = diags.Append(configDiags.InConfigBody(configBody)) 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. +` diff --git a/terraform/node_provider_test.go b/terraform/node_provider_test.go index d85bd64310..04d566f5c9 100644 --- a/terraform/node_provider_test.go +++ b/terraform/node_provider_test.go @@ -1,10 +1,16 @@ package terraform import ( + "fmt" + "strings" "testing" + "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" + "github.com/hashicorp/terraform/configs/configschema" + "github.com/hashicorp/terraform/providers" + "github.com/hashicorp/terraform/tfdiags" "github.com/zclconf/go-cty/cty" ) @@ -189,3 +195,235 @@ func TestNodeApplyableProviderExecute_sensitiveValidate(t *testing.T) { t.Errorf("wrong configuration value\ngot: %#v\nwant: %#v", got, want) } } + +func TestNodeApplyableProvider_Validate(t *testing.T) { + provider := &MockProvider{ + GetSchemaReturn: &ProviderSchema{ + Provider: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "region": { + Type: cty.String, + Required: true, + }, + }, + }, + }, + } + ctx := &MockEvalContext{ProviderProvider: provider} + ctx.installSimpleEval() + + t.Run("valid", func(t *testing.T) { + config := &configs.Provider{ + Name: "test", + Config: configs.SynthBody("", map[string]cty.Value{ + "region": cty.StringVal("mars"), + }), + } + + node := NodeApplyableProvider{ + NodeAbstractProvider: &NodeAbstractProvider{ + Addr: mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`), + Config: config, + }, + } + + diags := node.ValidateProvider(ctx, provider) + if diags.HasErrors() { + t.Errorf("unexpected error with valid config: %s", diags.Err()) + } + }) + + t.Run("missing required config", func(t *testing.T) { + node := NodeApplyableProvider{ + NodeAbstractProvider: &NodeAbstractProvider{ + Addr: mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`), + }, + } + + diags := node.ValidateProvider(ctx, provider) + if !diags.HasErrors() { + t.Error("missing expected error with invalid config") + } + }) +} + +//This test specifically tests responses from the +//providers.PrepareProviderConfigFn. See +//TestNodeApplyableProvider_ConfigProvider_config_fn_err for +//providers.ConfigureRequest responses. +func TestNodeApplyableProvider_ConfigProvider(t *testing.T) { + provider := &MockProvider{ + GetSchemaReturn: &ProviderSchema{ + Provider: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "region": { + Type: cty.String, + Optional: true, + }, + }, + }, + }, + } + // For this test, we're returning an error for an optional argument. This + // can happen for example if an argument is only conditionally required. + provider.PrepareProviderConfigFn = func(req providers.PrepareProviderConfigRequest) (resp providers.PrepareProviderConfigResponse) { + region := req.Config.GetAttr("region") + if region.IsNull() { + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("value is not found")) + } + return + } + ctx := &MockEvalContext{ProviderProvider: provider} + ctx.installSimpleEval() + + t.Run("valid", func(t *testing.T) { + config := &configs.Provider{ + Name: "test", + Config: configs.SynthBody("", map[string]cty.Value{ + "region": cty.StringVal("mars"), + }), + } + + node := NodeApplyableProvider{ + NodeAbstractProvider: &NodeAbstractProvider{ + Addr: mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`), + Config: config, + }, + } + + diags := node.ConfigureProvider(ctx, provider, false) + if diags.HasErrors() { + t.Errorf("unexpected error with valid config: %s", diags.Err()) + } + }) + + t.Run("missing required config (no config at all)", func(t *testing.T) { + node := NodeApplyableProvider{ + NodeAbstractProvider: &NodeAbstractProvider{ + Addr: mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`), + }, + } + + diags := node.ConfigureProvider(ctx, provider, false) + if !diags.HasErrors() { + t.Fatal("missing expected error with nil config") + } + if !strings.Contains(diags.Err().Error(), "requires explicit configuration") { + t.Errorf("diagnostic is missing \"requires explicit configuration\" message: %s", diags.Err()) + } + }) + + t.Run("missing required config", func(t *testing.T) { + config := &configs.Provider{ + Name: "test", + Config: hcl.EmptyBody(), + } + node := NodeApplyableProvider{ + NodeAbstractProvider: &NodeAbstractProvider{ + Addr: mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`), + Config: config, + }, + } + + diags := node.ConfigureProvider(ctx, provider, false) + if !diags.HasErrors() { + t.Fatal("missing expected error with invalid config") + } + if diags.Err().Error() != "value is not found" { + t.Errorf("wrong diagnostic: %s", diags.Err()) + } + }) + +} + +//This test is similar to TestNodeApplyableProvider_ConfigProvider, but tests responses from the providers.ConfigureRequest +func TestNodeApplyableProvider_ConfigProvider_config_fn_err(t *testing.T) { + provider := &MockProvider{ + GetSchemaReturn: &ProviderSchema{ + Provider: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "region": { + Type: cty.String, + Optional: true, + }, + }, + }, + }, + } + ctx := &MockEvalContext{ProviderProvider: provider} + ctx.installSimpleEval() + // For this test, provider.PrepareConfigFn will succeed every time but the + // ctx.ConfigureProviderFn will return an error if a value is not found. + // + // This is an unlikely but real situation that occurs: + // https://github.com/hashicorp/terraform/issues/23087 + ctx.ConfigureProviderFn = func(addr addrs.AbsProviderConfig, cfg cty.Value) (diags tfdiags.Diagnostics) { + if cfg.IsNull() { + diags = diags.Append(fmt.Errorf("no config provided")) + } else { + region := cfg.GetAttr("region") + if region.IsNull() { + diags = diags.Append(fmt.Errorf("value is not found")) + } + } + return + } + + t.Run("valid", func(t *testing.T) { + config := &configs.Provider{ + Name: "test", + Config: configs.SynthBody("", map[string]cty.Value{ + "region": cty.StringVal("mars"), + }), + } + + node := NodeApplyableProvider{ + NodeAbstractProvider: &NodeAbstractProvider{ + Addr: mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`), + Config: config, + }, + } + + diags := node.ConfigureProvider(ctx, provider, false) + if diags.HasErrors() { + t.Errorf("unexpected error with valid config: %s", diags.Err()) + } + }) + + t.Run("missing required config (no config at all)", func(t *testing.T) { + node := NodeApplyableProvider{ + NodeAbstractProvider: &NodeAbstractProvider{ + Addr: mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`), + }, + } + + diags := node.ConfigureProvider(ctx, provider, false) + if !diags.HasErrors() { + t.Fatal("missing expected error with nil config") + } + if !strings.Contains(diags.Err().Error(), "requires explicit configuration") { + t.Errorf("diagnostic is missing \"requires explicit configuration\" message: %s", diags.Err()) + } + }) + + t.Run("missing required config", func(t *testing.T) { + config := &configs.Provider{ + Name: "test", + Config: hcl.EmptyBody(), + } + node := NodeApplyableProvider{ + NodeAbstractProvider: &NodeAbstractProvider{ + Addr: mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`), + Config: config, + }, + } + + diags := node.ConfigureProvider(ctx, provider, false) + if !diags.HasErrors() { + t.Fatal("missing expected error with invalid config") + } + if diags.Err().Error() != "value is not found" { + t.Errorf("wrong diagnostic: %s", diags.Err()) + } + }) +}