diff --git a/.gitignore b/.gitignore index 80de8ab7..c0d0d098 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,7 @@ doc/source/_static/cyborg.conf.sample doc/source/_static/cyborg.policy.yaml.sample # Sample profile etc/cyborg/policy.json.sample +etc/cyborg/policy.yaml.sample etc/cyborg/cyborg.conf.sample .idea/* .vscode/* diff --git a/cyborg/cmd/status.py b/cyborg/cmd/status.py index 84799597..63925b7a 100644 --- a/cyborg/cmd/status.py +++ b/cyborg/cmd/status.py @@ -16,20 +16,38 @@ import sys from oslo_config import cfg from oslo_upgradecheck import upgradecheck +from oslo_utils import fileutils from cyborg.common.i18n import _ +CONF = cfg.CONF + + class Checks(upgradecheck.UpgradeCommands): """Various upgrade checks should be added as separate methods in this class and added to _upgrade_checks tuple. """ - def _check_placeholder(self): - # TODO(whoami-rajat):This is just a placeholder for upgrade checks, - # it should be removed when the actual checks are added - return upgradecheck.Result(upgradecheck.Code.SUCCESS) + def _check_policy_json(self): + "Checks to see if policy file is JSON-formatted policy file." + msg = _("Your policy file is JSON-formatted which is " + "deprecated since Victoria release (Cyborg 5.0.0). " + "You need to switch to YAML-formatted file. You can use the " + "``oslopolicy-convert-json-to-yaml`` tool to convert existing " + "JSON-formatted files to YAML-formatted files in a " + "backwards-compatible manner: " + "https://docs.openstack.org/oslo.policy/" + "latest/cli/oslopolicy-convert-json-to-yaml.html.") + status = upgradecheck.Result(upgradecheck.Code.SUCCESS) + # NOTE(gmann): Check if policy file exist and is in + # JSON format by actually loading the file not just + # by checking the extension. + policy_path = CONF.find_file(CONF.oslo_policy.policy_file) + if policy_path and fileutils.is_json(policy_path): + status = upgradecheck.Result(upgradecheck.Code.FAILURE, msg) + return status # The format of the check functions is to return an # oslo_upgradecheck.upgradecheck.Result @@ -39,8 +57,8 @@ class Checks(upgradecheck.UpgradeCommands): # in the returned Result's "details" attribute. The # summary will be rolled up at the end of the check() method. _upgrade_checks = ( - # In the future there should be some real checks added here - (_('Placeholder'), _check_placeholder), + # Added in Victoria + (_('Policy File JSON to YAML Migration'), _check_policy_json), ) diff --git a/cyborg/common/authorize_wsgi.py b/cyborg/common/authorize_wsgi.py index 51d8c048..1c3db892 100644 --- a/cyborg/common/authorize_wsgi.py +++ b/cyborg/common/authorize_wsgi.py @@ -16,6 +16,7 @@ import sys from oslo_concurrency import lockutils from oslo_config import cfg from oslo_log import log +from oslo_policy import opts from oslo_policy import policy from oslo_versionedobjects import base as object_base import pecan @@ -30,6 +31,36 @@ CONF = cfg.CONF LOG = log.getLogger(__name__) +# 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 +DEFAULT_POLICY_FILE = 'policy.yaml' +opts.set_defaults(CONF, DEFAULT_POLICY_FILE) + + +def pick_policy_file(policy_file): + # TODO(gmann): We have changed the default value of + # CONF.oslo_policy.policy_file option to 'policy.yaml' in Victoria + # release. To avoid breaking any deployment relying on default + # value, we need to add this is fallback logic to pick the old default + # policy file (policy.yaml) if exist. We can to remove this fallback + # logic sometime in future. + if policy_file: + return policy_file + + if CONF.oslo_policy.policy_file == DEFAULT_POLICY_FILE: + location = CONF.get_location('policy_file', 'oslo_policy').location + if CONF.find_file(CONF.oslo_policy.policy_file): + return CONF.oslo_policy.policy_file + elif location in [cfg.Locations.opt_default, + cfg.Locations.set_default]: + old_default = 'policy.json' + if CONF.find_file(old_default): + return old_default + # Return overridden value + return CONF.oslo_policy.policy_file + + @lockutils.synchronized('policy_enforcer', 'cyborg-') def init_enforcer(policy_file=None, rules=None, default_rule=None, use_conf=True, @@ -55,10 +86,12 @@ def init_enforcer(policy_file=None, rules=None, # loaded exactly once - when this module-global is initialized. # Defining these in the relevant API modules won't work # because API classes lack singletons and don't use globals. - _ENFORCER = policy.Enforcer(CONF, policy_file=policy_file, - rules=rules, - default_rule=default_rule, - use_conf=use_conf) + _ENFORCER = policy.Enforcer( + CONF, + policy_file=pick_policy_file(policy_file), + rules=rules, + default_rule=default_rule, + use_conf=use_conf) if suppress_deprecation_warnings: _ENFORCER.suppress_deprecation_warnings = True _ENFORCER.register_defaults(policies.list_policies()) diff --git a/cyborg/common/policy.py b/cyborg/common/policy.py index 1911a32f..68157902 100644 --- a/cyborg/common/policy.py +++ b/cyborg/common/policy.py @@ -19,7 +19,7 @@ """ from oslo_policy import policy # NOTE: to follow policy-in-code spec, we define defaults for -# the granular policies in code, rather than in policy.json. +# the granular policies in code, rather than in policy.yaml. # All of these may be overridden by configuration, but we can # depend on their existence throughout the code. diff --git a/cyborg/tests/unit/cmd/test_status.py b/cyborg/tests/unit/cmd/test_status.py index 0c97c94f..90900a53 100644 --- a/cyborg/tests/unit/cmd/test_status.py +++ b/cyborg/tests/unit/cmd/test_status.py @@ -12,19 +12,77 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo_upgradecheck.upgradecheck import Code +import fixtures +import os +import tempfile +import yaml + +from oslo_config import cfg +from oslo_serialization import jsonutils +from oslo_upgradecheck import upgradecheck from cyborg.cmd import status +from cyborg.common import authorize_wsgi from cyborg.tests import base -class TestUpgradeChecks(base.TestCase): +class TestUpgradeCheckPolicyJSON(base.TestCase): def setUp(self): - super(TestUpgradeChecks, self).setUp() - self.cmd = status.Checks() + super(TestUpgradeCheckPolicyJSON, self).setUp() + self.cmd = status.UpgradeCommands() + authorize_wsgi.CONF.clear_override('policy_file', group='oslo_policy') + self.data = { + 'rule_admin': 'True', + 'rule_admin2': 'is_admin:True' + } + self.temp_dir = self.useFixture(fixtures.TempDir()) + fd, self.json_file = tempfile.mkstemp(dir=self.temp_dir.path) + fd, self.yaml_file = tempfile.mkstemp(dir=self.temp_dir.path) - def test__check_placeholder(self): - check_result = self.cmd._check_placeholder() - self.assertEqual( - Code.SUCCESS, check_result.code) + with open(self.json_file, 'w') as fh: + jsonutils.dump(self.data, fh) + with open(self.yaml_file, 'w') as fh: + yaml.dump(self.data, fh) + + original_search_dirs = cfg._search_dirs + + def fake_search_dirs(dirs, name): + dirs.append(self.temp_dir.path) + return original_search_dirs(dirs, name) + + self.mock_search = self.useFixture(fixtures.MockPatch( + 'oslo_config.cfg._search_dirs')).mock + self.mock_search.side_effect = fake_search_dirs + + def test_policy_json_file_fail_upgrade(self): + # Test with policy json file full path set in config. + self.flags(policy_file=self.json_file, group="oslo_policy") + self.assertEqual(upgradecheck.Code.FAILURE, + self.cmd._check_policy_json().code) + + def test_policy_yaml_file_pass_upgrade(self): + # Test with full policy yaml file path set in config. + self.flags(policy_file=self.yaml_file, group="oslo_policy") + self.assertEqual(upgradecheck.Code.SUCCESS, + self.cmd._check_policy_json().code) + + def test_no_policy_file_pass_upgrade(self): + # Test with no policy file exist. + self.assertEqual(upgradecheck.Code.SUCCESS, + self.cmd._check_policy_json().code) + + def test_default_policy_yaml_file_pass_upgrade(self): + tmpfilename = os.path.join(self.temp_dir.path, 'policy.yaml') + with open(tmpfilename, 'w') as fh: + yaml.dump(self.data, fh) + self.assertEqual(upgradecheck.Code.SUCCESS, + self.cmd._check_policy_json().code) + + def test_old_default_policy_json_file_fail_upgrade(self): + self.flags(policy_file='policy.json', group="oslo_policy") + tmpfilename = os.path.join(self.temp_dir.path, 'policy.json') + with open(tmpfilename, 'w') as fh: + jsonutils.dump(self.data, fh) + self.assertEqual(upgradecheck.Code.FAILURE, + self.cmd._check_policy_json().code) diff --git a/cyborg/tests/unit/policy_fixture.py b/cyborg/tests/unit/policy_fixture.py index e8bd5828..64c9be98 100644 --- a/cyborg/tests/unit/policy_fixture.py +++ b/cyborg/tests/unit/policy_fixture.py @@ -36,7 +36,7 @@ class PolicyFixture(fixtures.Fixture): super(PolicyFixture, self).setUp() self.policy_dir = self.useFixture(fixtures.TempDir()) self.policy_file_name = os.path.join(self.policy_dir.path, - 'policy.json') + 'policy.yaml') with open(self.policy_file_name, 'w') as policy_file: policy_file.write(policy_data) policy_opts.set_defaults(CONF) diff --git a/devstack/settings b/devstack/settings index acbb0b6e..4cef8e58 100644 --- a/devstack/settings +++ b/devstack/settings @@ -15,7 +15,7 @@ CYBORG_AUTH_CACHE_DIR=${CYBORG_AUTH_CACHE_DIR:-/var/cache/cyborg} CYBORG_CONF_DIR=${CYBORG_CONF_DIR:-/etc/cyborg} CYBORG_CONF_FILE=$CYBORG_CONF_DIR/cyborg.conf CYBORG_API_PASTE_INI=$CYBORG_CONF_DIR/api-paste.ini -CYBORG_POLICY_JSON=$CYBORG_CONF_DIR/policy.json +CYBORG_POLICY_JSON=$CYBORG_CONF_DIR/policy.yaml CYBORG_SERVICE_HOST=${CYBORG_SERVICE_HOST:-$SERVICE_HOST} CYBORG_SERVICE_PORT=${CYBORG_SERVICE_PORT:-6666} CYBORG_SERVICE_PROTOCOL=${CYBORG_SERVICE_PROTOCOL:-$SERVICE_PROTOCOL} diff --git a/doc/source/configuration/sample_policy.rst b/doc/source/configuration/sample_policy.rst index ce3823e7..20d5df05 100644 --- a/doc/source/configuration/sample_policy.rst +++ b/doc/source/configuration/sample_policy.rst @@ -2,6 +2,15 @@ Cyborg Sample Policy ==================== +.. warning:: + + JSON formatted policy file is deprecated since Cyborg 5.0.0(Victoria). + Use YAML formatted file. Use `oslopolicy-convert-json-to-yaml`__ tool + to convert the existing JSON to YAML formatted policy file in backward + compatible way. + +.. __: https://docs.openstack.org/oslo.policy/latest/cli/oslopolicy-convert-json-to-yaml.html + The following is a sample cyborg policy file that has been auto-generated from default policy values in code. If you're using the default policies, then the maintenance of this file is not necessary, and it should not be copied into diff --git a/etc/cyborg/README.policy.json.txt b/etc/cyborg/README.policy.yaml.txt similarity index 51% rename from etc/cyborg/README.policy.json.txt rename to etc/cyborg/README.policy.yaml.txt index 70285457..1b99e45d 100644 --- a/etc/cyborg/README.policy.json.txt +++ b/etc/cyborg/README.policy.yaml.txt @@ -1,4 +1,4 @@ -To generate the sample policy.json file, run the following command from the top +To generate the sample policy.yaml file, run the following command from the top level of the cyborg directory: tox -egenpolicy diff --git a/etc/cyborg/policy.json b/etc/cyborg/policy.yaml similarity index 100% rename from etc/cyborg/policy.json rename to etc/cyborg/policy.yaml diff --git a/releasenotes/notes/policy-file-default-value-change-de14a3688357b081.yaml b/releasenotes/notes/policy-file-default-value-change-de14a3688357b081.yaml new file mode 100644 index 00000000..2624dc86 --- /dev/null +++ b/releasenotes/notes/policy-file-default-value-change-de14a3688357b081.yaml @@ -0,0 +1,14 @@ +--- +upgrade: + - | + The default value of ``[oslo_policy] policy_file`` config option has been + changed from ``policy.json`` + to ``policy.yaml``. Cyborg policy new defaults since 5.0.0 and current + default value of ``[oslo_policy] policy_file`` config option (``policy.json``) + does not work when ``policy.json`` is generated by + `oslopolicy-sample-generator `_ tool. + Refer to `bug 1875418 `_ + for more details. + Also check `oslopolicy-convert-json-to-yaml `_ + tool to convert the JSON to YAML formatted policy file in + backward compatible way. diff --git a/requirements.txt b/requirements.txt index 787850d3..d9d60ccb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,9 +17,9 @@ oslo.service>=1.0.0,!=1.28.1 # Apache-2.0 oslo.db>=4.44.0 # Apache-2.0 os-resource-classes>=0.5.0 # Apache-2.0 oslo.upgradecheck>=0.1.0 # Apache-2.0 -oslo.utils>=3.33.0 # Apache-2.0 +oslo.utils>=4.5.0 # Apache-2.0 oslo.versionedobjects>=1.31.2 # Apache-2.0 -oslo.policy>=2.3.0 # Apache-2.0 +oslo.policy>=3.4.0 # Apache-2.0 SQLAlchemy>=0.9.0,!=1.1.5,!=1.1.6,!=1.1.7,!=1.1.8 # MIT alembic>=0.8.10 # MIT stevedore>=1.5.0 # Apache-2.0 diff --git a/setup.cfg b/setup.cfg index f4eef3d8..38a88bcc 100644 --- a/setup.cfg +++ b/setup.cfg @@ -26,7 +26,7 @@ packages = cyborg data_files = etc/cyborg = - etc/cyborg/policy.json + etc/cyborg/policy.yaml etc/cyborg/api-paste.ini [entry_points] diff --git a/tools/config/cyborg-policy-generator.conf b/tools/config/cyborg-policy-generator.conf index 7c7748ca..a928b7b8 100644 --- a/tools/config/cyborg-policy-generator.conf +++ b/tools/config/cyborg-policy-generator.conf @@ -1,3 +1,3 @@ [DEFAULT] -output_file = etc/cyborg/policy.json.sample +output_file = etc/cyborg/policy.yaml.sample namespace = cyborg.api