Repository navigation
feat(storage): add bucket IP filtering samples and tests - #14328
Conversation
Add Java code samples and integration tests demonstrating Cloud Storage Bucket IP filtering. Fixes: b/544985518 [Generated-by: AI]
nidhiii-27
added
ai-generated
storage-sample-architect
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several Java code snippets and an integration test demonstrating how to manage GCS bucket IP filtering configurations. Specifically, it adds functionality to create, enable, disable, retrieve, list, and delete IP filtering rules on a bucket. Since there are no review comments, I have no feedback to provide.
| public class IpFilterTest extends TestBase { | ||
|
|
||
| @Test | ||
| public void testBucketIpFilterLifecycle() throws Exception { |
There was a problem hiding this comment.
Can we break this lifecycle test into smaller tests. We should generally try sticking to one behavior per test go/tott/726 so its easier to debug if one specific step fails.
There was a problem hiding this comment.
Done
Co-authored by AI Agent
| vpcSources.addAll(ipFilter.getVpcNetworkSources()); | ||
| } | ||
| if (vpcNetworkToDelete != null) { | ||
| Iterator<VpcNetworkSource> iterator = vpcSources.iterator(); |
There was a problem hiding this comment.
Can we simplify the explicit Iterator loop here by calling removeIf directly on the vpcSources list ?
There was a problem hiding this comment.
Done
Co-authored by AI Agent
- Simplify VPC network source removal in DeleteBucketIpFilter using removeIf - Break IpFilterTest lifecycle test into individual single-behavior unit/integration tests [Generated-by: AI]
Synchronize samples-bucket-ip-filter-java with main to resolve the Kokoro FetchFromGithub SCM checkout failure on PR #14328. [Generated-by: AI]
nidhiii-27
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
| if (existingIpFilter != null) { | ||
| if (existingIpFilter.getPublicNetworkSource() != null | ||
| && existingIpFilter.getPublicNetworkSource().getAllowedIpCidrRanges() != null) { | ||
| publicRanges.addAll(existingIpFilter.getPublicNetworkSource().getAllowedIpCidrRanges()); |
There was a problem hiding this comment.
Is there a specific reason we need to add IP ranges and enable the feature at the same time in this sample? Is it required by the API? Would it make sense to simplify this sample so it just flips the mode to "Enabled", similar to how the Disable sample works?
There was a problem hiding this comment.
Done. Simplified EnableBucketIpFilter to only flip the mode to "Enabled" (matching DisableBucketIpFilter and standard Java GCS samples), with initial rules configured during bucket creation in CreateBucketIpFilter.
Co-authored by AI Agent
- Simplify EnableBucketIpFilter to only flip mode to Enabled matching DisableBucketIpFilter - Simplify GetBucketIpFilter to print IpFilter directly - Streamline rule deletion in DeleteBucketIpFilter - Update IpFilterTest calls to match 2-argument signature of EnableBucketIpFilter [Generated-by: AI]
- Direct builder mutation on IpFilter avoiding extra tracking flags - Streamline parameter names to publicRange and vpcNetwork [Generated-by: AI]
Dhriti07
left a comment
Dhriti07
left a comment
There was a problem hiding this comment.
I have approved this pr - left a thought regarding DeleteBucketIpFilter sample before merging
…cketIpFilter Add an inline comment explaining that storage_delete_ip_filtering_rules selectively removes specific rules and showing how to clear the entire IP filter configuration via bucket.toBuilder().setIpFilter(null).build(). [Generated-by: AI]
Add Java code samples and integration tests demonstrating Cloud Storage Bucket IP filtering:
CreateBucketIpFilter.java)EnableBucketIpFilter.java)GetBucketIpFilter.java)DeleteBucketIpFilter.java)DisableBucketIpFilter.java)ListBucketsIpFilter.java)IpFilterTest.java)Fixes: b/544985518