bug fix for Instance module without resources/outputs was not reporting correct length. (#3469)

Signed-off-by: KrishnaSindhur <krishna.sindhur@harness.io>
This commit is contained in:
krishna sindhur
2025-12-01 19:45:10 +05:30
committed by GitHub
parent a89667e29c
commit 64d3940fc6
5 changed files with 213 additions and 2 deletions

View File

@@ -15,6 +15,7 @@ BUG FIXES:
- `for_each` inside `dynamic` blocks can now call provider-defined functions. ([#3429](https://github.com/opentofu/opentofu/issues/3429))
- In the unlikely event that text included in a diagnostic message includes C0 control characters (e.g. terminal escape sequences), OpenTofu will now replace them with printable characters to avoid the risk of inadvertently changing terminal state when stdout or stderr is a terminal. ([#3479](https://github.com/opentofu/opentofu/issues/3479))
- Fixed `length(module.foo)` returning 0 for module instances without outputs, even when `count` or `for_each` is set. ([#3067](https://github.com/opentofu/opentofu/issues/3067))
## Previous Releases

View File

@@ -68,6 +68,10 @@ type Evaluator struct {
// ensures they can be safely accessed and modified concurrently.
Changes *plans.ChangesSync
// InstanceExpander tracks the expansion of modules and resources, which
// is used to determine the set of instance keys for count and for_each.
InstanceExpander *instances.Expander
PlanTimestamp time.Time
}
@@ -440,6 +444,25 @@ func (d *evaluationStateData) GetModule(_ context.Context, addr addrs.ModuleCall
// module instance.
moduleInstances := map[addrs.InstanceKey]map[string]cty.Value{}
// Pre-populate moduleInstances using InstanceExpander to handle modules with no outputs.
// This ensures that expressions like length(module.foo) work correctly even when a module
// has no output values defined, by consulting the expansion state to determine which
// module instances exist based on count/for_each.
// We only do this during non-validation operations (plan, apply, etc.) because during
// validation, the InstanceExpander may not be fully populated yet.
if d.Evaluator.InstanceExpander != nil && d.Evaluator.Operation != walkValidate {
childModuleAddr := d.ModulePath.Module().Child(addr.Name)
moduleInstanceAddrs := d.Evaluator.InstanceExpander.ExpandModule(childModuleAddr)
for _, moduleInstanceAddr := range moduleInstanceAddrs {
_, callInstance := moduleInstanceAddr.CallInstance()
key := callInstance.Key
if _, exists := moduleInstances[key]; !exists {
moduleInstances[key] = map[string]cty.Value{}
}
}
}
// create a dummy object type for validation below
unknownMap := map[string]cty.Type{}

View File

@@ -10,11 +10,13 @@ import (
"testing"
"github.com/davecgh/go-spew/spew"
"github.com/hashicorp/hcl/v2"
"github.com/zclconf/go-cty/cty"
"github.com/opentofu/opentofu/internal/addrs"
"github.com/opentofu/opentofu/internal/configs"
"github.com/opentofu/opentofu/internal/configs/configschema"
"github.com/opentofu/opentofu/internal/instances"
"github.com/opentofu/opentofu/internal/lang/marks"
"github.com/opentofu/opentofu/internal/plans"
"github.com/opentofu/opentofu/internal/providers"
@@ -921,7 +923,187 @@ func TestEvaluatorGetModule(t *testing.T) {
}
}
// TestEvaluatorGetModule_ForEach verifies that GetModule correctly evaluates
// a module with for_each that has output values defined in state.
// This is a regression test to ensure the fix for (modules without outputs)
// doesn't break the existing behavior for modules WITH outputs.
func TestEvaluatorGetModule_ForEach(t *testing.T) {
expander := instances.NewExpander()
expander.SetModuleForEach(
addrs.RootModuleInstance,
addrs.ModuleCall{Name: "mods"},
map[string]cty.Value{
"a": cty.StringVal("first"),
"b": cty.StringVal("second"),
},
)
stateSync := states.BuildState(func(ss *states.SyncState) {
ss.SetOutputValue(
addrs.OutputValue{Name: "result"}.Absolute(addrs.ModuleInstance{
addrs.ModuleInstanceStep{Name: "mods", InstanceKey: addrs.StringKey("a")},
}),
cty.StringVal("output_a"),
false,
"",
)
ss.SetOutputValue(
addrs.OutputValue{Name: "result"}.Absolute(addrs.ModuleInstance{
addrs.ModuleInstanceStep{Name: "mods", InstanceKey: addrs.StringKey("b")},
}),
cty.StringVal("output_b"),
false,
"",
)
}).SyncWrapper()
evaluator := &Evaluator{
Meta: &ContextMeta{
Env: "test",
},
Config: &configs.Config{
Module: &configs.Module{
ModuleCalls: map[string]*configs.ModuleCall{
"mods": {
Name: "mods",
ForEach: hcl.StaticExpr(cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("first"),
"b": cty.StringVal("second"),
}), hcl.Range{}),
},
},
},
Children: map[string]*configs.Config{
"mods": {
Path: addrs.Module{"module.mods"},
Module: &configs.Module{
Outputs: map[string]*configs.Output{
"result": {
Name: "result",
},
},
},
},
},
},
State: stateSync,
Changes: plans.NewChanges().SyncWrapper(),
InstanceExpander: expander,
}
data := &evaluationStateData{
Evaluator: evaluator,
}
scope := evaluator.Scope(data, nil, nil, nil)
got, diags := scope.Data.GetModule(t.Context(), addrs.ModuleCall{
Name: "mods",
}, tfdiags.SourceRange{})
if len(diags) != 0 {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
}
want := cty.ObjectVal(map[string]cty.Value{
"a": cty.ObjectVal(map[string]cty.Value{
"result": cty.StringVal("output_a"),
}),
"b": cty.ObjectVal(map[string]cty.Value{
"result": cty.StringVal("output_b"),
}),
})
if !got.RawEquals(want) {
t.Errorf("wrong result:\ngot: %#v\nwant: %#v", got, want)
}
if got.LengthInt() != 2 {
t.Errorf("wrong length: got %d, want 2", got.LengthInt())
}
}
// TestEvaluatorGetModule_ForEachWithoutOutputs verifies that GetModule correctly returns
// the expected length for a module with for_each but no output values defined.
// This tests the fix for (modules without outputs) where length(module.empty) would incorrectly
// return 0 for modules without outputs, even when for_each has multiple keys.
func TestEvaluatorGetModule_ForEachWithoutOutputs(t *testing.T) {
expander := instances.NewExpander()
expander.SetModuleForEach(
addrs.RootModuleInstance,
addrs.ModuleCall{Name: "empty"},
map[string]cty.Value{
"x": cty.StringVal("first"),
"y": cty.StringVal("second"),
"z": cty.StringVal("third"),
},
)
evaluator := &Evaluator{
Meta: &ContextMeta{
Env: "test",
},
Config: &configs.Config{
Module: &configs.Module{
ModuleCalls: map[string]*configs.ModuleCall{
"empty": {
Name: "empty",
ForEach: hcl.StaticExpr(cty.MapVal(map[string]cty.Value{
"x": cty.StringVal("first"),
"y": cty.StringVal("second"),
"z": cty.StringVal("third"),
}), hcl.Range{}),
},
},
},
Children: map[string]*configs.Config{
"empty": {
Path: addrs.Module{"module.empty"},
Module: &configs.Module{
Outputs: map[string]*configs.Output{},
},
},
},
},
State: states.NewState().SyncWrapper(),
Changes: plans.NewChanges().SyncWrapper(),
InstanceExpander: expander,
}
data := &evaluationStateData{
Evaluator: evaluator,
}
scope := evaluator.Scope(data, nil, nil, nil)
got, diags := scope.Data.GetModule(t.Context(), addrs.ModuleCall{
Name: "empty",
}, tfdiags.SourceRange{})
if len(diags) != 0 {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
}
want := cty.ObjectVal(map[string]cty.Value{
"x": cty.EmptyObjectVal,
"y": cty.EmptyObjectVal,
"z": cty.EmptyObjectVal,
})
if !got.RawEquals(want) {
t.Errorf("wrong result:\ngot: %#v\nwant: %#v", got, want)
}
if got.LengthInt() != 3 {
t.Errorf("wrong length: got %d, want 3 (module has for_each with 3 keys)", got.LengthInt())
}
}
func evaluatorForModule(stateSync *states.SyncState, changesSync *plans.ChangesSync) *Evaluator {
expander := instances.NewExpander()
expander.SetModuleSingle(
addrs.RootModuleInstance,
addrs.ModuleCall{Name: "mod"},
)
return &Evaluator{
Meta: &ContextMeta{
Env: "foo",
@@ -952,7 +1134,8 @@ func evaluatorForModule(stateSync *states.SyncState, changesSync *plans.ChangesS
},
},
},
State: stateSync,
Changes: changesSync,
State: stateSync,
Changes: changesSync,
InstanceExpander: expander,
}
}

View File

@@ -98,6 +98,7 @@ func (w *ContextGraphWalker) EvalContext() EvalContext {
Plugins: w.Context.plugins,
VariableValues: w.variableValues,
VariableValuesLock: &w.variableValuesLock,
InstanceExpander: w.InstanceExpander,
PlanTimestamp: w.PlanTimestamp,
}

View File

@@ -98,6 +98,9 @@ func (tc *TestContext) evaluate(state *states.SyncState, changes *plans.ChangesS
}(),
VariableValuesLock: new(sync.Mutex),
PlanTimestamp: tc.Plan.Timestamp,
// InstanceExpander is intentionally nil for test contexts
// The GetModule function will fall back to using state/changes when it's nil
InstanceExpander: nil,
},
ModulePath: nil, // nil for the root module
InstanceKeyData: EvalDataForNoInstanceKey,