CASMPET-6723 catch UnicodeDecodeError from os/run_command

libcsm/bss/api.py

@@ -58,11 +58,11 @@ def get_bss_bootparams(self, xname: str) -> str:
                                     data=json.dumps(body))
         except requests.exceptions.RequestException as ex:
             raise requests.exceptions.RequestException(f'ERROR exception: \
-                {type(ex).__name__} when trying to get bootparameters')
+{type(ex).__name__} when trying to get bootparameters')
         if bss_response.status_code != http.HTTPStatus.OK:
             raise requests.exceptions.RequestException(f'ERROR Failed to get BSS \
-                bootparameters for {xname}. Recieved http response:\
-                {bss_response.status_code} from  BSS.')
+bootparameters for {xname}. Recieved http response:\
+{bss_response.status_code} from  BSS.')
         return bss_response.json()[0]
 
 
@@ -77,11 +77,11 @@ def patch_bss_bootparams(self, xname : str, bss_json) -> None:
                                 data=json.dumps(bss_json))
         except requests.exceptions.RequestException as ex:
             raise requests.exceptions.RequestException(f'ERROR exception: \
-                {type(ex).__name__} when trying to patch bootparameters')
+{type(ex).__name__} when trying to patch bootparameters')
         if patch_response.status_code != http.HTTPStatus.OK:
             raise requests.exceptions.RequestException(f'ERROR Failed to patch BSS \
-                bootparameters for {xname}. Recieved {patch_response.status_code} \
-                from as BSS response.')
+bootparameters for {xname}. Recieved {patch_response.status_code} \
+from as BSS response.')
         print('BSS entry patched')
 
     def set_bss_image(self, xname: str, image_dict: dict) -> None:
@@ -94,13 +94,13 @@ def set_bss_image(self, xname: str, image_dict: dict) -> None:
 
         if 'initrd' not in image_dict or 'kernel' not in image_dict or 'rootfs' not in image_dict:
             raise ValueError(f"ERROR set_bss_image has inputs 'xname' and 'image_dictonary' where \
-            'image_dictionary' is a dictionary containing values for 'initrd', 'kernel', and \
-            'rootfs'. The inputs recieved were xname:{xname}, image_dictionary:{image_dict}")
+'image_dictionary' is a dictionary containing values for 'initrd', 'kernel', and \
+'rootfs'. The inputs recieved were xname:{xname}, image_dictionary:{image_dict}")
 
         bss_json = self.get_bss_bootparams(xname)
         if 'initrd' not in bss_json or 'kernel' not in bss_json:
             raise KeyError(f"BSS bootparams did not have the expected keys 'initrd' or 'kernel'. \
-            Boot parameters recieved: {bss_json}")
+Boot parameters recieved: {bss_json}")
         # set new images
         bss_json['initrd'] = image_dict['initrd']
         bss_json['kernel'] = image_dict['kernel']
@@ -109,7 +109,7 @@ def set_bss_image(self, xname: str, image_dict: dict) -> None:
             current_rootfs = params.split("metal.server=", 1)[1].split(" ",1)[0]
         except Exception as exc:
             raise KeyError(f"ERROR could not find current metal.server image in {xname} \
-                bss params") from exc
+bss params") from exc
 
         bss_json['params'] = params.replace(current_rootfs, image_dict['rootfs'])
 

libcsm/hsm/components.py

