diff --git a/internal/command/plan_test.go b/internal/command/plan_test.go index 9e145e6bf3..2f81c421f3 100644 --- a/internal/command/plan_test.go +++ b/internal/command/plan_test.go @@ -437,10 +437,10 @@ func TestPlan_state(t *testing.T) { expected := cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("bar"), "ami": cty.NullVal(cty.String), - "network_interface": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{ + "network_interface": cty.ListValEmpty(cty.Object(map[string]cty.Type{ "device_index": cty.String, "description": cty.String, - }))), + })), }) if !expected.RawEquals(actual) { t.Fatalf("wrong prior state\ngot: %#v\nwant: %#v", actual, expected) @@ -479,10 +479,10 @@ func TestPlan_stateDefault(t *testing.T) { expected := cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("bar"), "ami": cty.NullVal(cty.String), - "network_interface": cty.NullVal(cty.List(cty.Object(map[string]cty.Type{ + "network_interface": cty.ListValEmpty(cty.Object(map[string]cty.Type{ "device_index": cty.String, "description": cty.String, - }))), + })), }) if !expected.RawEquals(actual) { t.Fatalf("wrong prior state\ngot: %#v\nwant: %#v", actual, expected) diff --git a/internal/terraform/context_refresh_test.go b/internal/terraform/context_refresh_test.go index ca456cee9e..9b4f4dab55 100644 --- a/internal/terraform/context_refresh_test.go +++ b/internal/terraform/context_refresh_test.go @@ -1598,3 +1598,88 @@ func TestContext2Refresh_dataSourceOrphan(t *testing.T) { t.Fatal("orphaned data source instance should not be read") } } + +// Legacy providers may return invalid null values for blocks, causing noise in +// the diff output and unexpected behavior with ignore_changes. Make sure +// refresh fixes these up before storing the state. +func TestContext2Refresh_reifyNullBlock(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +resource "test_resource" "foo" { +} +`, + }) + + p := new(MockProvider) + p.ReadResourceFn = func(req providers.ReadResourceRequest) providers.ReadResourceResponse { + // incorrectly return a null _set_block value + v := req.PriorState.AsValueMap() + v["set_block"] = cty.NullVal(v["set_block"].Type()) + return providers.ReadResourceResponse{NewState: cty.ObjectVal(v)} + } + + p.GetProviderSchemaResponse = getProviderSchemaResponseFromProviderSchema(&ProviderSchema{ + Provider: &configschema.Block{}, + ResourceTypes: map[string]*configschema.Block{ + "test_resource": { + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "set_block": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "a": {Type: cty.String, Optional: true}, + }, + }, + Nesting: configschema.NestingSet, + }, + }, + }, + }, + }) + p.PlanResourceChangeFn = testDiffFn + + fooAddr := addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_resource", + Name: "foo", + }.Instance(addrs.NoKey) + + state := states.NewState() + root := state.EnsureModule(addrs.RootModuleInstance) + root.SetResourceInstanceCurrent( + fooAddr, + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"foo", "network_interface":[]}`), + Dependencies: []addrs.ConfigResource{}, + }, + addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("test"), + Module: addrs.RootModule, + }, + ) + + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + + plan, diags := ctx.Plan(m, state, &PlanOpts{Mode: plans.RefreshOnlyMode}) + if diags.HasErrors() { + t.Fatalf("refresh errors: %s", diags.Err()) + } + + jsonState := plan.PriorState.ResourceInstance(fooAddr.Absolute(addrs.RootModuleInstance)).Current.AttrsJSON + + // the set_block should still be an empty container, and not null + expected := `{"id":"foo","set_block":[]}` + if string(jsonState) != expected { + t.Fatalf("invalid state\nexpected: %s\ngot: %s\n", expected, jsonState) + } +} diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 437d853021..4dabb7bedb 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -598,11 +598,23 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state return state, diags } + newState := objchange.NormalizeObjectFromLegacySDK(resp.NewState, schema) + if !newState.RawEquals(resp.NewState) { + // We had to fix up this object in some way, and we still need to + // accept any changes for compatibility, so all we can do is log a + // warning about the change. + log.Printf("[WARN] Provider %q produced an invalid new value containing null blocks for %q during refresh\n", n.ResolvedProvider.Provider, n.Addr) + } + + ret := state.DeepCopy() + ret.Value = newState + ret.Private = resp.Private + // 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. // In most cases these are not errors anyway, and represent "drift" from // external changes which will be handled by the subsequent plan. - if errs := objchange.AssertObjectCompatible(schema, priorVal, resp.NewState); len(errs) > 0 { + if errs := objchange.AssertObjectCompatible(schema, priorVal, ret.Value); len(errs) > 0 { var buf strings.Builder fmt.Fprintf(&buf, "[WARN] Provider %q produced an unexpected new value for %s during refresh.", n.ResolvedProvider.Provider.String(), absAddr) for _, err := range errs { @@ -611,10 +623,6 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state log.Print(buf.String()) } - ret := state.DeepCopy() - ret.Value = resp.NewState - ret.Private = resp.Private - // Call post-refresh hook diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { return h.PostRefresh(absAddr, hookGen, priorVal, ret.Value)