Skip to content

feat(storage): Support bucket encryption config#33452

Open
shubhangi-google wants to merge 22 commits intogoogleapis:mainfrom
shubhangi-google:support_bucket_encryption_config
Open

feat(storage): Support bucket encryption config#33452
shubhangi-google wants to merge 22 commits intogoogleapis:mainfrom
shubhangi-google:support_bucket_encryption_config

Conversation

@shubhangi-google
Copy link
Copy Markdown
Contributor

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

  • New Bucket Encryption Configuration Methods: Added new methods to Google::Cloud::Storage::Bucket for managing customer-managed, customer-supplied, and Google-managed encryption enforcement configurations, including getters and setters for customer_managed_encryption_enforcement_config, customer_supplied_encryption_enforcement_config, and google_managed_encryption_enforcement_config.
  • Unified Encryption Update Method: Introduced a new update_bucket_encryption_enforcement_config method to handle updates for different types of bucket encryption enforcement configurations in a unified manner.
  • Enhanced patch_gapi! Method: Modified the internal patch_gapi! method to accept an optional bucket_encryption_config parameter, allowing more flexible patching of encryption-related attributes.
  • New Samples and Acceptance Tests: Included new samples and corresponding acceptance tests to demonstrate how to get, set, update, and remove bucket encryption enforcement configurations.
  • Gemfile Updates: Added ostruct, cgi, and irb gems to the Gemfile for samples, addressing gems removed from Ruby core that are required for testing.

This Pr was created as old PR got corrupted - #32778, #32786

@shubhangi-google shubhangi-google marked this pull request as ready for review March 30, 2026 06:48
@shubhangi-google shubhangi-google requested review from a team and yoshi-approver as code owners March 30, 2026 06:48
@snippet-bot
Copy link
Copy Markdown

snippet-bot bot commented Mar 30, 2026

Here is the summary of changes.

You are about to add 3 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@cpriti-os cpriti-os added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 30, 2026
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 30, 2026
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
@bajajneha27 bajajneha27 changed the title storage(feat): Support bucket encryption config feat(storage): Support bucket encryption config Apr 2, 2026
end

it "deletes all encryption enforcement configs" do
# For the update, need to specify all three configs
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure if this statement is correct

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

got reference from Java pr

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@nidhiii-27 Is this correct statement?

# #
# storage = Google::Cloud::Storage.new
# bucket = storage.bucket "my-bucket"
# bucket.customer_managed_encryption_enforcement_config #=> Google::Apis::StorageV1::Bucket::Encryption::CustomerManagedEncryptionEnforcementConfig.new
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: are we also planning to provide the setter using a hash?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@bajajneha27 did not understand your concerns here
we are performing the operation as below
#set operation
customer_managed_config =
Google::Apis::StorageV1::Bucket::Encryption::CustomerManagedEncryptionEnforcementConfig.new(
restriction_mode: "NotRestricted"
)
customer_managed_encryption_enforcement_config = customer_managed_config

#get operation
bucket.customer_managed_encryption_enforcement_config
#<Google::Apis::StorageV1::Bucket::Encryption::CustomerManagedEncryptionEnforcementConfig:0x00007f64e9a69dc0
@effective_time=#<DateTime: 2026-04-02T10:09:23+00:00 ((2461133j,36563s,115000000n),+0s,2299161j)>,
@restriction_mode="FullyRestricted">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What I meant to ask is that, can we use the setter in following way as well:

bucket.customer_managed_config = { restriction_mode: "NotRestricted"

Because we usually do that in this codebase.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

modified the samples

# #
# storage = Google::Cloud::Storage.new
# bucket = storage.bucket "my-bucket"
# bucket.customer_managed_encryption_enforcement_config #=> Google::Apis::StorageV1::Bucket::Encryption::CustomerManagedEncryptionEnforcementConfig.new
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment is confusing. This is a getter method, so ideally we should be showing code sample for that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A gentle reminder on this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

Same comment as above.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

Same comment here as well

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

modified the samples

if_metageneration_match: nil,
if_metageneration_not_match: nil
if_metageneration_not_match: nil,
bucket_encryption_config: nil
Copy link
Copy Markdown
Contributor

@bajajneha27 bajajneha27 Apr 2, 2026

Choose a reason for hiding this comment

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

Why are we introducing new parameter here? Why can't we combine this in existing parameter attributes ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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::Encryption instance.
  • 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"
    }
  }
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@bajajneha27 can you please provide your views on this approach

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

@shubhangi-google shubhangi-google Apr 6, 2026

Choose a reason for hiding this comment

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

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

end

it "deletes all encryption enforcement configs" do
# For the update, need to specify all three configs
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@nidhiii-27 Is this correct statement?

# #
# storage = Google::Cloud::Storage.new
# bucket = storage.bucket "my-bucket"
# bucket.customer_managed_encryption_enforcement_config #=> Google::Apis::StorageV1::Bucket::Encryption::CustomerManagedEncryptionEnforcementConfig.new
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What I meant to ask is that, can we use the setter in following way as well:

bucket.customer_managed_config = { restriction_mode: "NotRestricted"

Because we usually do that in this codebase.

# #
# storage = Google::Cloud::Storage.new
# bucket = storage.bucket "my-bucket"
# bucket.customer_managed_encryption_enforcement_config #=> Google::Apis::StorageV1::Bucket::Encryption::CustomerManagedEncryptionEnforcementConfig.new
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

Same comment as above. This is a setter example

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

modified the samples

if_metageneration_match: nil,
if_metageneration_not_match: nil
if_metageneration_not_match: nil,
bucket_encryption_config: nil
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

bucket_with_key = Google::Cloud::Storage::Bucket.from_gapi bucket_gapi_with_key, storage.service
patch_bucket_gapi = Google::Apis::StorageV1::Bucket.new encryption: encryption_gapi(nil)
patch_bucket_gapi = Google::Apis::StorageV1::Bucket.new encryption: encryption_gapi(key_name: kms_key)
patch_bucket_gapi = Google::Apis::StorageV1::Bucket.new(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are we just overwriting the variable patch_bucket_gapi ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

was a mistake corrected it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants