Skip to content

Commit 1e06203

Browse files
use EvalContext.Path to get module path for deprecation surpression
I decided against it before since a lot of tests were panicing, but after consulting with peers I found out the tests were just minimally set up and before it was just fine without the current scope being set. Now we require it to be set as it would in a normal execution therefore I had to add a bit of setup to the tests
1 parent d91bbea commit 1e06203

20 files changed

Lines changed: 78 additions & 78 deletions

internal/terraform/context_validate_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4412,6 +4412,7 @@ module "sink" {
44124412
}))
44134413
}
44144414

4415+
// TODO: Shouldn't this one live in https://github.com/hashicorp/terraform/pull/38006
44154416
func TestContext2Validate_deprecated_resource(t *testing.T) {
44164417
m := testModuleInline(t, map[string]string{
44174418
"main.tf": `

internal/terraform/eval_count.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"github.com/zclconf/go-cty/cty/gocty"
1111

1212
"github.com/hashicorp/hcl/v2"
13-
"github.com/hashicorp/terraform/internal/addrs"
1413
"github.com/hashicorp/terraform/internal/lang/marks"
1514
"github.com/hashicorp/terraform/internal/tfdiags"
1615
)
@@ -29,8 +28,8 @@ import (
2928
// true instead permits unknown values, indicating them by returning the
3029
// placeholder value -1. Callers can assume that a return value of -1 without
3130
// any error diagnostics represents a valid unknown value.
32-
func evaluateCountExpression(expr hcl.Expression, ctx EvalContext, moduleAddr addrs.Module, allowUnknown bool) (int, tfdiags.Diagnostics) {
33-
countVal, diags := evaluateCountExpressionValue(expr, ctx, moduleAddr)
31+
func evaluateCountExpression(expr hcl.Expression, ctx EvalContext, allowUnknown bool) (int, tfdiags.Diagnostics) {
32+
countVal, diags := evaluateCountExpressionValue(expr, ctx)
3433
if !allowUnknown && !countVal.IsKnown() {
3534
// Currently this is a rather bad outcome from a UX standpoint, since we have
3635
// no real mechanism to deal with this situation and all we can do is produce
@@ -75,7 +74,7 @@ func evaluateCountExpression(expr hcl.Expression, ctx EvalContext, moduleAddr ad
7574
// evaluateCountExpressionValue is like evaluateCountExpression
7675
// except that it returns a cty.Value which must be a cty.Number and can be
7776
// unknown.
78-
func evaluateCountExpressionValue(expr hcl.Expression, ctx EvalContext, moduleAddr addrs.Module) (cty.Value, tfdiags.Diagnostics) {
77+
func evaluateCountExpressionValue(expr hcl.Expression, ctx EvalContext) (cty.Value, tfdiags.Diagnostics) {
7978
var diags tfdiags.Diagnostics
8079
nullCount := cty.NullVal(cty.Number)
8180
if expr == nil {
@@ -103,7 +102,7 @@ func evaluateCountExpressionValue(expr hcl.Expression, ctx EvalContext, moduleAd
103102
})
104103
}
105104

106-
countVal, deprecationDiags := ctx.Deprecations().Validate(countVal, moduleAddr, expr.Range().Ptr())
105+
countVal, deprecationDiags := ctx.Deprecations().Validate(countVal, ctx.Path().Module(), expr.Range().Ptr())
107106
diags = diags.Append(deprecationDiags)
108107

109108
// Sensitive values are allowed in count but not for_each. This is a

internal/terraform/eval_count_test.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,10 @@ func TestEvaluateCountExpression(t *testing.T) {
3434
t.Run(name, func(t *testing.T) {
3535
ctx := &MockEvalContext{}
3636
ctx.installSimpleEval()
37-
countVal, diags := evaluateCountExpression(test.Expr, ctx, addrs.RootModule, false)
37+
scopedCtx := ctx.withScope(evalContextModuleInstance{
38+
Addr: addrs.RootModuleInstance,
39+
})
40+
countVal, diags := evaluateCountExpression(test.Expr, scopedCtx, false)
3841

3942
if len(diags) != 0 {
4043
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
@@ -54,7 +57,10 @@ func TestEvaluateCountExpression_ephemeral(t *testing.T) {
5457
expr := hcltest.MockExprLiteral(cty.NumberIntVal(8).Mark(marks.Ephemeral))
5558
ctx := &MockEvalContext{}
5659
ctx.installSimpleEval()
57-
_, diags := evaluateCountExpression(expr, ctx, addrs.RootModule, false)
60+
scopedCtx := ctx.withScope(evalContextModuleInstance{
61+
Addr: addrs.RootModuleInstance,
62+
})
63+
_, diags := evaluateCountExpression(expr, scopedCtx, false)
5864
if !diags.HasErrors() {
5965
t.Fatalf("unexpected success; want error")
6066
}
@@ -83,7 +89,10 @@ func TestEvaluateCountExpression_allowUnknown(t *testing.T) {
8389
t.Run(name, func(t *testing.T) {
8490
ctx := &MockEvalContext{}
8591
ctx.installSimpleEval()
86-
countVal, diags := evaluateCountExpression(test.Expr, ctx, addrs.RootModule, true)
92+
scopedCtx := ctx.withScope(evalContextModuleInstance{
93+
Addr: addrs.RootModuleInstance,
94+
})
95+
countVal, diags := evaluateCountExpression(test.Expr, scopedCtx, true)
8796

8897
if len(diags) != 0 {
8998
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))

internal/terraform/eval_for_each.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,20 @@ import (
1919
// evaluateForEachExpression differs from evaluateForEachExpressionValue by
2020
// returning an error if the count value is not known, and converting the
2121
// cty.Value to a map[string]cty.Value for compatibility with other calls.
22-
func evaluateForEachExpression(expr hcl.Expression, ctx EvalContext, module addrs.Module, allowUnknown bool) (forEach map[string]cty.Value, known bool, diags tfdiags.Diagnostics) {
23-
return newForEachEvaluator(expr, ctx, module, allowUnknown).ResourceValue()
22+
func evaluateForEachExpression(expr hcl.Expression, ctx EvalContext, allowUnknown bool) (forEach map[string]cty.Value, known bool, diags tfdiags.Diagnostics) {
23+
return newForEachEvaluator(expr, ctx, allowUnknown).ResourceValue()
2424
}
2525

2626
// forEachEvaluator is the standard mechanism for interpreting an expression
2727
// given for a "for_each" argument on a resource, module, or import.
28-
func newForEachEvaluator(expr hcl.Expression, ctx EvalContext, module addrs.Module, allowUnknown bool) *forEachEvaluator {
28+
func newForEachEvaluator(expr hcl.Expression, ctx EvalContext, allowUnknown bool) *forEachEvaluator {
2929
if ctx == nil {
3030
panic("nil EvalContext")
3131
}
3232

3333
return &forEachEvaluator{
3434
ctx: ctx,
3535
expr: expr,
36-
moduleAddr: module,
3736
allowUnknown: allowUnknown,
3837
}
3938
}
@@ -50,8 +49,6 @@ type forEachEvaluator struct {
5049
ctx EvalContext
5150
expr hcl.Expression
5251

53-
moduleAddr addrs.Module
54-
5552
// TEMP: If allowUnknown is set then we skip the usual restriction that
5653
// unknown values are not allowed in for_each. A caller that sets this
5754
// must therefore be ready to deal with the result being unknown.
@@ -331,8 +328,7 @@ func (ev *forEachEvaluator) validateResourceOrActionForEach(forEachVal cty.Value
331328
}
332329

333330
// We don't care about the returned value here, only the diagnostics
334-
module := addrs.RootModule // TODO: FIXME
335-
_, deprecationDiags := ev.ctx.Deprecations().Validate(forEachVal, module, ev.expr.Range().Ptr())
331+
_, deprecationDiags := ev.ctx.Deprecations().Validate(forEachVal, ev.ctx.Path().Module(), ev.expr.Range().Ptr())
336332

337333
diags = diags.Append(deprecationDiags)
338334

internal/terraform/eval_for_each_test.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,10 @@ func TestEvaluateForEachExpression_valid(t *testing.T) {
8383
t.Run(name, func(t *testing.T) {
8484
ctx := &MockEvalContext{}
8585
ctx.installSimpleEval()
86-
forEachMap, _, diags := evaluateForEachExpression(test.Expr, ctx, addrs.RootModule, false)
86+
scopedCtx := ctx.withScope(evalContextModuleInstance{
87+
Addr: addrs.RootModuleInstance,
88+
})
89+
forEachMap, _, diags := evaluateForEachExpression(test.Expr, scopedCtx, false)
8790

8891
if len(diags) != 0 {
8992
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
@@ -202,7 +205,10 @@ func TestEvaluateForEachExpression_errors(t *testing.T) {
202205
t.Run(name, func(t *testing.T) {
203206
ctx := &MockEvalContext{}
204207
ctx.installSimpleEval()
205-
_, _, diags := evaluateForEachExpression(test.Expr, ctx, addrs.RootModule, false)
208+
scopedCtx := ctx.withScope(evalContextModuleInstance{
209+
Addr: addrs.RootModuleInstance,
210+
})
211+
_, _, diags := evaluateForEachExpression(test.Expr, scopedCtx, false)
206212

207213
if len(diags) != 1 {
208214
t.Fatalf("got %d diagnostics; want 1", len(diags))
@@ -262,7 +268,10 @@ func TestEvaluateForEachExpression_allowUnknown(t *testing.T) {
262268
t.Run(name, func(t *testing.T) {
263269
ctx := &MockEvalContext{}
264270
ctx.installSimpleEval()
265-
_, known, diags := evaluateForEachExpression(test.Expr, ctx, addrs.RootModule, true)
271+
scopedCtx := ctx.withScope(evalContextModuleInstance{
272+
Addr: addrs.RootModuleInstance,
273+
})
274+
_, known, diags := evaluateForEachExpression(test.Expr, scopedCtx, true)
266275

267276
// With allowUnknown set, all of these expressions should be treated
268277
// as valid for_each values.
@@ -285,7 +294,10 @@ func TestEvaluateForEachExpressionKnown(t *testing.T) {
285294
t.Run(name, func(t *testing.T) {
286295
ctx := &MockEvalContext{}
287296
ctx.installSimpleEval()
288-
diags := newForEachEvaluator(expr, ctx, addrs.RootModule, false).ValidateResourceValue()
297+
scopedCtx := ctx.withScope(evalContextModuleInstance{
298+
Addr: addrs.RootModuleInstance,
299+
})
300+
diags := newForEachEvaluator(expr, scopedCtx, false).ValidateResourceValue()
289301

290302
if len(diags) != 0 {
291303
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))

internal/terraform/node_action.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ func (n *nodeExpandActionDeclaration) recordActionData(ctx EvalContext, addr add
115115

116116
switch {
117117
case n.Config.Count != nil:
118-
count, countDiags := evaluateCountExpression(n.Config.Count, ctx, n.ModulePath(), false)
118+
count, countDiags := evaluateCountExpression(n.Config.Count, ctx, false)
119119
diags = diags.Append(countDiags)
120120
if countDiags.HasErrors() {
121121
return diags
@@ -130,7 +130,7 @@ func (n *nodeExpandActionDeclaration) recordActionData(ctx EvalContext, addr add
130130
}
131131

132132
case n.Config.ForEach != nil:
133-
forEach, known, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx, n.ModulePath(), false)
133+
forEach, known, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx, false)
134134
diags = diags.Append(forEachDiags)
135135
if forEachDiags.HasErrors() {
136136
return diags

internal/terraform/node_action_validate.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func (n *NodeValidatableAction) Execute(ctx EvalContext, _ walkOperation) tfdiag
5757

5858
// Basic type-checking of the count argument. More complete validation
5959
// of this will happen when we DynamicExpand during the plan walk.
60-
_, countDiags := evaluateCountExpressionValue(n.Config.Count, ctx, n.ModulePath())
60+
_, countDiags := evaluateCountExpressionValue(n.Config.Count, ctx)
6161
diags = diags.Append(countDiags)
6262

6363
case n.Config.ForEach != nil:
@@ -67,7 +67,7 @@ func (n *NodeValidatableAction) Execute(ctx EvalContext, _ walkOperation) tfdiag
6767
}
6868

6969
// Evaluate the for_each expression here so we can expose the diagnostics
70-
forEachDiags := newForEachEvaluator(n.Config.ForEach, ctx, n.ModulePath(), false).ValidateActionValue()
70+
forEachDiags := newForEachEvaluator(n.Config.ForEach, ctx, false).ValidateActionValue()
7171
diags = diags.Append(forEachDiags)
7272
}
7373

internal/terraform/node_local.go

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"github.com/hashicorp/terraform/internal/configs"
1515
"github.com/hashicorp/terraform/internal/dag"
1616
"github.com/hashicorp/terraform/internal/lang/langrefs"
17-
"github.com/hashicorp/terraform/internal/lang/marks"
1817
"github.com/hashicorp/terraform/internal/tfdiags"
1918
)
2019

@@ -240,22 +239,3 @@ func evaluateLocalValue(config *configs.Local, localAddr addrs.LocalValue, addrS
240239
}
241240
return val, diags
242241
}
243-
244-
func validateExprUsingDeprecatedValues(val cty.Value, expr hcl.Expression) tfdiags.Diagnostics {
245-
var diags tfdiags.Diagnostics
246-
_, pvms := val.UnmarkDeepWithPaths()
247-
for _, pvm := range pvms {
248-
for m := range pvm.Marks {
249-
if depMark, ok := m.(marks.DeprecationMark); ok {
250-
diags = diags.Append(
251-
&hcl.Diagnostic{
252-
Severity: hcl.DiagWarning,
253-
Summary: "Deprecated value used",
254-
Detail: depMark.Message,
255-
Subject: expr.Range().Ptr(),
256-
})
257-
}
258-
}
259-
}
260-
return diags
261-
}

internal/terraform/node_module_expand.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ func (n *nodeExpandModule) Execute(globalCtx EvalContext, op walkOperation) (dia
131131

132132
switch {
133133
case n.ModuleCall.Count != nil:
134-
count, ctDiags := evaluateCountExpression(n.ModuleCall.Count, moduleCtx, n.Addr, allowUnknown)
134+
count, ctDiags := evaluateCountExpression(n.ModuleCall.Count, moduleCtx, allowUnknown)
135135
diags = diags.Append(ctDiags)
136136
if diags.HasErrors() {
137137
return diags
@@ -144,7 +144,7 @@ func (n *nodeExpandModule) Execute(globalCtx EvalContext, op walkOperation) (dia
144144
}
145145

146146
case n.ModuleCall.ForEach != nil:
147-
forEach, known, feDiags := evaluateForEachExpression(n.ModuleCall.ForEach, moduleCtx, module.Module(), allowUnknown)
147+
forEach, known, feDiags := evaluateForEachExpression(n.ModuleCall.ForEach, moduleCtx, allowUnknown)
148148
diags = diags.Append(feDiags)
149149
if diags.HasErrors() {
150150
return diags
@@ -281,11 +281,11 @@ func (n *nodeValidateModule) Execute(globalCtx EvalContext, op walkOperation) (d
281281
// a full expansion, presuming these errors will be caught in later steps
282282
switch {
283283
case n.ModuleCall.Count != nil:
284-
_, countDiags := evaluateCountExpressionValue(n.ModuleCall.Count, moduleCtx, n.ModulePath())
284+
_, countDiags := evaluateCountExpressionValue(n.ModuleCall.Count, moduleCtx)
285285
diags = diags.Append(countDiags)
286286

287287
case n.ModuleCall.ForEach != nil:
288-
forEachDiags := newForEachEvaluator(n.ModuleCall.ForEach, moduleCtx, module.Module(), false).ValidateResourceValue()
288+
forEachDiags := newForEachEvaluator(n.ModuleCall.ForEach, moduleCtx, false).ValidateResourceValue()
289289
diags = diags.Append(forEachDiags)
290290
}
291291

internal/terraform/node_module_variable.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,11 @@ func (n *nodeModuleVariable) Execute(ctx EvalContext, op walkOperation) (diags t
230230
if diags.HasErrors() {
231231
return diags
232232
}
233-
diags = diags.Append(validateExprUsingDeprecatedValues(val, n.Expr))
233+
234+
if n.Expr != nil {
235+
_, deprecationDiags := ctx.Deprecations().Validate(val, n.ModulePath(), n.Expr.Range().Ptr())
236+
diags = diags.Append(deprecationDiags)
237+
}
234238

235239
// Set values for arguments of a child module call, for later retrieval
236240
// during expression evaluation.

0 commit comments

Comments
 (0)