@@ -41,7 +41,7 @@ def get_components(role_subrole: str, api_gateway_address="api-gw-service-nmn.lo
     auth.refresh_token()
     session = get_session()
     hsm_components_url = f'https://{api_gateway_address}/\
-        apis/smd/hsm/v2/State/Components'
+apis/smd/hsm/v2/State/Components'
     # get components
     if role_subrole not in ROLE_SUBROLES:
         raise KeyError(f'ERROR {role_subrole} is not a valid role_subrole')
@@ -52,8 +52,8 @@ def get_components(role_subrole: str, api_gateway_address="api-gw-service-nmn.lo
             headers={'Authorization': f'Bearer {auth.token}'})
     except requests.exceptions.RequestException as ex:
         raise requests.exceptions.RequestException(f'ERROR exception: \
-            {type(ex).__name__} when trying to get components')
+{type(ex).__name__} when trying to get components')
     if components_response.status_code != http.HTTPStatus.OK:
         raise requests.exceptions.RequestException(f'ERROR Failed \
-            to get components with subrole {subrole}')
+to get components with subrole {subrole}')
     return components_response

libcsm/os.py

@@ -86,9 +86,15 @@ def _run(self) -> None:
             self._return_code = error.errno
             LOG.error('Could not find command for given args: %s', self.args)
         else:
-            self._stdout = stdout.decode('utf8')
-            self._stderr = stderr.decode('utf8')
-            self._return_code = command.returncode
+            try:
+                self._stdout = stdout.decode('utf8')
+                self._stderr = stderr.decode('utf8')
+                self._return_code = command.returncode
+            except UnicodeDecodeError as error:
+                self._stderr = error
+                self._return_code = 1
+                LOG.error('Could not decode stdout or stderr recieved from given args: %s. \
+stdout: %s, stderr %s', self.args, stdout, stderr)
         self._duration = time() - start_time
         if self._return_code and self._duration:
             LOG.info(

libcsm/sls/api.py

@@ -55,10 +55,10 @@ def get_management_components_from_sls(self) -> requests.Response:
                 headers={'Authorization': f'Bearer {self._auth.token}'})
         except requests.exceptions.RequestException as ex:
             raise requests.exceptions.RequestException(f'ERROR exception: {type(ex).__name__} \
-                when trying to get management components from SLS') from ex
+when trying to get management components from SLS') from ex
         if components_response.status_code != http.HTTPStatus.OK:
             raise requests.exceptions.RequestException(f'ERROR Bad response \
-                recieved from SLS. Recived: {components_response}')
+recieved from SLS. Recived: {components_response}')
 
         return components_response
 
@@ -74,8 +74,8 @@ def get_xname(self, hostname: str) -> str:
                     return node['Xname']
             except KeyError as error:
                 raise KeyError(f'ERROR [ExtraProperties][Aliases] was not in the \
-                response from sls. These fields are expected in the json response. \
-                The resonponse was {components_response.json()}') from error
+response from sls. These fields are expected in the json response. \
+The resonponse was {components_response.json()}') from error
         raise ValueError(f'ERROR hostname:{hostname} was not found in management nodes.')
 
     def get_hostname(self, xname: str) -> str:
@@ -91,6 +91,6 @@ def get_hostname(self, xname: str) -> str:
                     return node['ExtraProperties']['Aliases'][0]
             except KeyError as error:
                 raise KeyError(f'ERROR [ExtraProperties][Aliases] was not in the \
-                    response from sls. These fields are expected in the json response. \
-                    The resonponse was {components_response.json()}') from error
+response from sls. These fields are expected in the json response. \
+The resonponse was {components_response.json()}') from error
         raise ValueError(f'ERROR xname:{xname} was not found in management nodes.')

libcsm/tests/test_os.py

@@ -25,6 +25,8 @@
 Tests for the ``os`` module.
 """
 from os import getcwd
+from subprocess import Popen
+import mock
 
 from libcsm.os import run_command
 from libcsm.os import chdir
@@ -53,6 +55,19 @@ def test_run_command(self) -> None:
             assert isinstance(result.return_code, int)
             assert result.stdout or result.stderr
 
+    def test_run_command_unicode_error(self) -> None:
+        """
+        Assert that UnicodeDecodeErrors are caught by the run_command 
+        and a non-zero return code is recieved.
+        """
+        # test with an invalid utf-8 byte
+        byte_string = b'\x9c'
+        with mock.patch.object(Popen, 'communicate', return_value=(byte_string, '')):
+            result = run_command(['ls','-l'])
+            assert isinstance(result.stderr, UnicodeDecodeError)
+            assert result.return_code == 1
+            assert result.stdout or result.stderr
+
     def test_chdir(self) -> None:
         """
         Assert that our context manager will change directories and