Skip to content

fix(storage): add samples for bucket encryption enforcement config#16062

Open
bajajneha27 wants to merge 2 commits intogoogleapis:v2.47.xfrom
bajajneha27:bucket-encryption-sample-fix
Open

fix(storage): add samples for bucket encryption enforcement config#16062
bajajneha27 wants to merge 2 commits intogoogleapis:v2.47.xfrom
bajajneha27:bucket-encryption-sample-fix

Conversation

@bajajneha27
Copy link
Copy Markdown
Contributor

No description provided.

@product-auto-label product-auto-label bot added api: storage Issues related to the Cloud Storage API. samples Issues that are directly related to samples. labels Apr 1, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +143 to +146
StatusOr<gcs::BucketMetadata> original =
client.GetBucketMetadata(bucket_name);

gcs::BucketMetadata updated_metadata = *original;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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;

Comment on lines +158 to +161
// For the update, need to specify all three configs, so keeping this same
// as before
encryption.customer_supplied_encryption_enforcement_config
.restriction_mode = "FullyRestricted";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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
Copy link
Copy Markdown

codecov bot commented Apr 1, 2026

Codecov Report

❌ Patch coverage is 94.95798% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.96%. Comparing base (2708d95) to head (8f7680d).

Files with missing lines Patch % Lines
...s/storage_bucket_encryption_enforcement_samples.cc 95.61% 5 Missing ⚠️
google/cloud/storage/bucket_metadata.cc 80.00% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bajajneha27 bajajneha27 marked this pull request as ready for review April 1, 2026 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API. samples Issues that are directly related to samples.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant