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

Fixes #29156: Failing sensitive variables values are not logged #30552

Merged
merged 2 commits into from May 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 31 additions & 18 deletions internal/terraform/eval_variable.go
Expand Up @@ -90,27 +90,40 @@ func prepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, raw *In
val, err := convert.Convert(given, convertTy)
if err != nil {
log.Printf("[ERROR] prepareFinalInputVariableValue: %s has unsuitable type\n got: %s\n want: %s", addr, given.Type(), convertTy)
var detail string
var subject *hcl.Range
if nonFileSource != "" {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid value for input variable",
Detail: fmt.Sprintf(
"Unsuitable value for %s %s: %s.",
addr, nonFileSource, err,
),
Subject: cfg.DeclRange.Ptr(),
})
detail = fmt.Sprintf(
"Unsuitable value for %s %s: %s.",
addr, nonFileSource, err,
)
subject = cfg.DeclRange.Ptr()
} else {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid value for input variable",
Detail: fmt.Sprintf(
"The given value is not suitable for %s declared at %s: %s.",
addr, cfg.DeclRange.String(), err,
),
Subject: sourceRange.ToHCL().Ptr(),
})
detail = fmt.Sprintf(
"The given value is not suitable for %s declared at %s: %s.",
addr, cfg.DeclRange.String(), err,
)
subject = sourceRange.ToHCL().Ptr()

// In some workflows, the operator running terraform does not have access to the variables
// themselves. They are for example stored in encrypted files that will be used by the CI toolset
// and not by the operator directly. In such a case, the failing secret value should not be
// displayed to the operator
if cfg.Sensitive {
detail = fmt.Sprintf(
"The given value is not suitable for %s, which is sensitive: %s. Invalid value defined at %s.",
addr, err, sourceRange.ToHCL(),
)
subject = cfg.DeclRange.Ptr()
}
}

diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid value for input variable",
Detail: detail,
Subject: subject,
})
// We'll return a placeholder unknown value to avoid producing
// redundant downstream errors.
return cty.UnknownVal(cfg.Type), diags
Expand Down
70 changes: 70 additions & 0 deletions internal/terraform/eval_variable_test.go
Expand Up @@ -60,6 +60,11 @@ func TestPrepareFinalInputVariableValue(t *testing.T) {
type = string
default = true
}
variable "constrained_string_sensitive_required" {
sensitive = true
nullable = false
type = string
}
`
cfg := testModuleInline(t, map[string]string{
"main.tf": cfgSrc,
Expand Down Expand Up @@ -390,6 +395,14 @@ func TestPrepareFinalInputVariableValue(t *testing.T) {
cty.UnknownVal(cty.String),
``,
},

// sensitive
{
"constrained_string_sensitive_required",
cty.UnknownVal(cty.String),
cty.UnknownVal(cty.String),
``,
},
}

for _, test := range tests {
Expand Down Expand Up @@ -560,4 +573,61 @@ func TestPrepareFinalInputVariableValue(t *testing.T) {
})
}
})

t.Run("SensitiveVariable error message variants, with source variants", func(t *testing.T) {
tests := []struct {
SourceType ValueSourceType
SourceRange tfdiags.SourceRange
WantTypeErr string
HideSubject bool
}{
{
ValueFromUnknown,
tfdiags.SourceRange{},
"Invalid value for input variable: Unsuitable value for var.constrained_string_sensitive_required set from outside of the configuration: string required.",
false,
},
{
ValueFromConfig,
tfdiags.SourceRange{
Filename: "example.tfvars",
Start: tfdiags.SourcePos(hcl.InitialPos),
End: tfdiags.SourcePos(hcl.InitialPos),
},
`Invalid value for input variable: The given value is not suitable for var.constrained_string_sensitive_required, which is sensitive: string required. Invalid value defined at example.tfvars:1,1-1.`,
true,
},
}

for _, test := range tests {
t.Run(fmt.Sprintf("%s %s", test.SourceType, test.SourceRange.StartString()), func(t *testing.T) {
varAddr := addrs.InputVariable{Name: "constrained_string_sensitive_required"}.Absolute(addrs.RootModuleInstance)
varCfg := variableConfigs[varAddr.Variable.Name]
t.Run("type error", func(t *testing.T) {
rawVal := &InputValue{
Value: cty.EmptyObjectVal,
SourceType: test.SourceType,
SourceRange: test.SourceRange,
}

_, diags := prepareFinalInputVariableValue(
varAddr, rawVal, varCfg,
)
if !diags.HasErrors() {
t.Fatalf("unexpected success; want error")
}

if got, want := diags.Err().Error(), test.WantTypeErr; got != want {
t.Errorf("wrong error\ngot: %s\nwant: %s", got, want)
}

if test.HideSubject {
if got, want := diags[0].Source().Subject.StartString(), test.SourceRange.StartString(); got == want {
t.Errorf("Subject start should have been hidden, but was %s", got)
}
}
})
})
}
})
}