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

Integrate with PlanResourceChange for SDKv2 based providers #1505

Closed
t0yv0 opened this issue Nov 3, 2023 · 2 comments · Fixed by #1614
Closed

Integrate with PlanResourceChange for SDKv2 based providers #1505

t0yv0 opened this issue Nov 3, 2023 · 2 comments · Fixed by #1614
Assignees
@t0yv0
Labels
area/tfbridge Issues in pkg/tfbridge, which provides interop between Pulumi and TF providers kind/enhancement Improvements or new features resolution/fixed This issue was fixed size/M Estimated effort to complete (up to 5 days).

Comments

@t0yv0
Copy link
Member

t0yv0 commented Nov 3, 2023 •
edited
Loading

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

Early notes on a possible refactor. PlanResourceChange method is the canonical integration point for computing diffs/plans accurately for SDKv2 based providers. Current implementation diverges from this, see #1282 for some correlated issues. We could consider building out support for some resources to opt to integrate via PlanResourcChange and hope it reduces the surface area for potential discrepancies with respect to TF providers.

There's some prior art:

  • code under tfshim/tfplugin5 sketches out one possible approach, but is not used in prod
  • code under pf integrates with Protocol 6 and does use PlanResourceChange and is used in prod; there are some known areas of issues and discrepancies from this experience:
    • defaults application
    • lacking detailed diff computation (TBD)
    • different approach to IgnoreChanges support

Affected area/feature
@t0yv0 t0yv0 added needs-triage Needs attention from the triage team kind/enhancement Improvements or new features labels Nov 3, 2023
@t0yv0
Copy link
Member Author

t0yv0 commented Nov 3, 2023 •
edited
Loading

CC @iwahbe capturing some notes. I'll correlate some issues later on. Notable flags in use that can ideally be subsumed by this:

@mikhailshilkov mikhailshilkov added area/tfbridge Issues in pkg/tfbridge, which provides interop between Pulumi and TF providers and removed needs-triage Needs attention from the triage team labels Nov 3, 2023
@t0yv0 t0yv0 mentioned this issue Nov 14, 2023
Closed
This was referenced Jan 8, 2024
Closed
Merged
Closed
@t0yv0
Copy link
Member Author

t0yv0 commented Jan 11, 2024

Adding some prior ticket lists that got patched with approximate methods but I believe root-cause is here.

#1231
#1021
#1521
#1033
#1314

@t0yv0 t0yv0 added the size/M Estimated effort to complete (up to 5 days). label Jan 22, 2024
t0yv0 added a commit that referenced this issue Jan 29, 2024
@t0yv0
Integrate with PlanResourceChange and ApplyResourceChange (#1614)
e91ed7e 
Fixes #1505

Requires pulumi/terraform-plugin-sdk#35

sdk-v2 bridge has a new option that changes the implementation of
resource lifecycle to go through TF SDKv2 gRPC methods.

```go
// Selectively opt-in resources that pass the filter to using PlanResourceChange. Resources are
// identified by their TF type name such as aws_ssm_document.
func WithPlanResourceChange(filter func(tfResourceType string) bool) providerOption { //nolint:revive
```

Methods from the TF SDKv2 lifecycle integrated with this flag:

- PlanResourceChange
- ApplyResourceChange
- ReadResource 
- ImportResourceState

Enables fixing:  pulumi/pulumi-aws#2555

Known differences: state returned by the new method will include
explicit entries `"foo": null` for properties that are known by the
schema but not set in the state. This seems to be benign for repeated
diffs and refresh applications.
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Jan 29, 2024
@t0yv0 t0yv0 mentioned this issue Jan 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@t0yv0 t0yv0

Labels
area/tfbridge Issues in pkg/tfbridge, which provides interop between Pulumi and TF providers kind/enhancement Improvements or new features resolution/fixed This issue was fixed size/M Estimated effort to complete (up to 5 days).
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Integrate with PlanResourceChange and ApplyResourceChange pulumi/pulumi-terraform-bridge
3 participants
@t0yv0 @mikhailshilkov @pulumi-bot