diff --git a/internal/terraform/context_apply2_test.go b/internal/terraform/context_apply2_test.go index 5d90e8f09..681595959 100644 --- a/internal/terraform/context_apply2_test.go +++ b/internal/terraform/context_apply2_test.go @@ -508,3 +508,69 @@ output "out" { } } } + +func TestContext2Apply_ignoreImpureFunctionChanges(t *testing.T) { + // The impure function call should not cause a planned change with + // ignore_changes + m := testModuleInline(t, map[string]string{ + "main.tf": ` +variable "pw" { + sensitive = true + default = "foo" +} + +resource "test_object" "x" { + test_map = { + string = "X${bcrypt(var.pw)}" + } + lifecycle { + ignore_changes = [ test_map["string"] ] + } +} + +resource "test_object" "y" { + test_map = { + string = "X${bcrypt(var.pw)}" + } + lifecycle { + ignore_changes = [ test_map ] + } +} + +`, + }) + + p := simpleMockProvider() + + ctx := testContext2(t, &ContextOpts{ + Config: m, + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + + _, diags := ctx.Plan() + if diags.HasErrors() { + t.Fatal(diags.ErrWithWarnings()) + } + + _, diags = ctx.Apply() + if diags.HasErrors() { + t.Fatal(diags.ErrWithWarnings()) + } + + // FINAL PLAN: + plan, diags := ctx.Plan() + if diags.HasErrors() { + t.Fatal(diags.ErrWithWarnings()) + } + + // make sure the same marks are compared in the next plan as well + for _, c := range plan.Changes.Resources { + if c.Action != plans.NoOp { + t.Logf("marks before: %#v", c.BeforeValMarks) + t.Logf("marks after: %#v", c.AfterValMarks) + t.Errorf("Unexpcetd %s change for %s", c.Action, c.Addr) + } + } +} diff --git a/internal/terraform/context_plan_test.go b/internal/terraform/context_plan_test.go index 4efc2f136..8a5bc9e6f 100644 --- a/internal/terraform/context_plan_test.go +++ b/internal/terraform/context_plan_test.go @@ -4794,7 +4794,7 @@ func TestContext2Plan_ignoreChangesSensitive(t *testing.T) { checkVals(t, objectVal(t, schema, map[string]cty.Value{ "id": cty.StringVal("bar"), - "ami": cty.StringVal("ami-abcd1234").Mark(marks.Sensitive), + "ami": cty.StringVal("ami-abcd1234"), "type": cty.StringVal("aws_instance"), }), ric.After) } diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index c3c28e2c6..d2279f88f 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -687,26 +687,24 @@ func (n *NodeAbstractResourceInstance) plan( 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() - log.Printf("[TRACE] Re-validating config for %q", n.Addr) - // Allow the provider to validate the final set of values. - // The config was statically validated early on, but there may have been - // unknown values which the provider could not validate at the time. + // Allow the provider to validate the final set of values. The config was + // statically validated early on, but there may have been unknown values + // which the provider could not validate at the time. + // // TODO: It would be more correct to validate the config after // ignore_changes has been applied, but the current implementation cannot // exclude computed-only attributes when given the `all` option. + + // we must unmark and use the original config, since the ignore_changes + // handling below needs access to the marks. + unmarkedConfigVal, _ := origConfigVal.UnmarkDeep() validateResp := provider.ValidateResourceConfig( providers.ValidateResourceConfigRequest{ TypeName: n.Addr.Resource.Resource.Type, Config: unmarkedConfigVal, }, ) - diags = diags.Append(validateResp.Diagnostics.InConfigBody(config.Config, n.Addr.String())) if diags.HasErrors() { return plan, state, diags @@ -717,13 +715,21 @@ func (n *NodeAbstractResourceInstance) plan( // 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(unmarkedPriorVal, unmarkedConfigVal) + // Here we operate on the marked values, so as to revert any changes to the + // marks as well as the value. + configValIgnored, ignoreChangeDiags := n.processIgnoreChanges(priorVal, origConfigVal) diags = diags.Append(ignoreChangeDiags) if ignoreChangeDiags.HasErrors() { return plan, state, diags } - proposedNewVal := objchange.ProposedNew(schema, unmarkedPriorVal, configValIgnored) + // Create an unmarked version of our config val and our prior val. + // Store the paths for the config val to re-mark after we've sent things + // over the wire. + unmarkedConfigVal, unmarkedPaths := configValIgnored.UnmarkDeepWithPaths() + unmarkedPriorVal, priorPaths := priorVal.UnmarkDeepWithPaths() + + proposedNewVal := objchange.ProposedNew(schema, unmarkedPriorVal, unmarkedConfigVal) // Call pre-diff hook diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { @@ -735,7 +741,7 @@ func (n *NodeAbstractResourceInstance) plan( resp := provider.PlanResourceChange(providers.PlanResourceChangeRequest{ TypeName: n.Addr.Resource.Resource.Type, - Config: configValIgnored, + Config: unmarkedConfigVal, PriorState: unmarkedPriorVal, ProposedNewState: proposedNewVal, PriorPrivate: priorPrivate, @@ -774,7 +780,7 @@ func (n *NodeAbstractResourceInstance) plan( return plan, state, diags } - if errs := objchange.AssertPlanValid(schema, unmarkedPriorVal, configValIgnored, plannedNewVal); len(errs) > 0 { + if errs := objchange.AssertPlanValid(schema, unmarkedPriorVal, unmarkedConfigVal, 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, @@ -1085,7 +1091,7 @@ func (n *NodeAbstractResource) processIgnoreChanges(prior, config cty.Value) (ct return config, nil } - ignoreChanges := n.Config.Managed.IgnoreChanges + ignoreChanges := traversalsToPaths(n.Config.Managed.IgnoreChanges) ignoreAll := n.Config.Managed.IgnoreAllChanges if len(ignoreChanges) == 0 && !ignoreAll { @@ -1100,14 +1106,16 @@ func (n *NodeAbstractResource) processIgnoreChanges(prior, config cty.Value) (ct return config, nil } - return processIgnoreChangesIndividual(prior, config, ignoreChanges) + ret, diags := processIgnoreChangesIndividual(prior, config, ignoreChanges) + + return ret, diags } -func processIgnoreChangesIndividual(prior, config cty.Value, ignoreChanges []hcl.Traversal) (cty.Value, tfdiags.Diagnostics) { - // When we walk below we will be using cty.Path values for comparison, so - // we'll convert our traversals here so we can compare more easily. - ignoreChangesPath := make([]cty.Path, len(ignoreChanges)) - for i, traversal := range ignoreChanges { +// Convert the hcl.Traversal values we get form the configuration to the +// cty.Path values we need to operate on the cty.Values +func traversalsToPaths(traversals []hcl.Traversal) []cty.Path { + paths := make([]cty.Path, len(traversals)) + for i, traversal := range traversals { path := make(cty.Path, len(traversal)) for si, step := range traversal { switch ts := step.(type) { @@ -1127,9 +1135,12 @@ func processIgnoreChangesIndividual(prior, config cty.Value, ignoreChanges []hcl panic(fmt.Sprintf("unsupported traversal step %#v", step)) } } - ignoreChangesPath[i] = path + paths[i] = path } + return paths +} +func processIgnoreChangesIndividual(prior, config cty.Value, ignoreChangesPath []cty.Path) (cty.Value, tfdiags.Diagnostics) { type ignoreChange struct { // Path is the full path, minus any trailing map index path cty.Path @@ -1175,8 +1186,7 @@ func processIgnoreChangesIndividual(prior, config cty.Value, ignoreChanges []hcl // here even if our ignored key doesn't change. That is OK since it // won't cause any changes in the transformation, but allows us to skip // breaking up the maps and checking for key existence here too. - eq := p.Equals(c) - if !eq.IsKnown() || eq.False() { + if !p.RawEquals(c) { // there a change to ignore at this path, store the prior value ignoredValues = append(ignoredValues, ignoreChange{icPath, p, key}) } @@ -1205,6 +1215,10 @@ func processIgnoreChangesIndividual(prior, config cty.Value, ignoreChanges []hcl return v, nil } + // The map values will remain as cty values, so we only need to store + // the marks from the outer map itself + v, vMarks := v.Unmark() + // The configMap is the current configuration value, which we will // mutate based on the ignored paths and the prior map value. var configMap map[string]cty.Value @@ -1234,11 +1248,17 @@ func processIgnoreChangesIndividual(prior, config cty.Value, ignoreChanges []hcl // return null for a key with a null value and for a non-existent // key. var priorMap map[string]cty.Value + + // We need to drop the marks from the ignored map for handling. We + // don't need to store these, as we now know the ignored value is + // only within the map, not the map itself. + ignoredVal, _ := ignored.value.Unmark() + switch { - case ignored.value.IsNull() || ignored.value.LengthInt() == 0: + case ignored.value.IsNull() || ignoredVal.LengthInt() == 0: priorMap = map[string]cty.Value{} default: - priorMap = ignored.value.AsValueMap() + priorMap = ignoredVal.AsValueMap() } key := ignored.key.AsString() @@ -1254,11 +1274,18 @@ func processIgnoreChangesIndividual(prior, config cty.Value, ignoreChanges []hcl } } + var newVal cty.Value if len(configMap) == 0 { - return cty.MapValEmpty(v.Type().ElementType()), nil + newVal = cty.MapValEmpty(v.Type().ElementType()) + } else { + newVal = cty.MapVal(configMap) } - return cty.MapVal(configMap), nil + if len(vMarks) > 0 { + newVal = v.WithMarks(vMarks) + } + + return newVal, nil }) return ret, nil } diff --git a/internal/terraform/reduce_plan_test.go b/internal/terraform/reduce_plan_test.go index 30d819aad..2d134d2c2 100644 --- a/internal/terraform/reduce_plan_test.go +++ b/internal/terraform/reduce_plan_test.go @@ -382,7 +382,7 @@ func TestProcessIgnoreChangesIndividual(t *testing.T) { ignore[i] = trav } - ret, diags := processIgnoreChangesIndividual(test.Old, test.New, ignore) + ret, diags := processIgnoreChangesIndividual(test.Old, test.New, traversalsToPaths(ignore)) if diags.HasErrors() { t.Fatal(diags.Err()) }