Skip to content
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
merged 22 commits into from
Aug 30, 2024

Conversation

TomasMorton
Copy link
Contributor

@TomasMorton TomasMorton commented Jul 23, 2024

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

  • Restore without new args
    • This is equivalent to using USE_SOURCE_ENCRYPTION
  • Restore with GOOGLE_DEFAULT_ENCRYPTION using -e
  • Restore with GOOGLE_DEFAULT_ENCRYPTION using --encryption-type
  • Restore with USE_SOURCE_ENCRYPTION using -e
  • Restore with USE_SOURCE_ENCRYPTION using --encryption-type
  • Restore with CUSTOMER_MANAGED_ENCRYPTION using -e and -k
  • Restore with CUSTOMER_MANAGED_ENCRYPTION using --encryption-type and --kms-key-name

Failure scenarios

  • --encryption-type provided without a value
    • firebase firestore:databases:restore -d database -b backup -e
    • error: option '-e, --encryption-type <encryptionType>' argument missing
  • --encryption type provided with an invalid value
    • firebase firestore:databases:restore -d database -b backup -e NOT_AN_OPTION
    • Error: Invalid value for flag --encryption-type. See firebase firestore:databases:restore --help for more info.
  • --kms-key-name provided without a value
    • firebase firestore:databases:restore -d database -b backup -k
    • error: option '-k, --kms-key-name <kmsKeyName>' argument missing
  • --kms-key-name provided without an --encryption-type
    • firebase firestore:databases:restore -d database -b backup -k key
    • Error: --kms-key-name can only be set when specifying an --encryption-type of CUSTOMER_MANAGED_ENCRYPTION.
  • --kms-key-name provided with --encryption-type of GOOGLE_DEFAULT_ENCRYPTION
    • firebase firestore:databases:restore -d database -b backup -e GOOGLE_DEFAULT_ENCRYPTION -k key
    • Error: --kms-key-name can only be set when specifying an --encryption-type of CUSTOMER_MANAGED_ENCRYPTION.
  • --kms-key-name provided with --encryption-type of USE_SOURCE_ENCRYPTION
    • firebase firestore:databases:restore -d database -b backup -e USE_SOURCE_ENCRYPTION -k key
    • Error: --kms-key-name can only be set when specifying an --encryption-type of CUSTOMER_MANAGED_ENCRYPTION.
  • no --kms-key-name provided with --encryption type of CUSTOMER_MANAGED_ENCRYPTION
    • firebase firestore:databases:restore -d database -b backup -e CUSTOMER_MANAGED_ENCRYPTION
    • Error: --kms-key-name must be provided when specifying an --encryption-type of CUSTOMER_MANAGED_ENCRYPTION.

Server-side failure scenarios

  • Key is disabled
    • 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" } }.
  • IAM policy for the key does not grant access
    • 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

@TomasMorton TomasMorton force-pushed the tm-firestore-restore-encryption branch 2 times, most recently from b941249 to b439608 Compare July 25, 2024 17:11
@TomasMorton TomasMorton marked this pull request as ready for review July 25, 2024 17:12
@TomasMorton TomasMorton requested a review from rwhogg July 25, 2024 17:12
Copy link
Contributor

@rwhogg rwhogg left a 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:

  1. Can you test with --json for completeness sake?
  2. Make sure the help text is clear whether a full resource path or just an ID is appropriate for the KMS key?

@TomasMorton
Copy link
Contributor Author

Looks good, thanks!

2 comments:

  1. Can you test with --json for completeness sake?
  2. 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 TomasMorton force-pushed the tm-firestore-restore-encryption branch 3 times, most recently from 41419c4 to fbc84e4 Compare July 30, 2024 19:52
@TomasMorton TomasMorton force-pushed the tm-firestore-restore-encryption branch from 3697688 to a85152d Compare July 30, 2024 20:18
@TomasMorton TomasMorton marked this pull request as draft August 2, 2024 14:29
@TomasMorton TomasMorton force-pushed the tm-firestore-restore-encryption branch 2 times, most recently from 2077b41 to f6e63d2 Compare August 5, 2024 18:55
@TomasMorton TomasMorton marked this pull request as ready for review August 5, 2024 18:59
@TomasMorton TomasMorton marked this pull request as draft August 6, 2024 20:28
@TomasMorton TomasMorton force-pushed the tm-firestore-restore-encryption branch from f6e63d2 to 48ef195 Compare August 27, 2024 18:06
@TomasMorton TomasMorton marked this pull request as ready for review August 27, 2024 19:09
@TomasMorton TomasMorton force-pushed the tm-firestore-restore-encryption branch 2 times, most recently from db03446 to 44d0152 Compare August 27, 2024 22:11
@TomasMorton TomasMorton force-pushed the tm-firestore-restore-encryption branch from 44d0152 to 612a039 Compare August 30, 2024 16:19
@TomasMorton TomasMorton enabled auto-merge (squash) August 30, 2024 16:19
@TomasMorton TomasMorton merged commit 0d5f749 into master Aug 30, 2024
40 of 41 checks passed
@TomasMorton TomasMorton deleted the tm-firestore-restore-encryption branch August 30, 2024 16:47
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants