Skip to content

Sanitize Identity field in InstanceDiff - #512

Merged
sergenyalcin merged 2 commits into
crossplane:mainfrom
sergenyalcin:ignore-identity-in-diff
Jul 31, 2025
Merged

Sanitize Identity field in InstanceDiff#512
sergenyalcin merged 2 commits into
crossplane:mainfrom
sergenyalcin:ignore-identity-in-diff

Conversation

@sergenyalcin

Copy link
Copy Markdown
Member

Description of your changes

This PR sanitizes Identity field in InstanceDiff. This is for preventing continuous diff.

I have:

  • Read and followed Upjet's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

https://github.com/crossplane-contrib/provider-upjet-aws/actions/runs/16419565210

@erhancagirici erhancagirici left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, lgtm. Dropped a few notes on compatibility aspects.

Comment thread go.mod
github.com/hashicorp/go-cty v1.5.0
github.com/hashicorp/hcl/v2 v2.23.0
github.com/hashicorp/terraform-json v0.25.0
github.com/hashicorp/terraform-plugin-framework v1.15.0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a general reminder, especially for testing:
some TF providers might still depend on a lower version of terraform-plugin-framework. This might potentially cause diamond dependency issue when a xp provider imports both upjet and upstream TF provider.

Comment thread go.mod
github.com/hashicorp/terraform-json v0.25.0
github.com/hashicorp/terraform-plugin-framework v1.15.0
github.com/hashicorp/terraform-plugin-go v0.28.0
github.com/hashicorp/terraform-plugin-sdk/v2 v2.37.0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to the other comment, a potential diamond dependency is also possible here.

// Sanitize Identity field in Diff.
// This causes continuous diff loop.
if instanceDiff != nil {
instanceDiff.Identity = nil

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might potentially need something similar for plugin framework diff checks. have we double checked this?

@turkenf turkenf mentioned this pull request Jul 29, 2025
3 tasks
Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
@sergenyalcin
sergenyalcin force-pushed the ignore-identity-in-diff branch from 13ebe7a to ffb68a0 Compare July 31, 2025 09:32

@erhancagirici erhancagirici left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sergenyalcin! LGTM. From testing, I can see that the diamond dependencies are not an issue and properly handled.

@sergenyalcin
sergenyalcin merged commit 96241b0 into crossplane:main Jul 31, 2025
6 checks passed
@ulucinar ulucinar added the bug Something isn't working label Nov 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants