diff --git a/internal/backend/local/backend_local.go b/internal/backend/local/backend_local.go index 9a63936022..1e73bf881d 100644 --- a/internal/backend/local/backend_local.go +++ b/internal/backend/local/backend_local.go @@ -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. // diff --git a/internal/command/meta_config.go b/internal/command/meta_config.go index de261f224e..41ef4d16f0 100644 --- a/internal/command/meta_config.go +++ b/internal/command/meta_config.go @@ -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 { diff --git a/internal/configs/config_test.go b/internal/configs/config_test.go index 5890d5ef6e..3defc3fdca 100644 --- a/internal/configs/config_test.go +++ b/internal/configs/config_test.go @@ -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 { diff --git a/internal/configs/module_merge.go b/internal/configs/module_merge.go index 0bfc68747e..eba5c4baf7 100644 --- a/internal/configs/module_merge.go +++ b/internal/configs/module_merge.go @@ -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 diff --git a/internal/configs/module_merge_test.go b/internal/configs/module_merge_test.go index 06bb268068..e4511eba9a 100644 --- a/internal/configs/module_merge_test.go +++ b/internal/configs/module_merge_test.go @@ -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, diff --git a/internal/configs/named_values.go b/internal/configs/named_values.go index 136ce582e5..84bc6c7e2e 100644 --- a/internal/configs/named_values.go +++ b/internal/configs/named_values.go @@ -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. diff --git a/internal/configs/testdata/variable-empty-deprecated/main.tf b/internal/configs/testdata/variable-empty-deprecated/main.tf new file mode 100644 index 0000000000..9b7a6f8962 --- /dev/null +++ b/internal/configs/testdata/variable-empty-deprecated/main.tf @@ -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" +} \ No newline at end of file diff --git a/internal/tofu/eval_variable.go b/internal/tofu/eval_variable.go index 2fae75b4b3..42ceb1bedb 100644 --- a/internal/tofu/eval_variable.go +++ b/internal/tofu/eval_variable.go @@ -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 }