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

bug: Terraform Bridge does not reliably perform Terraform state upgrades #923

Closed
Tracked by #886
tmeckel opened this issue Mar 29, 2023 · 1 comment · Fixed by #1226
Closed
Tracked by #886

bug: Terraform Bridge does not reliably perform Terraform state upgrades #923

tmeckel opened this issue Mar 29, 2023 · 1 comment · Fixed by #1226
Assignees
Labels
area/tfbridge Issues in pkg/tfbridge, which provides interop between Pulumi and TF providers kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Milestone

Comments

@tmeckel
Copy link
Contributor

tmeckel commented Mar 29, 2023

@t0yv0 Found the issue. Inside the Pulumi stack the resource id is lower case on the webHooks element:

/subscriptions/e5116c25-7bc9-4e96-9c3e-88257f68db7c/resourceGroups/RG-WKB-Test/providers/Microsoft.ContainerRegistry/registries/acr4189bf944305c36f/webhooks/b1ec43f3ce50aab6

image

The parser inside the AzureRM Provider expects webHooks:

image

The team introduced the change with:

hashicorp/terraform-provider-azurerm@821ea0a
hashicorp/terraform-provider-azurerm#19507

No issue when using Terraform, because the team added a (silent) state migration.

image

Pulumi isn't aware of state migrations because it handles the state on it's own. But in such cases this is a real problem, because in the current case, I've to update the Pulumi state manually and correct the casing from webhooks to webHooks.

Originally posted by @tmeckel in #720 (comment)

Affected versions: Upgrade from Azure Classic 5.16.0 to 5.38.0

Steps to reproduce:

  1. Deploy Azure Container Registry with a WebHook using 5.16.0
  2. Upgrade to Version 5.38.0
  3. Try to change the infrastructure (e.g. add a Storage Account). pulumi pre -r will show an error about a the missing webHooks element.
@t0yv0 t0yv0 added the needs-triage Needs attention from the triage team label Mar 29, 2023
@t0yv0
Copy link
Member

t0yv0 commented Mar 29, 2023

Thank you for working out another possible root cause of problem here!

@t0yv0 t0yv0 removed the needs-triage Needs attention from the triage team label Mar 29, 2023
@aq17 aq17 added the kind/bug Some behavior is incorrect or out of spec label Mar 29, 2023
@t0yv0 t0yv0 added the area/tfbridge Issues in pkg/tfbridge, which provides interop between Pulumi and TF providers label Apr 5, 2023
@t0yv0 t0yv0 mentioned this issue Apr 6, 2023
8 tasks
@t0yv0 t0yv0 self-assigned this Jun 20, 2023
@t0yv0 t0yv0 added this to the 0.90 milestone Jun 20, 2023
t0yv0 added a commit that referenced this issue Jun 20, 2023
Before this change, Pulumi would correctly run migrations on the resource state if those are specified in the upstream
provider. However, in the rare situation where the migrations would intentionally modify the ID field, the bridged
provider would ignore this modification. After the change, migrations of the ID field are respected.

Fixes #923.
t0yv0 added a commit that referenced this issue Jun 21, 2023
* Allow state upgraders to modify ID

Before this change, Pulumi would correctly run migrations on the resource state if those are specified in the upstream
provider. However, in the rare situation where the migrations would intentionally modify the ID field, the bridged
provider would ignore this modification. After the change, migrations of the ID field are respected.

Fixes #923.
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tfbridge Issues in pkg/tfbridge, which provides interop between Pulumi and TF providers kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants