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

Fit our state-setting model to the protocol #4328

Open
paddycarver opened this issue Aug 22, 2019 · 2 comments
Open

Fit our state-setting model to the protocol #4328

paddycarver opened this issue Aug 22, 2019 · 2 comments

Comments

@paddycarver
Copy link
Contributor

In learning more about the Terraform protocol, it came up that when a value exists in a Terraform config, only two possible values can be set in that field's state:

  • the exact, lexicographical representation that is entered into state
  • the exact, lexicographical representation that is entered in the config

This applies even when a value is computed, and failure to adhere to this may break if the field changes and is interpolated into another resource, as the plan will not match what was applied. In our current SDK, we're mostly getting away with this. In the next SDK, we will not get away with this. So we should correct this for 3.0.0.

We mostly do this in areas where we use self_links or other values that have multiple, semantically equivalent forms. The proposed strategy we should be following is to do this canonicalization when setting the value in state. Basically, when we're about to update a value in state, check if it's semantically equivalent to the value in the config or in state, and not change it if it is. We should still DiffSuppress (to avoid two equivalent representations causing a spurious diff) but this should keep us in line with the Terraform protocol's expectations.

@slevenick
Copy link
Collaborator

This may cause issues around TypeSets during import that require specifying custom hash functions.

If we have a field that is either a self link or a name in a set, it is impossible to ensure it is set to the config value because we don't have access to the config during import. This means during import we will set the value to whatever comes from the API, causing a diff with the config. This could be fixed with a DiffSuppressFunc, but because we are in a TypeSet, the hashes will be different, still causing a diff. We can fix it with both a custom hash function and a DiffSuppressFunc

@rileykarson
Copy link
Collaborator

This fell out of scope of 3.0.0. Added to 4.0.0 to ensure it's tracked for that ~next year, although it's likely we can do it without a breaking change spread over normal releases.

@danawillow danawillow added size/xl and removed bug labels May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants