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

Disentangle deciding DiffResponse from detailedDiff #2293

Closed
t0yv0 opened this issue Aug 9, 2024 · 2 comments
Closed

Disentangle deciding DiffResponse from detailedDiff #2293

t0yv0 opened this issue Aug 9, 2024 · 2 comments
Assignees
@VenelinMartinov
Labels
kind/task Work that's part of an ongoing epic resolution/fixed This issue was fixed
Milestone
0.110

Comments

@t0yv0
Copy link
Member

t0yv0 commented Aug 9, 2024 •
edited
Loading

This interaction between detailedDiff and DiffResponse needs to be removed:

pulumi-terraform-bridge/pkg/tfbridge/provider.go

Line 1122 in 46d7e98

if !diff.HasNoChanges() {

Resolves: #1501

This will undo: #1502

Assumes PlanResourceChange is enabled for roll-out. This change should be flagged together with the other changes for the accurate preview work.

Undoing the change risks regressing two important customer-visible behaviors, epic will have tasks to follow up so we have solutions for these:

Out of scope:
This was referenced Aug 9, 2024
Closed
Open
Open
@VenelinMartinov VenelinMartinov modified the milestone: 0.109 Aug 19, 2024
@VenelinMartinov
Copy link
Contributor

VenelinMartinov commented Aug 19, 2024 •
edited
Loading

I've hit #1501 in pulumi-azure while rolling out PRC: pulumi/pulumi-azure#2322 (comment)

One of the tests produces a diff due to

pulumi-terraform-bridge/pkg/tfbridge/provider.go

Line 1122 in 46d7e98

if !diff.HasNoChanges() {

The diff has no details and no explanation and should not be there as the stack has had no changes.

@VenelinMartinov VenelinMartinov mentioned this issue Aug 19, 2024
Closed
6 tasks
@mjeffryes mjeffryes added the kind/task Work that's part of an ongoing epic label Aug 23, 2024
This was referenced Aug 30, 2024
Closed
Merged
Merged
@mjeffryes mjeffryes added this to the 0.110 milestone Sep 12, 2024
VenelinMartinov added a commit that referenced this issue Sep 17, 2024
@VenelinMartinov
Detailed diff in cross tests (#2366)
4d6cd87 
This exposes the detailed diff in the cross-test interface for both TF
and Pulumi allowing us to write tests which assert on that.

Also adds a cross-test for
#1696 for the
existing `TestChangingMaxItems1FilterProperty`. This is one of the cases
where the detailed diff is load-bearing for the diff decision. The cross
test verifies that TF also finds a diff here, so our core diff algorithm
must be making the wrong decision.

Related to #2293
VenelinMartinov added a commit that referenced this issue Sep 20, 2024
@VenelinMartinov
Separate diff decision from presentation (#2379)
22452e6 
Adds a `DiffEqualDecisionOverride` to the shim layer which allows
provider implementations to specify a diff decision instead of relying
on the shim layer to do that.

Also adds a `DiffEqualDecisionOverride` to the PRC sdkv2 implementation
which opentofu does.

Also adds a feature flag `EnableAccurateBridgePreview` which guards this
feature.

will fix once rolled out:
#2293
will fix once rolled out:
#1501
will undo once rolled out:
#1502 as the
underlying issue was fixed during the PRC work.
Stacked on #2380
@VenelinMartinov VenelinMartinov added the resolution/fixed This issue was fixed label Sep 23, 2024
@VenelinMartinov
Copy link
Contributor

fixed in #2379

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@VenelinMartinov VenelinMartinov

Labels
kind/task Work that's part of an ongoing epic resolution/fixed This issue was fixed
Projects
None yet
Milestone
0.110
Development

No branches or pull requests
3 participants
@t0yv0 @mjeffryes @VenelinMartinov