Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecation Warning For Variables #36079

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions internal/configs/named_values.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ type Variable struct {
Nullable bool
NullableSet bool

Deprecated string
DeprecatedSet bool
DeprecatedRange hcl.Range

DeclRange hcl.Range
}

Expand Down Expand Up @@ -186,6 +190,13 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno
v.Default = val
}

if attr, exists := content.Attributes["deprecated"]; exists {
valDiags := gohcl.DecodeExpression(attr.Expr, nil, &v.Deprecated)
diags = append(diags, valDiags...)
v.DeprecatedSet = true
v.DeprecatedRange = attr.Range
}

for _, block := range content.Blocks {
switch block.Type {

Expand Down Expand Up @@ -499,6 +510,9 @@ var variableBlockSchema = &hcl.BodySchema{
{
Name: "nullable",
},
{
Name: "deprecated",
},
},
Blocks: []hcl.BlockHeaderSchema{
{
Expand Down
27 changes: 27 additions & 0 deletions internal/configs/named_values_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,30 @@ func TestVariableInvalidDefault(t *testing.T) {
}
}
}

func TestVariableDeprecation(t *testing.T) {
src := `
variable "foo" {
type = string
deprecated = "This variable is deprecated, use bar instead"
}
`

hclF, diags := hclsyntax.ParseConfig([]byte(src), "test.tf", hcl.InitialPos)
if diags.HasErrors() {
t.Fatal(diags.Error())
}

b, diags := parseConfigFile(hclF.Body, nil, false, false)
if diags.HasErrors() {
t.Fatalf("unexpected error: %q", diags)
}

if !b.Variables[0].DeprecatedSet {
t.Fatalf("expected variable to be deprecated")
}

if b.Variables[0].Deprecated != "This variable is deprecated, use bar instead" {
t.Fatalf("expected variable to have deprecation message")
}
}
133 changes: 133 additions & 0 deletions internal/terraform/context_plan2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6068,3 +6068,136 @@ data "test_data_source" "foo" {
_, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables)))
assertNoErrors(t, diags)
}

func TestContext2Plan_deprecated_variable(t *testing.T) {
m := testModuleInline(t, map[string]string{
"mod/main.tf": `
variable "old-and-used" {
type = string
deprecated = "module variable deprecation"
default = "optional"
}

variable "old-and-unused" {
type = string
deprecated = "module variable deprecation"
default = "optional"
}

variable "new" {
type = string
default = "optional"
}

output "use-everything" {
value = {
used = var.old-and-used
unused = var.old-and-unused
new = var.new
}
}
`,
"main.tf": `
variable "root-old-and-used" {
type = string
deprecated = "root variable deprecation"
default = "optional"
}

variable "root-old-and-unused" {
type = string
deprecated = "root variable deprecation"
default = "optional"
}

variable "new" {
type = string
default = "new"
}

module "old-mod" {
source = "./mod"
old-and-used = "old"
}

module "new-mod" {
source = "./mod"
new = "new"
}

output "use-everything" {
value = {
used = var.root-old-and-used
unused = var.root-old-and-unused
new = var.new
}
}
`,
})

p := new(testing_provider.MockProvider)
p.GetProviderSchemaResponse = getProviderSchemaResponseFromProviderSchema(&providerSchema{
ResourceTypes: map[string]*configschema.Block{
"test_resource": {
Attributes: map[string]*configschema.Attribute{
"attr": {
Type: cty.String,
Computed: true,
},
},
},
},
})

ctx := testContext2(t, &ContextOpts{
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
},
})

vars := InputValues{
"root-old-and-used": {
Value: cty.StringVal("root-old-and-used"),
},
"root-old-and-unused": {
Value: cty.NullVal(cty.String),
},
"new": {
Value: cty.StringVal("new"),
},
}

// We can find module variable deprecation during validation
diags := ctx.Validate(m, &ValidateOpts{})
var expectedValidateDiags tfdiags.Diagnostics
expectedValidateDiags = expectedValidateDiags.Append(
&hcl.Diagnostic{
Severity: hcl.DiagWarning,
Summary: "Usage of deprecated variable",
Detail: "module variable deprecation",
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 21, Column: 20, Byte: 346},
End: hcl.Pos{Line: 21, Column: 25, Byte: 351},
},
},
)
assertDiagnosticsMatch(t, diags, expectedValidateDiags)

_, diags = ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, vars))
var expectedPlanDiags tfdiags.Diagnostics
expectedPlanDiags = expectedPlanDiags.Append(
&hcl.Diagnostic{
Severity: hcl.DiagWarning,
Summary: "Usage of deprecated variable",
Detail: "root variable deprecation",
Subject: &hcl.Range{
Filename: filepath.Join(m.Module.SourceDir, "main.tf"),
Start: hcl.Pos{Line: 4, Column: 2, Byte: 48},
End: hcl.Pos{Line: 4, Column: 42, Byte: 88},
},
},
)

assertDiagnosticsMatch(t, diags, expectedPlanDiags)
}
9 changes: 9 additions & 0 deletions internal/terraform/node_module_variable.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,15 @@ func (n *nodeModuleVariable) evalModuleVariable(ctx EvalContext, validateOnly bo
finalVal, moreDiags := prepareFinalInputVariableValue(n.Addr, rawVal, n.Config)
diags = diags.Append(moreDiags)

if n.Config.DeprecatedSet && givenVal.IsWhollyKnown() && !givenVal.IsNull() && validateOnly {
Copy link
Member

@jbardin jbardin Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If validateOnly is true, then givenVal may likely be unknown (unless maybe it's a static literal in the config). Validation is done with as many values unknown as possible since it should work for all possible input values, and to get the most complete evaluation possible of all branches in the configuration

If you look above however (L295), is appears that cty.NilVal is used here to represent an entirely unset input variable (due to the legacy ability to assign null as a value to override module inputs), so simple direct comparison to that should suffice here.

diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagWarning,
Summary: "Usage of deprecated variable",
Detail: n.Config.Deprecated,
Subject: n.Expr.Range().Ptr(),
})
}

return finalVal, diags.ErrWithWarnings()
}

Expand Down
9 changes: 9 additions & 0 deletions internal/terraform/node_root_variable.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,15 @@ func (n *NodeRootVariable) Execute(ctx EvalContext, op walkOperation) tfdiags.Di
}
}

if n.Config.DeprecatedSet && givenVal.Value.IsWhollyKnown() && !givenVal.Value.IsNull() && op == walkPlan {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly here, you should be able to determine if the variable is set by it being non-null. It's unfortunate that we have to wait until plan however.

diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagWarning,
Summary: "Usage of deprecated variable",
Detail: n.Config.Deprecated,
Subject: &n.Config.DeprecatedRange,
})
}

if n.Planning {
if checkState := ctx.Checks(); checkState.ConfigHasChecks(n.Addr.InModule(addrs.RootModule)) {
ctx.Checks().ReportCheckableObjects(
Expand Down
Loading