[fabioaraujopt]

Description of your changes

#728 fixed filterInitExclusiveDiffs for CRD versions that serve a Terraform singleton list as an embedded object. initProviderOnlyPaths already handles the same case for the plugin framework client. The CLI-based client still has the bug.

NewFileProducer builds the lifecycle.ignore_changes list from spec.forProvider and spec.initProvider in the shape the CRD serves them. For an initProvider-only field inside such a block, it emits block["field"]. That path doesn't address the list block's element in Terraform, so the field isn't ignored. The correct path is block[0].field.

This PR computes the paths on copies of both parameter sets with ApplyTFConversions(_, config.ToTerraform) applied, so the paths follow the Terraform schema like the other two clients.

Why copies: ApplyTFConversions converts in place, and NewFileProducer still merges and converts the original maps afterwards. ToSingletonList isn't idempotent, so converting the originals early would wrap the block twice. The copy is a json.TFParser round trip, which is how generated GetParameters builds these maps.

The PR also adds TestFilterInitExclusiveDiffs, because #728 shipped without a test. The test drives filterInitExclusiveDiffs with the SDK's own InstanceDiff, not a hand-written one, so it checks the real attribute key format (high_availability.0.standby_availability_zone). The scenario is the azurerm_postgresql_flexible_server failover from crossplane-contrib/provider-upjet-azure#1303: both zones are set only in initProvider, and Azure swaps them. The cases cover:

• initProvider-only fields at the top level and inside the embedded object are filtered;
• a forProvider sibling inside the same embedded object still diffs;
• a field set in both forProvider and initProvider still diffs.

With #728's two ApplyTFConversions calls removed, all three cases fail on high_availability.0.standby_availability_zone. They also pass at 7b0ca4f, the upjet commit provider-upjet-azure v2.8.0 ships, so #1303 is fixed from v2.8.0.

Not changed here: an initProvider path that indexes into a TypeSet still can't match. SDK diff keys for set elements are hash codes, not positions. The plugin framework client skips such paths with indexesIntoSet.

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

• New TestWriteMainTF/SuccessManagementPoliciesSingletonListConversion: fails on main with block["standby_zone"] and passes with the fix (block[0].standby_zone).
• New TestFilterInitExclusiveDiffs: passes on main. It fails with fix: apply TF conversions before filtering initProvider-exclusive diffs #728's conversion removed, which confirms it covers fix: apply TF conversions before filtering initProvider-exclusive diffs #728.
• go test ./pkg/... -count=1 passes, and go vet is clean.
• golangci-lint run ./pkg/terraform/... ./pkg/controller/... with the pinned v2.13.2 reports 0 issues.
• make reviewable was not run, because I don't have the build submodule set up locally.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The changes derive Terraform ignore paths from converted copies of provider parameters and add tests for singleton-list ignore paths and initProvider diff filtering.

Changes: InitProvider handling

Layer / File(s)	Summary
Terraform-shaped ignore paths pkg/terraform/files.go, pkg/terraform/files_test.go	NewFileProducer derives ignore paths from copied provider parameters converted to Terraform shape. Conversion errors include resource context. The test checks the generated singleton-list configuration and ignore paths.
InitProvider diff filtering pkg/controller/external_tfpluginsdk_test.go	A new test checks remaining diff keys for initProvider-only fields, differing forProvider fields, and fields present in both providers.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: Minimal · up to aa583

No merge-blocking issue is established by the supplied evidence. The change appears mergeable subject to normal build and test checks.

Pre-merge checks: 6 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Title check		The title "fix(terraform): compute ignore_changes paths in the Terraform schema shape" describes the main change: ignore_changes paths for singleton lists are now computed in the Terraform schema shap…	Shorten the title to under 72 characters while keeping the same meaning. For example: "fix(terraform): compute ignore_changes paths in Terraform schema shape" (72 characters or fewer, verify the count before saving).

Passed checks: 6 passed

Check name	Status	Explanation
Description check		The description clearly explains the change. It covers the singleton-list ignore_changes path bug in the CLI-based client, the fix that computes paths on copies with ApplyTFConversions, the reason cop…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Configuration Api Breaking Changes		No files under pkg/config/** changed in the reviewed pull-request range. The diff only modifies pkg/controller/external_tfpluginsdk_test.go, pkg/terraform/files.go, and `pkg/terraform/files_test…
Generated Code Manual Edits		The pull request changes three Go files: pkg/controller/external_tfpluginsdk_test.go, pkg/terraform/files.go, and pkg/terraform/files_test.go. No changed file matches the zz_*.go pattern, so the gener…
Template Breaking Changes		PASS: The pull request does not change a controller template or generated controller implementation. The only pkg/controller/external*.go change is 160 lines added to `pkg/controller/external_tfplug…

Full details: Description check

Explanation

The description clearly explains the change. It covers the singleton-list ignore_changes path bug in the CLI-based client, the fix that computes paths on copies with ApplyTFConversions, the reason copies are needed, the new TestFilterInitExclusiveDiffs test, and the test results. It matches the changeset in pkg/terraform/files.go, files_test.go, and the controller test.

Full details: Configuration Api Breaking Changes

Explanation

No files under pkg/config/** changed in the reviewed pull-request range. The diff only modifies pkg/controller/external_tfpluginsdk_test.go, pkg/terraform/files.go, and pkg/terraform/files_test.go, so this check's configuration API breaking-change conditions are not applicable.

Full details: Generated Code Manual Edits

Explanation

The pull request changes three Go files: pkg/controller/external_tfpluginsdk_test.go, pkg/terraform/files.go, and pkg/terraform/files_test.go. No changed file matches the zz_*.go pattern, so the generated-code manual-edit condition does not apply.

Full details: Template Breaking Changes

Explanation

PASS: The pull request does not change a controller template or generated controller implementation. The only pkg/controller/external*.go change is 160 lines added to pkg/controller/external_tfpluginsdk_test.go, and the production behavior change is in pkg/terraform/files.go. Therefore the template-breaking-change condition is not applicable.

Full details: Title check

Explanation

The title "fix(terraform): compute ignore_changes paths in the Terraform schema shape" describes the main change: ignore_changes paths for singleton lists are now computed in the Terraform schema shape, as pkg/terraform/files.go does through copies of forProvider and initProvider. The title is 74 characters, which exceeds the 72-character limit in the title requirements, so the title check is not fully met on length. The wording is otherwise specific and accurate.

Resolution

Shorten the title to under 72 characters while keeping the same meaning. For example: "fix(terraform): compute ignore_changes paths in Terraform schema shape" (72 characters or fewer, verify the count before saving).

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: crossplane/upjet
• Branch: main
• File: .coderabbit.yaml
• Commit: ff4a298
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 87aed331-1587-4fb5-9976-85c84aeb246b

Commits

Reviewing files that changed from the base of the PR and between ff4a298 and aa58361.

Files selected for processing: 3

• pkg/controller/external_tfpluginsdk_test.go
• pkg/terraform/files.go
• pkg/terraform/files_test.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.