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.

Sign up for GitHub

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

Jump to bottom

PF update proposed new to pull recent upstream changes #2172

Closed
VenelinMartinov opened this issue Jul 10, 2024 · 2 comments · Fixed by #2188
Closed

PF update proposed new to pull recent upstream changes #2172

VenelinMartinov opened this issue Jul 10, 2024 · 2 comments · Fixed by #2188
Assignees
@t0yv0
Labels
kind/engineering Work that is not visible to an external user resolution/fixed This issue was fixed
Milestone
0.108

Comments

@VenelinMartinov
Copy link
Contributor

What happened?

We have an adaptation of upstream's objchange in PF: https://github.com/opentofu/opentofu/tree/main/internal/plans/objchange

pulumi-terraform-bridge/pf/internal/pfutils/proposed_new.go

Lines 26 to 27 in 18db7c5

// Computes the ProposedNewState from priorState and config.

This hasn't been updated in a while but upstream has had a few changes - we might need to update our copy too.

This is a bit more involved than it is in the SDKV2 bridge - there we have a direct copy of the code which we use as is - in PF it is re-implemented as it works with the tftypes instead of the cty values.

Example

.

Output of pulumi about

.

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).
@VenelinMartinov VenelinMartinov added the kind/engineering Work that is not visible to an external user label Jul 10, 2024
@VenelinMartinov VenelinMartinov mentioned this issue Jul 10, 2024
Closed
@t0yv0
Copy link
Member

t0yv0 commented Jul 16, 2024

So I think this is interesting, we should be using vendored objchange.ProposedNew instead of the homegrown version to minimize discrepancies. Looks like one attempt was made #1276 but rejected for expediency as it had some concerns. Still I think that might be the way, we might need to add test cases to push through concerns but ultimately use the implementation from opentofu.

@t0yv0 t0yv0 mentioned this issue Jul 16, 2024
Merged
@t0yv0 t0yv0 self-assigned this Jul 22, 2024
@t0yv0
Copy link
Member

t0yv0 commented Jul 22, 2024

I've started working toward this and will try to complete in M107. Thanks!

@mjeffryes mjeffryes added this to the 0.108 milestone Jul 24, 2024
t0yv0 added a commit that referenced this issue Jul 26, 2024
@t0yv0
Use OpenTofu planner with Plugin Framework providers (#2188)
7f2e4c0 
OpenTofu planner code is now used with Plugin Framework providers to
emulate Terraform CLI behavior for `objchange.ProposedNew`. Note that
this code was already in use for SDKv2-based resources prior to the
change. The vendored code replaces a custom implementation that is now
removed.

Fixes #2172

Observable changes: Plugin Framework based resources may now reorder
outputs for Set collections.

This change is known to fix bugs such as
#2192 - to be
confirmed with a regression test in a follow-up PR.
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@t0yv0 t0yv0

Labels
kind/engineering Work that is not visible to an external user resolution/fixed This issue was fixed
Projects
None yet
Milestone
0.108
Development

Successfully merging a pull request may close this issue.

Use OpenTofu planner with Plugin Framework providers pulumi/pulumi-terraform-bridge
4 participants
@t0yv0 @mjeffryes @VenelinMartinov @pulumi-bot