)]}'
{"specs/approved/node-owner-policy.rst":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"76d6940591ed9b7e12150f392a49057fbb597bc9","unresolved":false,"context_lines":[{"line_number":5,"context_line":" http://creativecommons.org/licenses/by/3.0/legalcode"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":8,"context_line":"Adding Policy Targets for Ironic Nodes"},{"line_number":9,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"https://storyboard.openstack.org/#!/story/2006506"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5faad753_16c60a9c","line":8,"updated":"2019-09-10 07:03:01.000000000","message":"I\u0027d reword this spec in terms of what you\u0027re trying to achieve, this seems just one of the components.","commit_id":"c041898160f67adbfa5c304bd077ba5cc16319ca"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"99fdee0c14d363296c162fb0e304d65f1480d33a","unresolved":false,"context_lines":[{"line_number":5,"context_line":" http://creativecommons.org/licenses/by/3.0/legalcode"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":8,"context_line":"Adding Policy Targets for Ironic Nodes"},{"line_number":9,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"https://storyboard.openstack.org/#!/story/2006506"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5faad753_6160c8a6","line":8,"in_reply_to":"5faad753_16c60a9c","updated":"2019-09-10 16:15:16.000000000","message":"Done - let me know if the change works for you!","commit_id":"c041898160f67adbfa5c304bd077ba5cc16319ca"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"76d6940591ed9b7e12150f392a49057fbb597bc9","unresolved":false,"context_lines":[{"line_number":45,"context_line":"  [4]_).  For example:"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"   *  \"is_node_owner\": \"project_id:%(node.owner)s\""},{"line_number":48,"context_line":"   *  \"baremetal:node:set_power_state\": \"rule:is_admin or rule:is_node_owner\""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"This update is enough to control access for most API functions - except for"},{"line_number":51,"context_line":"list functions. For those, we propose updating the node list query so that"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5faad753_36c1c681","line":48,"updated":"2019-09-10 07:03:01.000000000","message":"Which actions are we going to allow the owners, all? Including deleting nodes?","commit_id":"c041898160f67adbfa5c304bd077ba5cc16319ca"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"99fdee0c14d363296c162fb0e304d65f1480d33a","unresolved":false,"context_lines":[{"line_number":45,"context_line":"  [4]_).  For example:"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"   *  \"is_node_owner\": \"project_id:%(node.owner)s\""},{"line_number":48,"context_line":"   *  \"baremetal:node:set_power_state\": \"rule:is_admin or rule:is_node_owner\""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"This update is enough to control access for most API functions - except for"},{"line_number":51,"context_line":"list functions. For those, we propose updating the node list query so that"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5faad753_215ad0f2","line":48,"in_reply_to":"5faad753_36c1c681","updated":"2019-09-10 16:15:16.000000000","message":"Clarified.","commit_id":"c041898160f67adbfa5c304bd077ba5cc16319ca"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"76d6940591ed9b7e12150f392a49057fbb597bc9","unresolved":false,"context_lines":[{"line_number":50,"context_line":"This update is enough to control access for most API functions - except for"},{"line_number":51,"context_line":"list functions. For those, we propose updating the node list query so that"},{"line_number":52,"context_line":"non-admin results are filtered against the ``owner`` field, in a manner"},{"line_number":53,"context_line":"analogous to other OpenStack services."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Alternatives"},{"line_number":56,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5faad753_d6bf12f9","line":53,"updated":"2019-09-10 07:03:01.000000000","message":"This is a major change worth elaborating on. E.g. when do you use filtering? What if owner is None?","commit_id":"c041898160f67adbfa5c304bd077ba5cc16319ca"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"99fdee0c14d363296c162fb0e304d65f1480d33a","unresolved":false,"context_lines":[{"line_number":50,"context_line":"This update is enough to control access for most API functions - except for"},{"line_number":51,"context_line":"list functions. For those, we propose updating the node list query so that"},{"line_number":52,"context_line":"non-admin results are filtered against the ``owner`` field, in a manner"},{"line_number":53,"context_line":"analogous to other OpenStack services."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Alternatives"},{"line_number":56,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5faad753_415d8ce7","line":53,"in_reply_to":"5faad753_d6bf12f9","updated":"2019-09-10 16:15:16.000000000","message":"Added some detail.","commit_id":"c041898160f67adbfa5c304bd077ba5cc16319ca"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"76d6940591ed9b7e12150f392a49057fbb597bc9","unresolved":false,"context_lines":[{"line_number":51,"context_line":"list functions. For those, we propose updating the node list query so that"},{"line_number":52,"context_line":"non-admin results are filtered against the ``owner`` field, in a manner"},{"line_number":53,"context_line":"analogous to other OpenStack services."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Alternatives"},{"line_number":56,"context_line":"------------"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5faad753_f6ba4e08","line":54,"updated":"2019-09-10 07:03:01.000000000","message":"Another required change is updating the allocation API to filter nodes by owner, otherwise a user may be allocated a node they don\u0027t have access to.","commit_id":"c041898160f67adbfa5c304bd077ba5cc16319ca"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"81fdff7ec144cb096dd32da18efda8c0fe4e6fb0","unresolved":false,"context_lines":[{"line_number":51,"context_line":"list functions. For those, we propose updating the node list query so that"},{"line_number":52,"context_line":"non-admin results are filtered against the ``owner`` field, in a manner"},{"line_number":53,"context_line":"analogous to other OpenStack services."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Alternatives"},{"line_number":56,"context_line":"------------"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5faad753_80570303","line":54,"in_reply_to":"5faad753_2555011e","updated":"2019-09-10 14:12:30.000000000","message":"It\u0027s an interesting discussion topic, it depends on how you define \"owner\", which is the reason I\u0027m suggesting to reframe this specification around use cases. The way I understand it, owner is *non-admin* user that has exclusive access to some hardware. If so, it feels natural to only limit their allocations to the nodes they own. Am I missing something?","commit_id":"c041898160f67adbfa5c304bd077ba5cc16319ca"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"b00aa3d7703d70a4ac0b720813a7e5abc88624ea","unresolved":false,"context_lines":[{"line_number":51,"context_line":"list functions. For those, we propose updating the node list query so that"},{"line_number":52,"context_line":"non-admin results are filtered against the ``owner`` field, in a manner"},{"line_number":53,"context_line":"analogous to other OpenStack services."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Alternatives"},{"line_number":56,"context_line":"------------"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5faad753_80ddc343","line":54,"in_reply_to":"5faad753_80570303","updated":"2019-09-10 14:23:32.000000000","message":"Ah, so before Lars and I split this spec up, we were thinking of something like:\n\n* Owner: A node Owner who has full Ironic API access to a node.\n* User: A node User who has restricted Ironic API access to a node.\n\nSo an Owner is more of an admin, while the badly-named User is a non-admin user. I can add the \u0027Owner\u0027 clarification to this spec - would that help clarify things?","commit_id":"c041898160f67adbfa5c304bd077ba5cc16319ca"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"79f613f0c22495abf91652e828df5cf44875e227","unresolved":false,"context_lines":[{"line_number":51,"context_line":"list functions. For those, we propose updating the node list query so that"},{"line_number":52,"context_line":"non-admin results are filtered against the ``owner`` field, in a manner"},{"line_number":53,"context_line":"analogous to other OpenStack services."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Alternatives"},{"line_number":56,"context_line":"------------"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5faad753_e0bb57c1","line":54,"in_reply_to":"5faad753_80ddc343","updated":"2019-09-10 14:27:17.000000000","message":"No, owners are not an administrator, that\u0027s why we need to limit allocation API for them. Your initial assumption was right, I think.\n\n\u003e I can add the \u0027Owner\u0027 clarification to this spec - would that help clarify things?\n\nYes please! A few user stories wouldn\u0027t hurt either.","commit_id":"c041898160f67adbfa5c304bd077ba5cc16319ca"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"99fdee0c14d363296c162fb0e304d65f1480d33a","unresolved":false,"context_lines":[{"line_number":51,"context_line":"list functions. For those, we propose updating the node list query so that"},{"line_number":52,"context_line":"non-admin results are filtered against the ``owner`` field, in a manner"},{"line_number":53,"context_line":"analogous to other OpenStack services."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Alternatives"},{"line_number":56,"context_line":"------------"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5faad753_a16d6099","line":54,"in_reply_to":"5faad753_e0bb57c1","updated":"2019-09-10 16:15:16.000000000","message":"Added this clarification; Lars will add some user stories.","commit_id":"c041898160f67adbfa5c304bd077ba5cc16319ca"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"e0f4fcf132d08360b50b0c8b79b290f2c7f90fcb","unresolved":false,"context_lines":[{"line_number":51,"context_line":"list functions. For those, we propose updating the node list query so that"},{"line_number":52,"context_line":"non-admin results are filtered against the ``owner`` field, in a manner"},{"line_number":53,"context_line":"analogous to other OpenStack services."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Alternatives"},{"line_number":56,"context_line":"------------"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5faad753_2555011e","line":54,"in_reply_to":"5faad753_f6ba4e08","updated":"2019-09-10 14:09:14.000000000","message":"Thanks for the comments! One quick question; I don\u0027t think we intended the owner to be the only one to be able to allocate to the node; just be able to run node management functions. But is there a good reason to include allocations here as well?","commit_id":"c041898160f67adbfa5c304bd077ba5cc16319ca"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"76d6940591ed9b7e12150f392a49057fbb597bc9","unresolved":false,"context_lines":[{"line_number":173,"context_line":"Upgrades and Backwards Compatibility"},{"line_number":174,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"N/A"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"Documentation Impact"},{"line_number":179,"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":1,"id":"5faad753_96375ab1","line":176,"updated":"2019-09-10 07:03:01.000000000","message":"Well, not exactly N/A, you\u0027re changing the default behavior. What will happen for users that use the \u0027owner\u0027 field for something other than project ID?","commit_id":"c041898160f67adbfa5c304bd077ba5cc16319ca"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"99fdee0c14d363296c162fb0e304d65f1480d33a","unresolved":false,"context_lines":[{"line_number":173,"context_line":"Upgrades and Backwards Compatibility"},{"line_number":174,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"N/A"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"Documentation Impact"},{"line_number":179,"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":1,"id":"5faad753_610528c2","line":176,"in_reply_to":"5faad753_96375ab1","updated":"2019-09-10 16:15:16.000000000","message":"Added some detail regarding possible consequences.","commit_id":"c041898160f67adbfa5c304bd077ba5cc16319ca"},{"author":{"_account_id":8745,"name":"Lars Kellogg-Stedman","email":"lars@redhat.com","username":"lars"},"change_message_id":"4a6246f5fed5b939dae0e3f4c0b127aa211bd156","unresolved":false,"context_lines":[{"line_number":200,"context_line":"Testing"},{"line_number":201,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"Both unit tests and Tempest tests will be added."},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"Upgrades and Backwards Compatibility"},{"line_number":206,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\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":4,"id":"5faad753_1fa1df94","line":203,"range":{"start_line":203,"start_character":0,"end_line":203,"end_character":48},"updated":"2019-09-13 15:10:55.000000000","message":"I would use the active voice here:\n\n  We will add unit tests and tempest tests.","commit_id":"87609b4ede93d99f61935f4fadbe1770295d67b6"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"073818c6a65b8058a56efe54c4ef0e5295cdca73","unresolved":false,"context_lines":[{"line_number":200,"context_line":"Testing"},{"line_number":201,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"Both unit tests and Tempest tests will be added."},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"Upgrades and Backwards Compatibility"},{"line_number":206,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\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":4,"id":"5faad753_9f370f00","line":203,"range":{"start_line":203,"start_character":0,"end_line":203,"end_character":48},"in_reply_to":"5faad753_1fa1df94","updated":"2019-09-13 15:20:57.000000000","message":"Fair enough! Made the change.","commit_id":"87609b4ede93d99f61935f4fadbe1770295d67b6"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"f6adcfb52d09f7346990f134f3150b80d987c671","unresolved":false,"context_lines":[{"line_number":52,"context_line":"the same. In summary, we would like to:"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"* Assume that a node\u0027s ``owner`` field is set to the id of a Keystone project."},{"line_number":55,"context_line":"  We refer to this project as the \"node owner\"."},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"* Update the node controller so that policy checks also pass in information"},{"line_number":58,"context_line":"  about the node, including the ``owner`` field."}],"source_content_type":"text/x-rst","patch_set":5,"id":"3fa7e38b_6a0a338b","line":55,"updated":"2019-09-16 16:07:43.000000000","message":"I wonder about this assumption and whether this works for most cases.\n\nEg, a bunch of BM nodes are assigned to project FOO. 10 people / users are in project FOO. Do they want all users in project FOO to be able to have this control? Or would it be better to specify some subset of the users in project FOO, that have this admin control?\n\nI haven\u0027t looked at what nova or barbican does; just a thought.","commit_id":"b80c9edae7350cae4838c06ebd73fa6d4daaafb5"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"f3e90ae96286aba8fca429623640d5195a63d7f1","unresolved":false,"context_lines":[{"line_number":52,"context_line":"the same. In summary, we would like to:"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"* Assume that a node\u0027s ``owner`` field is set to the id of a Keystone project."},{"line_number":55,"context_line":"  We refer to this project as the \"node owner\"."},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"* Update the node controller so that policy checks also pass in information"},{"line_number":58,"context_line":"  about the node, including the ``owner`` field."}],"source_content_type":"text/x-rst","patch_set":5,"id":"3fa7e38b_538e0e28","line":55,"in_reply_to":"3fa7e38b_6a0a338b","updated":"2019-09-16 17:55:48.000000000","message":"That\u0027s a reasonable concern! I would think that in such a case, the admins would either create a new node-admin project, or simply ensure that the policy file doesn\u0027t allow an owner to do anything with the Ironic API.","commit_id":"b80c9edae7350cae4838c06ebd73fa6d4daaafb5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"f6adcfb52d09f7346990f134f3150b80d987c671","unresolved":false,"context_lines":[{"line_number":64,"context_line":"   *  \"is_node_owner\": \"project_id:%(node.owner)s\""},{"line_number":65,"context_line":"   *  \"baremetal:node:set_power_state\": \"rule:is_admin or rule:is_node_owner\""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"  The default generated policy file would give node owners access to all"},{"line_number":68,"context_line":"  actions that modify the state of nodes they own. A node owner will not be"},{"line_number":69,"context_line":"  able to create or delete nodes."},{"line_number":70,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"3fa7e38b_aa726b10","line":67,"updated":"2019-09-16 16:07:43.000000000","message":"this would break existing functionality. Shouldn\u0027t the default be to NOT give node owners access? Especially worries me, since you are making an assumption that the value in the node.owners field *should be* a keystone project.\n\nGoing back to my example above, maybe all/most users in project FOO should not have access, and they added \u0027FOO\u0027 as the node.owner. This will have security implications if all of a sudden, the users have admin access when that was not intended in the first place.","commit_id":"b80c9edae7350cae4838c06ebd73fa6d4daaafb5"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"f3e90ae96286aba8fca429623640d5195a63d7f1","unresolved":false,"context_lines":[{"line_number":64,"context_line":"   *  \"is_node_owner\": \"project_id:%(node.owner)s\""},{"line_number":65,"context_line":"   *  \"baremetal:node:set_power_state\": \"rule:is_admin or rule:is_node_owner\""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"  The default generated policy file would give node owners access to all"},{"line_number":68,"context_line":"  actions that modify the state of nodes they own. A node owner will not be"},{"line_number":69,"context_line":"  able to create or delete nodes."},{"line_number":70,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"3fa7e38b_d3cf5e55","line":67,"in_reply_to":"3fa7e38b_aa726b10","updated":"2019-09-16 17:55:48.000000000","message":"That\u0027s fair! I thought that a generated default policy file would not replace whatever existing policy file exists - but now that I think about it, if no policy file is being used, then the default goes back to the code, and that would change the behavior as you described.\n\nI\u0027ll update the spec to reverse what I said here: the default will be to have *no* additional access for owners.","commit_id":"b80c9edae7350cae4838c06ebd73fa6d4daaafb5"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"1de159287e2ddd4f45a9764eb941db99545ffe2e","unresolved":false,"context_lines":[{"line_number":63,"context_line":"   *  \"is_node_owner\": \"project_id:%(node.owner)s\""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"  The remainder of the policy file would stay the same, meaning that there is"},{"line_number":66,"context_line":"  no change to default API access."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"* Create documentation explaining how to update an Ironic policy file to give"},{"line_number":69,"context_line":"  API access to owners. For example:"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3fa7e38b_6ddf093a","line":66,"updated":"2019-09-25 13:37:35.000000000","message":"++","commit_id":"28817fe6bf5a1a17dfb9e2a1542444159397b45f"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"1de159287e2ddd4f45a9764eb941db99545ffe2e","unresolved":false,"context_lines":[{"line_number":66,"context_line":"  no change to default API access."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"* Create documentation explaining how to update an Ironic policy file to give"},{"line_number":69,"context_line":"  API access to owners. For example:"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"   *  \"baremetal:node:set_power_state\": \"rule:is_admin or rule:is_node_owner\""},{"line_number":72,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"3fa7e38b_0de8d5df","line":69,"updated":"2019-09-25 13:37:35.000000000","message":"I wonder if we can allow generating an example file doing that.. maybe not.","commit_id":"28817fe6bf5a1a17dfb9e2a1542444159397b45f"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"4b1b576c50eb23a9c506c812b0c16cc55eacd63b","unresolved":false,"context_lines":[{"line_number":66,"context_line":"  no change to default API access."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"* Create documentation explaining how to update an Ironic policy file to give"},{"line_number":69,"context_line":"  API access to owners. For example:"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"   *  \"baremetal:node:set_power_state\": \"rule:is_admin or rule:is_node_owner\""},{"line_number":72,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"3fa7e38b_08d3193a","line":69,"in_reply_to":"3fa7e38b_0de8d5df","updated":"2019-10-08 17:10:50.000000000","message":"Well, we could actually have that be the default maybe? They should be able to generate the policy file from the code as it should all be present in the code to begin with, at least for defaults.","commit_id":"28817fe6bf5a1a17dfb9e2a1542444159397b45f"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"1de159287e2ddd4f45a9764eb941db99545ffe2e","unresolved":false,"context_lines":[{"line_number":76,"context_line":"list functions. For those, we propose the following:"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"* Introducing a new policy rule called ``baremetal:node:list`` and using it"},{"line_number":79,"context_line":"  for the REST API node list functions."},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":82,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3fa7e38b_2de59109","line":79,"updated":"2019-09-25 13:37:35.000000000","message":"In what way exactly? call it for each returned node?","commit_id":"28817fe6bf5a1a17dfb9e2a1542444159397b45f"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"5c6f4c7a5a2db33a2e6cb2e56febcd4120946986","unresolved":false,"context_lines":[{"line_number":76,"context_line":"list functions. For those, we propose the following:"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"* Introducing a new policy rule called ``baremetal:node:list`` and using it"},{"line_number":79,"context_line":"  for the REST API node list functions."},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":82,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3fa7e38b_02117f52","line":79,"in_reply_to":"3fa7e38b_2de59109","updated":"2019-09-25 15:08:38.000000000","message":"Not exactly - the intent would be to expose the list API to everyone. However, right now the node controller API uses \u0027baremetal:node:get\u0027 as its policy check for the list function, and we don\u0027t want to expose the get function to everyone. \u0027baremetal:node:list\u0027 is simply a way to differentiate the two.\n\nI\u0027ll add some clarification to the spec.","commit_id":"28817fe6bf5a1a17dfb9e2a1542444159397b45f"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"1de159287e2ddd4f45a9764eb941db99545ffe2e","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"* Updating the default generated policy file to expose"},{"line_number":87,"context_line":"  ``baremetal:node:list`` to everyone."},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"Alternatives"},{"line_number":91,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3fa7e38b_cdedddf0","line":88,"updated":"2019-09-25 13:37:35.000000000","message":"We still need to do something about allocations. With your proposal a non-admin user issues `POST /v1/allocations` may end up with a node that they cannot access. And, unfortunately, it cannot be solved with a policy, so I wonder how to do it.\n\nAnother fun thing about allocations: we probably need an owner for them as well. Otherwise a non-admin user can see all allocations, even ones that were not created by them.","commit_id":"28817fe6bf5a1a17dfb9e2a1542444159397b45f"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"5c6f4c7a5a2db33a2e6cb2e56febcd4120946986","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"* Updating the default generated policy file to expose"},{"line_number":87,"context_line":"  ``baremetal:node:list`` to everyone."},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"Alternatives"},{"line_number":91,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3fa7e38b_22327b5f","line":88,"in_reply_to":"3fa7e38b_cdedddf0","updated":"2019-09-25 15:08:38.000000000","message":"I\u0027ll add some text clarifying that this spec does not deal with allocations (although it also does not change the current behavior), along with some notes regarding how this work might be approached in a follow up spec.","commit_id":"28817fe6bf5a1a17dfb9e2a1542444159397b45f"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"1de159287e2ddd4f45a9764eb941db99545ffe2e","unresolved":false,"context_lines":[{"line_number":107,"context_line":"REST API impact"},{"line_number":108,"context_line":"---------------"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"None"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"Client (CLI) impact"},{"line_number":113,"context_line":"-------------------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3fa7e38b_edea19d6","line":110,"updated":"2019-09-25 13:37:35.000000000","message":"Not really None. Feel free to just link back to the proposed changes.","commit_id":"28817fe6bf5a1a17dfb9e2a1542444159397b45f"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"5c6f4c7a5a2db33a2e6cb2e56febcd4120946986","unresolved":false,"context_lines":[{"line_number":107,"context_line":"REST API impact"},{"line_number":108,"context_line":"---------------"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"None"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"Client (CLI) impact"},{"line_number":113,"context_line":"-------------------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3fa7e38b_c25da715","line":110,"in_reply_to":"3fa7e38b_edea19d6","updated":"2019-09-25 15:08:38.000000000","message":"Done.","commit_id":"28817fe6bf5a1a17dfb9e2a1542444159397b45f"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"1de159287e2ddd4f45a9764eb941db99545ffe2e","unresolved":false,"context_lines":[{"line_number":192,"context_line":"----------"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"* Update node controller."},{"line_number":195,"context_line":"* Update allocation API."},{"line_number":196,"context_line":"* Add documentation."},{"line_number":197,"context_line":"* Write tests."},{"line_number":198,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"3fa7e38b_8dbda5d9","line":195,"updated":"2019-09-25 13:37:35.000000000","message":"This needs details somewhere above.","commit_id":"28817fe6bf5a1a17dfb9e2a1542444159397b45f"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"4b1b576c50eb23a9c506c812b0c16cc55eacd63b","unresolved":false,"context_lines":[{"line_number":63,"context_line":"   *  \"is_node_owner\": \"project_id:%(node.owner)s\""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"  The remainder of the policy file would stay the same, meaning that there is"},{"line_number":66,"context_line":"  no change to default API access."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"* Create documentation explaining how to update an Ironic policy file to give"},{"line_number":69,"context_line":"  API access to owners. For example:"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_a8220500","line":66,"updated":"2019-10-08 17:10:50.000000000","message":"I think we should actually change the API default such that is_node_owner grants access to the machine. We foretold that this field may be used in the future, and this doesn\u0027t seem like any sort of breaking change. It seems more of an unlocking/enabling change.","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"abc75a8f3d081247b99948df34c368ab7a1cefef","unresolved":false,"context_lines":[{"line_number":63,"context_line":"   *  \"is_node_owner\": \"project_id:%(node.owner)s\""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"  The remainder of the policy file would stay the same, meaning that there is"},{"line_number":66,"context_line":"  no change to default API access."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"* Create documentation explaining how to update an Ironic policy file to give"},{"line_number":69,"context_line":"  API access to owners. For example:"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_338d19d3","line":66,"updated":"2019-10-09 09:44:50.000000000","message":"We can do it in a new RFE, right?","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"5cce3b2e3601c1f51022b5d3acaacb93b2e2f6c6","unresolved":false,"context_lines":[{"line_number":63,"context_line":"   *  \"is_node_owner\": \"project_id:%(node.owner)s\""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"  The remainder of the policy file would stay the same, meaning that there is"},{"line_number":66,"context_line":"  no change to default API access."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"* Create documentation explaining how to update an Ironic policy file to give"},{"line_number":69,"context_line":"  API access to owners. For example:"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_8301183b","line":66,"in_reply_to":"3fa7e38b_338d19d3","updated":"2019-10-09 15:46:27.000000000","message":"of course. It makes me wonder if covering any nova centric stuff in that mgiht be good as well.","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"abc75a8f3d081247b99948df34c368ab7a1cefef","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"* Right now, the policy rule \u0027baremetal:node:get\u0027 is used for both ``get_all``"},{"line_number":79,"context_line":"  and ``get_one``. Adding a policy rule ``baremetal:node:list`` for the"},{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_d3bda568","line":80,"updated":"2019-10-09 09:44:50.000000000","message":"As it\u0027s written, it sounds like a breaking change. What will happen on upgrade if baremetal:node:get is set to something non-default?\n\nI\u0027m also not sure how exactly does it work. Why cannot we call baremetal:node:get per node rather than for the whole listing.\n\nNote that the following answer is incorrect: we just call baremetal:node:list and either return all nodes or deny access. An owner must be able to see a listing of their (and only their) nodes.","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"4b1b576c50eb23a9c506c812b0c16cc55eacd63b","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"* Right now, the policy rule \u0027baremetal:node:get\u0027 is used for both ``get_all``"},{"line_number":79,"context_line":"  and ``get_one``. Adding a policy rule ``baremetal:node:list`` for the"},{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_68688d86","line":80,"updated":"2019-10-08 17:10:50.000000000","message":"This makes sense. I can see an environment where one team manages the physical machines, and they only convey a UUID over, so the permissions may need to be different between the two for compliance and separation of duties reasons.","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"5cce3b2e3601c1f51022b5d3acaacb93b2e2f6c6","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"* Right now, the policy rule \u0027baremetal:node:get\u0027 is used for both ``get_all``"},{"line_number":79,"context_line":"  and ``get_one``. Adding a policy rule ``baremetal:node:list`` for the"},{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_e34c8c9f","line":80,"in_reply_to":"3fa7e38b_288e0379","updated":"2019-10-09 15:46:27.000000000","message":"We would have to extend list under the hood to be able to take an argument such that filtered results can be returned by the API by project membership, FWIW. I totally agree though, and I think a separate RFE for some of the interoperation mechanics with things like nova is going to be key for users.\n\nThe key is if ! is_admin, then we only return what is in the project list for nodes so the query then sends along the project ID and that list as a result MAY be empty.","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c7fb4c1bb999fd69e6c9ce05687e774c99395648","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"* Right now, the policy rule \u0027baremetal:node:get\u0027 is used for both ``get_all``"},{"line_number":79,"context_line":"  and ``get_one``. Adding a policy rule ``baremetal:node:list`` for the"},{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_c87f2f64","line":80,"in_reply_to":"3fa7e38b_88105750","updated":"2019-10-09 14:32:09.000000000","message":"I assumed that\u0027s what you suggest in the recent patchset, no?","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"4eb5f324a717b044313a4769bba2d74b05560257","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"* Right now, the policy rule \u0027baremetal:node:get\u0027 is used for both ``get_all``"},{"line_number":79,"context_line":"  and ``get_one``. Adding a policy rule ``baremetal:node:list`` for the"},{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_dd636f19","line":80,"in_reply_to":"3fa7e38b_9a373dff","updated":"2019-10-09 14:26:13.000000000","message":"It\u0027s critical to be able to show a user nodes that they own. It\u0027s a much bigger feature gap than with allocations, I don\u0027t think we can accept it.","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"5675df1c1b5ed8a6be7072bdf62cc2e9846edc6d","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"* Right now, the policy rule \u0027baremetal:node:get\u0027 is used for both ``get_all``"},{"line_number":79,"context_line":"  and ``get_one``. Adding a policy rule ``baremetal:node:list`` for the"},{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_288e0379","line":80,"in_reply_to":"3fa7e38b_c87f2f64","updated":"2019-10-09 14:33:14.000000000","message":"Haha, yes - I had some comment thread confusion.","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"88a4e0fa852e80451e5f0232576b41e5f59e1405","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"* Right now, the policy rule \u0027baremetal:node:get\u0027 is used for both ``get_all``"},{"line_number":79,"context_line":"  and ``get_one``. Adding a policy rule ``baremetal:node:list`` for the"},{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_9a373dff","line":80,"in_reply_to":"3fa7e38b_d3bda568","updated":"2019-10-09 13:50:50.000000000","message":"I\u0027ll update this to state that the \u0027baremetal:node:list\" target defaults to \"rule:baremetal:node:get\", which should address the problem Dmitry pointed out.\n\nHaving a policy check per node sounds like it could be expensive. Note that I address updating the node get_all function in the next bullet point.","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"682fae98b103d87cd19961b8e878491ce81747c0","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"* Right now, the policy rule \u0027baremetal:node:get\u0027 is used for both ``get_all``"},{"line_number":79,"context_line":"  and ``get_one``. Adding a policy rule ``baremetal:node:list`` for the"},{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_88105750","line":80,"in_reply_to":"3fa7e38b_dd636f19","updated":"2019-10-09 14:30:16.000000000","message":"I\u0027m unsure how to do that while still maintaining backwards compatibility. The closest I can think of is exposing \u0027baremetal:node:list\u0027 to everyone, and having \u0027get_all\u0027 do the filtering; is there an alternative you\u0027d suggest?","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"abc75a8f3d081247b99948df34c368ab7a1cefef","unresolved":false,"context_lines":[{"line_number":96,"context_line":"* Update the node controller policy check to pass in the ``owner`` from any"},{"line_number":97,"context_line":"  associated allocation."},{"line_number":98,"context_line":"* Update the policy file as one sees fit; node owners and allocation owners"},{"line_number":99,"context_line":"  can have different API access."},{"line_number":100,"context_line":""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_f3c0e1e2","line":99,"updated":"2019-10-09 09:44:50.000000000","message":"I\u0027d prefer to remove this plan completely, since I don\u0027t think it\u0027s correct, and I don\u0027t want to hold up this spec discussing allocations.\n\nI actually seem to have a plan to handle them though (not necessary as part of this spec):\n\n1. Add `allocation.owner` (as here)\n\n2. Create a new policy baremetal:allocation:set_owner (defaulting to is_admin). If it\u0027s true, an owner can be set explicitly or left None. If false - owner is forcibly set to the caller project ID.\n\n3. Modify allocation processing (in the conductor) to take owner in account, if set (i.e. make sure that `allocation.owner is None or allocation.owner \u003d\u003d node.owner`).\n\nThen to enable allocations for non-admin users it will be enough to allow them baremetal:allocation:create without touching baremetal:allocation:set_owner.\n\nThis seems simple enough for a follow-up RFE.","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"4b1b576c50eb23a9c506c812b0c16cc55eacd63b","unresolved":false,"context_lines":[{"line_number":96,"context_line":"* Update the node controller policy check to pass in the ``owner`` from any"},{"line_number":97,"context_line":"  associated allocation."},{"line_number":98,"context_line":"* Update the policy file as one sees fit; node owners and allocation owners"},{"line_number":99,"context_line":"  can have different API access."},{"line_number":100,"context_line":""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_e82a3d2a","line":99,"updated":"2019-10-08 17:10:50.000000000","message":"I\u0027m 99.95% sure, based on the wording, that this is for the future. I would more explicitly state that if so since it is planting a seed of what could be.","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"88a4e0fa852e80451e5f0232576b41e5f59e1405","unresolved":false,"context_lines":[{"line_number":96,"context_line":"* Update the node controller policy check to pass in the ``owner`` from any"},{"line_number":97,"context_line":"  associated allocation."},{"line_number":98,"context_line":"* Update the policy file as one sees fit; node owners and allocation owners"},{"line_number":99,"context_line":"  can have different API access."},{"line_number":100,"context_line":""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_3a1ba9ca","line":99,"in_reply_to":"3fa7e38b_f3c0e1e2","updated":"2019-10-09 13:50:50.000000000","message":"I\u0027ll take it out.","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"4b1b576c50eb23a9c506c812b0c16cc55eacd63b","unresolved":false,"context_lines":[{"line_number":225,"context_line":""},{"line_number":226,"context_line":"* If the ``owner`` field does not match a project ID (or is None), the"},{"line_number":227,"context_line":"  proposed update to the policy file will not give any non-admin access to"},{"line_number":228,"context_line":"  the Ironic API."},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"* This change has no end-user impact if the policy file is not updated. An"},{"line_number":231,"context_line":"  existing install can simply choose not to update their policy file."}],"source_content_type":"text/x-rst","patch_set":7,"id":"3fa7e38b_68716d0c","line":228,"updated":"2019-10-08 17:10:50.000000000","message":"This changes should we make it a default, then again we could do this change now and telegraph that we will make the change in the next release.","commit_id":"8e9198902bcae9ec12382591865c735c2235b8a4"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9312082ee028befc6b15d7a8705d51f581051060","unresolved":false,"context_lines":[{"line_number":75,"context_line":"This update is enough to control access for most API functions - except for"},{"line_number":76,"context_line":"list functions. For those, we propose the following:"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"* Right now, the policy rule \u0027baremetal:node:get\u0027 is used for both ``get_all``"},{"line_number":79,"context_line":"  and ``get_one``. Adding a policy rule ``baremetal:node:list`` for the"},{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3fa7e38b_1d56e72b","line":80,"range":{"start_line":78,"start_character":0,"end_line":80,"end_character":44},"updated":"2019-10-09 14:28:23.000000000","message":"It\u0027s a good addition, but with what is written below in place it\u0027s not clear why we need it.","commit_id":"963ff4f7cb98eebf3a748c074ac5742b72c5512e"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"2622035a1479533d726912a4578732051cbd14a7","unresolved":false,"context_lines":[{"line_number":75,"context_line":"This update is enough to control access for most API functions - except for"},{"line_number":76,"context_line":"list functions. For those, we propose the following:"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"* Right now, the policy rule \u0027baremetal:node:get\u0027 is used for both ``get_all``"},{"line_number":79,"context_line":"  and ``get_one``. Adding a policy rule ``baremetal:node:list`` for the"},{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3fa7e38b_88713779","line":80,"range":{"start_line":78,"start_character":0,"end_line":80,"end_character":44},"in_reply_to":"3fa7e38b_1d56e72b","updated":"2019-10-09 14:44:24.000000000","message":"Would the alternative be to simply remove the policy check for get_all altogether, and have the get_all function just implicitly do the correct filtering? I\u0027d just want the consistency of having policy checks for all API calls, but maybe that\u0027s not needed... ?","commit_id":"963ff4f7cb98eebf3a748c074ac5742b72c5512e"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"970b40fff56d966d81ab01d761401f3b767428f7","unresolved":false,"context_lines":[{"line_number":75,"context_line":"This update is enough to control access for most API functions - except for"},{"line_number":76,"context_line":"list functions. For those, we propose the following:"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"* Right now, the policy rule \u0027baremetal:node:get\u0027 is used for both ``get_all``"},{"line_number":79,"context_line":"  and ``get_one``. Adding a policy rule ``baremetal:node:list`` for the"},{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3fa7e38b_081a27cb","line":80,"range":{"start_line":78,"start_character":0,"end_line":80,"end_character":44},"in_reply_to":"3fa7e38b_88713779","updated":"2019-10-09 14:58:54.000000000","message":"Hmm, ignore me, I think this bit is fine. We need a way to prevent the access to node listing completely.","commit_id":"963ff4f7cb98eebf3a748c074ac5742b72c5512e"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"9312082ee028befc6b15d7a8705d51f581051060","unresolved":false,"context_lines":[{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"},{"line_number":84,"context_line":"  specified in the request context. These list functions already have the"},{"line_number":85,"context_line":"  option of filtering by ``owner`` [5]_."},{"line_number":86,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"3fa7e38b_fd366b03","line":83,"range":{"start_line":83,"start_character":6,"end_line":83,"end_character":20},"updated":"2019-10-09 14:28:23.000000000","message":"How do you define non-admins if you don\u0027t use policy for that? Or do you use policy for that? Please pardon my ignorance in keystone questions.","commit_id":"963ff4f7cb98eebf3a748c074ac5742b72c5512e"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"4b68b83e87faac7ab2bec874640d70ac38ed2be8","unresolved":false,"context_lines":[{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"},{"line_number":84,"context_line":"  specified in the request context. These list functions already have the"},{"line_number":85,"context_line":"  option of filtering by ``owner`` [5]_."},{"line_number":86,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"3fa7e38b_ae6c3bd0","line":83,"range":{"start_line":83,"start_character":6,"end_line":83,"end_character":20},"in_reply_to":"3fa7e38b_038a0869","updated":"2019-10-09 15:52:23.000000000","message":"The call seems to take ~0.015 seconds; I\u0027m not sure how this matches up with a typical large scale get_all though... ?","commit_id":"963ff4f7cb98eebf3a748c074ac5742b72c5512e"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"d2da08206349da7146a69f2845c9fd8b400fddb1","unresolved":false,"context_lines":[{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"},{"line_number":84,"context_line":"  specified in the request context. These list functions already have the"},{"line_number":85,"context_line":"  option of filtering by ``owner`` [5]_."},{"line_number":86,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"3fa7e38b_8948592d","line":83,"range":{"start_line":83,"start_character":6,"end_line":83,"end_character":20},"in_reply_to":"3fa7e38b_0e346f92","updated":"2019-10-09 16:29:15.000000000","message":"yep. I\u0027ve been thinking of a clearer name for list_all, but failed to come up with one.","commit_id":"963ff4f7cb98eebf3a748c074ac5742b72c5512e"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"db1a0d280f4087b231f1ee2cfd2623b5779bba3e","unresolved":false,"context_lines":[{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"},{"line_number":84,"context_line":"  specified in the request context. These list functions already have the"},{"line_number":85,"context_line":"  option of filtering by ``owner`` [5]_."},{"line_number":86,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"3fa7e38b_a36c54d0","line":83,"range":{"start_line":83,"start_character":6,"end_line":83,"end_character":20},"in_reply_to":"3fa7e38b_281e03bb","updated":"2019-10-09 15:17:26.000000000","message":"The call itself is probably pretty quick - no database or anything - but you have to supply node information for each and every node. It does feel potentially heavy for a large deployment.","commit_id":"963ff4f7cb98eebf3a748c074ac5742b72c5512e"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"1cc4e3278f9dc30a9389202330573b8b96d74ccf","unresolved":false,"context_lines":[{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"},{"line_number":84,"context_line":"  specified in the request context. These list functions already have the"},{"line_number":85,"context_line":"  option of filtering by ``owner`` [5]_."},{"line_number":86,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"3fa7e38b_0e346f92","line":83,"range":{"start_line":83,"start_character":6,"end_line":83,"end_character":20},"in_reply_to":"3fa7e38b_2e512bda","updated":"2019-10-09 15:58:36.000000000","message":"In that case, would it make sense to go with the baremetal:node:list_all and baremetal:node:list plan?","commit_id":"963ff4f7cb98eebf3a748c074ac5742b72c5512e"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"9016fec97ecab040c1fbaa5dda87c92334a55165","unresolved":false,"context_lines":[{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"},{"line_number":84,"context_line":"  specified in the request context. These list functions already have the"},{"line_number":85,"context_line":"  option of filtering by ``owner`` [5]_."},{"line_number":86,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"3fa7e38b_fba190aa","line":83,"range":{"start_line":83,"start_character":6,"end_line":83,"end_character":20},"in_reply_to":"3fa7e38b_8948592d","updated":"2019-10-09 17:29:22.000000000","message":"The policy match and checking request context after the db query is kind of why I\u0027d rather just perform the select from the DB upfront with the project_id, but we\u0027re really just in fine details at this point. I would only be worried about 10,000+ machine deployments and a performance impact that is across all nodes.","commit_id":"963ff4f7cb98eebf3a748c074ac5742b72c5512e"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"5ea1e0ae973b378bf5c57146c0d43e373328af10","unresolved":false,"context_lines":[{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"},{"line_number":84,"context_line":"  specified in the request context. These list functions already have the"},{"line_number":85,"context_line":"  option of filtering by ``owner`` [5]_."},{"line_number":86,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"3fa7e38b_038a0869","line":83,"range":{"start_line":83,"start_character":6,"end_line":83,"end_character":20},"in_reply_to":"3fa7e38b_a36c54d0","updated":"2019-10-09 15:20:27.000000000","message":"Maybe worth benchmarking it? I\u0027m not sure it matters compared to other things we do with nodes (and somebody with 3000 nodes probably shouldn\u0027t list them without a limit).\n\nI\u0027m not strictly against list_all, just considering alternatives.","commit_id":"963ff4f7cb98eebf3a748c074ac5742b72c5512e"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"684285512f127562575211f3730ae2bbcb4929d3","unresolved":false,"context_lines":[{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"},{"line_number":84,"context_line":"  specified in the request context. These list functions already have the"},{"line_number":85,"context_line":"  option of filtering by ``owner`` [5]_."},{"line_number":86,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"3fa7e38b_2e512bda","line":83,"range":{"start_line":83,"start_character":6,"end_line":83,"end_character":20},"in_reply_to":"3fa7e38b_ae6c3bd0","updated":"2019-10-09 15:57:15.000000000","message":"hmm, it\u0027s quite a lot. I wonder how much less will take a naive `node.owner \u003d\u003d context.project_id` (or how it\u0027s done)","commit_id":"963ff4f7cb98eebf3a748c074ac5742b72c5512e"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"970b40fff56d966d81ab01d761401f3b767428f7","unresolved":false,"context_lines":[{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"},{"line_number":84,"context_line":"  specified in the request context. These list functions already have the"},{"line_number":85,"context_line":"  option of filtering by ``owner`` [5]_."},{"line_number":86,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"3fa7e38b_281e03bb","line":83,"range":{"start_line":83,"start_character":6,"end_line":83,"end_character":20},"in_reply_to":"3fa7e38b_e8bc6bdd","updated":"2019-10-09 14:58:54.000000000","message":"This does seem similar to what I suggested for allocations. However, I do wonder if it\u0027s simpler to just check baremetal:node:get in a loop. Is it a heavy call?","commit_id":"963ff4f7cb98eebf3a748c074ac5742b72c5512e"},{"author":{"_account_id":7386,"name":"Tzu-Mainn Chen","email":"tzumainn@redhat.com","username":"tzumainn"},"change_message_id":"2622035a1479533d726912a4578732051cbd14a7","unresolved":false,"context_lines":[{"line_number":80,"context_line":"  former allows us to differentiate the two."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"* Updating the REST API node list functions to check the request context;"},{"line_number":83,"context_line":"  and for non-admins, to filter by the ``owner`` against the project"},{"line_number":84,"context_line":"  specified in the request context. These list functions already have the"},{"line_number":85,"context_line":"  option of filtering by ``owner`` [5]_."},{"line_number":86,"context_line":""}],"source_content_type":"text/x-rst","patch_set":8,"id":"3fa7e38b_e8bc6bdd","line":83,"range":{"start_line":83,"start_character":6,"end_line":83,"end_character":20},"in_reply_to":"3fa7e38b_fd366b03","updated":"2019-10-09 14:44:24.000000000","message":"That\u0027s a good point; I missed that.\n\nI think we need two additional rules instead of one: baremetal:node:list_all, which returns all nodes; and baremetal:node:list, which matches according to the owner field. Would that make sense to you?","commit_id":"963ff4f7cb98eebf3a748c074ac5742b72c5512e"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"47bbdd1b0133cfe594bb3a7cef11c6636585862f","unresolved":false,"context_lines":[{"line_number":54,"context_line":"* Assume that a node\u0027s ``owner`` field is set to the id of a Keystone project."},{"line_number":55,"context_line":"  We refer to this project as the \"node owner\"."},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"* Update the node controller so that policy checks also pass in information"},{"line_number":58,"context_line":"  about the node, including the ``owner`` field."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"* Update Ironic\u0027s default generated policy file to include an"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3fa7e38b_2ea7c4be","line":57,"range":{"start_line":57,"start_character":61,"end_line":57,"end_character":63},"updated":"2019-10-11 14:13:15.000000000","message":"nit: remove \u0027in\u0027","commit_id":"62dd67cc9ef0e3d16e78d5f55e2e32a5560a6914"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"fbe3f0fa955d46173ff26906a3176d391a60f3ec","unresolved":false,"context_lines":[{"line_number":78,"context_line":"* Right now, the policy rule \u0027baremetal:node:get\u0027 is used for both ``get_all``"},{"line_number":79,"context_line":"  and ``get_one``. We can add two new policy rules: ``baremetal:node:list_all``"},{"line_number":80,"context_line":"  for retrieving all nodes, and ``baremetal:node:list`` for retrieving nodes"},{"line_number":81,"context_line":"  whose ``owner`` field matches the querying project."},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"* Updating the REST API node list functions to first perform a policy check"},{"line_number":84,"context_line":"  against ``baremetal:node:list_all``. If it passes, then we return all nodes;"}],"source_content_type":"text/x-rst","patch_set":9,"id":"3fa7e38b_e973e67e","line":81,"updated":"2019-10-11 14:45:26.000000000","message":"We\u0027ll have to repeat all this for port, port groups and volume resources (anything that depends on a node, except for allocations).","commit_id":"62dd67cc9ef0e3d16e78d5f55e2e32a5560a6914"}]}