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

Do not upgrade forked terraform-plugin-sdk ahead of the bridge #243

Closed
t0yv0 opened this issue Jan 31, 2024 · 2 comments · Fixed by #248
Closed

Do not upgrade forked terraform-plugin-sdk ahead of the bridge #243

t0yv0 opened this issue Jan 31, 2024 · 2 comments · Fixed by #248
Assignees
Labels
kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Milestone

Comments

@t0yv0
Copy link
Member

t0yv0 commented Jan 31, 2024

What happened?

Per conversation in https://github.com/pulumi/upgrade-provider/pull/242/files it appears that maybe/possibly we're upgrading the fork of terraform-plugin-sdk to the latest from GitHub. That's may be a good option to have but it's not a good default. By default we should be trying to match whatever the bridge has when upgrading the bridge, because that is the version the bridge was tested with. Unfortunately go tooling does not make this trivial, but something along the lines of finding bridge sources and finding the go mod replace directive from those sources that the bridge was built with could do the trick. If helpful we can lift this to a text file in the bridge somehow.

Example

N/A

Output of pulumi about

N/A

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@t0yv0 t0yv0 added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Jan 31, 2024
@iwahbe iwahbe removed the needs-triage Needs attention from the triage team label Feb 1, 2024
@t0yv0
Copy link
Member Author

t0yv0 commented Feb 6, 2024

https://github.com/pulumi/pulumi-gcp/pull/1638/files#diff-0301265a2d4321ca487bbd7ba523f69633171cef680c0114e7ca8d2c34176b73R19 another one.. I was updating GCP to do a simple minor update and looks like the terraform-plugin-sdk is updating as part of it. That's a bit suspect.

@t0yv0
Copy link
Member Author

t0yv0 commented Feb 8, 2024

OK this is causing failure to pre-test bridge upgrade with a given sdk version

--target-bridge-version: 45cdb2507f387f34af187ca4021076549cbbf78e

pulumi/pulumi-github#559 and more

@t0yv0 t0yv0 closed this as completed in #248 Feb 8, 2024
t0yv0 added a commit that referenced this issue Feb 8, 2024
Fixes #243

After this change the terraform SDK v2 override follows the replace from
the selected bridge version exactly. I think this is what we want - this
is compensating for the inability of Go tooling to propagate the replace
to dependencies.

The change also starts to unconditionally insert this replace. This is
because on the latest bridge tfgen somehow depends on sdkv2 and even
providers such as pulumi-random that presumably do not need sdkv2 now
depend on it and need the replace accordingly, see
pulumi/pulumi-random#687 for example.
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Feb 8, 2024
@t0yv0 t0yv0 added this to the 0.100 milestone Feb 8, 2024
t0yv0 added a commit that referenced this issue Feb 9, 2024
Quick fix for a panic introduced in
#243

Would like to merge to prevent p1s, follow up with tests later.

Verified this works on pulumi-azuread.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

3 participants