From ef79698632384ea4426af9fd447827f6b6bfd12f Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Tue, 13 Aug 2024 10:45:26 -0700 Subject: [PATCH] Keep new RBAC disable by default & Gate fix for GLOBAL_VENV This commit squash the multiple changes: Changes 1: Keep new RBAC disable by default oslo.policy has enabled the new RBAC config options enforce_scope and enforce_new_defaults by default[1][2]. Tacker implemented the new RBAC in previous cycle with new RBAC disable by default. To give more time to operator, let's continue the same setting in this release also. Also, there are many test modification is needed for the new RBAC (using the new RBAC default role in tests) Unit tests log a lot of policy warnings, suppressing those[3] -https://fa3204066787dd37fd86-ea893277118f144d3b928cfbb4823c04.ssl.cf1.rackcdn.com/926089/1/check/openstack-tox-py311/9df6631/testr_results.html As oslo.policy enable them by default, we override the setting for the Tacker. NOTE: there is no change in behaviour, tacker continue with the old RBAC as default. ref: 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 2: Co-Authored-By: Ayumu Ueha This fixes the issues when "Set GLOBAL_VENV=true". 1: When running python in the subprocess on venv, the library import may fail to execute properly. This patch changes the command passed to subprocess from "python" to "sys.executable" [4]. 2: Remove compute node from nodeset for kubernetes jobs because it is not used in kubernetes jobs and to save the resources. Also left "GLOBAL_VENV=false" on the jobs related kubernetes to avoid the issue of setup on kuryr-kubernetes. [4] https://docs.python.org/3/library/sys.html#sys.executable [3] ----------- File "/home/zuul/src/opendev.org/openstack/tacker/tacker/policy.py", line 209, in authorize result = _ENFORCER.authorize(action, target, credentials, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/zuul/src/opendev.org/openstack/tacker/.tox/py311/lib/python3.11/site-packages/oslo_policy/policy.py", line 1189, in authorize return self.enforce( ^^^^^^^^^^^^^ File "/home/zuul/src/opendev.org/openstack/tacker/.tox/py311/lib/python3.11/site-packages/oslo_policy/policy.py", line 1093, in enforce raise exc(*args, **kwargs) tacker.common.exceptions.PolicyNotAuthorized: Policy doesn't allow os_nfv_orchestration_api:vnf_instances:cancel to be performed. ----------- Change-Id: I4f736660e3b1079883a7434847222f6a6589377f --- .zuul.yaml | 20 +++------------ requirements.txt | 2 +- tacker/policy.py | 25 +++++++++++++++---- .../conductor/vnflcm_driver_v2.py | 3 ++- .../infra_drivers/openstack/openstack.py | 5 ++-- tacker/tests/base.py | 2 +- tacker/tests/unit/api/v2/test_api_v2.py | 2 +- tacker/tests/unit/policies/base.py | 2 +- tacker/tests/unit/test_policy.py | 12 ++++----- 9 files changed, 39 insertions(+), 34 deletions(-) diff --git a/.zuul.yaml b/.zuul.yaml index 3e84de67a..a786a6a33 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -33,7 +33,7 @@ - compute2 - nodeset: - name: openstack-k8s-4-nodes-jammy + name: openstack-k8s-3-nodes-jammy nodes: - name: controller label: ubuntu-jammy @@ -41,18 +41,11 @@ label: ubuntu-jammy - name: controller-k8s label: ubuntu-jammy - - name: compute1 - label: ubuntu-jammy groups: - - name: compute - nodes: - - controller-k8s - - compute1 - name: subnode nodes: - controller-tacker - controller-k8s - - compute1 - name: switch nodes: - controller @@ -60,7 +53,6 @@ nodes: - controller-tacker - controller-k8s - - compute1 - job: name: tacker-functional-devstack-multinode-sol-parent @@ -105,8 +97,6 @@ OVN_L3_CREATE_PUBLIC_NETWORK: true OVN_DBS_LOG_LEVEL: dbg Q_ML2_PLUGIN_MECHANISM_DRIVERS: ovn,logger - # TODO(ueha): Remove this workarround if the Zuul jobs succeed with GROBAL_VENV=true - GLOBAL_VENV: false test_matrix_configs: [neutron] zuul_work_dir: src/opendev.org/openstack/tacker host-vars: @@ -218,8 +208,6 @@ L2_AGENT_EXTENSIONS: qos ENABLE_CHASSIS_AS_GW: false Q_ML2_PLUGIN_MECHANISM_DRIVERS: ovn,logger - # TODO(ueha): Remove this workarround if the Zuul jobs succeed with GROBAL_VENV=true - GLOBAL_VENV: false devstack_services: # To override the parent job's definitions. tls-proxy: false @@ -404,7 +392,7 @@ description: | Abstraction multinodes job for SOL devstack-based kubernetes functional tests - nodeset: openstack-k8s-4-nodes-jammy + nodeset: openstack-k8s-3-nodes-jammy pre-run: playbooks/devstack/pre.yaml run: playbooks/devstack/run.yaml post-run: playbooks/devstack/post.yaml @@ -543,7 +531,7 @@ USE_PYTHON3: true ENABLE_CHASSIS_AS_GW: false Q_ML2_PLUGIN_MECHANISM_DRIVERS: ovn,logger - # TODO(ueha): Remove this workarround if the Zuul jobs succeed with GROBAL_VENV=true + # TODO(ueha): Remove this workarround if the Zuul jobs succeed with GLOBAL_VENV=true GLOBAL_VENV: false devstack_services: dstat: false @@ -583,7 +571,7 @@ OVN_L3_CREATE_PUBLIC_NETWORK: true OVN_DBS_LOG_LEVEL: dbg Q_ML2_PLUGIN_MECHANISM_DRIVERS: ovn,logger - # TODO(ueha): Remove this workarround if the Zuul jobs succeed with GROBAL_VENV=true + # TODO(ueha): Remove this workarround if the Zuul jobs succeed with GLOBAL_VENV=true GLOBAL_VENV: false devstack_local_conf: post-config: diff --git a/requirements.txt b/requirements.txt index 182cef85b..4c6b50945 100644 --- a/requirements.txt +++ b/requirements.txt @@ -27,7 +27,7 @@ oslo.db>=5.0.0 # Apache-2.0 oslo.log>=3.36.0 # Apache-2.0 oslo.messaging>=14.2.0 # Apache-2.0 oslo.middleware>=3.31.0 # Apache-2.0 -oslo.policy>=3.6.0 # Apache-2.0 +oslo.policy>=3.11.0 # Apache-2.0 oslo.privsep>=2.4.0 # Apache-2.0 oslo.reports>=1.18.0 # Apache-2.0 oslo.rootwrap>=5.8.0 # Apache-2.0 diff --git a/tacker/policy.py b/tacker/policy.py index b018eb486..ac418b193 100644 --- a/tacker/policy.py +++ b/tacker/policy.py @@ -38,11 +38,17 @@ LOG = logging.getLogger(__name__) _ENFORCER = None ADMIN_CTX_POLICY = 'context_is_admin' -# TODO(gmann): Remove setting the default value of config policy_file -# once oslo_policy change the default value to 'policy.yaml'. -# https://github.com/openstack/oslo.policy/blob/a626ad12fe5a3abd49d70e3e5b95589d279ab578/oslo_policy/opts.py#L49 +# TODO(gmann): Remove setting the default value of config options: +# - 'policy_file' once oslo_policy change their default value to what is +# overridden here. +# - 'enforce_scope', and 'enforce_new_defaults' once cinder is ready with the +# new RBAC (oslo_policy enable them by default) DEFAULT_POLICY_FILE = 'policy.yaml' -opts.set_defaults(cfg.CONF, DEFAULT_POLICY_FILE) +opts.set_defaults( + cfg.CONF, + DEFAULT_POLICY_FILE, + enforce_scope=False, + enforce_new_defaults=False) def reset(): @@ -52,12 +58,21 @@ def reset(): _ENFORCER = None -def init(conf=cfg.CONF, policy_file=None): +def init(conf=cfg.CONF, policy_file=None, suppress_deprecation_warnings=False): """Init an instance of the Enforcer class.""" global _ENFORCER if not _ENFORCER: _ENFORCER = policy.Enforcer(conf, policy_file=policy_file) + # NOTE(gmann): Explicitly disable the warnings for policies + # changing their default check_str. During policy-defaults-refresh + # work, all the policy defaults have been changed and warning for + # each policy started filling the logs limit for various tool. + # Once we move to new defaults only world then we can enable these + # warning again. + _ENFORCER.suppress_default_change_warnings = True + if suppress_deprecation_warnings: + _ENFORCER.suppress_deprecation_warnings = True register_rules(_ENFORCER) _ENFORCER.load_rules() diff --git a/tacker/sol_refactored/conductor/vnflcm_driver_v2.py b/tacker/sol_refactored/conductor/vnflcm_driver_v2.py index 846411db6..85ba51679 100644 --- a/tacker/sol_refactored/conductor/vnflcm_driver_v2.py +++ b/tacker/sol_refactored/conductor/vnflcm_driver_v2.py @@ -16,6 +16,7 @@ import os import pickle import subprocess +import sys from oslo_log import log as logging from oslo_utils import uuidutils @@ -119,7 +120,7 @@ class VnfLcmDriverV2(object): # script is relative path to Definitions/xxx.yaml script_path = os.path.join(tmp_csar_dir, "Definitions", script) - out = subprocess.run(["python3", script_path], + out = subprocess.run([sys.executable, script_path], input=pickle.dumps(script_dict), stdout=subprocess.PIPE, stderr=subprocess.PIPE) diff --git a/tacker/sol_refactored/infra_drivers/openstack/openstack.py b/tacker/sol_refactored/infra_drivers/openstack/openstack.py index c58ad2cb3..b1f678a4e 100644 --- a/tacker/sol_refactored/infra_drivers/openstack/openstack.py +++ b/tacker/sol_refactored/infra_drivers/openstack/openstack.py @@ -19,6 +19,7 @@ import os import pickle import re import subprocess +import sys import yaml from dateutil import parser @@ -672,7 +673,7 @@ class Openstack(object): tmp_csar_dir = vnfd.make_tmp_csar_dir() script_path = os.path.join(tmp_csar_dir, script) - out = subprocess.run(["python3", script_path], + out = subprocess.run([sys.executable, script_path], input=pickle.dumps(vnfc_param), stdout=subprocess.PIPE, stderr=subprocess.PIPE) vnfd.remove_tmp_csar_dir(tmp_csar_dir) @@ -889,7 +890,7 @@ class Openstack(object): script_path = os.path.join( os.path.dirname(__file__), "userdata_main.py") - out = subprocess.run(["python3", script_path], + out = subprocess.run([sys.executable, script_path], input=pickle.dumps(script_dict), stdout=subprocess.PIPE, stderr=subprocess.PIPE) diff --git a/tacker/tests/base.py b/tacker/tests/base.py index f0cc912e5..545794951 100644 --- a/tacker/tests/base.py +++ b/tacker/tests/base.py @@ -156,7 +156,7 @@ class BaseTestCase(testtools.TestCase): cfg.CONF.set_override('state_path', self.temp_dir) self.setup_config() - policy.init() + policy.init(suppress_deprecation_warnings=True) self.addCleanup(policy.reset) self.addCleanup(mock.patch.stopall) self.addCleanup(CONF.reset) diff --git a/tacker/tests/unit/api/v2/test_api_v2.py b/tacker/tests/unit/api/v2/test_api_v2.py index 7dc40fce9..673f03b50 100644 --- a/tacker/tests/unit/api/v2/test_api_v2.py +++ b/tacker/tests/unit/api/v2/test_api_v2.py @@ -1049,7 +1049,7 @@ class JSONV2TestCase(APIv2TestBase, testlib_api.WebTestCase): def test_get_keystone_strip_admin_only_attribute(self): tenant_id = _uuid() # Inject rule in policy engine - policy.init() + policy.init(suppress_deprecation_warnings=True) common_policy._rules['get_network:name'] = common_policy.parse_rule( "rule:admin_only") res = self._test_get(tenant_id, tenant_id, 200) diff --git a/tacker/tests/unit/policies/base.py b/tacker/tests/unit/policies/base.py index 4c0fe9ea7..ddcf72e5a 100644 --- a/tacker/tests/unit/policies/base.py +++ b/tacker/tests/unit/policies/base.py @@ -46,7 +46,7 @@ class BasePolicyTest(base.TestCase): # policy and initialize again so that rule will be re-loaded # considering the enforce_new_defaults new value. policy.reset() - policy.init() + policy.init(suppress_deprecation_warnings=True) self.addCleanup(policy.reset) self.admin_project_id = uuids.admin_project_id diff --git a/tacker/tests/unit/test_policy.py b/tacker/tests/unit/test_policy.py index 8639070c0..6012fef7c 100644 --- a/tacker/tests/unit/test_policy.py +++ b/tacker/tests/unit/test_policy.py @@ -54,14 +54,14 @@ class PolicyFileTestCase(base.BaseTestCase): action = "example:test" with open(tmpfilename, "w") as policyfile: policyfile.write("""{"example:test": ""}""") - policy.init() + policy.init(suppress_deprecation_warnings=True) policy.enforce(self.context, action, self.target) with open(tmpfilename, "w") as policyfile: policyfile.write("""{"example:test": "!"}""") # NOTE(vish): reset stored policy cache so we don't have to # sleep(1) policy._POLICY_CACHE = {} - policy.init() + policy.init(suppress_deprecation_warnings=True) self.assertRaises(exceptions.PolicyNotAuthorized, policy.enforce, self.context, @@ -76,7 +76,7 @@ class PolicyTestCase(base.BaseTestCase): policy.reset() self.addCleanup(policy.reset) # NOTE(vish): preload rules to circumvent reloading from file - policy.init() + policy.init(suppress_deprecation_warnings=True) rules = { "true": '@', "example:allowed": '@', @@ -178,7 +178,7 @@ class DefaultPolicyTestCase(base.BaseTestCase): super(DefaultPolicyTestCase, self).setUp() self.skipTest("Not ready yet") policy.reset() - policy.init() + policy.init(suppress_deprecation_warnings=True) self.addCleanup(policy.reset) self.rules = { @@ -228,7 +228,7 @@ class TackerPolicyTestCase(base.BaseTestCase): super(TackerPolicyTestCase, self).setUp() self.skipTest("Not ready yet") policy.reset() - policy.init() + policy.init(suppress_deprecation_warnings=True) self.addCleanup(policy.reset) self.admin_only_legacy = "role:admin" self.admin_or_owner_legacy = "role:admin or tenant_id:%(tenant_id)s" @@ -477,7 +477,7 @@ class TackerPolicyTestCase(base.BaseTestCase): # Trigger a policy with rule admin_or_owner action = "create_network" target = {'tenant_id': 'fake'} - policy.init() + policy.init(suppress_deprecation_warnings=True) self.assertRaises(exceptions.PolicyCheckError, policy.enforce, self.context, action, target)