From fb98fc98fa4d10488c098b2c0379c22735792ebb Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Tue, 20 Oct 2020 12:20:52 -0400 Subject: [PATCH] terraform: Fix sensitive values in ignore changes Because ignore_changes configuration can refer to resource arguments which are assigned sensitive values, we need to unmark the resource object before processing. --- terraform/context_plan_test.go | 59 +++++++++++++++++++ terraform/eval_diff.go | 22 +++---- .../ignore-changes-sensitive.tf | 11 ++++ 3 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 terraform/testdata/plan-ignore-changes-sensitive/ignore-changes-sensitive.tf diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 96882212e..819870f96 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -4667,6 +4667,65 @@ func TestContext2Plan_ignoreChangesInMap(t *testing.T) { }), ric.After) } +func TestContext2Plan_ignoreChangesSensitive(t *testing.T) { + m := testModule(t, "plan-ignore-changes-sensitive") + p := testProvider("aws") + p.PlanResourceChangeFn = testDiffFn + + state := states.NewState() + root := state.EnsureModule(addrs.RootModuleInstance) + root.SetResourceInstanceCurrent( + mustResourceInstanceAddr("aws_instance.foo").Resource, + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"bar","ami":"ami-abcd1234","type":"aws_instance"}`), + }, + mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`), + ) + + ctx := testContext2(t, &ContextOpts{ + Config: m, + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p), + }, + Variables: InputValues{ + "foo": &InputValue{ + Value: cty.StringVal("ami-1234abcd"), + SourceType: ValueFromCaller, + }, + }, + State: state, + }) + + plan, diags := ctx.Plan() + if diags.HasErrors() { + t.Fatalf("unexpected errors: %s", diags.Err()) + } + + schema := p.GetSchemaReturn.ResourceTypes["aws_instance"] + ty := schema.ImpliedType() + + if len(plan.Changes.Resources) != 1 { + t.Fatal("expected 1 changes, got", len(plan.Changes.Resources)) + } + + res := plan.Changes.Resources[0] + ric, err := res.Decode(ty) + if err != nil { + t.Fatal(err) + } + + if ric.Addr.String() != "aws_instance.foo" { + t.Fatalf("unexpected resource: %s", ric.Addr) + } + + checkVals(t, objectVal(t, schema, map[string]cty.Value{ + "id": cty.StringVal("bar"), + "ami": cty.StringVal("ami-abcd1234"), + "type": cty.StringVal("aws_instance"), + }), ric.After) +} + func TestContext2Plan_moduleMapLiteral(t *testing.T) { m := testModule(t, "plan-module-map-literal") p := testProvider("aws") diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index 08aafb2f7..cc3a92175 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -202,24 +202,24 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { priorVal = cty.NullVal(schema.ImpliedType()) } + // Create an unmarked version of our config val and our prior val. + // Store the paths for the config val to re-markafter + // we've sent things over the wire. + unmarkedConfigVal, unmarkedPaths := origConfigVal.UnmarkDeepWithPaths() + unmarkedPriorVal, priorPaths := priorVal.UnmarkDeepWithPaths() + // ignore_changes is meant to only apply to the configuration, so it must // be applied before we generate a plan. This ensures the config used for // the proposed value, the proposed value itself, and the config presented // to the provider in the PlanResourceChange request all agree on the // starting values. - configValIgnored, ignoreChangeDiags := n.processIgnoreChanges(priorVal, origConfigVal) + configValIgnored, ignoreChangeDiags := n.processIgnoreChanges(unmarkedPriorVal, unmarkedConfigVal) diags = diags.Append(ignoreChangeDiags) if ignoreChangeDiags.HasErrors() { return nil, diags.Err() } - // Create an unmarked version of our config val and our prior val. - // Store the paths for the config val to re-markafter - // we've sent things over the wire. - unmarkedConfigVal, unmarkedPaths := configValIgnored.UnmarkDeepWithPaths() - unmarkedPriorVal, priorPaths := priorVal.UnmarkDeepWithPaths() - - proposedNewVal := objchange.ProposedNewObject(schema, unmarkedPriorVal, unmarkedConfigVal) + proposedNewVal := objchange.ProposedNewObject(schema, unmarkedPriorVal, configValIgnored) // Call pre-diff hook if !n.Stub { @@ -238,7 +238,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { validateResp := provider.ValidateResourceTypeConfig( providers.ValidateResourceTypeConfigRequest{ TypeName: n.Addr.Resource.Type, - Config: unmarkedConfigVal, + Config: configValIgnored, }, ) if validateResp.Diagnostics.HasErrors() { @@ -247,7 +247,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { resp := provider.PlanResourceChange(providers.PlanResourceChangeRequest{ TypeName: n.Addr.Resource.Type, - Config: unmarkedConfigVal, + Config: configValIgnored, PriorState: unmarkedPriorVal, ProposedNewState: proposedNewVal, PriorPrivate: priorPrivate, @@ -286,7 +286,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { return nil, diags.Err() } - if errs := objchange.AssertPlanValid(schema, unmarkedPriorVal, unmarkedConfigVal, plannedNewVal); len(errs) > 0 { + if errs := objchange.AssertPlanValid(schema, unmarkedPriorVal, configValIgnored, plannedNewVal); len(errs) > 0 { if resp.LegacyTypeSystem { // The shimming of the old type system in the legacy SDK is not precise // enough to pass this consistency check, so we'll give it a pass here, diff --git a/terraform/testdata/plan-ignore-changes-sensitive/ignore-changes-sensitive.tf b/terraform/testdata/plan-ignore-changes-sensitive/ignore-changes-sensitive.tf new file mode 100644 index 000000000..1f6cc98ac --- /dev/null +++ b/terraform/testdata/plan-ignore-changes-sensitive/ignore-changes-sensitive.tf @@ -0,0 +1,11 @@ +variable "foo" { + sensitive = true +} + +resource "aws_instance" "foo" { + ami = var.foo + + lifecycle { + ignore_changes = [ami] + } +}