diff --git a/gitopscli/__main__.py b/gitopscli/__main__.py index e1d57d8c..ca17cb45 100644 --- a/gitopscli/__main__.py +++ b/gitopscli/__main__.py @@ -17,6 +17,7 @@ def main() -> None: logging.exception(ex) else: logging.error(ex) + logging.error("Provide verbose flag '-v' for more error details...") sys.exit(1) diff --git a/gitopscli/cliparser.py b/gitopscli/cliparser.py index de0a37c1..44728213 100644 --- a/gitopscli/cliparser.py +++ b/gitopscli/cliparser.py @@ -13,7 +13,7 @@ VersionCommand, ) from gitopscli.git import GitProvider -from gitopscli.io.yaml_util import yaml_load +from gitopscli.io.yaml_util import yaml_load, YAMLException def parse_args(raw_args: List[str]) -> Tuple[bool, CommandArgs]: @@ -69,7 +69,7 @@ def __create_deploy_parser() -> ArgumentParser: parser.add_argument( "--values", help="YAML/JSON object with the YAML path as key and the desired value as value", - type=yaml_load, + type=__parse_yaml, required=True, ) parser.add_argument( @@ -236,6 +236,13 @@ def __parse_bool(value: str) -> bool: raise ArgumentTypeError(f"invalid bool value: '{value}'") +def __parse_yaml(value: str) -> Any: + try: + return yaml_load(value) + except YAMLException as ex: + raise ArgumentTypeError(f"invalid YAML value: '{value}'") from ex + + def __parse_git_provider(value: str) -> GitProvider: mapping = {"github": GitProvider.GITHUB, "bitbucket-server": GitProvider.BITBUCKET} assert set(mapping.values()) == set(GitProvider), "git provider mapping not exhaustive" diff --git a/gitopscli/commands/create_preview.py b/gitopscli/commands/create_preview.py index bb44ab84..60ec265f 100644 --- a/gitopscli/commands/create_preview.py +++ b/gitopscli/commands/create_preview.py @@ -4,7 +4,7 @@ from dataclasses import dataclass from typing import Any, Callable, Dict from gitopscli.git import GitApiConfig, GitRepo, GitRepoApi, GitRepoApiFactory -from gitopscli.io.yaml_util import update_yaml_file +from gitopscli.io.yaml_util import update_yaml_file, YAMLException from gitopscli.gitops_config import GitOpsConfig from gitopscli.gitops_exception import GitOpsException from .common import load_gitops_config @@ -126,5 +126,9 @@ def __update_yaml_file(git_repo: GitRepo, file_path: str, key: str, value: Any) full_file_path = git_repo.get_full_file_path(file_path) try: return update_yaml_file(full_file_path, key, value) + except (FileNotFoundError, IsADirectoryError) as ex: + raise GitOpsException(f"No such file: {file_path}") from ex + except YAMLException as ex: + raise GitOpsException(f"Error loading file: {file_path}") from ex except KeyError as ex: - raise GitOpsException(f"Key '{key}' not found in '{file_path}'") from ex + raise GitOpsException(f"Key '{key}' not found in file: {file_path}") from ex diff --git a/gitopscli/commands/deploy.py b/gitopscli/commands/deploy.py index 4c801c58..d7e7b7f2 100644 --- a/gitopscli/commands/deploy.py +++ b/gitopscli/commands/deploy.py @@ -3,7 +3,7 @@ from dataclasses import dataclass from typing import Any, Dict, Optional, Tuple from gitopscli.git import GitApiConfig, GitRepo, GitRepoApi, GitRepoApiFactory -from gitopscli.io.yaml_util import update_yaml_file, yaml_dump +from gitopscli.io.yaml_util import update_yaml_file, yaml_dump, YAMLException from gitopscli.gitops_exception import GitOpsException from .command import Command @@ -67,8 +67,10 @@ def __update_values(self, git_repo: GitRepo) -> Dict[str, Any]: updated_value = update_yaml_file(full_file_path, key, value) except (FileNotFoundError, IsADirectoryError) as ex: raise GitOpsException(f"No such file: {args.file}") from ex + except YAMLException as ex: + raise GitOpsException(f"Error loading file: {args.file}") from ex except KeyError as ex: - raise GitOpsException(f"Key '{key}' not found in {args.file}") from ex + raise GitOpsException(f"Key '{key}' not found in file: {args.file}") from ex if not updated_value: logging.info("Yaml property %s already up-to-date", key) diff --git a/gitopscli/git/git_repo.py b/gitopscli/git/git_repo.py index 5602ecc4..ee2bad92 100644 --- a/gitopscli/git/git_repo.py +++ b/gitopscli/git/git_repo.py @@ -1,8 +1,7 @@ import os import logging from types import TracebackType -from typing import Optional, Type -from typing_extensions import Literal +from typing import Optional, Type, Literal from git import Repo, GitError, GitCommandError from gitopscli.gitops_exception import GitOpsException from gitopscli.io.tmp_dir import create_tmp_dir, delete_tmp_dir diff --git a/gitopscli/io/yaml_util.py b/gitopscli/io/yaml_util.py index 21587a0f..b2b4de10 100644 --- a/gitopscli/io/yaml_util.py +++ b/gitopscli/io/yaml_util.py @@ -1,14 +1,22 @@ import re from io import StringIO from typing import Any -from ruamel.yaml import YAML +from ruamel.yaml import YAML, YAMLError -ARRAY_KEY_SEGMENT_PATTERN = re.compile(r"\[(\d+)\]") + +_ARRAY_KEY_SEGMENT_PATTERN = re.compile(r"\[(\d+)\]") + + +class YAMLException(Exception): + pass def yaml_file_load(file_path: str) -> Any: with open(file_path, "r") as stream: - return YAML().load(stream) + try: + return YAML().load(stream) + except YAMLError as ex: + raise YAMLException(f"Error parsing YAML file: {file_path}") from ex def yaml_file_dump(yaml: Any, file_path: str) -> None: @@ -17,7 +25,10 @@ def yaml_file_dump(yaml: Any, file_path: str) -> None: def yaml_load(yaml_str: str) -> Any: - return YAML().load(yaml_str) + try: + return YAML().load(yaml_str) + except YAMLError as ex: + raise YAMLException(f"Error parsing YAML string '{yaml_str}'") from ex def yaml_dump(yaml: Any) -> str: @@ -35,7 +46,7 @@ def update_yaml_file(file_path: str, key: str, value: Any) -> bool: for current_key_segment in key_segments: current_key_segments.append(current_key_segment) current_key = ".".join(current_key_segments) - is_array = ARRAY_KEY_SEGMENT_PATTERN.match(current_key_segment) + is_array = _ARRAY_KEY_SEGMENT_PATTERN.match(current_key_segment) if is_array: current_array_index = int(is_array.group(1)) if not isinstance(parent_item, list) or current_array_index >= len(parent_item): diff --git a/tests/commands/test_create_preview.py b/tests/commands/test_create_preview.py index 8efa55a6..46909d82 100644 --- a/tests/commands/test_create_preview.py +++ b/tests/commands/test_create_preview.py @@ -3,7 +3,7 @@ import shutil import logging from unittest.mock import call, Mock -from gitopscli.io.yaml_util import update_yaml_file +from gitopscli.io.yaml_util import update_yaml_file, YAMLException from gitopscli.git import GitRepo, GitRepoApi, GitRepoApiFactory, GitProvider from gitopscli.gitops_config import GitOpsConfig from gitopscli.gitops_exception import GitOpsException @@ -241,6 +241,56 @@ def test_create_preview_for_unknown_template(self): call.os.path.isdir("/tmp/created-tmp-dir/.preview-templates/my-app"), ] + def test_create_preview_values_yaml_not_found(self): + self.update_yaml_file_mock.side_effect = FileNotFoundError() + + try: + CreatePreviewCommand(ARGS).execute() + self.fail() + except GitOpsException as ex: + self.assertEqual("No such file: my-app-685912d3-preview/values.yaml", str(ex)) + + assert self.mock_manager.method_calls == [ + call.load_gitops_config(ARGS, "ORGA", "REPO",), + call.GitRepoApiFactory.create(ARGS, "TEAM_CONFIG_ORG", "TEAM_CONFIG_REPO",), + call.GitRepo(self.git_repo_api_mock), + call.GitRepo.checkout("master"), + call.GitRepo.get_full_file_path("my-app-685912d3-preview"), + call.os.path.isdir("/tmp/created-tmp-dir/my-app-685912d3-preview"), + call.logging.info("Use existing folder for preview: %s", "my-app-685912d3-preview"), + call.GitRepo.get_full_file_path("my-app-685912d3-preview/values.yaml"), + call.update_yaml_file( + "/tmp/created-tmp-dir/my-app-685912d3-preview/values.yaml", + "image.tag", + "3361723dbd91fcfae7b5b8b8b7d462fbc14187a9", + ), + ] + + def test_create_preview_values_yaml_parse_error(self): + self.update_yaml_file_mock.side_effect = YAMLException() + + try: + CreatePreviewCommand(ARGS).execute() + self.fail() + except GitOpsException as ex: + self.assertEqual("Error loading file: my-app-685912d3-preview/values.yaml", str(ex)) + + assert self.mock_manager.method_calls == [ + call.load_gitops_config(ARGS, "ORGA", "REPO",), + call.GitRepoApiFactory.create(ARGS, "TEAM_CONFIG_ORG", "TEAM_CONFIG_REPO",), + call.GitRepo(self.git_repo_api_mock), + call.GitRepo.checkout("master"), + call.GitRepo.get_full_file_path("my-app-685912d3-preview"), + call.os.path.isdir("/tmp/created-tmp-dir/my-app-685912d3-preview"), + call.logging.info("Use existing folder for preview: %s", "my-app-685912d3-preview"), + call.GitRepo.get_full_file_path("my-app-685912d3-preview/values.yaml"), + call.update_yaml_file( + "/tmp/created-tmp-dir/my-app-685912d3-preview/values.yaml", + "image.tag", + "3361723dbd91fcfae7b5b8b8b7d462fbc14187a9", + ), + ] + def test_create_preview_with_invalid_replacement_path(self): self.update_yaml_file_mock.side_effect = KeyError() @@ -248,7 +298,7 @@ def test_create_preview_with_invalid_replacement_path(self): CreatePreviewCommand(ARGS).execute() self.fail() except GitOpsException as ex: - self.assertEqual("Key 'image.tag' not found in 'my-app-685912d3-preview/values.yaml'", str(ex)) + self.assertEqual("Key 'image.tag' not found in file: my-app-685912d3-preview/values.yaml", str(ex)) assert self.mock_manager.method_calls == [ call.load_gitops_config(ARGS, "ORGA", "REPO",), @@ -278,7 +328,7 @@ def test_create_new_preview_invalid_chart_template(self): CreatePreviewCommand(ARGS).execute() self.fail() except GitOpsException as ex: - self.assertEqual("Key 'name' not found in 'my-app-685912d3-preview/Chart.yaml'", str(ex)) + self.assertEqual("Key 'name' not found in file: my-app-685912d3-preview/Chart.yaml", str(ex)) assert self.mock_manager.method_calls == [ call.load_gitops_config(ARGS, "ORGA", "REPO",), diff --git a/tests/commands/test_deploy.py b/tests/commands/test_deploy.py index 339c6623..c91b3745 100644 --- a/tests/commands/test_deploy.py +++ b/tests/commands/test_deploy.py @@ -7,7 +7,7 @@ from gitopscli.gitops_exception import GitOpsException from gitopscli.commands.deploy import DeployCommand from gitopscli.git import GitRepoApi, GitProvider, GitRepoApiFactory, GitRepo -from gitopscli.io.yaml_util import update_yaml_file +from gitopscli.io.yaml_util import update_yaml_file, YAMLException from .mock_mixin import MockMixin @@ -356,6 +356,37 @@ def test_file_not_found(self): call.update_yaml_file("/tmp/created-tmp-dir/test/file.yml", "a.b.c", "foo"), ] + def test_file_parse_error(self): + self.update_yaml_file_mock.side_effect = YAMLException + + args = DeployCommand.Args( + file="test/file.yml", + values={"a.b.c": "foo", "a.b.d": "bar"}, + username="USERNAME", + password="PASSWORD", + git_user="GIT_USER", + git_email="GIT_EMAIL", + create_pr=False, + auto_merge=False, + single_commit=False, + organisation="ORGA", + repository_name="REPO", + git_provider=GitProvider.GITHUB, + git_provider_url=None, + commit_message=None, + ) + with pytest.raises(GitOpsException) as ex: + DeployCommand(args).execute() + self.assertEqual(str(ex.value), "Error loading file: test/file.yml") + + assert self.mock_manager.method_calls == [ + call.GitRepoApiFactory.create(args, "ORGA", "REPO"), + call.GitRepo(self.git_repo_api_mock), + call.GitRepo.checkout("master"), + call.GitRepo.get_full_file_path("test/file.yml"), + call.update_yaml_file("/tmp/created-tmp-dir/test/file.yml", "a.b.c", "foo"), + ] + def test_key_not_found(self): self.update_yaml_file_mock.side_effect = KeyError("dummy key error") @@ -377,7 +408,7 @@ def test_key_not_found(self): ) with pytest.raises(GitOpsException) as ex: DeployCommand(args).execute() - self.assertEqual(str(ex.value), "Key 'a.b.c' not found in test/file.yml") + self.assertEqual(str(ex.value), "Key 'a.b.c' not found in file: test/file.yml") assert self.mock_manager.method_calls == [ call.GitRepoApiFactory.create(args, "ORGA", "REPO"), diff --git a/tests/io/test_yaml_util.py b/tests/io/test_yaml_util.py index 615871b1..663986b3 100644 --- a/tests/io/test_yaml_util.py +++ b/tests/io/test_yaml_util.py @@ -9,6 +9,7 @@ yaml_file_dump, yaml_load, yaml_dump, + YAMLException, update_yaml_file, merge_yaml_element, ) @@ -43,11 +44,19 @@ def test_yaml_file_load(self): def test_yaml_file_load_file_not_found(self): try: - self.assertEqual(yaml_file_load("unknown"), {"answer": {"is": "42"}}) + yaml_file_load("unknown") self.fail() except FileNotFoundError: pass + def test_yaml_file_load_yaml_exception(self): + path = self._create_file("{ INVALID YAML") + try: + yaml_file_load(path) + self.fail() + except YAMLException as ex: + self.assertEqual(f"Error parsing YAML file: {path}", str(ex)) + def test_yaml_file_dump(self): path = self._create_tmp_file_path() yaml_file_dump({"answer": {"is": "42"}}, path) @@ -75,6 +84,13 @@ def test_yaml_load(self): self.assertEqual(yaml_load("{answer: 42}"), {"answer": 42}) self.assertEqual(yaml_load("answer: 42"), {"answer": 42}) + def test_yaml_load_yaml_exception(self): + try: + yaml_load("{ INVALID YAML") + self.fail() + except YAMLException as ex: + self.assertEqual("Error parsing YAML string '{ INVALID YAML'", str(ex)) + def test_yaml_dump(self): self.assertEqual(yaml_dump({"answer": "42"}), "answer: '42'") self.assertEqual(yaml_dump({"answer": 42}), "answer: 42") diff --git a/tests/test_cliparser.py b/tests/test_cliparser.py index d4310295..68b70e73 100644 --- a/tests/test_cliparser.py +++ b/tests/test_cliparser.py @@ -1180,6 +1180,37 @@ def test_invalid_int(self): "gitopscli add-pr-comment: error: argument --pr-id: invalid int value: 'INVALID_INT'", last_stderr_line ) + def test_invalid_yaml(self): + exit_code, stdout, stderr = self._capture_parse_args( + [ + "deploy", + "--git-provider", + "github", + "--username", + "x", + "--password", + "x", + "--organisation", + "x", + "--repository-name", + "x", + "--git-user", + "x", + "--git-email", + "x", + "--file", + "x", + "--values", + "{ INVALID YAML", + ] + ) + self.assertEqual(exit_code, 2) + self.assertEqual("", stdout) + last_stderr_line = stderr.splitlines()[-1] + self.assertEqual( + "gitopscli deploy: error: argument --values: invalid YAML value: '{ INVALID YAML'", last_stderr_line, + ) + def test_invalid_git_provider(self): exit_code, stdout, stderr = self._capture_parse_args( [