From abaea457e5288ffec91545aaeda5250469184951 Mon Sep 17 00:00:00 2001 From: Al Bailey Date: Mon, 11 Jul 2022 18:49:51 +0000 Subject: [PATCH] Debian: Minor sw-patch cleanup - Adding missing requirements for sw-patch - Cleaning up redundant dependencies - Invoke clean as part of debian build - Rename Starlingx to StarlingX - Display error details for failed sw-patch commands - Fix a python3 string comparison affecting duplex - Fix report-app-dependencies - Remove 'six' from sw-patch since the code is py3 only - Fix a KeyError if restart_script not found in metadata. - Allow bandit to ignore 'input' checks on python3 since it is considered safe Test Plan: PASS: Build/Bootstrap/Unlock AIO-SX Debian PASS: sw-patch report-app-dependencies --app foo PASS: Upload/Apply/Remove/Delete a patch on AIO-DX system host-install was not tested on AIO-DX as that capability is still under active developement. Closes-Bug: #1976535 Signed-off-by: Al Bailey Change-Id: I94ec47e2f2087989da26ebab0e05c3b0d8f303e6 --- sw-patch/cgcs-patch/cgcs_patch/authapi/app.py | 2 +- sw-patch/cgcs-patch/cgcs_patch/base.py | 3 +- sw-patch/cgcs-patch/cgcs_patch/config.py | 2 +- .../cgcs-patch/cgcs_patch/patch_client.py | 15 +++-- .../cgcs-patch/cgcs_patch/patch_controller.py | 22 +++----- .../cgcs-patch/cgcs_patch/patch_functions.py | 3 +- .../cgcs_patch/tests/test_patch_controller.py | 2 +- sw-patch/cgcs-patch/requirements.txt | 5 +- sw-patch/cgcs-patch/tox.ini | 2 +- sw-patch/debian/deb_folder/control | 55 +++++++++++-------- sw-patch/debian/deb_folder/rules | 3 + tox.ini | 3 +- 12 files changed, 61 insertions(+), 56 deletions(-) diff --git a/sw-patch/cgcs-patch/cgcs_patch/authapi/app.py b/sw-patch/cgcs-patch/cgcs_patch/authapi/app.py index 4eed6d20..d96d2bd3 100755 --- a/sw-patch/cgcs-patch/cgcs_patch/authapi/app.py +++ b/sw-patch/cgcs-patch/cgcs_patch/authapi/app.py @@ -5,6 +5,7 @@ SPDX-License-Identifier: Apache-2.0 """ +import configparser from oslo_config import cfg import pecan @@ -13,7 +14,6 @@ from cgcs_patch.authapi import config from cgcs_patch.authapi import hooks from cgcs_patch.authapi import policy -from six.moves import configparser auth_opts = [ cfg.StrOpt('auth_strategy', diff --git a/sw-patch/cgcs-patch/cgcs_patch/base.py b/sw-patch/cgcs-patch/cgcs_patch/base.py index 8c743c09..efe822a5 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/base.py +++ b/sw-patch/cgcs-patch/cgcs_patch/base.py @@ -8,6 +8,7 @@ SPDX-License-Identifier: Apache-2.0 import socket import struct import subprocess +import sys import time import cgcs_patch.utils as utils @@ -154,7 +155,7 @@ class PatchService(object): cmd = "ip maddr show %s | awk 'BEGIN {ORS=\"\"}; {if ($2 == \"%s\") print $2}'" % \ (cfg.get_mgmt_iface(), self.mcast_addr) try: - result = subprocess.check_output(cmd, shell=True) + result = subprocess.check_output(cmd, shell=True).decode(sys.stdout.encoding) if result == self.mcast_addr: return diff --git a/sw-patch/cgcs-patch/cgcs_patch/config.py b/sw-patch/cgcs-patch/cgcs_patch/config.py index 0fe48d1c..31bed0fe 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/config.py +++ b/sw-patch/cgcs-patch/cgcs_patch/config.py @@ -4,10 +4,10 @@ Copyright (c) 2014-2022 Wind River Systems, Inc. SPDX-License-Identifier: Apache-2.0 """ +import configparser import io import logging import os -from six.moves import configparser import socket import tsconfig.tsconfig as tsc diff --git a/sw-patch/cgcs-patch/cgcs_patch/patch_client.py b/sw-patch/cgcs-patch/cgcs_patch/patch_client.py index 41bd9331..873c829b 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/patch_client.py +++ b/sw-patch/cgcs-patch/cgcs_patch/patch_client.py @@ -4,21 +4,17 @@ Copyright (c) 2014-2022 Wind River Systems, Inc. SPDX-License-Identifier: Apache-2.0 """ -from __future__ import print_function -from six.moves import input -import requests import json import os -import sys -import shutil import re -import time +import requests +import shutil import signal - import subprocess +import sys import textwrap +import time -# noinspection PyUnresolvedReferences from requests_toolbelt import MultipartEncoder import cgcs_patch.constants as constants @@ -878,6 +874,8 @@ def query_dependencies(debug, args): if 'patches' in data: for patch_id in sorted(data['patches']): print(patch_id) + if 'error' in data and data["error"] != "": + print("Error: %s" % data.get("error")) elif req.status_code == 500: print("An internal error has occurred. Please check /var/log/patching.log for details") @@ -1259,6 +1257,7 @@ def patch_report_app_dependencies_req(debug, args): # pylint: disable=unused-ar if req.status_code == 200: return 0 else: + print("An internal error has occurred. Please check /var/log/patching.log for details") return 1 diff --git a/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py b/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py index 5619c1bf..f309fe63 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py +++ b/sw-patch/cgcs-patch/cgcs_patch/patch_controller.py @@ -4,13 +4,12 @@ Copyright (c) 2014-2022 Wind River Systems, Inc. SPDX-License-Identifier: Apache-2.0 """ +import configparser import gc import json import os import select import shutil -import six -from six.moves import configparser import socket import subprocess import tarfile @@ -55,7 +54,6 @@ from cgcs_patch.base import PatchService import cgcs_patch.config as cfg import cgcs_patch.utils as utils - import cgcs_patch.messages as messages import cgcs_patch.constants as constants @@ -653,10 +651,7 @@ class PatchController(PatchService): pass def write_state_file(self): - if six.PY2: - config = configparser.ConfigParser() - elif six.PY3: - config = configparser.ConfigParser(strict=False) + config = configparser.ConfigParser(strict=False) cfgfile = open(state_file, 'w') @@ -666,10 +661,7 @@ class PatchController(PatchService): cfgfile.close() def read_state_file(self): - if six.PY2: - config = configparser.ConfigParser() - elif six.PY3: - config = configparser.ConfigParser(strict=False) + config = configparser.ConfigParser(strict=False) config.read(state_file) @@ -839,7 +831,7 @@ class PatchController(PatchService): Deletes the restart script (if any) associated with the patch :param patch_id: The patch ID ''' - if not self.patch_data.metadata[patch_id]["restart_script"]: + if not self.patch_data.metadata[patch_id].get("restart_script"): return restart_script_path = "%s/%s" % (root_scripts_dir, self.patch_data.metadata[patch_id]["restart_script"]) @@ -1965,7 +1957,7 @@ class PatchController(PatchService): for patch_id in self.patch_data.metadata: if (self.patch_data.metadata[patch_id]["patchstate"] in [constants.PARTIAL_APPLY, constants.PARTIAL_REMOVE]) \ - and self.patch_data.metadata[patch_id]["restart_script"]: + and self.patch_data.metadata[patch_id].get("restart_script"): try: restart_script_name = self.patch_data.metadata[patch_id]["restart_script"] restart_script_path = "%s/%s" \ @@ -1983,7 +1975,7 @@ class PatchController(PatchService): msg = "Failed to copy the restart script for %s" % patch_id LOG.exception(msg) raise PatchError(msg) - elif self.patch_data.metadata[patch_id]["restart_script"]: + elif self.patch_data.metadata[patch_id].get("restart_script"): try: restart_script_name = self.patch_data.metadata[patch_id]["restart_script"] restart_script_path = "%s/%s" \ @@ -2205,7 +2197,7 @@ class PatchController(PatchService): prefix=app_dependency_basename, dir=constants.PATCH_STORAGE_DIR) - os.write(tmpfile, json.dumps(self.app_dependencies)) + os.write(tmpfile, json.dumps(self.app_dependencies).encode()) os.close(tmpfile) os.rename(tmpfname, app_dependency_filename) diff --git a/sw-patch/cgcs-patch/cgcs_patch/patch_functions.py b/sw-patch/cgcs-patch/cgcs_patch/patch_functions.py index d5bad1be..0940a5ed 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/patch_functions.py +++ b/sw-patch/cgcs-patch/cgcs_patch/patch_functions.py @@ -854,7 +854,8 @@ class PatchFile(object): shutil.move("software.tar", "%s/%s-software.tar" % (abs_ostree_tar_dir, patch_id)) - if thispatch.metadata[patch_id]["restart_script"]: + # restart_script may not exist in metadata. + if thispatch.metadata[patch_id].get("restart_script"): if not os.path.exists(root_scripts_dir): os.makedirs(root_scripts_dir) restart_script_name = thispatch.metadata[patch_id]["restart_script"] diff --git a/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_controller.py b/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_controller.py index d5f2a419..fa28886f 100644 --- a/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_controller.py +++ b/sw-patch/cgcs-patch/cgcs_patch/tests/test_patch_controller.py @@ -12,7 +12,7 @@ from cgcs_patch.patch_controller import PatchController class CgcsPatchControllerTestCase(testtools.TestCase): - @mock.patch('six.moves.builtins.open') + @mock.patch('builtins.open') def test_cgcs_patch_controller_instantiate(self, _mock_open): pc = PatchController() self.assertIsNotNone(pc) diff --git a/sw-patch/cgcs-patch/requirements.txt b/sw-patch/cgcs-patch/requirements.txt index 3d5af389..38555d04 100644 --- a/sw-patch/cgcs-patch/requirements.txt +++ b/sw-patch/cgcs-patch/requirements.txt @@ -3,9 +3,10 @@ # process, which may cause wedges in the gate later. keystonemiddleware -oslo_config +lxml +oslo.config +netaddr pecan pycryptodomex -lxml requests_toolbelt sh diff --git a/sw-patch/cgcs-patch/tox.ini b/sw-patch/cgcs-patch/tox.ini index 5d325553..1a77cf55 100644 --- a/sw-patch/cgcs-patch/tox.ini +++ b/sw-patch/cgcs-patch/tox.ini @@ -5,7 +5,7 @@ # [tox] -envlist = pep8,py36,py39,pylint,bandit,cover +envlist = pep8,py39,pylint,bandit,cover minversion = 2.3.2 skipsdist = True diff --git a/sw-patch/debian/deb_folder/control b/sw-patch/debian/deb_folder/control index a90d7b74..71adfdc8 100644 --- a/sw-patch/debian/deb_folder/control +++ b/sw-patch/debian/deb_folder/control @@ -2,51 +2,58 @@ Source: cgcs-patch Section: admin Priority: optional Maintainer: StarlingX Developers -Build-Depends: debhelper-compat (= 13), dh-python, python3-setuptools, python3-all -Build-Depends-Indep: python3-keystonemiddleware, +Build-Depends: debhelper-compat (= 13), + dh-python, + python3-all, + python3-setuptools +Build-Depends-Indep: + python3-keystonemiddleware, + python3-lxml, + python3-mock, + python3-netaddr, python3-oslo.config, python3-pecan, python3-pycryptodome, - python3-lxml, python3-requests-toolbelt, - python3-mock, + python3-sh, python3-stestr, python3-testtools, - python3-six, - tsconfig, - python3-sh -Standards-Version: 4.4.1 + tsconfig +Standards-Version: 4.5.1 +Homepage: https://www.starlingx.io +Rules-Requires-Root: no Package: cgcs-patch Architecture: all -Depends: ${misc:Depends}, ${python3:Depends}, python3-cgcs-patch -Description: Starlingx platform patching - Starlingx platform patching system +Depends: python3-cgcs-patch +Description: StarlingX platform patching + StarlingX platform patching system Package: cgcs-patch-controller Architecture: all -Depends: ${misc:Depends}, ${python3:Depends}, python3-cgcs-patch, cgcs-patch -Description: Starlingx platform patching - Starlingx platform patching system +Depends: cgcs-patch +Description: StarlingX platform patching controller + StarlingX platform patching system controller Package: cgcs-patch-agent Architecture: all -Depends: ${misc:Depends}, ${python3:Depends}, python3-cgcs-patch, cgcs-patch -Description: Starlingx platform patching - Starlingx platform patching system +Depends: cgcs-patch +Description: StarlingX platform patching agent + StarlingX platform patching system agent Package: python3-cgcs-patch Architecture: all -Depends: ${python3:Depends}, +Depends: ${misc:Depends}, + ${python3:Depends}, python3-keystonemiddleware, + python3-lxml, python3-oslo.config, + python3-netaddr, python3-pecan, python3-pycryptodome, - python3-lxml, python3-requests-toolbelt, - python3-six, - tsconfig, - python3-sh -Description: Starlingx platfom patching (python3) - Starlingx platform patching system python libraries + python3-sh, + tsconfig +Description: StarlingX platfom patching (python3) + StarlingX platform patching system python libraries diff --git a/sw-patch/debian/deb_folder/rules b/sw-patch/debian/deb_folder/rules index 4bebe172..56442eb1 100755 --- a/sw-patch/debian/deb_folder/rules +++ b/sw-patch/debian/deb_folder/rules @@ -13,6 +13,9 @@ override_dh_auto_test: override_dh_auto_install: echo +override_dh_auto_clean: + python3 setup.py clean + override_dh_install: python3 setup.py install -f --install-layout=deb --root=${DEBIAN_DESTDIR} diff --git a/tox.ini b/tox.ini index dd12ec9d..40d0fccd 100644 --- a/tox.ini +++ b/tox.ini @@ -147,6 +147,7 @@ commands = {[testenv]commands} # 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 +# B322: Blacklist call to input (this is safe on python3) # B404: Import of subprocess module # B405: import xml.etree # B408: import xml.minidom @@ -155,7 +156,7 @@ commands = {[testenv]commands} # 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,B404,B405,B408,B413,B506,B602,B603,B607 +skips = B101,B104,B110,B303,B311,B314,B318,B322,B404,B405,B408,B413,B506,B602,B603,B607 exclude = tests [testenv:bandit]