feat(TPG>=6.37)!: Add support for ip_filter to storage buckets

[malikalrizky]

This commit introduces the ip_filter functionality to the google_storage_bucket resource, allowing for more granular, IP-based access controls.

The changes include:

• A new ip_filter variable in variables.tf to define the IP filtering rules.
• A dynamic ip_filter block in main.tf to apply the configuration to the storage buckets.
• An update to versions.tf to enforce a minimum terraform-provider-google version of 6.37.0, which is required for this feature.

[malikalrizky]

should I create test case?

[nitishchauhan94]

Any timelines when this feature branch would be reviewed and merged ?

[q2w]

Can you also run ENABLE_BPMETADATA=1 make generate_docs so that the metadata files are also updated!

[q2w]

/gcbrun

[malikalrizky]

Can you also run ENABLE_BPMETADATA=1 make generate_docs so that the metadata files are also updated!

done @q2w
can i have a review?

[q2w]

@malikalrizky The integration tests are failing because the apply is failing for both the examples.

TestMultipleBuckets 2025-08-22T16:55:06Z command.go:206: module.cloud_storage.google_storage_bucket.buckets["two"]: Creation complete after 3s [id=multiple-buckets-1elw-two-1fb0]
TestMultipleBuckets 2025-08-22T16:55:06Z command.go:206: 
TestMultipleBuckets 2025-08-22T16:55:06Z command.go:206: Error: Error reading bucket after creation: googleapi: Error 403: There is an IP filtering condition that is preventing access to the resource., forbidden
TestMultipleBuckets 2025-08-22T16:55:06Z command.go:206: 
TestMultipleBuckets 2025-08-22T16:55:06Z command.go:206:   with module.cloud_storage.google_storage_bucket.buckets["one"],
TestMultipleBuckets 2025-08-22T16:55:06Z command.go:206:   on ../../main.tf line 40, in resource "google_storage_bucket" "buckets":
TestMultipleBuckets 2025-08-22T16:55:06Z command.go:206:   40: resource "google_storage_bucket" "buckets" {
TestMultipleBuckets 2025-08-22T16:55:06Z command.go:206: 
TestMultipleBuckets 2025-08-22T16:55:06Z retry.go:99: Returning due to fatal error: FatalError{Underlying: error while running command: exit status 1; 
Error: Error reading bucket after creation: googleapi: Error 403: There is an IP filtering condition that is preventing access to the resource., forbidden

[malikalrizky]

@q2w got it. i saw allow_all_service_agent_access is only available on newer provider versions so i'll use the latest one available.

I have a question about the example configurations and how they interact with integration tests.

Currently, both examples reference the published registry versions:

• examples/simple_bucket/main.tf: source = "terraform-google-modules/cloud-storage/google//modules/simple_bucket" with version = "~> 10.0"
• examples/multiple_buckets/main.tf: source = "terraform-google-modules/cloud-storage/google" with version = "~> 10.0"

Question: For integration tests in CI/CD to work properly with new development features (like the new ip_filter fields), should these examples reference the local module paths instead?

For example:

• examples/simple_bucket/main.tf: source = "../../modules/simple_bucket"
• examples/multiple_buckets/main.tf: source = "../.."

Context:

• I'm adding new fields to the ip_filter variable (allow_cross_org_vpcs and allow_all_service_agent_access)
• The integration tests use CFT's tft.AutoDiscoverAndTest(t) which runs the examples
• If examples use registry versions, tests would run against the old published code, not the new development features

My understanding: Integration tests should validate the current development code, so examples should reference local paths during development. But I want to confirm the correct pattern before updating.

What's the recommended approach here? Should I:

• Update examples to use local paths for integration testing?
• Keep registry references and handle this differently?
• Use a different pattern entirely?

Thanks for the guidance!

[q2w]

@malikalrizky The registry module references are replaced with local once before running the integration tests. https://pantheon.corp.google.com/cloud-build/builds/043a2330-1f85-44e1-a852-a87e2ee4d20e;step=0?e=AdcAppManagementForCloudNextLaunch::AdcAppManagementForCloudNextEnabled,AdcCoreForCloudNextLaunch::AdcCoreForCloudNextEnabled,AdcPrivateCatalogLaunch::AdcPrivateCatalogEnabled&project=cloud-foundation-cicd

[q2w]

@malikalrizky Could you run terraform fmt as i see some formatting issues? https://github.com/terraform-google-modules/terraform-google-cloud-storage/actions/runs/17170009300/job/48833245607?pr=403

[malikalrizky]

@q2w got it. please help review again

[m0ps]

Hey, @q2w!
Could you please help review this PR again? The minimum provider version is updated to match the version where ip_filter support was added. I've also added tests to validate ip_filter configuration. Integration tests have been verified in our local environment, and they have completed successfully. I hope that now this PR is ready to merge.

[q2w]

Thanks @m0ps! I see that the tests are still failing even after enabling allow_all_service_agent_access. This is happening because tests are run using SA provisioned on the fly and doesn't use any service agent to access the buckets.

Since this change is time critical and has been tested locally, i would recommend making changes to example and tests in a followup PR.

@apeabody What do you think?

[m0ps]

Thanks for checking it, @apeabody and @q2w
So... I think I've found the problem... When we were running tests, we were running them from our local machines:

1. bootstrap test project with make docker_test_prepare
2. run integration tests with make docker_test_integration

I've tried to replicate the Cloud Build setup in our env, and I finally managed to replicate this issue. See the details below.
There are 2 types of ip_sources:

1. public_network_source
2. vpc_network_sources
   Examples (which are validated by integration tests) utilize the first option, and that's why it works if we run tests from local machines. But, since Cloud Run is running inside GCP, in a Google-managed VPC, it can't access the bucket after its creation. I've checked IP filtering docs (https://cloud.google.com/storage/docs/ip-filtering-overview#limitations) and didn't find any mentions of Cloud Build, so it's not clear if it is supported at all. But, even if so, I think it makes the test env setup too complicated. Maybe we should skip adding this feature to examples (to avoid its validation by integration tests)?

I'm looking forward to your reply, since this feature is really crucial for us.

[apeabody]

Thanks for checking it, @apeabody and @q2w So... I think I've found the problem... When we were running tests, we were running them from our local machines:

1. bootstrap test project with make docker_test_prepare
2. run integration tests with make docker_test_integration

I've tried to replicate the Cloud Build setup in our env, and I finally managed to replicate this issue. See the details below. There are 2 types of ip_sources:

1. public_network_source
2. vpc_network_sources
   Examples (which are validated by integration tests) utilize the first option, and that's why it works if we run tests from local machines. But, since Cloud Run is running inside GCP, in a Google-managed VPC, it can't access the bucket after its creation. I've checked IP filtering docs (https://cloud.google.com/storage/docs/ip-filtering-overview#limitations) and didn't find any mentions of Cloud Build, so it's not clear if it is supported at all. But, even if so, I think it makes the test env setup too complicated. Maybe we should skip adding this feature to examples (to avoid its validation by integration tests)?

I'm looking forward to your reply, since this feature is really crucial for us.

Thanks @m0ps - It looks like it should be possible to bypass the bucket IP filtering rules for the test SA: https://cloud.devsite.corp.google.com/storage/docs/bypass-ip-filter#bypass-filtering-rules. That should permit the SA to complete the required storage.buckets.get.

[m0ps]

Thanks for the hint, @apeabody, but as I mentioned in my prev message, it's not as easy as it looks at a glance. While there is a permission that can allow SA to bypass IP filtering, it allows bypassing only the specific API calls:

• Getting a bucket's metadata (GET Bucket)
• Updating a bucket (PATCH Bucket)
• Deleting a bucket (DELETE Bucket)

However, the simple_bucket example includes IAM roles assignment, which performs a setIamPolicy call, which can't be bypassed. So... I've updated examples to keep the ip_filter feature only for the multiple buckets. I hope you will find this compromise acceptable.

[apeabody]

Thanks for the hint, @apeabody, but as I mentioned in my prev message, it's not as easy as it looks at a glance. While there is a permission that can allow SA to bypass IP filtering, it allows bypassing only the specific API calls:

• Getting a bucket's metadata (GET Bucket)
• Updating a bucket (PATCH Bucket)
• Deleting a bucket (DELETE Bucket)

However, the simple_bucket example includes IAM roles assignment, which performs a setIamPolicy call, which can't be bypassed. So... I've updated examples to keep the ip_filter feature only for the multiple buckets. I hope you will find this compromise acceptable.

Thanks for the deep dive @malikalrizky. Yes, I was focused on the provider's storage.buckets.get, and I think this is acceptable.

[apeabody]

Thanks for the contribution @malikalrizky!