Skip to content

Add 'Bucket.default_kms_encryption_key' property.#5222

Merged
tseaver merged 3 commits into
storage-support_kms_keys-wipfrom
storage-support_kms_keys-bucket_default
Apr 26, 2018
Merged

Add 'Bucket.default_kms_encryption_key' property.#5222
tseaver merged 3 commits into
storage-support_kms_keys-wipfrom
storage-support_kms_keys-bucket_default

Conversation

@tseaver
Copy link
Copy Markdown
Contributor

@tseaver tseaver commented Apr 23, 2018 •
edited by dpebot
Loading

No description provided.

@tseaver tseaver added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. api: storage Issues related to the Cloud Storage API. labels Apr 23, 2018
@tseaver tseaver requested review from frankyn and theacodes April 23, 2018 20:12
@tseaver tseaver requested a review from lukesneeringer as a code owner April 23, 2018 20:12
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 23, 2018
@tseaver tseaver force-pushed the storage-support_kms_keys-bucket_default branch from ee9afaa to dc342a7 Compare April 23, 2018 21:33
frankyn
frankyn suggested changes Apr 24, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@frankyn frankyn left a comment

Choose a reason for hiding this comment

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

@tseaver, I notice that tests are not calling the service. Could you add tests which exercise the client library with GCS?

Additionally, for default_kms_key_name, I'm expecting to see an integration test which does the following:

  1. uploads an object to a bucket with a default kms key name
  2. verifies the default kms key name was used as the kms key name for the object
  3. download to verify contents

Comment thread storage/google/cloud/storage/bucket.py

See https://cloud.google.com/storage/docs/json_api/v1/buckets

:setter: Set default KMS encryption key for items in this bucket.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread storage/google/cloud/storage/bucket.py Outdated

@default_kms_encryption_key.setter
def default_kms_encryption_key(self, value):
"""Set default KMS encryption key for items in the bucket.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tseaver
Copy link
Copy Markdown
Contributor Author

tseaver commented Apr 26, 2018

@frankyn I planned to do the system tests in an additional PR (once we ironed out the API surface).

@frankyn
Copy link
Copy Markdown
Contributor

frankyn commented Apr 26, 2018

Thanks for clarifying!

@tseaver
Copy link
Copy Markdown
Contributor Author

tseaver commented Apr 26, 2018

@frankyn I also realized that default_kms_encryption_key was a poor name for the property: it should match the API name, which fits the semantics better (not always the case :). dad38b4 updates it to default_kms_key_name.

@tseaver
Copy link
Copy Markdown
Contributor Author

tseaver commented Apr 26, 2018

@dpebot please merge when green.

@dpebot
Copy link
Copy Markdown
Contributor

dpebot commented Apr 26, 2018

Okay! I'll merge when all statuses are green and all reviewers approve.

@dpebot dpebot added the automerge Merge the pull request once unit tests and other checks pass. label Apr 26, 2018
@dpebot dpebot self-assigned this Apr 26, 2018
@tseaver
Copy link
Copy Markdown
Contributor Author

tseaver commented Apr 26, 2018

Merging in spite of unrelated logging system test failures.

@tseaver tseaver merged commit 71e447f into storage-support_kms_keys-wip Apr 26, 2018
@tseaver tseaver deleted the storage-support_kms_keys-bucket_default branch April 26, 2018 20:53
tseaver added a commit that referenced this pull request May 9, 2018
@tseaver
Add 'Bucket.default_kms_key_name' property. (#5222)
8d3d3db
tseaver added a commit that referenced this pull request May 9, 2018
@tseaver
Storage: add support for KMS keys (#5259)
38c3e9a 
* Add support for blob-level KMS encryption keys (#5221)

* Add 'Bucket.default_kms_key_name' property. (#5222)

* Expose 'Blob.kms_key_name' as read-only property (#5249)

* Add system tests for GCS-KMS integration (#5257)
parthea pushed a commit that referenced this pull request Mar 9, 2026
@tseaver
Storage: add support for KMS keys (#5259)
592ca82 
* Add support for blob-level KMS encryption keys (#5221)

* Add 'Bucket.default_kms_key_name' property. (#5222)

* Expose 'Blob.kms_key_name' as read-only property (#5249)

* Add system tests for GCS-KMS integration (#5257)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lukesneeringer lukesneeringer Awaiting requested review from lukesneeringer

2 more reviewers

@theacodes theacodes theacodes approved these changes

@frankyn frankyn frankyn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dpebot dpebot

Labels

api: storage Issues related to the Cloud Storage API. automerge Merge the pull request once unit tests and other checks pass. cla: yes This human has signed the Contributor License Agreement. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tseaver @frankyn @dpebot @theacodes @googlebot