From 2657fdb3901b8c6eddd433a17390eb1237c5795d 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 | 21 +++- terraform/evaluate_test.go | 133 ++++++++++++++++++++++ website/upgrade-guides/0-14.html.markdown | 56 ++++++++- 5 files changed, 261 insertions(+), 16 deletions(-) diff --git a/experiments/experiment.go b/experiments/experiment.go index 552f775dc2..c39dac469a 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 8d76edc6bb..1c77293fd0 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 add559cbcc..3fd02772d6 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,11 +753,16 @@ 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 + 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 eb07af314f..2564a5c137 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, @@ -297,6 +300,136 @@ func TestEvaluatorGetResource(t *testing.T) { } } +// GetResource will return a planned object's After value +// if there is a change for that resource instance. +func TestEvaluatorGetResource_changes(t *testing.T) { + // Set up existing state + stateSync := states.BuildState(func(ss *states.SyncState) { + ss.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_resource", + Name: "foo", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectPlanned, + AttrsJSON: []byte(`{"id":"foo", "to_mark_val":"tacos", "sensitive_value":"abc"}`), + }, + addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("test"), + Module: addrs.RootModule, + }, + ) + }).SyncWrapper() + + // Create a change for the existing state resource, + // to exercise retrieving the After value of the change + changesSync := plans.NewChanges().SyncWrapper() + change := &plans.ResourceInstanceChange{ + Addr: mustResourceInstanceAddr("test_resource.foo"), + ProviderAddr: addrs.AbsProviderConfig{ + Module: addrs.RootModule, + Provider: addrs.NewDefaultProvider("test"), + }, + Change: plans.Change{ + Action: plans.Update, + // Provide an After value that contains a marked value + After: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + "to_mark_val": cty.StringVal("pizza").Mark("sensitive"), + "sensitive_value": cty.StringVal("abc"), + }), + }, + } + + // Set up our schemas + schemas := &Schemas{ + Providers: map[addrs.Provider]*ProviderSchema{ + addrs.NewDefaultProvider("test"): { + Provider: &configschema.Block{}, + ResourceTypes: map[string]*configschema.Block{ + "test_resource": { + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + }, + "to_mark_val": { + Type: cty.String, + Computed: true, + }, + "sensitive_value": { + Type: cty.String, + Computed: true, + Sensitive: true, + }, + }, + }, + }, + }, + }, + } + + // The resource we'll inspect + addr := addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_resource", + Name: "foo", + } + schema, _ := schemas.ResourceTypeConfig(addrs.NewDefaultProvider("test"), addr.Mode, addr.Type) + // This encoding separates out the After's marks into its AfterValMarks + csrc, _ := change.Encode(schema.ImpliedType()) + changesSync.AppendResourceInstanceChange(csrc) + + evaluator := &Evaluator{ + Meta: &ContextMeta{ + Env: "foo", + }, + Changes: changesSync, + Config: &configs.Config{ + Module: &configs.Module{ + ManagedResources: map[string]*configs.Resource{ + "test_resource.foo": &configs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_resource", + Name: "foo", + Provider: addrs.Provider{ + Hostname: addrs.DefaultRegistryHost, + Namespace: "hashicorp", + Type: "test", + }, + }, + }, + // Necessary while provider sensitive attrs are experimental + ActiveExperiments: experiments.NewSet(experiments.SuppressProviderSensitiveAttrs), + }, + }, + State: stateSync, + Schemas: schemas, + } + + data := &evaluationStateData{ + Evaluator: evaluator, + } + scope := evaluator.Scope(data, nil) + + want := cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + "to_mark_val": cty.StringVal("pizza").Mark("sensitive"), + "sensitive_value": cty.StringVal("abc").Mark("sensitive"), + }) + + got, diags := scope.Data.GetResource(addr, tfdiags.SourceRange{}) + + if len(diags) != 0 { + t.Errorf("unexpected diagnostics %s", spew.Sdump(diags)) + } + + if !got.RawEquals(want) { + t.Errorf("wrong result:\ngot: %#v\nwant: %#v", got, want) + } +} + func TestEvaluatorGetModule(t *testing.T) { // Create a new evaluator with an existing state stateSync := states.BuildState(func(ss *states.SyncState) { diff --git a/website/upgrade-guides/0-14.html.markdown b/website/upgrade-guides/0-14.html.markdown index 8a3eb0f768..d2ce7bf7c6 100644 --- a/website/upgrade-guides/0-14.html.markdown +++ b/website/upgrade-guides/0-14.html.markdown @@ -323,12 +323,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