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

Provider is lacking state upgraders #136

Open
lukas-w opened this issue Jan 6, 2025 · 2 comments
Open

Provider is lacking state upgraders #136

lukas-w opened this issue Jan 6, 2025 · 2 comments

Comments

@lukas-w
Copy link

lukas-w commented Jan 6, 2025

All of this provider's resource & data source schemas specify a non-default SchemaVersion, but no StateUpgraders, making it impossible for users to upgrade even to the next patch version in some cases if using the affected resources.

For example, after upgrading to 0.6.2 in an existing deployment, attempting to replace a neon_branch resource fails with the following output due to the schema version having changed via acbcd77:

Error writing state file: schema version 7 for neon_branch.staging in state does not match version 8 from the provider
Error: Terraform exited with code 1.
Error: Process completed with exit code 1.

Although not explicitly required AFAIK, Terraform providers are widely expected to follow semantic versioning and this project appears to intend to do so as well, since with releases containing documented breaking changes, the minor pre-release version was incremented. If users of old minor releases should be able to upgrade to the newest patch version without risk as can be expected with semantic versioning, it appears that some (or most?) minor releases would need a new patch release to add the required state upgraders. Old release notes on GitHub should have a warning with instructions to skip the faulty releases where applicable.

Even with an update explicitly containing breaking changes however, migrating to it should be made possible. Currently if using an affected resource, the only way to upgrade appears to be to destroy the resource first and re-deploy after the upgrade. This is especially unacceptable in a provider managing database resources potentially containing live production data.

Some details that should be kept in mind when implementing a fix:

  • Prior to v0.3.0, all resources shared a global version (7 in the latest release before that) due to a misunderstanding of the purpose of SchemaVersion field. This was fixed by setting the versions to 7 or 8 individually via 36ac73d. Prior to the fix, some SchemaVersion increments may have been unneeded, making their upgrade function a no-op.
  • Apparently to fix the same misunderstanding, all data source schemas' versions were decremented from 8 down to 1 in f7b102c as released in v0.5.0. This breaks Terraform SDK's requirement of incrementing the version by one for state migrations. I don't think that the SDK enforces this anywhere, but now that that requirement is broken, newer releases may encounter versions with a higher SchemaVersion in the wild. Since all data sources were introduced with version 8 from the start, there's no ambiguity yet. Special care must be taken to ensure that version 8 is not re-used in future releases.
  • As the SDK's concept of schema versioning was and may still be misunderstood by the author, one should pay careful attention to any breaking schema changes that were made without incrementing the version, especially before 36ac73d & f7b102c. If there are any, they may need an additional version increment with more complex logic in the state upgrader to infer the used schema from the data structure.
@kislerdm
Copy link
Owner

Thanks for your concern. My only recommendation would be to use the provider from v0.6.3 up. Moving forward though, I'd appreciate constructive feedback and PRs as opposed to insinuations about one's understanding of SDK :)

@lukas-w
Copy link
Author

lukas-w commented Jan 30, 2025

I'm sorry that this came across as insinuations, it was not indented to. I suppose my bulleted list with past commits that introduced breaking changes can sound like unnecessarily criticism. Please be assured that this was only meant constructively to aid any attempts to fix this, as the past errors and non-incremental version changes - even though they were fixed - make it a non-trivial task. Since your most recent release still included a version increase without migration and I've found no existing bug report, I assumed that you were likely not yet aware of this issue. So I wanted to include a heads-up to anyone attempting to fix this that more breaking changes could have been made since 0.6.2 and since the issue's reporting, because I didn't have the time to review everything new on master and didn't know when you would read this issue. Hence the admittedly insensitive "may still be misunderstood by the author" note - apologies. I hope this explanation helps to see my words in a more favourable light :)

Or is there something else that you feel was badly phrased or not constructive?

My only recommendation would be to use the provider from v0.6.3 up.

I feel like this misses the point of the problem. Upgrading to v0.6.3 was exactly what broke my Terraform workspace and triggered me to investigate and open this issue (I gave v0.6.2 as example because that's the version that introduced the breaking change). I was lucky enough that my affected deployment was a testing environment that I could just destroy and re-deploy. I'm not sure how solvable the problem is when there's production data at stake. Therefore, shouldn't the recommendation be not to upgrade until the issue is addressed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants