From f36111954b703c309ab4c62a00d4b7ca69bf8375 Mon Sep 17 00:00:00 2001 From: Andreas Jaeger Date: Sun, 29 Mar 2020 15:13:18 +0200 Subject: [PATCH] Update hacking for Python3 The repo is Python 3 now, so update hacking to version 3.0 which supports Python 3. Fix problems found. Update local hacking checks for new flake8. Change-Id: Ic440219814ee0c2b98217e9a821f38f5baf482ec --- keystone/api/role_inferences.py | 1 + keystone/application_credential/schema.py | 4 +-- keystone/cmd/doctor/ldap.py | 2 +- keystone/common/sql/core.py | 1 + keystone/common/validation/parameter_types.py | 4 +-- keystone/conf/security_compliance.py | 4 +-- keystone/federation/utils.py | 1 + keystone/identity/backends/ldap/common.py | 1 + keystone/identity/core.py | 36 +++++++++---------- keystone/identity/schema.py | 2 +- keystone/notifications.py | 1 + keystone/resource/schema.py | 2 +- keystone/tests/hacking/checks.py | 15 ++++---- .../tests/unit/common/test_notifications.py | 2 +- keystone/tests/unit/core.py | 2 +- keystone/tests/unit/test_cli.py | 8 ++--- keystone/tests/unit/test_v3_catalog.py | 2 +- keystone/tests/unit/test_v3_federation.py | 4 +-- keystone/tests/unit/test_v3_identity.py | 4 +-- keystone/tests/unit/test_validation.py | 6 ++-- keystone/tests/unit/utils.py | 2 +- test-requirements.txt | 2 +- tox.ini | 12 +++++-- 23 files changed, 66 insertions(+), 52 deletions(-) diff --git a/keystone/api/role_inferences.py b/keystone/api/role_inferences.py index a007f71e6d..a7f501425b 100644 --- a/keystone/api/role_inferences.py +++ b/keystone/api/role_inferences.py @@ -67,4 +67,5 @@ class RoleInferencesAPI(ks_flask.APIBase): rel='role_inferences') ] + APIs = (RoleInferencesAPI,) diff --git a/keystone/application_credential/schema.py b/keystone/application_credential/schema.py index 9f2b50c20b..f6138db22e 100644 --- a/keystone/application_credential/schema.py +++ b/keystone/application_credential/schema.py @@ -38,11 +38,11 @@ _access_rules_properties = { 'type': 'string', 'minLength': 0, 'maxLength': 225, - 'pattern': '^\/.*' + 'pattern': r'^\/.*' }, 'method': { 'type': 'string', - 'pattern': '^(POST|GET|HEAD|PATCH|PUT|DELETE)$' + 'pattern': r'^(POST|GET|HEAD|PATCH|PUT|DELETE)$' }, 'service': parameter_types.id_string, 'id': parameter_types.id_string, diff --git a/keystone/cmd/doctor/ldap.py b/keystone/cmd/doctor/ldap.py index 7c56851f93..96e006840f 100644 --- a/keystone/cmd/doctor/ldap.py +++ b/keystone/cmd/doctor/ldap.py @@ -21,7 +21,7 @@ import keystone.conf CONF = keystone.conf.CONF -CONFIG_REGEX = '^keystone\..*?\.conf$' +CONFIG_REGEX = r'^keystone\..*?\.conf$' def symptom_LDAP_user_enabled_emulation_dn_ignored(): diff --git a/keystone/common/sql/core.py b/keystone/common/sql/core.py index b91b39c31b..ed84e5893e 100644 --- a/keystone/common/sql/core.py +++ b/keystone/common/sql/core.py @@ -111,6 +111,7 @@ def initialize_decorator(init): init(self, *args, **kwargs) return initialize + ModelBase.__init__ = initialize_decorator(ModelBase.__init__) diff --git a/keystone/common/validation/parameter_types.py b/keystone/common/validation/parameter_types.py index abe8da6e4a..2ceefea623 100644 --- a/keystone/common/validation/parameter_types.py +++ b/keystone/common/validation/parameter_types.py @@ -26,7 +26,7 @@ name = { 'type': 'string', 'minLength': 1, 'maxLength': 255, - 'pattern': '[\S]+' + 'pattern': r'[\S]+' } external_id_string = { @@ -41,7 +41,7 @@ id_string = { 'maxLength': 64, # TODO(lbragstad): Find a way to make this configurable such that the end # user chooses how much control they want over id_strings with a regex - 'pattern': '^[a-zA-Z0-9-]+$' + 'pattern': r'^[a-zA-Z0-9-]+$' } mapping_id_string = { diff --git a/keystone/conf/security_compliance.py b/keystone/conf/security_compliance.py index 5b27141b08..5d528d0dea 100644 --- a/keystone/conf/security_compliance.py +++ b/keystone/conf/security_compliance.py @@ -98,9 +98,9 @@ password_regex = cfg.StrOpt( The regular expression used to validate password strength requirements. By default, the regular expression will match any password. The following is an example of a pattern which requires at least 1 letter, 1 digit, and have a -minimum length of 7 characters: ^(?=.*\d)(?=.*[a-zA-Z]).{7,}$ This feature +minimum length of 7 characters: ^(?=.*\\\d)(?=.*[a-zA-Z]).{7,}$ This feature depends on the `sql` backend for the `[identity] driver`. -""")) +""")) # noqa: W605 password_regex_description = cfg.StrOpt( 'password_regex_description', diff --git a/keystone/federation/utils.py b/keystone/federation/utils.py index 95a8e83058..e4b7ecc0e5 100644 --- a/keystone/federation/utils.py +++ b/keystone/federation/utils.py @@ -39,6 +39,7 @@ class UserType(object): EPHEMERAL = 'ephemeral' LOCAL = 'local' + ROLE_PROPERTIES = { "type": "array", "items": { diff --git a/keystone/identity/backends/ldap/common.py b/keystone/identity/backends/ldap/common.py index 058aec423c..91e0713357 100644 --- a/keystone/identity/backends/ldap/common.py +++ b/keystone/identity/backends/ldap/common.py @@ -77,6 +77,7 @@ def utf8_encode(value): raise TypeError("value must be basestring, " "not %s" % value_cls_name) + _utf8_decoder = codecs.getdecoder('utf-8') diff --git a/keystone/identity/core.py b/keystone/identity/core.py index 80e860159e..38ebe2fbdd 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -1513,26 +1513,26 @@ class Manager(manager.Manager): def shadow_federated_user(self, idp_id, protocol_id, unique_id, display_name, email=None, group_ids=None): - """Map a federated user to a user. + """Map a federated user to a user. - :param idp_id: identity provider id - :param protocol_id: protocol id - :param unique_id: unique id for the user within the IdP - :param display_name: user's display name - :param email: user's email - :param group_ids: list of group ids to add the user to + :param idp_id: identity provider id + :param protocol_id: protocol id + :param unique_id: unique id for the user within the IdP + :param display_name: user's display name + :param email: user's email + :param group_ids: list of group ids to add the user to - :returns: dictionary of the mapped User entity - """ - user_dict = self._shadow_federated_user( - idp_id, protocol_id, unique_id, display_name, email) - # Note(knikolla): The shadowing operation can be cached, - # however we need to update the expiring group memberships. - if group_ids: - for group_id in group_ids: - PROVIDERS.shadow_users_api.add_user_to_group_expires( - user_dict['id'], group_id) - return user_dict + :returns: dictionary of the mapped User entity + """ + user_dict = self._shadow_federated_user( + idp_id, protocol_id, unique_id, display_name, email) + # Note(knikolla): The shadowing operation can be cached, + # however we need to update the expiring group memberships. + if group_ids: + for group_id in group_ids: + PROVIDERS.shadow_users_api.add_user_to_group_expires( + user_dict['id'], group_id) + return user_dict class MappingManager(manager.Manager): diff --git a/keystone/identity/schema.py b/keystone/identity/schema.py index e39c376dbe..0031578cbd 100644 --- a/keystone/identity/schema.py +++ b/keystone/identity/schema.py @@ -23,7 +23,7 @@ _identity_name = { 'type': 'string', 'minLength': 1, 'maxLength': 255, - 'pattern': '[\S]+' + 'pattern': r'[\S]+' } # Schema for Identity v3 API diff --git a/keystone/notifications.py b/keystone/notifications.py index 65980b800a..e536ebdd43 100644 --- a/keystone/notifications.py +++ b/keystone/notifications.py @@ -838,6 +838,7 @@ def _add_username_to_initiator(initiator): return initiator + emit_event = CadfNotificationWrapper diff --git a/keystone/resource/schema.py b/keystone/resource/schema.py index dac73f1221..404b4a89ed 100644 --- a/keystone/resource/schema.py +++ b/keystone/resource/schema.py @@ -18,7 +18,7 @@ _name_properties = { 'type': 'string', 'minLength': 1, 'maxLength': 64, - 'pattern': '[\S]+' + 'pattern': r'[\S]+' } _project_tag_name_properties = { diff --git a/keystone/tests/hacking/checks.py b/keystone/tests/hacking/checks.py index 38ca8d5485..13b1429717 100644 --- a/keystone/tests/hacking/checks.py +++ b/keystone/tests/hacking/checks.py @@ -24,6 +24,7 @@ please see pycodestyle.py. """ import ast +from hacking import core import re @@ -73,6 +74,9 @@ class CheckForMutableDefaultArgs(BaseASTChecker): """ + name = "check_for_mutable_default_args" + version = "1.0" + CHECK_DESC = 'K001 Using mutable as a function/method default' MUTABLES = ( ast.List, ast.ListComp, @@ -88,6 +92,7 @@ class CheckForMutableDefaultArgs(BaseASTChecker): super(CheckForMutableDefaultArgs, self).generic_visit(node) +@core.flake8ext def block_comments_begin_with_a_space(physical_line, line_number): """There should be a space after the # of block comments. @@ -114,6 +119,8 @@ def block_comments_begin_with_a_space(physical_line, line_number): class CheckForTranslationIssues(BaseASTChecker): + name = "check_for_translation_issues" + version = "1.0" LOGGING_CHECK_DESC = 'K005 Using translated string in logging' USING_DEPRECATED_WARN = 'K009 Using the deprecated Logger.warn' LOG_MODULES = ('logging', 'oslo_log.log') @@ -297,6 +304,7 @@ class CheckForTranslationIssues(BaseASTChecker): self.add_error(msg, message=self.LOGGING_CHECK_DESC) +@core.flake8ext def dict_constructor_with_sequence_copy(logical_line): """Should use a dict comprehension instead of a dict constructor. @@ -318,10 +326,3 @@ def dict_constructor_with_sequence_copy(logical_line): if dict_constructor_with_sequence_re.match(logical_line): yield (0, MESSAGE) - - -def factory(register): - register(CheckForMutableDefaultArgs) - register(block_comments_begin_with_a_space) - register(CheckForTranslationIssues) - register(dict_constructor_with_sequence_copy) diff --git a/keystone/tests/unit/common/test_notifications.py b/keystone/tests/unit/common/test_notifications.py index 26623d1f83..b0fb720f11 100644 --- a/keystone/tests/unit/common/test_notifications.py +++ b/keystone/tests/unit/common/test_notifications.py @@ -759,7 +759,7 @@ class CADFNotificationsForPCIDSSEvents(BaseNotificationTest): conf.config(group='security_compliance', minimum_password_age=2) conf.config(group='security_compliance', - password_regex='^(?=.*\d)(?=.*[a-zA-Z]).{7,}$') + password_regex=r'^(?=.*\d)(?=.*[a-zA-Z]).{7,}$') conf.config(group='security_compliance', password_regex_description='1 letter, 1 digit, 7 chars') diff --git a/keystone/tests/unit/core.py b/keystone/tests/unit/core.py index ffdf525779..a42db22b6b 100644 --- a/keystone/tests/unit/core.py +++ b/keystone/tests/unit/core.py @@ -86,7 +86,7 @@ IN_MEM_DB_CONN_STRING = 'sqlite://' # Strictly matches ISO 8601 timestamps with subsecond precision like: # 2016-06-28T20:48:56.000000Z TIME_FORMAT = '%Y-%m-%dT%H:%M:%S.%fZ' -TIME_FORMAT_REGEX = '^\d{4}-[0-1]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d\.\d{6}Z$' +TIME_FORMAT_REGEX = r'^\d{4}-[0-1]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d\.\d{6}Z$' exception._FATAL_EXCEPTION_FORMAT_ERRORS = True os.makedirs(TMPDIR) diff --git a/keystone/tests/unit/test_cli.py b/keystone/tests/unit/test_cli.py index 0c29de9471..572660a92e 100644 --- a/keystone/tests/unit/test_cli.py +++ b/keystone/tests/unit/test_cli.py @@ -1313,7 +1313,7 @@ class SecurityComplianceDoctorTests(unit.TestCase): # Symptom Detected: Regular expression is invalid self.config_fixture.config( group='security_compliance', - password_regex='^^(??=.*\d)$') + password_regex=r'^^(??=.*\d)$') self.assertTrue( security_compliance.symptom_invalid_password_regular_expression()) @@ -1321,7 +1321,7 @@ class SecurityComplianceDoctorTests(unit.TestCase): # No Symptom Detected: Regular expression is valid self.config_fixture.config( group='security_compliance', - password_regex='^(?=.*\d)(?=.*[a-zA-Z]).{7,}$') + password_regex=r'^(?=.*\d)(?=.*[a-zA-Z]).{7,}$') self.assertFalse( security_compliance.symptom_invalid_password_regular_expression()) @@ -1337,7 +1337,7 @@ class SecurityComplianceDoctorTests(unit.TestCase): # Symptom Detected: Regular expression is set but description is not self.config_fixture.config( group='security_compliance', - password_regex='^(?=.*\d)(?=.*[a-zA-Z]).{7,}$') + password_regex=r'^(?=.*\d)(?=.*[a-zA-Z]).{7,}$') self.config_fixture.config( group='security_compliance', password_regex_description=None) @@ -1350,7 +1350,7 @@ class SecurityComplianceDoctorTests(unit.TestCase): desc = '1 letter, 1 digit, and a minimum length of 7 is required' self.config_fixture.config( group='security_compliance', - password_regex='^(?=.*\d)(?=.*[a-zA-Z]).{7,}$') + password_regex=r'^(?=.*\d)(?=.*[a-zA-Z]).{7,}$') self.config_fixture.config( group='security_compliance', password_regex_description=desc) diff --git a/keystone/tests/unit/test_v3_catalog.py b/keystone/tests/unit/test_v3_catalog.py index f2d3a1a3c1..a5b8b04aa2 100644 --- a/keystone/tests/unit/test_v3_catalog.py +++ b/keystone/tests/unit/test_v3_catalog.py @@ -683,7 +683,7 @@ class CatalogTestCase(test_v3.RestfulTestCase): def test_deleting_endpoint_with_space_in_url(self): # add a space to all urls (intentional "i d" to test bug) - url_with_space = "http://127.0.0.1:8774 /v1.1/\$(tenant_i d)s" + url_with_space = "http://127.0.0.1:8774 /v1.1/\\$(tenant_i d)s" # create a v3 endpoint ref ref = unit.new_endpoint_ref(service_id=self.service['id'], diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py index 4ab8c02b13..b0cd5581bc 100644 --- a/keystone/tests/unit/test_v3_federation.py +++ b/keystone/tests/unit/test_v3_federation.py @@ -4347,8 +4347,8 @@ class SAMLGenerationTests(test_v3.RestfulTestCase): # The function __str__ in subprocess.CalledProcessError is # different between py3.6 and lower python version. expected_log = ( - "Error when signing assertion, reason: Command '%s' returned " - "non-zero exit status %s\.? %s\n" % + r"Error when signing assertion, reason: Command '%s' returned " + r"non-zero exit status %s\.? %s\n" % (CONF.saml.xmlsec1_binary, sample_returncode, sample_output)) self.assertRegex(logger_fixture.output, re.compile(r'%s' % expected_log)) diff --git a/keystone/tests/unit/test_v3_identity.py b/keystone/tests/unit/test_v3_identity.py index 71e0ec4d00..bce955ca5f 100644 --- a/keystone/tests/unit/test_v3_identity.py +++ b/keystone/tests/unit/test_v3_identity.py @@ -1000,7 +1000,7 @@ class PasswordValidationTestCase(ChangePasswordTestCase): # passwords requires: 1 letter, 1 digit, 7 chars self.config_fixture.config(group='security_compliance', password_regex=( - '^(?=.*\d)(?=.*[a-zA-Z]).{7,}$')) + r'^(?=.*\d)(?=.*[a-zA-Z]).{7,}$')) def test_create_user_with_invalid_password(self): user = unit.new_user_ref(domain_id=self.domain_id) @@ -1020,7 +1020,7 @@ class PasswordValidationTestCase(ChangePasswordTestCase): def test_changing_password_with_simple_password_strength(self): # password requires: any non-whitespace character self.config_fixture.config(group='security_compliance', - password_regex='[\S]+') + password_regex=r'[\S]+') self.change_password(password='simple', original_password=self.user_ref['password'], expected_status=http.client.NO_CONTENT) diff --git a/keystone/tests/unit/test_validation.py b/keystone/tests/unit/test_validation.py index 40fefdd76b..851b7d7f54 100644 --- a/keystone/tests/unit/test_validation.py +++ b/keystone/tests/unit/test_validation.py @@ -2515,7 +2515,7 @@ class PasswordValidationTestCase(unit.TestCase): # passwords requires: 1 letter, 1 digit, 7 chars self.config_fixture.config(group='security_compliance', password_regex=( - '^(?=.*\d)(?=.*[a-zA-Z]).{7,}$')) + r'^(?=.*\d)(?=.*[a-zA-Z]).{7,}$')) def test_password_validate_with_valid_strong_password(self): password = 'mypassword2' @@ -2541,14 +2541,14 @@ class PasswordValidationTestCase(unit.TestCase): def test_password_validate_with_invalid_password_regex(self): # invalid regular expression, missing beginning '[' self.config_fixture.config(group='security_compliance', - password_regex='\S]+') + password_regex=r'\S]+') password = 'mypassword2' self.assertRaises(exception.PasswordValidationError, validators.validate_password, password) # fix regular expression and validate self.config_fixture.config(group='security_compliance', - password_regex='[\S]+') + password_regex=r'[\S]+') validators.validate_password(password) diff --git a/keystone/tests/unit/utils.py b/keystone/tests/unit/utils.py index 00e48bd03c..7dfb73fc0e 100644 --- a/keystone/tests/unit/utils.py +++ b/keystone/tests/unit/utils.py @@ -85,7 +85,7 @@ def wip(message, expected_exception=Exception, bug=None): __e = None try: f(*args, **kwargs) - except Exception as __e: + except Exception as __e: # noqa F841 if (expected_exception != Exception and not isinstance(__e, expected_exception)): raise AssertionError( diff --git a/test-requirements.txt b/test-requirements.txt index 3e53e25538..6c27909b24 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -2,7 +2,7 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. -hacking>=1.1.0,<1.2.0 # Apache-2.0 +hacking>=3.0,<3.1.0 # Apache-2.0 pep257==0.7.0 # MIT License flake8-docstrings==0.2.1.post1 # MIT bashate>=0.5.1 # Apache-2.0 diff --git a/tox.ini b/tox.ini index a4967fe1eb..5b31596b52 100644 --- a/tox.ini +++ b/tox.ini @@ -119,7 +119,8 @@ enable-extensions = H203,H904 # TODO(wxy): Fix the pep8 issue. # E402: module level import not at top of file # W503: line break before binary operator -ignore = D100,D101,D102,D103,D104,D203,E402,W503 +# W504 line break after binary operator +ignore = D100,D101,D102,D103,D104,D203,E402,W503,W504 exclude=.venv,.git,.tox,build,dist,*lib/python*,*egg,tools,vendor,.update-venv,*.ini,*.po,*.pot max-complexity=24 @@ -167,7 +168,14 @@ commands = oslopolicy-sample-generator --config-file config-generator/keystone-p import_exceptions = keystone.i18n six.moves -local-check-factory = keystone.tests.hacking.checks.factory + +[flake8:local-plugins] +extension = + K001 = checks:CheckForMutableDefaultArgs + K002 = checks:block_comments_begin_with_a_space + K005 = checks:CheckForTranslationIssues + K008 = checks:dict_constructor_with_sequence_copy +paths = ./keystone/tests/hacking [testenv:bindep] # Do not install any requirements. We want this to be fast and work even if