Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs/design: Upgrade Resource State #228

Merged
merged 7 commits into from Apr 20, 2022

Conversation

bflad
Copy link
Member

@bflad bflad commented Nov 29, 2021

Reference: #42

@bflad bflad added design-doc Issues or pull requests about a design document, capturing design ideas and trade-offs. sdkv2-parity Issues tracking feature parity with terraform-plugin-sdk v2 and PRs working towards it. labels Nov 29, 2021
@bflad
Copy link
Member Author

bflad commented Nov 29, 2021

Just to jot down some potential proposals so they are out of my mental state/notes:

  • Implementing something similar to SDKv2 with the top level schema type including version and state upgraders fields
  • Overloading the underlying schema types (e.g. Attribute) to include state upgrade fields such as "Removed" or "UpgradeState"
  • (Maybe cursed idea) Overloading the tfsdk struct tag to include versioning information

@bflad bflad added this to the v0.6.0 milestone Jan 11, 2022
@bflad bflad modified the milestones: v0.6.0, v0.7.0 Mar 10, 2022
@hashicorp-cla
Copy link

hashicorp-cla commented Mar 12, 2022

CLA assistant check
All committers have signed the CLA.

@bflad bflad force-pushed the bflad-docs-design-UpgradeResourceState branch from 40d5e01 to 49dcbe1 Compare April 13, 2022 18:29
@bflad bflad marked this pull request as ready for review April 14, 2022 15:25
@bflad bflad requested a review from a team as a code owner April 14, 2022 15:25
@bflad bflad removed their assignment Apr 14, 2022
Comment on lines +325 to +344
The framework could allow provider developers to optionally extend their `ResourceType` with a new `UpgradeState` method.

For example in the framework:

```go
// New type
type ResourceTypeWithUpgradeState interface {
UpgradeState(context.Context, /*...*/) /*...*/
}
```

Which would result in the following optional provider implementation:

```go
func (rt ExampleResourceType) UpgradeState(ctx context.Context, /*...*/) /*...*/ {
/* ... */
}
```

However, there is an immediate drawback that there would be no access to the provider client during upgrade state operations. This is considered a non-starter due to the goals of this functionality.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although the headings are different, this text is identical to the following section - New ResourceWithUpgradeState Interface.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey sorry, I'm not sure what you mean here. 😅 ResourceType and Resource are very different implementation details. Providers get the opportunity to inject their clients into Resource, but not ResourceType.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad 😄

Co-authored-by: Kit Ewbank <Kit_Ewbank@hotmail.com>
bflad added a commit that referenced this pull request Apr 18, 2022
Reference: #42
Reference: #228

Support provider defined `UpgradeResourceState` RPC handling, by introducing an optional `ResourceWithUpgradeState` interface type, with an `UpgradeState` method. Each underlying state version upgrade implementation is expected to consume the prior state, perform any necessary data manipulations, then respond with the upgraded state.

This framework implementation differs from the terraform-plugin-sdk implementation:

- State upgraders are specified via a mapping, rather than a slice with underlying version field. This should prevent certain classes of coding issues.
- State upgraders must be wholly contained from the prior state version to the current schema version. The framework does not loop through each successive version because attempting to recreate the `tfprotov6.RawState` for each intermediate version request would be very problematic. For example, terraform-plugin-go does not implement functionality for marshalling a `RawState`. Provider developers can use their own coding techniques to reduce code duplications when multiple versions need the same logic.
- Specifying the full prior schema is now an optional implementation detail. Working with the lower level data types is more challenging, however this has been a repeated feature request.

There are some quirks and potential future enhancements to the framework `UpgradeResourceState` handling:

- Past and current versions Terraform CLI will call `UpgradeResourceState` even if the state version matches the current schema version. This implementation keeps the framework's prior logic to roundtrip the existing state into the upgraded state. It may be possible to stop this Terraform CLI behavior with protocol version 6, although the logic would need to remain for backwards compatibility.
- It may be possible to help provider developers simplify logic by attempting to automatically populate compatible parts of the upgraded state from the prior state. This can potentially be done at a later time.
@bflad
Copy link
Member Author

bflad commented Apr 18, 2022

