From bd70bc63eb1660dbcc9e7c1070f1a0bd00418a62 Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Mon, 26 Oct 2020 11:48:17 -0400 Subject: [PATCH] Add provider sensitivity propagation experiment Rolls back marking attributes providers mark as sensitive to an `experiment` and adds associated docs and adjustments to the upgrade guide. --- experiments/experiment.go | 6 ++- terraform/context_apply_test.go | 61 ++++++++++++++++++++--- terraform/evaluate.go | 19 +++++-- terraform/evaluate_test.go | 5 ++ website/upgrade-guides/0-14.html.markdown | 56 ++++++++++++++++++++- 5 files changed, 132 insertions(+), 15 deletions(-) diff --git a/experiments/experiment.go b/experiments/experiment.go index 552f775dc..c39dac469 100644 --- a/experiments/experiment.go +++ b/experiments/experiment.go @@ -13,8 +13,9 @@ type Experiment string // Each experiment is represented by a string that must be a valid HCL // identifier so that it can be specified in configuration. const ( - VariableValidation = Experiment("variable_validation") - ModuleVariableOptionalAttrs = Experiment("module_variable_optional_attrs") + VariableValidation = Experiment("variable_validation") + ModuleVariableOptionalAttrs = Experiment("module_variable_optional_attrs") + SuppressProviderSensitiveAttrs = Experiment("provider_sensitive_attrs") ) func init() { @@ -22,6 +23,7 @@ func init() { // a current or a concluded experiment. registerConcludedExperiment(VariableValidation, "Custom variable validation can now be used by default, without enabling an experiment.") registerCurrentExperiment(ModuleVariableOptionalAttrs) + registerCurrentExperiment(SuppressProviderSensitiveAttrs) } // GetCurrent takes an experiment name and returns the experiment value diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 8d76edc6b..1c77293fd 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -11869,6 +11869,60 @@ variable "sensitive_map" { resource "test_resource" "foo" { value = var.sensitive_map.x +} +`, + }) + + p := testProvider("test") + p.ApplyResourceChangeFn = testApplyFn + p.PlanResourceChangeFn = testDiffFn + + ctx := testContext2(t, &ContextOpts{ + Config: m, + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + + plan, diags := ctx.Plan() + if diags.HasErrors() { + t.Fatalf("plan errors: %s", diags.Err()) + } + + verifySensitiveValue := func(pvms []cty.PathValueMarks) { + if len(pvms) != 1 { + t.Fatalf("expected 1 sensitive path, got %d", len(pvms)) + } + pvm := pvms[0] + if gotPath, wantPath := pvm.Path, cty.GetAttrPath("value"); !gotPath.Equals(wantPath) { + t.Errorf("wrong path\n got: %#v\nwant: %#v", gotPath, wantPath) + } + if gotMarks, wantMarks := pvm.Marks, cty.NewValueMarks("sensitive"); !gotMarks.Equal(wantMarks) { + t.Errorf("wrong marks\n got: %#v\nwant: %#v", gotMarks, wantMarks) + } + } + + addr := mustResourceInstanceAddr("test_resource.foo") + fooChangeSrc := plan.Changes.ResourceInstance(addr) + verifySensitiveValue(fooChangeSrc.AfterValMarks) + + state, diags := ctx.Apply() + if diags.HasErrors() { + t.Fatalf("apply errors: %s", diags.Err()) + } + + fooState := state.ResourceInstance(addr) + verifySensitiveValue(fooState.Current.AttrSensitivePaths) +} + +func TestContext2Apply_variableSensitivityProviders(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +terraform { + experiments = [provider_sensitive_attrs] +} + +resource "test_resource" "foo" { sensitive_value = "should get marked" } @@ -11917,10 +11971,6 @@ resource "test_resource" "baz" { } } - addr := mustResourceInstanceAddr("test_resource.foo") - fooChangeSrc := plan.Changes.ResourceInstance(addr) - verifySensitiveValue(fooChangeSrc.AfterValMarks) - // Sensitive attributes (defined by the provider) are marked // as sensitive when referenced from another resource // "bar" references sensitive resources in "foo" @@ -11937,9 +11987,6 @@ resource "test_resource" "baz" { t.Fatalf("apply errors: %s", diags.Err()) } - fooState := state.ResourceInstance(addr) - verifySensitiveValue(fooState.Current.AttrSensitivePaths) - barState := state.ResourceInstance(barAddr) verifySensitiveValue(barState.Current.AttrSensitivePaths) diff --git a/terraform/evaluate.go b/terraform/evaluate.go index b403afa08..3fd02772d 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/configs/configschema" + "github.com/hashicorp/terraform/experiments" "github.com/hashicorp/terraform/instances" "github.com/hashicorp/terraform/lang" "github.com/hashicorp/terraform/plans" @@ -752,9 +753,14 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc continue } + // EXPERIMENTAL: Suppressing provider-defined sensitive attrs + // from Terraform output. + // If our schema contains sensitive values, mark those as sensitive - if schema.ContainsSensitive() { - val = markProviderSensitiveAttributes(schema, val) + if moduleConfig.Module.ActiveExperiments.Has(experiments.SuppressProviderSensitiveAttrs) { + if schema.ContainsSensitive() { + val = markProviderSensitiveAttributes(schema, val) + } } instances[key] = val.MarkWithPaths(change.AfterValMarks) continue @@ -774,9 +780,14 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc } val := ios.Value + // EXPERIMENTAL: Suppressing provider-defined sensitive attrs + // from Terraform output. + // If our schema contains sensitive values, mark those as sensitive - if schema.ContainsSensitive() { - val = markProviderSensitiveAttributes(schema, val) + if moduleConfig.Module.ActiveExperiments.Has(experiments.SuppressProviderSensitiveAttrs) { + if schema.ContainsSensitive() { + val = markProviderSensitiveAttributes(schema, val) + } } instances[key] = val } diff --git a/terraform/evaluate_test.go b/terraform/evaluate_test.go index 5e838cd0d..2564a5c13 100644 --- a/terraform/evaluate_test.go +++ b/terraform/evaluate_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/configs/configschema" + "github.com/hashicorp/terraform/experiments" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/tfdiags" @@ -168,6 +169,8 @@ func TestEvaluatorGetResource(t *testing.T) { ManagedResources: map[string]*configs.Resource{ "test_resource.foo": rc, }, + // Necessary while provider sensitive attrs are experimental + ActiveExperiments: experiments.NewSet(experiments.SuppressProviderSensitiveAttrs), }, }, State: stateSync, @@ -397,6 +400,8 @@ func TestEvaluatorGetResource_changes(t *testing.T) { }, }, }, + // Necessary while provider sensitive attrs are experimental + ActiveExperiments: experiments.NewSet(experiments.SuppressProviderSensitiveAttrs), }, }, State: stateSync, diff --git a/website/upgrade-guides/0-14.html.markdown b/website/upgrade-guides/0-14.html.markdown index 0a9441378..c779ac50e 100644 --- a/website/upgrade-guides/0-14.html.markdown +++ b/website/upgrade-guides/0-14.html.markdown @@ -321,12 +321,64 @@ instead of the actual value. Terraform v0.14 introduces a more extensive version of that behavior where Terraform will track when you write an expression whose result is derived from a -[sensitive input variable](/docs/configuration/outputs.html#sensitive-suppressing-values-in-cli-output), +[sensitive input variable](/docs/configuration/outputs.html#sensitive-suppressing-values-in-cli-output) or [sensitive output value](/docs/configuration/variables.html#suppressing-values-in-cli-output), -or an attribute defined as sensitive by a provider, and so after upgrading to Terraform v0.14 you may find that more values are obscured in the Terraform plan output than would have been in Terraform v0.13. +If a sensitive value (either derived from a sensitive input variable or a sensitive output variable) is used in another module output, that output must be marked `sensitive` as well to be explicit about this data being passed through Terraform: + +```terraform +variable "foo" { + sensitive = true +} + +output "bar" { + value = var.foo + # sensitive must be true when referencing a sensitive input variable + sensitive = true +} +``` + +There is also experimental behavior that will extend this sensitivity-awareness to attributes providers define as sensitive. You can enable this feature by activating the experiment in the `terraform` block: + +``` +terraform { + experiments = [provider_sensitive_attrs] +} +``` + +If you enable this experiment, attributes that are defined by a given _provider_ as sensitive will have the same sensitivity-tracking behavior as sensitive input values and outputs. For example, the [`vault_generic_secret`](https://registry.terraform.io/providers/hashicorp/vault/latest/docs/data-sources/generic_secret) data source has an attribute `data` that is sensitive according to this provider's schema. + +``` +# mod/main.tf + +terraform { + experiments = [provider_sensitive_attrs] +} + +data "vault_generic_secret" "foobar" { + path = "secret/foobar" +} + +output "token" { + value = vault_generic_secret.foobar.data["token"] + # a error will display if sensitive = true is not here +} +``` + +If you do not add `sensitive = true` to the output referencing that sensitive attribute, you will get an error: + +``` +Error: Output refers to sensitive values + + on mod/main.tf line 6: + 6: output "token" { + +Expressions used in outputs can only refer to sensitive values if the +sensitive attribute is true. +``` + For this feature we've taken the approach that it's better to be conservative and obscure _potentially-sensitive_ values at the expense of potentially also obscuring some values that aren't sensitive. Unfortunately this means that