From c52672cbbe4322694e318f676c51c02b1dd5d3da Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 30 Sep 2020 17:40:39 -0400 Subject: [PATCH 1/2] log inconsistencies during refresh We can't make these errors because there's no way to exempt legacy providers from the check, but we can at least log them for troubleshooting. --- terraform/eval_refresh.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/terraform/eval_refresh.go b/terraform/eval_refresh.go index 021da2b5d..737484c45 100644 --- a/terraform/eval_refresh.go +++ b/terraform/eval_refresh.go @@ -3,12 +3,14 @@ package terraform import ( "fmt" "log" + "strings" "github.com/zclconf/go-cty/cty" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" + "github.com/hashicorp/terraform/plans/objchange" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" @@ -122,6 +124,17 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) { return nil, diags.Err() } + // We have no way to exempt provider using the legacy SDK from this check, + // so we can only log inconsistencies with the updated state values. + if errs := objchange.AssertObjectCompatible(schema, priorVal, resp.NewState); len(errs) > 0 { + var buf strings.Builder + fmt.Fprintf(&buf, "[WARN] Provider %q produced an unexpected new value for %s during refresh.", n.ProviderAddr.Provider.String(), absAddr) + for _, err := range errs { + fmt.Fprintf(&buf, "\n - %s", tfdiags.FormatError(err)) + } + log.Print(buf.String()) + } + newState := state.DeepCopy() newState.Value = resp.NewState newState.Private = resp.Private From 98b568ad01297c4a3c81f14d4bedf51bcb1156a1 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 30 Sep 2020 18:04:40 -0400 Subject: [PATCH 2/2] remove unused refresh node There is no longer a refresh walk, so we no longer use this type. --- terraform/eval_read_data.go | 114 ------------------------------------ 1 file changed, 114 deletions(-) diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go index 9235130bd..33eac066d 100644 --- a/terraform/eval_read_data.go +++ b/terraform/eval_read_data.go @@ -10,7 +10,6 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/plans" - "github.com/hashicorp/terraform/plans/objchange" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" @@ -194,116 +193,3 @@ func (n *evalReadData) providerMetas(ctx EvalContext) (cty.Value, tfdiags.Diagno } return metaConfigVal, diags } - -// evalReadDataRefresh is an EvalNode implementation that handled the data -// resource lifecycle during refresh -type evalReadDataRefresh struct { - evalReadData -} - -func (n *evalReadDataRefresh) Eval(ctx EvalContext) (interface{}, error) { - var diags tfdiags.Diagnostics - - if n.ProviderSchema == nil || *n.ProviderSchema == nil { - return nil, fmt.Errorf("provider schema not available for %s", n.Addr) - } - - absAddr := n.Addr.Absolute(ctx.Path()) - config := *n.Config - providerSchema := *n.ProviderSchema - schema, _ := providerSchema.SchemaForResourceAddr(n.Addr.ContainingResource()) - if schema == nil { - // Should be caught during validation, so we don't bother with a pretty error here - return nil, fmt.Errorf("provider %q does not support data source %q", n.ProviderAddr.Provider.String(), n.Addr.Resource.Type) - } - - objTy := schema.ImpliedType() - priorVal := cty.NullVal(objTy) - if n.State != nil && *n.State != nil { - priorVal = (*n.State).Value - } - - forEach, _ := evaluateForEachExpression(config.ForEach, ctx) - keyData := EvalDataForInstanceKey(n.Addr.Key, forEach) - - configVal, _, configDiags := ctx.EvaluateBlock(config.Config, schema, nil, keyData) - diags = diags.Append(configDiags) - if configDiags.HasErrors() { - return nil, diags.ErrWithWarnings() - } - - configKnown := configVal.IsWhollyKnown() - // If our configuration contains any unknown values, then we must defer the - // read until plan or apply. If we've never read this data source and we - // have any depends_on, we will have to defer reading until plan to resolve - // the dependency changes. - // Assuming we can read the data source with depends_on if we have - // existing state is a compromise to prevent data sources from continually - // showing a diff. We have to make the assumption that if we have a prior - // state, since there are no prior dependency changes happening during - // refresh, that we can read this resource. If there are dependency updates - // in the config, they we be discovered in plan and the data source will be - // read again. - if !configKnown || (priorVal.IsNull() && n.forcePlanRead()) { - if configKnown { - log.Printf("[TRACE] evalReadDataRefresh: %s configuration is fully known, but we're forcing a read plan to be created", absAddr) - } else { - log.Printf("[TRACE] evalReadDataRefresh: %s configuration not fully known yet, so deferring to apply phase", absAddr) - } - - // We need to store a change so tat other references to this data - // source can resolve correctly, since the state is not going to be up - // to date. - *n.OutputChange = &plans.ResourceInstanceChange{ - Addr: absAddr, - ProviderAddr: n.ProviderAddr, - Change: plans.Change{ - Action: plans.Read, - Before: priorVal, - After: objchange.PlannedDataResourceObject(schema, configVal), - }, - } - - *n.State = &states.ResourceInstanceObject{ - Value: cty.NullVal(objTy), - Status: states.ObjectPlanned, - } - - return nil, diags.ErrWithWarnings() - } - - if err := ctx.Hook(func(h Hook) (HookAction, error) { - return h.PreRefresh(absAddr, states.CurrentGen, priorVal) - }); err != nil { - diags = diags.Append(err) - return nil, diags.ErrWithWarnings() - } - - newVal, readDiags := n.readDataSource(ctx, configVal) - diags = diags.Append(readDiags) - if diags.HasErrors() { - return nil, diags.ErrWithWarnings() - } - - // This may still have been refreshed with references to resources that - // will be updated, but that will be caught as a change during plan. - *n.State = &states.ResourceInstanceObject{ - Value: newVal, - Status: states.ObjectReady, - } - - if err := ctx.Hook(func(h Hook) (HookAction, error) { - return h.PostRefresh(absAddr, states.CurrentGen, priorVal, newVal) - }); err != nil { - diags = diags.Append(err) - } - - return nil, diags.ErrWithWarnings() -} - -// forcePlanRead determines if we need to override the usual behavior of -// immediately reading from the data source where possible, instead forcing us -// to generate a plan. -func (n *evalReadDataRefresh) forcePlanRead() bool { - return len(n.Config.DependsOn) > 0 || len(n.dependsOn) > 0 || n.forceDependsOn -}