Sanitize Identity field in InstanceDiff - #512
Conversation
erhancagirici
left a comment
There was a problem hiding this comment.
Overall, lgtm. Dropped a few notes on compatibility aspects.
| 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 |
There was a problem hiding this comment.
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.
| 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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
we might potentially need something similar for plugin framework diff checks. have we double checked this?
Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>
13ebe7a to
ffb68a0
Compare
erhancagirici
left a comment
There was a problem hiding this comment.
Thanks @sergenyalcin! LGTM. From testing, I can see that the diamond dependencies are not an issue and properly handled.
Description of your changes
This PR sanitizes
Identityfield inInstanceDiff. This is for preventing continuous diff.I have:
make reviewableto ensure this PR is ready for review.backport release-x.ylabels to auto-backport this PR if necessary.How has this code been tested
https://github.com/crossplane-contrib/provider-upjet-aws/actions/runs/16419565210