)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"d80bdd31d97bdb1a4ec344a8e6fb693a01982148","unresolved":false,"context_lines":[{"line_number":10,"context_line":"for all four device operations: get_one, get_all, disable, and"},{"line_number":11,"context_line":"enable."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Read operations use project_manager_or_admin, granting the manager"},{"line_number":14,"context_line":"persona visibility into hardware topology for capacity planning and"},{"line_number":15,"context_line":"troubleshooting. Device disable and enable remain restricted to"},{"line_number":16,"context_line":"cloud admins as they affect shared physical infrastructure."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Each new policy includes a deprecated_rule bridge matching the"},{"line_number":19,"context_line":"current legacy check string (rule:admin_api) so that existing"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"489d4f5c_69e337c5","line":16,"range":{"start_line":13,"start_character":0,"end_line":16,"end_character":59},"updated":"2026-06-26 08:26:08.000000000","message":"yes correct.\n\nDevice infos (GPUs, FPGAs, their state) are infrastructure detail. Only managers doing capacity planning and admins need that visibility. Regular readers/members don\u0027t.","commit_id":"a64a3a88ca6adf2abc3112b8e22fe7e39c5a69ae"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"d80bdd31d97bdb1a4ec344a8e6fb693a01982148","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"abe08e62_ac09253a","updated":"2026-06-26 08:26:08.000000000","message":"looks good.","commit_id":"a64a3a88ca6adf2abc3112b8e22fe7e39c5a69ae"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e9ac7d3ac2a9524ac18c12da08ccd37c7e752a6b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"574139ec_cd594b3d","updated":"2026-06-22 23:45:42.000000000","message":"teim-ci: auto","commit_id":"a64a3a88ca6adf2abc3112b8e22fe7e39c5a69ae"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4a3a22811188ec92f390154003ac2315816f188a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"bc7f7191_278169c6","updated":"2026-06-22 08:35:15.000000000","message":"teim-ci: auto","commit_id":"a64a3a88ca6adf2abc3112b8e22fe7e39c5a69ae"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"e9cb67dcbf6e59f7f6357dfc21e56d7489204efc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"755e262e_fdcec7c3","in_reply_to":"abe08e62_ac09253a","updated":"2026-06-29 10:07:51.000000000","message":"Tempest results from cyborg-tempest-srbac https://review.opendev.org/c/openstack/cyborg/+/992332 \nhttps://5126e99bb555fcd10074-f8d3aa66ce3962c6e3c5c35044bdfdd4.ssl.cf1.rackcdn.com/openstack/21ccea317c924406b12ce9ab3cc22cc5/job-output.txt\n```\n  SRBAC tests for device policies:\n  - test_admin_can_read_devices — ok (admin via implied manager)\n  - test_reader_cannot_list_devices — ok (reader lacks manager)\n  - test_member_cannot_list_devices — ok (member lacks manager)\n\n```","commit_id":"a64a3a88ca6adf2abc3112b8e22fe7e39c5a69ae"}],"cyborg/common/policy.py":[{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"d80bdd31d97bdb1a4ec344a8e6fb693a01982148","unresolved":false,"context_lines":[{"line_number":26,"context_line":"#       All of these may be overridden by configuration, but we can"},{"line_number":27,"context_line":"#       depend on their existence throughout the code."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"device_policies \u003d ["},{"line_number":30,"context_line":"    policy.RuleDefault("},{"line_number":31,"context_line":"        \u0027cyborg:device:get_one\u0027,"},{"line_number":32,"context_line":"        \u0027rule:admin_api\u0027,"},{"line_number":33,"context_line":"        description\u003d\u0027Show device detail\u0027,"},{"line_number":34,"context_line":"        scope_types\u003d[\u0027project\u0027],"},{"line_number":35,"context_line":"    ),"},{"line_number":36,"context_line":"    policy.RuleDefault("},{"line_number":37,"context_line":"        \u0027cyborg:device:get_all\u0027,"},{"line_number":38,"context_line":"        \u0027rule:admin_api\u0027,"},{"line_number":39,"context_line":"        description\u003d\u0027Retrieve all device records\u0027,"},{"line_number":40,"context_line":"        scope_types\u003d[\u0027project\u0027],"},{"line_number":41,"context_line":"    ),"},{"line_number":42,"context_line":"    policy.RuleDefault("},{"line_number":43,"context_line":"        \u0027cyborg:device:disable\u0027,"},{"line_number":44,"context_line":"        \u0027rule:admin_api\u0027,"},{"line_number":45,"context_line":"        description\u003d\u0027Disable a device\u0027,"},{"line_number":46,"context_line":"        scope_types\u003d[\u0027project\u0027],"},{"line_number":47,"context_line":"    ),"},{"line_number":48,"context_line":"    policy.RuleDefault("},{"line_number":49,"context_line":"        \u0027cyborg:device:enable\u0027,"},{"line_number":50,"context_line":"        \u0027rule:admin_api\u0027,"},{"line_number":51,"context_line":"        description\u003d\u0027Enable a device\u0027,"},{"line_number":52,"context_line":"        scope_types\u003d[\u0027project\u0027],"},{"line_number":53,"context_line":"    ),"},{"line_number":54,"context_line":"]"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"deployable_policies \u003d ["},{"line_number":57,"context_line":"    policy.RuleDefault("},{"line_number":58,"context_line":"        \u0027cyborg:deployable:get_one\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1c6dfded_b64c8d17","side":"PARENT","line":55,"range":{"start_line":29,"start_character":0,"end_line":55,"end_character":1},"updated":"2026-06-26 08:26:08.000000000","message":"All 4 RuleDefault are moved to DeprecatedDefault in cyborg/policies/devices.py","commit_id":"024b06d6e6047beffe2536c7f2b6c1bebeb6d979"}],"cyborg/policies/devices.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"b64cfdaf565e299b18525ca26cdb4be634b604d1","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    ),"},{"line_number":37,"context_line":"    deprecated_since\u003d\u00272026.2\u0027,"},{"line_number":38,"context_line":")"},{"line_number":39,"context_line":"deprecated_disable \u003d policy.DeprecatedRule("},{"line_number":40,"context_line":"    name\u003d\u0027cyborg:device:disable\u0027,"},{"line_number":41,"context_line":"    check_str\u003d\u0027rule:admin_api\u0027,"},{"line_number":42,"context_line":"    deprecated_reason\u003d("}],"source_content_type":"text/x-python","patch_set":4,"id":"895984e0_e1e83ad3","line":39,"updated":"2026-06-23 00:27:35.000000000","message":"DeprecatedRule bridges for disable/enable use the same check_str (rule:admin_api) as the new DocumentedRuleDefault default, making each bridge a functional no-op that ORs a check with itself.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Clarifying or simplifying the redundant bridges reduces policy complexity and avoids implying a behavioral change during migration where none occurs. The deprecated_reason text itself acknowledges the default is retained.\n\n**Recommendation**:\nEither omit deprecated_rule for disable/enable since old and new checks are identical, or add a code comment explaining these bridges exist solely for API consistency with the read-operation pattern and oslo.policy deprecation tracking. The current approach is not wrong, just redundant.","commit_id":"a64a3a88ca6adf2abc3112b8e22fe7e39c5a69ae"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6fedbc76e011c65f0b5b38b56b275c1822a48a73","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    ),"},{"line_number":37,"context_line":"    deprecated_since\u003d\u00272026.2\u0027,"},{"line_number":38,"context_line":")"},{"line_number":39,"context_line":"deprecated_disable \u003d policy.DeprecatedRule("},{"line_number":40,"context_line":"    name\u003d\u0027cyborg:device:disable\u0027,"},{"line_number":41,"context_line":"    check_str\u003d\u0027rule:admin_api\u0027,"},{"line_number":42,"context_line":"    deprecated_reason\u003d("}],"source_content_type":"text/x-python","patch_set":4,"id":"8ea8d56f_813891a9","line":39,"in_reply_to":"34e0e0d4_d8bd7b05","updated":"2026-06-30 11:11:12.000000000","message":"i created a patch at the end for this","commit_id":"a64a3a88ca6adf2abc3112b8e22fe7e39c5a69ae"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"aeb39f3ba65cf314974788c6a40ec10db7b85961","unresolved":true,"context_lines":[{"line_number":36,"context_line":"    ),"},{"line_number":37,"context_line":"    deprecated_since\u003d\u00272026.2\u0027,"},{"line_number":38,"context_line":")"},{"line_number":39,"context_line":"deprecated_disable \u003d policy.DeprecatedRule("},{"line_number":40,"context_line":"    name\u003d\u0027cyborg:device:disable\u0027,"},{"line_number":41,"context_line":"    check_str\u003d\u0027rule:admin_api\u0027,"},{"line_number":42,"context_line":"    deprecated_reason\u003d("}],"source_content_type":"text/x-python","patch_set":4,"id":"34e0e0d4_d8bd7b05","line":39,"in_reply_to":"895984e0_e1e83ad3","updated":"2026-06-26 11:37:33.000000000","message":"this is somehtin i need to double chekc but i guess that should work","commit_id":"a64a3a88ca6adf2abc3112b8e22fe7e39c5a69ae"}],"cyborg/tests/unit/policies/test_devices.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"b64cfdaf565e299b18525ca26cdb4be634b604d1","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        #    also ORs that rule\u0027s check with its own deprecated bridge"},{"line_number":49,"context_line":"        #    (is_admin:True or project_id:%(project_id)s)."},{"line_number":50,"context_line":"        #"},{"line_number":51,"context_line":"        # get_all uses need_target\u003dFalse so target[\u0027project_id\u0027] equals"},{"line_number":52,"context_line":"        # the caller\u0027s project_id. The deprecated bridge on"},{"line_number":53,"context_line":"        # project_manager_or_admin (project_id:%(project_id)s) matches any"},{"line_number":54,"context_line":"        # project-scoped context, so all project-scoped contexts are"}],"source_content_type":"text/x-python","patch_set":4,"id":"b2237257_613a75ad","line":51,"updated":"2026-06-23 00:27:35.000000000","message":"Test comments reference need_target\u003dFalse/need_target\u003dTrue, a parameter removed by change 992722. authorize_wsgi now always builds target from the request context; the need_target concept no longer exists.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Future maintainers reading these comments will look for a need_target parameter that does not exist, causing confusion about how the policy target is constructed. The comments describe stale behavior rather than the actual code path.\n\n**Suggestion**:\nRewrite comments to describe current behavior: authorize_wsgi always passes {\u0027project_id\u0027: context.project_id, \u0027user_id\u0027: context.user_id} as the policy target. Replace \u0027need_target\u003dFalse\u0027 with \u0027the target project_id equals the caller project_id\u0027 and remove need_target\u003dTrue references at lines 78-79.","commit_id":"a64a3a88ca6adf2abc3112b8e22fe7e39c5a69ae"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"b64cfdaf565e299b18525ca26cdb4be634b604d1","unresolved":false,"context_lines":[{"line_number":75,"context_line":"            set(self.all_contexts) - set(self.read_authorized_contexts)"},{"line_number":76,"context_line":"        )"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        # disable and enable remain admin-only (same new default and"},{"line_number":79,"context_line":"        # deprecated bridge: rule:admin_api). need_target\u003dTrue with no"},{"line_number":80,"context_line":"        # _get_resource means target is {} so manager check never matches."},{"line_number":81,"context_line":"        self.write_authorized_contexts \u003d ["}],"source_content_type":"text/x-python","patch_set":4,"id":"a18e97d4_632eaaa8","line":78,"updated":"2026-06-23 00:27:35.000000000","message":"Test comment at lines 78-79 claims need_target\u003dTrue leaves target as empty {} so the manager check never matches. This is inaccurate: authorize_wsgi always builds target from context with project_id and user_id.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: The claim that target is {} is factually wrong since authorize_wsgi always populates project_id and user_id from the context. The real reason disable/enable are admin-only is that the check_str is rule:admin_api (role:admin) which non-admin contexts lack.\n\n**Suggestion**:\nUpdate the comment to explain that disable/enable use check_str\u003dbase.ADMIN (role:admin), so only contexts carrying the admin role pass. The target construction is irrelevant since admin_api has no project_id requirement.","commit_id":"a64a3a88ca6adf2abc3112b8e22fe7e39c5a69ae"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"1e6fbfef0eb3c626eaa61c5c1cf8260f8bebc9a9","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        mock_dep_get.return_value \u003d mock_dep"},{"line_number":156,"context_line":"        mock_attr_filter.return_value \u003d [mock.MagicMock(value\u003d\u0027CUSTOM_FOO\u0027)]"},{"line_number":157,"context_line":"        mock_pc_cls.return_value \u003d mock.MagicMock()"},{"line_number":158,"context_line":"        for context in self.write_authorized_contexts:"},{"line_number":159,"context_line":"            headers \u003d self.gen_headers(context)"},{"line_number":160,"context_line":"            response \u003d self.post_json("},{"line_number":161,"context_line":"                DEVICE_URL + \u0027/%s/disable\u0027 % fake_dev.uuid,"}],"source_content_type":"text/x-python","patch_set":4,"id":"cd42b977_41972f3f","line":158,"range":{"start_line":158,"start_character":28,"end_line":158,"end_character":54},"updated":"2026-06-26 08:34:12.000000000","message":"Nice catch on fixing the test context list.","commit_id":"a64a3a88ca6adf2abc3112b8e22fe7e39c5a69ae"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"aeb39f3ba65cf314974788c6a40ec10db7b85961","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        mock_dep_get.return_value \u003d mock_dep"},{"line_number":156,"context_line":"        mock_attr_filter.return_value \u003d [mock.MagicMock(value\u003d\u0027CUSTOM_FOO\u0027)]"},{"line_number":157,"context_line":"        mock_pc_cls.return_value \u003d mock.MagicMock()"},{"line_number":158,"context_line":"        for context in self.write_authorized_contexts:"},{"line_number":159,"context_line":"            headers \u003d self.gen_headers(context)"},{"line_number":160,"context_line":"            response \u003d self.post_json("},{"line_number":161,"context_line":"                DEVICE_URL + \u0027/%s/disable\u0027 % fake_dev.uuid,"}],"source_content_type":"text/x-python","patch_set":4,"id":"6600534f_5a946d60","line":158,"range":{"start_line":158,"start_character":28,"end_line":158,"end_character":54},"in_reply_to":"cd42b977_41972f3f","updated":"2026-06-26 11:37:33.000000000","message":"before this patch read was just admin so now that im being more granular about this updatring it made sense","commit_id":"a64a3a88ca6adf2abc3112b8e22fe7e39c5a69ae"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d6d90f0122b922971a896f0b8c25e59b8bbb608f","unresolved":false,"context_lines":[{"line_number":139,"context_line":"                ):"},{"line_number":140,"context_line":"                    self.get_json(DEVICE_URL + \u0027/%s\u0027 % uuid, headers\u003dheaders)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    @mock.patch("},{"line_number":143,"context_line":"        \u0027cyborg.api.controllers.v2.devices.placement_client.PlacementClient\u0027,"},{"line_number":144,"context_line":"        autospec\u003dTrue,"},{"line_number":145,"context_line":"    )"},{"line_number":146,"context_line":"    @mock.patch(\u0027cyborg.objects.Attribute.get_by_filter\u0027, autospec\u003dTrue)"},{"line_number":147,"context_line":"    @mock.patch(\u0027cyborg.objects.Deployable.get_by_id\u0027, autospec\u003dTrue)"},{"line_number":148,"context_line":"    @mock.patch(\u0027cyborg.objects.Device.save\u0027, autospec\u003dTrue)"},{"line_number":149,"context_line":"    @mock.patch(\u0027cyborg.objects.Device.get\u0027, autospec\u003dTrue)"},{"line_number":150,"context_line":"    def test_disable_device_success("},{"line_number":151,"context_line":"        self,"},{"line_number":152,"context_line":"        mock_get,"}],"source_content_type":"text/x-python","patch_set":5,"id":"8669f673_ae903558","line":149,"range":{"start_line":142,"start_character":0,"end_line":149,"end_character":59},"updated":"2026-07-06 23:01:31.000000000","message":"normally i would say this is over mocked but these test are only testing the policy layer os all the object leve call shoudl be mocked.\n\nit woudl probaly be a good idea to move this logic into a class method in the future so this\n\nhttps://github.com/openstack/cyborg/blob/master/cyborg/api/controllers/v2/devices.py#L155-L179\n\nbecome one call `device \u003d object.Device.disable(context, uuid)`\nbut that is also preebt messay as wehn we disabel a device we also have to find the deployable and update the corresponding placement resouce provider.\n\nthat is not somethign we shoudl be doing in the api.\n\nso really we shoudl be turning this methos into an RPC call to the cybrog agent so that it can handel this properly.\n\ncuyrrently the update to this form the api can race with the conductor updating the inventoy on because of the agent sending an period update.\n\nanyway this test is correct but the disable logic is in the wrong place and likely buggy...","commit_id":"d84b6ad47faebd1bc0c810574dd9e5b3e69a3bb9"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"58eb2eb3581a9194b68940ac5eec98aea05a118a","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            )"},{"line_number":136,"context_line":"            self.assertEqual(uuid, response[\u0027uuid\u0027])"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def test_get_one_device_forbidden(self):"},{"line_number":139,"context_line":"        uuid \u003d self.fake_devices[0][\u0027uuid\u0027]"},{"line_number":140,"context_line":"        for context in self.read_unauthorized_contexts:"},{"line_number":141,"context_line":"            with self.subTest(context\u003dcontext):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3b102d07_98fb81cb","line":138,"updated":"2026-07-21 11:23:11.000000000","message":"System-scope forbidden tests were added for get_all and disable but are missing for get_one and enable across both test classes. The ARQ policy migration (same SRBAC series) includes system-scope forbidden tests for every operation, establishing the expected pattern.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: System-scoped tokens could regress to being accepted by get_one or enable endpoints without a test catching it, since only get_all and disable are verified for scope enforcement. All four device operations use scope_types\u003d[\u0027project\u0027], so all should be tested.\n\n**Recommendation**:\nAdd test_get_one_device_system_scope_forbidden and test_enable_device_system_scope_forbidden in DevicePolicyTest, and corresponding new-defaults variants in DevicePolicyNewDefaultsTest, following the existing test_get_all_devices_system_scope_forbidden pattern.","commit_id":"1ddb15cdcadfceac30cdff52f37185736688b8bc"}]}