feat(storage): Support bucket encryption config#33452
feat(storage): Support bucket encryption config#33452shubhangi-google wants to merge 24 commits intogoogleapis:mainfrom
Conversation
Fix missing newline at end of file in bucket_encryption_test.rb
|
Here is the summary of changes. You are about to add 3 region tags.
This comment is generated by snippet-bot.
|
Removed an unnecessary blank line before the encryption configuration documentation.
Removed unnecessary comment line in bucket.rb
Removed unnecessary comment line in bucket.rb.
Removed unnecessary comment line in bucket.rb
| end | ||
|
|
||
| it "deletes all encryption enforcement configs" do | ||
| # For the update, need to specify all three configs |
There was a problem hiding this comment.
Not sure if this statement is correct
| # # | ||
| # storage = Google::Cloud::Storage.new | ||
| # bucket = storage.bucket "my-bucket" | ||
| # bucket.customer_managed_encryption_enforcement_config #=> Google::Apis::StorageV1::Bucket::Encryption::CustomerManagedEncryptionEnforcementConfig.new |
There was a problem hiding this comment.
This comment is confusing. This is a getter method, so ideally we should be showing code sample for that.
There was a problem hiding this comment.
A gentle reminder on this
There was a problem hiding this comment.
modified the samples
| # | ||
| # storage = Google::Cloud::Storage.new | ||
| # bucket = storage.bucket "my-bucket" | ||
| # bucket.customer_supplied_encryption_enforcement_config #=> Google::Apis::StorageV1::Bucket::Encryption::CustomerSuppliedEncryptionEnforcementConfig.new |
There was a problem hiding this comment.
modified the samples
| # | ||
| # storage = Google::Cloud::Storage.new | ||
| # bucket = storage.bucket "my-bucket" | ||
| # bucket.google_managed_encryption_enforcement_config #=> Google::Apis::StorageV1::Bucket::Encryption::GoogleManagedEncryptionEnforcementConfig.new |
There was a problem hiding this comment.
Same comment here as well
There was a problem hiding this comment.
modified the samples
| if_metageneration_match: nil, | ||
| if_metageneration_not_match: nil | ||
| if_metageneration_not_match: nil, | ||
| bucket_encryption_config: nil |
There was a problem hiding this comment.
Why are we introducing new parameter here? Why can't we combine this in existing parameter attributes ?
There was a problem hiding this comment.
When using the standard bucket.update or @gapi.send(:encryption) patterns, the Ruby SDK was capturing the entire existing encryption state. This included server-generated fields like effectiveTime. When this "polluted" object was sent back via the PATCH API, Google rejected it with an InvalidArgumentError because those fields were not in the expected format.
The Fix
We implemented the update_bucket_encryption_enforcement_configmethod to send only the changed data to the patch request.
How it Works
- Isolation: Instead of modifying the existing bucket object, the method initializes a fresh, empty
Google::Apis::StorageV1::Bucket::Encryptioninstance. - Specific Assignment: It uses dynamic dispatch (
public_send) to set only the specific enforcement type requested (e.g.,customer_supplied).
update_bucket_encryption_enforcement_config sends the correct patch json in bucket_encryption_config
Comparison of JSON Payloads
Wrong Request (Polluted)
{
"encryption": {
"customerManagedEncryptionEnforcementConfig": {
"restrictionMode": "NotRestricted"
},
"customerSuppliedEncryptionEnforcementConfig": {
"restrictionMode": "NotRestricted"
},
"googleManagedEncryptionEnforcementConfig": {
"effectiveTime": "2026-03-25T11:40:17.182+00:00",
"restrictionMode": "FullyRestricted"
}
}
}Correct request
{
"encryption": {
"customerManagedEncryptionEnforcementConfig": {
"restrictionMode": "NotRestricted"
}
}
}There was a problem hiding this comment.
@bajajneha27 can you please provide your views on this approach
There was a problem hiding this comment.
User can still pass effectiveTime if they want to, in the new attribute.
I think we should not add a separate parameter just for this. We should make use of existing attribute
There was a problem hiding this comment.
Besides, if user wants to update other attributes AND bucket_encryption_configs, that won't work
https://github.com/googleapis/google-cloud-ruby/pull/33452/changes#diff-1c445f12b16180bcbdde4cab77c9b5ca55acc3fff5a45c1dd7d2bb4abb64bf76R3410-R3414
There was a problem hiding this comment.
Hi @bajajneha27 the existing patch method is sending all the encryption attribute (the existing one and the updated patch together)
as shown in this example #33452 (comment)
here the time format which we are receiving is UTC format from server and API is expecting RFC 3339 format as per encryption.googleManagedEncryptionEnforcementConfig.effectiveTime hence we are getting invalid argument error in response
as a solution I am creating a new method to update the encryption enforcement config which will send only the updated patch
this method is only for handling encryption_config so concern for other attributes will not be valid in this case
There was a problem hiding this comment.
I don't follow. Why would you get Invalid Argument error if you're not passing effective_time at all ?
What happens if I want to pass bucket_encryption_config and other attributes as well?
There was a problem hiding this comment.
Example we want to update customerManagedEncryptionEnforcementConfig restriction mode to "FullyRestricted"
When using the standard bucket.update the @gapi.send(:encryption) patterns, the Ruby SDK is capturing the entire existing encryption state and updating the customerManagedEncryptionEnforcementConfig restriction mode in it and send the updated encryption hash . which includes effective time we are receiving from API which is in UTC format
example:
{
"encryption": {
"customerManagedEncryptionEnforcementConfig": {
"restrictionMode": "NotRestricted"
},
"customerSuppliedEncryptionEnforcementConfig": {
"restrictionMode": "NotRestricted"
},
"googleManagedEncryptionEnforcementConfig": {
"effectiveTime": "2026-03-25T11:40:17.182+00:00",
"restrictionMode": "FullyRestricted"
}
}
}when this hash is submitted to server its returning error
Google::Cloud::InvalidArgumentError: invalid: Invalid argument. because server is expecting the effectiveTime in 1985-04-12T23:20:50.52Z format and not in UTC format
google-cloud-storage/test/google/cloud/storage/bucket_encryption_test.rb
Outdated
Show resolved
Hide resolved
| end | ||
|
|
||
| it "deletes all encryption enforcement configs" do | ||
| # For the update, need to specify all three configs |
| # # | ||
| # storage = Google::Cloud::Storage.new | ||
| # bucket = storage.bucket "my-bucket" | ||
| # bucket.customer_managed_encryption_enforcement_config #=> Google::Apis::StorageV1::Bucket::Encryption::CustomerManagedEncryptionEnforcementConfig.new |
There was a problem hiding this comment.
A gentle reminder on this
| # bucket.update_bucket_encryption_enforcement_config new_config | ||
| # | ||
| def update_bucket_encryption_enforcement_config incoming_config | ||
| attr_name = case incoming_config |
There was a problem hiding this comment.
AFAIU, this code will work only if we update just one config. What if I want to update multiple configs at the same time? For ex. GoogleManagedEncryptionEnforcementConfig and CustomerManagedEncryptionEnforcementConfig
There was a problem hiding this comment.
modified the code now the code will work for the hash we receive
| # storage = Google::Cloud::Storage.new | ||
| # bucket = storage.bucket "my-bucket" | ||
| # | ||
| # new_config = Google::Apis::StorageV1::Bucket::Encryption::CustomerSuppliedEncryptionEnforcementConfig.new restriction_mode: "NotRestricted" |
There was a problem hiding this comment.
The code sample here is for setting the config while the method is a getter here. We should update the sample to show how to use the getter method
There was a problem hiding this comment.
modified the samples
| # | ||
| # storage = Google::Cloud::Storage.new | ||
| # bucket = storage.bucket "my-bucket" | ||
| # new_config= Google::Apis::StorageV1::Bucket::Encryption::GoogleManagedEncryptionEnforcementConfig.new restriction_mode: "NotRestricted" |
There was a problem hiding this comment.
Same comment as above. This is a setter example
There was a problem hiding this comment.
modified the samples
| if_metageneration_match: nil, | ||
| if_metageneration_not_match: nil | ||
| if_metageneration_not_match: nil, | ||
| bucket_encryption_config: nil |
There was a problem hiding this comment.
User can still pass effectiveTime if they want to, in the new attribute.
I think we should not add a separate parameter just for this. We should make use of existing attribute
| if_metageneration_match: nil, | ||
| if_metageneration_not_match: nil | ||
| if_metageneration_not_match: nil, | ||
| bucket_encryption_config: nil |
There was a problem hiding this comment.
Besides, if user wants to update other attributes AND bucket_encryption_configs, that won't work
https://github.com/googleapis/google-cloud-ruby/pull/33452/changes#diff-1c445f12b16180bcbdde4cab77c9b5ca55acc3fff5a45c1dd7d2bb4abb64bf76R3410-R3414
google-cloud-storage/test/google/cloud/storage/bucket_encryption_test.rb
Show resolved
Hide resolved
| # bucket = storage.bucket "my-bucket" | ||
| # | ||
| # # Set restriction mode to FullyRestricted | ||
| # new_config = { restriction_mode: "FullyRestricted" } |
There was a problem hiding this comment.
Thanks for updating this. We should also add example of how we can set it using the request object.
| if_metageneration_match: nil, | ||
| if_metageneration_not_match: nil | ||
| if_metageneration_not_match: nil, | ||
| bucket_encryption_config: nil |
There was a problem hiding this comment.
I don't follow. Why would you get Invalid Argument error if you're not passing effective_time at all ?
What happens if I want to pass bucket_encryption_config and other attributes as well?
This pull request significantly enhances the Google Cloud Storage client library by introducing comprehensive support for managing bucket encryption enforcement configurations. Users can now define and enforce default encryption behaviors for their buckets and files using customer-managed, customer-supplied, or Google-managed encryption keys. The changes provide robust API methods, update internal mechanisms for handling these configurations, and include practical examples and tests to ensure functionality and ease of use.
Highlights
Google::Cloud::Storage::Bucketfor managing customer-managed, customer-supplied, and Google-managed encryption enforcement configurations, including getters and setters forcustomer_managed_encryption_enforcement_config,customer_supplied_encryption_enforcement_config, andgoogle_managed_encryption_enforcement_config.update_bucket_encryption_enforcement_configmethod to handle updates for different types of bucket encryption enforcement configurations in a unified manner.patch_gapi!Method: Modified the internalpatch_gapi!method to accept an optionalbucket_encryption_configparameter, allowing more flexible patching of encryption-related attributes.ostruct,cgi, andirbgems to theGemfilefor samples, addressing gems removed from Ruby core that are required for testing.This Pr was created as old PR got corrupted - #32778, #32786