Update patch set 2

Patch Set 2: Code-Review-1

(9 comments)

Patch-set: 2
Reviewer: Gerrit User 27615 <27615@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Code-Review=-1, 7853a84186431a86f70374036486e763467f13ea
Attention: {"person_ident":"Gerrit User 28271 \u003c28271@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_27615\u003e replied on the change"}
This commit is contained in:
Gerrit User 27615 2024-05-08 15:30:40 +00:00 committed by Gerrit Code Review
parent d0fc8abb8e
commit 0c31386142
2 changed files with 200 additions and 0 deletions

View File

@ -0,0 +1,182 @@
{
"comments": [
{
"unresolved": false,
"key": {
"uuid": "cd1e8fbe_7ff14734",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 2
},
"lineNbr": 0,
"author": {
"id": 27615
},
"writtenOn": "2024-05-08T15:30:40Z",
"side": 1,
"message": "few comments inline, most of them are just queries to understand the feature and impact better.\nThe major concern i have is the encryption/decryption happening at the OSC layer. maybe it\u0027s just fine but it seems really strange to have a major part of the feature done in the client instead of the targeted service (i.e. glance or glance_store)",
"revId": "b080d6628f0b1b9f9ac650cf5359a0ce9d91b469",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "487eb1a9_dcc39aee",
"filename": "specs/2024.2/approved/glance/standardized_image_encryption.rst",
"patchSetId": 2
},
"lineNbr": 64,
"author": {
"id": 27615
},
"writtenOn": "2024-05-08T15:30:40Z",
"side": 1,
"message": "I\u0027m unsure if we want to have this capability in OSC since I only see OSC as a tool to provide CLI interface to users and not actually perform functionality for any API or resource.\nStephen can comment better on it so i think it\u0027s good to have him included in the discussion.\n\nAlso why are we not doing it in the glance or glance_store codebase? glance_store does the actual upload of the image into the backend in chunks so it should be feasible to encrypt it at that time right?",
"range": {
"startLine": 63,
"startChar": 52,
"endLine": 64,
"endChar": 68
},
"revId": "b080d6628f0b1b9f9ac650cf5359a0ce9d91b469",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "73db1257_df734752",
"filename": "specs/2024.2/approved/glance/standardized_image_encryption.rst",
"patchSetId": 2
},
"lineNbr": 78,
"author": {
"id": 27615
},
"writtenOn": "2024-05-08T15:30:40Z",
"side": 1,
"message": "again I\u0027m unsure if we want to delegate the decryption task to the client ...",
"range": {
"startLine": 77,
"startChar": 39,
"endLine": 78,
"endChar": 64
},
"revId": "b080d6628f0b1b9f9ac650cf5359a0ce9d91b469",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "3398bc4d_612aab3b",
"filename": "specs/2024.2/approved/glance/standardized_image_encryption.rst",
"patchSetId": 2
},
"lineNbr": 92,
"author": {
"id": 27615
},
"writtenOn": "2024-05-08T15:30:40Z",
"side": 1,
"message": "what is the use case of keeping the key after the image is deleted?\nedit: after reading L#140, looks like we are delegating the task from cinder/nova to glance for deleting the key but I still don\u0027t understand the case where this parameter is set to NO and we delete the image and the key remains in barbican, then how will nova/cinder delete the key? or if there is another use case for that key after the image is gone",
"range": {
"startLine": 91,
"startChar": 65,
"endLine": 92,
"endChar": 23
},
"revId": "b080d6628f0b1b9f9ac650cf5359a0ce9d91b469",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "5474d7e7_c541324c",
"filename": "specs/2024.2/approved/glance/standardized_image_encryption.rst",
"patchSetId": 2
},
"lineNbr": 112,
"author": {
"id": 27615
},
"writtenOn": "2024-05-08T15:30:40Z",
"side": 1,
"message": "i think we are referring to the upload volume to image feature here, so we could upload encrypted volumes as images.",
"range": {
"startLine": 112,
"startChar": 3,
"endLine": 112,
"endChar": 63
},
"revId": "b080d6628f0b1b9f9ac650cf5359a0ce9d91b469",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "e8bf674f_767ea3ba",
"filename": "specs/2024.2/approved/glance/standardized_image_encryption.rst",
"patchSetId": 2
},
"lineNbr": 114,
"author": {
"id": 27615
},
"writtenOn": "2024-05-08T15:30:40Z",
"side": 1,
"message": "this looks a little ambiguous to me, do we mean we can create a bootable volume from such an image? what happens if i create an unencrypted volume from an image that is created by uploading an encrypted volume?\n\n1. create encrypted volume\n2. upload encrypted volume to image\n3. create unencrypted volume from the image -- what happens in this case?",
"range": {
"startLine": 113,
"startChar": 47,
"endLine": 114,
"endChar": 29
},
"revId": "b080d6628f0b1b9f9ac650cf5359a0ce9d91b469",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "ecf5e39e_1f47994d",
"filename": "specs/2024.2/approved/glance/standardized_image_encryption.rst",
"patchSetId": 2
},
"lineNbr": 187,
"author": {
"id": 27615
},
"writtenOn": "2024-05-08T15:30:40Z",
"side": 1,
"message": "good to have an example of payload for cinder/nova reference as to which properties needs to be provided",
"range": {
"startLine": 186,
"startChar": 52,
"endLine": 187,
"endChar": 16
},
"revId": "b080d6628f0b1b9f9ac650cf5359a0ce9d91b469",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "64f3c370_d34cbc82",
"filename": "specs/2024.2/approved/glance/standardized_image_encryption.rst",
"patchSetId": 2
},
"lineNbr": 247,
"author": {
"id": 27615
},
"writtenOn": "2024-05-08T15:30:40Z",
"side": 1,
"message": "do we have other key manager support in OpenStack? and do they support the requirements of this feature? not necessary but it would simplify the spec if we just mention Barbican (specific project) instead of key manager (generic term) but that\u0027s just a suggestion and I\u0027m not very inclined on making that change",
"range": {
"startLine": 247,
"startChar": 23,
"endLine": 247,
"endChar": 31
},
"revId": "b080d6628f0b1b9f9ac650cf5359a0ce9d91b469",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
}
]
}

View File

@ -273,6 +273,24 @@
"message": "A comment for the reviewers from Nova and Cinder:\n\nNova uses passphrases while Cinder uses keys and hexlifies them. In Glance we will only store the reference of the secret (id of key or passphrase). When retrieving the secret from Barbican in Cinder or Nova, there needs to be a different handling implemented for passphrases and keys.\n\nThis will and cannot be part of the Glance implementation.",
"revId": "fc0e4cd737cf50ffcfb4cac02e000bad544d847a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "9f3b0daf_93b1618c",
"filename": "specs/2024.2/approved/glance/standardized_image_encryption.rst",
"patchSetId": 1
},
"lineNbr": 294,
"author": {
"id": 27615
},
"writtenOn": "2024-05-08T15:30:40Z",
"side": 1,
"message": "since we agreed to do a cinder spec, this seems like a very good point to mention there.",
"parentUuid": "bc7b65ae_9ac072cc",
"revId": "fc0e4cd737cf50ffcfb4cac02e000bad544d847a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
}
]
}