diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index 43cca531a..442d5000a 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -6,6 +6,7 @@ * #691: Started customization of PTB workflows by defining the YML schema * #712: Added basic logging to workflow processing +* #714: Added logic to modify a workflow using the .workflow-patcher.yml ## Documentation diff --git a/exasol/toolbox/util/workflows/exceptions.py b/exasol/toolbox/util/workflows/exceptions.py index 82155657a..5a884c9b9 100644 --- a/exasol/toolbox/util/workflows/exceptions.py +++ b/exasol/toolbox/util/workflows/exceptions.py @@ -52,3 +52,58 @@ class InvalidWorkflowPatcherYamlError(YamlError): """ message_template = "File '{file_path}' is malformed; it failed Pydantic validation." + + +class InvalidWorkflowPatcherEntryError(YamlError): + """ + Raised when the :class:`WorkflowPatcher` is used but one of the specified keys it + listed does not exist in the relevant workflow template file. + """ + + message_template = ( + "In file '{file_path}', an entry '{entry}' does not exist in " + "the workflow template. Please fix the entry." + ) + + +class YamlKeyError(Exception): + """ + Base exception for when a specified value cannot be found in a YAML. + """ + + message_template = "An error occurred with job: '{job_name}'" + + def __init__(self, job_name: str): + self.job_name = job_name + # Format the template defined in the subclass + message = self.message_template.format(job_name=job_name) + super().__init__(message) + + +class YamlJobValueError(Exception): + """ + Raised when a job cannot be found in a YAML file. + """ + + message_template = "Job '{job_name}' could not be found" + + def __init__(self, job_name: str): + self.job_name = job_name + # Format the template defined in the subclass + message = self.message_template.format(job_name=job_name) + super().__init__(message) + + +class YamlStepIdValueError(YamlKeyError): + """ + Raised when a step_id associated with a specific job cannot be found in a YAML file. + """ + + message_template = "Step_id '{step_id}' not found in job '{job_name}'" + + def __init__(self, step_id: str, job_name: str): + self.step_id = step_id + self.job_name = job_name + + message = self.message_template.format(step_id=step_id, job_name=job_name) + super(YamlKeyError, self).__init__(message) diff --git a/exasol/toolbox/util/workflows/process_template.py b/exasol/toolbox/util/workflows/process_template.py index 40cc2ef24..1051bb93c 100644 --- a/exasol/toolbox/util/workflows/process_template.py +++ b/exasol/toolbox/util/workflows/process_template.py @@ -1,6 +1,21 @@ +import copy +from dataclasses import dataclass + +from ruamel.yaml import CommentedMap + +from exasol.toolbox.util.workflows import logger +from exasol.toolbox.util.workflows.exceptions import ( + YamlJobValueError, + YamlStepIdValueError, +) +from exasol.toolbox.util.workflows.patch_workflow import ( + ActionType, + WorkflowCommentedMap, +) from exasol.toolbox.util.workflows.render_yaml import YamlRenderer +@dataclass(frozen=True) class WorkflowRenderer(YamlRenderer): """ The :class:`WorkflowRenderer` renders a workflow template provided by the PTB into @@ -9,9 +24,97 @@ class WorkflowRenderer(YamlRenderer): - standardizing formatting via ruamel.yaml for a consistent output. """ + patch_yaml: WorkflowCommentedMap | None + def render(self) -> str: """ Render the template to the contents of a valid GitHub workflow. """ workflow_dict = self.get_yaml_dict() + + if self.patch_yaml: + logger.debug("Modify workflow custom yaml") + workflow_modifier = WorkflowModifier( + workflow_dict=workflow_dict, patch_yaml=self.patch_yaml + ) + workflow_dict = workflow_modifier.get_patched_workflow() + return self.get_as_string(workflow_dict) + + +@dataclass +class WorkflowModifier: + workflow_dict: CommentedMap + patch_yaml: WorkflowCommentedMap + + def __post_init__(self): + # Perform deepcopy to ensure this instance owns its data + self.workflow_dict = copy.deepcopy(self.workflow_dict) + + @property + def jobs_dict(self) -> CommentedMap: + return self.workflow_dict["jobs"] + + def _get_step_list(self, job_name: str) -> CommentedMap: + self._verify_job_exists(job_name=job_name) + return self.jobs_dict[job_name]["steps"] + + def _customize_steps(self, step_customizations: CommentedMap) -> None: + """ + Customize the steps of jobs specified in `step_customizations` in a workflow + (`workflow_dict`). If a `step_id` or its parent `job` cannot be found, an + exception is raised. + """ + for patch in step_customizations: + job_name = patch["job"] + patch_id = patch["step_id"] + idx = self._get_step_index(job_name=job_name, step_id=patch_id) + + step_list = self._get_step_list(job_name=job_name) + if patch["action"] == ActionType.REPLACE.value: + logger.debug( + f"Replace YAML at step_id '{patch_id}' in job '{job_name}'" + ) + step_list[idx : idx + 1] = patch["content"] + + elif patch["action"] == ActionType.INSERT_AFTER.value: + logger.debug( + f"Insert YAML after step_id '{patch_id}' in job '{job_name}'" + ) + step_list[idx + 1 : idx + 1] = patch["content"] + + def _get_step_index(self, job_name: str, step_id: str) -> int: + steps = self._get_step_list(job_name=job_name) + for index, step in enumerate(steps): + if step["id"] == step_id: + return index + raise YamlStepIdValueError(step_id=step_id, job_name=job_name) + + def _remove_jobs(self, remove_jobs: CommentedMap) -> None: + """ + Remove the jobs specified in `remove_jobs` in a workflow yaml (`workflow_dict`). + If a `job` cannot be found, an exception is raised. + """ + for job_name in remove_jobs: + self._verify_job_exists(job_name) + logger.debug(f"Remove job '{job_name}'") + self.jobs_dict.pop(job_name) + + def _verify_job_exists(self, job_name: str) -> None: + if job_name not in self.jobs_dict: + raise YamlJobValueError(job_name=job_name) + + def get_patched_workflow(self): + """ + Patch the `workflow_dict`. As dictionaries are mutable structures, we directly + take advantage of this by having it modified in this class's internal methods + without explicit returns. + """ + + if remove_jobs := self.patch_yaml.get("remove_jobs", {}): + self._remove_jobs(remove_jobs=remove_jobs) + + if step_customizations := self.patch_yaml.get("step_customizations", {}): + self._customize_steps(step_customizations=step_customizations) + + return self.workflow_dict diff --git a/exasol/toolbox/util/workflows/workflow.py b/exasol/toolbox/util/workflows/workflow.py index db4b6b0ad..70dd1d457 100644 --- a/exasol/toolbox/util/workflows/workflow.py +++ b/exasol/toolbox/util/workflows/workflow.py @@ -31,6 +31,7 @@ def load_from_template(cls, file_path: Path, github_template_dict: dict[str, Any workflow_renderer = WorkflowRenderer( github_template_dict=github_template_dict, file_path=file_path, + patch_yaml=None, ) workflow = workflow_renderer.render() return cls(content=workflow) diff --git a/test/unit/util/workflows/patch_workflow_test.py b/test/unit/util/workflows/patch_workflow_test.py index 5ac0405a4..a3618b792 100644 --- a/test/unit/util/workflows/patch_workflow_test.py +++ b/test/unit/util/workflows/patch_workflow_test.py @@ -96,8 +96,8 @@ def test_raises_error_for_unknown_action( with pytest.raises( InvalidWorkflowPatcherYamlError, match="is malformed; it failed Pydantic validation", - ) as exc: + ) as ex: workflow_patcher.content - underlying_error = exc.value.__cause__ + underlying_error = ex.value.__cause__ assert isinstance(underlying_error, ValidationError) diff --git a/test/unit/util/workflows/process_template_test.py b/test/unit/util/workflows/process_template_test.py new file mode 100644 index 000000000..76f851add --- /dev/null +++ b/test/unit/util/workflows/process_template_test.py @@ -0,0 +1,211 @@ +import pytest +from ruamel.yaml import CommentedMap + +from exasol.toolbox.util.workflows.exceptions import ( + YamlJobValueError, + YamlStepIdValueError, +) +from exasol.toolbox.util.workflows.patch_workflow import ActionType +from exasol.toolbox.util.workflows.process_template import WorkflowModifier +from exasol.toolbox.util.workflows.render_yaml import YamlRenderer +from noxconfig import PROJECT_CONFIG + +WORKFLOW_YAML = """ +name: Checks + +on: + workflow_call: + +jobs: + build-documentation-and-check-links: + name: Docs + runs-on: "ubuntu-24.04" + permissions: + contents: read + steps: + - name: Check out Repository + id: check-out-repository + uses: actions/checkout@v6 + + run-unit-tests: + name: Run Unit Tests (Python-${{ matrix.python-versions }}) + runs-on: "ubuntu-24.04" + permissions: + contents: read + strategy: + fail-fast: false + matrix: + python-versions: ["3.10", "3.11", "3.12", "3.13", "3.14"] + + steps: + - name: Check out Repository + id: check-out-repository + uses: actions/checkout@v6 + +""" + + +@pytest.fixture +def workflow_name(): + return "checks.yml" + + +@pytest.fixture +def checks_yaml(tmp_path, workflow_name): + file_path = tmp_path / workflow_name + file_path.write_text(WORKFLOW_YAML) + return file_path + + +@pytest.fixture +def workflow_dict(checks_yaml) -> CommentedMap: + return YamlRenderer( + github_template_dict=PROJECT_CONFIG.github_template_dict, file_path=checks_yaml + ).get_yaml_dict() + + +class TestWorkflowModifier: + @staticmethod + def test__remove_jobs_works( + workflow_name, workflow_dict, workflow_patcher, remove_job_yaml + ): + workflow_modifier = WorkflowModifier( + workflow_dict=workflow_dict, + patch_yaml=workflow_patcher.extract_by_workflow(workflow_name), + ) + + result = workflow_modifier.get_patched_workflow() + + # The original was not altered as it was deepcopied before modifications. + assert list(workflow_dict["jobs"].keys()) == [ + "build-documentation-and-check-links", + "run-unit-tests", + ] + # The original and resulting workflows should have the same values here. + assert result["name"] == workflow_dict["name"] + assert result["on"] == workflow_dict["on"] + assert ( + result["jobs"]["run-unit-tests"] == workflow_dict["jobs"]["run-unit-tests"] + ) + # The resulting workflow has job "build-documentation-and-check-links" removed. + assert list(result["jobs"].keys()) == ["run-unit-tests"] + + @staticmethod + @pytest.mark.parametrize( + "step_customization_yaml", + [ActionType.REPLACE.value], + indirect=True, + ) + def test__customize_steps_replacement_works( + workflow_name, workflow_dict, workflow_patcher, step_customization_yaml + ): + workflow_modifier = WorkflowModifier( + workflow_dict=workflow_dict, + patch_yaml=workflow_patcher.extract_by_workflow(workflow_name), + ) + + result = workflow_modifier.get_patched_workflow() + + # The original and resulting workflows should have the same values here. + assert result["name"] == workflow_dict["name"] + assert result["on"] == workflow_dict["on"] + assert list(result["jobs"].keys()) == list(workflow_dict["jobs"].keys()) + assert ( + result["jobs"]["build-documentation-and-check-links"] + == workflow_dict["jobs"]["build-documentation-and-check-links"] + ) + # The replaced step operation was done in job `run-unit-tests` for step + # `check-out-repository`. The replacement was mostly identical to the original + # value, but it has a `with`. + assert result["jobs"]["run-unit-tests"]["steps"][0].pop("with") == CommentedMap( + {"fetch-depth": 0} + ) + # Without the `with`, they should be the same, as that's how the test is set up. + assert ( + result["jobs"]["run-unit-tests"] == workflow_dict["jobs"]["run-unit-tests"] + ) + + @staticmethod + @pytest.mark.parametrize( + "step_customization_yaml", + [ActionType.INSERT_AFTER.value], + indirect=True, + ) + def test__customize_steps_insertion_after_works( + workflow_name, workflow_dict, workflow_patcher, step_customization_yaml + ): + workflow_modifier = WorkflowModifier( + workflow_dict=workflow_dict, + patch_yaml=workflow_patcher.extract_by_workflow(workflow_name), + ) + + result = workflow_modifier.get_patched_workflow() + + # The original and internal workflows should have the same values here. + assert result["name"] == workflow_dict["name"] + assert result["on"] == workflow_dict["on"] + assert ( + result["jobs"]["build-documentation-and-check-links"] + == workflow_dict["jobs"]["build-documentation-and-check-links"] + ) + # The insert after job added a step at the end of `run-unit-tests`. + assert ( + len(result["jobs"]["run-unit-tests"]["steps"]) + == len(workflow_dict["jobs"]["run-unit-tests"]["steps"]) + 1 + == 2 + ) + assert ( + result["jobs"]["run-unit-tests"]["steps"][0] + == workflow_dict["jobs"]["run-unit-tests"]["steps"][0] + ) + # The inserted after was done in job `run-unit-tests`, after step + # `check-out-repository`. It has a `with` but is otherwise identical to + # the preceding step. + assert result["jobs"]["run-unit-tests"]["steps"][1].pop("with") == CommentedMap( + {"fetch-depth": 0} + ) + assert ( + result["jobs"]["run-unit-tests"]["steps"][1] + == result["jobs"]["run-unit-tests"]["steps"][0] + ) + + +class TestWorkflowModifierExceptions: + @staticmethod + def test_job_does_not_exist_raises_exception( + workflow_name, workflow_dict, workflow_patcher, remove_job_yaml + ): + # Remove job that would be otherwise removed by the WorkflowModifier + job_name = "build-documentation-and-check-links" + workflow_dict["jobs"].pop(job_name) + + workflow_modifier = WorkflowModifier( + workflow_dict=workflow_dict, + patch_yaml=workflow_patcher.extract_by_workflow(workflow_name), + ) + + with pytest.raises(YamlJobValueError, match=job_name): + workflow_modifier.get_patched_workflow() + + @staticmethod + @pytest.mark.parametrize( + "step_customization_yaml", + [ActionType.REPLACE.value], + indirect=True, + ) + def test_step_id_does_not_exist_raises_exception( + workflow_name, workflow_dict, workflow_patcher, step_customization_yaml + ): + # Remove step_id that would be otherwise replaced by the WorkflowModifier + job_name = "run-unit-tests" + step_id = "check-out-repository" + step = workflow_dict["jobs"][job_name]["steps"].pop(0) + assert step["id"] == step_id + + workflow_modifier = WorkflowModifier( + workflow_dict=workflow_dict, + patch_yaml=workflow_patcher.extract_by_workflow(workflow_name), + ) + + with pytest.raises(YamlStepIdValueError, match=step_id): + workflow_modifier.get_patched_workflow() diff --git a/test/unit/util/workflows/render_yaml_test.py b/test/unit/util/workflows/render_yaml_test.py index 9630b33eb..91367d576 100644 --- a/test/unit/util/workflows/render_yaml_test.py +++ b/test/unit/util/workflows/render_yaml_test.py @@ -153,29 +153,6 @@ def test_keeps_quotes_for_variables_as_is(test_yml, yaml_renderer): yaml_dict = yaml_renderer.get_yaml_dict() assert yaml_renderer.get_as_string(yaml_dict) == cleandoc(expected_yaml) - @staticmethod - def test_updates_jinja_variables(test_yml, yaml_renderer): - input_yaml = """ - - name: Setup Python & Poetry Environment - uses: exasol/python-toolbox/.github/actions/python-environment@v5 - with: - python-version: "(( minimum_python_version ))" - poetry-version: "(( dependency_manager_version ))" - """ - expected_yaml = """ - - name: Setup Python & Poetry Environment - uses: exasol/python-toolbox/.github/actions/python-environment@v5 - with: - python-version: "3.10" - poetry-version: "2.3.0" - """ - - content = cleandoc(input_yaml) - test_yml.write_text(content) - - yaml_dict = yaml_renderer.get_yaml_dict() - assert yaml_renderer.get_as_string(yaml_dict) == cleandoc(expected_yaml) - @staticmethod def test_preserves_list_format(test_yml, yaml_renderer): input_yaml = """ @@ -200,43 +177,6 @@ def test_preserves_list_format(test_yml, yaml_renderer): yaml_dict = yaml_renderer.get_yaml_dict() assert yaml_renderer.get_as_string(yaml_dict) == cleandoc(input_yaml) - @staticmethod - def test_jinja_variable_unknown(test_yml, yaml_renderer): - input_yaml = """ - - name: Setup Python & Poetry Environment - uses: exasol/python-toolbox/.github/actions/python-environment@v5 - with: - poetry-version: "(( bad_jinja ))" - """ - - content = cleandoc(input_yaml) - test_yml.write_text(content) - - with pytest.raises( - TemplateRenderingError, match="Check for Jinja-related errors." - ) as exc: - yaml_renderer.get_yaml_dict() - assert isinstance(exc.value.__cause__, UndefinedError) - assert "'bad_jinja' is undefined" in str(exc.value.__cause__) - - @staticmethod - def test_jinja_variable_unclosed(test_yml, yaml_renderer): - input_yaml = """ - - name: Setup Python & Poetry Environment - uses: exasol/python-toolbox/.github/actions/python-environment@v5 - with: - python-version: "(( minimum_python_version )" - """ - content = cleandoc(input_yaml) - test_yml.write_text(content) - - with pytest.raises( - TemplateRenderingError, match="Check for Jinja-related errors." - ) as exc: - yaml_renderer.get_yaml_dict() - assert isinstance(exc.value.__cause__, TemplateSyntaxError) - assert "unexpected ')'" in str(exc.value.__cause__) - @staticmethod def test_parsing_fails_when_yaml_malformed(test_yml, yaml_renderer): bad_template = """ @@ -260,11 +200,11 @@ def test_parsing_fails_when_yaml_malformed(test_yml, yaml_renderer): with pytest.raises( YamlParsingError, match="Check for invalid YAML syntax." - ) as excinfo: + ) as ex: yaml_renderer.get_yaml_dict() - assert isinstance(excinfo.value.__cause__, ParserError) - assert "while parsing a block collection" in str(excinfo.value.__cause__) + assert isinstance(ex.value.__cause__, ParserError) + assert "while parsing a block collection" in str(ex.value.__cause__) @staticmethod def test_yaml_cannot_output_to_string(test_yml, yaml_renderer): @@ -281,8 +221,70 @@ def test_yaml_cannot_output_to_string(test_yml, yaml_renderer): yaml_dict = yaml_renderer.get_yaml_dict() yaml_dict["steps"][0]["name"] = lambda x: x + 1 - with pytest.raises(YamlOutputError, match="could not be output") as excinfo: + with pytest.raises(YamlOutputError, match="could not be output") as ex: yaml_renderer.get_as_string(yaml_dict) - assert isinstance(excinfo.value.__cause__, RepresenterError) - assert "cannot represent an object" in str(excinfo.value.__cause__) + assert isinstance(ex.value.__cause__, RepresenterError) + assert "cannot represent an object" in str(ex.value.__cause__) + + +class TestYamlRendererJinja: + @staticmethod + def test_updates_jinja_variables(test_yml, yaml_renderer): + input_yaml = """ + - name: Setup Python & Poetry Environment + uses: exasol/python-toolbox/.github/actions/python-environment@v5 + with: + python-version: "(( minimum_python_version ))" + poetry-version: "(( dependency_manager_version ))" + """ + expected_yaml = """ + - name: Setup Python & Poetry Environment + uses: exasol/python-toolbox/.github/actions/python-environment@v5 + with: + python-version: "3.10" + poetry-version: "2.3.0" + """ + + content = cleandoc(input_yaml) + test_yml.write_text(content) + + yaml_dict = yaml_renderer.get_yaml_dict() + assert yaml_renderer.get_as_string(yaml_dict) == cleandoc(expected_yaml) + + @staticmethod + def test_jinja_variable_unknown(test_yml, yaml_renderer): + input_yaml = """ + - name: Setup Python & Poetry Environment + uses: exasol/python-toolbox/.github/actions/python-environment@v5 + with: + poetry-version: "(( bad_jinja ))" + """ + + content = cleandoc(input_yaml) + test_yml.write_text(content) + + with pytest.raises( + TemplateRenderingError, match="Check for Jinja-related errors." + ) as ex: + yaml_renderer.get_yaml_dict() + assert isinstance(ex.value.__cause__, UndefinedError) + assert "'bad_jinja' is undefined" in str(ex.value.__cause__) + + @staticmethod + def test_jinja_variable_unclosed(test_yml, yaml_renderer): + input_yaml = """ + - name: Setup Python & Poetry Environment + uses: exasol/python-toolbox/.github/actions/python-environment@v5 + with: + python-version: "(( minimum_python_version )" + """ + content = cleandoc(input_yaml) + test_yml.write_text(content) + + with pytest.raises( + TemplateRenderingError, match="Check for Jinja-related errors." + ) as ex: + yaml_renderer.get_yaml_dict() + assert isinstance(ex.value.__cause__, TemplateSyntaxError) + assert "unexpected ')'" in str(ex.value.__cause__)