From 9951ae10c592b30fd329248ce0021f8e3a956364 Mon Sep 17 00:00:00 2001 From: Shane Brown Date: Fri, 18 Aug 2023 10:41:13 -0600 Subject: [PATCH] Add config validation for multi-platform images --- buildrunner/config.py | 10 + buildrunner/config_model.py | 150 +++++++++++++++ requirements.in | 2 + requirements.txt | 4 +- tests/test_config_validation.py | 319 ++++++++++++++++++++++++++++++++ 5 files changed, 484 insertions(+), 1 deletion(-) create mode 100644 buildrunner/config_model.py create mode 100644 tests/test_config_validation.py diff --git a/buildrunner/config.py b/buildrunner/config.py index 2219df69..9c83fd56 100644 --- a/buildrunner/config.py +++ b/buildrunner/config.py @@ -21,6 +21,8 @@ import jinja2 +from pydantic import ValidationError + from buildrunner.errors import ( BuildRunnerConfigurationError, BuildRunnerVersionError, @@ -34,6 +36,8 @@ load_config, ) +from buildrunner import config_model + from . import fetch MASTER_GLOBAL_CONFIG_FILE = '/etc/buildrunner/buildrunner.yaml' @@ -370,6 +374,12 @@ def load_config(self, cfg_file, ctx=None, log_file=True): config = self._reorder_dependency_steps(config) + # Validate the config + try: + config_model.Config(**config) + except (ValidationError, ValueError) as err: + raise BuildRunnerConfigurationError(f"Invalid configuration: {err}") # pylint: disable=raise-missing-from + return config def get_temp_dir(self): diff --git a/buildrunner/config_model.py b/buildrunner/config_model.py new file mode 100644 index 00000000..d522e57c --- /dev/null +++ b/buildrunner/config_model.py @@ -0,0 +1,150 @@ +""" +Copyright 2023 Adobe +All Rights Reserved. + +NOTICE: Adobe permits you to use, modify, and distribute this file in accordance +with the terms of the Adobe license agreement accompanying it. +""" + +from typing import Optional +from pydantic import BaseModel + + +class StepBuild(BaseModel): + """ Build model within a step """ + path: Optional[str] + dockerfile: Optional[str] + pull: Optional[bool] + platform: Optional[str] + platforms: Optional[list[str]] + + +class StepPushDict(BaseModel): + """ Push model within a step """ + repository: str + tags: list[str] + + +class Step(BaseModel): + """ Step model """ + build: Optional[StepBuild] + push: Optional[StepPushDict | list[str | StepPushDict] | str] + + def is_multi_platform(self): + """ + Check if the step is a multi-platform build step + """ + return self.build is not None and \ + self.build.platforms is not None + + +class Config(BaseModel): + """ Top level config model """ + version: Optional[float] + steps: dict[str, Step] + + def __init__(self, **data) -> None: + super().__init__(**data) + self.validate() + + def has_multi_platform_build(self): + """ + Check if the config file has multi-platform build steps + + Returns: + bool: True if the config file has multi-platform build steps, False otherwise + """ + for step in self.steps.values(): + if step.is_multi_platform(): + return True + return False + + def validate_push(self, + push: StepPushDict | list[str | StepPushDict] | str, + mp_push_tags: set[str], + step_name: str, + update_mp_push_tags: bool = True): + """ + Validate push step + + Args: + push (StepPushDict | list[str | StepPushDict] | str): Push step + mp_push_tags (set[str]): Set of all tags used in multi-platform build steps + step_name (str): Name of the step + update_mp_push_tags (bool, optional): Whether to update the set of tags used in multi-platform steps. + + Raises: + ValueError: If the config file is invalid + """ + # Check for valid push section, duplicate mp tags are not allowed + if push is not None: + name = None + names = None + if isinstance(push, str): + name = push + if ":" not in name: + name = f'{name}:latest' + + if isinstance(push, StepPushDict): + names = [f"{push.repository}:{tag}" for tag in push.tags] + + if names is not None: + for current_name in names: + if current_name in mp_push_tags: + # raise ValueError(f'Cannot specify duplicate tag {current_name} in build step {step_name}') + raise ValueError(f'Cannot specify duplicate tag {current_name} in build step {step_name}') + + if name is not None and name in mp_push_tags: + # raise ValueError(f'Cannot specify duplicate tag {name} in build step {step_name}') + raise ValueError(f'Cannot specify duplicate tag {name} in build step {step_name}') + + if update_mp_push_tags and names is not None: + mp_push_tags.update(names) + + if update_mp_push_tags and name is not None: + mp_push_tags.add(name) + + def validate_multi_platform_build(self, mp_push_tags: set[str]): + """ + Validate multi-platform build steps + + Args: + mp_push_tags (set[str]): Set of all tags used in multi-platform build steps + + Raises: + ValueError | pydantic.ValidationError: If the config file is invalid + """ + # Iterate through each step + for step_name, step in self.steps.items(): + if step.is_multi_platform(): + if step.build.platform is not None: + raise ValueError(f'Cannot specify both platform ({step.build.platform}) and ' + f'platforms ({step.build.platforms}) in build step {step_name}') + + if not isinstance(step.build.platforms, list): + raise ValueError(f'platforms must be a list in build step {step_name}') + + # Check for valid push section, duplicate mp tags are not allowed + self.validate_push(step.push, mp_push_tags, step_name) + + def validate(self): + """ + Validate the config file + + Raises: + ValueError | pydantic.ValidationError : If the config file is invalid + """ + if self.has_multi_platform_build(): + mp_push_tags = set() + self.validate_multi_platform_build(mp_push_tags) + + # Validate that all tags are unique across all multi-platform step + for step_name, step in self.steps.items(): + + # Check that there are no single platform tags that match multi-platform tags + if not step.is_multi_platform(): + if step.push is not None: + self.validate_push(push=step.push, + mp_push_tags=mp_push_tags, + step_name=step_name, + update_mp_push_tags=False) diff --git a/requirements.in b/requirements.in index 0e1c65f5..6a36905c 100644 --- a/requirements.in +++ b/requirements.in @@ -11,3 +11,5 @@ vcsinfo>=2.1.105 graphlib-backport>=1.0.3 timeout-decorator>=0.5.0 python-on-whales>=0.61.0 +# python-on-whales requires pydantic 1.10.11 08/2023 +pydantic>=1.10.11 diff --git a/requirements.txt b/requirements.txt index e33cf554..dda7360c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -73,7 +73,9 @@ pkginfo==1.9.6 pycparser==2.21 # via cffi pydantic==1.10.11 - # via python-on-whales + # via + # -r requirements.in + # python-on-whales pygments==2.15.1 # via # readme-renderer diff --git a/tests/test_config_validation.py b/tests/test_config_validation.py new file mode 100644 index 00000000..650e4868 --- /dev/null +++ b/tests/test_config_validation.py @@ -0,0 +1,319 @@ + +import buildrunner.config_model as config_model +from pydantic import ValidationError +import pytest + + +def test_valid_version_config(): + # Invalid version + config = { + 'version': 'string' + } + with pytest.raises(ValidationError): + config_model.Config(**config) + + # Valid version + config = { + 'version': 2.0, + 'steps': { + } + } + try: + config_model.Config(**config) + except ValidationError as err: + pytest.fail(f'Config should be valid {err}') + + # Optional version + config = { + 'steps': { + } + } + try: + config_model.Config(**config) + except ValidationError as err: + pytest.fail(f'Config should be valid {err}') + +def test_platform_and_platforms_invalid(): + # Invalid to have platform and platforms + config = { + 'steps': { + 'build-container-multi-platform': { + 'build': { + 'path': '.', + 'dockerfile': 'Dockerfile', + 'pull': False, + 'platform': 'linux/amd64', + 'platforms': [ + 'linux/amd64', + 'linux/arm64', + ], + }, + 'push': { + 'repository': 'mytest-reg/buildrunner-test-multi-platform', + 'tags': [ 'latest' ], + }, + }, + } + } + with pytest.raises(ValueError): + config_model.Config(**config) + + +def test_platforms_invalid(): + # Invalid to have platforms as a string, it should be a list + config = { + 'steps': { + 'build-container-multi-platform': { + 'build': { + 'path': '.', + 'dockerfile': 'Dockerfile', + 'pull': False, + 'platforms': 'linux/amd64', + }, + 'push': { + 'repository': 'mytest-reg/buildrunner-test-multi-platform', + 'tags': [ 'latest' ], + }, + }, + } + } + with pytest.raises(ValueError): + config_model.Config(**config) + +def test_valid_platforms(): + config = { + 'steps': { + 'build-container-multi-platform': { + 'build': { + 'path': '.', + 'dockerfile': 'Dockerfile', + 'pull': False, + 'platforms': [ + 'linux/amd64', + 'linux/arm64', + ], + }, + 'push': { + 'repository': 'mytest-reg/buildrunner-test-multi-platform', + 'tags': [ 'latest' ], + }, + }, + } + } + model = config_model.Config(**config) + assert model.steps['build-container-multi-platform'] + assert model.steps['build-container-multi-platform'].build + assert model.steps['build-container-multi-platform'].build.platforms == ['linux/amd64', 'linux/arm64'] + assert model.steps['build-container-multi-platform'].build.platform is None + + +def test_duplicate_mp_tags_dictionary_invalid(): + # Invalid to have duplicate multi-platform tag + # Testing with both dictionary format + config = { + 'steps': { + 'build-container-multi-platform1': { + 'build': { + 'platforms': [ + 'linux/amd64', + 'linux/arm64', + ], + }, + 'push': { + 'repository': 'mytest-reg/buildrunner-test-multi-platform', + 'tags': [ 'latest' ], + }, + }, + 'build-container-multi-platform2': { + 'build': { + 'platforms': [ + 'linux/amd64', + 'linux/arm64', + ], + }, + 'push': { + 'repository': 'mytest-reg/buildrunner-test-multi-platform', + 'tags': [ 'latest' ], + }, + }, + } + } + with pytest.raises(ValueError): + config_model.Config(**config) + +def test_duplicate_mp_tags_strings_invalid(): + # Invalid to have duplicate multi-platform tag + # Testing with both string format, one inferred 'latest' the other explicit 'latest' + config = { + 'steps': { + 'build-container-multi-platform1': { + 'build': { + 'platforms': [ + 'linux/amd64', + 'linux/arm64', + ], + }, + # No tag is given so 'latest' is assumed + 'push': 'mytest-reg/buildrunner-test-multi-platform', + }, + 'build-container-multi-platform2': { + 'build': { + 'platforms': [ + 'linux/amd64', + 'linux/arm64', + ], + }, + 'push': 'mytest-reg/buildrunner-test-multi-platform:latest', + }, + } + } + with pytest.raises(ValueError): + config_model.Config(**config) + + # Indentical tags in same string format + config = { + 'steps': { + 'build-container-multi-platform1': { + 'build': { + 'platforms': [ + 'linux/amd64', + 'linux/arm64', + ], + }, + 'push': 'mytest-reg/buildrunner-test-multi-platform:latest', + }, + 'build-container-multi-platform2': { + 'build': { + 'platforms': [ + 'linux/amd64', + 'linux/arm64', + ], + }, + 'push': 'mytest-reg/buildrunner-test-multi-platform:latest', + }, + } + } + with pytest.raises(ValueError): + config_model.Config(**config) + +def test_duplicate_mp_tags_strings_valid(): + # Same string format but different MP tags + config = { + 'steps': { + 'build-container-multi-platform1': { + 'build': { + 'platforms': [ + 'linux/amd64', + 'linux/arm64', + ], + }, + 'push': 'mytest-reg/buildrunner-test-multi-platform:latest', + }, + 'build-container-multi-platform2': { + 'build': { + 'platforms': [ + 'linux/amd64', + 'linux/arm64', + ], + }, + 'push': 'mytest-reg/buildrunner-test-multi-platform:not-latest', + }, + } + } + model = config_model.Config(**config) + assert model.steps['build-container-multi-platform1'] + assert model.steps['build-container-multi-platform1'].build + assert model.steps['build-container-multi-platform1'].build.platforms == ['linux/amd64', 'linux/arm64'] + assert model.steps['build-container-multi-platform1'].build.platform is None + assert model.steps['build-container-multi-platform1'].push == 'mytest-reg/buildrunner-test-multi-platform:latest' + + assert model.steps['build-container-multi-platform2'] + assert model.steps['build-container-multi-platform2'].build + assert model.steps['build-container-multi-platform2'].build.platforms == ['linux/amd64', 'linux/arm64'] + assert model.steps['build-container-multi-platform2'].build.platform is None + assert model.steps['build-container-multi-platform2'].push == 'mytest-reg/buildrunner-test-multi-platform:not-latest' + +def test_duplicate_mp_tags_platform_platforms_invalid(): + # Invalid to have duplicate multi-platform tag and single platform tag + config = { + 'steps': { + 'build-container-multi-platform1': { + 'build': { + 'platforms': [ + 'linux/amd64', + 'linux/arm64', + ], + }, + 'push': 'mytest-reg/buildrunner-test-multi-platform:latest', + }, + 'build-container-single-platform': { + 'build': { + 'platform': 'linux/arm64' + }, + 'push': 'mytest-reg/buildrunner-test-multi-platform:latest', + }, + } + } + with pytest.raises(ValueError): + config_model.Config(**config) + +def test_valid_config(): + # Sample valid config, but not exhaustive + config = { + 'version': 2.0, + 'steps': { + 'build-container-single-platform1': { + 'build': { + 'path': '.', + 'dockerfile': 'Dockerfile', + 'pull': False, + 'platform': 'linux/amd64', + }, + 'push': { + 'repository': 'mytest-reg/buildrunner-test', + 'tags': [ 'latest' ], + }, + }, + 'build-container-multi-platform2': { + 'build': { + 'path': '.', + 'dockerfile': 'Dockerfile', + 'pull': False, + 'platforms': [ + 'linux/amd64', + 'linux/arm64', + ], + }, + 'push': { + 'repository': 'mytest-reg/buildrunner-test-multi-platform', + 'tags': [ 'latest' ], + }, + }, + 'build-container-multi-platform-push3': { + 'build': { + 'path': '.', + 'dockerfile': 'Dockerfile', + 'pull': False, + 'platforms': [ + 'linux/amd64', + 'linux/arm64', + ], + }, + 'push': [ + 'myimages/image1', + { + 'repository': 'myimages/image2', + 'tags': [ 'latest' ], + } + ], + }, + } + } + + try: + model = config_model.Config(**config) + assert model.steps + assert model.version + + except ValidationError as err: + pytest.fail(f'Config should be valid {err}')