don't compare plan marks for missing values

If a resource has a change in marks from the prior state, we need to
notify the user that an update is going to be necessary to at least
store that new value in the state. If the provider however returns the
prior state value in lieu of a new config value, we need to be sure to
filter any new marks for comparison as well. The comparison of the prior
marks and new marks must take into account whether those new marks could
even be applied, because if the value is unchanged the new marks may be
completely irrelevant.
This commit is contained in:
James Bardin 2023-03-20 16:23:52 -04:00
parent a4e92f3fca
commit 81b74cdb22
3 changed files with 88 additions and 2 deletions

View File

@ -4035,3 +4035,66 @@ func TestContext2Plan_dataSourceReadPlanError(t *testing.T) {
t.Fatalf("failed to round-trip through planfile: %s", err) t.Fatalf("failed to round-trip through planfile: %s", err)
} }
} }
func TestContext2Plan_ignoredMarkedValue(t *testing.T) {
m := testModuleInline(t, map[string]string{
"main.tf": `
resource "test_object" "a" {
map = {
prior = "value"
new = sensitive("ignored")
}
}
`})
testProvider := &MockProvider{
GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{
ResourceTypes: map[string]providers.Schema{
"test_object": providers.Schema{
Block: &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"map": {
Type: cty.Map(cty.String),
Optional: true,
},
},
},
},
},
},
}
testProvider.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) {
// We're going to ignore any changes here and return the prior state.
resp.PlannedState = req.PriorState
return resp
}
state := states.NewState()
root := state.RootModule()
root.SetResourceInstanceCurrent(
mustResourceInstanceAddr("test_object.a").Resource,
&states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{"map":{"prior":"value"}}`),
Dependencies: []addrs.ConfigResource{},
},
mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`),
)
ctx := testContext2(t, &ContextOpts{
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("test"): testProviderFuncFixed(testProvider),
},
})
// plan+apply to create the initial state
opts := SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))
plan, diags := ctx.Plan(m, state, opts)
assertNoErrors(t, diags)
for _, c := range plan.Changes.Resources {
if c.Action != plans.NoOp {
t.Errorf("unexpected %s change for %s", c.Action, c.Addr)
}
}
}

View File

@ -7,6 +7,22 @@ import (
"github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty"
) )
// filterMarks removes any PathValueMarks from marks which cannot be applied to
// the given value. When comparing existing marks to those from a map or other
// dynamic value, we may not have values at the same paths and need to strip
// out irrelevant marks.
func filterMarks(val cty.Value, marks []cty.PathValueMarks) []cty.PathValueMarks {
var res []cty.PathValueMarks
for _, mark := range marks {
// any error here just means the path cannot apply to this value, so we
// don't need this mark for comparison.
if _, err := mark.Path.Apply(val); err == nil {
res = append(res, mark)
}
}
return res
}
// marksEqual compares 2 unordered sets of PathValue marks for equality, with // marksEqual compares 2 unordered sets of PathValue marks for equality, with
// the comparison using the cty.PathValueMarks.Equal method. // the comparison using the cty.PathValueMarks.Equal method.
func marksEqual(a, b []cty.PathValueMarks) bool { func marksEqual(a, b []cty.PathValueMarks) bool {

View File

@ -1074,8 +1074,15 @@ func (n *NodeAbstractResourceInstance) plan(
} }
// If we plan to write or delete sensitive paths from state, // If we plan to write or delete sensitive paths from state,
// this is an Update action // this is an Update action.
if action == plans.NoOp && !marksEqual(unmarkedPaths, priorPaths) { //
// We need to filter out any marks which may not apply to the new planned
// value before comparison. The one case where a provider is allowed to
// return a different value from the configuration is when a config change
// is not functionally significant and the prior state can be returned. If a
// new mark was also discarded from that config change, it needs to be
// ignored here to prevent an errant update action.
if action == plans.NoOp && !marksEqual(filterMarks(plannedNewVal, unmarkedPaths), priorPaths) {
action = plans.Update action = plans.Update
} }