feat: customer-managed encryption keys #1274
Conversation
product-auto-label
bot
added
the
api: spanner
google-cla
bot
added
the
cla: yes
|
Here is the summary of changes. You are about to add 3 region tags.
This comment is generated by snippet-bot.
|
Codecov Report
@@ Coverage Diff @@
## master #1274 +/- ##
=======================================
Coverage 98.59% 98.60%
=======================================
Files 23 23
Lines 21841 21899 +58
Branches 1094 1099 +5
=======================================
+ Hits 21535 21593 +58
Misses 297 297
Partials 9 9
Continue to review full report at Codecov.
|
|
PTAL @olavloite |
|
|
||
| const request = { | ||
| encryptionConfig: { | ||
| kmsKeyName: keyName, |
There was a problem hiding this comment.
Shouldn't this also include encryptionType: 'CUSTOMER_MANAGED_ENCRYPTION',? Or is it only required for backups?
There was a problem hiding this comment.
Yeah this is confusing. But CreateDatabaseRequest has an EncryptionConfig field which only lets you specify the kms_key_name.
There was a problem hiding this comment.
I think this might be because by default, it uses GOOGLE_DEFAULT_ENCRYPTION. But if the customer specifies an encryption key, then it has to be CUSTOMER_MANAGED_ENCRYPTION.
| @@ -893,6 +1013,29 @@ describe('Spanner', () => { | |||
| ); | |||
| }); | |||
|
|
|||
| // restore_backup_with_encryption_key | |||
| it('should restore database from a backup using an encryption key', async function () { | |||
There was a problem hiding this comment.
For the Java client we have merged all backup/restore system tests into one as backups and restores are very slow. Is that something that we should consider for Node as well?
There was a problem hiding this comment.
I wish we could do that but I guess the samples were designed such that we take a separate backup and do a separate restore for CMEK. That means I have to test them separately here :( But if it becomes too flaky over time, then I may have to end up skipping these.
| * const [, restoreOperation] = await database.restore( | ||
| * backupName, | ||
| * { | ||
| * encryptionConfig: { |
There was a problem hiding this comment.
Depending on how often we think that users will use encrypted backups, I would consider leaving the encryption information out of this example. As it is now, it at first hand seems to be something that is required for all restore operations, while it is only required if you actually have an encrypted backup.
There was a problem hiding this comment.
Yeah, I think it's reasonable to remove this as it's not something you have to specify by default. Sometimes we have multiple examples so I may consider adding a separate one.
BTW, for restoring with an encryption key, it's not a requirement to specify the encryption key if the backup was encrypted. If the backup was encrypted, then the restored database is automatically encrypted with the same key as the backup. You only have to specify the key here if you want to encrypt it with a different key.
skuruppu
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
Supports creating databases and backups, and restoring databases with a customer-managed encryption key.