Merge pull request #137 from baloise/fixes

fix: some minor fixes and logging improvements

Author: christiansiegel

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)
 
 

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"

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

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)

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

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):

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,14 +241,64 @@ 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()
 
         try:
             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",),

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"),

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")

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(
             [