Proof of concept implementation (and testing examples) for current recommendation: #292

This by no means is meant to represent that other proposals are off the table or the recommendation cannot be changed, however it hopefully gives some additional confidence in the potential implementation.

bflad added a commit that referenced this pull request Apr 19, 2022
Reference: #42
Reference: #228

Support provider defined `UpgradeResourceState` RPC handling, by introducing an optional `ResourceWithUpgradeState` interface type, with an `UpgradeState` method. Each underlying state version upgrade implementation is expected to consume the prior state, perform any necessary data manipulations, then respond with the upgraded state.

This framework implementation differs from the terraform-plugin-sdk implementation:

- State upgraders are specified via a mapping, rather than a slice with underlying version field. This should prevent certain classes of coding issues.
- State upgraders must be wholly contained from the prior state version to the current schema version. The framework does not loop through each successive version because attempting to recreate the `tfprotov6.RawState` for each intermediate version request would be very problematic. For example, terraform-plugin-go does not implement functionality for marshalling a `RawState`. Provider developers can use their own coding techniques to reduce code duplications when multiple versions need the same logic.
- Specifying the full prior schema is now an optional implementation detail. Working with the lower level data types is more challenging, however this has been a repeated feature request.

There are some quirks and potential future enhancements to the framework `UpgradeResourceState` handling:

- Past and current versions Terraform CLI will call `UpgradeResourceState` even if the state version matches the current schema version. This implementation keeps the framework's prior logic to roundtrip the existing state into the upgraded state. It may be possible to stop this Terraform CLI behavior with protocol version 6, although the logic would need to remain for backwards compatibility.
- It may be possible to help provider developers simplify logic by attempting to automatically populate compatible parts of the upgraded state from the prior state. This can potentially be done at a later time.
Copy link
Contributor

@bookshelfdave bookshelfdave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a good read, thanks for describing pros and cons for each proposal!

docs/design/upgrade-resource-state.md Outdated Show resolved Hide resolved
Co-authored-by: Dave Parfitt <58244+bookshelfdave@users.noreply.github.com>
@bflad
Copy link
Member Author

bflad commented Apr 20, 2022

If anyone has further feedback, please reach out to me directly. We can also consider different approaches before we reach 1.0.

@bflad bflad merged commit 180d63e into main Apr 20, 2022
@bflad bflad deleted the bflad-docs-design-UpgradeResourceState branch April 20, 2022 19:26
bflad added a commit that referenced this pull request Apr 21, 2022
Reference: #42
Reference: #228

Support provider defined `UpgradeResourceState` RPC handling, by introducing an optional `ResourceWithUpgradeState` interface type, with an `UpgradeState` method. Each underlying state version upgrade implementation is expected to consume the prior state, perform any necessary data manipulations, then respond with the upgraded state.

This framework implementation differs from the terraform-plugin-sdk implementation:

- State upgraders are specified via a mapping, rather than a slice with underlying version field. This should prevent certain classes of coding issues.
- State upgraders must be wholly contained from the prior state version to the current schema version. The framework does not loop through each successive version because attempting to recreate the `tfprotov6.RawState` for each intermediate version request would be very problematic. For example, terraform-plugin-go does not implement functionality for marshalling a `RawState`. Provider developers can use their own coding techniques to reduce code duplications when multiple versions need the same logic.
- Specifying the full prior schema is now an optional implementation detail. Working with the lower level data types is more challenging, however this has been a repeated feature request.

There are some quirks and potential future enhancements to the framework `UpgradeResourceState` handling:

- Past and current versions Terraform CLI will call `UpgradeResourceState` even if the state version matches the current schema version. This implementation keeps the framework's prior logic to roundtrip the existing state into the upgraded state. It may be possible to stop this Terraform CLI behavior with protocol version 6, although the logic would need to remain for backwards compatibility.
- It may be possible to help provider developers simplify logic by attempting to automatically populate compatible parts of the upgraded state from the prior state. This can potentially be done at a later time.
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
design-doc Issues or pull requests about a design document, capturing design ideas and trade-offs. sdkv2-parity Issues tracking feature parity with terraform-plugin-sdk v2 and PRs working towards it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants