Merge pull request #115 from shanejbrown/main

Add check and tests for automatic buildrunner id image tag

buildrunner/__init__.py

@@ -237,7 +237,9 @@ def __init__(
         if not _run_config_file or not os.path.exists(_run_config_file):
             raise BuildRunnerConfigurationError("Cannot find build configuration file")
 
-        self.run_config = self.global_config.load_config(_run_config_file)
+        self.run_config = self.global_config.load_config(
+            cfg_file=_run_config_file, default_tag=sanitize_tag(self.build_id)
+        )
 
         if not isinstance(self.run_config, dict) or "steps" not in self.run_config:
             cfg_file = _run_config_file if _run_config_file else "provided config"

buildrunner/config.py

@@ -19,7 +19,7 @@
 import re
 import sys
 import tempfile
-from typing import Optional
+from typing import Optional, Union
 
 import jinja2
 
@@ -327,7 +327,70 @@ def _load_config_files(self, cfg_files=None, ctx=None, log_file=True):
 
         return context
 
-    def load_config(self, cfg_file, ctx=None, log_file=True):
+    def _set_default_tag(self, config, default_tag) -> dict:
+        """
+        Set default tag if not set for each image
+
+        Args:
+            config (dict):  configuration
+            default_tag (str): default tag
+
+            Returns:
+                dict: configuration
+        """
+
+        def add_default_tag_to_tags(config: Union[str, dict], default_tag: str):
+            # Add default tag to tags list if not in the list already
+            if isinstance(config, dict):
+                tags = config.get("tags")
+                if not tags:
+                    tags = []
+                assert isinstance(tags, list)
+                if default_tag not in tags:
+                    tags.append(default_tag)
+
+            # Convert to dictionary and add default tag if not listed already
+            elif isinstance(config, str):
+                image_name = config.split(":")
+                step_config = dict()
+                step_config["repository"] = image_name[0]
+                step_config["tags"] = []
+
+                # Check if image name has defined a tag, if so add it to tags
+                if len(image_name) > 1:
+                    tag = image_name[1]
+                    assert tag
+                    step_config["tags"].append(tag)
+
+                # Add default tag if not in the list already
+                if default_tag not in step_config["tags"]:
+                    step_config["tags"].append(default_tag)
+
+                config = step_config
+            return config
+
+        steps = config.get("steps")
+        if isinstance(steps, dict):
+            for step_name, step in steps.items():
+                for substep_name, substep in step.items():
+                    if substep_name in ["push", "commit"]:
+                        # Add default tag to tags list if not in the list
+                        if isinstance(substep, list):
+                            curr_image_infos = []
+                            for push_config in substep:
+                                curr_image_infos.append(
+                                    add_default_tag_to_tags(push_config, default_tag)
+                                )
+                            config["steps"][step_name][substep_name] = curr_image_infos
+                        else:
+                            curr_image_info = add_default_tag_to_tags(
+                                substep, default_tag
+                            )
+                            config["steps"][step_name][substep_name] = curr_image_info
+
+        return config
+
+    def load_config(self, cfg_file, ctx=None, log_file=True, default_tag=None) -> dict:
         """
         Load a config file templating it with Jinja and parsing the YAML.
 
@@ -396,6 +459,9 @@ def load_config(self, cfg_file, ctx=None, log_file=True):
 
             config = self._reorder_dependency_steps(config)
 
+            # Always add default tag if not set
+            config = self._set_default_tag(config, default_tag)
+
             errors = validate_config(**config)
             if errors:
                 raise BuildRunnerConfigurationError(

buildrunner/steprunner/tasks/push.py

@@ -101,10 +101,6 @@ def __init__(self, step_runner, config, commit_only=False):
             self._repos = [self._get_repo_definition(config)]
 
     def run(self, context):  # pylint: disable=too-many-branches
-        # Determine internal tag based on source control information and build number
-        # Note: do not log sanitization as it would need to happen almost every time
-        default_tag = sanitize_tag(self.step_runner.build_runner.build_id)
-
         # Tag multi-platform images
         built_image = context.get("mp_built_image")
         if built_image:
@@ -118,8 +114,6 @@ def run(self, context):  # pylint: disable=too-many-branches
             ]
 
             for repo in self._repos:
-                # Always add default tag and then add it to the built image info
-                repo.tags.append(default_tag)
                 tagged_image = built_image.add_tagged_image(repo.repository, repo.tags)
 
                 # Add tagged image refs to committed images for use in determining if pull should be true/false
@@ -166,9 +160,6 @@ def run(self, context):  # pylint: disable=too-many-branches
             self.step_runner.build_runner.generated_images.append(image_to_use)
 
             for repo in self._repos:
-                # Always add default tag
-                repo.tags.append(default_tag)
-
                 if self._commit_only:
                     self.step_runner.log.write(
                         f'Committing resulting image as "{repo.repository}" with tags {", ".join(repo.tags)}.\n'

tests/test_config_validation/test_retagging.py

@@ -1,7 +1,17 @@
+import os
+import tempfile
+from unittest import mock
+import pytest
 import yaml
+from buildrunner import BuildRunner
+from buildrunner.errors import BuildRunnerConfigurationError
 from buildrunner.validation.config import validate_config, Errors, RETAG_ERROR_MESSAGE
 
 
+TEST_DIR = os.path.dirname(os.path.abspath(__file__))
+BLANK_GLOBAL_CONFIG = os.path.join(TEST_DIR, "files/blank_global_config.yaml")
+
+
 def test_invalid_multiplatform_retagging_with_push():
     # Retagging a multiplatform image is not supported
     config_yaml = """
@@ -231,3 +241,155 @@ def test_reusing_multi_platform_images():
     config = yaml.load(config_yaml, Loader=yaml.Loader)
     errors = validate_config(**config)
     assert errors is None
+
+
+@mock.patch("buildrunner.config.DEFAULT_GLOBAL_CONFIG_FILES", [])
+@mock.patch("buildrunner.detect_vcs")
+def test_valid_config_with_buildrunner_build_tag(detect_vcs_mock):
+    # Tests that "BUILDRUNNER_BUILD_DOCKER_TAG" is replaced with id_string-build_number
+
+    config_yaml = """
+    steps:
+        build-container-multi-platform:
+            build:
+                dockerfile: |
+                    FROM {{ DOCKER_REGISTRY }}/busybox
+                platforms:
+                    - linux/amd64
+                    - linux/arm64/v8
+            push:
+                - repository: user1/buildrunner-test-multi-platform
+                  tags: [ 'latest', '0.0.1', {{ BUILDRUNNER_BUILD_DOCKER_TAG }} ]
+
+        use-built-image1:
+            run:
+                image: user1/buildrunner-test-multi-platform
+                cmd: echo "Hello World"
+    """
+
+    id_string = "main-921.ie02ed8.m1705616822"
+    build_number = 342
+    type(detect_vcs_mock.return_value).id_string = mock.PropertyMock(
+        return_value=id_string
+    )
+
+    with tempfile.TemporaryDirectory() as tmpdirname:
+        with tempfile.NamedTemporaryFile(
+            dir=tmpdirname, mode="w", delete=False
+        ) as tmpfile:
+            tmpfile.write(config_yaml)
+            tmpfile.close()
+            try:
+                runner = BuildRunner(
+                    build_dir=tmpdirname,
+                    build_results_dir=tmpdirname,
+                    global_config_file=None,
+                    run_config_file=tmpfile.name,
+                    build_time=0,
+                    build_number=build_number,
+                    push=False,
+                    cleanup_images=False,
+                    cleanup_cache=False,
+                    steps_to_run=None,
+                    publish_ports=False,
+                    log_generated_files=False,
+                    docker_timeout=30,
+                    local_images=False,
+                    platform=None,
+                    disable_multi_platform=False,
+                )
+                config = runner.run_config
+                assert isinstance(config, dict)
+                assert f"{id_string}-{build_number}" in config.get("steps").get(
+                    "build-container-multi-platform"
+                ).get("push")[0].get("tags")
+            except Exception as e:
+                assert False, f"Unexpected exception raised: {e}"
+
+
+@pytest.mark.parametrize(
+    "config_json",
+    [
+        """
+    steps:
+        build-container-multi-platform:
+            build:
+                dockerfile: |
+                    FROM {{ DOCKER_REGISTRY }}/busybox
+                platforms:
+                    - linux/amd64
+                    - linux/arm64/v8
+            push:
+                - repository: user1/buildrunner-test-multi-platform
+                  tags: [ 'latest', '0.0.1', {{ BUILDRUNNER_BUILD_DOCKER_TAG }} ]
+
+        use-built-image1:
+            run:
+                image: user1/buildrunner-test-multi-platform:{{ BUILDRUNNER_BUILD_DOCKER_TAG }}
+                cmd: echo "Hello World"
+            push:
+                repository: user1/buildrunner-test-multi-platform2
+                tags: [ 'latest' ]
+    """,
+        """
+    steps:
+        build-container-multi-platform:
+            build:
+                dockerfile: |
+                    FROM {{ DOCKER_REGISTRY }}/busybox
+                platforms:
+                    - linux/amd64
+                    - linux/arm64/v8
+            push:
+                - repository: user1/buildrunner-test-multi-platform
+                  tags: [ 'latest', '0.0.1' ]
+
+        use-built-image1:
+            run:
+                image: user1/buildrunner-test-multi-platform:{{ BUILDRUNNER_BUILD_DOCKER_TAG }}
+                cmd: echo "Hello World"
+            push: user1/buildrunner-test-multi-platform2
+    """,
+    ],
+    ids=["buildrunnder_build_tag_explict", "buildrunnder_build_tag_implied"],
+)
+@mock.patch("buildrunner.config.DEFAULT_GLOBAL_CONFIG_FILES", [])
+@mock.patch("buildrunner.detect_vcs")
+def test_invalid_retagging_with_buildrunner_build_tag(detect_vcs_mock, config_json):
+    # Tests that BUILDRUNNER_BUILD_DOCKER_TAG is added to push tags and fails for re-tagging
+    id_string = "main-921.ie02ed8.m1705616822"
+    build_number = 342
+    type(detect_vcs_mock.return_value).id_string = mock.PropertyMock(
+        return_value=id_string
+    )
+
+    with tempfile.TemporaryDirectory() as tmpdirname:
+        with tempfile.NamedTemporaryFile(
+            dir=tmpdirname, mode="w", delete=False
+        ) as tmpfile:
+            tmpfile.write(config_json)
+            tmpfile.close()
+            with pytest.raises(BuildRunnerConfigurationError) as excinfo:
+                BuildRunner(
+                    build_dir=tmpdirname,
+                    build_results_dir=tmpdirname,
+                    global_config_file=None,
+                    run_config_file=tmpfile.name,
+                    build_time=0,
+                    build_number=build_number,
+                    push=False,
+                    cleanup_images=False,
+                    cleanup_cache=False,
+                    steps_to_run=None,
+                    publish_ports=False,
+                    log_generated_files=False,
+                    docker_timeout=30,
+                    local_images=False,
+                    platform=None,
+                    disable_multi_platform=False,
+                )
+            assert RETAG_ERROR_MESSAGE in excinfo.value.args[0]
+            assert (
+                f"user1/buildrunner-test-multi-platform:{id_string}-{build_number}"
+                in excinfo.value.args[0]
+            )