)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"24b8bdfda5919323fc52d830b35bc8a3bdb3a2ae","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4724a3a1_6e86057d","updated":"2022-05-10 21:20:17.000000000","message":"Some questions and suggestions for clarifications noted inline.","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18d72f9a7ee5ba743675c8573f1708d399ac65ee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"83885cbe_57209b0b","updated":"2022-05-12 09:21:39.000000000","message":"Thanks for the review Brian. Please see my replies inline.","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"d37bd77222a598da604dc2304bdd1eed69f193fb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3777f506_093c0f7a","updated":"2022-05-13 12:20:51.000000000","message":"I am very much against this proposal. Most of my serious concerns are in the comments of the various \"impact\" sections.\n\nIn short having \"admin only\" API which use case is service-service interaction opens such a massive can of worms we cannot afford to deal with.\n\nSecond part of my concern is related to the consuming services. It took years for Cinder and Nova to adopt Images API v2, I don\u0027t expect anything less for this meaning that we would need to carry both approaches in the codebase for foreseeable future.\n\nMost of the issues that this proposal is trying to address could be addressed by adopting service tokens with minimal changes in the consuming services (If I understand correctly, both Cinder and Nova has the concept already in place, so this would be matter of including the service token to the calls rather than refactoring all the handling) and maintaining the authorisation of any said activities in Glance with full traceability.\n\nShould this proposal move away from the admin-only nature of service-service communication, I\u0027m more than happy to lift my -2 and continue discussion.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"3dd9051d980767edae52055d474284a7e09dfc70","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ab0b179e_c31909f0","updated":"2022-05-17 08:02:05.000000000","message":"Thanks Erno for the review.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0fb784de3c7c2a49f30471e3ef0f8b58c5358e7a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0e4aff7e_7d17ff2c","updated":"2022-05-16 11:34:59.000000000","message":"Thanks Erno, Please find my replies inline.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0fb784de3c7c2a49f30471e3ef0f8b58c5358e7a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d2ec6e1e_75b33fbb","in_reply_to":"3777f506_093c0f7a","updated":"2022-05-16 11:34:59.000000000","message":"Hi Erno,\n\n\u003e In short having \"admin only\" API which use case is service-service interaction opens such a massive can of worms we cannot afford to deal with.\n\nThe use case is service to service interaction but we also support add/update/delete locations via image-update call which we still need a way to do hence the location APIs. Most of the logic around this already exists but it has a lot of nested conditionals that makes the code readability poor and also we have ``show_multiple_locations`` option that we want to remove which is an essential condition in the image-update location code.\nAlso we will update this code to use service role which will reduce the authorization of another service using locations like Cinder.\n\n\n\u003e Second part of my concern is related to the consuming services. It took years for Cinder and Nova to adopt Images API v2, I don\u0027t expect anything less for this meaning that we would need to carry both approaches in the codebase for foreseeable future.\n\nOur plan is to remove the existing code in favor of the new APIs.\n``show_multiple_locations`` is a deprecated option since Newton so deployments should not be using it and if they are, they should expect things to break when a deprecated option is removed. I think currently, only users using the optimizations for the glance cinder interaction set this config option to True which we will already handle on the Cinder side.\n\n\u003e Most of the issues that this proposal is trying to address could be addressed by adopting service tokens with minimal changes in the consuming services (If I understand correctly, both Cinder and Nova has the concept already in place, so this would be matter of including the service token to the calls rather than refactoring all the handling) and maintaining the authorisation of any said activities in Glance with full traceability.\n\nI\u0027m not sure how service token is helpful in this case? Service tokens are used to renew/refresh the token\u0027s life for long running operations and if you follow the keystone doc[1], the receiver of service token has no idea about it so I\u0027ve no idea how it helps in this case.\nIf you\u0027re referring to the service role in keystone, that hasn\u0027t been implemented yet and we can\u0027t delay every feature depending on that.\n\n\u003e Should this proposal move away from the admin-only nature of service-service communication, I\u0027m more than happy to lift my -2 and continue discussion.\n\nAs said above, doing operations on locations is not only required by services but will also be required by admins so it\u0027s not only focusing on the cinder/nova use case but also providing set of APIs to make location add/remove/update easier.\n\n[1] https://docs.openstack.org/cinder/latest/configuration/block-storage/service-token.html","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5435dcfb5cb2bedd5fec92d7a238f2911a7f39d3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"475abc78_361123b2","updated":"2022-05-18 13:50:10.000000000","message":"Revisions look good, some suggestions noted inline.  I didn\u0027t address Erno\u0027s larger concerns about whether this API should exist at all; that might be better discussed at the weekly glance meeting.","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7ca97d7f03894c4ca4194852b452b60268a69a97","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"39725c85_c36f59c6","updated":"2022-05-19 11:16:19.000000000","message":"Thanks Brian","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ce0c71e4893eddfe1a45ec9109b6c247187b053b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"c477197c_644fd780","updated":"2022-05-23 18:28:50.000000000","message":"Thanks Dan for the review. Few comments inline and I will update the proposal to move from admin only APIs to service role.","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"084e134b817d6399d64ab06ac57af137d80476e0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"25e9e9f8_776c26d4","updated":"2022-05-19 18:10:35.000000000","message":"We definitely need to socialize this with the other projects before we commit. Presumably cinder is on board, but there needs to be a nova spec I think. I don\u0027t anticipate a lot of pushback on it, especially since it will make glance similar to how we interact with the other services. Do you plan to write that?","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c37df3b2c493b110d09a7b4bc4c6aa7f3f39b483","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"6182e793_715ff7fa","in_reply_to":"25e9e9f8_776c26d4","updated":"2022-05-23 18:38:36.000000000","message":"Since the new glance location APIs are going to be service specific and the only change in services like nova and cinder will be in the glance code calling glanceclient methods like add_location[1] in nova, I think this should be a fairly minimal change and should not introduce a new microversion for the same (since the calls are internal and nothing changes on the request/response interface). IMO a spec will be overkill (unless I\u0027m missing something) for this type of minimal change which are already described in the glance spec. what do you think?\n\n[1] https://github.com/openstack/nova/blob/master/nova/image/glance.py#L561-L562","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1f6d8c35182ca1174ad0db8fae314fb44522b44a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"8901b0cb_557bcc16","in_reply_to":"6182e793_715ff7fa","updated":"2022-05-24 13:49:59.000000000","message":"I was definitely thinking that a nova spec would be required, but re-reading your comments below.... maybe this _will_ be fully contained within glanceclient? I guess it seems like we do a lot of our own logic in nova around this so I was assuming it would need to change, but maybe I\u0027m wrong. I haven\u0027t looked at that code in a while.","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18979951baaa9828097ea848d3e3d490480b4e76","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"f173b518_cd5be3e7","in_reply_to":"8901b0cb_557bcc16","updated":"2022-05-24 15:19:00.000000000","message":"I\u0027m also not super familiar with the nova code around this but I assumed it would be similar to cinder where we just call glanceclient and it does the work so the main changes will be the way we call glanceclient for location operations.","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f64df97ea77263a8dd9040b4636ccaf4c177c9be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"1523fdfb_64453462","updated":"2022-06-08 05:30:03.000000000","message":"Thanks Erno","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ab2369798e1f8bc28720e83d443c8bcf7ec6c9f0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"d38bc380_7b5cfdd5","updated":"2022-06-08 07:22:29.000000000","message":"Thanks Abhishek","commit_id":"5f1944543590c60393ae45f1c0242f9b798acd5a"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"2052edf381d8d88025e99a4278f571b67a0efa7b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"2f107040_9b7c7de7","updated":"2022-06-08 12:24:35.000000000","message":"Thanks Rajat!\n\nI\u0027m starting to like this more. I think dropping the extra calls that were just providing parity, but not necessarily important for the usecase, makes this much cleaner and less scary beast (we\u0027re not implementing all the same issues we used to and call it different thing). We can always extend the scope if there is use/need for it but once the API is released, it\u0027s pain to get rid of.\n\nThanks for being patient with this and iterating over the feedback.","commit_id":"5f1944543590c60393ae45f1c0242f9b798acd5a"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"fb6fe9f38979af079d390338dfc47d4fa1261ebc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"2807d24c_60f4eb82","updated":"2022-06-09 23:18:55.000000000","message":"A few questions, thanks for the work on this!","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6d4ef8c055243a5985cc528d52a551e455b796ae","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"6cddecbe_78dfdd23","updated":"2022-06-10 07:07:07.000000000","message":"Thanks Dan and Cyril for the reviews.","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c0ad8ed4bec162bdfcdfef92aabaecda7d85974c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"3e2cd3e6_5fb2b418","updated":"2022-06-13 16:42:59.000000000","message":"Nothing major from me.","commit_id":"c2fbc80968496044e0e8dfb858fab00bcba97844"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"6e3edff4a735abca558023e330691c6446f7564b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"d7eaec75_30a36974","updated":"2022-06-14 03:21:58.000000000","message":"Thanks for the answers, I\u0027m just updating my question about the error codes that might be returned when adding a location.","commit_id":"c2fbc80968496044e0e8dfb858fab00bcba97844"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"450afc87272ca3007483ed6a6473a4272cd384bc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"c2281379_4ffa789f","updated":"2022-06-15 05:15:15.000000000","message":"Looks good me just one comment inside, once it is addressed I will put +2 from my side.","commit_id":"2b92aebb19773cc97f974949c6e2c317aa9e3a59"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"a0794b0982c2774c806dfb670647f81ba546e45c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"a500a94d_b2ea95fe","updated":"2022-06-15 07:12:29.000000000","message":"Thanks Abhishek, please find my reply inline","commit_id":"2b92aebb19773cc97f974949c6e2c317aa9e3a59"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"4d669b6b15926bdf271fb998f3e6e6bea6275e30","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"2c63374a_fc95944c","updated":"2022-06-14 05:57:48.000000000","message":"Thanks Cyril and Erno","commit_id":"2b92aebb19773cc97f974949c6e2c317aa9e3a59"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5a9ea488aafc7c89cec0e8b86ef7b26e1766c239","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"ba358b00_7db0c983","updated":"2022-06-15 16:49:26.000000000","message":"I think this is good enough to merge to get started on the glance side. I think there are still nova (and maybe cinder?) details to work out, but I think the glance plan is pretty clear at the moment and since this is a multi-cycle dance we should get it rolling. At least it\u0027s now been socialized on both sides of each of those seams and people agree this is a reasonable approach.","commit_id":"1184422804fd51260654aa60b21017a6355863de"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"cea59415e7490e0c2836a0e2978757adb41004e2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"3404ac6f_f50b719b","updated":"2022-06-17 19:09:35.000000000","message":"I\u0027m OK with this proposal.  My preference would be to move all location handling to this new API, so I agree with Dan that it would be good to implement the location uuid now so the GET response won\u0027t have to change when delete and update are added later.  But if we think all we really need to kill off OSSN-0065 is create and get-all, then I\u0027m all for it.","commit_id":"1184422804fd51260654aa60b21017a6355863de"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"73873bdc79b99cc1f24f355ede232e20e40fd329","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"221e5d9e_c771e145","updated":"2022-06-15 08:00:17.000000000","message":"Looks good to me, thank you!!","commit_id":"1184422804fd51260654aa60b21017a6355863de"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8fe08772ea6f4dc1e5cee1ea806943c512adae52","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"f8de4b0f_8fb337e1","updated":"2022-06-15 16:31:47.000000000","message":"Nothing really holding on my side, but we still need to paper out the details in Nova in order to make sure operators are able to upgrade without changing first their config options.","commit_id":"1184422804fd51260654aa60b21017a6355863de"}],"specs/zed/approved/glance/new-location-info-apis.rst":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"24b8bdfda5919323fc52d830b35bc8a3bdb3a2ae","unresolved":true,"context_lines":[{"line_number":14,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Currently we have a security vulnerability with ``show_multiple_locations``"},{"line_number":17,"context_line":"config option, OSSN-65 [1]_, due to which, if we enable"},{"line_number":18,"context_line":"``show_multiple_locations`` and the policies for add/update"},{"line_number":19,"context_line":"(set_image_location), get (get_image_location) and remove"},{"line_number":20,"context_line":"(delete_image_location) locations are set for non-admins then non-admin users"}],"source_content_type":"text/x-rst","patch_set":1,"id":"fc1b5b4f_13eb97bf","line":17,"range":{"start_line":17,"start_character":20,"end_line":17,"end_character":22},"updated":"2022-05-10 21:20:17.000000000","message":"0065","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18d72f9a7ee5ba743675c8573f1708d399ac65ee","unresolved":false,"context_lines":[{"line_number":14,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Currently we have a security vulnerability with ``show_multiple_locations``"},{"line_number":17,"context_line":"config option, OSSN-65 [1]_, due to which, if we enable"},{"line_number":18,"context_line":"``show_multiple_locations`` and the policies for add/update"},{"line_number":19,"context_line":"(set_image_location), get (get_image_location) and remove"},{"line_number":20,"context_line":"(delete_image_location) locations are set for non-admins then non-admin users"}],"source_content_type":"text/x-rst","patch_set":1,"id":"1f5f83fd_43e49885","line":17,"range":{"start_line":17,"start_character":20,"end_line":17,"end_character":22},"in_reply_to":"fc1b5b4f_13eb97bf","updated":"2022-05-12 09:21:39.000000000","message":"Done","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"24b8bdfda5919323fc52d830b35bc8a3bdb3a2ae","unresolved":true,"context_lines":[{"line_number":17,"context_line":"config option, OSSN-65 [1]_, due to which, if we enable"},{"line_number":18,"context_line":"``show_multiple_locations`` and the policies for add/update"},{"line_number":19,"context_line":"(set_image_location), get (get_image_location) and remove"},{"line_number":20,"context_line":"(delete_image_location) locations are set for non-admins then non-admin users"},{"line_number":21,"context_line":"can modify location data to corrupt the image."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"The default policy for ``set_image_location`` is set to"}],"source_content_type":"text/x-rst","patch_set":1,"id":"6ee41529_6f50abc9","line":20,"range":{"start_line":20,"start_character":56,"end_line":20,"end_character":57},"updated":"2022-05-10 21:20:17.000000000","message":"I suggest adding here: (as they must be, or else a non-admin user cannot associate data with an image record, or retrieve image data, or delete image data),","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18d72f9a7ee5ba743675c8573f1708d399ac65ee","unresolved":false,"context_lines":[{"line_number":17,"context_line":"config option, OSSN-65 [1]_, due to which, if we enable"},{"line_number":18,"context_line":"``show_multiple_locations`` and the policies for add/update"},{"line_number":19,"context_line":"(set_image_location), get (get_image_location) and remove"},{"line_number":20,"context_line":"(delete_image_location) locations are set for non-admins then non-admin users"},{"line_number":21,"context_line":"can modify location data to corrupt the image."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"The default policy for ``set_image_location`` is set to"}],"source_content_type":"text/x-rst","patch_set":1,"id":"3b5d5038_7588ee73","line":20,"range":{"start_line":20,"start_character":56,"end_line":20,"end_character":57},"in_reply_to":"6ee41529_6f50abc9","updated":"2022-05-12 09:21:39.000000000","message":"Done","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"24b8bdfda5919323fc52d830b35bc8a3bdb3a2ae","unresolved":true,"context_lines":[{"line_number":18,"context_line":"``show_multiple_locations`` and the policies for add/update"},{"line_number":19,"context_line":"(set_image_location), get (get_image_location) and remove"},{"line_number":20,"context_line":"(delete_image_location) locations are set for non-admins then non-admin users"},{"line_number":21,"context_line":"can modify location data to corrupt the image."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"The default policy for ``set_image_location`` is set to"},{"line_number":24,"context_line":"``ADMIN_OR_PROJECT_MEMBER`` and for ``get_image_location`` it is"}],"source_content_type":"text/x-rst","patch_set":1,"id":"54268cec_b5e05199","line":21,"range":{"start_line":21,"start_character":36,"end_line":21,"end_character":45},"updated":"2022-05-10 21:20:17.000000000","message":"an image that they own.","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18d72f9a7ee5ba743675c8573f1708d399ac65ee","unresolved":false,"context_lines":[{"line_number":18,"context_line":"``show_multiple_locations`` and the policies for add/update"},{"line_number":19,"context_line":"(set_image_location), get (get_image_location) and remove"},{"line_number":20,"context_line":"(delete_image_location) locations are set for non-admins then non-admin users"},{"line_number":21,"context_line":"can modify location data to corrupt the image."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"The default policy for ``set_image_location`` is set to"},{"line_number":24,"context_line":"``ADMIN_OR_PROJECT_MEMBER`` and for ``get_image_location`` it is"}],"source_content_type":"text/x-rst","patch_set":1,"id":"4a36a0b5_5b287ad0","line":21,"range":{"start_line":21,"start_character":36,"end_line":21,"end_character":45},"in_reply_to":"54268cec_b5e05199","updated":"2022-05-12 09:21:39.000000000","message":"Done","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"24b8bdfda5919323fc52d830b35bc8a3bdb3a2ae","unresolved":true,"context_lines":[{"line_number":20,"context_line":"(delete_image_location) locations are set for non-admins then non-admin users"},{"line_number":21,"context_line":"can modify location data to corrupt the image."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"The default policy for ``set_image_location`` is set to"},{"line_number":24,"context_line":"``ADMIN_OR_PROJECT_MEMBER`` and for ``get_image_location`` it is"},{"line_number":25,"context_line":"``ADMIN_OR_PROJECT_READER`` which allows non-admin users to add and get"},{"line_number":26,"context_line":"locations when we configure ``show_multiple_locations`` to True which is not"},{"line_number":27,"context_line":"ideal."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Proposed change"},{"line_number":30,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"b6654d77_7610a58d","line":27,"range":{"start_line":23,"start_character":0,"end_line":27,"end_character":6},"updated":"2022-05-10 21:20:17.000000000","message":"I suggest putting this the other way:\n\nWhen show_multiple_locations is False, non-admin users cannot modify image locations via the image-update API call, even if they have the {get,set,delete}_image_location permissions.  However, there are some popular use cases where other services can bypass Glance and store or access image data directly in the backend by writing or reading image locations, using the image owner\u0027s credentials, and this is why operators want to set show_multiple_locations to True.  What operators want to do, however, is to enable optimized image data access; exposing image locations to non-admin users is a side-effect, not the goal.  We currently recommend that operators who want to use optimized data access use a specialized Glance instance for services, and only expose glance-api to end users with show_multiple_locations set False.  This is obviously a cumbersome and unsatisfactory solution.","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18d72f9a7ee5ba743675c8573f1708d399ac65ee","unresolved":false,"context_lines":[{"line_number":20,"context_line":"(delete_image_location) locations are set for non-admins then non-admin users"},{"line_number":21,"context_line":"can modify location data to corrupt the image."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"The default policy for ``set_image_location`` is set to"},{"line_number":24,"context_line":"``ADMIN_OR_PROJECT_MEMBER`` and for ``get_image_location`` it is"},{"line_number":25,"context_line":"``ADMIN_OR_PROJECT_READER`` which allows non-admin users to add and get"},{"line_number":26,"context_line":"locations when we configure ``show_multiple_locations`` to True which is not"},{"line_number":27,"context_line":"ideal."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Proposed change"},{"line_number":30,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"fbf1940c_b2de1538","line":27,"range":{"start_line":23,"start_character":0,"end_line":27,"end_character":6},"in_reply_to":"b6654d77_7610a58d","updated":"2022-05-12 09:21:39.000000000","message":"Done\nThis sounds much better, Thanks!","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"24b8bdfda5919323fc52d830b35bc8a3bdb3a2ae","unresolved":true,"context_lines":[{"line_number":30,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"The proposal is to introduce 5 new APIs for add, get, get all, update and"},{"line_number":33,"context_line":"delete location(s) for a particular image (NOTE: get all policy is for all"},{"line_number":34,"context_line":"images). We will need to modify existing method that image update uses to add,"},{"line_number":35,"context_line":"get, update and delete image location(s)."},{"line_number":36,"context_line":"All the APIs will be admin only so non-admin users won\u0027t be able to perform"}],"source_content_type":"text/x-rst","patch_set":1,"id":"1146dd25_6c6e4730","line":33,"range":{"start_line":33,"start_character":57,"end_line":33,"end_character":63},"updated":"2022-05-10 21:20:17.000000000","message":"??","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18d72f9a7ee5ba743675c8573f1708d399ac65ee","unresolved":false,"context_lines":[{"line_number":30,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"The proposal is to introduce 5 new APIs for add, get, get all, update and"},{"line_number":33,"context_line":"delete location(s) for a particular image (NOTE: get all policy is for all"},{"line_number":34,"context_line":"images). We will need to modify existing method that image update uses to add,"},{"line_number":35,"context_line":"get, update and delete image location(s)."},{"line_number":36,"context_line":"All the APIs will be admin only so non-admin users won\u0027t be able to perform"}],"source_content_type":"text/x-rst","patch_set":1,"id":"d54f81e6_3bc39d6b","line":33,"range":{"start_line":33,"start_character":57,"end_line":33,"end_character":63},"in_reply_to":"1146dd25_6c6e4730","updated":"2022-05-12 09:21:39.000000000","message":"Done","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"24b8bdfda5919323fc52d830b35bc8a3bdb3a2ae","unresolved":true,"context_lines":[{"line_number":32,"context_line":"The proposal is to introduce 5 new APIs for add, get, get all, update and"},{"line_number":33,"context_line":"delete location(s) for a particular image (NOTE: get all policy is for all"},{"line_number":34,"context_line":"images). We will need to modify existing method that image update uses to add,"},{"line_number":35,"context_line":"get, update and delete image location(s)."},{"line_number":36,"context_line":"All the APIs will be admin only so non-admin users won\u0027t be able to perform"},{"line_number":37,"context_line":"operations on locations. The image update operation will not be able to perform"},{"line_number":38,"context_line":"location operations and we will recommend using the new location APIs for any"}],"source_content_type":"text/x-rst","patch_set":1,"id":"a65f7e64_5f01bbe5","line":35,"updated":"2022-05-10 21:20:17.000000000","message":"This might be more clear if you say it like this:\n\nThe proposal is the following:\n\n1. Do not allow image locations to be modified using the image-update call.  Note that the default setting for show_multiple_locations already blocks image location modification via the image-update call, so this change will not affect users in the default configuration.\n\n2. Introduce 5 new API calls ...  that allow modification of image locations.  These calls will replace the image-update mechanism.\n\n(Then leave a blank line so that the discussion of admin/non-admin can be discussed in a different paragraph.)","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5435dcfb5cb2bedd5fec92d7a238f2911a7f39d3","unresolved":false,"context_lines":[{"line_number":32,"context_line":"The proposal is to introduce 5 new APIs for add, get, get all, update and"},{"line_number":33,"context_line":"delete location(s) for a particular image (NOTE: get all policy is for all"},{"line_number":34,"context_line":"images). We will need to modify existing method that image update uses to add,"},{"line_number":35,"context_line":"get, update and delete image location(s)."},{"line_number":36,"context_line":"All the APIs will be admin only so non-admin users won\u0027t be able to perform"},{"line_number":37,"context_line":"operations on locations. The image update operation will not be able to perform"},{"line_number":38,"context_line":"location operations and we will recommend using the new location APIs for any"}],"source_content_type":"text/x-rst","patch_set":1,"id":"024ab458_cfbb0e0b","line":35,"in_reply_to":"250fe365_4f0de4bf","updated":"2022-05-18 13:50:10.000000000","message":"Ack","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18d72f9a7ee5ba743675c8573f1708d399ac65ee","unresolved":true,"context_lines":[{"line_number":32,"context_line":"The proposal is to introduce 5 new APIs for add, get, get all, update and"},{"line_number":33,"context_line":"delete location(s) for a particular image (NOTE: get all policy is for all"},{"line_number":34,"context_line":"images). We will need to modify existing method that image update uses to add,"},{"line_number":35,"context_line":"get, update and delete image location(s)."},{"line_number":36,"context_line":"All the APIs will be admin only so non-admin users won\u0027t be able to perform"},{"line_number":37,"context_line":"operations on locations. The image update operation will not be able to perform"},{"line_number":38,"context_line":"location operations and we will recommend using the new location APIs for any"}],"source_content_type":"text/x-rst","patch_set":1,"id":"250fe365_4f0de4bf","line":35,"in_reply_to":"a65f7e64_5f01bbe5","updated":"2022-05-12 09:21:39.000000000","message":"we are planning to do these changes in phases\n\n1) introduce new location APIs (currently not used by cinder/nova)\n2) Modify cinder/nova code to use the new location APIs\n3) Remove/modify code in image-update operation to not allow locations update via image-update\n\nI will rephrase this to be more clear","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"24b8bdfda5919323fc52d830b35bc8a3bdb3a2ae","unresolved":true,"context_lines":[{"line_number":37,"context_line":"operations on locations. The image update operation will not be able to perform"},{"line_number":38,"context_line":"location operations and we will recommend using the new location APIs for any"},{"line_number":39,"context_line":"location operations. This will happen once the consumers (nova, cinder etc)"},{"line_number":40,"context_line":"start using new location APIs."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"The config option ``show_multiple_locations`` is deprecated but we will keep"},{"line_number":43,"context_line":"the config option until the consumers of glance locations (nova, cinder etc)"}],"source_content_type":"text/x-rst","patch_set":1,"id":"c5b402ea_45a3b163","line":40,"updated":"2022-05-10 21:20:17.000000000","message":"Question: once the Location API is implemented, is there any reason to keep the  {get,set,delete}_image_location policies?  Actually, as soon as the image-update call is changed so that you can\u0027t use it to modify locations, aren\u0027t those policies no longer useful?","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5435dcfb5cb2bedd5fec92d7a238f2911a7f39d3","unresolved":false,"context_lines":[{"line_number":37,"context_line":"operations on locations. The image update operation will not be able to perform"},{"line_number":38,"context_line":"location operations and we will recommend using the new location APIs for any"},{"line_number":39,"context_line":"location operations. This will happen once the consumers (nova, cinder etc)"},{"line_number":40,"context_line":"start using new location APIs."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"The config option ``show_multiple_locations`` is deprecated but we will keep"},{"line_number":43,"context_line":"the config option until the consumers of glance locations (nova, cinder etc)"}],"source_content_type":"text/x-rst","patch_set":1,"id":"0b734421_bf27f977","line":40,"in_reply_to":"2a83e392_4f6509fa","updated":"2022-05-18 13:50:10.000000000","message":"Ack","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18d72f9a7ee5ba743675c8573f1708d399ac65ee","unresolved":true,"context_lines":[{"line_number":37,"context_line":"operations on locations. The image update operation will not be able to perform"},{"line_number":38,"context_line":"location operations and we will recommend using the new location APIs for any"},{"line_number":39,"context_line":"location operations. This will happen once the consumers (nova, cinder etc)"},{"line_number":40,"context_line":"start using new location APIs."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"The config option ``show_multiple_locations`` is deprecated but we will keep"},{"line_number":43,"context_line":"the config option until the consumers of glance locations (nova, cinder etc)"}],"source_content_type":"text/x-rst","patch_set":1,"id":"2a83e392_4f6509fa","line":40,"in_reply_to":"c5b402ea_45a3b163","updated":"2022-05-12 09:21:39.000000000","message":"Currently nova/cinder rely on the image-update operation and we need to maintain backward compatibility. Once nova/cinder has shifted to using new location APIs, we can remove those policies and rely on ones added for new location APIs.\nAlso i think some policies are used at other places as well so we would need to keep those but they won\u0027t be no longer used in image-update operation after phase 3.","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"24b8bdfda5919323fc52d830b35bc8a3bdb3a2ae","unresolved":true,"context_lines":[{"line_number":39,"context_line":"location operations. This will happen once the consumers (nova, cinder etc)"},{"line_number":40,"context_line":"start using new location APIs."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"The config option ``show_multiple_locations`` is deprecated but we will keep"},{"line_number":43,"context_line":"the config option until the consumers of glance locations (nova, cinder etc)"},{"line_number":44,"context_line":"start using the new location APIs."},{"line_number":45,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"6c2c23da_d7e1aa8f","line":42,"range":{"start_line":42,"start_character":46,"end_line":42,"end_character":59},"updated":"2022-05-10 21:20:17.000000000","message":"has been deprecated since Newton","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18d72f9a7ee5ba743675c8573f1708d399ac65ee","unresolved":false,"context_lines":[{"line_number":39,"context_line":"location operations. This will happen once the consumers (nova, cinder etc)"},{"line_number":40,"context_line":"start using new location APIs."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"The config option ``show_multiple_locations`` is deprecated but we will keep"},{"line_number":43,"context_line":"the config option until the consumers of glance locations (nova, cinder etc)"},{"line_number":44,"context_line":"start using the new location APIs."},{"line_number":45,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"94f408bf_cdbf9c96","line":42,"range":{"start_line":42,"start_character":46,"end_line":42,"end_character":59},"in_reply_to":"6c2c23da_d7e1aa8f","updated":"2022-05-12 09:21:39.000000000","message":"Done","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"24b8bdfda5919323fc52d830b35bc8a3bdb3a2ae","unresolved":true,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"The config option ``show_multiple_locations`` is deprecated but we will keep"},{"line_number":43,"context_line":"the config option until the consumers of glance locations (nova, cinder etc)"},{"line_number":44,"context_line":"start using the new location APIs."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"Alternatives"},{"line_number":47,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"e6bbf3a7_fd3d618a","line":44,"updated":"2022-05-10 21:20:17.000000000","message":"Question: why keep it?  It allows you to see the locations, but if you change the image-update call, you (well, Nova and Cinder) won\u0027t be able modify them, which I think is the only reason to expose them in the first place?","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5435dcfb5cb2bedd5fec92d7a238f2911a7f39d3","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"The config option ``show_multiple_locations`` is deprecated but we will keep"},{"line_number":43,"context_line":"the config option until the consumers of glance locations (nova, cinder etc)"},{"line_number":44,"context_line":"start using the new location APIs."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"Alternatives"},{"line_number":47,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"1386c1ea_29c801b0","line":44,"in_reply_to":"07a82fb3_575af416","updated":"2022-05-18 13:50:10.000000000","message":"Ack","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18d72f9a7ee5ba743675c8573f1708d399ac65ee","unresolved":true,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"The config option ``show_multiple_locations`` is deprecated but we will keep"},{"line_number":43,"context_line":"the config option until the consumers of glance locations (nova, cinder etc)"},{"line_number":44,"context_line":"start using the new location APIs."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"Alternatives"},{"line_number":47,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"07a82fb3_575af416","line":44,"in_reply_to":"e6bbf3a7_fd3d618a","updated":"2022-05-12 09:21:39.000000000","message":"See explanation above","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"24b8bdfda5919323fc52d830b35bc8a3bdb3a2ae","unresolved":true,"context_lines":[{"line_number":46,"context_line":"Alternatives"},{"line_number":47,"context_line":"------------"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"Add new separate policies for locations that are not shared with other operations."},{"line_number":50,"context_line":"This is an option but it again introduces the same risks if these new policies are"},{"line_number":51,"context_line":"overriden to be allowed for non-admins."},{"line_number":52,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"68e42dca_d5812fae","line":49,"range":{"start_line":49,"start_character":0,"end_line":49,"end_character":39},"updated":"2022-05-10 21:20:17.000000000","message":"Oh, I thought new policies for the new Location API calls were part of your proposal.  Or were you planning to re-use the current location policies?  I think that\u0027s problematic because of how they\u0027re used by the ImageLocationsProxy.\n\nhttps://opendev.org/openstack/glance/src/commit/331ce59a50524e65630aed2d888af30f4494563d/glance/api/policy.py#L376-L425","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5435dcfb5cb2bedd5fec92d7a238f2911a7f39d3","unresolved":false,"context_lines":[{"line_number":46,"context_line":"Alternatives"},{"line_number":47,"context_line":"------------"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"Add new separate policies for locations that are not shared with other operations."},{"line_number":50,"context_line":"This is an option but it again introduces the same risks if these new policies are"},{"line_number":51,"context_line":"overriden to be allowed for non-admins."},{"line_number":52,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"1a7bfe82_46bf0271","line":49,"range":{"start_line":49,"start_character":0,"end_line":49,"end_character":39},"in_reply_to":"2151fe53_629ba71d","updated":"2022-05-18 13:50:10.000000000","message":"Ack","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18d72f9a7ee5ba743675c8573f1708d399ac65ee","unresolved":true,"context_lines":[{"line_number":46,"context_line":"Alternatives"},{"line_number":47,"context_line":"------------"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"Add new separate policies for locations that are not shared with other operations."},{"line_number":50,"context_line":"This is an option but it again introduces the same risks if these new policies are"},{"line_number":51,"context_line":"overriden to be allowed for non-admins."},{"line_number":52,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"2151fe53_629ba71d","line":49,"range":{"start_line":49,"start_character":0,"end_line":49,"end_character":39},"in_reply_to":"68e42dca_d5812fae","updated":"2022-05-12 09:21:39.000000000","message":"Initially i thought we could add new policies that are not shared at other places and that would do the work but guess not, I will remove this section since the proposal of the spec also requires new policies.","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"24b8bdfda5919323fc52d830b35bc8a3bdb3a2ae","unresolved":true,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"Add new separate policies for locations that are not shared with other operations."},{"line_number":50,"context_line":"This is an option but it again introduces the same risks if these new policies are"},{"line_number":51,"context_line":"overriden to be allowed for non-admins."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"Data model impact"},{"line_number":54,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"a6a6cf57_8b0fc0b4","line":51,"updated":"2022-05-10 21:20:17.000000000","message":"OK, I think I may have mis-understood your point here.  The alternative you\u0027re proposing is that you would continue to perform location modification by the image-update call, but you would introduce new policies specifically for that call?  I guess the big drawback is that you\u0027d still need to expose the locations to non-services.","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18d72f9a7ee5ba743675c8573f1708d399ac65ee","unresolved":true,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"Add new separate policies for locations that are not shared with other operations."},{"line_number":50,"context_line":"This is an option but it again introduces the same risks if these new policies are"},{"line_number":51,"context_line":"overriden to be allowed for non-admins."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"Data model impact"},{"line_number":54,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"c90b8a7f_72f33be7","line":51,"in_reply_to":"a6a6cf57_8b0fc0b4","updated":"2022-05-12 09:21:39.000000000","message":"Yeah, that\u0027s a problem so will remove this section in the next patch.","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5435dcfb5cb2bedd5fec92d7a238f2911a7f39d3","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"Add new separate policies for locations that are not shared with other operations."},{"line_number":50,"context_line":"This is an option but it again introduces the same risks if these new policies are"},{"line_number":51,"context_line":"overriden to be allowed for non-admins."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"Data model impact"},{"line_number":54,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"2be7d00b_307b46e9","line":51,"in_reply_to":"c90b8a7f_72f33be7","updated":"2022-05-18 13:50:10.000000000","message":"Ack","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"24b8bdfda5919323fc52d830b35bc8a3bdb3a2ae","unresolved":true,"context_lines":[{"line_number":109,"context_line":""},{"line_number":110,"context_line":"  This will show all the locations for all images."},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"  GET /v2/locations"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"  * JSON response body"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"6abe840d_f31cb8bd","line":112,"updated":"2022-05-10 21:20:17.000000000","message":"What\u0027s the use case for this call?  I can see wanting to get a list of images by store (which is going to be an expensive call due to how the \u0027store\u0027 is stored in a JSON string in the location metadata, but ignore that), but I\u0027m not sure why you\u0027d want to get a list of all the locations?\n\nI\u0027m asking because I think the URL for the other calls should be:\n\n    VERB  /v2/images/{image_id}/locations\n\nbut that obviously won\u0027t work for this call.","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0fb784de3c7c2a49f30471e3ef0f8b58c5358e7a","unresolved":false,"context_lines":[{"line_number":109,"context_line":""},{"line_number":110,"context_line":"  This will show all the locations for all images."},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"  GET /v2/locations"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"  * JSON response body"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"24f365d1_b18e9c89","line":112,"in_reply_to":"16748bea_cefb843e","updated":"2022-05-16 11:34:59.000000000","message":"Ack, will remove this new API.","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18d72f9a7ee5ba743675c8573f1708d399ac65ee","unresolved":true,"context_lines":[{"line_number":109,"context_line":""},{"line_number":110,"context_line":"  This will show all the locations for all images."},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"  GET /v2/locations"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"  * JSON response body"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"cd7ccd7a_6de0e628","line":112,"in_reply_to":"6abe840d_f31cb8bd","updated":"2022-05-12 09:21:39.000000000","message":"As an operator, If we want to do location operations on different images, we should be able to see which all locations are set for images. This is for a better UX to be able to list all images with their location property. Maybe this can be done with image-list with a location filter but since we were creating new APIs for locations, this made sense to me. let me know your thoughts.\n\nDone changes for other location APIs as suggested.","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"d37bd77222a598da604dc2304bdd1eed69f193fb","unresolved":true,"context_lines":[{"line_number":109,"context_line":""},{"line_number":110,"context_line":"  This will show all the locations for all images."},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"  GET /v2/locations"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"  * JSON response body"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"16748bea_cefb843e","line":112,"in_reply_to":"cd7ccd7a_6de0e628","updated":"2022-05-13 12:20:51.000000000","message":"I tend to agree with Brian here. This sounds like heavy call with very little use. I\u0027d prefer leaving this out unless there is real use case this would address.","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"cd981d44574cff774b9d3241b45c2120c7bb129a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2dea9ffa_c358233a","line":260,"updated":"2022-05-10 21:22:58.000000000","message":"Might want to add some of these: \n\n- Deprecate `show_multiple_locations` option | https://review.opendev.org/c/open\nstack/glance/+/313936\n- Update deprecated show_multiple_locations helptext | https://review.opendev.org/c/openstack/glance/+/426283\n- Update show_multiple_locations deprecation note | https://review.opendev.org/c/openstack/glance/+/625702\n- the original security bug: https://bugs.launchpad.net/ossn/+bug/1549483","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18d72f9a7ee5ba743675c8573f1708d399ac65ee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8739fe9e_57fa5348","line":260,"in_reply_to":"2dea9ffa_c358233a","updated":"2022-05-12 09:21:39.000000000","message":"Done","commit_id":"66a9efc60a5ba6cbc220de4ac0ac0ad2e8988488"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"d37bd77222a598da604dc2304bdd1eed69f193fb","unresolved":true,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"We are going to add 5 new location APIs:"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"* Add Location"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"  This will add a new location to an existing image."},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"  POST /v2/images/{image_id}/locations"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"  * JSON request body"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    .. code-block:: json"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        {"},{"line_number":87,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":88,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":89,"context_line":"        }"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"  * JSON response body"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    .. code-block:: json"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        {"},{"line_number":96,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":97,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":98,"context_line":"        }"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"* Get Location(s)"},{"line_number":101,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"377a7792_a37bc4fa","line":98,"range":{"start_line":76,"start_character":0,"end_line":98,"end_character":9},"updated":"2022-05-13 12:20:51.000000000","message":"What is the expected behaviour when there is existing location recorded for the image?","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0fb784de3c7c2a49f30471e3ef0f8b58c5358e7a","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"We are going to add 5 new location APIs:"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"* Add Location"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"  This will add a new location to an existing image."},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"  POST /v2/images/{image_id}/locations"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"  * JSON request body"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    .. code-block:: json"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        {"},{"line_number":87,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":88,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":89,"context_line":"        }"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"  * JSON response body"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    .. code-block:: json"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        {"},{"line_number":96,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":97,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":98,"context_line":"        }"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"* Get Location(s)"},{"line_number":101,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"afcf5a07_05d7278c","line":98,"range":{"start_line":76,"start_character":0,"end_line":98,"end_character":9},"in_reply_to":"377a7792_a37bc4fa","updated":"2022-05-16 11:34:59.000000000","message":"We will fail with 400 Bad Request, I will document that in next PS.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"de464b37d46039cb46405a32415f22476cd1d0b2","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"We are going to add 5 new location APIs:"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"* Add Location"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"  This will add a new location to an existing image."},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"  POST /v2/images/{image_id}/locations"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"  * JSON request body"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    .. code-block:: json"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        {"},{"line_number":87,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":88,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":89,"context_line":"        }"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"  * JSON response body"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    .. code-block:: json"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        {"},{"line_number":96,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":97,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":98,"context_line":"        }"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"* Get Location(s)"},{"line_number":101,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"2d542ccd_f68e088b","line":98,"range":{"start_line":76,"start_character":0,"end_line":98,"end_character":9},"in_reply_to":"afcf5a07_05d7278c","updated":"2022-05-16 12:35:57.000000000","message":"I think 409 (Conflict) will be better here","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"d37bd77222a598da604dc2304bdd1eed69f193fb","unresolved":true,"context_lines":[{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    .. code-block:: json"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        ["},{"line_number":111,"context_line":"            {"},{"line_number":112,"context_line":"                \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":113,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":114,"context_line":"            },"},{"line_number":115,"context_line":"            {"},{"line_number":116,"context_line":"                \"url\": \"cinder://cephdriver-1/11b4fa9f-a44b-46c9-950c-0026c467252c\","},{"line_number":117,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027cephdriver-1\u0027}\""},{"line_number":118,"context_line":"            }"},{"line_number":119,"context_line":"        ]"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"* Get All Locations"},{"line_number":122,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"22c954c5_231470cc","line":119,"range":{"start_line":110,"start_character":0,"end_line":119,"end_character":9},"updated":"2022-05-13 12:20:51.000000000","message":"Any specific reason why we are not exposing some of the location metadata here? Mainly the ID and status comes to mind.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0fb784de3c7c2a49f30471e3ef0f8b58c5358e7a","unresolved":true,"context_lines":[{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    .. code-block:: json"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        ["},{"line_number":111,"context_line":"            {"},{"line_number":112,"context_line":"                \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":113,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":114,"context_line":"            },"},{"line_number":115,"context_line":"            {"},{"line_number":116,"context_line":"                \"url\": \"cinder://cephdriver-1/11b4fa9f-a44b-46c9-950c-0026c467252c\","},{"line_number":117,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027cephdriver-1\u0027}\""},{"line_number":118,"context_line":"            }"},{"line_number":119,"context_line":"        ]"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"* Get All Locations"},{"line_number":122,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"9ea1fdf7_d201e671","line":119,"range":{"start_line":110,"start_character":0,"end_line":119,"end_character":9},"in_reply_to":"22c954c5_231470cc","updated":"2022-05-16 11:34:59.000000000","message":"I\u0027m not sure about the ask here, we will be exposing all the metadata in locations. should i add other parameters to this example?\nAlso I think glance doesn\u0027t add the ID and status of locations so where does it come from?","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"3dd9051d980767edae52055d474284a7e09dfc70","unresolved":false,"context_lines":[{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    .. code-block:: json"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        ["},{"line_number":111,"context_line":"            {"},{"line_number":112,"context_line":"                \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":113,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":114,"context_line":"            },"},{"line_number":115,"context_line":"            {"},{"line_number":116,"context_line":"                \"url\": \"cinder://cephdriver-1/11b4fa9f-a44b-46c9-950c-0026c467252c\","},{"line_number":117,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027cephdriver-1\u0027}\""},{"line_number":118,"context_line":"            }"},{"line_number":119,"context_line":"        ]"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"* Get All Locations"},{"line_number":122,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"4922c9e9_984740ae","line":119,"range":{"start_line":110,"start_character":0,"end_line":119,"end_character":9},"in_reply_to":"9ea1fdf7_d201e671","updated":"2022-05-17 08:02:05.000000000","message":"Done","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"d37bd77222a598da604dc2304bdd1eed69f193fb","unresolved":true,"context_lines":[{"line_number":129,"context_line":"    .. code-block:: json"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        {"},{"line_number":132,"context_line":"            \"image1\":"},{"line_number":133,"context_line":"                ["},{"line_number":134,"context_line":"                    {"},{"line_number":135,"context_line":"                        \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","}],"source_content_type":"text/x-rst","patch_set":2,"id":"ab456249_0efd1cab","line":132,"range":{"start_line":132,"start_character":13,"end_line":132,"end_character":19},"updated":"2022-05-13 12:20:51.000000000","message":"I\u0027m assuming these should be image IDs","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0fb784de3c7c2a49f30471e3ef0f8b58c5358e7a","unresolved":true,"context_lines":[{"line_number":129,"context_line":"    .. code-block:: json"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        {"},{"line_number":132,"context_line":"            \"image1\":"},{"line_number":133,"context_line":"                ["},{"line_number":134,"context_line":"                    {"},{"line_number":135,"context_line":"                        \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","}],"source_content_type":"text/x-rst","patch_set":2,"id":"eaa43abc_65478481","line":132,"range":{"start_line":132,"start_character":13,"end_line":132,"end_character":19},"in_reply_to":"ab456249_0efd1cab","updated":"2022-05-16 11:34:59.000000000","message":"Yes, i will replace this with proper UUIDs","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"3dd9051d980767edae52055d474284a7e09dfc70","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    .. code-block:: json"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        {"},{"line_number":132,"context_line":"            \"image1\":"},{"line_number":133,"context_line":"                ["},{"line_number":134,"context_line":"                    {"},{"line_number":135,"context_line":"                        \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","}],"source_content_type":"text/x-rst","patch_set":2,"id":"39d4d767_9ec60ed8","line":132,"range":{"start_line":132,"start_character":13,"end_line":132,"end_character":19},"in_reply_to":"eaa43abc_65478481","updated":"2022-05-17 08:02:05.000000000","message":"Since we removed this API, no need of this change","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"d37bd77222a598da604dc2304bdd1eed69f193fb","unresolved":true,"context_lines":[{"line_number":153,"context_line":"                ]"},{"line_number":154,"context_line":"        }"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"* Update Location"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"  This will modify the metadata or location of an existing image."},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"  PUT /v2/images/{image_id}/locations"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"  * JSON request body"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    .. code-block:: json"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        {"},{"line_number":167,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":168,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":169,"context_line":"        }"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"  * JSON response body"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    .. code-block:: json"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"        {"},{"line_number":177,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":178,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":179,"context_line":"        }"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"* Delete Location"},{"line_number":182,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"97b4aed6_08a995b1","line":179,"range":{"start_line":156,"start_character":0,"end_line":179,"end_character":9},"updated":"2022-05-13 12:20:51.000000000","message":"How is this supposed to work, say the image has 3 locations already? Is this to replace all of the locations? As the location is associated with actual (immutable) data sitting on disk, this modify API sounds like massive hazard waiting to happen.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"3dd9051d980767edae52055d474284a7e09dfc70","unresolved":false,"context_lines":[{"line_number":153,"context_line":"                ]"},{"line_number":154,"context_line":"        }"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"* Update Location"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"  This will modify the metadata or location of an existing image."},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"  PUT /v2/images/{image_id}/locations"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"  * JSON request body"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    .. code-block:: json"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        {"},{"line_number":167,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":168,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":169,"context_line":"        }"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"  * JSON response body"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    .. code-block:: json"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"        {"},{"line_number":177,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":178,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":179,"context_line":"        }"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"* Delete Location"},{"line_number":182,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"5db67c78_c48c7e63","line":179,"range":{"start_line":156,"start_character":0,"end_line":179,"end_character":9},"in_reply_to":"1a829e09_a6027ee6","updated":"2022-05-17 08:02:05.000000000","message":"Done","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0fb784de3c7c2a49f30471e3ef0f8b58c5358e7a","unresolved":true,"context_lines":[{"line_number":153,"context_line":"                ]"},{"line_number":154,"context_line":"        }"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"* Update Location"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"  This will modify the metadata or location of an existing image."},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"  PUT /v2/images/{image_id}/locations"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"  * JSON request body"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    .. code-block:: json"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        {"},{"line_number":167,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":168,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":169,"context_line":"        }"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"  * JSON response body"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    .. code-block:: json"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"        {"},{"line_number":177,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":178,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":179,"context_line":"        }"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"* Delete Location"},{"line_number":182,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"1a829e09_a6027ee6","line":179,"range":{"start_line":156,"start_character":0,"end_line":179,"end_character":9},"in_reply_to":"97b4aed6_08a995b1","updated":"2022-05-16 11:34:59.000000000","message":"Yeah this was not written correctly, the idea is to be able to update an existing location and not update all locations at once. Will correct this in next PS.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"d37bd77222a598da604dc2304bdd1eed69f193fb","unresolved":true,"context_lines":[{"line_number":178,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":179,"context_line":"        }"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"* Delete Location"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"  This will delete a location from an existing image."},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"  DELETE /v2/images/{image_id}/locations"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"  * JSON request body"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    .. code-block:: json"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        {"},{"line_number":192,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":193,"context_line":"        }"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"Security impact"},{"line_number":196,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"4e343953_03c302f8","line":193,"range":{"start_line":181,"start_character":0,"end_line":193,"end_character":9},"updated":"2022-05-13 12:20:51.000000000","message":"Body in DELETE call has been very controversial topic for years and in my understanding there is good chance that it will be dropped on the way depending what\u0027s between the client and service. What is the expected behaviour when no body is provided? Any reason why this is not \"\"\"DELETE /v2/images/{image_id}/locations/{location_id}\"\"\"?","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0fb784de3c7c2a49f30471e3ef0f8b58c5358e7a","unresolved":true,"context_lines":[{"line_number":178,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":179,"context_line":"        }"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"* Delete Location"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"  This will delete a location from an existing image."},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"  DELETE /v2/images/{image_id}/locations"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"  * JSON request body"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    .. code-block:: json"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        {"},{"line_number":192,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":193,"context_line":"        }"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"Security impact"},{"line_number":196,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"f5594e16_51da3c9d","line":193,"range":{"start_line":181,"start_character":0,"end_line":193,"end_character":9},"in_reply_to":"4e343953_03c302f8","updated":"2022-05-16 11:34:59.000000000","message":"Will update this as per suggestion to avoid body.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"3dd9051d980767edae52055d474284a7e09dfc70","unresolved":false,"context_lines":[{"line_number":178,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":179,"context_line":"        }"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"* Delete Location"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"  This will delete a location from an existing image."},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"  DELETE /v2/images/{image_id}/locations"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"  * JSON request body"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    .. code-block:: json"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        {"},{"line_number":192,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":193,"context_line":"        }"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"Security impact"},{"line_number":196,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"9074a205_9db4a2b6","line":193,"range":{"start_line":181,"start_character":0,"end_line":193,"end_character":9},"in_reply_to":"f5594e16_51da3c9d","updated":"2022-05-17 08:02:05.000000000","message":"Done","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"d37bd77222a598da604dc2304bdd1eed69f193fb","unresolved":true,"context_lines":[{"line_number":195,"context_line":"Security impact"},{"line_number":196,"context_line":"---------------"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"None. All APIs are admin only and won\u0027t leak any sensitive data to non-admin"},{"line_number":199,"context_line":"users."},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"Notifications impact"},{"line_number":202,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"9ece0280_638d12ff","line":199,"range":{"start_line":198,"start_character":0,"end_line":199,"end_character":6},"updated":"2022-05-13 12:20:51.000000000","message":"For API that is expected to be used for Service-Service calls this is horrible idea as it requires admin credentials (that are unlimited by their scope in Open Stack) to be stored in the calling service.\n\nCompared to the current situation we will loose traceability of who has performed these actions in logs as all the operations on project owned resources would be made using an admin account.\n\nAs adminnness is all powering, there is no authorization who can perform said actions to the project owned resources in Glance side. All the Authrisation is expected to happen in the service calling these APIs.\n\nThere is very little expectations of actual human admin performing these actions rather than service performing them. This means that leaking or not of any sensitive data is fully dependant of the consuming service. As the expectation is that the consuming service is calling these on behalf of non-admin user in Glance point of view, this is always leaking sensitive data as we have no control of the authorisation.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0fb784de3c7c2a49f30471e3ef0f8b58c5358e7a","unresolved":true,"context_lines":[{"line_number":195,"context_line":"Security impact"},{"line_number":196,"context_line":"---------------"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"None. All APIs are admin only and won\u0027t leak any sensitive data to non-admin"},{"line_number":199,"context_line":"users."},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"Notifications impact"},{"line_number":202,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"bfaff9d3_49524569","line":199,"range":{"start_line":198,"start_character":0,"end_line":199,"end_character":6},"in_reply_to":"9ece0280_638d12ff","updated":"2022-05-16 11:34:59.000000000","message":"\u003e For API that is expected to be used for Service-Service calls this is horrible idea as it requires admin credentials (that are unlimited by their scope in Open Stack) to be stored in the calling service.\n\nCompared to the current situation we will loose traceability of who has performed these actions in logs as all the operations on project owned resources would be made using an admin account.\n\nWe will move to using service role when it is implemented in keystone, until then we only have the option to keep this admin only to avoid the security threat.\n\n\u003e As adminnness is all powering, there is no authorization who can perform said actions to the project owned resources in Glance side. All the Authrisation is expected to happen in the service calling these APIs.\n\nservice and admin users\n\n\u003e There is very little expectations of actual human admin performing these actions rather than service performing them.\n\nCurrently we allow end users to perform the image location operations so it makes sense to allow that in the new design as well even if there is a little usage there.\n\n\u003e This means that leaking or not of any sensitive data is fully dependant of the consuming service. As the expectation is that the consuming service is calling these on behalf of non-admin user in Glance point of view, this is always leaking sensitive data as we have no control of the authorisation.\n\nThere is a difference between a non-admin user calling glance directly to do location operations and a service (like cinder) does this on behalf of a non-admin user for an operation like upload volume to image. In the latter case, Cinder is responsible to ensure that a volume entry exists on behalf of the new location added and performs all the checks around that so we can be assured that the location is not Fake and in the former one, the user can do anything.\nEven though the service calls glance on behalf of non-admin users, the non-admins users can\u0027t randomly demand Cinder to add/remove locations, the control is still with Cinder and not non-admins.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fc83e78e027f19ff4cefff965eaf7d78a993a76b","unresolved":false,"context_lines":[{"line_number":195,"context_line":"Security impact"},{"line_number":196,"context_line":"---------------"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"None. All APIs are admin only and won\u0027t leak any sensitive data to non-admin"},{"line_number":199,"context_line":"users."},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"Notifications impact"},{"line_number":202,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"0fe4f122_136eef05","line":199,"range":{"start_line":198,"start_character":0,"end_line":199,"end_character":6},"in_reply_to":"9f368427_afc590af","updated":"2022-05-24 06:11:46.000000000","message":"Done","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"084e134b817d6399d64ab06ac57af137d80476e0","unresolved":true,"context_lines":[{"line_number":195,"context_line":"Security impact"},{"line_number":196,"context_line":"---------------"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"None. All APIs are admin only and won\u0027t leak any sensitive data to non-admin"},{"line_number":199,"context_line":"users."},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"Notifications impact"},{"line_number":202,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"e9731895_b143eeeb","line":199,"range":{"start_line":198,"start_character":0,"end_line":199,"end_character":6},"in_reply_to":"bfaff9d3_49524569","updated":"2022-05-19 18:10:35.000000000","message":"I agree with Erno that this should not be an admin thing. Meaning, we should not add a policy making this admin-only at this point and require nova (et al) to have an admin user in order to twiddle it. We should go ahead and encode the service role into our policy and tell people that we\u0027re migrating to that alongside keystone codifying it as a standard role.\n\nI understand the disappointment in losing the full audit chain by moving to a special user to do the location updating, but as noted in this week\u0027s meeting, that\u0027s how a lot of interactions between services work today, like nova-\u003eneutron, nova-\u003ecinder, nova-\u003eironic, etc. I think in this particular case, the audit chain for nova adding or deleting a location on an image should be pretty straightforward to chase, and the req-id should provide suitable linkage if we do it right.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ce0c71e4893eddfe1a45ec9109b6c247187b053b","unresolved":true,"context_lines":[{"line_number":195,"context_line":"Security impact"},{"line_number":196,"context_line":"---------------"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"None. All APIs are admin only and won\u0027t leak any sensitive data to non-admin"},{"line_number":199,"context_line":"users."},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"Notifications impact"},{"line_number":202,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"9f368427_afc590af","line":199,"range":{"start_line":198,"start_character":0,"end_line":199,"end_character":6},"in_reply_to":"e9731895_b143eeeb","updated":"2022-05-23 18:28:50.000000000","message":"Ack, will update the proposal to use the service role in these new APIs and when the role is ready to be used from keystone, we can make the changes in nova and cinder respectively.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"d37bd77222a598da604dc2304bdd1eed69f193fb","unresolved":true,"context_lines":[{"line_number":216,"context_line":"Performance Impact"},{"line_number":217,"context_line":"------------------"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"None"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"Other deployer impact"},{"line_number":222,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"d1e3b4f6_b79398c2","line":219,"range":{"start_line":219,"start_character":0,"end_line":219,"end_character":4},"updated":"2022-05-13 12:20:51.000000000","message":"Especially the list all can have very significant performance impact on the database.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"3dd9051d980767edae52055d474284a7e09dfc70","unresolved":false,"context_lines":[{"line_number":216,"context_line":"Performance Impact"},{"line_number":217,"context_line":"------------------"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"None"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"Other deployer impact"},{"line_number":222,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"f8813e32_b372948a","line":219,"range":{"start_line":219,"start_character":0,"end_line":219,"end_character":4},"in_reply_to":"66b2baa0_d4c054c8","updated":"2022-05-17 08:02:05.000000000","message":"Done","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0fb784de3c7c2a49f30471e3ef0f8b58c5358e7a","unresolved":true,"context_lines":[{"line_number":216,"context_line":"Performance Impact"},{"line_number":217,"context_line":"------------------"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"None"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"Other deployer impact"},{"line_number":222,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"66b2baa0_d4c054c8","line":219,"range":{"start_line":219,"start_character":0,"end_line":219,"end_character":4},"in_reply_to":"d1e3b4f6_b79398c2","updated":"2022-05-16 11:34:59.000000000","message":"Removed that API","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"d37bd77222a598da604dc2304bdd1eed69f193fb","unresolved":true,"context_lines":[{"line_number":221,"context_line":"Other deployer impact"},{"line_number":222,"context_line":"---------------------"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"None"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"Developer impact"},{"line_number":227,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"0c15a00a_de9ee828","line":224,"range":{"start_line":224,"start_character":0,"end_line":224,"end_character":4},"updated":"2022-05-13 12:20:51.000000000","message":"There is need to create and store admin credentials for the services consuming these APIs","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0fb784de3c7c2a49f30471e3ef0f8b58c5358e7a","unresolved":true,"context_lines":[{"line_number":221,"context_line":"Other deployer impact"},{"line_number":222,"context_line":"---------------------"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"None"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"Developer impact"},{"line_number":227,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"44ad7951_92eff178","line":224,"range":{"start_line":224,"start_character":0,"end_line":224,"end_character":4},"in_reply_to":"0c15a00a_de9ee828","updated":"2022-05-16 11:34:59.000000000","message":"That is on the service side and not on glance side. Glance will just accept a context and check if it\u0027s an admin user or a service role (when keystone implements it) or not.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f64df97ea77263a8dd9040b4636ccaf4c177c9be","unresolved":false,"context_lines":[{"line_number":221,"context_line":"Other deployer impact"},{"line_number":222,"context_line":"---------------------"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"None"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"Developer impact"},{"line_number":227,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"9b2bdf1b_0355d373","line":224,"range":{"start_line":224,"start_character":0,"end_line":224,"end_character":4},"in_reply_to":"44ad7951_92eff178","updated":"2022-06-08 05:30:03.000000000","message":"Changes on consumer services won\u0027t be needed anymore.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"d37bd77222a598da604dc2304bdd1eed69f193fb","unresolved":true,"context_lines":[{"line_number":226,"context_line":"Developer impact"},{"line_number":227,"context_line":"----------------"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"None"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"Implementation"},{"line_number":232,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":2,"id":"35a545bf_1bdc852d","line":229,"range":{"start_line":229,"start_character":0,"end_line":229,"end_character":4},"updated":"2022-05-13 12:20:51.000000000","message":"Consuming services calling these APIs need to produce Authorisation framework before said calls are issued. This has significant Developer impact on the consuming services which developers are not expected to be proficient on the details of image management.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"3dd9051d980767edae52055d474284a7e09dfc70","unresolved":false,"context_lines":[{"line_number":226,"context_line":"Developer impact"},{"line_number":227,"context_line":"----------------"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"None"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"Implementation"},{"line_number":232,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":2,"id":"9bfee8ad_9fe4f650","line":229,"range":{"start_line":229,"start_character":0,"end_line":229,"end_character":4},"in_reply_to":"0e05298f_f2c6b55c","updated":"2022-05-17 08:02:05.000000000","message":"Done","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0fb784de3c7c2a49f30471e3ef0f8b58c5358e7a","unresolved":true,"context_lines":[{"line_number":226,"context_line":"Developer impact"},{"line_number":227,"context_line":"----------------"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"None"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"Implementation"},{"line_number":232,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":2,"id":"0e05298f_f2c6b55c","line":229,"range":{"start_line":229,"start_character":0,"end_line":229,"end_character":4},"in_reply_to":"35a545bf_1bdc852d","updated":"2022-05-16 11:34:59.000000000","message":"Ack, will mention that. JFYI, I will be doing modifications from the Cinder side to support this and we can discuss the same with Nova team as well.","commit_id":"8b85c7e13960504bc6bcf77d3619687a17eb8bc2"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5435dcfb5cb2bedd5fec92d7a238f2911a7f39d3","unresolved":true,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"There will be 3 phases in which the work will be done as follows:"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"1. Introduce 5 new API calls ... that allow modification of image locations."},{"line_number":44,"context_line":"These calls will replace the image-update mechanism when consumers"},{"line_number":45,"context_line":"(cinder/nova) start using the new location APIs."},{"line_number":46,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"348c4ce8_e245de95","line":43,"range":{"start_line":43,"start_character":29,"end_line":43,"end_character":33},"updated":"2022-05-18 13:50:10.000000000","message":"you don\u0027t need the \u0027...\u0027, they were in my comment because i was too lazy to write out the actual calls!  But you probably don\u0027t need to say the actual calls here anyway.\n\nAlso, I think we only have 4 calls on version of your patch?","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7ca97d7f03894c4ca4194852b452b60268a69a97","unresolved":true,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"There will be 3 phases in which the work will be done as follows:"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"1. Introduce 5 new API calls ... that allow modification of image locations."},{"line_number":44,"context_line":"These calls will replace the image-update mechanism when consumers"},{"line_number":45,"context_line":"(cinder/nova) start using the new location APIs."},{"line_number":46,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"f3374888_5f730569","line":43,"range":{"start_line":43,"start_character":29,"end_line":43,"end_character":33},"in_reply_to":"348c4ce8_e245de95","updated":"2022-05-19 11:16:19.000000000","message":"Sorry, my bad. I thought it was used as a conjunction but i thought it wrong.\nAnd yes, there are 4 APIs, I will update this section.","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ab9216a55d32a0a989eee24a37c3ef1e09dd64da","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"There will be 3 phases in which the work will be done as follows:"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"1. Introduce 5 new API calls ... that allow modification of image locations."},{"line_number":44,"context_line":"These calls will replace the image-update mechanism when consumers"},{"line_number":45,"context_line":"(cinder/nova) start using the new location APIs."},{"line_number":46,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"80aa0b9b_b42b0af0","line":43,"range":{"start_line":43,"start_character":29,"end_line":43,"end_character":33},"in_reply_to":"f3374888_5f730569","updated":"2022-05-19 11:19:40.000000000","message":"Done","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5435dcfb5cb2bedd5fec92d7a238f2911a7f39d3","unresolved":true,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        {"},{"line_number":87,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":88,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":89,"context_line":"        }"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"  * JSON response body"}],"source_content_type":"text/x-rst","patch_set":3,"id":"c21b48f9_03e6335c","line":88,"range":{"start_line":88,"start_character":12,"end_line":88,"end_character":50},"updated":"2022-05-18 13:50:10.000000000","message":"I don\u0027t think we want to allow this ... the current code (I believe) figures out which store is being used from the location uri.  If I\u0027m right about that, then we either restrict what metadata can be included here, or, we only allow the location uri in the request body, and if you want to add additional metadata, you do that by using the PUT location metadata call.","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"7862744d6ef8fb95bbefebb176f40bbbd8f032c2","unresolved":true,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        {"},{"line_number":87,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":88,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":89,"context_line":"        }"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"  * JSON response body"}],"source_content_type":"text/x-rst","patch_set":3,"id":"c6e86c51_012f2023","line":88,"range":{"start_line":88,"start_character":12,"end_line":88,"end_character":50},"in_reply_to":"92c2f511_fadf3e1c","updated":"2022-06-03 11:49:09.000000000","message":"I think I agree with Brian here. If we are creating new API, I do not see reason to not fix possible breaking points like this.","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7ca97d7f03894c4ca4194852b452b60268a69a97","unresolved":true,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        {"},{"line_number":87,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":88,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":89,"context_line":"        }"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"  * JSON response body"}],"source_content_type":"text/x-rst","patch_set":3,"id":"92c2f511_fadf3e1c","line":88,"range":{"start_line":88,"start_character":12,"end_line":88,"end_character":50},"in_reply_to":"c21b48f9_03e6335c","updated":"2022-05-19 11:16:19.000000000","message":"Currently we allow sending metadata while adding location[1] so I\u0027ve kept the same design for the new API.\n\n[1] https://github.com/openstack/python-glanceclient/blob/master/glanceclient/v2/images.py#L486-L487","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7532275c25510ee8df902978d9085dbeab29b636","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        {"},{"line_number":87,"context_line":"            \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":88,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":89,"context_line":"        }"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"  * JSON response body"}],"source_content_type":"text/x-rst","patch_set":3,"id":"bd85c500_9a98c608","line":88,"range":{"start_line":88,"start_character":12,"end_line":88,"end_character":50},"in_reply_to":"c6e86c51_012f2023","updated":"2022-06-08 05:33:44.000000000","message":"Done","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5435dcfb5cb2bedd5fec92d7a238f2911a7f39d3","unresolved":true,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"  This will show all the locations associated to an existing image."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"  GET /v2/images/{image_id}/locations"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"  * JSON response body"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"92da3e80_77593c6d","line":108,"updated":"2022-05-18 13:50:10.000000000","message":"Question: if there are no locations on the image, do you plan to return a 404 or an empty list?","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ce0c71e4893eddfe1a45ec9109b6c247187b053b","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"  This will show all the locations associated to an existing image."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"  GET /v2/images/{image_id}/locations"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"  * JSON response body"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"92607fe1_6dc1da5d","line":108,"in_reply_to":"2c05034e_30d345b9","updated":"2022-05-23 18:28:50.000000000","message":"Ack","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7ca97d7f03894c4ca4194852b452b60268a69a97","unresolved":true,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"  This will show all the locations associated to an existing image."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"  GET /v2/images/{image_id}/locations"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"  * JSON response body"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"af399246_fce5850f","line":108,"in_reply_to":"92da3e80_77593c6d","updated":"2022-05-19 11:16:19.000000000","message":"Since having no locations means the image is not stored anywhere (and just a DB entry) which i think can be possible if we create an empty image (queued state) and fill data later with import plugin methods, I think we should return an empty list like we do when there are no images and we do image list.","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"084e134b817d6399d64ab06ac57af137d80476e0","unresolved":true,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"  This will show all the locations associated to an existing image."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"  GET /v2/images/{image_id}/locations"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"  * JSON response body"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"2c05034e_30d345b9","line":108,"in_reply_to":"af399246_fce5850f","updated":"2022-05-19 18:10:35.000000000","message":"Agree. 404 if $image_id does not exist, empty list if it does and has no locations.","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5435dcfb5cb2bedd5fec92d7a238f2911a7f39d3","unresolved":true,"context_lines":[{"line_number":115,"context_line":"            {"},{"line_number":116,"context_line":"                \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":117,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027,"},{"line_number":118,"context_line":"                              \u0027id\u0027: \u00273925e12f-0414-47e0-a7c4-3cb90cb79c81\u0027,"},{"line_number":119,"context_line":"                              \u0027status\u0027: \u0027active\u0027}\""},{"line_number":120,"context_line":"            },"},{"line_number":121,"context_line":"            {"},{"line_number":122,"context_line":"                \"url\": \"cinder://cephdriver-1/11b4fa9f-a44b-46c9-950c-0026c467252c\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"f0ff1d4a_62cab428","line":119,"range":{"start_line":118,"start_character":0,"end_line":119,"end_character":48},"updated":"2022-05-18 13:50:10.000000000","message":"This is a good idea to expose the id, because it can be used in the URL of some of the other calls (as opposed to the location itself, which will probably have to be url-encoded, which will give people an opportunity to make mistakes).","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"18979951baaa9828097ea848d3e3d490480b4e76","unresolved":false,"context_lines":[{"line_number":115,"context_line":"            {"},{"line_number":116,"context_line":"                \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":117,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027,"},{"line_number":118,"context_line":"                              \u0027id\u0027: \u00273925e12f-0414-47e0-a7c4-3cb90cb79c81\u0027,"},{"line_number":119,"context_line":"                              \u0027status\u0027: \u0027active\u0027}\""},{"line_number":120,"context_line":"            },"},{"line_number":121,"context_line":"            {"},{"line_number":122,"context_line":"                \"url\": \"cinder://cephdriver-1/11b4fa9f-a44b-46c9-950c-0026c467252c\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"b4ba5c63_da03d9f6","line":119,"range":{"start_line":118,"start_character":0,"end_line":119,"end_character":48},"in_reply_to":"4bfc7878_48efe37f","updated":"2022-05-24 15:19:00.000000000","message":"Sure, I will add a new field instead of modifying the existing Integer field (id).\n\nSorry, missed your question. Yes, since we will be creating new UUIDs for every location entry, we can return it in the response of location create call, will update the API response.","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ce0c71e4893eddfe1a45ec9109b6c247187b053b","unresolved":true,"context_lines":[{"line_number":115,"context_line":"            {"},{"line_number":116,"context_line":"                \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":117,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027,"},{"line_number":118,"context_line":"                              \u0027id\u0027: \u00273925e12f-0414-47e0-a7c4-3cb90cb79c81\u0027,"},{"line_number":119,"context_line":"                              \u0027status\u0027: \u0027active\u0027}\""},{"line_number":120,"context_line":"            },"},{"line_number":121,"context_line":"            {"},{"line_number":122,"context_line":"                \"url\": \"cinder://cephdriver-1/11b4fa9f-a44b-46c9-950c-0026c467252c\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"ea490c7d_5e620eab","line":119,"range":{"start_line":118,"start_character":0,"end_line":119,"end_character":48},"in_reply_to":"75166fa7_dbfc1897","updated":"2022-05-23 18:28:50.000000000","message":"I think UUID would be a stronger alternative to the currently assigned IDs in locations DB. We can probably write a migration to update existing IDs to UUIDs and code to ensure we always store a UUID against a location entry. (Also alter table from integer to String type)","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"084e134b817d6399d64ab06ac57af137d80476e0","unresolved":true,"context_lines":[{"line_number":115,"context_line":"            {"},{"line_number":116,"context_line":"                \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":117,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027,"},{"line_number":118,"context_line":"                              \u0027id\u0027: \u00273925e12f-0414-47e0-a7c4-3cb90cb79c81\u0027,"},{"line_number":119,"context_line":"                              \u0027status\u0027: \u0027active\u0027}\""},{"line_number":120,"context_line":"            },"},{"line_number":121,"context_line":"            {"},{"line_number":122,"context_line":"                \"url\": \"cinder://cephdriver-1/11b4fa9f-a44b-46c9-950c-0026c467252c\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"75166fa7_dbfc1897","line":119,"range":{"start_line":118,"start_character":0,"end_line":119,"end_character":48},"in_reply_to":"a9e63868_d1873dec","updated":"2022-05-19 18:10:35.000000000","message":"So this is just a generated uuid for the location entry in glance, not an ID in the backend, is that right? If so, why isn\u0027t it returned in the create-location call so that you can refer to the thing you just created deterministically?","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1f6d8c35182ca1174ad0db8fae314fb44522b44a","unresolved":false,"context_lines":[{"line_number":115,"context_line":"            {"},{"line_number":116,"context_line":"                \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":117,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027,"},{"line_number":118,"context_line":"                              \u0027id\u0027: \u00273925e12f-0414-47e0-a7c4-3cb90cb79c81\u0027,"},{"line_number":119,"context_line":"                              \u0027status\u0027: \u0027active\u0027}\""},{"line_number":120,"context_line":"            },"},{"line_number":121,"context_line":"            {"},{"line_number":122,"context_line":"                \"url\": \"cinder://cephdriver-1/11b4fa9f-a44b-46c9-950c-0026c467252c\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"4bfc7878_48efe37f","line":119,"range":{"start_line":118,"start_character":0,"end_line":119,"end_character":48},"in_reply_to":"e3525c88_2de426ba","updated":"2022-05-24 13:49:59.000000000","message":"You should definitely not straight convert the integer to a string, and you should also not remove the integer id, just add the uuid. Table joins are much faster on integer keys than strings. So internally relations should continue to use the integer id, just use the uuid for external reference.\n\nHowever, you didn\u0027t answer my question - can we not return the uuid during the create operation so that we can easily reference the location we just created in subsequent calls?","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fc83e78e027f19ff4cefff965eaf7d78a993a76b","unresolved":false,"context_lines":[{"line_number":115,"context_line":"            {"},{"line_number":116,"context_line":"                \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":117,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027,"},{"line_number":118,"context_line":"                              \u0027id\u0027: \u00273925e12f-0414-47e0-a7c4-3cb90cb79c81\u0027,"},{"line_number":119,"context_line":"                              \u0027status\u0027: \u0027active\u0027}\""},{"line_number":120,"context_line":"            },"},{"line_number":121,"context_line":"            {"},{"line_number":122,"context_line":"                \"url\": \"cinder://cephdriver-1/11b4fa9f-a44b-46c9-950c-0026c467252c\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"e3525c88_2de426ba","line":119,"range":{"start_line":118,"start_character":0,"end_line":119,"end_character":48},"in_reply_to":"ea490c7d_5e620eab","updated":"2022-05-24 06:11:46.000000000","message":"Done","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7ca97d7f03894c4ca4194852b452b60268a69a97","unresolved":true,"context_lines":[{"line_number":115,"context_line":"            {"},{"line_number":116,"context_line":"                \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":117,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027,"},{"line_number":118,"context_line":"                              \u0027id\u0027: \u00273925e12f-0414-47e0-a7c4-3cb90cb79c81\u0027,"},{"line_number":119,"context_line":"                              \u0027status\u0027: \u0027active\u0027}\""},{"line_number":120,"context_line":"            },"},{"line_number":121,"context_line":"            {"},{"line_number":122,"context_line":"                \"url\": \"cinder://cephdriver-1/11b4fa9f-a44b-46c9-950c-0026c467252c\","}],"source_content_type":"text/x-rst","patch_set":3,"id":"a9e63868_d1873dec","line":119,"range":{"start_line":118,"start_character":0,"end_line":119,"end_character":48},"in_reply_to":"f0ff1d4a_62cab428","updated":"2022-05-19 11:16:19.000000000","message":"I think this is a user provided ID and not generated by glance? I couldn\u0027t find ID in the image_locations table (in metadata) apart from the integer column which i think auto increments.\n\nid \u003d Column(Integer, primary_key\u003dTrue, nullable\u003dFalse)\n\nmysql\u003e select * from image_locations limit 1;\n+----+--------------------------------------+-----------------------------------------------------------+---------------------+---------------------+------------+---------+--------------------------+--------+\n| id | image_id                             | value                                                     | created_at          | updated_at          | deleted_at | deleted | meta_data                | status |\n+----+--------------------------------------+-----------------------------------------------------------+---------------------+---------------------+------------+---------+--------------------------+--------+\n|  1 | 6a2ea172-630b-4877-8a5f-f310aecf3060 | cinder://lvmdriver-1/bffdffff-6943-483c-8315-3c8b9fe1e407 | 2022-04-18 07:15:09 | 2022-04-18 07:15:09 | NULL       |       0 | {\"store\": \"lvmdriver-1\"} | active |\n+----+--------------------------------------+-----------------------------------------------------------+---------------------+---------------------+------------+---------+--------------------------+--------+","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5435dcfb5cb2bedd5fec92d7a238f2911a7f39d3","unresolved":true,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"* Update Location"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"  This will modify the metadata or location of an existing image."},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"  PUT /v2/images/{image_id}/locations"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"a6a2f69a_2b0b57d1","line":131,"range":{"start_line":131,"start_character":59,"end_line":131,"end_character":64},"updated":"2022-05-18 13:50:10.000000000","message":"1. This seems to be a weird way to change a location, that is, by replacing its url.  The url is basically the essence of a location, so if you want to change the url, you are really talking about a new location.  I think that should be done by deleting the old location and adding a new one.\n\n2. If you go along with #1, then this call can be \"update location metadata\" and the URL pattern can be:\n\n  PUT /v2/images/{image_id}/locations/{location_id}\nor\n  PUT /v2/images/{image_id}/locations/{location_id}/metadata\n\nand then the request body can be simplified accordingly.\n\nWe\u0027ll have to think carefully about what metadata can be modified, though.  Definitely not \u0027id\u0027, probably not \u0027status\u0027 (though I\u0027m not sure about that one), probably not \u0027store\u0027 (because \u0027store\u0027 and \u0027url\u0027 can\u0027t be modified independently, I don\u0027t think?).","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7ca97d7f03894c4ca4194852b452b60268a69a97","unresolved":true,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"* Update Location"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"  This will modify the metadata or location of an existing image."},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"  PUT /v2/images/{image_id}/locations"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"f62cc25e_1facc70f","line":131,"range":{"start_line":131,"start_character":59,"end_line":131,"end_character":64},"in_reply_to":"a6a2f69a_2b0b57d1","updated":"2022-05-19 11:16:19.000000000","message":"Agree with point 1, i think we should not allow updating locations but only the metadata of locations.\n\nI will update the API as suggested and can mention the metadata which should be restricted, although i don\u0027t think we restrict anything in the current code.","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ab9216a55d32a0a989eee24a37c3ef1e09dd64da","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"* Update Location"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"  This will modify the metadata or location of an existing image."},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"  PUT /v2/images/{image_id}/locations"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"224b1949_2909dbd7","line":131,"range":{"start_line":131,"start_character":59,"end_line":131,"end_character":64},"in_reply_to":"f62cc25e_1facc70f","updated":"2022-05-19 11:19:40.000000000","message":"Done","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5435dcfb5cb2bedd5fec92d7a238f2911a7f39d3","unresolved":true,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"  This will delete a location from an existing image."},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"  DELETE /v2/images/{image_id}/locations/{location_url}"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"Security impact"},{"line_number":163,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"b8f90f94_715143de","line":160,"range":{"start_line":160,"start_character":41,"end_line":160,"end_character":55},"updated":"2022-05-18 13:50:10.000000000","message":"I suggest using {location_id} instead.","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"084e134b817d6399d64ab06ac57af137d80476e0","unresolved":true,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"  This will delete a location from an existing image."},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"  DELETE /v2/images/{image_id}/locations/{location_url}"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"Security impact"},{"line_number":163,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"a5d7568b_32dd4607","line":160,"range":{"start_line":160,"start_character":41,"end_line":160,"end_character":55},"in_reply_to":"25e1052d_13d16d8f","updated":"2022-05-19 18:10:35.000000000","message":"Yeah, embedding the location_url in the resource url is a bad idea, IMHO. Definitely need to use an identifier of some sort. Also, presumably some upgrade or topology change could end up with a location url changing without needing to break everything else, and a stable id would keep the linkages in place properly.\n\nSame argument for L136 of course.","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fc83e78e027f19ff4cefff965eaf7d78a993a76b","unresolved":false,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"  This will delete a location from an existing image."},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"  DELETE /v2/images/{image_id}/locations/{location_url}"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"Security impact"},{"line_number":163,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"583ac33b_13ce7a95","line":160,"range":{"start_line":160,"start_character":41,"end_line":160,"end_character":55},"in_reply_to":"8fdecd9f_f8686911","updated":"2022-05-24 06:11:46.000000000","message":"Done","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ce0c71e4893eddfe1a45ec9109b6c247187b053b","unresolved":true,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"  This will delete a location from an existing image."},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"  DELETE /v2/images/{image_id}/locations/{location_url}"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"Security impact"},{"line_number":163,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"8fdecd9f_f8686911","line":160,"range":{"start_line":160,"start_character":41,"end_line":160,"end_character":55},"in_reply_to":"a5d7568b_32dd4607","updated":"2022-05-23 18:28:50.000000000","message":"Ack, I\u0027ve proposed a solution to this on L#121, let me know your thoughts and I will update the spec.","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7ca97d7f03894c4ca4194852b452b60268a69a97","unresolved":true,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"  This will delete a location from an existing image."},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"  DELETE /v2/images/{image_id}/locations/{location_url}"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"Security impact"},{"line_number":163,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"25e1052d_13d16d8f","line":160,"range":{"start_line":160,"start_character":41,"end_line":160,"end_character":55},"in_reply_to":"b8f90f94_715143de","updated":"2022-05-19 11:16:19.000000000","message":"The only reference for location_id I found is in the database call[1] which is compared to the ID field in image_locations table. As mentioned above, the ID in locations table is just a auto increment number specifying the row number well and could be OK for internal purpose but it isn\u0027t exposed anywhere and doesn\u0027t look like a good candidate for API request. location_url is the only unique identifier I\u0027m aware of in the image_locations table.\n\n[1] https://github.com/openstack/glance/blob/master/glance/db/sqlalchemy/api.py#L1101","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5435dcfb5cb2bedd5fec92d7a238f2911a7f39d3","unresolved":true,"context_lines":[{"line_number":217,"context_line":"  operations."},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"* Update location commands on glanceclient side to call new location APIs."},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"Dependencies"},{"line_number":222,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":223,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"3727a1a4_f175bd2b","line":220,"updated":"2022-05-18 13:50:10.000000000","message":"* Make sure part of the release note for this change includes a reminder to operators about the deprecated (since Newton) show_multiple_locations option and that this time we\u0027re serious about removing it (probably not in AA because nova and cinder may still be using it in AA), but maybe we can say it will be gone in BB.","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7ca97d7f03894c4ca4194852b452b60268a69a97","unresolved":true,"context_lines":[{"line_number":217,"context_line":"  operations."},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"* Update location commands on glanceclient side to call new location APIs."},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"Dependencies"},{"line_number":222,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":223,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"484bc852_6d500b7b","line":220,"in_reply_to":"3727a1a4_f175bd2b","updated":"2022-05-19 11:16:19.000000000","message":"I think that\u0027s an important work item as well and good to document. will add it in the next PS.","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ab9216a55d32a0a989eee24a37c3ef1e09dd64da","unresolved":false,"context_lines":[{"line_number":217,"context_line":"  operations."},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"* Update location commands on glanceclient side to call new location APIs."},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"Dependencies"},{"line_number":222,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":223,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"7c268d42_0f609726","line":220,"in_reply_to":"484bc852_6d500b7b","updated":"2022-05-19 11:19:40.000000000","message":"Done","commit_id":"fef39908fde36014aeb311f249f25f2083729191"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"084e134b817d6399d64ab06ac57af137d80476e0","unresolved":true,"context_lines":[{"line_number":33,"context_line":"users is a side-effect, not the goal.  We currently recommend that operators"},{"line_number":34,"context_line":"who want to use optimized data access use a specialized Glance instance for"},{"line_number":35,"context_line":"services, and only expose glance-api to end users with show_multiple_locations"},{"line_number":36,"context_line":"set False.  This is obviously a cumbersome and unsatisfactory solution."},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"Proposed change"},{"line_number":39,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":6,"id":"5199d1ca_ed077594","line":36,"range":{"start_line":36,"start_character":12,"end_line":36,"end_character":71},"updated":"2022-05-19 18:10:35.000000000","message":"I think this is too strong. It\u0027s not perfect, and has some holes, but AFAIK it\u0027s pretty much the reason we have internal and external API refs in the catalog.","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ce0c71e4893eddfe1a45ec9109b6c247187b053b","unresolved":true,"context_lines":[{"line_number":33,"context_line":"users is a side-effect, not the goal.  We currently recommend that operators"},{"line_number":34,"context_line":"who want to use optimized data access use a specialized Glance instance for"},{"line_number":35,"context_line":"services, and only expose glance-api to end users with show_multiple_locations"},{"line_number":36,"context_line":"set False.  This is obviously a cumbersome and unsatisfactory solution."},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"Proposed change"},{"line_number":39,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":6,"id":"afd6ff4e_462de81f","line":36,"range":{"start_line":36,"start_character":12,"end_line":36,"end_character":71},"in_reply_to":"5199d1ca_ed077594","updated":"2022-05-23 18:28:50.000000000","message":"Ack, will update this line.","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fc83e78e027f19ff4cefff965eaf7d78a993a76b","unresolved":false,"context_lines":[{"line_number":33,"context_line":"users is a side-effect, not the goal.  We currently recommend that operators"},{"line_number":34,"context_line":"who want to use optimized data access use a specialized Glance instance for"},{"line_number":35,"context_line":"services, and only expose glance-api to end users with show_multiple_locations"},{"line_number":36,"context_line":"set False.  This is obviously a cumbersome and unsatisfactory solution."},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"Proposed change"},{"line_number":39,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":6,"id":"8dabc8c5_43b1e3d1","line":36,"range":{"start_line":36,"start_character":12,"end_line":36,"end_character":71},"in_reply_to":"afd6ff4e_462de81f","updated":"2022-05-24 06:11:46.000000000","message":"Done","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"084e134b817d6399d64ab06ac57af137d80476e0","unresolved":true,"context_lines":[{"line_number":215,"context_line":"* Update existing code in the image update operation to perform location"},{"line_number":216,"context_line":"  operations."},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"* Update location commands on glanceclient side to call new location APIs."},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"* Add a releasenote mentioning that we will remove the config option"},{"line_number":221,"context_line":"  ``show_multiple_locations`` when the consumers (nova/cinder) shift to using"}],"source_content_type":"text/x-rst","patch_set":6,"id":"18141bf4_41e52516","line":218,"updated":"2022-05-19 18:10:35.000000000","message":"There is a lot packed into this, which includes nova and cinder using the client differently before it can be fully tested and we move it to the next stage.\n\nI suspect we need specs in the nova and cinder project, which are cross-referenced here.","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ce0c71e4893eddfe1a45ec9109b6c247187b053b","unresolved":true,"context_lines":[{"line_number":215,"context_line":"* Update existing code in the image update operation to perform location"},{"line_number":216,"context_line":"  operations."},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"* Update location commands on glanceclient side to call new location APIs."},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"* Add a releasenote mentioning that we will remove the config option"},{"line_number":221,"context_line":"  ``show_multiple_locations`` when the consumers (nova/cinder) shift to using"}],"source_content_type":"text/x-rst","patch_set":6,"id":"21eefc77_8190bf52","line":218,"in_reply_to":"18141bf4_41e52516","updated":"2022-05-23 18:28:50.000000000","message":"Does glanceclient have support for checking API version and do the request accordingly? or we would need a new set of commands for these new APIs? or we can just implement methods in glanceclient that will be directly called by nova and cinder and we won\u0027t expose them to the end users via CLI?","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fc83e78e027f19ff4cefff965eaf7d78a993a76b","unresolved":false,"context_lines":[{"line_number":215,"context_line":"* Update existing code in the image update operation to perform location"},{"line_number":216,"context_line":"  operations."},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"* Update location commands on glanceclient side to call new location APIs."},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"* Add a releasenote mentioning that we will remove the config option"},{"line_number":221,"context_line":"  ``show_multiple_locations`` when the consumers (nova/cinder) shift to using"}],"source_content_type":"text/x-rst","patch_set":6,"id":"9f899e6c_2b3c936c","line":218,"in_reply_to":"21eefc77_8190bf52","updated":"2022-05-24 06:11:46.000000000","message":"Done","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"084e134b817d6399d64ab06ac57af137d80476e0","unresolved":true,"context_lines":[{"line_number":230,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"* Unit Tests"},{"line_number":233,"context_line":"* Functional Tests"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"Documentation Impact"},{"line_number":236,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":6,"id":"6d0f7abf_349fbf54","line":233,"updated":"2022-05-19 18:10:35.000000000","message":"I think we very clearly need integration tests for something like this, as it\u0027ll affect nova, cinder, glance, ceph, at a minimum. Being a new API that deals with backends, I would think specific tempest tests for the APIs themselves should also be written and I think we should be able to make those fairly comprehensive and out-of-band if we have a cinder backend right? Because we can drive cinder and glance via the API (unlike ceph) to make sure we can add locations properly?","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6d4ef8c055243a5985cc528d52a551e455b796ae","unresolved":true,"context_lines":[{"line_number":230,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"* Unit Tests"},{"line_number":233,"context_line":"* Functional Tests"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"Documentation Impact"},{"line_number":236,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":6,"id":"da92fc1d_e4924e3f","line":233,"in_reply_to":"19211661_00c141f6","updated":"2022-06-10 07:07:07.000000000","message":"Ack, will update it.","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"26bffd912820780456dfeb42a0f8ef456965e145","unresolved":true,"context_lines":[{"line_number":230,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"* Unit Tests"},{"line_number":233,"context_line":"* Functional Tests"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"Documentation Impact"},{"line_number":236,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":6,"id":"19211661_00c141f6","line":233,"in_reply_to":"2c01bc84_1590ab90","updated":"2022-06-09 15:32:07.000000000","message":"The same will be true for nova using glance and ceph, so that\u0027s good. However, that\u0027s incidental testing, which may not cover the full surface or cover error situations, which are always good to exercise with APIs like this.\n\nSo yes, testing from nova and cinder is great, but it still seems like there\u0027s room for actual specific tempest tests for this...","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ce0c71e4893eddfe1a45ec9109b6c247187b053b","unresolved":true,"context_lines":[{"line_number":230,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"* Unit Tests"},{"line_number":233,"context_line":"* Functional Tests"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"Documentation Impact"},{"line_number":236,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":6,"id":"2c01bc84_1590ab90","line":233,"in_reply_to":"6d0f7abf_349fbf54","updated":"2022-05-23 18:28:50.000000000","message":"We\u0027ve a job in cinder[1] that tests create volume from image and other tests[2] which exercises the flow of getting and adding locations from glance. If we have the new APIs implemented in glance, we can propose a patch to cinder using these APIs (and removing show_multiple_locations) to check if it works correctly and that should verify it. We can also add the suggested tempest tests for other operations like in nova glance interaction.\n\n\n[1] https://github.com/openstack/cinder/blob/master/.zuul.yaml#L297-L312\n[2] https://8b1c6e668bf7c30e406f-898f26705d12920a8415687f7498c84f.ssl.cf5.rackcdn.com/839257/3/check/cinder-for-glance-optimized/9ac4d5f/testr_results.html","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6e0f1d3f7fdc01dafc54fe88f58e41afc9ed47ab","unresolved":false,"context_lines":[{"line_number":230,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"* Unit Tests"},{"line_number":233,"context_line":"* Functional Tests"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"Documentation Impact"},{"line_number":236,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":6,"id":"6356de52_cb65e897","line":233,"in_reply_to":"da92fc1d_e4924e3f","updated":"2022-06-10 07:11:47.000000000","message":"Done","commit_id":"61f17734a6274195615cc6b5b4a58e3389c87fe1"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"7862744d6ef8fb95bbefebb176f40bbbd8f032c2","unresolved":true,"context_lines":[{"line_number":38,"context_line":"Proposed change"},{"line_number":39,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"One prerequisite for this work is to introduce the concept of location UUID."},{"line_number":42,"context_line":"Currently there is an ID field in the ``image_locations`` table which is an"},{"line_number":43,"context_line":"integer type and increments as we add location entries to the"},{"line_number":44,"context_line":"``image_locations`` table. We need another field for uniqueness and to make"},{"line_number":45,"context_line":"it consistent with other resources for user operations hence we will be adding"},{"line_number":46,"context_line":"the UUIDs column to the existing ``image_locations`` table."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"This will require altering the ``image_locations`` table to add a new column"},{"line_number":49,"context_line":"``UUID`` of type ``String`` and also a database migration to add UUIDs to"}],"source_content_type":"text/x-rst","patch_set":8,"id":"81e2141c_8ca881d0","line":46,"range":{"start_line":41,"start_character":0,"end_line":46,"end_character":59},"updated":"2022-06-03 11:49:09.000000000","message":"I don\u0027t see reason for this. The image location table already references the current \u0027id\u0027 as primary key with mandatory \u0027image_id\u0027 [0]. This combination is always unique anyways and I don\u0027t see reason why we would ever need/want to reference location outside of it\u0027s image context.\n\nEspecially as all the calls defined below do include the image id as per how the path is defined.\n\n[0] https://github.com/openstack/glance/blob/master/glance/db/sqlalchemy/models.py#L188-L189","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f64df97ea77263a8dd9040b4636ccaf4c177c9be","unresolved":true,"context_lines":[{"line_number":38,"context_line":"Proposed change"},{"line_number":39,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"One prerequisite for this work is to introduce the concept of location UUID."},{"line_number":42,"context_line":"Currently there is an ID field in the ``image_locations`` table which is an"},{"line_number":43,"context_line":"integer type and increments as we add location entries to the"},{"line_number":44,"context_line":"``image_locations`` table. We need another field for uniqueness and to make"},{"line_number":45,"context_line":"it consistent with other resources for user operations hence we will be adding"},{"line_number":46,"context_line":"the UUIDs column to the existing ``image_locations`` table."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"This will require altering the ``image_locations`` table to add a new column"},{"line_number":49,"context_line":"``UUID`` of type ``String`` and also a database migration to add UUIDs to"}],"source_content_type":"text/x-rst","patch_set":8,"id":"c789b1b3_b9489357","line":46,"range":{"start_line":41,"start_character":0,"end_line":46,"end_character":59},"in_reply_to":"81e2141c_8ca881d0","updated":"2022-06-08 05:30:03.000000000","message":"The current \u0027id\u0027 field is an integer one and is not exposed to the end user. Are you suggesting to expose the id field while adding a location and use it for other operations like update, delete etc? I haven\u0027t seen an integer field exposed to end users in any API as a unique identifier so IMO uuid is still a better option here but let\u0027s see what others think.","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"3c79b37b0913008a8806c4de4e4cdec3f1a65b68","unresolved":true,"context_lines":[{"line_number":38,"context_line":"Proposed change"},{"line_number":39,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"One prerequisite for this work is to introduce the concept of location UUID."},{"line_number":42,"context_line":"Currently there is an ID field in the ``image_locations`` table which is an"},{"line_number":43,"context_line":"integer type and increments as we add location entries to the"},{"line_number":44,"context_line":"``image_locations`` table. We need another field for uniqueness and to make"},{"line_number":45,"context_line":"it consistent with other resources for user operations hence we will be adding"},{"line_number":46,"context_line":"the UUIDs column to the existing ``image_locations`` table."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"This will require altering the ``image_locations`` table to add a new column"},{"line_number":49,"context_line":"``UUID`` of type ``String`` and also a database migration to add UUIDs to"}],"source_content_type":"text/x-rst","patch_set":8,"id":"dd876e12_80bb3a1a","line":46,"range":{"start_line":41,"start_character":0,"end_line":46,"end_character":59},"in_reply_to":"c789b1b3_b9489357","updated":"2022-06-08 06:22:22.000000000","message":"You are mentioning below that this spec is not going to implement new update and delete location API, so still this is required?","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ab2369798e1f8bc28720e83d443c8bcf7ec6c9f0","unresolved":false,"context_lines":[{"line_number":38,"context_line":"Proposed change"},{"line_number":39,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"One prerequisite for this work is to introduce the concept of location UUID."},{"line_number":42,"context_line":"Currently there is an ID field in the ``image_locations`` table which is an"},{"line_number":43,"context_line":"integer type and increments as we add location entries to the"},{"line_number":44,"context_line":"``image_locations`` table. We need another field for uniqueness and to make"},{"line_number":45,"context_line":"it consistent with other resources for user operations hence we will be adding"},{"line_number":46,"context_line":"the UUIDs column to the existing ``image_locations`` table."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"This will require altering the ``image_locations`` table to add a new column"},{"line_number":49,"context_line":"``UUID`` of type ``String`` and also a database migration to add UUIDs to"}],"source_content_type":"text/x-rst","patch_set":8,"id":"0a6f0138_16b5f017","line":46,"range":{"start_line":41,"start_character":0,"end_line":46,"end_character":59},"in_reply_to":"dd876e12_80bb3a1a","updated":"2022-06-08 07:22:29.000000000","message":"Done\nSomehow i thought it was still used in the GET API. removed this part now.","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"7862744d6ef8fb95bbefebb176f40bbbd8f032c2","unresolved":true,"context_lines":[{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    - Error - 404 (Image ID does not exist)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"* Update Location Metadata"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"  This will modify the metadata of a location of an existing image. We can"},{"line_number":150,"context_line":"  restrict certain metadata fields from updation like ``store``."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"  PUT /v2/images/{image_id}/locations/{location_id}/metadata"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"  * JSON request body"},{"line_number":155,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"efbf549b_44d385bd","line":152,"range":{"start_line":147,"start_character":0,"end_line":152,"end_character":60},"updated":"2022-06-03 11:49:09.000000000","message":"I\u0027m not sure what is the benefit of this call.","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7532275c25510ee8df902978d9085dbeab29b636","unresolved":false,"context_lines":[{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    - Error - 404 (Image ID does not exist)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"* Update Location Metadata"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"  This will modify the metadata of a location of an existing image. We can"},{"line_number":150,"context_line":"  restrict certain metadata fields from updation like ``store``."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"  PUT /v2/images/{image_id}/locations/{location_id}/metadata"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"  * JSON request body"},{"line_number":155,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"06ce5d3b_940bfac0","line":152,"range":{"start_line":147,"start_character":0,"end_line":152,"end_character":60},"in_reply_to":"809dc470_51b8013f","updated":"2022-06-08 05:33:44.000000000","message":"Removed this since it serves no benefit in the current usecase.","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f64df97ea77263a8dd9040b4636ccaf4c177c9be","unresolved":true,"context_lines":[{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    - Error - 404 (Image ID does not exist)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"* Update Location Metadata"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"  This will modify the metadata of a location of an existing image. We can"},{"line_number":150,"context_line":"  restrict certain metadata fields from updation like ``store``."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"  PUT /v2/images/{image_id}/locations/{location_id}/metadata"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"  * JSON request body"},{"line_number":155,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"809dc470_51b8013f","line":152,"range":{"start_line":147,"start_character":0,"end_line":152,"end_character":60},"in_reply_to":"efbf549b_44d385bd","updated":"2022-06-08 05:30:03.000000000","message":"The usecase is if an end user wants to associate certain attributes with a particular location. Since we already have a metadata field and we already allow updating it via the current image update call, this seemed beneficial as a new API.\nThe initial thought behind this was that users could perform location operations but the authorization has changed from ADMIN -\u003e SERVICE so not sure anymore.","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"7862744d6ef8fb95bbefebb176f40bbbd8f032c2","unresolved":true,"context_lines":[{"line_number":169,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027cephdriver-1\u0027}\""},{"line_number":170,"context_line":"        }"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"* Delete Location"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"  This will delete a location from an existing image."},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"  DELETE /v2/images/{image_id}/locations/{location_id}"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"Security impact"},{"line_number":179,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":8,"id":"2f74b882_aff83b76","line":176,"range":{"start_line":172,"start_character":0,"end_line":176,"end_character":54},"updated":"2022-06-03 11:49:09.000000000","message":"I\u0027m not sure if we need this call either. With multi-store config model we already are able to remove image from a store, this feels redundant and cumbersome.","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f64df97ea77263a8dd9040b4636ccaf4c177c9be","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027cephdriver-1\u0027}\""},{"line_number":170,"context_line":"        }"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"* Delete Location"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"  This will delete a location from an existing image."},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"  DELETE /v2/images/{image_id}/locations/{location_id}"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"Security impact"},{"line_number":179,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":8,"id":"d00d86ba_3523d73a","line":176,"range":{"start_line":172,"start_character":0,"end_line":176,"end_character":54},"in_reply_to":"2f74b882_aff83b76","updated":"2022-06-08 05:30:03.000000000","message":"Ack, will remove this\nNOTE: in favor of https://docs.openstack.org/api-ref/image/v2/index.html?expanded\u003ddelete-image-from-store-detail#delete-image-from-store","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7532275c25510ee8df902978d9085dbeab29b636","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027cephdriver-1\u0027}\""},{"line_number":170,"context_line":"        }"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"* Delete Location"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"  This will delete a location from an existing image."},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"  DELETE /v2/images/{image_id}/locations/{location_id}"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"Security impact"},{"line_number":179,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":8,"id":"9304d70f_e1218d18","line":176,"range":{"start_line":172,"start_character":0,"end_line":176,"end_character":54},"in_reply_to":"d00d86ba_3523d73a","updated":"2022-06-08 05:33:44.000000000","message":"Done","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"7862744d6ef8fb95bbefebb176f40bbbd8f032c2","unresolved":true,"context_lines":[{"line_number":190,"context_line":"Other end user impact"},{"line_number":191,"context_line":"---------------------"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"Since the new APIs are for service to service interaction, there is not much"},{"line_number":194,"context_line":"value to expose them via client commands. We will add methods to the client"},{"line_number":195,"context_line":"(that will call the new location APIs) that will be used by other services"},{"line_number":196,"context_line":"like cinder and nova but those methods won\u0027t be exposed via the shell to end"},{"line_number":197,"context_line":"users. End users can still use the existing commands (that internally calls"}],"source_content_type":"text/x-rst","patch_set":8,"id":"7b3345a0_364107fe","line":194,"range":{"start_line":193,"start_character":0,"end_line":194,"end_character":41},"updated":"2022-06-03 11:49:09.000000000","message":"As we do need to write the actual client code, there is very little work to expose the commands on the command line. There is nothing preventing admin getting service level token. optionally we could set the policies for admin and service. There is plenty of value and very little work to do so.\n\nThese options become especially important if the old locations api is getting removed as it\u0027s the only way to use http-store.","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"80f726363e079e6271137cdf74dd9d14ffc3ce44","unresolved":true,"context_lines":[{"line_number":190,"context_line":"Other end user impact"},{"line_number":191,"context_line":"---------------------"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"Since the new APIs are for service to service interaction, there is not much"},{"line_number":194,"context_line":"value to expose them via client commands. We will add methods to the client"},{"line_number":195,"context_line":"(that will call the new location APIs) that will be used by other services"},{"line_number":196,"context_line":"like cinder and nova but those methods won\u0027t be exposed via the shell to end"},{"line_number":197,"context_line":"users. End users can still use the existing commands (that internally calls"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3251b40e_2cfb543b","line":194,"range":{"start_line":193,"start_character":0,"end_line":194,"end_character":41},"in_reply_to":"2eb20014_426bb6e6","updated":"2022-06-08 18:56:17.000000000","message":"We are not removing the support of operations performed on locations via the image-update call rather we are introducing a new code path via these new APIs to be used by nova and cinder to add/get locations for optimization purposes.\nSo I guess for now, operators/end-users should use the existing commands (mentioned below) which use the image-update call to perform location operations and not rely on the new add/get APIs that are specific for service to service interaction.\nIf in future, we plan to remove support of image-update call to modify locations and add new APIs to support all cases, then it makes sense to introduce new commands or modify existing commands to use new location APIs.","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"26bffd912820780456dfeb42a0f8ef456965e145","unresolved":true,"context_lines":[{"line_number":190,"context_line":"Other end user impact"},{"line_number":191,"context_line":"---------------------"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"Since the new APIs are for service to service interaction, there is not much"},{"line_number":194,"context_line":"value to expose them via client commands. We will add methods to the client"},{"line_number":195,"context_line":"(that will call the new location APIs) that will be used by other services"},{"line_number":196,"context_line":"like cinder and nova but those methods won\u0027t be exposed via the shell to end"},{"line_number":197,"context_line":"users. End users can still use the existing commands (that internally calls"}],"source_content_type":"text/x-rst","patch_set":8,"id":"e226639a_a73cfd8d","line":194,"range":{"start_line":193,"start_character":0,"end_line":194,"end_character":41},"in_reply_to":"3251b40e_2cfb543b","updated":"2022-06-09 15:32:07.000000000","message":"I guess I\u0027m confused here. I thought you said there was no change required to nova (for example) because it would all be hidden in the client, but then also that there\u0027s no client changes needed here. Are you conflating \"client\" with \"CLI\" ? If so, it might be good to change the words used to make it clearer.","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f64df97ea77263a8dd9040b4636ccaf4c177c9be","unresolved":true,"context_lines":[{"line_number":190,"context_line":"Other end user impact"},{"line_number":191,"context_line":"---------------------"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"Since the new APIs are for service to service interaction, there is not much"},{"line_number":194,"context_line":"value to expose them via client commands. We will add methods to the client"},{"line_number":195,"context_line":"(that will call the new location APIs) that will be used by other services"},{"line_number":196,"context_line":"like cinder and nova but those methods won\u0027t be exposed via the shell to end"},{"line_number":197,"context_line":"users. End users can still use the existing commands (that internally calls"}],"source_content_type":"text/x-rst","patch_set":8,"id":"b9e7a517_10e5fead","line":194,"range":{"start_line":193,"start_character":0,"end_line":194,"end_character":41},"in_reply_to":"7b3345a0_364107fe","updated":"2022-06-08 05:30:03.000000000","message":"Do you mean we should expose the APIs as client commands so if an admin wants to get service level token or alter the policy rule to perform any such operation, they can?\n\nThe original design was to remove old locations code in favor of these new APIs.","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"2052edf381d8d88025e99a4278f571b67a0efa7b","unresolved":true,"context_lines":[{"line_number":190,"context_line":"Other end user impact"},{"line_number":191,"context_line":"---------------------"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"Since the new APIs are for service to service interaction, there is not much"},{"line_number":194,"context_line":"value to expose them via client commands. We will add methods to the client"},{"line_number":195,"context_line":"(that will call the new location APIs) that will be used by other services"},{"line_number":196,"context_line":"like cinder and nova but those methods won\u0027t be exposed via the shell to end"},{"line_number":197,"context_line":"users. End users can still use the existing commands (that internally calls"}],"source_content_type":"text/x-rst","patch_set":8,"id":"2eb20014_426bb6e6","line":194,"range":{"start_line":193,"start_character":0,"end_line":194,"end_character":41},"in_reply_to":"b9e7a517_10e5fead","updated":"2022-06-08 12:24:35.000000000","message":"Yep, that was my point. If we are removing the old locations API via image update call, we should have what ever new APIs we expose available in CLI for the admins to use either with custom policies or utilizing the service token manually. Still better options than needing to curl it because we didn\u0027t bother to write those 20lines of boilerplate code to expose the functions to CLI.","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6d4ef8c055243a5985cc528d52a551e455b796ae","unresolved":true,"context_lines":[{"line_number":190,"context_line":"Other end user impact"},{"line_number":191,"context_line":"---------------------"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"Since the new APIs are for service to service interaction, there is not much"},{"line_number":194,"context_line":"value to expose them via client commands. We will add methods to the client"},{"line_number":195,"context_line":"(that will call the new location APIs) that will be used by other services"},{"line_number":196,"context_line":"like cinder and nova but those methods won\u0027t be exposed via the shell to end"},{"line_number":197,"context_line":"users. End users can still use the existing commands (that internally calls"}],"source_content_type":"text/x-rst","patch_set":8,"id":"f90d45c7_8350452e","line":194,"range":{"start_line":193,"start_character":0,"end_line":194,"end_character":41},"in_reply_to":"e226639a_a73cfd8d","updated":"2022-06-10 07:07:07.000000000","message":"Yeah, sorry for the confusion, I meant CLI when i wrote client commands. will update it.","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6e0f1d3f7fdc01dafc54fe88f58e41afc9ed47ab","unresolved":false,"context_lines":[{"line_number":190,"context_line":"Other end user impact"},{"line_number":191,"context_line":"---------------------"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"Since the new APIs are for service to service interaction, there is not much"},{"line_number":194,"context_line":"value to expose them via client commands. We will add methods to the client"},{"line_number":195,"context_line":"(that will call the new location APIs) that will be used by other services"},{"line_number":196,"context_line":"like cinder and nova but those methods won\u0027t be exposed via the shell to end"},{"line_number":197,"context_line":"users. End users can still use the existing commands (that internally calls"}],"source_content_type":"text/x-rst","patch_set":8,"id":"f86dafef_f5af1a81","line":194,"range":{"start_line":193,"start_character":0,"end_line":194,"end_character":41},"in_reply_to":"f90d45c7_8350452e","updated":"2022-06-10 07:11:47.000000000","message":"Done","commit_id":"7977df163fe74c03d52f846612e222ec587e4daa"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"2052edf381d8d88025e99a4278f571b67a0efa7b","unresolved":true,"context_lines":[{"line_number":126,"context_line":"            {"},{"line_number":127,"context_line":"                \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":128,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027,"},{"line_number":129,"context_line":"                              \u0027id\u0027: \u00273925e12f-0414-47e0-a7c4-3cb90cb79c81\u0027,"},{"line_number":130,"context_line":"                              \u0027status\u0027: \u0027active\u0027}\""},{"line_number":131,"context_line":"            },"},{"line_number":132,"context_line":"            {"}],"source_content_type":"text/x-rst","patch_set":10,"id":"2911b64a_82a479ba","line":129,"range":{"start_line":129,"start_character":36,"end_line":129,"end_character":74},"updated":"2022-06-08 12:24:35.000000000","message":"I think this ID is just the integer index and it should be either exposed on all entries or not at all. (see the example output below.)","commit_id":"5f1944543590c60393ae45f1c0242f9b798acd5a"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"80f726363e079e6271137cdf74dd9d14ffc3ce44","unresolved":false,"context_lines":[{"line_number":126,"context_line":"            {"},{"line_number":127,"context_line":"                \"url\": \"cinder://lvmdriver-1/0f031ed1-5872-43d5-a638-4b0d07c10ab5\","},{"line_number":128,"context_line":"                \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027,"},{"line_number":129,"context_line":"                              \u0027id\u0027: \u00273925e12f-0414-47e0-a7c4-3cb90cb79c81\u0027,"},{"line_number":130,"context_line":"                              \u0027status\u0027: \u0027active\u0027}\""},{"line_number":131,"context_line":"            },"},{"line_number":132,"context_line":"            {"}],"source_content_type":"text/x-rst","patch_set":10,"id":"7010fc63_690a76f2","line":129,"range":{"start_line":129,"start_character":36,"end_line":129,"end_character":74},"in_reply_to":"2911b64a_82a479ba","updated":"2022-06-08 18:56:17.000000000","message":"Done","commit_id":"5f1944543590c60393ae45f1c0242f9b798acd5a"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"fb6fe9f38979af079d390338dfc47d4fa1261ebc","unresolved":true,"context_lines":[{"line_number":21,"context_line":"or else a non-admin user cannot associate data with an image record, or"},{"line_number":22,"context_line":"retrieve image data, or delete image data), then non-admin users"},{"line_number":23,"context_line":"can modify location data to corrupt an image that they own."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"When show_multiple_locations is False, non-admin users cannot modify image"},{"line_number":26,"context_line":"locations via the image-update API call, even if they have the"},{"line_number":27,"context_line":"``{get,set,delete}_image_location`` permissions.  However, there are some"}],"source_content_type":"text/x-rst","patch_set":11,"id":"61a56d71_06e7a193","line":24,"updated":"2022-06-09 23:18:55.000000000","message":"So this might just be my French brain here, but this one-sentence paragraph is tough to understand. There is a mix of conditons (\"if\"), lists and parentheses. If another patchset is needed, I would really like to see this broken down in multiple short, simple sentences.","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6e0f1d3f7fdc01dafc54fe88f58e41afc9ed47ab","unresolved":false,"context_lines":[{"line_number":21,"context_line":"or else a non-admin user cannot associate data with an image record, or"},{"line_number":22,"context_line":"retrieve image data, or delete image data), then non-admin users"},{"line_number":23,"context_line":"can modify location data to corrupt an image that they own."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"When show_multiple_locations is False, non-admin users cannot modify image"},{"line_number":26,"context_line":"locations via the image-update API call, even if they have the"},{"line_number":27,"context_line":"``{get,set,delete}_image_location`` permissions.  However, there are some"}],"source_content_type":"text/x-rst","patch_set":11,"id":"afb3c562_93f6a3c9","line":24,"in_reply_to":"61a56d71_06e7a193","updated":"2022-06-10 07:11:47.000000000","message":"Done","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"fb6fe9f38979af079d390338dfc47d4fa1261ebc","unresolved":true,"context_lines":[{"line_number":50,"context_line":"3. Remove ``show_multiple_locations`` config option when it is no longer"},{"line_number":51,"context_line":"   required by other services (cinder/nova) to perform operations on"},{"line_number":52,"context_line":"   locations."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"The config option ``show_multiple_locations`` has been deprecated since Newton"},{"line_number":55,"context_line":"but we will keep the config option until the consumers of glance locations"},{"line_number":56,"context_line":"(nova, cinder etc) start using the new location APIs."}],"source_content_type":"text/x-rst","patch_set":11,"id":"7f6808c5_c6d980ba","line":53,"updated":"2022-06-09 23:18:55.000000000","message":"Are we deprecating the location-related operations in \"PATCH /v2/images/{image_id}\"? They seem a tiny bit buggy (https://bugs.launchpad.net/glance/+bug/1914531) and having multiple ways of editing locations might be confusing to users.","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"6e3edff4a735abca558023e330691c6446f7564b","unresolved":false,"context_lines":[{"line_number":50,"context_line":"3. Remove ``show_multiple_locations`` config option when it is no longer"},{"line_number":51,"context_line":"   required by other services (cinder/nova) to perform operations on"},{"line_number":52,"context_line":"   locations."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"The config option ``show_multiple_locations`` has been deprecated since Newton"},{"line_number":55,"context_line":"but we will keep the config option until the consumers of glance locations"},{"line_number":56,"context_line":"(nova, cinder etc) start using the new location APIs."}],"source_content_type":"text/x-rst","patch_set":11,"id":"118e17c7_6eb762ae","line":53,"in_reply_to":"41302b07_432c3089","updated":"2022-06-14 03:21:58.000000000","message":"Ack","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6d4ef8c055243a5985cc528d52a551e455b796ae","unresolved":true,"context_lines":[{"line_number":50,"context_line":"3. Remove ``show_multiple_locations`` config option when it is no longer"},{"line_number":51,"context_line":"   required by other services (cinder/nova) to perform operations on"},{"line_number":52,"context_line":"   locations."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"The config option ``show_multiple_locations`` has been deprecated since Newton"},{"line_number":55,"context_line":"but we will keep the config option until the consumers of glance locations"},{"line_number":56,"context_line":"(nova, cinder etc) start using the new location APIs."}],"source_content_type":"text/x-rst","patch_set":11,"id":"41302b07_432c3089","line":53,"in_reply_to":"7f6808c5_c6d980ba","updated":"2022-06-10 07:07:07.000000000","message":"No, we are not. The initial intent of this spec was to have new set of APIs that would replace the image update call for location operations but that had some concerns so now the current spec is narrowed down to only serve the purpose of service to service interaction.\n\nCurrently most of the users that want to have optimizations enabled for glance cinder use this config option ``show_multiple_locations`` which also exposes the security vulnerability so we\u0027re addressing those usecases. Also this option is deprecated so removing it should not be controversial but will be done after nova and cinder adopts to these new location APIs which is not part of the initial work i.e. to introduce new location APIs.","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"fb6fe9f38979af079d390338dfc47d4fa1261ebc","unresolved":true,"context_lines":[{"line_number":63,"context_line":"``Update``: For service to service interaction, there is no value in updating"},{"line_number":64,"context_line":"the metadata of a location. This would be beneficial if we plan to remove the"},{"line_number":65,"context_line":"existing location code from image-update call and support the usecase of"},{"line_number":66,"context_line":"operators/end-users doing location operations."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"``Delete``: We already have `Delete Image From Store`_ API for this purpose."},{"line_number":69,"context_line":"Currently, the policy rule defaults to ``role:admin`` which will be changed to"}],"source_content_type":"text/x-rst","patch_set":11,"id":"326dbfce_7b13604c","line":66,"updated":"2022-06-09 23:18:55.000000000","message":"Some users feel the need to *insert* locations (rather than just append them). See https://bugzilla.redhat.com/show_bug.cgi?id\u003d1891504 for instance. Is their use case covered by this spec?","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6d4ef8c055243a5985cc528d52a551e455b796ae","unresolved":true,"context_lines":[{"line_number":63,"context_line":"``Update``: For service to service interaction, there is no value in updating"},{"line_number":64,"context_line":"the metadata of a location. This would be beneficial if we plan to remove the"},{"line_number":65,"context_line":"existing location code from image-update call and support the usecase of"},{"line_number":66,"context_line":"operators/end-users doing location operations."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"``Delete``: We already have `Delete Image From Store`_ API for this purpose."},{"line_number":69,"context_line":"Currently, the policy rule defaults to ``role:admin`` which will be changed to"}],"source_content_type":"text/x-rst","patch_set":11,"id":"a33834aa_b37bbf05","line":66,"in_reply_to":"326dbfce_7b13604c","updated":"2022-06-10 07:07:07.000000000","message":"No, as said above, initially it was but now we are focusing on service to service interaction and nova and cinder doesn\u0027t require this call. Strictly speaking, this is not to target end users/operators but other services.","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"6e3edff4a735abca558023e330691c6446f7564b","unresolved":false,"context_lines":[{"line_number":63,"context_line":"``Update``: For service to service interaction, there is no value in updating"},{"line_number":64,"context_line":"the metadata of a location. This would be beneficial if we plan to remove the"},{"line_number":65,"context_line":"existing location code from image-update call and support the usecase of"},{"line_number":66,"context_line":"operators/end-users doing location operations."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"``Delete``: We already have `Delete Image From Store`_ API for this purpose."},{"line_number":69,"context_line":"Currently, the policy rule defaults to ``role:admin`` which will be changed to"}],"source_content_type":"text/x-rst","patch_set":11,"id":"1f46c2cf_16e3b032","line":66,"in_reply_to":"a33834aa_b37bbf05","updated":"2022-06-14 03:21:58.000000000","message":"Ack","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"26bffd912820780456dfeb42a0f8ef456965e145","unresolved":true,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"``Delete``: We already have `Delete Image From Store`_ API for this purpose."},{"line_number":69,"context_line":"Currently, the policy rule defaults to ``role:admin`` which will be changed to"},{"line_number":70,"context_line":"``role:admin or role:service``."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"Alternatives"},{"line_number":73,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":11,"id":"859702cd_fb80a634","line":70,"updated":"2022-06-09 15:32:07.000000000","message":"Even though both of these are not being implemented right away, I still think that having a unique identifier in the GET result is prudent. If we add update/delete in the future, then instead of just allowing those operations, we also have to change the behavior of GET to enable it.\n\nI also think that returning the full location from POST, with the generated unique identifier creates the proper client behavior workflow of \"Create a thing, grab the identifier\", even if there\u0027s no immediate need at the moment.\n\nHaving APIs that are supposed to be RESTful that are designed in a way that they *cannot* be RESTful without changes is not very productive, IMHO.\n\nNot a hard -1 thing for me, but I think we should strongly consider \"doing it right\".","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e9cb5866b5e9604c91b832f7444853f5ab98098e","unresolved":true,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"``Delete``: We already have `Delete Image From Store`_ API for this purpose."},{"line_number":69,"context_line":"Currently, the policy rule defaults to ``role:admin`` which will be changed to"},{"line_number":70,"context_line":"``role:admin or role:service``."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"Alternatives"},{"line_number":73,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":11,"id":"8379b389_c6953668","line":70,"in_reply_to":"2b5fa260_05e70839","updated":"2022-06-10 14:09:12.000000000","message":"Well, he said he didn\u0027t see the need for it, but it didn\u0027t seem like he was necessarily very opposed to it.\n\nYears of experience cleaning up problems in nova that stemmed from exposing row-correlation primary key IDs to external users tells me that it is absolutely a terrible idea to do so. Just because imageid+id is unique doesn\u0027t mean we should expose those, regardless of whether or not they\u0027re only ever referenced within the scope of an image.\n\nAnyway, you\u0027ve cut down the functionality of what you\u0027re proposing here to the point that it doesn\u0027t _need_ the unique ID part, so there\u0027s no point in demanding that you do it. However, if it were me, I\u0027d just go ahead and do that now so that we\u0027re not substantially changing the API later. If the argument is really that integer IDs are better, I\u0027ll be glad to further support that position, and if the concern is over the work required to add and test them, then I\u0027m also willing to help with that part.\n\nBut again, I acknowledge it\u0027s not critical for this first pass, so I shan\u0027t die on this hill :)","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c0ad8ed4bec162bdfcdfef92aabaecda7d85974c","unresolved":true,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"``Delete``: We already have `Delete Image From Store`_ API for this purpose."},{"line_number":69,"context_line":"Currently, the policy rule defaults to ``role:admin`` which will be changed to"},{"line_number":70,"context_line":"``role:admin or role:service``."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"Alternatives"},{"line_number":73,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":11,"id":"e1fc468c_29524962","line":70,"in_reply_to":"8379b389_c6953668","updated":"2022-06-13 16:42:59.000000000","message":"My biggest concern is how heavy the migration operation will be, even being just one-off task. For a cloud that has thousands or tens of thousands of images with potentially multiple locations on them updating every single one having uuid, not because we need one but because it would be cool to have one, is not exactly something I\u0027d be happy to force in.\n\nSure it will also add work, and unnecessary tests to be ran in the infra multiple times for every patch in the future. So I see very little wins to implement them.","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6d4ef8c055243a5985cc528d52a551e455b796ae","unresolved":true,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"``Delete``: We already have `Delete Image From Store`_ API for this purpose."},{"line_number":69,"context_line":"Currently, the policy rule defaults to ``role:admin`` which will be changed to"},{"line_number":70,"context_line":"``role:admin or role:service``."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"Alternatives"},{"line_number":73,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":11,"id":"2b5fa260_05e70839","line":70,"in_reply_to":"859702cd_fb80a634","updated":"2022-06-10 07:07:07.000000000","message":"I agree with what you said but introducing the unique identifier has conflicting opinion from different reviewers. Initially I tried to introduce a new UUID field in locations table but Erno has the opinion that our current ID (integer) field combined with image id acts as a unique identifier (see previous patchsets for reference). The unique identifier was important before for update and delete call but going into this discussion without any current use seems exhausting. I can add the integer ID field (which we currently have in locations table) as a response for add, get if you agree with Erno\u0027s opinion but if we plan to add the new UUID field, it will be a whole new discussion and will also increase the scope of work, testing and possible regression cases which I would suggest to do as a followup. let me know what you think.","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5a9ea488aafc7c89cec0e8b86ef7b26e1766c239","unresolved":true,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"``Delete``: We already have `Delete Image From Store`_ API for this purpose."},{"line_number":69,"context_line":"Currently, the policy rule defaults to ``role:admin`` which will be changed to"},{"line_number":70,"context_line":"``role:admin or role:service``."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"Alternatives"},{"line_number":73,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":11,"id":"2d80288a_a2bbe08d","line":70,"in_reply_to":"e1fc468c_29524962","updated":"2022-06-15 16:49:26.000000000","message":"I understand the data migration issue, and would definitely not recommend the naive approach of converting everything all at once during an upgrade. In other projects we have millions of resources that have to be updated in the same way. The established practice for something like this is to migrate-on-load.\n\nA pre-existing thing doesn\u0027t need a unique identifier until it\u0027s examined, so any time we load a location record (due to an API request or other action that looks at it), if it doesn\u0027t have an identifier, we generate one, save it back, and then continue on. From the outside, it looks like everything is already migrated, but in reality it just happens when we look. Like \"Schrödinger resources\".","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"fb6fe9f38979af079d390338dfc47d4fa1261ebc","unresolved":true,"context_lines":[{"line_number":109,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":110,"context_line":"        }"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    - Error - 409 (Location already exists)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"* Get Location(s)"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"7bcd9f0f_d60a2694","line":112,"range":{"start_line":112,"start_character":6,"end_line":112,"end_character":11},"updated":"2022-06-09 23:18:55.000000000","message":"I take it this can also return a 404?","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6d4ef8c055243a5985cc528d52a551e455b796ae","unresolved":true,"context_lines":[{"line_number":109,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":110,"context_line":"        }"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    - Error - 409 (Location already exists)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"* Get Location(s)"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"9a873bdc_2fd69075","line":112,"range":{"start_line":112,"start_character":6,"end_line":112,"end_character":11},"in_reply_to":"7bcd9f0f_d60a2694","updated":"2022-06-10 07:07:07.000000000","message":"404 is generally for GET requests when a resource is not found, I\u0027m not sure how a location add call can get a 404.","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"4d669b6b15926bdf271fb998f3e6e6bea6275e30","unresolved":false,"context_lines":[{"line_number":109,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":110,"context_line":"        }"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    - Error - 409 (Location already exists)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"* Get Location(s)"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"ea8bd80c_88afe06d","line":112,"range":{"start_line":112,"start_character":6,"end_line":112,"end_character":11},"in_reply_to":"99755933_7edc290b","updated":"2022-06-14 05:57:48.000000000","message":"Done\nWas checking if 403 or 401 made sense here but 403 suits more since users have valid credentials but not enough authority to access this API.","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e9cb5866b5e9604c91b832f7444853f5ab98098e","unresolved":true,"context_lines":[{"line_number":109,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":110,"context_line":"        }"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    - Error - 409 (Location already exists)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"* Get Location(s)"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"bdc44b3b_733655cb","line":112,"range":{"start_line":112,"start_character":6,"end_line":112,"end_character":11},"in_reply_to":"9a873bdc_2fd69075","updated":"2022-06-10 14:09:12.000000000","message":"Yeah, a POST should only return a 404 if they use a URL that doesn\u0027t exist. If it does and you\u0027re not allowed to post there, then it should be 405: Method Not Allowed.","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"6e3edff4a735abca558023e330691c6446f7564b","unresolved":true,"context_lines":[{"line_number":109,"context_line":"            \"metadata\": \"{\u0027store\u0027: \u0027lvmdriver-1\u0027}\""},{"line_number":110,"context_line":"        }"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    - Error - 409 (Location already exists)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"* Get Location(s)"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-rst","patch_set":11,"id":"99755933_7edc290b","line":112,"range":{"start_line":112,"start_character":6,"end_line":112,"end_character":11},"in_reply_to":"bdc44b3b_733655cb","updated":"2022-06-14 03:21:58.000000000","message":"Sorry, I meant 403 (Permission Denied). If a user attempts to add a location to an image they are not allowed to view/modify.","commit_id":"5c85c34300a7298e5f59738ba332a839e6c38958"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"c0ad8ed4bec162bdfcdfef92aabaecda7d85974c","unresolved":true,"context_lines":[{"line_number":22,"context_line":"else a non-admin user cannot associate data with an image record, or retrieve"},{"line_number":23,"context_line":"image data, or delete image data."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"When show_multiple_locations is False, non-admin users cannot modify image"},{"line_number":26,"context_line":"locations via the image-update API call, even if they have the"},{"line_number":27,"context_line":"``{get,set,delete}_image_location`` permissions.  However, there are some"},{"line_number":28,"context_line":"popular use cases where other services can bypass Glance and store or access"}],"source_content_type":"text/x-rst","patch_set":13,"id":"d0ae2bfb_4fca5a01","line":25,"range":{"start_line":25,"start_character":39,"end_line":25,"end_character":48},"updated":"2022-06-13 16:42:59.000000000","message":"Not 100% but I\u0027m pretty sure this is redundant and misleading. I don\u0027t think even admin can be poking the locatins through image-update call if the show_multipe_locations is set to false.","commit_id":"c2fbc80968496044e0e8dfb858fab00bcba97844"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"4d669b6b15926bdf271fb998f3e6e6bea6275e30","unresolved":false,"context_lines":[{"line_number":22,"context_line":"else a non-admin user cannot associate data with an image record, or retrieve"},{"line_number":23,"context_line":"image data, or delete image data."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"When show_multiple_locations is False, non-admin users cannot modify image"},{"line_number":26,"context_line":"locations via the image-update API call, even if they have the"},{"line_number":27,"context_line":"``{get,set,delete}_image_location`` permissions.  However, there are some"},{"line_number":28,"context_line":"popular use cases where other services can bypass Glance and store or access"}],"source_content_type":"text/x-rst","patch_set":13,"id":"38865f49_5b4efc20","line":25,"range":{"start_line":25,"start_character":39,"end_line":25,"end_character":48},"in_reply_to":"d0ae2bfb_4fca5a01","updated":"2022-06-14 05:57:48.000000000","message":"Done, yeah any user is not allowed for location operations unless show_multipe_locations is true. https://github.com/openstack/glance/blob/bfcab95ff2d7e13afe05fed8835159949490afd8/glance/api/v2/images.py#L1043","commit_id":"c2fbc80968496044e0e8dfb858fab00bcba97844"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"450afc87272ca3007483ed6a6473a4272cd384bc","unresolved":true,"context_lines":[{"line_number":195,"context_line":""},{"line_number":196,"context_line":"* Add 2 new Location APIs for add and get operations."},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"* Modify consumers like cinder and nova to use the new location APIs."},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"* Add a releasenote mentioning that we will remove the config option"},{"line_number":201,"context_line":"  ``show_multiple_locations`` when the consumers (nova/cinder) shift to using"}],"source_content_type":"text/x-rst","patch_set":14,"id":"3e1408a2_4dabc2d3","line":198,"updated":"2022-06-15 05:15:15.000000000","message":"I think one work item should be to modify delete_from_store policy to role:admin or role:service","commit_id":"2b92aebb19773cc97f974949c6e2c317aa9e3a59"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"033b30532f9be11db5e554f31c4c097a2710396e","unresolved":true,"context_lines":[{"line_number":195,"context_line":""},{"line_number":196,"context_line":"* Add 2 new Location APIs for add and get operations."},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"* Modify consumers like cinder and nova to use the new location APIs."},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"* Add a releasenote mentioning that we will remove the config option"},{"line_number":201,"context_line":"  ``show_multiple_locations`` when the consumers (nova/cinder) shift to using"}],"source_content_type":"text/x-rst","patch_set":14,"id":"37952db1_60a9309b","line":198,"in_reply_to":"11303e33_5b240f8a","updated":"2022-06-15 07:48:19.000000000","message":"Agree, then you can remove it from proposed change section at line #68","commit_id":"2b92aebb19773cc97f974949c6e2c317aa9e3a59"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"e7cd22b9909b5e322c970a97a771367bdecb16b3","unresolved":false,"context_lines":[{"line_number":195,"context_line":""},{"line_number":196,"context_line":"* Add 2 new Location APIs for add and get operations."},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"* Modify consumers like cinder and nova to use the new location APIs."},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"* Add a releasenote mentioning that we will remove the config option"},{"line_number":201,"context_line":"  ``show_multiple_locations`` when the consumers (nova/cinder) shift to using"}],"source_content_type":"text/x-rst","patch_set":14,"id":"7c3b5edd_f1d22787","line":198,"in_reply_to":"37952db1_60a9309b","updated":"2022-06-15 07:54:45.000000000","message":"Ack, updated the part in proposed change.","commit_id":"2b92aebb19773cc97f974949c6e2c317aa9e3a59"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"a0794b0982c2774c806dfb670647f81ba546e45c","unresolved":true,"context_lines":[{"line_number":195,"context_line":""},{"line_number":196,"context_line":"* Add 2 new Location APIs for add and get operations."},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"* Modify consumers like cinder and nova to use the new location APIs."},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"* Add a releasenote mentioning that we will remove the config option"},{"line_number":201,"context_line":"  ``show_multiple_locations`` when the consumers (nova/cinder) shift to using"}],"source_content_type":"text/x-rst","patch_set":14,"id":"11303e33_5b240f8a","line":198,"in_reply_to":"3e1408a2_4dabc2d3","updated":"2022-06-15 07:12:29.000000000","message":"Currently we are not calling delete from store API from nova or cinder side and for the current nova/cinder-glance optimizations, we don\u0027t need it as well (that we are trying to target with this spec)\nSo maybe this can be done in a followup work if we want to introduce the support for update/delete location calls?","commit_id":"2b92aebb19773cc97f974949c6e2c317aa9e3a59"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8fe08772ea6f4dc1e5cee1ea806943c512adae52","unresolved":true,"context_lines":[{"line_number":30,"context_line":"using the image owner\u0027s credentials, and this is why operators want to set"},{"line_number":31,"context_line":"show_multiple_locations to True.  What operators want to do, however, is to"},{"line_number":32,"context_line":"enable optimized image data access; exposing image locations to non-admin"},{"line_number":33,"context_line":"users is a side-effect, not the goal.  We currently recommend that operators"},{"line_number":34,"context_line":"who want to use optimized data access use a specialized Glance instance for"},{"line_number":35,"context_line":"services, and only expose glance-api to end users with show_multiple_locations"},{"line_number":36,"context_line":"set False.  This is inconvinient for certain users."},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"Proposed change"},{"line_number":39,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":15,"id":"7ad62b94_5087990d","line":36,"range":{"start_line":33,"start_character":39,"end_line":36,"end_character":51},"updated":"2022-06-15 16:31:47.000000000","message":"Yup, thanks for describing this. Operators shouldn\u0027t default to show_multiple_locations to True unless they\u0027re 100% sure about *why* they need to set it to True.\nOh, and FWIW I may have some concern by this config-driven API behaviour, but let\u0027s not open a side discussion here about this even if I feel bad for public clouds wanting to have interop.","commit_id":"1184422804fd51260654aa60b21017a6355863de"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8fe08772ea6f4dc1e5cee1ea806943c512adae52","unresolved":true,"context_lines":[{"line_number":43,"context_line":"1. Introduce 2 new API calls that allow operations on image locations which"},{"line_number":44,"context_line":"   are described in detail in the `REST API impact`_ section."},{"line_number":45,"context_line":"   These calls will replace the image-update mechanism for consumers"},{"line_number":46,"context_line":"   like cinder and nova."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"2. Modify the consumer (cinder/nova) code to use the new location APIs."},{"line_number":49,"context_line":""}],"source_content_type":"text/x-rst","patch_set":15,"id":"bd50f1fd_55abc3f9","line":46,"updated":"2022-06-15 16:31:47.000000000","message":"provided we keep the existing API calls during a certain timeframe for obvious upgrade reasons.","commit_id":"1184422804fd51260654aa60b21017a6355863de"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8fe08772ea6f4dc1e5cee1ea806943c512adae52","unresolved":true,"context_lines":[{"line_number":45,"context_line":"   These calls will replace the image-update mechanism for consumers"},{"line_number":46,"context_line":"   like cinder and nova."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"2. Modify the consumer (cinder/nova) code to use the new location APIs."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"3. Remove ``show_multiple_locations`` config option when it is no longer"},{"line_number":51,"context_line":"   required by other services (cinder/nova) to perform operations on"}],"source_content_type":"text/x-rst","patch_set":15,"id":"511a8a36_4fd8a170","line":48,"updated":"2022-06-15 16:31:47.000000000","message":"I just hope such changes be enough self-contained to not require too much effort (and paperwork in Nova)","commit_id":"1184422804fd51260654aa60b21017a6355863de"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8fe08772ea6f4dc1e5cee1ea806943c512adae52","unresolved":true,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"The config option ``show_multiple_locations`` has been deprecated since Newton"},{"line_number":55,"context_line":"but we will keep the config option until the consumers of glance locations"},{"line_number":56,"context_line":"(nova, cinder etc) start using the new location APIs."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"We will introduce 2 new policies for each API performing different operations"},{"line_number":59,"context_line":"like add and get which will default to the ``service`` role for authorization."}],"source_content_type":"text/x-rst","patch_set":15,"id":"52283f1c_ee4668af","line":56,"updated":"2022-06-15 16:31:47.000000000","message":"Huzzah !","commit_id":"1184422804fd51260654aa60b21017a6355863de"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8fe08772ea6f4dc1e5cee1ea806943c512adae52","unresolved":true,"context_lines":[{"line_number":154,"context_line":"---------------------"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"Since the new APIs are for service to service interaction, there is not much"},{"line_number":157,"context_line":"value to expose them via CLI. We will add methods to the client"},{"line_number":158,"context_line":"(that will call the new location APIs) that will be used by other services"},{"line_number":159,"context_line":"like cinder and nova but those methods won\u0027t be exposed via the shell to end"},{"line_number":160,"context_line":"users. End users can still use the existing commands (that internally calls"},{"line_number":161,"context_line":"the image-update API) to perform operations on locations:"},{"line_number":162,"context_line":""}],"source_content_type":"text/x-rst","patch_set":15,"id":"ff7966f1_70d696d7","line":159,"range":{"start_line":157,"start_character":30,"end_line":159,"end_character":20},"updated":"2022-06-15 16:31:47.000000000","message":"I would appreciate more details about the impact on the client side and how other projects would consume those client changes, but that\u0027s a start at least.","commit_id":"1184422804fd51260654aa60b21017a6355863de"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8fe08772ea6f4dc1e5cee1ea806943c512adae52","unresolved":true,"context_lines":[{"line_number":178,"context_line":"----------------"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"Consumers like Cinder and Nova need to implement code to call the new APIs"},{"line_number":181,"context_line":"for location operations."},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"Implementation"},{"line_number":184,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":15,"id":"02954d0b_a01914f3","line":181,"updated":"2022-06-15 16:31:47.000000000","message":"again, I\u0027m a bit curious but that\u0027s maybe something we should discuss out of this spec (and within the nova community, this hopefully being a specless bp for tracking purposes)","commit_id":"1184422804fd51260654aa60b21017a6355863de"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5a9ea488aafc7c89cec0e8b86ef7b26e1766c239","unresolved":true,"context_lines":[{"line_number":178,"context_line":"----------------"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"Consumers like Cinder and Nova need to implement code to call the new APIs"},{"line_number":181,"context_line":"for location operations."},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"Implementation"},{"line_number":184,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":15,"id":"6ec25d6b_8ed9ce93","line":181,"in_reply_to":"02954d0b_a01914f3","updated":"2022-06-15 16:49:26.000000000","message":"Yeah I think at least a bp for sure, but since it will require docs, config, renos, devstack, I think it\u0027s worth us asking for a full spec. Doesn\u0027t need to be a lot of detail there, but this it not invisible to devs or operators, so I think it\u0027s fine to expect one. We can point to this one for some of the detail to save on verbosity.","commit_id":"1184422804fd51260654aa60b21017a6355863de"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8fe08772ea6f4dc1e5cee1ea806943c512adae52","unresolved":true,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"* Add 2 new Location APIs for add and get operations."},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"* Modify consumers like cinder and nova to use the new location APIs."},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"* Add a releasenote mentioning that we will remove the config option"},{"line_number":203,"context_line":"  ``show_multiple_locations`` when the consumers (nova/cinder) shift to using"}],"source_content_type":"text/x-rst","patch_set":15,"id":"0ccee94e_3c628dae","line":200,"updated":"2022-06-15 16:31:47.000000000","message":"keeping in mind that for compatibility reasons, we should propose a backwards-compatible call for one cycle in Nova until operators provide the new credentials (disclaimer: nova brainstorm to happen for upgrade path)","commit_id":"1184422804fd51260654aa60b21017a6355863de"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5a9ea488aafc7c89cec0e8b86ef7b26e1766c239","unresolved":true,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"* Add 2 new Location APIs for add and get operations."},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"* Modify consumers like cinder and nova to use the new location APIs."},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"* Add a releasenote mentioning that we will remove the config option"},{"line_number":203,"context_line":"  ``show_multiple_locations`` when the consumers (nova/cinder) shift to using"}],"source_content_type":"text/x-rst","patch_set":15,"id":"f7be4c74_f6fe0f1c","line":200,"in_reply_to":"0ccee94e_3c628dae","updated":"2022-06-15 16:49:26.000000000","message":"Yes, I think the original mechanism has to stay in place for many releases, even if it\u0027s deprecated and/or warns people when it\u0027s turned on. Likewise, I think nova needs to implement this such that both mechanisms are supported, depending on whether or not glance service credentials are configured. As noted in the IRC conversation, people could absolutely be running up-to-date nova with a much older glance. If we\u0027re going to force an upgrade of the latter, there has to be some serious warning.","commit_id":"1184422804fd51260654aa60b21017a6355863de"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8fe08772ea6f4dc1e5cee1ea806943c512adae52","unresolved":true,"context_lines":[{"line_number":214,"context_line":"* Unit Tests"},{"line_number":215,"context_line":"* Functional Tests"},{"line_number":216,"context_line":"* Integration Tests"},{"line_number":217,"context_line":"* Tempest Tests"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"Documentation Impact"},{"line_number":220,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":15,"id":"25a143f8_df236888","line":217,"range":{"start_line":217,"start_character":0,"end_line":217,"end_character":15},"updated":"2022-06-15 16:31:47.000000000","message":"please.","commit_id":"1184422804fd51260654aa60b21017a6355863de"}]}