-
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 CMEK support in Firebase CLI for Firestore databases #7479
Conversation
2027667
to
3385e56
Compare
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.
LGTM with some small suggestions
@@ -25,6 +25,10 @@ export const command = new Command("firestore:databases:create <database>") | |||
"--point-in-time-recovery <enablement>", | |||
"Whether to enable the PITR feature on this database, for example 'ENABLED' or 'DISABLED'. Default is 'DISABLED'", | |||
) | |||
.option( | |||
"--kms-key-name <kmsKeyName>", | |||
"The resource ID of a Cloud KMS key. If set, the database created will be a Customer-managed Encryption Key (CMEK) database encrypted with this key. This feature is allowlist only in initial launch.", |
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.
Can you add a TODO with a buganizer link to clean this message up once its open to all?
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.
Sure, have added.
const databaseResp: types.DatabaseResp = await api.createDatabase( | ||
options.project, | ||
database, | ||
options.location, | ||
type, | ||
deleteProtectionState, | ||
pointInTimeRecoveryEnablement, | ||
cmekConfig, |
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.
Optional: Create database has 7 args - consider switching this to a parameter object (ie https://go/tott/550)
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.
Have switched to a param object, thanks
Could you add a CHANGELOG entry for this? |
abe465c
to
39533ac
Compare
Description
This lets Firebase CLI create a CMEK (Customer-managed encryption keys) Firestore database and view the key associated with a CMEK database.
--kms-key-name
option to create a CMEK database. (No change for Update because we currently don't support update a CMEK database's key)Kms Key Name
andActive Key Versions
for CMEK databases.Scenarios Tested
kms-key-name
provided, a CMEK database is created--kms-key-name
is misspelled--kms-key-name
is provided with an empty value--kms-key-name
optionKms Key Name
andActive Key Version
rowsKms Key Name
whenActive Key Version
is not available from backend yet right after database creation (it needs 10min to populate)Kms Key Name
andActive Key Version
rowsSample Commands
firebase firetore:databases:create <database>
# as before, to create a non-CMEK databasefirebase firetore:databases:create <database> --kms-key-name <key>