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

terraform test: remove marks before passing variables as inputs to a plan #34190

Merged
merged 3 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
61 changes: 53 additions & 8 deletions internal/backend/local/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/hashicorp/terraform/internal/backend"
"github.com/hashicorp/terraform/internal/command/views"
"github.com/hashicorp/terraform/internal/configs"
"github.com/hashicorp/terraform/internal/lang/marks"
"github.com/hashicorp/terraform/internal/logging"
"github.com/hashicorp/terraform/internal/moduletest"
configtest "github.com/hashicorp/terraform/internal/moduletest/config"
Expand Down Expand Up @@ -372,14 +373,19 @@ func (runner *TestFileRunner) run(run *moduletest.Run, file *moduletest.File, st
return state, false
}

variables, variableDiags := runner.GetVariables(config, run, file, references)
variables, variableDiags := runner.GetVariables(config, run, references)
run.Diagnostics = run.Diagnostics.Append(variableDiags)
if variableDiags.HasErrors() {
run.Status = moduletest.Error
return state, false
}

planCtx, plan, planDiags := runner.plan(config, state, run, file, runner.FilterVariablesToConfig(config, variables), references, start)
// FilterVariablesToConfig only returns warnings, so we don't check the
// returned diags for errors.
setVariables, setVariableDiags := runner.FilterVariablesToConfig(config, variables)
run.Diagnostics = run.Diagnostics.Append(setVariableDiags)

