diff --git a/internal/command/jsonformat/state_test.go b/internal/command/jsonformat/state_test.go index 69030a1e48..aeb3cf8dd2 100644 --- a/internal/command/jsonformat/state_test.go +++ b/internal/command/jsonformat/state_test.go @@ -252,7 +252,6 @@ func basicState(t *testing.T) *states.State { t.Errorf("root module is nil; want valid object") } - rootModule.SetLocalValue("foo", cty.StringVal("foo value")) state.SetOutputValue( addrs.OutputValue{Name: "bar"}.Absolute(addrs.RootModuleInstance), cty.StringVal("bar value"), false, diff --git a/internal/states/module.go b/internal/states/module.go index e9f6355b38..2c22cea8d0 100644 --- a/internal/states/module.go +++ b/internal/states/module.go @@ -5,7 +5,6 @@ package states import ( "github.com/hashicorp/terraform/internal/addrs" - "github.com/zclconf/go-cty/cty" ) // Module is a container for the states of objects within a particular module. @@ -15,18 +14,13 @@ type Module struct { // Resources contains the state for each resource. The keys in this map are // an implementation detail and must not be used by outside callers. Resources map[string]*Resource - - // LocalValues contains the value for each named output value. The keys - // in this map are local value names. - LocalValues map[string]cty.Value } // NewModule constructs an empty module state for the given module address. func NewModule(addr addrs.ModuleInstance) *Module { return &Module{ - Addr: addr, - Resources: map[string]*Resource{}, - LocalValues: map[string]cty.Value{}, + Addr: addr, + Resources: map[string]*Resource{}, } } @@ -247,19 +241,6 @@ func (ms *Module) maybeRestoreResourceInstanceDeposed(addr addrs.ResourceInstanc return true } -// SetLocalValue writes a local value into the state, overwriting any -// existing value of the same name. -func (ms *Module) SetLocalValue(name string, value cty.Value) { - ms.LocalValues[name] = value -} - -// RemoveLocalValue removes the local value of the given name from the state, -// if it exists. This method is a no-op if there is no value of the given -// name. -func (ms *Module) RemoveLocalValue(name string) { - delete(ms.LocalValues, name) -} - // PruneResourceHusks is a specialized method that will remove any Resource // objects that do not contain any instances, even if they have an EachMode. // diff --git a/internal/states/state.go b/internal/states/state.go index 151155ed0c..4766fd99b0 100644 --- a/internal/states/state.go +++ b/internal/states/state.go @@ -7,10 +7,9 @@ import ( "fmt" "sort" - "github.com/zclconf/go-cty/cty" - "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/getproviders" + "github.com/zclconf/go-cty/cty" ) // State is the top-level type of a Terraform state. @@ -323,16 +322,6 @@ func (s *State) RemoveOutputValue(addr addrs.AbsOutputValue) { delete(s.RootOutputValues, addr.OutputValue.Name) } -// LocalValue returns the value of the named local value with the given address, -// or cty.NilVal if no such value is tracked in the state. -func (s *State) LocalValue(addr addrs.AbsLocalValue) cty.Value { - ms := s.Module(addr.Module) - if ms == nil { - return cty.NilVal - } - return ms.LocalValues[addr.LocalValue.Name] -} - // ProviderAddrs returns a list of all of the provider configuration addresses // referenced throughout the receiving state. // diff --git a/internal/states/state_deepcopy.go b/internal/states/state_deepcopy.go index c1984834d9..5bd88f7777 100644 --- a/internal/states/state_deepcopy.go +++ b/internal/states/state_deepcopy.go @@ -59,16 +59,10 @@ func (ms *Module) DeepCopy() *Module { for k, r := range ms.Resources { resources[k] = r.DeepCopy() } - localValues := make(map[string]cty.Value, len(ms.LocalValues)) - for k, v := range ms.LocalValues { - // cty.Value is immutable, so we don't need to copy these. - localValues[k] = v - } return &Module{ - Addr: ms.Addr, // technically mutable, but immutable by convention - Resources: resources, - LocalValues: localValues, + Addr: ms.Addr, // technically mutable, but immutable by convention + Resources: resources, } } diff --git a/internal/states/state_test.go b/internal/states/state_test.go index 73ca8b83a6..ef49bb95f2 100644 --- a/internal/states/state_test.go +++ b/internal/states/state_test.go @@ -28,7 +28,6 @@ func TestState(t *testing.T) { t.Errorf("root module is nil; want valid object") } - rootModule.SetLocalValue("foo", cty.StringVal("foo value")) state.SetOutputValue( addrs.OutputValue{Name: "bar"}.Absolute(addrs.RootModuleInstance), cty.StringVal("bar value"), false, @@ -91,9 +90,6 @@ func TestState(t *testing.T) { Modules: map[string]*Module{ "": { Addr: addrs.RootModuleInstance, - LocalValues: map[string]cty.Value{ - "foo": cty.StringVal("foo value"), - }, Resources: map[string]*Resource{ "test_thing.baz": { Addr: addrs.Resource{ @@ -120,19 +116,16 @@ func TestState(t *testing.T) { }, }, "module.child": { - Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey), - LocalValues: map[string]cty.Value{}, - Resources: map[string]*Resource{}, + Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey), + Resources: map[string]*Resource{}, }, `module.multi["a"]`: { - Addr: addrs.RootModuleInstance.Child("multi", addrs.StringKey("a")), - LocalValues: map[string]cty.Value{}, - Resources: map[string]*Resource{}, + Addr: addrs.RootModuleInstance.Child("multi", addrs.StringKey("a")), + Resources: map[string]*Resource{}, }, `module.multi["b"]`: { - Addr: addrs.RootModuleInstance.Child("multi", addrs.StringKey("b")), - LocalValues: map[string]cty.Value{}, - Resources: map[string]*Resource{}, + Addr: addrs.RootModuleInstance.Child("multi", addrs.StringKey("b")), + Resources: map[string]*Resource{}, }, }, } @@ -189,7 +182,6 @@ func TestStateDeepCopy(t *testing.T) { t.Fatalf("root module is nil; want valid object") } - rootModule.SetLocalValue("foo", cty.StringVal("foo value")) state.SetOutputValue( addrs.OutputValue{Name: "bar"}.Absolute(rootModule.Addr), cty.StringVal("bar value"), false, diff --git a/internal/states/sync.go b/internal/states/sync.go index 84839f420c..c3c480a4ee 100644 --- a/internal/states/sync.go +++ b/internal/states/sync.go @@ -103,44 +103,6 @@ func (s *SyncState) RemoveOutputValue(addr addrs.AbsOutputValue) { s.state.RemoveOutputValue(addr) } -// LocalValue returns the current value associated with the given local value -// address. -func (s *SyncState) LocalValue(addr addrs.AbsLocalValue) cty.Value { - s.lock.RLock() - // cty.Value is immutable, so we don't need any extra copying here. - ret := s.state.LocalValue(addr) - s.lock.RUnlock() - return ret -} - -// SetLocalValue writes a given output value into the state, overwriting -// any existing value of the same name. -// -// If the module containing the local value is not yet tracked in state then it -// will be added as a side-effect. -func (s *SyncState) SetLocalValue(addr addrs.AbsLocalValue, value cty.Value) { - defer s.beginWrite()() - - ms := s.state.EnsureModule(addr.Module) - ms.SetLocalValue(addr.LocalValue.Name, value) -} - -// RemoveLocalValue removes the stored value for the local value with the -// given address. -// -// If this results in its containing module being empty, the module will be -// pruned from the state as a side-effect. -func (s *SyncState) RemoveLocalValue(addr addrs.AbsLocalValue) { - defer s.beginWrite()() - - ms := s.state.Module(addr.Module) - if ms == nil { - return - } - ms.RemoveLocalValue(addr.LocalValue.Name) - s.maybePruneModule(addr.Module) -} - // Resource returns a snapshot of the state of the resource with the given // address, or nil if no such resource is tracked. // diff --git a/internal/terraform/context_apply2_test.go b/internal/terraform/context_apply2_test.go index 0f257189b0..2f522abef2 100644 --- a/internal/terraform/context_apply2_test.go +++ b/internal/terraform/context_apply2_test.go @@ -2272,16 +2272,29 @@ locals { t.Errorf("expected no errors, but got %s", diags) } - state, diags := ctx.Apply(plan, m, nil) - if diags.HasErrors() { - t.Errorf("expected no errors, but got %s", diags) + g, _, diags := ctx.applyGraph(plan, m, &ApplyOpts{}, true) + assertNoDiagnostics(t, diags) + + // The local value should've been pruned from the graph because nothing + // refers to it. + gotGraph := g.String() + wantGraph := `provider["registry.terraform.io/hashicorp/test"] +provider["registry.terraform.io/hashicorp/test"] (close) + test_object.a +root + provider["registry.terraform.io/hashicorp/test"] (close) +test_object.a + test_object.a (expand) +test_object.a (expand) + provider["registry.terraform.io/hashicorp/test"] +` + if diff := cmp.Diff(wantGraph, gotGraph); diff != "" { + t.Errorf("wrong apply graph\n%s", diff) } - // We didn't specify any external references, so the unreferenced local - // value should have been tidied up and never made it into the state. - module := state.RootModule() - if len(module.LocalValues) > 0 { - t.Errorf("expected no local values in the state but found %d", len(module.LocalValues)) + _, diags = ctx.Apply(plan, m, nil) + if diags.HasErrors() { + t.Errorf("expected no errors, but got %s", diags) } } @@ -2315,17 +2328,36 @@ locals { t.Errorf("expected no errors, but got %s", diags) } - state, diags := ctx.Apply(plan, m, nil) + g, _, diags := ctx.applyGraph(plan, m, &ApplyOpts{}, true) if diags.HasErrors() { t.Errorf("expected no errors, but got %s", diags) } - // We did specify the local value in the external references, so it should - // have been preserved even though it is not referenced by anything directly - // in the config. - module := state.RootModule() - if module.LocalValues["local_value"].AsString() != "foo" { - t.Errorf("expected local value to be \"foo\" but was \"%s\"", module.LocalValues["local_value"].AsString()) + // The local value should remain in the graph because the external + // reference uses it. + gotGraph := g.String() + wantGraph := ` + local.local_value (expand) +local.local_value (expand) + test_object.a +provider["registry.terraform.io/hashicorp/test"] +provider["registry.terraform.io/hashicorp/test"] (close) + test_object.a +root + + provider["registry.terraform.io/hashicorp/test"] (close) +test_object.a + test_object.a (expand) +test_object.a (expand) + provider["registry.terraform.io/hashicorp/test"] +` + if diff := cmp.Diff(wantGraph, gotGraph); diff != "" { + t.Errorf("wrong graph\n%s", diff) + } + + _, diags = ctx.Apply(plan, m, nil) + if diags.HasErrors() { + t.Errorf("expected no errors, but got %s", diags) } } diff --git a/internal/terraform/context_plan2_test.go b/internal/terraform/context_plan2_test.go index cf2c5876ba..787e8d34cd 100644 --- a/internal/terraform/context_plan2_test.go +++ b/internal/terraform/context_plan2_test.go @@ -4234,6 +4234,10 @@ resource "test_object" "a" { locals { local_value = test_object.a.test_string } + +output "from_local_value" { + value = local.local_value +} `, }) @@ -4253,20 +4257,20 @@ locals { module := state.RootModule() // So, the original state shouldn't have been updated at all. - if len(module.LocalValues) > 0 { - t.Errorf("expected no local values in the state but found %d", len(module.LocalValues)) + if len(state.RootOutputValues) > 0 { + t.Errorf("expected no root output values in the state but found %d", len(state.RootOutputValues)) } if len(module.Resources) > 0 { - t.Errorf("expected no resources in the state but found %d", len(module.LocalValues)) + t.Errorf("expected no resources in the state but found %d", len(module.Resources)) } // But, this makes it hard for the testing framework to valid things about // the returned plan. So, the plan contains the planned state: module = plan.PlannedState.RootModule() - if module.LocalValues["local_value"].AsString() != "foo" { - t.Errorf("expected local value to be \"foo\" but was \"%s\"", module.LocalValues["local_value"].AsString()) + if got, want := plan.PlannedState.RootOutputValues["from_local_value"].Value.AsString(), "foo"; got != want { + t.Errorf("expected local value to be %q but was %q", want, got) } if module.ResourceInstance(addr.Resource).Current.Status != states.ObjectPlanned { diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index 077c41aec3..ddad69ae5d 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -303,12 +303,7 @@ func (d *evaluationStateData) GetLocalValue(addr addrs.LocalValue, rng tfdiags.S return cty.DynamicVal, diags } - val := d.Evaluator.State.LocalValue(addr.Absolute(d.ModulePath)) - if val == cty.NilVal { - // Not evaluated yet? - val = cty.DynamicVal - } - + val := d.Evaluator.NamedValues.GetLocalValue(addr.Absolute(d.ModulePath)) return val, diags } diff --git a/internal/terraform/node_external_reference.go b/internal/terraform/node_external_reference.go index 59ac112a11..8050d15e3f 100644 --- a/internal/terraform/node_external_reference.go +++ b/internal/terraform/node_external_reference.go @@ -3,7 +3,13 @@ package terraform -import "github.com/hashicorp/terraform/internal/addrs" +import ( + "fmt" + "sort" + "strings" + + "github.com/hashicorp/terraform/internal/addrs" +) // nodeExternalReference allows external callers (such as the testing framework) // to provide the list of references they are making into the graph. This @@ -32,3 +38,13 @@ func (n *nodeExternalReference) ModulePath() addrs.Module { func (n *nodeExternalReference) References() []*addrs.Reference { return n.ExternalReferences } + +// Name implements dag.NamedVertex +func (n *nodeExternalReference) Name() string { + names := make([]string, len(n.ExternalReferences)) + for i, ref := range n.ExternalReferences { + names[i] = ref.DisplayString() + } + sort.Strings(names) + return fmt.Sprintf("", strings.Join(names, ", ")) +} diff --git a/internal/terraform/node_local.go b/internal/terraform/node_local.go index 7d955e514f..ad5943cab9 100644 --- a/internal/terraform/node_local.go +++ b/internal/terraform/node_local.go @@ -161,13 +161,7 @@ func (n *NodeLocal) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Di return diags } - state := ctx.State() - if state == nil { - diags = diags.Append(fmt.Errorf("cannot write local value to nil state")) - return diags - } - - state.SetLocalValue(addr.Absolute(ctx.Path()), val) + ctx.NamedValues().SetLocalValue(addr.Absolute(ctx.Path()), val) return diags } diff --git a/internal/terraform/node_local_test.go b/internal/terraform/node_local_test.go index 6e11d74f7d..ba28c118e9 100644 --- a/internal/terraform/node_local_test.go +++ b/internal/terraform/node_local_test.go @@ -4,17 +4,15 @@ package terraform import ( - "reflect" "testing" - "github.com/davecgh/go-spew/spew" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" - "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/hcl2shim" + "github.com/hashicorp/terraform/internal/namedvals" "github.com/hashicorp/terraform/internal/states" ) @@ -48,14 +46,16 @@ func TestNodeLocalExecute(t *testing.T) { t.Fatal(diags.Error()) } + localAddr := addrs.LocalValue{Name: "foo"}.Absolute(addrs.RootModuleInstance) n := &NodeLocal{ - Addr: addrs.LocalValue{Name: "foo"}.Absolute(addrs.RootModuleInstance), + Addr: localAddr, Config: &configs.Local{ Expr: expr, }, } ctx := &MockEvalContext{ - StateState: states.NewState().SyncWrapper(), + StateState: states.NewState().SyncWrapper(), + NamedValuesState: namedvals.NewState(), EvaluateExprResult: hcl2shim.HCL2ValueFromConfigValue(test.Want), } @@ -69,18 +69,19 @@ func TestNodeLocalExecute(t *testing.T) { } } - ms := ctx.StateState.Module(addrs.RootModuleInstance) - gotLocals := ms.LocalValues - wantLocals := map[string]cty.Value{} - if test.Want != nil { - wantLocals["foo"] = hcl2shim.HCL2ValueFromConfigValue(test.Want) - } - - if !reflect.DeepEqual(gotLocals, wantLocals) { - t.Errorf( - "wrong locals after Eval\ngot: %swant: %s", - spew.Sdump(gotLocals), spew.Sdump(wantLocals), - ) + if test.Err { + if ctx.NamedValues().HasLocalValue(localAddr) { + t.Errorf("have value for %s, but wanted none", localAddr) + } + } else { + if !ctx.NamedValues().HasLocalValue(localAddr) { + t.Fatalf("no value for %s", localAddr) + } + got := ctx.NamedValues().GetLocalValue(localAddr) + want := hcl2shim.HCL2ValueFromConfigValue(test.Want) + if !want.RawEquals(got) { + t.Errorf("wrong value for %s\ngot: %#v\nwant: %#v", localAddr, got, want) + } } }) }