Skip to content

Commit

Permalink
Fix planning. (#176)
Browse files Browse the repository at this point in the history
* Fix planning.

This fixes #175. Essentially, our behavior did exactly what we thought
it did, but didn't combine with Terraform in the ways we thought it
would. We need to set all computed attributes that are null in the
_config_ to unknown, not all computed attributes that are null in the
_plan_ to unknown, because once a computed attribute has a value in
state, it will not have a null plan again unless the provider modifies
the plan to null explicitly. This means once a computed attribute gets a
value, under the existing code, it can't be updated by the provider.

As part of this change, I moved when the null->unknown transform
happens. It used to happen at the very end, which gave provider
developers no opportunity to override the auto-unknown values with
values the provider may know at plan time, such as a provider-level
default or the combination of other fields, etc. By doing this first,
the provider has the ability to understand that the value is unknown and
replace it with a known value.

I also fixed a bug that would return an error when trying to use
Schema.AttributeAtPath with a path that pointed to an element inside a
list, map, or set nested attribute.

I also fixed plan modification to still run resource-level plan
modification even when the plan is null (i.e., the resource is being
deleted) so that diagnostics can be returned. This is useful, for
example, when a resource _can't_ be deleted on the API, but the provider
is going to just remove it from Terraform's state. This allows provider
developers to surface a warning at plan time to practitioners about this
limitation, letting them know _before_ they apply the change that that
is what's going to happen.

Finally, some tests had nonsensical inputs to them that Terraform could
never produce--a null config that still had planned values. I updated
those tests to have more realistic values by removing the plan and
removing the RequiresReplace set in the response, as a deleted resource
needing to be replaced doesn't super make sense.

* Revert t.Log() for diagnostics debugging for now

It will likely get documented and consistently applied later either as-is or using a custom go-cmp formatter. See also #135.

* Revert Schema.AttributeAtPath implicit parent Attribute handling and add unit testing

* populate missing config values in tests

* test nested unknown value setting

* map AttributeType -> ElementType

Co-authored-by: Brian Flad <bflad417@gmail.com>
Co-authored-by: Katy Moe <katy@katy.moe>
  • Loading branch information
3 people authored Sep 28, 2021
1 parent 766037f commit cf6f240
Show file tree
Hide file tree
Showing 7 changed files with 1,186 additions and 75 deletions.
4 changes: 4 additions & 0 deletions tfsdk/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ func (s Schema) AttributeAtPath(path *tftypes.AttributePath) (Attribute, error)
return Attribute{}, ErrPathInsideAtomicAttribute
}

if _, ok := res.(nestedAttributes); ok {
return Attribute{}, ErrPathInsideAtomicAttribute
}

a, ok := res.(Attribute)
if !ok {
return Attribute{}, fmt.Errorf("got unexpected type %T", res)
Expand Down
Loading

0 comments on commit cf6f240

Please sign in to comment.