-
Notifications
You must be signed in to change notification settings - Fork 950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for setting the encryption configuration of restored firestore databases #7483
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TomasMorton
force-pushed
the
tm-firestore-restore-encryption
branch
2 times, most recently
from
July 25, 2024 17:11
b941249
to
b439608
Compare
rwhogg
approved these changes
Jul 26, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
2 comments:
- Can you test with --json for completeness sake?
- Make sure the help text is clear whether a full resource path or just an ID is appropriate for the KMS key?
Thanks for the review. I've tested everything with --json. Updated to say "path" not "ID", thanks for catching. |
TomasMorton
force-pushed
the
tm-firestore-restore-encryption
branch
3 times, most recently
from
July 30, 2024 19:52
41419c4
to
fbc84e4
Compare
TomasMorton
force-pushed
the
tm-firestore-restore-encryption
branch
from
July 30, 2024 20:18
3697688
to
a85152d
Compare
TomasMorton
force-pushed
the
tm-firestore-restore-encryption
branch
2 times, most recently
from
August 5, 2024 18:55
2077b41
to
f6e63d2
Compare
TomasMorton
force-pushed
the
tm-firestore-restore-encryption
branch
from
August 27, 2024 18:06
f6e63d2
to
48ef195
Compare
TomasMorton
force-pushed
the
tm-firestore-restore-encryption
branch
2 times, most recently
from
August 27, 2024 22:11
db03446
to
44d0152
Compare
joehan
approved these changes
Aug 30, 2024
TomasMorton
force-pushed
the
tm-firestore-restore-encryption
branch
from
August 30, 2024 16:19
44d0152
to
612a039
Compare
hlshen
pushed a commit
that referenced
this pull request
Sep 3, 2024
…store databases (#7483) * Add EncryptionConfig API type * Add encryptionType command option for GOOGLE_DEFAULT_ENCRYPTION * Accept and parse option for GOOGLE_DEFAULT_ENCRYPTION * Set encryption config values in the API request * Add support for USE_BACKUP_ENCRYPTION * Fix casing of helpCommandText constant * Add support for CUSTOMER_MANAGED_ENCRYPTION * Format changes * Add missing bracket in encryption type info * Don't allow an empty encryption type * Validate if kms-key-name is set at the appropriate time * Use enum for encryption type * Throw an error if a key is set without an encryption type * Improve error messages * Specify that USE_BACKUP_ENCRYPTION is the default * Move undefined option into the switch statement * Improve help text * Update --kms-key-name to match new arg in create db * Remove duplicate `kmsKeyName` declaration. * Update help text for --kms-key-name * Use new API fields * Update changelog for firestore restore with encryption config
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Add support for setting the encryption configuration of the new database in
firebase:databases:restore
.Note: This is the same PR as #7463, but not using a fork.
Scenarios Tested
Success scenarios
Failure scenarios
error: option '-e, --encryption-type <encryptionType>' argument missing
Error: Invalid value for flag --encryption-type. See firebase firestore:databases:restore --help for more info.
error: option '-k, --kms-key-name <kmsKeyName>' argument missing
Error: --kms-key-name can only be set when specifying an --encryption-type of CUSTOMER_MANAGED_ENCRYPTION.
Error: --kms-key-name can only be set when specifying an --encryption-type of CUSTOMER_MANAGED_ENCRYPTION.
Error: --kms-key-name can only be set when specifying an --encryption-type of CUSTOMER_MANAGED_ENCRYPTION.
Error: --kms-key-name must be provided when specifying an --encryption-type of CUSTOMER_MANAGED_ENCRYPTION.
Server-side failure scenarios
Error: HTTP Error: 400, The customer-managed encryption key required by the requested resource is not accessible. Error reason: generic::failed_precondition: projects/tlmorton-cmek/locations/us-central1/keyRings/key-ring-1/cryptoKeys/bulk-key-99/cryptoKeyVersions/1 is not enabled, current state is: DISABLED. [google.rpc.error_details_ext] { message: "projects/tlmorton-cmek/locations/us-central1/keyRings/key-ring-1/cryptoKeys/bulk-key-99/cryptoKeyVersions/1 is not enabled, current state is: DISABLED." details { [type.googleapis.com/google.rpc.PreconditionFailure] { violations { type: "KEY_DISABLED" subject: "projects/tlmorton-cmek/locations/us-central1/keyRings/key-ring-1/cryptoKeys/bulk-key-99/cryptoKeyVersions/1" } } } details { [type.googleapis.com/google.rpc.DebugInfo] { } } } [cloud_kms.FreshnessTokenExtension] { proto_freshness_token { name: "projects/tlmorton-cmek/locations/us-central1/keyRings/key-ring-1/cryptoKeys/bulk-key-99/grants/0a2aab3f2456ad16" ckv_update_timestamp_micros: 1721845906568431 key_version_name: "projects/tlmorton-cmek/locations/us-central1/keyRings/key-ring-1/cryptoKeys/bulk-key-99/cryptoKeyVersions/1" } }.
Error: HTTP Error: 400, The provided cmek key does not exist, or does not exist in the same location as the database (please find more details in https://cloud.google.com/firestore/docs/use-cmek#create-key), or the service account does not have cloudkms.cryptoKeyVersions.useToEncrypt permission on it.
Sample Commands
firebase firestore:databases:restore -d database -b backup
firebase firestore:databases:restore -d database -b backup -e GOOGLE_DEFAULT_ENCRYPTION
firebase firestore:databases:restore -d database -b backup --encryption-type USE_SOURCE_ENCRYPTION
firebase firestore:databases:restore -d database -b backup -e CUSTOMER_MANAGED_ENCRYPTION -k key