Skip to content

Fix: google_service_networking_connection perma-diff on reserved_peering_ranges#16191

Closed
SCSAndre wants to merge 10 commits intoGoogleCloudPlatform:mainfrom
SCSAndre:fix-servicenetworking-permadiff
Closed

Fix: google_service_networking_connection perma-diff on reserved_peering_ranges#16191
SCSAndre wants to merge 10 commits intoGoogleCloudPlatform:mainfrom
SCSAndre:fix-servicenetworking-permadiff

Conversation

@SCSAndre
Copy link
Contributor

@SCSAndre SCSAndre commented Jan 22, 2026

Description

Fixes #20398

This PR resolves a perma-diff issue in google_service_networking_connection where the order of reserved_peering_ranges caused Terraform to detect changes during every plan, even if the set of ranges remained identical.

Changes

  • Schema Change: added a DiffSuppressFunc to reserved_peering_ranges in resource_service_networking_connection.go to ignore ordering differences between config and state.
  • Helper Function: added stringListDiffSuppress to handle the unordered comparison of the string list.
  • Test: Added TestAccServiceNetworkingConnection_reorder to resource_service_networking_connection_test.go to verify that changing the order of ranges does not trigger an update.

Breaking Change Note

This change utilizes DiffSuppressFunc on the existing TypeList schema rather than converting to TypeSet.

  • Impact: None. This is not a breaking change. Existing state files remain compatible as the schema type has not changed.

Verification

I have verified this locally by running the new acceptance test:
make testacc TEST=./google/services/servicenetworking TESTARGS='-run TestAccServiceNetworkingConnection_reorder'
Result: PASS

`google_service_networking_connection`: fixed a permanent diff issue where changing the order of `reserved_peering_ranges` triggered an unnecessary update.

…ompute_security_policy

This change promotes the request_body_inspection_size field from Beta to GA
in the google_compute_security_policy resource.

The GA Compute API (v1) now supports this field with values: 8KB, 16KB, 32KB,
48KB, and 64KB for WAF request body inspection configuration.

Changes:
- Removed version guards from schema definition
- Removed version guards from expand/flatten functions
- Updated ForceSendFields to include field for all versions
- Removed [Beta] marker from documentation
- Enabled tests for GA provider

```release-note:enhancement
compute: promoted `request_body_inspection_size` to GA in `google_compute_security_policy`
```
@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Jan 22, 2026
@github-actions github-actions bot requested a review from shuyama1 January 22, 2026 21:58
@github-actions
Copy link

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

Googlers: For automatic test runs see go/terraform-auto-test-runs.

@shuyama1, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 97 insertions(+), 3 deletions(-))
google-beta provider: Diff ( 3 files changed, 97 insertions(+), 3 deletions(-))

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

  • Field reserved_peering_ranges changed from TypeList to TypeSet on google_service_networking_connection - reference

If you believe this detection to be incorrect please raise the concern with your reviewer.
If you intend to make this change you will need to wait for a major release window.
An override-breaking-change label can be added to allow merging.

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 9
Passed tests: 8
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages
  • servicenetworking

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccServiceNetworkingConnection_reorder

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccServiceNetworkingConnection_reorder [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🟢 All tests passed!

View the build log or the debug log for each test

Copy link
Member

@shuyama1 shuyama1 left a comment

Choose a reason for hiding this comment

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

@SCSAndre
Copy link
Contributor Author

Hey @shuyama1, I thought it would be a minor release, considering the documentation:

https://googlecloudplatform.github.io/magic-modules/breaking-changes/make-a-breaking-change/#make-a-breaking-change:

 [...] "Changing a List to a Set where the field is unordered and the order returned from the API changes unpredictably".

@github-actions github-actions bot requested a review from shuyama1 January 26, 2026 15:46
@SCSAndre
Copy link
Contributor Author

SCSAndre commented Jan 26, 2026

But in the opposite direction the documentation (https://googlecloudplatform.github.io/magic-modules/breaking-changes/breaking-changes/#field-level-breaking-changes) states that the same modification can be an example of a breaking change.

"Field-level breaking changes:

. Removing or renaming a field
. Changing field output type
. Between primitive types, like changing a String to an Integer
. Between complex types like changing a List to a Set."

@SCSAndre
Copy link
Contributor Author

@shuyama1 maybe I could try to solve it using a DiffSuppressFunc?

Copy link
Member

@shuyama1 shuyama1 left a comment

Choose a reason for hiding this comment

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

Thanks for looking into the docs!

My understanding is that the API preserves the order of the list as sent (evidenced by the fact mentioned in the GH issue that static lists work fine without diffs). Therefore, implementing this as a List is technically correct behavior.

Changing from List to Set is a breaking change because it breaks index-based interpolations. We can only make this change in a minor release if the API returns items in an unpredictable order (which would make index references unreliable anyway), but that seems not the case here.

- Updated resource_service_networking_connection.go
- Modified resource_service_networking_connection_meta.yaml
- Resolves connection reordering permadiff issue
@SCSAndre SCSAndre force-pushed the fix-servicenetworking-permadiff branch from 5277d17 to 486f2dd Compare January 28, 2026 20:45
@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Jan 28, 2026
@github-actions github-actions bot requested a review from shuyama1 January 28, 2026 20:46
@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Jan 29, 2026
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 2 files changed, 128 insertions(+), 4 deletions(-))
google-beta provider: Diff ( 2 files changed, 128 insertions(+), 4 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 9
Passed tests: 8
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages
  • servicenetworking

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccServiceNetworkingConnection_reorder

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccServiceNetworkingConnection_reorder [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

@SCSAndre
Copy link
Contributor Author

SCSAndre commented Jan 29, 2026

Closing this PR to reopen with a clean branch that contains only the relevant commits.
New PR incoming with the same fix.
@shuyama1

@SCSAndre
Copy link
Contributor Author

SCSAndre commented Jan 29, 2026

Closed in favor of clean PR: #16259

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants