Re: [openstack-dev] [nova] Policy rules are killed by the context admin check
Le 23 avr. 2015 04:49, Alex Xu sou...@gmail.com a écrit : 2015-04-23 6:55 GMT+08:00 Matt Riedemann mrie...@linux.vnet.ibm.com: On 4/22/2015 8:32 AM, Sylvain Bauza wrote: Hi, By discussing on a specific bug [1], I just discovered that the admin context check which was done at the DB level has been moved to the API level thanks to the api-policy-v3 blueprint [2] That behaviour still leads to a bug if the operator wants to change an endpoint policy by leaving it end-user but still continues to be denied because of that, as it will forbid any non-admin user to call the methods (even if authorize() grants the request) I consequently opened a bug [3] for this but I'm also concerned about the backportability of that and why it shouldn't fixed in v2.0 too. Releasing the check by removing it sounds an acceptable change, as it fixes a bug without changing the expected behaviour [4]. The impact of the change sounds also minimal with a very precise scope (ie. leave the policy rules work as they are expected) [5] Folks, thoughts ? -Sylvain [1] https://bugs.launchpad.net/nova/+bug/1447084 [2] https://review.openstack.org/#/q/project:openstack/nova+branch:master+topic:bp/v3-api-policy,n,z [3] https://bugs.launchpad.net/nova/+bug/1447164 [4] https://wiki.openstack.org/wiki/APIChangeGuidelines#Generally_Considered_OK Fixing a bug so that a request which resulted in an error response before is now successful [5] https://wiki.openstack.org/wiki/StableBranch#Stable_branch_policy __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev I don't disagree, see bug 1168488 from way back in grizzly. The only thing would be we'd have to make sure the default rule is admin for any v2 extensions which are enforcing an admin context today. Agree, if we want to fix those for v2, we need make sure the default rule is admin. And do you mean [3] want to fix this for v2 both in Kilo and Liberty? For liberty, we can do that, but I think we will switch to v2.1 very soon. Not sure it is still worth to do that. For kilo, some of api is pretty easy to fix by just removing 'require_admin_context()'. But there still have many of policy patches didn't merged into the master yet. like: https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/nova-api-policy-final-part,n,z https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/v3-api-policy,n,z https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:remove_qutoa_hardcode_permission,n,z https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:remove_quotaclass_hardcode_permission,n,z Should we back-port them all? Wrt all the necessary backports, I'm eventually rather in favor of an opportunistic approach and only backport what has been reported as a bug, ie. [1] That has also the benefit of not proposing a stable patch which was not cherry-picked (like providing an elevated context), which I disapprove. -Sylvain -- Thanks, Matt Riedemann __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] Policy rules are killed by the context admin check
On 4/22/2015 8:32 AM, Sylvain Bauza wrote: Hi, By discussing on a specific bug [1], I just discovered that the admin context check which was done at the DB level has been moved to the API level thanks to the api-policy-v3 blueprint [2] That behaviour still leads to a bug if the operator wants to change an endpoint policy by leaving it end-user but still continues to be denied because of that, as it will forbid any non-admin user to call the methods (even if authorize() grants the request) I consequently opened a bug [3] for this but I'm also concerned about the backportability of that and why it shouldn't fixed in v2.0 too. Releasing the check by removing it sounds an acceptable change, as it fixes a bug without changing the expected behaviour [4]. The impact of the change sounds also minimal with a very precise scope (ie. leave the policy rules work as they are expected) [5] Folks, thoughts ? -Sylvain [1] https://bugs.launchpad.net/nova/+bug/1447084 [2] https://review.openstack.org/#/q/project:openstack/nova+branch:master+topic:bp/v3-api-policy,n,z [3] https://bugs.launchpad.net/nova/+bug/1447164 [4] https://wiki.openstack.org/wiki/APIChangeGuidelines#Generally_Considered_OK Fixing a bug so that a request which resulted in an error response before is now successful [5] https://wiki.openstack.org/wiki/StableBranch#Stable_branch_policy __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev I don't disagree, see bug 1168488 from way back in grizzly. The only thing would be we'd have to make sure the default rule is admin for any v2 extensions which are enforcing an admin context today. -- Thanks, Matt Riedemann __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] Policy rules are killed by the context admin check
2015-04-23 6:55 GMT+08:00 Matt Riedemann mrie...@linux.vnet.ibm.com: On 4/22/2015 8:32 AM, Sylvain Bauza wrote: Hi, By discussing on a specific bug [1], I just discovered that the admin context check which was done at the DB level has been moved to the API level thanks to the api-policy-v3 blueprint [2] That behaviour still leads to a bug if the operator wants to change an endpoint policy by leaving it end-user but still continues to be denied because of that, as it will forbid any non-admin user to call the methods (even if authorize() grants the request) I consequently opened a bug [3] for this but I'm also concerned about the backportability of that and why it shouldn't fixed in v2.0 too. Releasing the check by removing it sounds an acceptable change, as it fixes a bug without changing the expected behaviour [4]. The impact of the change sounds also minimal with a very precise scope (ie. leave the policy rules work as they are expected) [5] Folks, thoughts ? -Sylvain [1] https://bugs.launchpad.net/nova/+bug/1447084 [2] https://review.openstack.org/#/q/project:openstack/nova+branch:master+topic:bp/v3-api-policy,n,z [3] https://bugs.launchpad.net/nova/+bug/1447164 [4] https://wiki.openstack.org/wiki/APIChangeGuidelines#Generally_Considered_OK Fixing a bug so that a request which resulted in an error response before is now successful [5] https://wiki.openstack.org/wiki/StableBranch#Stable_branch_policy __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev I don't disagree, see bug 1168488 from way back in grizzly. The only thing would be we'd have to make sure the default rule is admin for any v2 extensions which are enforcing an admin context today. Agree, if we want to fix those for v2, we need make sure the default rule is admin. And do you mean [3] want to fix this for v2 both in Kilo and Liberty? For liberty, we can do that, but I think we will switch to v2.1 very soon. Not sure it is still worth to do that. For kilo, some of api is pretty easy to fix by just removing 'require_admin_context()'. But there still have many of policy patches didn't merged into the master yet. like: https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/nova-api-policy-final-part,n,z https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/v3-api-policy,n,z https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:remove_qutoa_hardcode_permission,n,z https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:remove_quotaclass_hardcode_permission,n,z Should we back-port them all? -- Thanks, Matt Riedemann __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] Policy rules are killed by the context admin check
On Wednesday, April 22, 2015, Matt Riedemann mrie...@linux.vnet.ibm.com wrote: On 4/22/2015 8:32 AM, Sylvain Bauza wrote: Hi, By discussing on a specific bug [1], I just discovered that the admin context check which was done at the DB level has been moved to the API level thanks to the api-policy-v3 blueprint [2] That behaviour still leads to a bug if the operator wants to change an endpoint policy by leaving it end-user but still continues to be denied because of that, as it will forbid any non-admin user to call the methods (even if authorize() grants the request) I consequently opened a bug [3] for this but I'm also concerned about the backportability of that and why it shouldn't fixed in v2.0 too. Releasing the check by removing it sounds an acceptable change, as it fixes a bug without changing the expected behaviour [4]. The impact of the change sounds also minimal with a very precise scope (ie. leave the policy rules work as they are expected) [5] Folks, thoughts ? -Sylvain [1] https://bugs.launchpad.net/nova/+bug/1447084 [2] https://review.openstack.org/#/q/project:openstack/nova+branch:master+topic:bp/v3-api-policy,n,z [3] https://bugs.launchpad.net/nova/+bug/1447164 [4] https://wiki.openstack.org/wiki/APIChangeGuidelines#Generally_Considered_OK Fixing a bug so that a request which resulted in an error response before is now successful [5] https://wiki.openstack.org/wiki/StableBranch#Stable_branch_policy __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev I don't disagree, see bug 1168488 from way back in grizzly. The only thing would be we'd have to make sure the default rule is admin for any v2 extensions which are enforcing an admin context today. This sounds like a sane approach. --Morgan -- Thanks, Matt Riedemann __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
