Skip to content

Commit

Permalink
fix panic when updating a sensitive field that requires replacement
Browse files Browse the repository at this point in the history
  • Loading branch information
liamcervante committed Jun 21, 2024
1 parent 71d14e7 commit e2d7e34
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 5 deletions.
60 changes: 60 additions & 0 deletions internal/terraform/context_plan2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5723,3 +5723,63 @@ output "value" {
_, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables)))
assertNoErrors(t, diags)
}

func TestContext2Plan_sensitiveRequiredReplace(t *testing.T) {
m := testModuleInline(t, map[string]string{
"main.tf": `
resource "test_object" "obj" {
value = "changed"
}
`,
})
p := new(testing_provider.MockProvider)
p.GetProviderSchemaResponse = &providers.GetProviderSchemaResponse{
ResourceTypes: map[string]providers.Schema{
"test_object": {
Block: &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"value": {
Type: cty.String,
Required: true,
Sensitive: true,
},
},
},
},
},
}
p.PlanResourceChangeResponse = &providers.PlanResourceChangeResponse{
PlannedState: cty.ObjectVal(map[string]cty.Value{
"value": cty.StringVal("changed"),
}),
RequiresReplace: []cty.Path{
cty.GetAttrPath("value"),
},
}

ctx := testContext2(t, &ContextOpts{
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
},
})

state := states.BuildState(func(s *states.SyncState) {
s.SetResourceInstanceCurrent(
mustResourceInstanceAddr("test_object.obj"),
&states.ResourceInstanceObjectSrc{
AttrsJSON: mustParseJson(map[string]any{
"value": "original",
}),
AttrSensitivePaths: []cty.Path{
cty.GetAttrPath("value"),
},
Status: states.ObjectReady,
},
mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`))
})

_, diags := ctx.Plan(m, state, nil)
if len(diags) > 0 {
t.Errorf("unexpected diags\n%s", diags)
}
}
12 changes: 7 additions & 5 deletions internal/terraform/node_resource_abstract_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -1068,7 +1068,7 @@ func (n *NodeAbstractResourceInstance) plan(
plannedNewVal = marks.MarkPaths(plannedNewVal, marks.Sensitive, sensitivePaths)
}

reqRep, reqRepDiags := getRequiredReplaces(priorVal, plannedNewVal, resp.RequiresReplace, n.ResolvedProvider.Provider, n.Addr)
reqRep, reqRepDiags := getRequiredReplaces(unmarkedPriorVal, unmarkedPlannedNewVal, resp.RequiresReplace, n.ResolvedProvider.Provider, n.Addr)
diags = diags.Append(reqRepDiags)
if diags.HasErrors() {
return nil, nil, deferred, keyData, diags
Expand Down Expand Up @@ -2804,6 +2804,11 @@ func getAction(addr addrs.AbsResourceInstance, priorVal, plannedNewVal cty.Value
// actually changed -- particularly after we may have undone some of the
// changes in processIgnoreChanges -- so now we'll filter that list to
// include only where changes are detected.
//
// Both the priorVal and plannedNewVal should be unmarked before calling this
// function. This function exposes nothing about the priorVal or plannedVal
// except for the paths that require replacement which can be deduced from the
// type with or without marks.
func getRequiredReplaces(priorVal, plannedNewVal cty.Value, requiredReplaces []cty.Path, providerAddr tfaddr.Provider, addr addrs.AbsResourceInstance) (cty.PathSet, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics

Expand Down Expand Up @@ -2848,10 +2853,7 @@ func getRequiredReplaces(priorVal, plannedNewVal cty.Value, requiredReplaces []c
plannedChangedVal = cty.NullVal(priorChangedVal.Type())
}

// Unmark for this value for the equality test. If only sensitivity has changed,
// this does not require an Update or Replace
unmarkedPlannedChangedVal, _ := plannedChangedVal.UnmarkDeep()
eqV := unmarkedPlannedChangedVal.Equals(priorChangedVal)
eqV := plannedChangedVal.Equals(priorChangedVal)
if !eqV.IsKnown() || eqV.False() {
reqRep.Add(path)
}
Expand Down

0 comments on commit e2d7e34

Please sign in to comment.