planCtx, plan, planDiags := runner.plan(config, state, run, file, setVariables, references, start)
if run.Config.Command == configs.PlanTestCommand {
// Then we want to assess our conditions and diagnostics differently.
planDiags = run.ValidateExpectedFailures(planDiags)
Expand Down Expand Up @@ -563,16 +569,22 @@ func (runner *TestFileRunner) destroy(config *configs.Config, state *states.Stat

var diags tfdiags.Diagnostics

variables, variableDiags := runner.GetVariables(config, run, file, nil)
variables, variableDiags := runner.GetVariables(config, run, nil)
diags = diags.Append(variableDiags)

if diags.HasErrors() {
return state, diags
}

// During the destroy operation, we don't add warnings from this operation.
// Anything that would have been reported here was already reported during
// the original plan, and a successful destroy operation is the only thing
// we care about.
setVariables, _ := runner.FilterVariablesToConfig(config, variables)

planOpts := &terraform.PlanOpts{
Mode: plans.DestroyMode,
SetVariables: runner.FilterVariablesToConfig(config, variables),
SetVariables: setVariables,
}

tfCtx, ctxDiags := terraform.NewContext(runner.Suite.Opts)
Expand Down Expand Up @@ -951,7 +963,7 @@ func (runner *TestFileRunner) cleanup(file *moduletest.File) {
// more variables than are required by the config. FilterVariablesToConfig
// should be called before trying to use these variables within a Terraform
// plan, apply, or destroy operation.
func (runner *TestFileRunner) GetVariables(config *configs.Config, run *moduletest.Run, file *moduletest.File, references []*addrs.Reference) (terraform.InputValues, tfdiags.Diagnostics) {
func (runner *TestFileRunner) GetVariables(config *configs.Config, run *moduletest.Run, references []*addrs.Reference) (terraform.InputValues, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics

// relevantVariables contains the variables that are of interest to this
Expand Down Expand Up @@ -1124,17 +1136,50 @@ func (runner *TestFileRunner) GetVariables(config *configs.Config, run *modulete
// This function is essentially the opposite of AddVariablesToConfig which
// makes the config match the variables rather than the variables match the
// config.
func (runner *TestFileRunner) FilterVariablesToConfig(config *configs.Config, values terraform.InputValues) terraform.InputValues {
//
// warnOnSensitivityChange will prompt this function to add warnings if an input
Copy link
Member

@jbardin jbardin Nov 7, 2023

Choose a reason for hiding this comment

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

Is this a function/variable name that was refactored out elsewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, initially I had an argument that meant the function would only selectively add the warnings if this was true but then I realised the caller could just ignore the diagnostics so we didn't need to modify the function to exclude them internally.

I must've forgot update the doc comment when I removed it!

// variable is marked as sensitive and the config variable it is being assigned
// to is not.
//
// This function can only return warnings, and the callers can rely on this so
// please check the callers of this function if you add any error diagnostics.
func (runner *TestFileRunner) FilterVariablesToConfig(config *configs.Config, values terraform.InputValues) (terraform.InputValues, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics

filtered := make(terraform.InputValues)
for name, value := range values {
if _, exists := config.Module.Variables[name]; !exists {
variableConfig, exists := config.Module.Variables[name]
if !exists {
// Only include values that are actually required by the config.
continue
}

if marks.Has(value.Value, marks.Sensitive) {
unmarkedValue, _ := value.Value.Unmark()
if !variableConfig.Sensitive {
// Then we are passing a sensitive value into a non-sensitive
// variable. Let's add a warning and tell the user they should
// mark the config as sensitive as well. If the config variable
// is sensitive, then we don't need to worry.
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagWarning,
Summary: "Sensitive metadata on variable lost",
Detail: fmt.Sprintf("The input variable is marked as sensitive, while the receiving configuration is not. The underlying sensitive information may be exposed when var.%s is referenced. Mark the variable block in the configuration as sensitive to resolve this warning.", variableConfig.Name),
Subject: value.SourceRange.ToHCL().Ptr(),
})
}

// Set the unmarked value into the input value.
value = &terraform.InputValue{
Value: unmarkedValue,
SourceType: value.SourceType,
SourceRange: value.SourceRange,
}
}

filtered[name] = value
}
return filtered
return filtered, diags
}

// AddVariablesToConfig extends the provided config to ensure it has definitions
Expand Down
74 changes: 74 additions & 0 deletions internal/command/test_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1560,6 +1560,80 @@ operation, and the specified output value is only known after apply.

}

func TestTest_SensitiveInputValues(t *testing.T) {
td := t.TempDir()
testCopyDir(t, testFixturePath(path.Join("test", "sensitive_input_values")), td)
defer testChdir(t, td)()

provider := testing_command.NewProvider(nil)

providerSource, close := newMockProviderSource(t, map[string][]string{
"test": {"1.0.0"},
})
defer close()

streams, done := terminal.StreamsForTesting(t)
view := views.NewView(streams)
ui := new(cli.MockUi)

meta := Meta{
testingOverrides: metaOverridesForProvider(provider.Provider),
Ui: ui,
View: view,
Streams: streams,
ProviderSource: providerSource,
}

init := &InitCommand{
Meta: meta,
}

if code := init.Run(nil); code != 0 {
t.Fatalf("expected status code 0 but got %d: %s", code, ui.ErrorWriter)
}

c := &TestCommand{
Meta: meta,
}

code := c.Run([]string{"-no-color"})
output := done(t)

if code != 0 {
t.Errorf("expected status code 0 but got %d", code)
}

expected := `main.tftest.hcl... in progress
run "setup"... pass
run "test"... pass

Warning: Sensitive metadata on variable lost

on main.tftest.hcl line 13, in run "test":
13: password = run.setup.password

The input variable is marked as sensitive, while the receiving configuration
is not. The underlying sensitive information may be exposed when var.password
is referenced. Mark the variable block in the configuration as sensitive to
resolve this warning.

main.tftest.hcl... tearing down
main.tftest.hcl... pass

Success! 2 passed, 0 failed.
`

actual := output.All()

if diff := cmp.Diff(actual, expected); len(diff) > 0 {
t.Errorf("output didn't match expected:\nexpected:\n%s\nactual:\n%s\ndiff:\n%s", expected, actual, diff)
}

if provider.ResourceCount() > 0 {
t.Errorf("should have deleted all resources on completion but left %v", provider.ResourceString())
}
}

// This test takes around 10 seconds to complete, as we're testing the progress
// updates that are printed every 2 seconds. Sorry!
func TestTest_LongRunningTest(t *testing.T) {
Expand Down
8 changes: 8 additions & 0 deletions internal/command/testdata/test/sensitive_input_values/main.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
variable "password" {
type = string
}

output "password" {
value = var.password
sensitive = true
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
run "setup" {
variables {
password = "password"
}

module {
source = "./setup"
}
}

run "test" {
variables {
password = run.setup.password
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
variable "password" {
sensitive = true
type = string
}

output "password" {
value = var.password
sensitive = true
}