diff --git a/HACKING.rst b/HACKING.rst index 7e57e61a..5943d3aa 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -1,4 +1,11 @@ oslo.policy Style Commandments ====================================================== -Read the OpenStack Style Commandments http://docs.openstack.org/developer/hacking/ +- Step 1: Read the OpenStack Style Commandments + http://docs.openstack.org/developer/hacking/ +- Step 2: Read on + +oslo.policy Specific Commandments +--------------------------------- + +- Avoid using "double quotes" where you can reasonably use 'single quotes' \ No newline at end of file diff --git a/oslo_policy/_checks.py b/oslo_policy/_checks.py index 7860b61d..c0e4c4a0 100644 --- a/oslo_policy/_checks.py +++ b/oslo_policy/_checks.py @@ -54,7 +54,7 @@ class FalseCheck(BaseCheck): def __str__(self): """Return a string representation of this check.""" - return "!" + return '!' def __call__(self, target, cred, enforcer): """Check the policy.""" @@ -68,7 +68,7 @@ class TrueCheck(BaseCheck): def __str__(self): """Return a string representation of this check.""" - return "@" + return '@' def __call__(self, target, cred, enforcer): """Check the policy.""" @@ -91,7 +91,7 @@ class Check(BaseCheck): def __str__(self): """Return a string representation of this check.""" - return "%s:%s" % (self.kind, self.match) + return '%s:%s' % (self.kind, self.match) class NotCheck(BaseCheck): @@ -109,7 +109,7 @@ class NotCheck(BaseCheck): def __str__(self): """Return a string representation of this check.""" - return "not %s" % self.rule + return 'not %s' % self.rule def __call__(self, target, cred, enforcer): """Check the policy. @@ -135,7 +135,7 @@ class AndCheck(BaseCheck): def __str__(self): """Return a string representation of this check.""" - return "(%s)" % ' and '.join(str(r) for r in self.rules) + return '(%s)' % ' and '.join(str(r) for r in self.rules) def __call__(self, target, cred, enforcer): """Check the policy. @@ -179,7 +179,7 @@ class OrCheck(BaseCheck): def __str__(self): """Return a string representation of this check.""" - return "(%s)" % ' or '.join(str(r) for r in self.rules) + return '(%s)' % ' or '.join(str(r) for r in self.rules) def __call__(self, target, cred, enforcer): """Check the policy. @@ -229,7 +229,7 @@ def register(name, func=None): return decorator -@register("rule") +@register('rule') class RuleCheck(Check): """Recursively checks credentials based on the defined rules.""" @@ -241,7 +241,7 @@ class RuleCheck(Check): return False -@register("role") +@register('role') class RoleCheck(Check): """Check that there is a matching role in the ``creds`` dict.""" @@ -273,7 +273,7 @@ class HttpCheck(Check): 'credentials': jsonutils.dumps(creds)} post_data = urlparse.urlencode(data) f = urlrequest.urlopen(url, post_data) - return f.read() == "True" + return f.read() == 'True' @register(None) diff --git a/oslo_policy/_parser.py b/oslo_policy/_parser.py index 41cb7012..dfa51415 100644 --- a/oslo_policy/_parser.py +++ b/oslo_policy/_parser.py @@ -141,7 +141,7 @@ class ParseState(object): """ if len(self.values) != 1: - raise ValueError("Could not parse rule") + raise ValueError('Could not parse rule') return self.values[0] @reducer('(', 'check', ')') @@ -201,7 +201,7 @@ def _parse_check(rule): try: kind, match = rule.split(':', 1) except Exception: - LOG.exception(_LE("Failed to understand rule %s") % rule) + LOG.exception(_LE('Failed to understand rule %s') % rule) # If the rule is invalid, we'll fail closed return _checks.FalseCheck() @@ -211,7 +211,7 @@ def _parse_check(rule): elif None in _checks.registered_checks: return _checks.registered_checks[None](kind, match) else: - LOG.error(_LE("No handler for matches of kind %s") % kind) + LOG.error(_LE('No handler for matches of kind %s') % kind) return _checks.FalseCheck() @@ -327,7 +327,7 @@ def _parse_text_rule(rule): return state.result except ValueError: # Couldn't parse the rule - LOG.exception(_LE("Failed to understand rule %s") % rule) + LOG.exception(_LE('Failed to understand rule %s') % rule) # Fail closed return _checks.FalseCheck() diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py index e4619149..87aad7c5 100644 --- a/oslo_policy/policy.py +++ b/oslo_policy/policy.py @@ -246,7 +246,7 @@ class PolicyNotAuthorized(Exception): """Default exception raised for policy enforcement failure.""" def __init__(self, rule): - msg = _("Policy doesn't allow %s to be performed.") % rule + msg = _('Policy does not allow %s to be performed.') % rule super(PolicyNotAuthorized, self).__init__(msg) @@ -350,8 +350,8 @@ class Enforcer(object): """ if not isinstance(rules, dict): - raise TypeError(_("Rules must be an instance of dict or Rules, " - "got %s instead") % type(rules)) + raise TypeError(_('Rules must be an instance of dict or Rules, ' + 'got %s instead') % type(rules)) self.use_conf = use_conf if overwrite: self.rules = Rules(rules, self.default_rule) @@ -405,7 +405,7 @@ class Enforcer(object): if reloaded or not self.rules or not overwrite: rules = Rules.load_json(data, self.default_rule) self.set_rules(rules, overwrite=overwrite, use_conf=True) - LOG.debug("Reloaded policy file: %(path)s", + LOG.debug('Reloaded policy file: %(path)s', {'path': path}) def _get_policy_path(self, path): @@ -466,7 +466,7 @@ class Enforcer(object): # Evaluate the rule result = self.rules[rule](target, creds, self) except KeyError: - LOG.debug("Rule [%s] doesn't exist" % rule) + LOG.debug('Rule [%s] does not exist' % rule) # If the rule doesn't exist, fail closed result = False diff --git a/oslo_policy/tests/test_checks.py b/oslo_policy/tests/test_checks.py index f7df15a2..bb8ab138 100644 --- a/oslo_policy/tests/test_checks.py +++ b/oslo_policy/tests/test_checks.py @@ -196,21 +196,21 @@ class GenericCheckTestCase(base.PolicyBaseTestCase): self.enforcer), True) def test_constant_literal_mismatch(self): - check = _checks.GenericCheck("True", '%(enabled)s') + check = _checks.GenericCheck('True', '%(enabled)s') self.assertEqual(check(dict(enabled=False), {}, self.enforcer), False) def test_constant_literal_accept(self): - check = _checks.GenericCheck("True", '%(enabled)s') + check = _checks.GenericCheck('True', '%(enabled)s') self.assertEqual(check(dict(enabled=True), {}, self.enforcer), True) def test_deep_credentials_dictionary_lookup(self): - check = _checks.GenericCheck("a.b.c.d", 'APPLES') + check = _checks.GenericCheck('a.b.c.d', 'APPLES') credentials = {'a': {'b': {'c': {'d': 'APPLES'}}}} @@ -224,19 +224,19 @@ class GenericCheckTestCase(base.PolicyBaseTestCase): # First a valid check - rest of case is expecting failures # Should prove the basic credentials structure before we test # for failure cases. - check = _checks.GenericCheck("o.t", 'ORANGES') + check = _checks.GenericCheck('o.t', 'ORANGES') self.assertEqual(check({}, credentials, self.enforcer), True) # Case where final key is missing - check = _checks.GenericCheck("o.v", 'ORANGES') + check = _checks.GenericCheck('o.v', 'ORANGES') self.assertEqual(check({}, credentials, self.enforcer), False) # Attempt to access key under a missing dictionary - check = _checks.GenericCheck("q.v", 'APPLES') + check = _checks.GenericCheck('q.v', 'APPLES') self.assertEqual(check({}, credentials, self.enforcer), False) diff --git a/oslo_policy/tests/test_parser.py b/oslo_policy/tests/test_parser.py index bae3cb71..66c64320 100644 --- a/oslo_policy/tests/test_parser.py +++ b/oslo_policy/tests/test_parser.py @@ -45,8 +45,8 @@ class ParseCheckTestCase(test_base.BaseTestCase): self.assertTrue(isinstance(result, _checks.FalseCheck)) @mock.patch.object(_checks, 'registered_checks', { - 'spam': mock.Mock(return_value="spam_check"), - None: mock.Mock(return_value="none_check"), + 'spam': mock.Mock(return_value='spam_check'), + None: mock.Mock(return_value='none_check'), }) def test_check(self): result = _parser._parse_check('spam:handler') @@ -57,7 +57,7 @@ class ParseCheckTestCase(test_base.BaseTestCase): self.assertFalse(_checks.registered_checks[None].called) @mock.patch.object(_checks, 'registered_checks', { - None: mock.Mock(return_value="none_check"), + None: mock.Mock(return_value='none_check'), }) def test_check_default(self): result = _parser._parse_check('spam:handler') @@ -188,7 +188,7 @@ class ParseStateMetaTestCase(test_base.BaseTestCase): elif reduction == ['g', 'h', 'i']: self.assertEqual(reducer, 'reduce2') else: - self.fail("Unrecognized reducer discovered") + self.fail('Unrecognized reducer discovered') class ParseStateTestCase(test_base.BaseTestCase): @@ -387,7 +387,7 @@ class ParseRuleTestCase(test_base.BaseTestCase): @mock.patch.object(_parser, '_parse_list_rule', return_value='list rule') def test_parse_rule_string(self, mock_parse_list_rule, mock_parse_text_rule): - result = _parser.parse_rule("a string") + result = _parser.parse_rule('a string') self.assertEqual(result, 'text rule') self.assertFalse(mock_parse_list_rule.called) diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py index 96427f89..cab63b8d 100644 --- a/oslo_policy/tests/test_policy.py +++ b/oslo_policy/tests/test_policy.py @@ -79,7 +79,7 @@ class RulesTestCase(test_base.BaseTestCase): self.assertEqual(rules.default_rule, 'default') self.assertEqual(rules, dict( - admin_or_owner=[["role:admin"], ["project_id:%(project_id)s"]], + admin_or_owner=[['role:admin'], ['project_id:%(project_id)s']], default=[], )) @@ -88,7 +88,7 @@ class RulesTestCase(test_base.BaseTestCase): "admin_or_owner": "role:admin or project_id:%(project_id)s" }""" rules = policy.Rules(dict( - admin_or_owner="role:admin or project_id:%(project_id)s", + admin_or_owner='role:admin or project_id:%(project_id)s', )) self.assertEqual(str(rules), exemplar) @@ -174,7 +174,7 @@ class EnforcerTest(base.PolicyBaseTestCase): }""" rules = policy.Rules.load_json(rules_json) self.enforcer.set_rules(rules) - action = "cloudwatch:PutMetricData" + action = 'cloudwatch:PutMetricData' creds = {'roles': ''} self.assertEqual(self.enforcer.enforce(action, {}, creds), True) @@ -187,7 +187,7 @@ class EnforcerTest(base.PolicyBaseTestCase): default_rule = _checks.TrueCheck() enforcer = policy.Enforcer(cfg.CONF, default_rule=default_rule) enforcer.set_rules(rules) - action = "cloudwatch:PutMetricData" + action = 'cloudwatch:PutMetricData' creds = {'roles': ''} self.assertEqual(enforcer.enforce(action, {}, creds), True) @@ -204,10 +204,10 @@ class EnforcerTest(base.PolicyBaseTestCase): # Call enforce(), it will load rules from # policy configuration files, to overwrite # existing fake ones. - self.assertFalse(self.enforcer.enforce("test", {}, - {"roles": ["test"]})) - self.assertTrue(self.enforcer.enforce("default", {}, - {"roles": ["fakeB"]})) + self.assertFalse(self.enforcer.enforce('test', {}, + {'roles': ['test']})) + self.assertTrue(self.enforcer.enforce('default', {}, + {'roles': ['fakeB']})) # Check against rule dict again from # enforcer object directly. @@ -232,12 +232,12 @@ class EnforcerTest(base.PolicyBaseTestCase): # Call enforce(), it will load rules from # policy configuration files, to merge with # existing fake ones. - self.assertTrue(self.enforcer.enforce("test", {}, - {"roles": ["test"]})) + self.assertTrue(self.enforcer.enforce('test', {}, + {'roles': ['test']})) # The existing rules have a same key with # new loaded ones will be overwrote. - self.assertFalse(self.enforcer.enforce("default", {}, - {"roles": ["fakeZ"]})) + self.assertFalse(self.enforcer.enforce('default', {}, + {'roles': ['fakeZ']})) # Check against rule dict again from # enforcer object directly. @@ -255,15 +255,15 @@ class EnforcerTest(base.PolicyBaseTestCase): # policy configure files. enforcer = policy.Enforcer(cfg.CONF) self.assertTrue(enforcer.use_conf) - self.assertTrue(enforcer.enforce("default", {}, - {"roles": ["fakeB"]})) - self.assertFalse(enforcer.enforce("test", {}, - {"roles": ["test"]})) + self.assertTrue(enforcer.enforce('default', {}, + {'roles': ['fakeB']})) + self.assertFalse(enforcer.enforce('test', {}, + {'roles': ['test']})) # After enforcement the flag should # be remained there. self.assertTrue(enforcer.use_conf) - self.assertFalse(enforcer.enforce("_dynamic_test_rule", {}, - {"roles": ["test"]})) + self.assertFalse(enforcer.enforce('_dynamic_test_rule', {}, + {'roles': ['test']})) # Then if configure file got changed, # reloading will be triggered when calling # enforcer(), this case could happen only @@ -282,8 +282,8 @@ class EnforcerTest(base.PolicyBaseTestCase): with open(enforcer.policy_path, 'w') as f: f.write(jsonutils.dumps(rules)) - self.assertTrue(enforcer.enforce("_dynamic_test_rule", {}, - {"roles": ["test"]})) + self.assertTrue(enforcer.enforce('_dynamic_test_rule', {}, + {'roles': ['test']})) def test_enforcer_force_reload_false(self): self.enforcer.set_rules({'test': 'test'}) @@ -339,32 +339,32 @@ class CheckFunctionTestCase(base.PolicyBaseTestCase): def test_check_explicit(self): rule = base.FakeCheck() - result = self.enforcer.enforce(rule, "target", "creds") - self.assertEqual(result, ("target", "creds", self.enforcer)) + result = self.enforcer.enforce(rule, 'target', 'creds') + self.assertEqual(result, ('target', 'creds', self.enforcer)) def test_check_no_rules(self): self.conf.set_override('policy_file', 'empty.json', group='oslo_policy') self.enforcer.default_rule = None self.enforcer.load_rules() - result = self.enforcer.enforce('rule', "target", "creds") + result = self.enforcer.enforce('rule', 'target', 'creds') self.assertEqual(result, False) def test_check_with_rule(self): self.enforcer.set_rules(dict(default=base.FakeCheck())) - result = self.enforcer.enforce("default", "target", "creds") + result = self.enforcer.enforce('default', 'target', 'creds') - self.assertEqual(result, ("target", "creds", self.enforcer)) + self.assertEqual(result, ('target', 'creds', self.enforcer)) def test_check_raises(self): self.enforcer.set_rules(dict(default=_checks.FalseCheck())) try: self.enforcer.enforce('rule', 'target', 'creds', - True, MyException, "arg1", - "arg2", kw1="kwarg1", kw2="kwarg2") + True, MyException, 'arg1', + 'arg2', kw1='kwarg1', kw2='kwarg2') except MyException as exc: - self.assertEqual(exc.args, ("arg1", "arg2")) - self.assertEqual(exc.kwargs, dict(kw1="kwarg1", kw2="kwarg2")) + self.assertEqual(exc.args, ('arg1', 'arg2')) + self.assertEqual(exc.kwargs, dict(kw1='kwarg1', kw2='kwarg2')) else: - self.fail("enforcer.enforce() failed to raise requested exception") + self.fail('enforcer.enforce() failed to raise requested exception')