From 240347fc3e066a998b0ad7c472019d7ca6c9b4da Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Fri, 2 Aug 2024 11:10:55 -0700 Subject: [PATCH] Fix tests for oslo.policy new defaults enable by default oslo.policy has enabled the new RBAC config options enforce_scope and enforce_new_defaults by default[1][2]. octavia is ready with the new RBAC but we need to fix the test and some system scope cleanup in base rules. Needed-By: https://review.opendev.org/c/openstack/requirements/+/925464 [1] https://review.opendev.org/c/openstack/oslo.policy/+/924283 [2] https://review.opendev.org/c/openstack/releases/+/925032 Change-Id: Ifbec670e1afa86725d2659a185f6c185abbefb16 --- octavia/common/policy.py | 5 ++--- octavia/policies/base.py | 21 +------------------ .../api/v2/test_availability_zones.py | 2 +- .../tests/functional/api/v2/test_flavors.py | 4 ++-- .../functional/api/v2/test_health_monitor.py | 10 ++++----- .../tests/functional/api/v2/test_l7policy.py | 10 ++++----- .../tests/functional/api/v2/test_l7rule.py | 10 ++++----- .../tests/functional/api/v2/test_listener.py | 12 +++++------ .../functional/api/v2/test_load_balancer.py | 14 ++++++------- .../tests/functional/api/v2/test_member.py | 10 ++++----- octavia/tests/functional/api/v2/test_pool.py | 10 ++++----- .../tests/functional/api/v2/test_quotas.py | 18 ++++++++-------- octavia/tests/unit/common/test_policy.py | 9 +++----- 13 files changed, 56 insertions(+), 79 deletions(-) diff --git a/octavia/common/policy.py b/octavia/common/policy.py index 8cf22c37c2..e7e13f6027 100644 --- a/octavia/common/policy.py +++ b/octavia/common/policy.py @@ -137,9 +137,8 @@ class Policy(oslo_policy.Enforcer): try: result = self.enforce('context_is_admin', credentials, credentials) except oslo_policy.InvalidScope as e: - # This will happen if the token being used is not system scoped - # which is required for the admin roles when scope checking is - # enabled. + # This will happen if the token being used is system scoped + # when scope checking is enabled. LOG.warning(str(e)) return False return result diff --git a/octavia/policies/base.py b/octavia/policies/base.py index efaba9080f..740bf83a1c 100644 --- a/octavia/policies/base.py +++ b/octavia/policies/base.py @@ -41,20 +41,6 @@ rules = [ # OpenStack wide scoped rules - # System scoped Administrator - policy.RuleDefault( - name='system-admin', - check_str='role:admin and ' - 'system_scope:all', - scope_types=[constants.RBAC_SCOPE_PROJECT]), - - # System scoped Reader - policy.RuleDefault( - name='system-reader', - check_str='role:reader and ' - 'system_scope:all', - scope_types=[constants.RBAC_SCOPE_PROJECT]), - # Project scoped Member policy.RuleDefault( name='project-member', @@ -85,13 +71,10 @@ rules = [ # role:load-balancer_admin # User is considered an admin for all load-balancer APIs including # resources owned by others. - # role:admin and system_scope:all - # User is admin to all service APIs, including Octavia. policy.RuleDefault( name='context_is_admin', check_str='role:load-balancer_admin or ' - 'rule:system-admin or ' 'role:admin', deprecated_rule=deprecated_context_is_admin, scope_types=[constants.RBAC_SCOPE_PROJECT]), @@ -115,8 +98,7 @@ rules = [ policy.RuleDefault( name='load-balancer:global_observer', - check_str='role:load-balancer_global_observer or ' - 'rule:system-reader', + check_str='role:load-balancer_global_observer', scope_types=[constants.RBAC_SCOPE_PROJECT]), policy.RuleDefault( @@ -132,7 +114,6 @@ rules = [ name='load-balancer:admin', check_str='is_admin:True or ' 'role:load-balancer_admin or ' - 'rule:system-admin or ' 'role:admin', scope_types=[constants.RBAC_SCOPE_PROJECT]), diff --git a/octavia/tests/functional/api/v2/test_availability_zones.py b/octavia/tests/functional/api/v2/test_availability_zones.py index bfe057d1a3..854bb7c9af 100644 --- a/octavia/tests/functional/api/v2/test_availability_zones.py +++ b/octavia/tests/functional/api/v2/test_availability_zones.py @@ -214,7 +214,7 @@ class TestAvailabilityZones(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, diff --git a/octavia/tests/functional/api/v2/test_flavors.py b/octavia/tests/functional/api/v2/test_flavors.py index d2d726d8a1..a73cf7c54f 100644 --- a/octavia/tests/functional/api/v2/test_flavors.py +++ b/octavia/tests/functional/api/v2/test_flavors.py @@ -214,7 +214,7 @@ class TestFlavors(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -308,7 +308,7 @@ class TestFlavors(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, diff --git a/octavia/tests/functional/api/v2/test_health_monitor.py b/octavia/tests/functional/api/v2/test_health_monitor.py index 2c31140662..5ad43fb125 100644 --- a/octavia/tests/functional/api/v2/test_health_monitor.py +++ b/octavia/tests/functional/api/v2/test_health_monitor.py @@ -137,7 +137,7 @@ class TestHealthMonitor(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -293,7 +293,7 @@ class TestHealthMonitor(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -1258,7 +1258,7 @@ class TestHealthMonitor(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -1714,7 +1714,7 @@ class TestHealthMonitor(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -2064,7 +2064,7 @@ class TestHealthMonitor(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, diff --git a/octavia/tests/functional/api/v2/test_l7policy.py b/octavia/tests/functional/api/v2/test_l7policy.py index de9c003a7b..eb314f8920 100644 --- a/octavia/tests/functional/api/v2/test_l7policy.py +++ b/octavia/tests/functional/api/v2/test_l7policy.py @@ -76,7 +76,7 @@ class TestL7Policy(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -209,7 +209,7 @@ class TestL7Policy(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -685,7 +685,7 @@ class TestL7Policy(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -919,7 +919,7 @@ class TestL7Policy(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -1165,7 +1165,7 @@ class TestL7Policy(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, diff --git a/octavia/tests/functional/api/v2/test_l7rule.py b/octavia/tests/functional/api/v2/test_l7rule.py index ce1ca695cf..0c0237e3f6 100644 --- a/octavia/tests/functional/api/v2/test_l7rule.py +++ b/octavia/tests/functional/api/v2/test_l7rule.py @@ -76,7 +76,7 @@ class TestL7Rule(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -175,7 +175,7 @@ class TestL7Rule(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -542,7 +542,7 @@ class TestL7Rule(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -921,7 +921,7 @@ class TestL7Rule(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -1125,7 +1125,7 @@ class TestL7Rule(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, diff --git a/octavia/tests/functional/api/v2/test_listener.py b/octavia/tests/functional/api/v2/test_listener.py index 18788132ee..8d08d2cf70 100644 --- a/octavia/tests/functional/api/v2/test_listener.py +++ b/octavia/tests/functional/api/v2/test_listener.py @@ -112,7 +112,7 @@ class TestListener(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -558,7 +558,7 @@ class TestListener(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -976,7 +976,7 @@ class TestListener(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -2106,7 +2106,7 @@ class TestListener(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -2267,7 +2267,7 @@ class TestListener(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -2926,7 +2926,7 @@ class TestListener(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, diff --git a/octavia/tests/functional/api/v2/test_load_balancer.py b/octavia/tests/functional/api/v2/test_load_balancer.py index 9c0875b13b..960010a254 100644 --- a/octavia/tests/functional/api/v2/test_load_balancer.py +++ b/octavia/tests/functional/api/v2/test_load_balancer.py @@ -995,7 +995,7 @@ class TestLoadBalancer(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -1306,7 +1306,7 @@ class TestLoadBalancer(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -1892,7 +1892,7 @@ class TestLoadBalancer(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -2092,7 +2092,7 @@ class TestLoadBalancer(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -2276,7 +2276,7 @@ class TestLoadBalancer(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -4008,7 +4008,7 @@ class TestLoadBalancerGraph(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -4111,7 +4111,7 @@ class TestLoadBalancerGraph(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, diff --git a/octavia/tests/functional/api/v2/test_member.py b/octavia/tests/functional/api/v2/test_member.py index 14e507c165..38497338ec 100644 --- a/octavia/tests/functional/api/v2/test_member.py +++ b/octavia/tests/functional/api/v2/test_member.py @@ -89,7 +89,7 @@ class TestMember(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -194,7 +194,7 @@ class TestMember(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -529,7 +529,7 @@ class TestMember(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -1178,7 +1178,7 @@ class TestMember(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -1360,7 +1360,7 @@ class TestMember(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, diff --git a/octavia/tests/functional/api/v2/test_pool.py b/octavia/tests/functional/api/v2/test_pool.py index c86f1438ef..148bdb6c6e 100644 --- a/octavia/tests/functional/api/v2/test_pool.py +++ b/octavia/tests/functional/api/v2/test_pool.py @@ -105,7 +105,7 @@ class TestPool(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -247,7 +247,7 @@ class TestPool(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -742,7 +742,7 @@ class TestPool(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -1274,7 +1274,7 @@ class TestPool(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -2102,7 +2102,7 @@ class TestPool(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, diff --git a/octavia/tests/functional/api/v2/test_quotas.py b/octavia/tests/functional/api/v2/test_quotas.py index 7e7415ca68..7a85bc594d 100644 --- a/octavia/tests/functional/api/v2/test_quotas.py +++ b/octavia/tests/functional/api/v2/test_quotas.py @@ -394,7 +394,7 @@ class TestQuotas(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -440,7 +440,7 @@ class TestQuotas(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_observer'], + 'roles': ['load-balancer_observer', 'reader'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -480,18 +480,18 @@ class TestQuotas(base.BaseAPITest): self._assert_quotas_equal(quotas, quota2) def test_get_Authorized_member(self): - self._test_get_Authorized('load-balancer_member') + self._test_get_Authorized(['load-balancer_member', 'member']) def test_get_Authorized_observer(self): - self._test_get_Authorized('load-balancer_observer') + self._test_get_Authorized(['load-balancer_observer', 'reader']) def test_get_Authorized_global_observer(self): - self._test_get_Authorized('load-balancer_global_observer') + self._test_get_Authorized(['load-balancer_global_observer']) def test_get_Authorized_quota_admin(self): - self._test_get_Authorized('load-balancer_quota_admin') + self._test_get_Authorized(['load-balancer_quota_admin']) - def _test_get_Authorized(self, role): + def _test_get_Authorized(self, roles): project1_id = uuidutils.generate_uuid() quota1 = self.create_quota( project_id=project1_id, lb_quota=1, member_quota=1 @@ -509,7 +509,7 @@ class TestQuotas(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': [role], + 'roles': roles, 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, @@ -704,7 +704,7 @@ class TestQuotas(base.BaseAPITest): 'is_admin_project': True, 'service_project_domain_id': None, 'service_project_id': None, - 'roles': ['load-balancer_member'], + 'roles': ['load-balancer_member', 'member'], 'user_id': None, 'is_admin': False, 'service_user_domain_id': None, diff --git a/octavia/tests/unit/common/test_policy.py b/octavia/tests/unit/common/test_policy.py index edad63459b..cfb92d4a33 100644 --- a/octavia/tests/unit/common/test_policy.py +++ b/octavia/tests/unit/common/test_policy.py @@ -164,23 +164,20 @@ class PolicyTestCase(base.TestCase): def test_check_is_admin_fail(self): self.assertFalse(policy.get_enforcer().check_is_admin(self.context)) - # TODO(johnsom) When oslo.policy changes "enforce_new_defaults" to True - # this test will fail as "system_scope:all" will be required. - # This test and the conditional in common/policy.py can then - # be removed in favor of test_check_is_admin_new_defaults(). def test_check_is_admin(self): self.context = context.RequestContext('admin', project_id='fake', roles=['AdMiN']) self.assertTrue(policy.get_enforcer().check_is_admin(self.context)) - def test_check_is_admin_new_defaults(self): + def test_check_is_admin_with_system_scope_token(self): conf = oslo_fixture.Config(config.cfg.CONF) conf.config(group="oslo_policy", enforce_new_defaults=True) + conf.config(group="oslo_policy", enforce_scope=True) self.context = context.RequestContext('admin', roles=['AdMiN'], system_scope='all') - self.assertTrue(policy.get_enforcer().check_is_admin(self.context)) + self.assertFalse(policy.get_enforcer().check_is_admin(self.context)) def test_get_enforcer(self): self.assertTrue(isinstance(policy.get_no_context_enforcer(),