search

pulumi / pulumi-terraform-bridge

A library allowing providers built with the Terraform Plugin SDK to be bridged into Pulumi.
Apache License 2.0
183 stars 41 forks source link

Fix bridge not running state upgrades from 0 -> 1 #2081

Closed VenelinMartinov closed 3 hours ago

VenelinMartinov commented 2 weeks ago

fixes https://github.com/pulumi/pulumi-terraform-bridge/issues/2039

The bridge assumes that a missing schema_version in the meta state means "current version". That is wrong as we only miss the schema_version if it is 0 - we write all non-zero versions to the state.

Any resources created under a v0 TF schema version will not save the the v0 in the state. This means that if this resource then gets a TF state upgrade, we will assume we are already on the new version and NOT run the state upgrade as we should.

This PR changes the assumption to mean missing schema_version means "v0". Note that TestUpgradeInputsStringBasicNonZeroVersionSame passed before, which shows that any existing resources which had state upgrades defined should have the correct state version in their meta property.

It also adds tests around this both for PRC and non-PRC.

The feature is flagged behind a provider flag, supplied in ProviderInfo - the feature flag mechanism is adapted from the sdkv2 feature flag mechanism.

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 63.63636% with 48 lines in your changes missing coverage. Please review.

Project coverage is 60.62%. Comparing base (0b6ff1b) to head (906529a). Report is 5 commits behind head on master.

Files Patch % Lines
pkg/tfbridge/provider.go 45.65% 19 Missing and 6 partials :warning:
pkg/tests/cross-tests/upgrade_state_check.go 72.46% 17 Missing and 2 partials :warning:
pkg/tfbridge/schema.go 66.66% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2081 +/- ## ========================================== - Coverage 60.70% 60.62% -0.08% ========================================== Files 350 350 Lines 45735 45898 +163 ========================================== + Hits 27763 27827 +64 - Misses 16454 16531 +77 - Partials 1518 1540 +22 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

VenelinMartinov commented 2 weeks ago

My only concern here is migrations

I'm not sure I understand what you mean here - do you mean moving from previous bridge to this one?

VenelinMartinov commented 1 day ago

I updated the code here to guard the change behind a provider flag, LMK what you think.

As for rollout we can do a light version of the PRC plan:

  1. Cloudflare + GCP
  2. AWS
  3. Everything else
iwahbe commented 23 hours ago

I updated the code here to guard the change behind a provider flag, LMK what you think.

As for rollout we can do a light version of the PRC plan:

  1. Cloudflare + GCP
  2. AWS
  3. Everything else

The rollout plan sounds good, I'm assuming ~2 weeks per stage. As part of merging, please create an issue for each stage of the plan in this repo to track the rollout of the feature and the removal of the flag post-rollout.

VenelinMartinov commented 3 hours ago

Opened https://github.com/pulumi/pulumi-terraform-bridge/issues/2133 for the rollout