)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":38762,"name":"Sahil Kumbhar","display_name":"sakumbha","email":"sakumbha@redhat.com","username":"sakumbha"},"change_message_id":"2b42ca25203c529d393bf3f15fcc7843db14d0b4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"48ce3526_ab2b97e1","updated":"2026-06-02 17:04:16.000000000","message":"recheck","commit_id":"2f5e731aeb155384c254bdf11f22144b273b7d0b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"8727a1a9277b72b20fe68a67808a34e457422980","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"c99e8d33_9fbf7840","updated":"2026-06-19 15:12:53.000000000","message":"+1 until we have glance_store merge and release","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":38762,"name":"Sahil Kumbhar","display_name":"sakumbha","email":"sakumbha@redhat.com","username":"sakumbha"},"change_message_id":"59e2b21c88ea84d7e0a70c165ff60eda1856fd29","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"d58b37da_752720ac","updated":"2026-06-07 18:25:00.000000000","message":"Manual Testing Reference https://etherpad.opendev.org/p/s3-credential-free-manual-testing","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"60aa4dee2b70f32d0cc64023428e8f039531590b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"99bc8a16_4c323e37","updated":"2026-06-12 18:28:16.000000000","message":"Some concerns about the implementation.","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"a65929740b9be8eb8d2c7c7d46ed0ae201eda39c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"874d56e5_bfbd7028","updated":"2026-07-01 13:15:47.000000000","message":"Good to merge now, glance_store patch merged and released [1].\n\n[1]: https://docs.openstack.org/releasenotes/glance_store/unreleased.html#relnotes-5-6-0","commit_id":"e59614a2d8aa68c4462521f3d1cdf84557743216"},{"author":{"_account_id":38762,"name":"Sahil Kumbhar","display_name":"sakumbha","email":"sakumbha@redhat.com","username":"sakumbha"},"change_message_id":"c68410efb74989e1abd09cc2c1c11c1a88af7dbe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"23f50728_6cf9b2c8","updated":"2026-06-30 02:14:51.000000000","message":"recheck","commit_id":"e59614a2d8aa68c4462521f3d1cdf84557743216"}],"glance/common/store_utils.py":[{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"60aa4dee2b70f32d0cc64023428e8f039531590b","unresolved":true,"context_lines":[{"line_number":359,"context_line":"        # Multistore, find by store name"},{"line_number":360,"context_line":"        store_instance \u003d location_map[scheme][store_name][\u0027store\u0027]"},{"line_number":361,"context_line":"    else:"},{"line_number":362,"context_line":"        # Old single store instance. Find by bucket and update store name"},{"line_number":363,"context_line":"        store_result \u003d _find_store_by_bucket(parsed, location_map, scheme)"},{"line_number":364,"context_line":"        if store_result:"},{"line_number":365,"context_line":"            store_name, store_instance \u003d store_result"}],"source_content_type":"text/x-python","patch_set":6,"id":"b22debac_7e310531","side":"PARENT","line":362,"range":{"start_line":362,"start_character":0,"end_line":362,"end_character":8},"updated":"2026-06-12 18:28:16.000000000","message":"Do we still handle single store with this new version of the code?","commit_id":"eac2fa47f26da3515c7a1e8c91226750517c52d4"},{"author":{"_account_id":38762,"name":"Sahil Kumbhar","display_name":"sakumbha","email":"sakumbha@redhat.com","username":"sakumbha"},"change_message_id":"1bdd0af80344aa47d218e09b098244cda762f63b","unresolved":true,"context_lines":[{"line_number":359,"context_line":"        # Multistore, find by store name"},{"line_number":360,"context_line":"        store_instance \u003d location_map[scheme][store_name][\u0027store\u0027]"},{"line_number":361,"context_line":"    else:"},{"line_number":362,"context_line":"        # Old single store instance. Find by bucket and update store name"},{"line_number":363,"context_line":"        store_result \u003d _find_store_by_bucket(parsed, location_map, scheme)"},{"line_number":364,"context_line":"        if store_result:"},{"line_number":365,"context_line":"            store_name, store_instance \u003d store_result"}],"source_content_type":"text/x-python","patch_set":6,"id":"34989a21_bcc9661b","side":"PARENT","line":362,"range":{"start_line":362,"start_character":0,"end_line":362,"end_character":8},"in_reply_to":"b22debac_7e310531","updated":"2026-06-15 11:32:24.000000000","message":"Yes single store is handled. in `get()` method in location.py in else condition.","commit_id":"eac2fa47f26da3515c7a1e8c91226750517c52d4"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"60aa4dee2b70f32d0cc64023428e8f039531590b","unresolved":true,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"def update_store_in_locations(context, image, image_repo):"},{"line_number":181,"context_line":"    store_updated \u003d False"},{"line_number":182,"context_line":"    for loc in image.locations:"},{"line_number":183,"context_line":"        if loc[\u0027url\u0027].startswith((\u0027s3://\u0027, \u0027s3+http://\u0027, \u0027s3+https://\u0027)):"},{"line_number":184,"context_line":"            if _update_s3_location_credentials(loc):"},{"line_number":185,"context_line":"                store_updated \u003d True"}],"source_content_type":"text/x-python","patch_set":6,"id":"92279333_0a8cc813","line":182,"range":{"start_line":182,"start_character":15,"end_line":182,"end_character":30},"updated":"2026-06-12 18:28:16.000000000","message":"So we used to update the S3 location (when doing credential rotation) at the end of this function. Now we do the new update (removing credentials) at the beginning. Does it matter where we put this code block?","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":38762,"name":"Sahil Kumbhar","display_name":"sakumbha","email":"sakumbha@redhat.com","username":"sakumbha"},"change_message_id":"1bdd0af80344aa47d218e09b098244cda762f63b","unresolved":true,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"def update_store_in_locations(context, image, image_repo):"},{"line_number":181,"context_line":"    store_updated \u003d False"},{"line_number":182,"context_line":"    for loc in image.locations:"},{"line_number":183,"context_line":"        if loc[\u0027url\u0027].startswith((\u0027s3://\u0027, \u0027s3+http://\u0027, \u0027s3+https://\u0027)):"},{"line_number":184,"context_line":"            if _update_s3_location_credentials(loc):"},{"line_number":185,"context_line":"                store_updated \u003d True"}],"source_content_type":"text/x-python","patch_set":6,"id":"116929b7_dedec368","line":182,"range":{"start_line":182,"start_character":15,"end_line":182,"end_character":30},"in_reply_to":"92279333_0a8cc813","updated":"2026-06-15 11:32:24.000000000","message":"Yes, Position at beginning is needed. `update_s3_location_credentials` must run before `_get_store_id_from_uri` because this func uses uri.startswith(url_prefix) to match db url. \n\nfor e.g db url\u003d `s3://key:secret@host/bucket/key`\nand cred free url \u003d`s3://host/bucket/key` would fail to match.\n\nSo removing cred first works correctly.\n\nAnd in old credential rotation there were a credential in the URL so it was a match.","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"172713f76ffa168f8f6baa7b527d6e046a063e55","unresolved":true,"context_lines":[{"line_number":252,"context_line":"    scheme \u003d urlparse.urlparse(uri).scheme"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    uri_without_query \u003d uri.split(\u0027?\u0027)[0].split(\u0027#\u0027)[0]"},{"line_number":255,"context_line":"    at_pos \u003d uri_without_query.rfind(\u0027@\u0027)"},{"line_number":256,"context_line":"    if at_pos \u003d\u003d -1:"},{"line_number":257,"context_line":"        return False"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bd8b9df8_6fc91c49","line":255,"range":{"start_line":255,"start_character":4,"end_line":255,"end_character":41},"updated":"2026-06-09 12:08:54.000000000","message":"what if @ appears only in the S3 object key (not in credentials), could migration corrupt the URL?","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":38762,"name":"Sahil Kumbhar","display_name":"sakumbha","email":"sakumbha@redhat.com","username":"sakumbha"},"change_message_id":"f7221481540f5d04545e8bc1e8eb68033a55d850","unresolved":true,"context_lines":[{"line_number":252,"context_line":"    scheme \u003d urlparse.urlparse(uri).scheme"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    uri_without_query \u003d uri.split(\u0027?\u0027)[0].split(\u0027#\u0027)[0]"},{"line_number":255,"context_line":"    at_pos \u003d uri_without_query.rfind(\u0027@\u0027)"},{"line_number":256,"context_line":"    if at_pos \u003d\u003d -1:"},{"line_number":257,"context_line":"        return False"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1b548323_cd2cbb67","line":255,"range":{"start_line":255,"start_character":4,"end_line":255,"end_character":41},"in_reply_to":"789024ac_cf33828c","updated":"2026-06-10 06:25:32.000000000","message":"Are you referring image_id as a object key? If so \u0027@\u0027 will not appear in it becoz image_id is build from (0-9), (a-f) \u0026 hypen(-).","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"22e9ddcf5be2e0474d13d7a6a9606077ae9cfc36","unresolved":true,"context_lines":[{"line_number":252,"context_line":"    scheme \u003d urlparse.urlparse(uri).scheme"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    uri_without_query \u003d uri.split(\u0027?\u0027)[0].split(\u0027#\u0027)[0]"},{"line_number":255,"context_line":"    at_pos \u003d uri_without_query.rfind(\u0027@\u0027)"},{"line_number":256,"context_line":"    if at_pos \u003d\u003d -1:"},{"line_number":257,"context_line":"        return False"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"e2b5d41a_509a33d2","line":255,"range":{"start_line":255,"start_character":4,"end_line":255,"end_character":41},"in_reply_to":"789024ac_cf33828c","updated":"2026-06-11 05:04:12.000000000","message":"yes, I mean the S3 object name — the last part of the location path after the bucket. Just wanted to confirm if there is possibility of someone manually registers a location with a non-UUID key containing @ ?\n\nI just felt that rfind(\u0027@\u0027) not good Python practice here. The right approach could be stripping — with split(\u0027@\u0027, 1) + credential validation","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"354a1c910b6ee9cc169ee108e6aaa76700eac570","unresolved":true,"context_lines":[{"line_number":252,"context_line":"    scheme \u003d urlparse.urlparse(uri).scheme"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    uri_without_query \u003d uri.split(\u0027?\u0027)[0].split(\u0027#\u0027)[0]"},{"line_number":255,"context_line":"    at_pos \u003d uri_without_query.rfind(\u0027@\u0027)"},{"line_number":256,"context_line":"    if at_pos \u003d\u003d -1:"},{"line_number":257,"context_line":"        return False"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"789024ac_cf33828c","line":255,"range":{"start_line":255,"start_character":4,"end_line":255,"end_character":41},"in_reply_to":"bd8b9df8_6fc91c49","updated":"2026-06-09 12:12:03.000000000","message":"what do you mean by object key?","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":38762,"name":"Sahil Kumbhar","display_name":"sakumbha","email":"sakumbha@redhat.com","username":"sakumbha"},"change_message_id":"a5995fb147f7e33ca2c58c6e76a761109df2d229","unresolved":true,"context_lines":[{"line_number":252,"context_line":"    scheme \u003d urlparse.urlparse(uri).scheme"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    uri_without_query \u003d uri.split(\u0027?\u0027)[0].split(\u0027#\u0027)[0]"},{"line_number":255,"context_line":"    at_pos \u003d uri_without_query.rfind(\u0027@\u0027)"},{"line_number":256,"context_line":"    if at_pos \u003d\u003d -1:"},{"line_number":257,"context_line":"        return False"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"12e1afbc_e05b587c","line":255,"range":{"start_line":255,"start_character":4,"end_line":255,"end_character":41},"in_reply_to":"e2b5d41a_509a33d2","updated":"2026-06-11 10:25:49.000000000","message":"I don\u0027t think it is possible to register a location using a non-UUID key without directly manipulating the database.\n\nAnd if we use split(\u0027@\u0027, 1), it will fail when the secret key itself contains \u0027@\u0027 as part of the secret key would be incorrectly treated as the host. Therefore, I think rfind(\u0027@\u0027) is the better approach here\ne.g url:\ns3://access:sec@ret/host/bucket/image_id","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"60aa4dee2b70f32d0cc64023428e8f039531590b","unresolved":true,"context_lines":[{"line_number":258,"context_line":""},{"line_number":259,"context_line":"    host_and_path \u003d uri_without_query[at_pos + 1:]"},{"line_number":260,"context_line":"    loc[\u0027url\u0027] \u003d \"%s://%s\" % (scheme, host_and_path)"},{"line_number":261,"context_line":"    return True"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"def get_updated_store_location(locations, context):"}],"source_content_type":"text/x-python","patch_set":6,"id":"bf9b2df8_b2c2117d","line":261,"range":{"start_line":261,"start_character":11,"end_line":261,"end_character":15},"updated":"2026-06-12 18:28:16.000000000","message":"So here we are removing query parameters (such as ?foo\u003dbar) and fragments (such as #baz). The previous version of the URL rotation had a test explicitely preventing that (test_update_s3_url_preserves_query_and_fragment).\n\nAre you doing this on purpose?","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"e8176747acecda41a52fb37418b9bf5e023b1107","unresolved":true,"context_lines":[{"line_number":258,"context_line":""},{"line_number":259,"context_line":"    host_and_path \u003d uri_without_query[at_pos + 1:]"},{"line_number":260,"context_line":"    loc[\u0027url\u0027] \u003d \"%s://%s\" % (scheme, host_and_path)"},{"line_number":261,"context_line":"    return True"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"def get_updated_store_location(locations, context):"}],"source_content_type":"text/x-python","patch_set":6,"id":"a9e2a3a7_58b6d215","line":261,"range":{"start_line":261,"start_character":11,"end_line":261,"end_character":15},"in_reply_to":"3f21e404_5290bf6d","updated":"2026-06-16 14:48:00.000000000","message":"OK I\u0027m a tiny bit worried about this.\n\n@akekane@redhat.com Are we fine dropping queries/fragments?","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"8c4bab90e21fb9c60681253f541aed7a09ff25a0","unresolved":true,"context_lines":[{"line_number":258,"context_line":""},{"line_number":259,"context_line":"    host_and_path \u003d uri_without_query[at_pos + 1:]"},{"line_number":260,"context_line":"    loc[\u0027url\u0027] \u003d \"%s://%s\" % (scheme, host_and_path)"},{"line_number":261,"context_line":"    return True"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"def get_updated_store_location(locations, context):"}],"source_content_type":"text/x-python","patch_set":6,"id":"dd0523cb_0d7bfdb2","line":261,"range":{"start_line":261,"start_character":11,"end_line":261,"end_character":15},"in_reply_to":"a9e2a3a7_58b6d215","updated":"2026-06-16 15:15:59.000000000","message":"I think we need to understand how query parameters were generated and preserve them for new credential url as well. If those are not generated earlier then we can assume that it was dead code for compatibility with other drivers which we can remove now.","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":38762,"name":"Sahil Kumbhar","display_name":"sakumbha","email":"sakumbha@redhat.com","username":"sakumbha"},"change_message_id":"1bdd0af80344aa47d218e09b098244cda762f63b","unresolved":true,"context_lines":[{"line_number":258,"context_line":""},{"line_number":259,"context_line":"    host_and_path \u003d uri_without_query[at_pos + 1:]"},{"line_number":260,"context_line":"    loc[\u0027url\u0027] \u003d \"%s://%s\" % (scheme, host_and_path)"},{"line_number":261,"context_line":"    return True"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"def get_updated_store_location(locations, context):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f21e404_5290bf6d","line":261,"range":{"start_line":261,"start_character":11,"end_line":261,"end_character":15},"in_reply_to":"bf9b2df8_b2c2117d","updated":"2026-06-15 11:32:24.000000000","message":"`_update_s3_url_` was called repeatedly, so preserving all components was necessary. But now our `_update_s3_location_credentials` one time migration to strip credentials\n\nAnd also `s3 drivers get_uri()` only produces `scheme://host/bucket/key - no query parameters or fragments are generated\n\nSo if i\u0027m not missing anything then preserving it is not required here.","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":38762,"name":"Sahil Kumbhar","display_name":"sakumbha","email":"sakumbha@redhat.com","username":"sakumbha"},"change_message_id":"bd0acee7cd8430d171eb4e6ddeb3c9236730ac66","unresolved":true,"context_lines":[{"line_number":258,"context_line":""},{"line_number":259,"context_line":"    host_and_path \u003d uri_without_query[at_pos + 1:]"},{"line_number":260,"context_line":"    loc[\u0027url\u0027] \u003d \"%s://%s\" % (scheme, host_and_path)"},{"line_number":261,"context_line":"    return True"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"def get_updated_store_location(locations, context):"}],"source_content_type":"text/x-python","patch_set":6,"id":"f896689d_1efe13ce","line":261,"range":{"start_line":261,"start_character":11,"end_line":261,"end_character":15},"in_reply_to":"dd0523cb_0d7bfdb2","updated":"2026-06-19 05:32:19.000000000","message":"I checked all code for create s3 URLs in glance_store. I didn\u0027t find anything which add `?` or `#` query parameters or fragments to s3 URIs. \n\nStill i used `split(\u0027?\u0027)[0].split(\u0027#\u0027)[0]` logic as defensive code and it was there before.","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"8727a1a9277b72b20fe68a67808a34e457422980","unresolved":false,"context_lines":[{"line_number":258,"context_line":""},{"line_number":259,"context_line":"    host_and_path \u003d uri_without_query[at_pos + 1:]"},{"line_number":260,"context_line":"    loc[\u0027url\u0027] \u003d \"%s://%s\" % (scheme, host_and_path)"},{"line_number":261,"context_line":"    return True"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"def get_updated_store_location(locations, context):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9daf49de_3d093700","line":261,"range":{"start_line":261,"start_character":11,"end_line":261,"end_character":15},"in_reply_to":"f896689d_1efe13ce","updated":"2026-06-19 15:12:53.000000000","message":"Acknowledged","commit_id":"86592635590b312a38ea3d868bd32ab62f63f779"}]}
