From 95ca2a42f14683ed2dee1a670a04bb02f5311cdf Mon Sep 17 00:00:00 2001 From: Justintime50 <39606064+Justintime50@users.noreply.github.com> Date: Mon, 9 Oct 2023 13:20:57 -0600 Subject: [PATCH] fix: ascii encoding for subprocess output (closes #81) --- CHANGELOG.md | 4 ++++ harvey/app.py | 13 +++++-------- harvey/deployments.py | 12 +++++------- harvey/git.py | 25 ++++++------------------- harvey/utils/utils.py | 15 +++++++++++++++ test/unit/test_deployments.py | 2 ++ test/unit/test_git.py | 2 ++ 7 files changed, 39 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e8dd4f..cde14c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # CHANGELOG +## Next Release + +- Fixes a bug that couldn't deserialize subprocess output correctly due to ascii encoding (closes #81) + ## v1.0.1 (2023-09-05) - Fixes a bug where `page_size` for the `/projects` endpoint wasn't respected diff --git a/harvey/app.py b/harvey/app.py index 9b0888f..3d8f3c7 100644 --- a/harvey/app.py +++ b/harvey/app.py @@ -1,5 +1,4 @@ import os -import subprocess # nosec import threading from typing import ( Any, @@ -34,7 +33,10 @@ from harvey.repos.locks import retrieve_locks from harvey.repos.projects import retrieve_projects from harvey.repos.webhooks import retrieve_webhook -from harvey.utils.utils import setup_logger +from harvey.utils.utils import ( + run_subprocess_command, + setup_logger, +) # Must remain at the top of this file to take effect, we load both from the path and locally @@ -60,12 +62,7 @@ def bootstrap() -> bool: debug = _get_debug_bool(Config.log_level) # Ensure the correct Docker Compose version is available - docker_compose_version = subprocess.check_output( # nosec - ['docker', 'compose', 'version'], - stderr=subprocess.STDOUT, - text=True, - timeout=3, - ) + docker_compose_version = run_subprocess_command(['docker', 'compose', 'version']) if REQUIRED_DOCKER_COMPOSE_VERSION not in docker_compose_version: raise HarveyError(f'Harvey requires Docker Compose {REQUIRED_DOCKER_COMPOSE_VERSION}.') diff --git a/harvey/deployments.py b/harvey/deployments.py index 48cc79f..c14ee09 100644 --- a/harvey/deployments.py +++ b/harvey/deployments.py @@ -24,7 +24,10 @@ kill_deployment, succeed_deployment, ) -from harvey.utils.utils import get_utc_timestamp +from harvey.utils.utils import ( + get_utc_timestamp, + run_subprocess_command, +) from harvey.webhooks import Webhook @@ -264,12 +267,7 @@ def deploy(config: Dict[str, Any], webhook: Dict[str, Any], output: str) -> str: # fmt: on try: - compose_output = subprocess.check_output( # nosec - compose_command, - stderr=subprocess.STDOUT, - text=True, - timeout=Config.operation_timeout, - ) + compose_output = run_subprocess_command(compose_command) execution_time = f'Deploy stage execution time: {get_utc_timestamp() - start_time}' final_output = f'{compose_output}\n{execution_time}' logger.info(final_output) diff --git a/harvey/git.py b/harvey/git.py index e153c70..813c5a8 100644 --- a/harvey/git.py +++ b/harvey/git.py @@ -3,13 +3,13 @@ from typing import ( Any, Dict, - List, ) import woodchips from harvey.config import Config from harvey.utils.deployments import kill_deployment +from harvey.utils.utils import run_subprocess_command from harvey.webhooks import Webhook @@ -33,8 +33,9 @@ def clone_repo(project_path: str, webhook: Dict[str, Any]) -> str: command_output = '' try: - command = ['git', 'clone', '--depth=1', Webhook.repo_url(webhook), project_path] - command_output = Git._git_subprocess(command) + command_output = run_subprocess_command( + ['git', 'clone', '--depth=1', Webhook.repo_url(webhook), project_path] + ) logger.debug(command_output) except subprocess.TimeoutExpired: kill_deployment( @@ -57,8 +58,7 @@ def pull_repo(project_path: str, webhook: Dict[str, Any], pull_attempt: int = 1) command_output = '' try: - command = ['git', '-C', project_path, 'pull', '--rebase'] - command_output = Git._git_subprocess(command) + command_output = run_subprocess_command(['git', '-C', project_path, 'pull', '--rebase']) logger.debug(f'{command_output}') except subprocess.TimeoutExpired: kill_deployment( @@ -72,8 +72,7 @@ def pull_repo(project_path: str, webhook: Dict[str, Any], pull_attempt: int = 1) logger.info(f'Attempting to stash {Webhook.repo_full_name(webhook)} before pulling again...') try: - command = ['git', '-C', project_path, 'stash'] - command_output = Git._git_subprocess(command) + command_output = run_subprocess_command(['git', '-C', project_path, 'stash']) logger.debug(f'{command_output}') except (subprocess.CalledProcessError, subprocess.TimeoutExpired): kill_deployment( @@ -88,15 +87,3 @@ def pull_repo(project_path: str, webhook: Dict[str, Any], pull_attempt: int = 1) command_output = Git.pull_repo(project_path, webhook, pull_attempt) return command_output - - @staticmethod - def _git_subprocess(command: List[str]) -> str: - """Runs a git command via subprocess.""" - command_output = subprocess.check_output( # nosec - command, - stderr=subprocess.STDOUT, - text=True, - timeout=Config.operation_timeout, - ) - - return command_output diff --git a/harvey/utils/utils.py b/harvey/utils/utils.py index eefbbe5..cd8a9c7 100644 --- a/harvey/utils/utils.py +++ b/harvey/utils/utils.py @@ -1,4 +1,6 @@ import datetime +import subprocess # nosec +from typing import List import woodchips @@ -26,3 +28,16 @@ def setup_logger(): def get_utc_timestamp(): """Returns the UTC timestamp of right now.""" return datetime.datetime.now(datetime.timezone.utc) + + +def run_subprocess_command(command: List[str]) -> str: + """Runs a shell command via subprocess.""" + command_output = subprocess.check_output( # nosec + command, + stderr=subprocess.STDOUT, + text=True, + encoding='utf8', + timeout=Config.operation_timeout, + ) + + return command_output diff --git a/test/unit/test_deployments.py b/test/unit/test_deployments.py index 9707df0..71f3cb7 100644 --- a/test/unit/test_deployments.py +++ b/test/unit/test_deployments.py @@ -162,6 +162,7 @@ def test_deploy_stage_success(mock_subprocess, mock_healthcheck, mock_path_exist ], stderr=-2, text=True, + encoding='utf8', timeout=300, ) @@ -209,5 +210,6 @@ def test_deploy_stage_prod_compose_success(mock_subprocess, mock_healthcheck, mo ], stderr=-2, text=True, + encoding='utf8', timeout=300, ) diff --git a/test/unit/test_git.py b/test/unit/test_git.py index 8800042..e95fd3a 100644 --- a/test/unit/test_git.py +++ b/test/unit/test_git.py @@ -31,6 +31,7 @@ def test_clone_repo(mock_subprocess, mock_logger, mock_project_path, mock_webhoo ['git', 'clone', '--depth=1', 'https://test-ssh-url.com', 'harvey/projects/test_user/test-repo-name'], stderr=-2, text=True, + encoding='utf8', timeout=300, ) @@ -63,6 +64,7 @@ def test_pull_repo(mock_subprocess, mock_logger, mock_project_path, mock_webhook ['git', '-C', 'harvey/projects/test_user/test-repo-name', 'pull', '--rebase'], stderr=-2, text=True, + encoding='utf8', timeout=300, )