samples(Storage): Add samples and tests for Bucket IP filter#3334
Draft
mahendra-google wants to merge 25 commits into
Draft
samples(Storage): Add samples and tests for Bucket IP filter#3334mahendra-google wants to merge 25 commits into
mahendra-google wants to merge 25 commits into
Conversation
- Added CreateBucketWithIpFilterSample for bucket security configuration. - Added ListBucketIpFiltersSample to demonstrate filter retrieval. - Included integration tests to verify IP filter logic.
- Addressing gemini review feedback on xml docs.
…tructure, and adjust test teardown registration
…ter integration tests
- Modify other samples and tests
… as per google doc
…ket ip filter. - Remove redundant bucket cleanup code from tests
|
Here is the summary of changes. You are about to add 7 region tags.
This comment is generated by snippet-bot.
|
product-auto-label
Bot
added
api: storage
![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 a suite of samples and tests for managing Google Cloud Storage bucket IP filtering, covering operations such as creation, updates, deletion, and bypass rules. The review feedback identifies several improvement opportunities: replacing hardcoded project IDs in tests and samples with dynamic values to ensure portability, optimizing the deletion logic to prevent unnecessary API calls when an IP range is missing, and adding a project ID parameter to the update sample for consistent resource name construction.
| Assert.NotNull(publicRanges); | ||
| Assert.Contains(newPublicRange, publicRanges); | ||
| var vpcNetwork = updatedBucket.IpFilter.VpcNetworkSources? | ||
| .FirstOrDefault(v => v.Network == "projects/storage-sdk-vendor/global/networks/default"); |
There was a problem hiding this comment.
The project ID storage-sdk-vendor is hardcoded in the VPC network path assertion. This will cause the test to fail in any environment where the project ID differs. Use _fixture.ProjectId to make the test portable.
.FirstOrDefault(v => v.Network == $"projects/{_fixture.ProjectId}/global/networks/default");
Comment on lines
+37
to
+39
| bucket.IpFilter.PublicNetworkSource.AllowedIpCidrRanges.Remove(ipAddress); | ||
| storage.UpdateBucket(bucket); | ||
| Console.WriteLine($"IP Address ({ipAddress}) from Allowed CIDR Ranges has been deleted from the bucket: {bucketName}"); |
There was a problem hiding this comment.
The code performs a bucket update and logs a success message even if the specified IP address was not found in the allowed ranges. It is better to check the return value of Remove() to avoid redundant API calls and provide accurate feedback to the user.
if (bucket.IpFilter.PublicNetworkSource.AllowedIpCidrRanges.Remove(ipAddress))
{
storage.UpdateBucket(bucket);
Console.WriteLine($"IP Address ({ipAddress}) from Allowed CIDR Ranges has been deleted from the bucket: {bucketName}");
}
else
{
Console.WriteLine($"IP Address ({ipAddress}) was not found in the Allowed CIDR Ranges for bucket: {bucketName}");
}
Comment on lines
+31
to
+32
| public Bucket UpdateBucketIpFilter(string bucketName = "your-unique-bucket-name", string publicRange = "192.0.2.0/24", | ||
| string vpcRange = "10.0.0.0/24") |
There was a problem hiding this comment.
This sample should accept a projectId parameter to construct the VPC network resource name dynamically, rather than using a hardcoded placeholder like your-project. This ensures consistency with other samples in this pull request, such as CreateBucketWithIpFilterSample.
public Bucket UpdateBucketIpFilter(string projectId = "your-project-id", string bucketName = "your-unique-bucket-name", string publicRange = "192.0.2.0/24",
string vpcRange = "10.0.0.0/24")| } | ||
| else | ||
| { | ||
| string networkName = "projects/your-project/global/networks/default"; |
There was a problem hiding this comment.
Use the projectId parameter to construct the network resource name instead of a hardcoded placeholder.
string networkName = $"projects/{projectId}/global/networks/default";| string newPublicRange = "192.0.2.0/24"; | ||
| string newVpcRange = "10.0.0.0/24"; | ||
| _fixture.CreateBucket(bucketName, multiVersion: false, ipFilter: true, registerForDeletion: true); | ||
| var updatedBucket = updateSample.UpdateBucketIpFilter(bucketName, newPublicRange, newVpcRange); |
There was a problem hiding this comment.
Pass the project ID from the fixture to the updated UpdateBucketIpFilter method.
var updatedBucket = updateSample.UpdateBucketIpFilter(_fixture.ProjectId, bucketName, newPublicRange, newVpcRange);
|
my buckets are supposed to be private
…On Tue, May 19, 2026, 8:57 AM gemini-code-assist[bot] < ***@***.***> wrote:
***@***.***[bot]* commented on this pull request.
Code Review
This pull request introduces a suite of samples and tests for managing
Google Cloud Storage bucket IP filtering, covering operations such as
creation, updates, deletion, and bypass rules. The review feedback
identifies several improvement opportunities: replacing hardcoded project
IDs in tests and samples with dynamic values to ensure portability,
optimizing the deletion logic to prevent unnecessary API calls when an IP
range is missing, and adding a project ID parameter to the update sample
for consistent resource name construction.
------------------------------
In storage/api/Storage.Samples.Tests/UpdateBucketIpFilterTest.cs
<#3334 (comment)>
:
> + [Fact]
+ public void TestUpdateBucketIpFilter()
+ {
+ var updateSample = new UpdateBucketIpFilterSample();
+ var bucketName = _fixture.GenerateBucketName();
+ string newPublicRange = "192.0.2.0/24";
+ string newVpcRange = "10.0.0.0/24";
+ _fixture.CreateBucket(bucketName, multiVersion: false, ipFilter: true, registerForDeletion: true);
+ var updatedBucket = updateSample.UpdateBucketIpFilter(bucketName, newPublicRange, newVpcRange);
+ Assert.NotNull(updatedBucket.IpFilter);
+ Assert.Equal("Disabled", updatedBucket.IpFilter.Mode);
+ var publicRanges = updatedBucket.IpFilter.PublicNetworkSource?.AllowedIpCidrRanges;
+ Assert.NotNull(publicRanges);
+ Assert.Contains(newPublicRange, publicRanges);
+ var vpcNetwork = updatedBucket.IpFilter.VpcNetworkSources?
+ .FirstOrDefault(v => v.Network == "projects/storage-sdk-vendor/global/networks/default");
[image: high]
<https://camo.githubusercontent.com/7559374fd248a2a146dfe7112beda558979c4f6d02dbe7c5161fd893fe834423/68747470733a2f2f7777772e677374617469632e636f6d2f636f64657265766965776167656e742f686967682d7072696f726974792e737667>
The project ID storage-sdk-vendor is hardcoded in the VPC network path
assertion. This will cause the test to fail in any environment where the
project ID differs. Use _fixture.ProjectId to make the test portable.
.FirstOrDefault(v => v.Network == $"projects/{_fixture.ProjectId}/global/networks/default");
------------------------------
In storage/api/Storage.Samples/DeleteBucketIpFilter.cs
<#3334 (comment)>
:
> + bucket.IpFilter.PublicNetworkSource.AllowedIpCidrRanges.Remove(ipAddress);
+ storage.UpdateBucket(bucket);
+ Console.WriteLine($"IP Address ({ipAddress}) from Allowed CIDR Ranges has been deleted from the bucket: {bucketName}");
[image: medium]
<https://camo.githubusercontent.com/32601710f6703a1d3cdbb05c7f9f05d1d8c88abc4d4e4d4e25ff218874a45279/68747470733a2f2f7777772e677374617469632e636f6d2f636f64657265766965776167656e742f6d656469756d2d7072696f726974792e737667>
The code performs a bucket update and logs a success message even if the
specified IP address was not found in the allowed ranges. It is better to
check the return value of Remove() to avoid redundant API calls and
provide accurate feedback to the user.
if (bucket.IpFilter.PublicNetworkSource.AllowedIpCidrRanges.Remove(ipAddress))
{
storage.UpdateBucket(bucket);
Console.WriteLine($"IP Address ({ipAddress}) from Allowed CIDR Ranges has been deleted from the bucket: {bucketName}");
}
else
{
Console.WriteLine($"IP Address ({ipAddress}) was not found in the Allowed CIDR Ranges for bucket: {bucketName}");
}
------------------------------
In storage/api/Storage.Samples/UpdateBucketIpFilter.cs
<#3334 (comment)>
:
> + public Bucket UpdateBucketIpFilter(string bucketName = "your-unique-bucket-name", string publicRange = "192.0.2.0/24",
+ string vpcRange = "10.0.0.0/24")
[image: medium]
<https://camo.githubusercontent.com/32601710f6703a1d3cdbb05c7f9f05d1d8c88abc4d4e4d4e25ff218874a45279/68747470733a2f2f7777772e677374617469632e636f6d2f636f64657265766965776167656e742f6d656469756d2d7072696f726974792e737667>
This sample should accept a projectId parameter to construct the VPC
network resource name dynamically, rather than using a hardcoded
placeholder like your-project. This ensures consistency with other
samples in this pull request, such as CreateBucketWithIpFilterSample.
public Bucket UpdateBucketIpFilter(string projectId = "your-project-id", string bucketName = "your-unique-bucket-name", string publicRange = "192.0.2.0/24",
string vpcRange = "10.0.0.0/24")
------------------------------
In storage/api/Storage.Samples/UpdateBucketIpFilter.cs
<#3334 (comment)>
:
> + bucket.IpFilter.VpcNetworkSources = new List<Bucket.IpFilterData.VpcNetworkSourcesData>();
+ }
+
+ var existingVpcNetwork = bucket.IpFilter.VpcNetworkSources.FirstOrDefault();
+
+ if (existingVpcNetwork != null)
+ {
+ if (existingVpcNetwork.AllowedIpCidrRanges == null)
+ {
+ existingVpcNetwork.AllowedIpCidrRanges = new List<string>();
+ }
+ existingVpcNetwork.AllowedIpCidrRanges.Add(vpcRange);
+ }
+ else
+ {
+ string networkName = "projects/your-project/global/networks/default";
[image: medium]
<https://camo.githubusercontent.com/32601710f6703a1d3cdbb05c7f9f05d1d8c88abc4d4e4d4e25ff218874a45279/68747470733a2f2f7777772e677374617469632e636f6d2f636f64657265766965776167656e742f6d656469756d2d7072696f726974792e737667>
Use the projectId parameter to construct the network resource name
instead of a hardcoded placeholder.
string networkName = $"projects/{projectId}/global/networks/default";
------------------------------
In storage/api/Storage.Samples.Tests/UpdateBucketIpFilterTest.cs
<#3334 (comment)>
:
> + private readonly StorageFixture _fixture;
+
+ public UpdateBucketIpFilterTest(StorageFixture fixture)
+ {
+ _fixture = fixture;
+ }
+
+ [Fact]
+ public void TestUpdateBucketIpFilter()
+ {
+ var updateSample = new UpdateBucketIpFilterSample();
+ var bucketName = _fixture.GenerateBucketName();
+ string newPublicRange = "192.0.2.0/24";
+ string newVpcRange = "10.0.0.0/24";
+ _fixture.CreateBucket(bucketName, multiVersion: false, ipFilter: true, registerForDeletion: true);
+ var updatedBucket = updateSample.UpdateBucketIpFilter(bucketName, newPublicRange, newVpcRange);
[image: medium]
<https://camo.githubusercontent.com/32601710f6703a1d3cdbb05c7f9f05d1d8c88abc4d4e4d4e25ff218874a45279/68747470733a2f2f7777772e677374617469632e636f6d2f636f64657265766965776167656e742f6d656469756d2d7072696f726974792e737667>
Pass the project ID from the fixture to the updated UpdateBucketIpFilter
method.
var updatedBucket = updateSample.UpdateBucketIpFilter(_fixture.ProjectId, bucketName, newPublicRange, newVpcRange);
—
Reply to this email directly, view it on GitHub
<#3334 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BNPSYU6TRRKQG4FQVHNJ4ZL43RR6BAVCNFSM6AAAAACZENO2VSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DGMJZGYZTEMZQG4>
.
You are receiving this because you are subscribed to this thread.Message
ID: <GoogleCloudPlatform/dotnet-docs-samples/pull/3334/review/4319632307@
github.com>
|
Contributor
Author
Thanks for taking the time to review the code! Could you please point me to the specific line of code where the buckets are private? I want to make sure I get that fixed and —keeping that data private is definitely a priority for our customers. Appreciate the help! |
Contributor
Author
|
I have moved this PR to Draft mode for now. Based on the updates in issue #422766398 (comment 8), the backend tags for the Bucket IP Filter samples are not fully ready. I will mark this back as ready for review as soon as those tags are fixed. |
Updated the start and end region tags for the listbuckets with ip filter code samples
…ateBucketIpFilter Updates the UpdateBucketIpFilter method signature to accept projectId and vpcNetwork parameters instead of relying on a hardcoded string. This allows users to pass in actual configuration values and use dynamic string interpolation. Updates the test accordingly
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR Introduces code samples and integration tests for the full Bucket IP Filter lifecycle.
Operations Covered are :
Create / Update / Delete: Management of IP filter rule sets.
Get / List: Retrieval of configuration metadata.
Disable / Bypass: Logic for pausing filters or configuring administrative overrides.