fix(storage): add samples for bucket encryption enforcement config#16062
fix(storage): add samples for bucket encryption enforcement config#16062bajajneha27 wants to merge 2 commits intogoogleapis:v2.47.xfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for bucket encryption enforcement configurations (GMEK, CMEK, and CSEK) and removes the effectiveTime field from metadata parsing and serialization in both REST and gRPC paths. It also adds a new example file for managing these enforcement policies. Feedback focuses on ensuring consistent error handling for StatusOr objects and correcting logic in the update example to avoid overwriting existing configurations with hardcoded values.
| StatusOr<gcs::BucketMetadata> original = | ||
| client.GetBucketMetadata(bucket_name); | ||
|
|
||
| gcs::BucketMetadata updated_metadata = *original; |
There was a problem hiding this comment.
The StatusOr<T> returned by GetBucketMetadata should be checked for an error before being dereferenced. While dereferencing a StatusOr with an error will throw an exception, the idiomatic and recommended approach is to check its status explicitly, as is done in other examples in this file. This ensures consistent error handling and makes the code's intent clearer.
StatusOr<gcs::BucketMetadata> original = client.GetBucketMetadata(bucket_name);
if (!original) throw std::move(original).status();
gcs::BucketMetadata updated_metadata = *original;| // For the update, need to specify all three configs, so keeping this same | ||
| // as before | ||
| encryption.customer_supplied_encryption_enforcement_config | ||
| .restriction_mode = "FullyRestricted"; |
There was a problem hiding this comment.
The comment on these lines is misleading, and the code is incorrect if the intention is to preserve the existing configuration for CSEK. The encryption object is initialized with the bucket's existing configuration. By not modifying encryption.customer_supplied_encryption_enforcement_config, its value is preserved. These lines overwrite the preserved value with a hardcoded one and should be removed.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v2.47.x #16062 +/- ##
========================================
Coverage 92.96% 92.96%
========================================
Files 2460 2461 +1
Lines 228901 228990 +89
========================================
+ Hits 212807 212890 +83
- Misses 16094 16100 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.