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

Test: TF diff decision and pulumi diff decision match (SDKv2) #2298

Closed
t0yv0 opened this issue Aug 9, 2024 · 1 comment · Fixed by #2354
Closed

Test: TF diff decision and pulumi diff decision match (SDKv2) #2298

t0yv0 opened this issue Aug 9, 2024 · 1 comment · Fixed by #2354
Assignees
Labels
kind/task Work that's part of an ongoing epic resolution/fixed This issue was fixed

Comments

@t0yv0
Copy link
Member

t0yv0 commented Aug 9, 2024

We are looking to backfill the testsuite to increase our confidence that bridged providers work correctly and derisk rolling out the changes aimed at making the previews more accurate (per Epic).

With these OpenTOFU references:
https://github.com/opentofu/opentofu/blob/main/internal/plans/action.go#L11
https://github.com/opentofu/opentofu/blob/main/internal/tofu/node_resource_abstract_instance.go#L1019

The TF plans can be:

NoOp             Action = 0
Create           Action = '+'
Read             Action = '←'
Update           Action = '~'
DeleteThenCreate Action = '∓'
CreateThenDelete Action = '±'
Delete           Action = '-'
Forget           Action = '.'

Create, Delete, Forget are not of interest here, but the other plans are subject to customization based on TF features and are of interest.

Pulumi provider is somewhat limited in influencing the plans picked up by Pulumi, but it has some say in DiffResponse:

It can return changes: DiffResponse_DIFF_NONE
It can return changes: DiffResponse_DIFF_SOME

  • non-empty DiffResponse.Replaces will indicate a replacement CreateThenDelete replacement
  • non-empty DiffResponse.Replaces with DeleteBeforeReplace flag set indicate DeleteThenCreate replacement

Looking to ramp up or build a testsuite that lets us know that for a range of possible TF resource schemas, and (R1, R2) resource configuration pairs, the plans picked up by TF match the plans picked up by the bridged provider.

Specific feature of interest: setting ForceNew: true on *schema.Schema is expected to turn Update into replacement plans.

Out of scope: testing ignoreChanges.

@mjeffryes mjeffryes added the kind/task Work that's part of an ongoing epic label Aug 23, 2024
VenelinMartinov added a commit that referenced this issue Aug 28, 2024
This PR expands the diff cross-tests for SDKv2 and adds test cases for
all TF attribute and block types.

The cross-tests can now match the following against TF:
- creates
- deletes
- replaces with deleteBeforeReplace
- replaces without deleteBeforeReplace

Note that the last two required some workarounds in the cross-tests as
TF and pulumi differ on the order of operations here: TF deletes first
and creates after, while Pulumi defaults to create first and delete
after. This means that in order to match the behaviour we now use the TF
`lifecycle`'s `create_before_destroy`
(https://developer.hashicorp.com/terraform/language/meta-arguments/lifecycle#create_before_destroy)
by default and remove it for `deleteBeforeReplace` cases.

This also adds test cases for:
- Create
- Delete
- Update without a diff
- Update with a diff
- Replace
- Replace with deleteBeforeReplace

Each of these is handled for the following TF types
- string
- int
- float
- bool
- list attributes
- set attributes
- maps
- list blocks
- set blocks

fixes #2298
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Aug 28, 2024
@pulumi-bot
Copy link
Contributor

This issue has been addressed in PR #2354 and shipped in release v3.90.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/task Work that's part of an ongoing epic resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants