feat(storage): add support for bucket ip filter #15250
Conversation
product-auto-label
bot
added
the
api: storage
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15250 +/- ##
==========================================
+ Coverage 92.93% 92.94% +0.01%
==========================================
Files 2394 2396 +2
Lines 215384 215803 +419
==========================================
+ Hits 200163 200574 +411
- Misses 15221 15229 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| std::ostream& operator<<(std::ostream& os, | ||
| BucketIpFilterPublicNetworkSource const& rhs) { | ||
| os << "BucketIpFilterPublicNetworkSource={allowed_ip_cidr_ranges=["; |
There was a problem hiding this comment.
return os << "BucketIpFilterPublicNetworkSource={allowed_ip_cidr_ranges=["
<< absl::StrJoin(rhs.allowed_ip_cidr_ranges, ", ")
<< "]}";
|
|
||
| std::ostream& operator<<(std::ostream& os, | ||
| BucketIpFilterVpcNetworkSource const& rhs) { | ||
| os << "BucketIpFilterVpcNetworkSource={network=" << rhs.network |
There was a problem hiding this comment.
return os << "BucketIpFilterVpcNetworkSource={"
<< "network=" << rhs.network
<< ", allowed_ip_cidr_ranges=["
<< absl::StrJoin(rhs.allowed_ip_cidr_ranges, ", ")
<< "]}";
| } | ||
| if (rhs.vpc_network_sources) { | ||
| os << "vpc_network_sources=["; | ||
| char const* sep = ""; |
There was a problem hiding this comment.
absl::StrJoin(*rhs.vpc_network_sources, ", ")
| std::ostream& operator<<(std::ostream& os, BucketIpFilter const& rhs) { | ||
| google::cloud::internal::IosFlagsSaver save_format(os); | ||
| os << "BucketIpFilter={"; | ||
| os << "mode=" << rhs.mode.value_or("") << ", "; |
There was a problem hiding this comment.
nit: This , can be trailing in the output, if no if condition is true underneath.
| copy.allowed_ip_cidr_ranges.pop_back(); | ||
| EXPECT_NE(source, copy); | ||
| } | ||
|
|
There was a problem hiding this comment.
Another test should be added to ensure that underline structure is ordered strcuture.
TEST(BucketIpFilterTest, PublicNetworkSourceOrderMatters) {
BucketIpFilterPublicNetworkSource const source1{{"1.2.3.4/32", "5.6.7.8/32"}};
BucketIpFilterPublicNetworkSource const source2{{"5.6.7.8/32", "1.2.3.4/32"}};
// The two sources have the same elements but in a different order.
// They should NOT be equal.
EXPECT_NE(source1, source2);
}
| @@ -192,6 +192,49 @@ Status ParseIamConfiguration(BucketMetadata& meta, nlohmann::json const& json) { | |||
| return Status{}; | |||
| } | |||
|
|
|||
| Status ParseIpFilter(BucketMetadata& meta, nlohmann::json const& json) { | |||
There was a problem hiding this comment.
This code can be made more readable by using from_json semantics.
| @@ -402,6 +445,38 @@ void ToJsonIamConfiguration(nlohmann::json& json, BucketMetadata const& meta) { | |||
| json["iamConfiguration"] = std::move(value); | |||
| } | |||
|
|
|||
| void ToJsonIpFilter(nlohmann::json& json, BucketMetadata const& meta) { | |||
There was a problem hiding this comment.
Ditto, to_json semantics seems to be potential candidate here.
| }, | ||
| "vpcNetworkSources": [{ | ||
| "network": "projects/p/global/networks/n", | ||
| "allowedIpCidrRanges": ["5.6.7.8/32"] |
There was a problem hiding this comment.
Add more testcase for multiple IpCidrRanges in same network.
| @@ -209,6 +209,45 @@ Status PatchIamConfig(Bucket& b, nlohmann::json const& i) { | |||
| return Status{}; | |||
| } | |||
|
|
|||
| Status PatchIpFilter(Bucket& b, nlohmann::json const& p) { | |||
| if (p.is_null()) { | |||
| }, | ||
| "vpcNetworkSources": [{ | ||
| "network": "projects/p/global/networks/n", | ||
| "allowedIpCidrRanges": ["5.6.7.8/32"] |
There was a problem hiding this comment.
would be good to add testcase which handles multiple ipCidrRanges for same vpcNetwork instead of only one.
This change is