Update patch set 2
Patch Set 2: Code-Review-1
(10 comments)
Patch-set: 2
Reviewer: Gerrit User 597 <597@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Code-Review=-1, f0ab5139247ccced1c99fa5b18ed4cbc68336644
Attention: {"person_ident":"Gerrit User 597 \u003c597@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_597\u003e replied on the change"}
This commit is contained in:
Gerrit User 597
Gerrit Code Review
parent
af016f5340
commit
6b4020ac23
|
|
@ -67,6 +67,176 @@
|
|||
"message": "How does this interact with the backup import/export functionality? Does it work at all? Is it allowed for encrypted backups?",
|
||||
"revId": "0cfece2ed6af598b97959ae23024344f4817809f",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": false,
|
||||
"key": {
|
||||
"uuid": "8e181359_8e2b7181",
|
||||
"filename": "/PATCHSET_LEVEL",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 0,
|
||||
"author": {
|
||||
"id": 597
|
||||
},
|
||||
"writtenOn": "2024-02-17T02:02:23Z",
|
||||
"side": 1,
|
||||
"message": "I think that my own, Rajat\u0027s, and Christian\u0027s comments on the patchset 1 were somewhat useful. My comments this time (set 2) are partially redundant with set 1. But hopefully not as verbose.\n\nI heard no argument against allowing key rotation. This time I\u0027m a little more specific about the new configuration parameters. Note that I avoided an array. Not sure if this is really necessary.\n\nI wish to avoid a new column in the DB if at all possible.\n\nI\u0027m going to start with a pilot implementation now, and we can have both it and the spec adjusted as discussion progresses.",
|
||||
"revId": "0cfece2ed6af598b97959ae23024344f4817809f",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "9cc5334d_cb1bd072",
|
||||
"filename": "specs/2024.1/chunked-backup-encryption.rst",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 15,
|
||||
"author": {
|
||||
"id": 597
|
||||
},
|
||||
"writtenOn": "2024-02-17T02:02:23Z",
|
||||
"side": 1,
|
||||
"message": "\"may be undesirable\" surely",
|
||||
"revId": "0cfece2ed6af598b97959ae23024344f4817809f",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "ce1d6e9a_2775f621",
|
||||
"filename": "specs/2024.1/chunked-backup-encryption.rst",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 22,
|
||||
"author": {
|
||||
"id": 597
|
||||
},
|
||||
"writtenOn": "2024-02-17T02:02:23Z",
|
||||
"side": 1,
|
||||
"message": "just \"access to that information\" sounds better",
|
||||
"revId": "0cfece2ed6af598b97959ae23024344f4817809f",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "5c44f6a2_80aeb23f",
|
||||
"filename": "specs/2024.1/chunked-backup-encryption.rst",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 26,
|
||||
"author": {
|
||||
"id": 597
|
||||
},
|
||||
"writtenOn": "2024-02-17T02:02:23Z",
|
||||
"side": 1,
|
||||
"message": "No need to appeal to an undefined recommendation authority. Just plainly state that encrypting backups removes the hazard of direct access to raw backups at provider, and therefore we should implement it.",
|
||||
"revId": "0cfece2ed6af598b97959ae23024344f4817809f",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "70aa7eac_0f056afc",
|
||||
"filename": "specs/2024.1/chunked-backup-encryption.rst",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 32,
|
||||
"author": {
|
||||
"id": 597
|
||||
},
|
||||
"writtenOn": "2024-02-17T02:02:23Z",
|
||||
"side": 1,
|
||||
"message": "Overall, the problem definiton is much too verbose, IMHO. But it is correct.",
|
||||
"revId": "0cfece2ed6af598b97959ae23024344f4817809f",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "6e57743f_4e7d75b0",
|
||||
"filename": "specs/2024.1/chunked-backup-encryption.rst",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 46,
|
||||
"author": {
|
||||
"id": 597
|
||||
},
|
||||
"writtenOn": "2024-02-17T02:02:23Z",
|
||||
"side": 1,
|
||||
"message": "This was mentioned in the patchset 1 comments: not having a key rotation is a non-starter, IMHO. Static keys offer advantages in DR scenario, but let\u0027s at least have 2, at least for the restore side. I\u0027m concerned about a shift in a government mandate would require a weird workaround like running 2 Cinder backup services simultaneously.",
|
||||
"revId": "0cfece2ed6af598b97959ae23024344f4817809f",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "b56855a3_1e2b8999",
|
||||
"filename": "specs/2024.1/chunked-backup-encryption.rst",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 63,
|
||||
"author": {
|
||||
"id": 597
|
||||
},
|
||||
"writtenOn": "2024-02-17T02:02:23Z",
|
||||
"side": 1,
|
||||
"message": "I think we\u0027ll need to have backup_decryption_key_a and backup_decryption_key_b, with a syntax \"fernet:NNNNNNNN\". Other equivalent configuration parameters are possible, I\u0027m not wedded to the specifics.",
|
||||
"revId": "0cfece2ed6af598b97959ae23024344f4817809f",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "a905c3bb_8ad6dea0",
|
||||
"filename": "specs/2024.1/chunked-backup-encryption.rst",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 87,
|
||||
"author": {
|
||||
"id": 597
|
||||
},
|
||||
"writtenOn": "2024-02-17T02:02:23Z",
|
||||
"side": 1,
|
||||
"message": "In that case you can save a fingerprint of the key in the metadata too. Sounds like good news for my demand of supporting 2 or more decryption keys.",
|
||||
"revId": "0cfece2ed6af598b97959ae23024344f4817809f",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "c78b7c62_a645e0db",
|
||||
"filename": "specs/2024.1/chunked-backup-encryption.rst",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 131,
|
||||
"author": {
|
||||
"id": 597
|
||||
},
|
||||
"writtenOn": "2024-02-17T02:02:23Z",
|
||||
"side": 1,
|
||||
"message": "I\u0027m not sure if this is necessary. Keeping this information in DB allows operators to list backups and obtain the information about the encryption. So that much is good (although it needs to be weighed against accessing the metadata in S3 and getting the truth from the source). But do we not have a better space to keep this than a whole new column?",
|
||||
"revId": "0cfece2ed6af598b97959ae23024344f4817809f",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "ca986e6a_636776b8",
|
||||
"filename": "specs/2024.1/chunked-backup-encryption.rst",
|
||||
"patchSetId": 2
|
||||
},
|
||||
"lineNbr": 173,
|
||||
"author": {
|
||||
"id": 597
|
||||
},
|
||||
"writtenOn": "2024-02-17T02:02:23Z",
|
||||
"side": 1,
|
||||
"message": "The impact upon migrations and upgrades is why I wish to explore ways to live without schema changes, and the consequences (e.g. extra access to S3).",
|
||||
"revId": "0cfece2ed6af598b97959ae23024344f4817809f",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
}
|
||||
]
|
||||
}
|
||||
Loading…
Reference in New Issue