From fb04e6ea97b1f457cb0b44f4086f2ba4f67c23af Mon Sep 17 00:00:00 2001 From: Gregory Thiemonge Date: Fri, 3 Feb 2023 13:53:10 +0100 Subject: [PATCH] Fix legacy admin in RBAC tests The legacy admin continues to work as it is with the sRBAC [0] if os_system_admin is allowed, os_admin should be allowed. [0] https://governance.openstack.org/tc/goals/selected/\ consistent-and-secure-rbac.html\#legacy-admin-continues-to-work-as-it-is Depends-On: https://review.opendev.org/c/openstack/octavia/+/875620 Change-Id: I10d497a4b4e3a3b21cb24dba73d5074a71a3d381 --- octavia_tempest_plugin/tests/RBAC_tests.py | 68 ++++++++++------------ 1 file changed, 32 insertions(+), 36 deletions(-) diff --git a/octavia_tempest_plugin/tests/RBAC_tests.py b/octavia_tempest_plugin/tests/RBAC_tests.py index d31d5060..8dae024f 100644 --- a/octavia_tempest_plugin/tests/RBAC_tests.py +++ b/octavia_tempest_plugin/tests/RBAC_tests.py @@ -155,15 +155,14 @@ class RBACTestsMixin(test.BaseTestCase): """ allowed_list = copy.deepcopy(expected_allowed) - # os_admin is a special case as it is valid with the old defaults, - # but will not be with the new defaults and/or token scoping. - # The old keystone role "admin" becomes project scoped "admin" - # instead of being a global admin. - # To keep the tests simple, handle that edge case here. - # TODO(johnsom) Once token scope is default, remove this. - if ('os_system_admin' in expected_allowed and - not CONF.load_balancer.enforce_new_defaults and - not CONF.enforce_scope.octavia): + # The legacy admin behavior changed during the sRBAC development, + # os_admin is still a valid admin [0] + # [0] https://governance.openstack.org/tc/goals/selected/ + # consistent-and-secure-rbac.html + # #legacy-admin-continues-to-work-as-it-is + # TODO(gthiemonge) we may have to revisit it in the future if the + # legacy admin scope changes. + if 'os_system_admin' in expected_allowed: allowed_list.append('os_admin') # #### Test that disallowed credentials cannot access the API. @@ -244,15 +243,14 @@ class RBACTestsMixin(test.BaseTestCase): """ allowed_list = copy.deepcopy(expected_allowed) - # os_admin is a special case as it is valid with the old defaults, - # but will not be with the new defaults and/or token scoping. - # The old keystone role "admin" becomes project scoped "admin" - # instead of being a global admin. - # To keep the tests simple, handle that edge case here. - # TODO(johnsom) Once token scope is default, remove this. - if ('os_system_admin' in expected_allowed and - not CONF.load_balancer.enforce_new_defaults and - not CONF.enforce_scope.octavia): + # The legacy admin behavior changed during the sRBAC development, + # os_admin is still a valid admin [0] + # [0] https://governance.openstack.org/tc/goals/selected/ + # consistent-and-secure-rbac.html + # #legacy-admin-continues-to-work-as-it-is + # TODO(gthiemonge) we may have to revisit it in the future if the + # legacy admin scope changes. + if 'os_system_admin' in expected_allowed: allowed_list.append('os_admin') # #### Test that disallowed credentials cannot access the API. @@ -371,15 +369,14 @@ class RBACTestsMixin(test.BaseTestCase): """ allowed_list = copy.deepcopy(expected_allowed) - # os_admin is a special case as it is valid with the old defaults, - # but will not be with the new defaults and/or token scoping. - # The old keystone role "admin" becomes project scoped "admin" - # instead of being a global admin. - # To keep the tests simple, handle that edge case here. - # TODO(johnsom) Once token scope is default, remove this. - if ('os_system_admin' in expected_allowed and - not CONF.load_balancer.enforce_new_defaults and - not CONF.enforce_scope.octavia): + # The legacy admin behavior changed during the sRBAC development, + # os_admin is still a valid admin [0] + # [0] https://governance.openstack.org/tc/goals/selected/ + # consistent-and-secure-rbac.html + # #legacy-admin-continues-to-work-as-it-is + # TODO(gthiemonge) we may have to revisit it in the future if the + # legacy admin scope changes. + if 'os_system_admin' in expected_allowed: allowed_list.append('os_admin') for cred in allowed_list: @@ -439,15 +436,14 @@ class RBACTestsMixin(test.BaseTestCase): """ allowed_list = copy.deepcopy(expected_allowed) - # os_admin is a special case as it is valid with the old defaults, - # but will not be with the new defaults and/or token scoping. - # The old keystone role "admin" becomes project scoped "admin" - # instead of being a global admin. - # To keep the tests simple, handle that edge case here. - # TODO(johnsom) Once token scope is default, remove this. - if ('os_system_admin' in expected_allowed and - not CONF.load_balancer.enforce_new_defaults and - not CONF.enforce_scope.octavia): + # The legacy admin behavior changed during the sRBAC development, + # os_admin is still a valid admin [0] + # [0] https://governance.openstack.org/tc/goals/selected/ + # consistent-and-secure-rbac.html + # #legacy-admin-continues-to-work-as-it-is + # TODO(gthiemonge) we may have to revisit it in the future if the + # legacy admin scope changes. + if 'os_system_admin' in expected_allowed: allowed_list.append('os_admin') for cred in allowed_list: