Merge pull request #26444 from hashicorp/jbardin/value-checks

Add some log warnings and remove dead code

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

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