From 9d7eb9742e70c3b6bfe2e7c577feff90f769719d Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 3 Sep 2025 14:51:24 -0700 Subject: [PATCH] lang/eval: Enough resource support for basic validation to work Signed-off-by: Martin Atkins --- internal/lang/eval/config_validate_test.go | 61 +++++++++ internal/lang/eval/instance_selector.go | 121 +++++++++++++++-- .../eval/internal/configgraph/diagnostics.go | 2 +- .../internal/configgraph/input_variable.go | 2 +- .../eval/internal/configgraph/instances.go | 20 ++- .../internal/configgraph/module_instance.go | 122 +++++++++++++++++- .../eval/internal/configgraph/output_value.go | 2 +- .../eval/internal/configgraph/resource.go | 9 +- .../internal/configgraph/resource_instance.go | 10 +- 9 files changed, 317 insertions(+), 32 deletions(-) diff --git a/internal/lang/eval/config_validate_test.go b/internal/lang/eval/config_validate_test.go index 653c86fac3..0631ef87e9 100644 --- a/internal/lang/eval/config_validate_test.go +++ b/internal/lang/eval/config_validate_test.go @@ -13,7 +13,9 @@ import ( "github.com/opentofu/opentofu/internal/addrs" "github.com/opentofu/opentofu/internal/configs" + "github.com/opentofu/opentofu/internal/configs/configschema" "github.com/opentofu/opentofu/internal/lang/eval" + "github.com/opentofu/opentofu/internal/providers" "github.com/opentofu/opentofu/internal/tfdiags" ) @@ -154,6 +156,65 @@ func TestValidate_valuesOnlyCycle(t *testing.T) { gotDiags := configInst.Validate(t.Context()) gotDiags.Sort() // we don't care what order they are in assertDiagnosticsMatch(t, gotDiags, wantDiags) +} + +func TestValidate_resourceValid(t *testing.T) { + configInst, diags := eval.NewConfigInstance(t.Context(), &eval.ConfigCall{ + EvalContext: &eval.EvalContext{ + Modules: eval.ModulesForTesting(map[addrs.ModuleSourceLocal]*configs.Module{ + addrs.ModuleSourceLocal("."): configs.ModuleFromStringForTesting(t, ` + terraform { + required_providers { + foo = { + source = "test/foo" + } + } + } + variable "in" { + type = string + } + resource "foo" "bar" { + name = var.in + } + output "out" { + value = foo.bar.id + } + `), + }), + Providers: eval.ProvidersForTesting(map[addrs.Provider]*providers.GetProviderSchemaResponse{ + addrs.MustParseProviderSourceString("test/foo"): { + Provider: providers.Schema{ + Block: &configschema.Block{}, + }, + ResourceTypes: map[string]providers.Schema{ + "foo": { + Block: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "name": { + Type: cty.String, + Required: true, + }, + "id": { + Type: cty.String, + Computed: true, + }, + }, + }, + }, + }, + }, + }), + }, + RootModuleSource: addrs.ModuleSourceLocal("."), + InputValues: eval.InputValuesForTesting(map[string]cty.Value{ + "in": cty.StringVal("foo bar baz"), + }), + }) + if diags.HasErrors() { + t.Fatalf("unexpected errors: %s", diags.Err()) + } + + diags = configInst.Validate(t.Context()) if diags.HasErrors() { t.Fatalf("unexpected errors: %s", diags.Err()) } diff --git a/internal/lang/eval/instance_selector.go b/internal/lang/eval/instance_selector.go index aa91b3cce7..41fa6c580e 100644 --- a/internal/lang/eval/instance_selector.go +++ b/internal/lang/eval/instance_selector.go @@ -7,9 +7,13 @@ package eval import ( "context" + "errors" + "fmt" "github.com/hashicorp/hcl/v2" "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/convert" + "github.com/zclconf/go-cty/cty/gocty" "github.com/opentofu/opentofu/internal/addrs" "github.com/opentofu/opentofu/internal/instances" @@ -23,13 +27,22 @@ func compileInstanceSelector(ctx context.Context, declScope exprs.Scope, forEach // because we expect the configs package to check that. if forEachExpr != nil { - return compileInstanceSelectorForEach(ctx, declScope, forEachExpr) + return compileInstanceSelectorForEach(ctx, exprs.NewClosure( + exprs.EvalableHCLExpression(forEachExpr), + declScope, + )) } if countExpr != nil { - return compileInstanceSelectorCount(ctx, declScope, countExpr) + return compileInstanceSelectorCount(ctx, exprs.NewClosure( + exprs.EvalableHCLExpression(countExpr), + declScope, + )) } if enabledExpr != nil { - return compileInstanceSelectorEnabled(ctx, declScope, enabledExpr) + return compileInstanceSelectorEnabled(ctx, exprs.NewClosure( + exprs.EvalableHCLExpression(enabledExpr), + declScope, + )) } return compileInstanceSelectorSingleton(ctx) } @@ -47,15 +60,107 @@ func compileInstanceSelectorSingleton(_ context.Context) configgraph.InstanceSel } } -func compileInstanceSelectorCount(_ context.Context, declScope exprs.Scope, expr hcl.Expression) configgraph.InstanceSelector { - panic("unimplemented") +func compileInstanceSelectorCount(_ context.Context, countValuer exprs.Valuer) configgraph.InstanceSelector { + countValuer = configgraph.ValuerOnce(countValuer) + return &instanceSelector{ + keyType: addrs.IntKeyType, + sourceRange: nil, + selectInstances: func(ctx context.Context) (configgraph.Maybe[configgraph.InstancesSeq], cty.ValueMarks, tfdiags.Diagnostics) { + var count int + countVal, diags := countValuer.Value(ctx) + if diags.HasErrors() { + return nil, nil, diags + } + countVal, marks := countVal.Unmark() + countVal, err := convert.Convert(countVal, cty.Number) + if err == nil && !countVal.IsKnown() { + // We represent "unknown" by returning a nil configgraph.Maybe + // without any error diagnostics, but we will still report + // what marks we found on the unknown value. + return nil, marks, diags + } + if err == nil && countVal.IsNull() { + err = errors.New("must not be null") + } + if err == nil { + err = gocty.FromCtyValue(countVal, &count) + } + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid value for instance count", + Detail: fmt.Sprintf("Unsuitable value for the \"count\" meta-argument: %s.", tfdiags.FormatError(err)), + Subject: configgraph.MaybeHCLSourceRange(countValuer.ValueSourceRange()), + }) + return nil, marks, diags + } + // If we manage to get here then "count" is the desired number of + // instances, and so we'll yield incrementing integers up to + // that number, exclusive. + seq := func(yield func(addrs.InstanceKey, instances.RepetitionData) bool) { + for i := range count { + more := yield(addrs.IntKey(i), instances.RepetitionData{ + CountIndex: cty.NumberIntVal(int64(i)), + }) + if !more { + break + } + } + } + return configgraph.Known(seq), nil, nil + }, + } } -func compileInstanceSelectorEnabled(_ context.Context, declScope exprs.Scope, expr hcl.Expression) configgraph.InstanceSelector { - panic("unimplemented") +func compileInstanceSelectorEnabled(_ context.Context, enabledValuer exprs.Valuer) configgraph.InstanceSelector { + enabledValuer = configgraph.ValuerOnce(enabledValuer) + return &instanceSelector{ + keyType: addrs.NoKeyType, + sourceRange: nil, + selectInstances: func(ctx context.Context) (configgraph.Maybe[configgraph.InstancesSeq], cty.ValueMarks, tfdiags.Diagnostics) { + var enabled bool + enabledVal, diags := enabledValuer.Value(ctx) + if diags.HasErrors() { + return nil, nil, diags + } + enabledVal, marks := enabledVal.Unmark() + enabledVal, err := convert.Convert(enabledVal, cty.Bool) + if err == nil && !enabledVal.IsKnown() { + // We represent "unknown" by returning a nil configgraph.Maybe + // without any error diagnostics, but we will still report + // what marks we found on the unknown value. + return nil, marks, diags + } + if err == nil && enabledVal.IsNull() { + err = errors.New("must not be null") + } + if err == nil { + err = gocty.FromCtyValue(enabledVal, &enabled) + } + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid value for instance enabled", + Detail: fmt.Sprintf("Unsuitable value for the \"enabled\" meta-argument: %s.", tfdiags.FormatError(err)), + Subject: configgraph.MaybeHCLSourceRange(enabledValuer.ValueSourceRange()), + }) + return nil, marks, diags + } + // If we manage to get here then "enabled" is true only if there + // should be an instance of this resource. + seq := func(yield func(addrs.InstanceKey, instances.RepetitionData) bool) { + if enabled { + yield(addrs.NoKey, instances.RepetitionData{}) + } + } + return configgraph.Known(seq), nil, nil + }, + } } -func compileInstanceSelectorForEach(_ context.Context, declScope exprs.Scope, expr hcl.Expression) configgraph.InstanceSelector { +func compileInstanceSelectorForEach(_ context.Context, forEachValuer exprs.Valuer) configgraph.InstanceSelector { + // TODO: The logic for this one is a little more complex so I'll come + // back to this once more of the rest of this is working. panic("unimplemented") } diff --git a/internal/lang/eval/internal/configgraph/diagnostics.go b/internal/lang/eval/internal/configgraph/diagnostics.go index 9f7d46b6c2..574e8c56c1 100644 --- a/internal/lang/eval/internal/configgraph/diagnostics.go +++ b/internal/lang/eval/internal/configgraph/diagnostics.go @@ -13,7 +13,7 @@ import ( "github.com/opentofu/opentofu/internal/tfdiags" ) -func maybeHCLSourceRange(maybeRng *tfdiags.SourceRange) *hcl.Range { +func MaybeHCLSourceRange(maybeRng *tfdiags.SourceRange) *hcl.Range { if maybeRng == nil { return nil } diff --git a/internal/lang/eval/internal/configgraph/input_variable.go b/internal/lang/eval/internal/configgraph/input_variable.go index 7983766002..4010bbe4ac 100644 --- a/internal/lang/eval/internal/configgraph/input_variable.go +++ b/internal/lang/eval/internal/configgraph/input_variable.go @@ -78,7 +78,7 @@ func (i *InputVariable) Value(ctx context.Context) (cty.Value, tfdiags.Diagnosti Severity: hcl.DiagError, Summary: "Invalid value for input variable", Detail: fmt.Sprintf("Unsuitable value for variable %q: %s.", i.Addr.Variable.Name, tfdiags.FormatError(err)), - Subject: maybeHCLSourceRange(i.ValueSourceRange()), + Subject: MaybeHCLSourceRange(i.ValueSourceRange()), }) finalV = cty.UnknownVal(i.TargetType.WithoutOptionalAttributesDeep()) } diff --git a/internal/lang/eval/internal/configgraph/instances.go b/internal/lang/eval/internal/configgraph/instances.go index 8fdbec3ad4..02823308e1 100644 --- a/internal/lang/eval/internal/configgraph/instances.go +++ b/internal/lang/eval/internal/configgraph/instances.go @@ -206,6 +206,8 @@ func valueForInstances[T exprs.Valuer](ctx context.Context, insts *compiledInsta attrs := make(map[string]cty.Value, len(insts.Instances)) for key, obj := range insts.Instances { attrName := string(key.(addrs.StringKey)) // panic here means buggy [InstanceSelector] + // Diagnostics for this are collected directly from the instance + // when the CheckAll tree walk visits it. attrs[attrName] = diagsHandledElsewhere(obj.Value(ctx)) } return cty.ObjectVal(attrs).WithMarks(insts.ValueMarks) @@ -214,11 +216,23 @@ func valueForInstances[T exprs.Valuer](ctx context.Context, insts *compiledInsta if _, ok := insts.Instances[addrs.WildcardKey{addrs.StringKeyType}]; ok { // In this case we cannot predict anything about the placeholder // result because if we don't know how many instances we have - // // then we cannot even predict the tuple type. + // then we cannot even predict the tuple type. return cty.DynamicVal.WithMarks(insts.ValueMarks) } - // TODO: Implement - panic("value construction for \"count\" resources not yet implemented") + elems := make([]cty.Value, len(insts.Instances)) + for i := range elems { + inst, ok := insts.Instances[addrs.IntKey(i)] + if !ok { + // This should not be possible for a correct [InstanceSelector] + // but this is not the place to deal with that. + elems[i] = cty.DynamicVal + continue + } + // Diagnostics for this are collected directly from the instance + // when the CheckAll tree walk visits it. + elems[i] = diagsHandledElsewhere(inst.Value(ctx)) + } + return cty.TupleVal(elems).WithMarks(insts.ValueMarks) default: // Should not get here because [InstanceSelector] is not allowed to diff --git a/internal/lang/eval/internal/configgraph/module_instance.go b/internal/lang/eval/internal/configgraph/module_instance.go index 84aa99ff37..8eda68ffc4 100644 --- a/internal/lang/eval/internal/configgraph/module_instance.go +++ b/internal/lang/eval/internal/configgraph/module_instance.go @@ -166,13 +166,56 @@ func (m *ModuleInstance) ResolveAttr(ref hcl.TraverseAttr) (exprs.Attribute, tfd }) return nil, diags + // All of these resource-related symbols ultimately end up in + // [ModuleInstance.resolveResourceAttr] after indirecting through + // one or two more attribute steps. + case "resource": + return exprs.NestedSymbolTable(&moduleInstanceResourceSymbolTable{ + mode: addrs.ManagedResourceMode, + moduleInst: m, + startRng: ref.SrcRange, + }), diags + case "data": + return exprs.NestedSymbolTable(&moduleInstanceResourceSymbolTable{ + mode: addrs.DataResourceMode, + moduleInst: m, + }), diags + case "ephemeral": + return exprs.NestedSymbolTable(&moduleInstanceResourceSymbolTable{ + mode: addrs.EphemeralResourceMode, + moduleInst: m, + }), diags default: - // TODO: Once we support resource references this case should be treated - // as the beginning of a reference to a managed resource, as a - // shorthand omitting the "resource." prefix. - diags = diags.Append(fmt.Errorf("no support for %q references yet", ref.Name)) + // We treat all unrecognized prefixes as a shorthand for "resource." + // where the first segment is the resource type name. + return exprs.NestedSymbolTable(&moduleInstanceResourceSymbolTable{ + mode: addrs.ManagedResourceMode, + typeName: ref.Name, + moduleInst: m, + }), diags + } +} + +func (m *ModuleInstance) resolveResourceAttr(addr addrs.Resource, rng tfdiags.SourceRange) (exprs.Attribute, tfdiags.Diagnostics) { + // This function handles references like "aws_instance.foo" and + // "data.aws_subnet.bar" after the intermediate steps have been + // collected using [moduleInstanceResourceSymbolTable]. Refer to + // [ModuleInstance.ResourceAttr] for the beginning of this process. + + var diags tfdiags.Diagnostics + r, ok := m.ResourceNodes[addr] + if !ok { + // TODO: Try using "didyoumean" with resource types and names that + // _are_ declared in the module to see if we can suggest an alternatve. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Reference to undeclared resource variable", + Detail: fmt.Sprintf("There is no declaration of resource %s in this module.", addr), + Subject: rng.ToHCL().Ptr(), + }) return nil, diags } + return exprs.ValueOf(r), diags } func (m *ModuleInstance) resolveSimpleChildAttr(topSymbol string, ref hcl.TraverseAttr) (exprs.Attribute, tfdiags.Diagnostics) { @@ -317,6 +360,77 @@ func (m *ModuleInstance) AnnounceAllGraphevalRequests(announce func(workgraph.Re } } +type moduleInstanceResourceSymbolTable struct { + mode addrs.ResourceMode + // We reuse this type for both the first step like "data." and the + // second step like "data.foo.". typeName is the empty string for + // the first step, and then populated in the second step. + typeName string + moduleInst *ModuleInstance + startRng hcl.Range +} + +var _ exprs.SymbolTable = (*moduleInstanceResourceSymbolTable)(nil) + +// HandleInvalidStep implements exprs.SymbolTable. +func (m *moduleInstanceResourceSymbolTable) HandleInvalidStep(rng tfdiags.SourceRange) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + if m.typeName == "" { + // We're at the first step and expecting a resource type name, then. + adjective := "" + switch m.mode { + case addrs.ManagedResourceMode: + adjective = "managed " + case addrs.DataResourceMode: + adjective = "data " + case addrs.EphemeralResourceMode: + adjective = "ephemeral " + default: + // We'll just omit any adjective if it isn't one we know, though + // we should ideally update the above if we add a new resource mode. + } + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid reference to resource", + Detail: fmt.Sprintf("An attribute access is required here, naming the type of %sresource to refer to.", adjective), + Subject: rng.ToHCL().Ptr(), + }) + } else { + // We're at the second step and expecting a resource name. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid reference to resource", + Detail: fmt.Sprintf("An attribute access is required here, giving the name of the %q resource to refer to.", m.typeName), + Subject: rng.ToHCL().Ptr(), + }) + } + return diags +} + +// ResolveAttr implements exprs.SymbolTable. +func (m *moduleInstanceResourceSymbolTable) ResolveAttr(ref hcl.TraverseAttr) (exprs.Attribute, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + if m.typeName == "" { + // We're at the first step and expecting a resource type name, then. + // We'll return a new instance with the given type name populated + // so that we can collect the resource name from the next step. + return exprs.NestedSymbolTable(&moduleInstanceResourceSymbolTable{ + mode: m.mode, + typeName: ref.Name, + moduleInst: m.moduleInst, + startRng: m.startRng, + }), diags + } + // We're at the second step and expecting a resource name. We'll now + // delegate back to the main module instance to handle the reference. + addr := addrs.Resource{ + Mode: m.mode, + Type: m.typeName, + Name: ref.Name, + } + return m.moduleInst.resolveResourceAttr(addr, tfdiags.SourceRangeFromHCL(hcl.RangeBetween(m.startRng, ref.SrcRange))) +} + // moduleInstNestedSymbolTable is a common implementation for all of the // various "simple" nested symbol table prefixes in a module instance's // top-level scope, handling the typical case where there's a fixed prefix diff --git a/internal/lang/eval/internal/configgraph/output_value.go b/internal/lang/eval/internal/configgraph/output_value.go index e16f43c20b..14125b5b31 100644 --- a/internal/lang/eval/internal/configgraph/output_value.go +++ b/internal/lang/eval/internal/configgraph/output_value.go @@ -122,7 +122,7 @@ func (o *OutputValue) Value(ctx context.Context) (cty.Value, tfdiags.Diagnostics Severity: hcl.DiagError, Summary: "Invalid value for output value", Detail: fmt.Sprintf("Unsuitable value for output value %q: %s.", o.Addr.OutputValue.Name, tfdiags.FormatError(err)), - Subject: maybeHCLSourceRange(o.ValueSourceRange()), + Subject: MaybeHCLSourceRange(o.ValueSourceRange()), }) finalV = cty.UnknownVal(o.TargetType.WithoutOptionalAttributesDeep()) } diff --git a/internal/lang/eval/internal/configgraph/resource.go b/internal/lang/eval/internal/configgraph/resource.go index 88b1b168e7..d25f3f48d1 100644 --- a/internal/lang/eval/internal/configgraph/resource.go +++ b/internal/lang/eval/internal/configgraph/resource.go @@ -114,16 +114,15 @@ func (r *Resource) decideInstances(ctx context.Context) (*compiledInstances[*Res } func (r *Resource) compileInstance(ctx context.Context, key addrs.InstanceKey, repData instances.RepetitionData) *ResourceInstance { - var ret *ResourceInstance - ret = &ResourceInstance{ - Addr: r.Addr.Instance(key), - Provider: r.Provider, - GetResultValue: r.GetInstanceResultValue(ctx, ret), + ret := &ResourceInstance{ + Addr: r.Addr.Instance(key), + Provider: r.Provider, ConfigValuer: ValuerOnce(exprs.NewClosure( r.ConfigEvalable, instanceLocalScope(r.ParentScope, repData), )), } + ret.GetResultValue = r.GetInstanceResultValue(ctx, ret) return ret } diff --git a/internal/lang/eval/internal/configgraph/resource_instance.go b/internal/lang/eval/internal/configgraph/resource_instance.go index 7bbca27b1e..e35f7a6c70 100644 --- a/internal/lang/eval/internal/configgraph/resource_instance.go +++ b/internal/lang/eval/internal/configgraph/resource_instance.go @@ -76,19 +76,11 @@ func (ri *ResourceInstance) StaticCheckTraversal(traversal hcl.Traversal) tfdiag return ri.ConfigValuer.StaticCheckTraversal(traversal) } -// ConfigValue returns the validated configuration value, or a placeholder -// to use instead of an invalid configuration value. -func (ri *ResourceInstance) ConfigValue(ctx context.Context) (cty.Value, tfdiags.Diagnostics) { - // TODO: Check preconditions before calling Value, and then call the - // provider's own validate function after calling Value. - return ri.ConfigValuer.Value(ctx) -} - // Value implements exprs.Valuer. func (ri *ResourceInstance) Value(ctx context.Context) (cty.Value, tfdiags.Diagnostics) { // We use the configuration value here only for its marks, since that // allows us to propagate any - configVal, diags := ri.ConfigValue(ctx) + configVal, diags := ri.ConfigValuer.Value(ctx) if diags.HasErrors() { // If we don't have a valid config value then we'll stop early // with an unknown value placeholder so that the external process