)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"cf8fc3bbd470050fcd05359f3151013981ce35dc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c4cab589_a9041408","updated":"2025-03-08 17:41:16.000000000","message":"See inline for what I think is problematic.\n\nAs far as what to do right now ... we could go with this patch (since it will work in the default setup) and have a Known Issue in Epoxy that we could try to fix in oslo.policy during Flamingo.\n\nOr, we could do something like cinder did as part of the fix for CVE-2023-2088, where instead of using policies to detect the presence of a service token with an appropriate role, we directly use the service_token_roles config opt in the [keystone_authtoken] part of glance.conf.  (The cinder code is [0]).  While you might argue that this is less flexible than using pure policy config, what we could do is still have a policy rule for this call, but always allow a service to execute it (which is sort of what cinder does for volume detach, though the workflow is a bit more complicated [1]).\n\nI\u0027d probably go with the second option, because it might still be possible to get it done in Epoxy, and also because the service roles are very important given that they are usually used to escalate privileges ... so I think it would be better to keep their configuration in one place (the config file) instead of both the config file and the policy file--too much room for error.  (Though, admittedly, you\u0027re not dealing with a CVE here.  Not yet, anyway!)\n\n\n[0] https://github.com/openstack/cinder/blob/28789f0dd01198c89e064aaed65fa2df31472685/cinder/volume/api.py#L900-L910\n[1] https://github.com/openstack/cinder/blob/28789f0dd01198c89e064aaed65fa2df31472685/cinder/volume/api.py#L859-L865","commit_id":"21c478b9c84193557ba157e93a6a9e70784f9094"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e106bf75ec5df601e80ddf70e3f36921aaf66610","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0d5dc97f_7ea0f680","updated":"2025-03-10 12:22:06.000000000","message":"Thanks for checking this.  Looks like everything is good after all.","commit_id":"21c478b9c84193557ba157e93a6a9e70784f9094"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"29b229659dba36d2920d751fee3cedfb7dd0ad30","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b92d2529_6a8354d3","updated":"2025-02-19 18:10:08.000000000","message":"recheck functional job failed with py312 on a scrubber test which i don\u0027t have much clue about but looks unrelated\n\nTraceback (most recent call last):\n  File \"/home/zuul/src/opendev.org/openstack/glance/glance/tests/functional/serial/test_scrubber.py\", line 289, in test_scrubber_restore_image\n    self.assertEqual(0, exitcode)\n  File \"/home/zuul/src/opendev.org/openstack/glance/.tox/functional-py312/lib/python3.12/site-packages/testtools/testcase.py\", line 419, in assertEqual\n    self.assertThat(observed, matcher, message)\n  File \"/home/zuul/src/opendev.org/openstack/glance/.tox/functional-py312/lib/python3.12/site-packages/testtools/testcase.py\", line 509, in assertThat\n    raise mismatch_error\ntesttools.matchers._impl.MismatchError: 0 !\u003d 1","commit_id":"21c478b9c84193557ba157e93a6a9e70784f9094"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5eee580852df45b8090ded14b8d82546da1a426a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0b8c084f_52941362","updated":"2025-02-19 14:04:57.000000000","message":"recheck the cinder import job failed\nformat test failed with issue while deleting the image, looks unrelated to this patch\n\nraceback (most recent call last):\n  File \"/opt/stack/tempest/tempest/api/image/v2/test_images_formats.py\", line 145, in test_accept_reject_formats_import\n    self.client.delete_image(image[\u0027id\u0027])\n  File \"/opt/stack/tempest/tempest/lib/services/image/v2/images_client.py\", line 90, in delete_image\n    resp, _ \u003d self.delete(url)\n              ^^^^^^^^^^^^^^^^\n  File \"/opt/stack/tempest/tempest/lib/common/rest_client.py\", line 359, in delete\n    return self.request(\u0027DELETE\u0027, url, extra_headers, headers, body)\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/opt/stack/tempest/tempest/lib/common/rest_client.py\", line 762, in request\n    self._error_checker(resp, resp_body)\n  File \"/opt/stack/tempest/tempest/lib/common/rest_client.py\", line 947, in _error_checker\n    raise exceptions.ServerFault(resp_body, resp\u003dresp,\ntempest.lib.exceptions.ServerFault: Got server fault\nDetails: The server has either erred or is incapable of performing the requested operation.\u003cbr /\u003e\u003cbr /\u003e","commit_id":"21c478b9c84193557ba157e93a6a9e70784f9094"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"17587efff9ef75cea527aaec56f8987e676636a5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e7549ab8_cc5e4aa8","updated":"2025-02-20 05:53:31.000000000","message":"recheck unrelated failure - test_accept_reject_formats_import","commit_id":"21c478b9c84193557ba157e93a6a9e70784f9094"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ebb1e525d59c50b052f5b90fe116ebdce7d9b899","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ef1c04db_93a1a07b","in_reply_to":"c4cab589_a9041408","updated":"2025-03-10 09:05:56.000000000","message":"Hi Brian,\nThanks for the suggestions. As checked inline, service_roles with a list works as expected. I\u0027m not really sure about the reason for cinder to do it in-code but in this case, if the concern is regarding the service_roles field containing a list as a \"value\", it should already be handled by oslo.policy in the generic check.","commit_id":"21c478b9c84193557ba157e93a6a9e70784f9094"}],"glance/policies/base.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"cf8fc3bbd470050fcd05359f3151013981ce35dc","unresolved":true,"context_lines":[{"line_number":89,"context_line":")"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"SERVICE \u003d \u0027rule:service_api\u0027"},{"line_number":92,"context_line":"SERVICE_ROLE \u003d \u0027service_roles:service\u0027"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"ADMIN_OR_SERVICE_ROLE \u003d f\u0027{ADMIN} or {SERVICE_ROLE}\u0027"},{"line_number":95,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7546126b_0bc5acad","line":92,"range":{"start_line":92,"start_character":15,"end_line":92,"end_character":38},"updated":"2025-03-08 17:41:16.000000000","message":"(NOTE: you\u0027ll have to check me on this, but I did a bunch of digging around in oslo.policy and I\u0027m pretty sure what I\u0027m about to say is correct.)\n\nThis is going to be fragile.  This is a \"generic check\" [0].  The reason it is working is because under the default setup, the service_roles attribute populated in the context will only have the value \u0027service\u0027, but it could have others (just like \u0027roles\u0027 usually gives you a list).  If service_roles is a list of values (like we usually have for \u0027roles\u0027, for example, [\u0027service\u0027, \u0027automated\u0027]), I don\u0027t think this will match.\n\nThe reason why it works with \"role:whatever\" even when a context has roles with value [\u0027one\u0027, \u0027two\u0027, \u0027whatever\u0027], is that \u0027role\u0027 is a \"Special Check\" in the policy language [1], and it has special code to handle matching one out of a list of values [2].\n\n\n[0] https://github.com/openstack/oslo.policy/blob/3bb5d923dabcb6554c3e6aace8edd78085bb6037/oslo_policy/policy.py#L103\n[1] https://github.com/openstack/oslo.policy/blob/3bb5d923dabcb6554c3e6aace8edd78085bb6037/oslo_policy/policy.py#L149\n[2] https://github.com/openstack/oslo.policy/blob/3bb5d923dabcb6554c3e6aace8edd78085bb6037/oslo_policy/_checks.py#L268-L281","commit_id":"21c478b9c84193557ba157e93a6a9e70784f9094"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ebb1e525d59c50b052f5b90fe116ebdce7d9b899","unresolved":true,"context_lines":[{"line_number":89,"context_line":")"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"SERVICE \u003d \u0027rule:service_api\u0027"},{"line_number":92,"context_line":"SERVICE_ROLE \u003d \u0027service_roles:service\u0027"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"ADMIN_OR_SERVICE_ROLE \u003d f\u0027{ADMIN} or {SERVICE_ROLE}\u0027"},{"line_number":95,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9831728d_967c151a","line":92,"range":{"start_line":92,"start_character":15,"end_line":92,"end_character":38},"in_reply_to":"7546126b_0bc5acad","updated":"2025-03-10 09:05:56.000000000","message":"Hi Brian,\nThanks for the initial analysis. As i can see in the generic check code, it should work with a list[1].\n\"test_value\" is analogous to \"creds\" in the RoleCheck[2]\n\nUPDATE:\nI went ahead and tested the workflow and as assumed, it works!\nNOTE: I\u0027m using the demo user since the ORCheck passes if the first check of \"admin\" returns True.\n\nHere the service_roles already had a list\n(Pdb) self._context.service_roles\n[\u0027service\u0027, \u0027creator\u0027]\n\nThese are the 3 parameter values of GenericCheck (_find_in_dict[3])\n\n\u003e /opt/stack/data/venv/lib/python3.10/site-packages/oslo_policy/_checks.py(296)_find_in_dict()\n\n(Pdb) path_segments\n[\u0027service_roles\u0027]\n\n(Pdb) match\n\u0027service\u0027\n\n(Pdb) test_value\n{\u0027user_id\u0027: \u00272b32c86a9bfd40e6898f6b9d50c72bff\u0027, \u0027user_domain_id\u0027: \u0027default\u0027, \u0027system_scope\u0027: None, \u0027domain_id\u0027: None, \u0027project_id\u0027: \u00275cd72b01e6e840048fa4b9760070ac90\u0027, \u0027project_domain_id\u0027: \u0027default\u0027, \u0027roles\u0027: [\u0027reader\u0027, \u0027anotherrole\u0027, \u0027member\u0027], \u0027is_admin_project\u0027: True, \u0027service_user_id\u0027: \u0027c0f5d13c65b242469729c011b50f3a58\u0027, \u0027service_user_domain_id\u0027: \u0027default\u0027, \u0027service_project_id\u0027: \u00271bf98b3a889449ffada2502eaac4c080\u0027, \u0027service_project_domain_id\u0027: \u0027default\u0027, \u0027service_roles\u0027: [\u0027service\u0027, \u0027creator\u0027]}\n\n\nHere is the PDB from rest of the workflow, it returns True eventually and the operation succeeds\n\n-\u003e if isinstance(test_value, list):\n(Pdb) test_value\n[\u0027service\u0027, \u0027creator\u0027]\n\n\u003e /opt/stack/data/venv/lib/python3.10/site-packages/oslo_policy/_checks.py(312)_find_in_dict()\n-\u003e if len(path_segments) \u003d\u003d 0:\n\n(Pdb) \n\u003e /opt/stack/data/venv/lib/python3.10/site-packages/oslo_policy/_checks.py(313)_find_in_dict()\n-\u003e return match \u003d\u003d str(test_value)\n\n(Pdb) match\n\u0027service\u0027\n\n(Pdb) test_value\n\u0027service\u0027\n\n\u003e /opt/stack/data/venv/lib/python3.10/site-packages/oslo_policy/_checks.py(322)_find_in_dict()\n-\u003e return True\n\n[1] https://github.com/openstack/oslo.policy/blob/3bb5d923dabcb6554c3e6aace8edd78085bb6037/oslo_policy/_checks.py#L319-L322\n[2] https://github.com/openstack/oslo.policy/blob/3bb5d923dabcb6554c3e6aace8edd78085bb6037/oslo_policy/_checks.py#L279\n[3] https://github.com/openstack/oslo.policy/blob/3bb5d923dabcb6554c3e6aace8edd78085bb6037/oslo_policy/_checks.py#L296","commit_id":"21c478b9c84193557ba157e93a6a9e70784f9094"}]}