Skip to content

Conversation

@paddycarver
Copy link

The tfprotov5.RawState type exposes state through a []byte containing
JSON when upgrading resource state with the UpgradeResourceState RPC. We
had not surfaced any JSON parsing or decoding in the exported API, so
providers would need to parse the JSON themselves.

To solve this, we use the same Unmarshal interface we use for parsing
JSON from DynamicValues, letting users receive a tftypes.Value from a
RawState without needing to worry about encoding... mostly.

There is one hitch. The Flatmap syntax doesn't have a canonical
translation (yet? As far as I know) to Terraform's type system. So we
sidestep that by returning an error, for the moment. In the future, we
could conceivably institute a mapping, though I'm not convinced it's a
good idea to, given the number of assumptions that would need to be
made. It is a larger undertaking to do that in a predictable, reliable
way. Provider developers should provide their own mapping of Flatmap to
tftypes.Value.

The Flatmap property should only ever be populated in one specific
scenario, as best I can tell:

  1. The user was using a version of Terraform below 0.12, and that is
    the last version of Terraform to write the state file.
  2. The user upgrades to 0.12+ and obtains a new release of the provider
  3. The new release of the provider is built on plugin-go

At that point, the plugin-go-based provider would be the first version
of the provider to write the state after 0.12, meaning it would be its
responsibility to upgrade the state between the flatmap syntax and
Terraform's type system.

It is unknown how likely that scenario is, but I want to document it
here to avoid any confusion.

The tfprotov5.RawState type exposes state through a []byte containing
JSON when upgrading resource state with the UpgradeResourceState RPC. We
had not surfaced any JSON parsing or decoding in the exported API, so
providers would need to parse the JSON themselves.

To solve this, we use the same Unmarshal interface we use for parsing
JSON from DynamicValues, letting users receive a tftypes.Value from a
RawState without needing to worry about encoding... mostly.

There is one hitch. The Flatmap syntax doesn't have a canonical
translation (yet? As far as I know) to Terraform's type system. So we
sidestep that by returning an error, for the moment. In the future, we
could conceivably institute a mapping, though I'm not convinced it's a
good idea to, given the number of assumptions that would need to be
made. It is a larger undertaking to do that in a predictable, reliable
way. Provider developers should provide their own mapping of Flatmap to
tftypes.Value.

The Flatmap property should only ever be populated in one specific
scenario, as best I can tell:

1. The user _was_ using a version of Terraform below 0.12, and that is
the last version of Terraform to write the state file.
2. The user upgrades to 0.12+ and obtains a new release of the provider
3. The new release of the provider is built on plugin-go

At that point, the plugin-go-based provider would be the first version
of the provider to write the state after 0.12, meaning it would be its
responsibility to upgrade the state between the flatmap syntax and
Terraform's type system.

It is unknown how likely that scenario is, but I want to document it
here to avoid any confusion.
@paddycarver paddycarver added the enhancement New feature or request label Nov 10, 2020
@paddycarver paddycarver requested a review from a team November 10, 2020 10:28
@alexsomesan
Copy link
Member

Excellent !
Excellent

Copy link
Contributor

@paultyng paultyng left a comment

Choose a reason for hiding this comment

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

LGTM, although not sure if a well known error is useful for just one case here and not the other.

@paddycarver
Copy link
Author

We can address that as part of #4, when we determine what our well-known error philosophy for this module is.

@paddycarver paddycarver merged commit 4698f8a into main Nov 16, 2020
@paddycarver paddycarver deleted the paddy_unmarshal_rawstate branch November 16, 2020 22:08
@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 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants