Skip to content

Commit

Permalink
plans/objchange: Providers must honor their unknown value refinements
Browse files Browse the repository at this point in the history
If the original value was unknown but its range was refined then the
provider must return a value that is within the refined range, because
otherwise downstream planning decisions could be invalidated.

This relies on cty's definition of whether a value is in a refined range,
which has pretty good coverage for the "false" case and so should give a
pretty good signal, but it'll probably improve over time and so providers
must not rely on any loopholes in the current implementation and must
keep their promises even if Terraform can't currently check them.
  • Loading branch information
apparentlymart committed May 22, 2023
1 parent 957e2e6 commit 2159365
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 2 deletions.
9 changes: 7 additions & 2 deletions internal/plans/objchange/compatible.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu
var errs []error
var atRoot string
if len(path) == 0 {
atRoot = "Root resource "
atRoot = "Root object "
}

if planned.IsNull() && !actual.IsNull() {
Expand Down Expand Up @@ -216,7 +216,12 @@ func assertValueCompatible(planned, actual cty.Value, path cty.Path) []error {

if !planned.IsKnown() {
// We didn't know what were going to end up with during plan, so
// anything goes during apply.
// the final value needs only to match the type and refinements of
// the unknown value placeholder.
plannedRng := planned.Range()
if ok := plannedRng.Includes(actual); ok.IsKnown() && ok.False() {
errs = append(errs, path.NewErrorf("final value %#v does not conform to planning placeholder %#v", actual, planned))
}
return errs
}

Expand Down
59 changes: 59 additions & 0 deletions internal/plans/objchange/compatible_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,65 @@ func TestAssertObjectCompatible(t *testing.T) {
`.name: was cty.StringVal("wotsit"), but now cty.StringVal("thingy")`,
},
},
{
&configschema.Block{
Attributes: map[string]*configschema.Attribute{
"name": {
Type: cty.String,
Required: true,
},
},
},
cty.ObjectVal(map[string]cty.Value{
"name": cty.UnknownVal(cty.String),
}),
cty.ObjectVal(map[string]cty.Value{
"name": cty.Zero,
}),
[]string{
`.name: wrong final value type: string required`,
},
},
{
&configschema.Block{
Attributes: map[string]*configschema.Attribute{
"name": {
Type: cty.String,
Required: true,
},
},
},
cty.ObjectVal(map[string]cty.Value{
"name": cty.UnknownVal(cty.String).RefineNotNull(),
}),
cty.ObjectVal(map[string]cty.Value{
"name": cty.NullVal(cty.String),
}),
[]string{
`.name: final value cty.NullVal(cty.String) does not conform to planning placeholder cty.UnknownVal(cty.String).RefineNotNull()`,
},
},
{
&configschema.Block{
Attributes: map[string]*configschema.Attribute{
"name": {
Type: cty.String,
Required: true,
},
},
},
cty.ObjectVal(map[string]cty.Value{
"name": cty.UnknownVal(cty.String).Refine().
StringPrefix("boop:").
NewValue(),
}),
cty.ObjectVal(map[string]cty.Value{
"name": cty.StringVal("thingy"),
}),
[]string{
`.name: final value cty.StringVal("thingy") does not conform to planning placeholder cty.UnknownVal(cty.String).Refine().StringPrefixFull("boop:").NewValue()`,
},
},
{
&configschema.Block{
Attributes: map[string]*configschema.Attribute{
Expand Down

0 comments on commit 2159365

Please sign in to comment.