fix small pr suggestions

Signed-off-by: Andrei Ciobanu <andrei.ciobanu@opentofu.org>
This commit is contained in:
Andrei Ciobanu 2025-04-04 12:02:44 +03:00 committed by Christian Mesh
parent 216691b03c
commit 8a55dc29da
8 changed files with 65 additions and 32 deletions

View file

@ -446,7 +446,7 @@ func (b *Local) interactiveCollectVariables(ctx context.Context, existing map[st
rawValue, err := uiInput.Input(ctx, &tofu.InputOpts{
Id: fmt.Sprintf("var.%s", name),
Query: fmt.Sprintf("var.%s", name),
Description: variableInputDescription(vc),
Description: vc.InputPrompt(),
Secret: vc.Sensitive,
})
if err != nil {
@ -461,21 +461,6 @@ func (b *Local) interactiveCollectVariables(ctx context.Context, existing map[st
return ret
}
// variableInputDescription compiles a message for the description when the variable input is needed.
// This is returning only the configs.Variable#Description if the variable is not deprecated,
// or it returns both, the description and the deprecation message in one string to be able to
// notify the user about the deprecation of the variable.
func variableInputDescription(v *configs.Variable) string {
switch {
case v.Description != "" && v.DeprecatedSet:
return fmt.Sprintf("%s.\nVariable is marked as deprecated with the following message: %s", v.Description, v.Deprecated)
case v.DeprecatedSet:
return fmt.Sprintf("Variable is marked as deprecated with the following message: %s", v.Deprecated)
default:
return v.Description
}
}
// stubUnsetVariables ensures that all required variables defined in the
// configuration exist in the resulting map, by adding new elements as necessary.
//

View file

@ -164,18 +164,11 @@ func (m *Meta) getInput(ctx context.Context, variable *configs.Variable) (string
return "", fmt.Errorf("input is disabled")
}
varDesc := variable.Description
switch {
case variable.Description != "" && variable.DeprecatedSet:
varDesc = fmt.Sprintf("%s.\nVariable is marked as deprecated with the following message: %s", variable.Description, variable.Deprecated)
case variable.DeprecatedSet:
varDesc = fmt.Sprintf("Variable is marked as deprecated with the following message: %s", variable.Deprecated)
}
uiInput := m.UIInput()
rawValue, err := uiInput.Input(ctx, &tofu.InputOpts{
Id: fmt.Sprintf("var.%s", variable.Name),
Query: fmt.Sprintf("var.%s", variable.Name),
Description: varDesc,
Description: variable.InputPrompt(),
Secret: variable.Sensitive,
})
if err != nil {

View file

@ -709,6 +709,25 @@ func TestConfigImportProviderWithNoResourceProvider(t *testing.T) {
})
}
func TestConfigWithDeprecatedVariables(t *testing.T) {
src, err := os.ReadFile("testdata/variable-empty-deprecated/main.tf")
if err != nil {
t.Fatal(err)
}
parser := testParser(map[string]string{
"main.tf": string(src),
})
_, diags := parser.LoadConfigFile("main.tf")
// The lack of a diagnostic for the "without_deprecated" variable validates also that a variable without any "deprecated" field specified
// is parsed correctly
assertExactDiagnostics(t, diags, []string{
`main.tf:1,10-33: Invalid deprecated value; The variable name "with_empty_deprecated" 'deprecated' field is defined but is empty.`,
`main.tf:7,10-39: Invalid deprecated value; The variable name "with_deprecated_only_spaces" 'deprecated' field is defined but is empty.`,
})
}
func TestTransformForTest(t *testing.T) {
str := func(providers map[string]string) string {

View file

@ -51,9 +51,8 @@ func (v *Variable) merge(ov *Variable) hcl.Diagnostics {
v.Sensitive = ov.Sensitive
v.SensitiveSet = ov.SensitiveSet
}
if ov.DeprecatedSet {
if ov.Deprecated != "" {
v.Deprecated = ov.Deprecated
v.DeprecatedSet = ov.DeprecatedSet
}
if ov.Default != cty.NilVal {
v.Default = ov.Default

View file

@ -31,7 +31,6 @@ func TestModuleOverrideVariable(t *testing.T) {
DescriptionSet: true,
Default: cty.StringVal("b_override"),
Deprecated: "b_override deprecated",
DeprecatedSet: true,
Nullable: false,
NullableSet: true,
Type: cty.String,
@ -57,7 +56,6 @@ func TestModuleOverrideVariable(t *testing.T) {
DescriptionSet: true,
Default: cty.StringVal("b_override partial"),
Deprecated: "b_override deprecated",
DeprecatedSet: true,
Nullable: true,
NullableSet: false,
Type: cty.String,

View file

@ -7,6 +7,7 @@ package configs
import (
"fmt"
"strings"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/ext/typeexpr"
@ -42,7 +43,6 @@ type Variable struct {
DescriptionSet bool
SensitiveSet bool
DeprecatedSet bool
// Nullable indicates that null is a valid value for this variable. Setting
// Nullable to false means that the module can expect this variable to
@ -127,8 +127,15 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno
if attr, exists := content.Attributes["deprecated"]; exists {
valDiags := gohcl.DecodeExpression(attr.Expr, nil, &v.Deprecated)
if strings.TrimSpace(v.Deprecated) == "" {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid deprecated value",
Detail: fmt.Sprintf("The variable name %q 'deprecated' field is defined but is empty.", v.Name),
Subject: &block.LabelRanges[0],
})
}
diags = append(diags, valDiags...)
v.DeprecatedSet = true
}
if attr, exists := content.Attributes["nullable"]; exists {
@ -288,6 +295,22 @@ func (v *Variable) Required() bool {
return v.Default == cty.NilVal
}
// InputPrompt returns the text that will be shown during prompting for the variable input when required but on value given.
// This method is meant to return also the deprecated message together with the description when both exists or only the
// deprecated info when the description is missing.
// Other than these 2 cases, the method is keeping the default behavior in case of both (deprecated and description missing),
// by returning the description. This will keep the previous behavior where during prompting will be shown only the
// variable name.
func (v *Variable) InputPrompt() string {
switch {
case v.Description != "" && v.Deprecated != "":
return fmt.Sprintf("%s.\nVariable is marked as deprecated with the following message: %s", v.Description, v.Deprecated)
case v.Deprecated != "":
return fmt.Sprintf("Variable is marked as deprecated with the following message: %s", v.Deprecated)
}
return v.Description
}
// VariableParsingMode defines how values of a particular variable given by
// text-only mechanisms (command line arguments and environment variables)
// should be parsed to produce the final value.

View file

@ -0,0 +1,16 @@
variable "with_empty_deprecated" {
type = string
description = "variable that is having the deprecated field specified but is having no value"
deprecated = ""
}
variable "with_deprecated_only_spaces" {
type = string
description = "variable that is having the deprecated field specified only with blank characters"
deprecated = " "
}
variable "without_deprecated" {
type = string
description = "no deprecated field defined so it should work fine"
}

View file

@ -480,7 +480,7 @@ You can correct this by removing references to sensitive values, or by carefully
// evalVariableDeprecation checks if a variable is deprecated and if so it returns a warning diagnostic to be shown to the user
func evalVariableDeprecation(addr addrs.AbsInputVariableInstance, config *configs.Variable, expr hcl.Expression, ctx EvalContext) tfdiags.Diagnostics {
if !config.DeprecatedSet {
if config.Deprecated == "" {
log.Printf("[TRACE] evalVariableDeprecation: variable %s is having no deprecation configured", addr)
return nil
}
@ -490,7 +490,7 @@ func evalVariableDeprecation(addr addrs.AbsInputVariableInstance, config *config
return nil
}
val := ctx.GetVariableValue(addr)
if val.IsNull() {
if val == cty.NilVal || val.IsNull() {
log.Printf("[TRACE] evalVariableDeprecation: variable %s is marked as deprecated null value given", addr)
return nil
}