Plan modification design doc

[kmoe]

The first step towards #34.

[ewbankkit]

Suggested change

Rather than exposing Terraform plans to provider developers, `helper/schema` has as a first-class concept the _resource diff_. Providers can optionally define a `CustomizeDiff` method on the `Resource` struct, which resembles a CRUD method, except that instead of `ResourceData` the function is supplied a `*ResourceDiff`, and is called during several points in the resource lifecycle, and must therefore be "resilient to support all scenarios".

Rather than exposing Terraform plans to provider developers, `helper/schema` has as a first-class concept the _resource diff_. Providers can optionally define a `CustomizeDiff` method on the `Resource` struct, which resembles a CRUD method, except that instead of `ResourceData` the function is supplied a `ResourceDiff`, and is called during several points in the resource lifecycle, and must therefore be "resilient to support all scenarios".

[kmoe]

The signature is type CustomizeDiffFunc func(context.Context, *ResourceDiff, interface{}) error - any particular reason to omit the pointer here?

[bflad]

Overall this is looking really good -- I'm curious about a few points here, which happy to discuss offline:

• Are documentation hooks desired for this functionality now or in the future?
• Are there concerns with proliferating fields on attributes?
• Is it important to allow attribute-level plan modifications access to the provider?

[paddycarver]

Left some thoughts inline. I didn't see a convenient place to put it in, but I'm also interested in how this may impact the docs generator / language server and how we may be able to surface these behaviors to those tools, and how these possible paths forward interact with our capabilities there.

[kmoe]

Thanks, all. Yes, PlanModifier (whatever we call it) is the missing piece here! Revising and dealing with all comments.

[kmoe]

Hmm, actually I really like the typed validators in #65 but I didn't dare propose a StringAttributePlanModifier. Since it looks like our proposed solutions for plan customisation and validation are converging, I think it's worth considering offering the same kind of strong typing for both eventually.

[paddycarver]

I'm in agreement with your conclusions. I am still unhappy about the type assertions going on with attr.Values. I think it limits our composability of these blocks significantly.

What if we offer an attr.ValueAs helper that does the same reflection Get does (and maybe attr.ValueIsKnown / attr.ValueIsNull helpers that use the ToTerraformValue to compare to nil and tftypes.UnknownValue) that will allow us to use attr.Values without limiting the types of the values? Then we offer the same way to get as values that CRUD uses, for example.

[paddycarver]

A couple minor comments, but it seems solid and reasonable to me, and the comments are things it's reasonable enough to change later, I think, so this has my 👍 🚀

[bflad]

Overall very well done!

One thing I'm curious about is how the provider instance will be available to this functionality if necessary, but it is fine to leave that as an implementation detail if the design does not inhibit that possibility.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.