diff --git a/sonic_package_manager/service_creator/creator.py b/sonic_package_manager/service_creator/creator.py index 17993be83c..a22b454de6 100644 --- a/sonic_package_manager/service_creator/creator.py +++ b/sonic_package_manager/service_creator/creator.py @@ -2,10 +2,11 @@ import contextlib import os +import sys import stat import subprocess from collections import defaultdict -from typing import Dict, Type +from typing import Dict, Type, List import jinja2 as jinja2 from config.config_mgmt import ConfigMgmt @@ -96,22 +97,22 @@ def remove_if_exists(path): os.remove(path) log.info(f'removed {path}') +def is_list_of_strings(command): + return isinstance(command, list) and all(isinstance(item, str) for item in command) -def run_command(command: str): +def run_command(command: List[str]): """ Run arbitrary bash command. Args: - command: String command to execute as bash script + command: List of Strings command to execute as bash script Raises: ServiceCreatorError: Raised when the command return code is not 0. """ - + if not is_list_of_strings(command): + sys.exit("Input command should be a list of strings") log.debug(f'running command: {command}') - proc = subprocess.Popen(command, - shell=True, - executable='/bin/bash', - stdout=subprocess.PIPE) + proc = subprocess.Popen(command, stdout=subprocess.PIPE) (_, _) = proc.communicate() if proc.returncode != 0: raise ServiceCreatorError(f'Failed to execute "{command}"') @@ -647,4 +648,4 @@ def _post_operation_hook(self): """ Common operations executed after service is created/removed. """ if not in_chroot(): - run_command('systemctl daemon-reload') + run_command(['systemctl', 'daemon-reload']) diff --git a/tests/sonic_package_manager/test_service_creator.py b/tests/sonic_package_manager/test_service_creator.py index c97d362626..1ddbad6045 100644 --- a/tests/sonic_package_manager/test_service_creator.py +++ b/tests/sonic_package_manager/test_service_creator.py @@ -1,8 +1,9 @@ #!/usr/bin/env python import os +import sys import copy -from unittest.mock import Mock, MagicMock, call +from unittest.mock import Mock, MagicMock, call, patch import pytest @@ -62,6 +63,22 @@ def manifest(): ] }) +def test_is_list_of_strings(): + output = is_list_of_strings(['a', 'b', 'c']) + assert output is True + + output = is_list_of_strings('abc') + assert output is False + + output = is_list_of_strings(['a', 'b', 1]) + assert output is False + +def test_run_command(): + with pytest.raises(SystemExit) as e: + run_command('echo 1') + + with pytest.raises(ServiceCreatorError) as e: + run_command([sys.executable, "-c", "import sys; sys.exit(6)"]) @pytest.fixture() def service_creator(mock_feature_registry, @@ -203,6 +220,11 @@ def test_service_creator_autocli(sonic_fs, manifest, mock_cli_gen, any_order=True ) +def test_service_creator_post_operation_hook(sonic_fs, manifest, mock_sonic_db, mock_config_mgmt, service_creator): + with patch('sonic_package_manager.service_creator.creator.run_command') as run_command: + with patch('sonic_package_manager.service_creator.creator.in_chroot', MagicMock(return_value=False)): + service_creator._post_operation_hook() + run_command.assert_called_with(['systemctl', 'daemon-reload']) def test_feature_registration(mock_sonic_db, manifest): mock_connector = Mock()