From e7b2d98ca39df9119e6ee04b524c77b68f749478 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 2 Nov 2020 18:23:00 -0500 Subject: [PATCH 1/2] Use prepared config in provider.Configure Core is only using the PrepareProviderConfig call for the validation part of the method, but we should be re-validating the final config immediately before Configure. This change elects to not start using the PreparedConfig here, since there is no useful reason for it at this point, and it would introduce a functional difference between terraform releases that can be avoided. --- command/apply_test.go | 3 --- terraform/context_plan_test.go | 5 +++++ terraform/node_provider.go | 28 +++++++++++++++++++++++++--- terraform/provider_mock.go | 1 + 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/command/apply_test.go b/command/apply_test.go index 407d00305..1f75fc6fa 100644 --- a/command/apply_test.go +++ b/command/apply_test.go @@ -798,9 +798,6 @@ func TestApply_planNoModuleFiles(t *testing.T) { planPath, } apply.Run(args) - if p.PrepareProviderConfigCalled { - t.Fatal("Prepare provider config should not be called with a plan") - } } func TestApply_refresh(t *testing.T) { diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index be7fda7ef..d6fd5fe74 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -78,6 +78,11 @@ func TestContext2Plan_basic(t *testing.T) { t.Fatal("unknown instance:", i) } } + + if !p.PrepareProviderConfigCalled { + t.Fatal("provider config was not checked before Configure") + } + } func TestContext2Plan_createBefore_deposed(t *testing.T) { diff --git a/terraform/node_provider.go b/terraform/node_provider.go index 3ea0485d4..9566a136b 100644 --- a/terraform/node_provider.go +++ b/terraform/node_provider.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/tfdiags" + "github.com/zclconf/go-cty/cty" ) // NodeApplyableProvider represents a provider during an apply. @@ -108,8 +109,29 @@ func (n *NodeApplyableProvider) ConfigureProvider(ctx EvalContext, provider prov return diags } - configDiags := ctx.ConfigureProvider(n.Addr, configVal) - configDiags = configDiags.InConfigBody(configBody) + // Allow the provider to validate and insert any defaults into the full + // configuration. + req := providers.PrepareProviderConfigRequest{ + Config: configVal, + } - return configDiags + // 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 the provider returns something different, log a warning to help + // indicate to provider developers that the value is not used. + preparedCfg := prepareResp.PreparedConfig + if preparedCfg != cty.NilVal && !preparedCfg.IsNull() && !preparedCfg.RawEquals(configVal) { + log.Printf("[WARN] PrepareProviderConfig from %q changed the config value, but that value is unused", n.Addr) + } + + configDiags := ctx.ConfigureProvider(n.Addr, configVal) + diags = diags.Append(configDiags.InConfigBody(configBody)) + + return diags } diff --git a/terraform/provider_mock.go b/terraform/provider_mock.go index bf3aca327..2a6f6dbf0 100644 --- a/terraform/provider_mock.go +++ b/terraform/provider_mock.go @@ -133,6 +133,7 @@ func (p *MockProvider) PrepareProviderConfig(r providers.PrepareProviderConfigRe if p.PrepareProviderConfigFn != nil { return p.PrepareProviderConfigFn(r) } + p.PrepareProviderConfigResponse.PreparedConfig = r.Config return p.PrepareProviderConfigResponse } From 73680546a8c43a20d76671372163eca07412f2cb Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 4 Nov 2020 13:02:04 -0500 Subject: [PATCH 2/2] udpate PrepareProviderConfig docs Indicate that the PreparedConfig is not used by core. --- providers/provider.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/providers/provider.go b/providers/provider.go index 2d9db44bb..24f06a6cb 100644 --- a/providers/provider.go +++ b/providers/provider.go @@ -14,8 +14,10 @@ type Interface interface { // GetSchema returns the complete schema for the provider. GetSchema() GetSchemaResponse - // PrepareProviderConfig allows the provider to validate the configuration - // values, and set or override any values with defaults. + // PrepareProviderConfig allows the provider to validate the configuration. + // The PrepareProviderConfigResponse.PreparedConfig field is unused. The + // final configuration is not stored in the state, and any modifications + // that need to be made must be made during the Configure method call. PrepareProviderConfig(PrepareProviderConfigRequest) PrepareProviderConfigResponse // ValidateResourceTypeConfig allows the provider to validate the resource @@ -99,7 +101,7 @@ type PrepareProviderConfigRequest struct { } type PrepareProviderConfigResponse struct { - // PreparedConfig is the configuration as prepared by the provider. + // PreparedConfig is unused. PreparedConfig cty.Value // Diagnostics contains any warnings or errors from the method call. Diagnostics tfdiags.Diagnostics