From 37cf9487439df520dbdeb9890a274d85bd06d342 Mon Sep 17 00:00:00 2001 From: Al Bailey Date: Thu, 28 Apr 2022 14:21:12 +0000 Subject: [PATCH] Cleanup tox targets for debian patching - Removed the py27 target for sw-patch since it is python3 only. - Set the base python in tox.ini to python3. - Removed the site-packages directive for pylint since site level rpm component no longer needs to be installed. - Added the pep8 target (it just calls flake8). - Removed redundant settings already set at testenv level. - Cleaned up bandit suppressions that were not needed. - Cleaned up the flake8 suppressions that were not needed. - Cleaned up the pylint suppressions that were not needed. - Minor code cleanup to reduce number of flake8 suppressions - Minor code cleanup to reduce number of pylint suppressions - Updated the copyright dates for updated source files Test Plan: Tox Story: 2009969 Task: 45209 Signed-off-by: Al Bailey Change-Id: Ifccf2a274530b14bacb6ce2dc32f8cca01e26217 --- .../cgcs-patch/cgcs_patch/authapi/hooks.py | 4 +- sw-patch/cgcs-patch/cgcs_patch/config.py | 6 +- .../cgcs-patch/cgcs_patch/patch_controller.py | 4 +- .../cgcs-patch/cgcs_patch/patch_signing.py | 21 ++---- .../cgcs_patch/tests/test_patch_agent.py | 9 ++- .../cgcs_patch_id/patch_id_allocator.py | 6 +- .../patch_id_allocator_server.py | 4 +- sw-patch/cgcs-patch/pylint.rc | 8 +- sw-patch/cgcs-patch/tox.ini | 75 +++++++------------ 9 files changed, 50 insertions(+), 87 deletions(-) diff --git a/sw-patch/cgcs-patch/cgcs_patch/authapi/hooks.py b/sw-patch/cgcs-patch/cgcs_patch/authapi/hooks.py index c4d2353e..d3147dc6 100755 --- a/sw-patch/cgcs-patch/cgcs_patch/authapi/hooks.py +++ b/sw-patch/cgcs-patch/cgcs_patch/authapi/hooks.py @@ -2,7 +2,7 @@ # # Copyright © 2012 New Dream Network, LLC (DreamHost) # -# Author: Doug Hellmann +# Author: Doug Hellmann # noqa: H105 # # Licensed under the Apache License, Version 2.0 (the "License"); you may # not use this file except in compliance with the License. You may obtain @@ -16,7 +16,7 @@ # License for the specific language governing permissions and limitations # under the License. # -# Copyright (c) 2013-2017 Wind River Systems, Inc. +# Copyright (c) 2013-2022 Wind River Systems, Inc. # diff --git a/sw-patch/cgcs-patch/cgcs_patch/config.py b/sw-patch/cgcs-patch/cgcs_patch/config.py index 2e9f9842..1eb5bdcd 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/config.py +++ b/sw-patch/cgcs-patch/cgcs_patch/config.py @@ -1,5 +1,5 @@ """ -Copyright (c) 2014-2017 Wind River Systems, Inc. +Copyright (c) 2014-2022 Wind River Systems, Inc. SPDX-License-Identifier: Apache-2.0 @@ -74,7 +74,7 @@ def read_config(): # The platform.conf file has no section headers, which causes problems # for ConfigParser. So we'll fake it out. - ini_str = u'[platform_conf]\n' + open(tsc.PLATFORM_CONF_FILE, 'r').read() + ini_str = '[platform_conf]\n' + open(tsc.PLATFORM_CONF_FILE, 'r').read() ini_fp = io.StringIO(ini_str) config.readfp(ini_fp) @@ -122,7 +122,7 @@ def get_mgmt_iface(): # The platform.conf file has no section headers, which causes problems # for ConfigParser. So we'll fake it out. - ini_str = u'[platform_conf]\n' + open(tsc.PLATFORM_CONF_FILE, 'r').read() + ini_str = '[platform_conf]\n' + open(tsc.PLATFORM_CONF_FILE, 'r').read() ini_fp = io.StringIO(ini_str) config.readfp(ini_fp) diff --git a/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py b/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py index 1be1f881..506e679e 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py +++ b/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py @@ -1648,7 +1648,7 @@ class PatchController(PatchService): msg.send(self.sock_out) self.socket_lock.release() - # Now we wait, up to two mins... TODO: Wait on a condition + # Now we wait, up to two mins. future enhancement: Wait on a condition my_ip = cfg.get_mgmt_ip() sync_rc = False max_time = time.time() + 120 @@ -2004,7 +2004,7 @@ class PatchController(PatchService): LOG.info("host-install async_req: %s", msg) return dict(info=msg_info, warning=msg_warning, error=msg_error) - # Now we wait, up to ten mins... TODO: Wait on a condition + # Now we wait, up to ten mins. future enhancement: Wait on a condition resp_rx = False max_time = time.time() + 600 while time.time() < max_time: diff --git a/sw-patch/cgcs-patch/cgcs_patch/patch_signing.py b/sw-patch/cgcs-patch/cgcs_patch/patch_signing.py index 37884ee4..b6ac15fa 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/patch_signing.py +++ b/sw-patch/cgcs-patch/cgcs_patch/patch_signing.py @@ -1,5 +1,5 @@ """ -Copyright (c) 2017 Wind River Systems, Inc. +Copyright (c) 2017-2022 Wind River Systems, Inc. SPDX-License-Identifier: Apache-2.0 @@ -8,12 +8,7 @@ SPDX-License-Identifier: Apache-2.0 import os from Cryptodome.Signature import PKCS1_PSS from Cryptodome.Hash import SHA256 -from Cryptodome.PublicKey import RSA # pylint: disable=unused-import -from Cryptodome.Util.asn1 import DerSequence # pylint: disable=unused-import -from binascii import a2b_base64 # pylint: disable=unused-import -from cgcs_patch.patch_verify import read_RSA_key -from cgcs_patch.patch_verify import cert_type_formal_str -from cgcs_patch.patch_verify import cert_type_dev_str +from cgcs_patch import patch_verify # To save memory, read and hash 1M of files at a time default_blocksize = 1 * 1024 * 1024 @@ -22,8 +17,8 @@ default_blocksize = 1 * 1024 * 1024 # # The (currently hardcoded) path on the signing server will be replaced # by the capability to specify filename from calling function. -private_key_files = {cert_type_formal_str: '/signing/keys/formal-private-key.pem', - cert_type_dev_str: os.path.expandvars('$MY_REPO/build-tools/signing/dev-private-key.pem') +private_key_files = {patch_verify.cert_type_formal_str: '/signing/keys/formal-private-key.pem', + patch_verify.cert_type_dev_str: os.path.expandvars('$MY_REPO/build-tools/signing/dev-private-key.pem') } @@ -59,15 +54,15 @@ def sign_files(filenames, signature_file, private_key=None, cert_type=None): dict_key = cert_type filename = private_key_files[dict_key] # print 'cert_type given: Checking to see if ' + filename + ' exists\n' - if not os.path.exists(filename) and dict_key == cert_type_formal_str: + if not os.path.exists(filename) and dict_key == patch_verify.cert_type_formal_str: # The formal key is asked for, but is not locally available, # substitute the dev key, and we will try to resign with the formal later. - dict_key = cert_type_dev_str + dict_key = patch_verify.cert_type_dev_str filename = private_key_files[dict_key] need_resign_with_formal = True if os.path.exists(filename): # print 'Getting private key from ' + filename + '\n' - private_key = read_RSA_key(open(filename, 'rb').read()) + private_key = patch_verify.read_RSA_key(open(filename, 'rb').read()) else: # Search for available keys for dict_key in private_key_files.keys(): @@ -75,7 +70,7 @@ def sign_files(filenames, signature_file, private_key=None, cert_type=None): # print 'Search for available keys: Checking to see if ' + filename + ' exists\n' if os.path.exists(filename): # print 'Getting private key from ' + filename + '\n' - private_key = read_RSA_key(open(filename, 'rb').read()) + private_key = patch_verify.read_RSA_key(open(filename, 'rb').read()) assert (private_key is not None), "Could not find signing key" diff --git a/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_agent.py b/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_agent.py index f28dbb81..a35dd910 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_agent.py +++ b/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_agent.py @@ -1,11 +1,10 @@ # # SPDX-License-Identifier: Apache-2.0 # -# Copyright (c) 2019 Wind River Systems, Inc. +# Copyright (c) 2019-2022 Wind River Systems, Inc. # import mock -import six # pylint: disable=unused-import import sys import testtools @@ -19,11 +18,13 @@ sys.modules['dnf.transaction'] = mock.Mock() sys.modules['libdnf'] = mock.Mock() sys.modules['libdnf.transaction'] = mock.Mock() -import cgcs_patch.patch_agent # noqa: E402 +# Need to suppress E402 because the sys.modules need +# to be mocked before importing patch_agent +from cgcs_patch import patch_agent # noqa: E402 class CgcsPatchAgentTestCase(testtools.TestCase): def test_cgcs_patch_agent_instantiate(self): - pc = cgcs_patch.patch_agent.PatchAgent() + pc = patch_agent.PatchAgent() self.assertIsNotNone(pc) diff --git a/sw-patch/cgcs-patch/cgcs_patch_id/patch_id_allocator.py b/sw-patch/cgcs-patch/cgcs_patch_id/patch_id_allocator.py index d553472f..c10f0715 100755 --- a/sw-patch/cgcs-patch/cgcs_patch_id/patch_id_allocator.py +++ b/sw-patch/cgcs-patch/cgcs_patch_id/patch_id_allocator.py @@ -1,14 +1,10 @@ #!/usr/bin/python # -# Copyright (c) 2013-2014 Wind River Systems, Inc. +# Copyright (c) 2013-2022 Wind River Systems, Inc. # # SPDX-License-Identifier: Apache-2.0 # - - import fcntl -import string -import time directory = "/localdisk/designer/jenkins/patch_ids" diff --git a/sw-patch/cgcs-patch/cgcs_patch_id/patch_id_allocator_server.py b/sw-patch/cgcs-patch/cgcs_patch_id/patch_id_allocator_server.py index 84ae04d8..51cafa2d 100755 --- a/sw-patch/cgcs-patch/cgcs_patch_id/patch_id_allocator_server.py +++ b/sw-patch/cgcs-patch/cgcs_patch_id/patch_id_allocator_server.py @@ -1,12 +1,10 @@ #!/usr/bin/env python # -# Copyright (c) 2013-2014 Wind River Systems, Inc. +# Copyright (c) 2013-2022 Wind River Systems, Inc. # # SPDX-License-Identifier: Apache-2.0 # -import os -import sys import web import patch_id_allocator as pida diff --git a/sw-patch/cgcs-patch/pylint.rc b/sw-patch/cgcs-patch/pylint.rc index e5286b6b..885377ac 100644 --- a/sw-patch/cgcs-patch/pylint.rc +++ b/sw-patch/cgcs-patch/pylint.rc @@ -126,20 +126,14 @@ enable=E1603,E1609,E1610,E1602,E1606,E1608,E1607,E1605,E1604,E1601,E1611,W1652, # --enable=similarities". If you want to run only the classes checker, but have # no Warning level messages displayed, use"--disable=all --enable=classes # --disable=W" -# H216, unittest mock # W0107 unnecessary-pass -# W0511 fixme # W0602 global-variable-not-assigned # W0603 global-statement # W0703 broad-except # W0707 raise-missing-from -# W1406 redundant-u-string-prefix # W1505 deprecated-method # W1514 unspecified-encoding -# Disable Python3 checkers: -# W1618: no-absolute-import -disable=C, H216, R, - W0107, W0511, W0602, W0603, W0703, W0707, W1406, W1505, W1514, W1618 +disable=C,R,W0107,W0602,W0603,W0703,W0707,W1505,W1514 [REPORTS] diff --git a/sw-patch/cgcs-patch/tox.ini b/sw-patch/cgcs-patch/tox.ini index 4c172d79..e7e318f7 100644 --- a/sw-patch/cgcs-patch/tox.ini +++ b/sw-patch/cgcs-patch/tox.ini @@ -5,13 +5,29 @@ # [tox] -envlist = flake8,py36,py39,pylint,cover +envlist = pep8,py36,py39,pylint,bandit,cover minversion = 2.3.2 skipsdist = True stxdir = {toxinidir}/../../.. [testenv] +basepython = python3 + +deps = -r{toxinidir}/requirements.txt + -r{toxinidir}/test-requirements.txt + -e{[tox]stxdir}/fault/fm-api/source + -e{[tox]stxdir}/config/sysinv/sysinv/sysinv + -e{[tox]stxdir}/config/tsconfig/tsconfig + +install_command = pip install \ + -v -v -v \ + -c{env:UPPER_CONSTRAINTS_FILE:https://opendev.org/starlingx/root/raw/branch/master/build-tools/requirements/debian/upper-constraints.txt} \ + {opts} {packages} + +passenv = + XDG_CACHE_HOME + setenv = VIRTUAL_ENV={envdir} LANG=en_US.UTF-8 LANGUAGE=en_US:en @@ -25,21 +41,8 @@ setenv = VIRTUAL_ENV={envdir} PYTHONWARNINGS=default::DeprecationWarning PIP_DISABLE_PIP_VERSION_CHECK=1 -passenv = - XDG_CACHE_HOME - sitepackages = False -install_command = pip install \ - -v -v -v \ - -c{env:UPPER_CONSTRAINTS_FILE:https://opendev.org/starlingx/root/raw/branch/master/build-tools/requirements/debian/upper-constraints.txt} \ - {opts} {packages} - -deps = -r{toxinidir}/requirements.txt - -r{toxinidir}/test-requirements.txt - -e{[tox]stxdir}/fault/fm-api/source - -e{[tox]stxdir}/config/sysinv/sysinv/sysinv - -e{[tox]stxdir}/config/tsconfig/tsconfig - +usedevelop = true whitelist_externals = find sh @@ -49,25 +52,13 @@ commands = stestr run {posargs} stestr slowest -[testenv:py27] -basepython = python2.7 -commands = {[testenv:stestr]commands} - [testenv:py36] basepython = python3.6 commands = {[testenv:stestr]commands} [testenv:py39] basepython = python3.9 -install_command = pip install \ - -v -v -v \ - -c{env:UPPER_CONSTRAINTS_FILE:https://opendev.org/starlingx/root/raw/branch/master/build-tools/requirements/debian/upper-constraints.txt} \ - {opts} {packages} -commands = - find . -name "*.pyc" -delete - stestr run {posargs} - stestr slowest - +commands = {[testenv:stestr]commands} [bandit] # B101: Test for use of assert @@ -75,19 +66,15 @@ commands = # B110: Try, Except, Pass detected. # B303: Use of insecure MD2, MD4, MD5, or SHA1 hash function. # B311: Standard pseudo-random generators are not suitable for security/cryptographic purposes -# B314: Blacklisted calls to xml.etree.ElementTree # B318: Blacklisted calls to xml.dom.minidom # B320: Blacklisted calls to lxml.etree.parse # B404: Import of subprocess module -# B405: import xml.etree # B408: import xml.minidom # B410: import etree -# B413: import pyCrypto -# B506: Test for use of yaml load # B602: Test for use of popen with shell equals true # B603: Test for use of subprocess without shell equals true # B607: Test for starting a process with a partial path -skips = B101,B104,B110,B303,B311,B314,B318,B320,B404,B405,B408,B410,B413,B506,B602,B603,B607 +skips = B101,B104,B110,B303,B311,B318,B320,B404,B408,B410,B602,B603,B607 exclude = tests [testenv:bandit] @@ -97,9 +84,6 @@ commands = bandit --ini tox.ini -n 5 -r cgcs_patch [flake8] # ignore below errors , will fix flake8 errors in future -# H101 Use TODO(NAME) -# H102 Apache 2.0 license header not found -# H105 Don't use author tags # H306 imports not in alphabetical order # H401 docstring should not start with a space # H404 multi line docstring should start without a leading new line @@ -108,28 +92,23 @@ commands = bandit --ini tox.ini -n 5 -r cgcs_patch # W504 line break after binary operator # E501 line too long. skipped because some of the code files include templates # that end up quite wide -# F401 'XXXXX' imported but unused show-source = True -ignore = H101,H102,H105,H306,H401,H404,H405, - W504,E501,F401 +ignore = H306,H401,H404,H405,W504,E501 + exclude = .venv,.git,.tox,dist,doc,*lib/python*,*egg,build,release-tag-* # H106: Don't put vim configuration in source files (off by default). # H203: Use assertIs(Not)None to check for None (off by default). # (todo) enable H904 Delay string interpolations at logging calls (off by default) -enable-extensions = H106 H203 +enable-extensions = H106 H203,H904 max-line-length = 80 [testenv:flake8] -basepython = python3 -deps = {[testenv]deps} -usedevelop = False -#skip_install = True -commands = - flake8 {posargs} . +commands = flake8 {posargs} + +[testenv:pep8] +commands = flake8 {posargs} [testenv:pylint] -basepython = python3 -sitepackages = True commands = pylint cgcs_patch --rcfile=./pylint.rc [testenv:cover]