diff --git a/doc/changes/changes_10.0.0.md b/doc/changes/changes_10.0.0.md index 216a0a2ed..d8ba3cd22 100644 --- a/doc/changes/changes_10.0.0.md +++ b/doc/changes/changes_10.0.0.md @@ -48,7 +48,7 @@ This release fixes vulnerabilities by updating dependencies: * #699: Added `all-extras` support to the Python environment GitHub action * #875: Added `name` attribute to generated workflow jobs using `-extension.yml` workflows -## Bug +## Bug Fix * #744: Updated nox DB-version handling to use `BaseConfig.minimum_exasol_version` instead hardcoded `7.1.9` @@ -80,4 +80,4 @@ This release fixes vulnerabilities by updating dependencies: * Updated dependency `pip-audit:2.10.0` to `2.10.1` * Updated dependency `pylint:4.0.5` to `4.0.6` * Updated dependency `pytest:9.0.3` to `9.1.1` -* Updated dependency `zizmor:1.25.2` to `1.26.1` \ No newline at end of file +* Updated dependency `zizmor:1.25.2` to `1.26.1` diff --git a/doc/changes/changes_10.2.0.md b/doc/changes/changes_10.2.0.md index a8848b691..e00721c99 100644 --- a/doc/changes/changes_10.2.0.md +++ b/doc/changes/changes_10.2.0.md @@ -6,7 +6,7 @@ This minor release adds automated vulnerability updates through Nox session `vulnerabilities:update` and improves the `dependency-update.yml`. It also includes a few workflow-related bug fixes and documentation updates. -## Bug +## Bug Fix * #909: Updated `cd.yml` workflow so that `cd-extension.yml` workflow depends on `build-and-publish`. This ensures that the custom release workflow only runs when the PyPi release was successful. * #910: Added `gh-pages.yml` to be ignored when `has_documentation=False` in the `PROJECT_CONFIG` diff --git a/doc/changes/changes_10.2.1.md b/doc/changes/changes_10.2.1.md index 98cdcb506..c29ae2b39 100644 --- a/doc/changes/changes_10.2.1.md +++ b/doc/changes/changes_10.2.1.md @@ -1,7 +1,5 @@ # 10.2.1 - 2026-07-08 -## Summary - ## Bug Fix * #920: Ensured extracted secrets are unique and alphabetically sorted from the custom workflows diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index 228c80dee..e813a4483 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -2,6 +2,10 @@ ## Summary +## Feature + +* #922: Extended `custom_workflows` of `github_template_dict` for automatic custom workflow permissions extraction + ## Refactoring * #924: Removed `lint:dependencies` usage from `report.yml` and added deprecation notice diff --git a/doc/user_guide/features/github_workflows/workflow_variables.rst b/doc/user_guide/features/github_workflows/workflow_variables.rst index 01f0ff384..838f77077 100644 --- a/doc/user_guide/features/github_workflows/workflow_variables.rst +++ b/doc/user_guide/features/github_workflows/workflow_variables.rst @@ -19,12 +19,22 @@ standardized baseline that can be overridden in individual projects. :start-at: github_template_dict :end-before: @computed_field +.. _custom_workflow_metadata: + +Custom Workflow Metadata +^^^^^^^^^^^^^^^^^^^^^^^^ + +The PTB extracts metadata from reusable custom workflow files and exposes it +through :py:attr:`exasol.toolbox.config.BaseConfig.github_template_dict` under the +``custom_workflows`` entry. PTB-controlled workflow templates use that metadata +when they call reusable workflows. + .. _custom_workflow_secrets: -Custom Workflow Secrets -^^^^^^^^^^^^^^^^^^^^^^^ +Secrets +------- -The PTB extracts secret names from reusable custom workflow files and exposes them +The PTB extracts secret names from custom workflow files and exposes them through :py:attr:`exasol.toolbox.config.BaseConfig.github_template_dict` under the ``custom_workflows`` entry. PTB-controlled workflow templates use those extracted names when they call reusable workflows and forward secrets via ``secrets:``. @@ -50,6 +60,33 @@ For example, ``slow-checks.yml`` can define its reusable workflow header like th Those extracted secret names are then made available to the PTB templates that reference the custom workflow. +.. _custom_workflow_permissions: + +Permissions +----------- + +The PTB extracts the permissions required by custom workflow files. It reads every job's +``permissions`` block and combines the results into a single ordered mapping, where +the most permissive level wins. + +Please only configure the minimum required permissions by granting the least required +access. In practice, ``contents: read`` is the most common baseline for workflows, and +other permissions should only be added when it is truly required. + +For example, a custom workflow can declare permissions like this: + +.. code-block:: yaml + + name: Slow-Checks + + on: + workflow_call: + + jobs: + run-integration-tests: + permissions: + contents: read + .. _workflow_matrix: Matrix Combinations @@ -64,7 +101,7 @@ Extending the Matrix ^^^^^^^^^^^^^^^^^^^^ If you need to expose additional values via the ``matrix.yml``, you can extend -:class:`exasol.toolbox.config.BaseConfig`. +:class:`exasol.toolbox.config.BaseConfig`. The example adds two additional matrix dimensions: A declared one `extra_matrix_value` and a computed one `computed_matrix_value`. Each of them diff --git a/exasol/toolbox/templates/github/workflows/cd.yml b/exasol/toolbox/templates/github/workflows/cd.yml index 58eceed90..a856759cf 100644 --- a/exasol/toolbox/templates/github/workflows/cd.yml +++ b/exasol/toolbox/templates/github/workflows/cd.yml @@ -38,8 +38,12 @@ jobs: (( secret_name )): ${{ secrets.(( secret_name )) }} (% endfor %) (% endif %) + (% if custom_workflows["cd-extension"].permissions %) permissions: - contents: write + (% for permission_name, permission_level in custom_workflows["cd-extension"].permissions.items() %) + (( permission_name )): (( permission_level )) + (% endfor %) + (% endif %) (% endif %) (% if has_documentation %) diff --git a/exasol/toolbox/templates/github/workflows/ci.yml b/exasol/toolbox/templates/github/workflows/ci.yml index c3c095328..da524c3a7 100644 --- a/exasol/toolbox/templates/github/workflows/ci.yml +++ b/exasol/toolbox/templates/github/workflows/ci.yml @@ -15,8 +15,12 @@ jobs: (( secret_name )): ${{ secrets.(( secret_name )) }} (% endfor %) (% endif %) + (% if custom_workflows["merge-gate"].permissions %) permissions: - contents: read + (% for permission_name, permission_level in custom_workflows["merge-gate"].permissions.items() %) + (( permission_name )): (( permission_level )) + (% endfor %) + (% endif %) report: name: Report diff --git a/exasol/toolbox/templates/github/workflows/fast-tests.yml b/exasol/toolbox/templates/github/workflows/fast-tests.yml index 0d4156809..c0151676a 100644 --- a/exasol/toolbox/templates/github/workflows/fast-tests.yml +++ b/exasol/toolbox/templates/github/workflows/fast-tests.yml @@ -52,6 +52,10 @@ jobs: (( secret_name )): ${{ secrets.(( secret_name )) }} (% endfor %) (% endif %) + (% if custom_workflows["fast-tests-extension"].permissions %) permissions: - contents: read + (% for permission_name, permission_level in custom_workflows["fast-tests-extension"].permissions.items() %) + (( permission_name )): (( permission_level )) + (% endfor %) + (% endif %) (% endif %) diff --git a/exasol/toolbox/templates/github/workflows/merge-gate.yml b/exasol/toolbox/templates/github/workflows/merge-gate.yml index d68adf12c..8ee19db55 100644 --- a/exasol/toolbox/templates/github/workflows/merge-gate.yml +++ b/exasol/toolbox/templates/github/workflows/merge-gate.yml @@ -59,8 +59,12 @@ jobs: (( secret_name )): ${{ secrets.(( secret_name )) }} (% endfor %) (% endif %) + (% if custom_workflows["slow-checks"].permissions %) permissions: - contents: read + (% for permission_name, permission_level in custom_workflows["slow-checks"].permissions.items() %) + (( permission_name )): (( permission_level )) + (% endfor %) + (% endif %) (% if custom_workflows["merge-gate-extension"].exists %) merge-gate-extension: @@ -72,8 +76,12 @@ jobs: (( secret_name )): ${{ secrets.(( secret_name )) }} (% endfor %) (% endif %) + (% if custom_workflows["merge-gate-extension"].permissions %) permissions: - contents: read + (% for permission_name, permission_level in custom_workflows["merge-gate-extension"].permissions.items() %) + (( permission_name )): (( permission_level )) + (% endfor %) + (% endif %) (% endif %) # This job ensures inputs have been executed successfully. diff --git a/exasol/toolbox/templates/github/workflows/periodic-validation.yml b/exasol/toolbox/templates/github/workflows/periodic-validation.yml index 017f1c0fb..ae3adaa43 100644 --- a/exasol/toolbox/templates/github/workflows/periodic-validation.yml +++ b/exasol/toolbox/templates/github/workflows/periodic-validation.yml @@ -51,8 +51,12 @@ jobs: (( secret_name )): ${{ secrets.(( secret_name )) }} (% endfor %) (% endif %) + (% if custom_workflows["slow-checks"].permissions %) permissions: - contents: read + (% for permission_name, permission_level in custom_workflows["slow-checks"].permissions.items() %) + (( permission_name )): (( permission_level )) + (% endfor %) + (% endif %) report: name: Report diff --git a/exasol/toolbox/util/workflows/custom_workflow.py b/exasol/toolbox/util/workflows/custom_workflow.py index 49c375b61..941142e0e 100644 --- a/exasol/toolbox/util/workflows/custom_workflow.py +++ b/exasol/toolbox/util/workflows/custom_workflow.py @@ -1,5 +1,6 @@ from __future__ import annotations +from enum import IntEnum from pathlib import Path from pydantic import ( @@ -13,12 +14,43 @@ from exasol.toolbox.util.workflows.render_yaml import parse_yaml_text +class PermissionRank(IntEnum): + """GitHub permission levels ranked from least to most permissive.""" + + NONE = 0 + READ = 1 + WRITE = 2 + + @classmethod + def _missing_(cls, value: object) -> PermissionRank: + """Convert a GitHub permission string to its rank.""" + if isinstance(value, str): + for rank in cls: + if rank.name.lower() == value: + return rank + raise ValueError(f"Unknown GitHub permission level: {value!r}") + + +def merge_permissions(permission_maps: list[dict[str, str]]) -> dict[str, str]: + """Merge permission maps to keep the greater permission.""" + merged_permissions: dict[str, str] = {} + for permission_map in permission_maps: + for permission_name, requested_level in permission_map.items(): + current_level = merged_permissions.get(permission_name, None) + if current_level is None: + merged_permissions[permission_name] = requested_level + continue + if PermissionRank(requested_level) > PermissionRank(current_level): # type: ignore[arg-type] + merged_permissions[permission_name] = requested_level + return merged_permissions + + class CustomWorkflow(BaseModel): """A project-owned workflow used for seeded workflows and extensions. These workflows are seeded by the PTB or extend PTB-provided workflows, but they are maintained by the project itself rather than the PTB. See - `Custom Workflows `__. + `Custom Workflows `__. """ model_config = ConfigDict(frozen=True, arbitrary_types_allowed=True) @@ -53,3 +85,28 @@ def extract_secrets(self) -> tuple[str, ...]: if secrets := workflow_call.get("secrets", {}): return tuple(secrets.keys()) return () + + def extract_permissions(self) -> dict[str, str]: + """Return the effective job permissions required by the workflow. + + The extractor scans all jobs and merges their ``permissions`` blocks into a + single mapping. When the same permission appears multiple times, the more + permissive level wins while preserving the first-seen order of the keys. + + For example, a custom workflow can declare permissions like this: + + .. code-block:: yaml + + name: Slow-Checks + + on: + workflow_call: + + jobs: + run-integration-tests: + permissions: + contents: read + """ + jobs = self.yaml_content.get("jobs", {}) + permission_maps = [job.get("permissions", {}) for job in jobs.values()] + return merge_permissions(permission_maps) diff --git a/exasol/toolbox/util/workflows/custom_workflow_extractor.py b/exasol/toolbox/util/workflows/custom_workflow_extractor.py index aeab01b38..8637fd7e3 100644 --- a/exasol/toolbox/util/workflows/custom_workflow_extractor.py +++ b/exasol/toolbox/util/workflows/custom_workflow_extractor.py @@ -8,7 +8,12 @@ field_validator, ) -from exasol.toolbox.util.workflows.custom_workflow import CustomWorkflow +from exasol.toolbox.util.workflows.custom_workflow import ( + CustomWorkflow, + merge_permissions, +) + +MINIMUM_GITHUB_PERMISSIONS: dict[str, str] = {"contents": "read"} class CustomWorkflowEntry(BaseModel): @@ -16,6 +21,7 @@ class CustomWorkflowEntry(BaseModel): exists: bool secrets: tuple[str, ...] + permissions: dict[str, str] @field_validator("secrets", mode="before") @classmethod @@ -23,6 +29,14 @@ def _normalize_secrets(cls, secrets: tuple[str, ...]) -> list[str]: """Return unique secret names in alphabetical order.""" return sorted(set(secrets)) + @field_validator("permissions", mode="before") + @classmethod + def _normalize_permissions( + cls, permissions: list[dict[str, str]] + ) -> dict[str, str]: + """Merge permission maps while preserving first-seen order.""" + return merge_permissions(permissions) + class CustomWorkflowExtractor(BaseModel): model_config = ConfigDict(frozen=True) @@ -43,13 +57,18 @@ def _build_custom_workflow_entry( file_path = self.github_workflow_directory / f"{workflow}.yml" secrets: tuple[str, ...] = () + permissions: list[dict[str, str]] = [] if file_path.is_file(): custom_workflow = CustomWorkflow.load_from_file(file_path=file_path) secrets = custom_workflow.extract_secrets() - + permissions = [ + MINIMUM_GITHUB_PERMISSIONS, + custom_workflow.extract_permissions(), + ] return CustomWorkflowEntry( exists=file_path.exists(), secrets=secrets, + permissions=permissions, ) def _build_merge_gate_entry( @@ -61,6 +80,11 @@ def _build_merge_gate_entry( + custom_workflows_dict["slow-checks"].secrets # from the `report.yml` + (self.sonar_token_name,), + permissions=[ + MINIMUM_GITHUB_PERMISSIONS, + custom_workflows_dict["merge-gate-extension"].permissions, + custom_workflows_dict["slow-checks"].permissions, + ], ) def build_custom_workflow_dict( diff --git a/exasol/toolbox/util/workflows/exceptions.py b/exasol/toolbox/util/workflows/exceptions.py index 1ec4b5479..a87e131eb 100644 --- a/exasol/toolbox/util/workflows/exceptions.py +++ b/exasol/toolbox/util/workflows/exceptions.py @@ -99,18 +99,6 @@ def __init__(self, workflow_name: str, valid_workflows): ) -class NotMaintainedWorkflowError(ValueError): - """ - Raised when a PTB-seeded workflow is requested in an existing project. - """ - - def __init__(self, workflow_name: str): - super().__init__( - f"Workflow '{workflow_name}' is a PTB-seeded workflow that is " - "originally provided by the PTB and can only be seeded for a new project." - ) - - class YamlKeyError(Exception): """ Base exception for when a specified value cannot be found in a YAML. diff --git a/exasol/toolbox/util/workflows/templates.py b/exasol/toolbox/util/workflows/templates.py index 521d1e587..b854e09c7 100644 --- a/exasol/toolbox/util/workflows/templates.py +++ b/exasol/toolbox/util/workflows/templates.py @@ -4,10 +4,7 @@ import importlib_resources as resources -from exasol.toolbox.util.workflows.exceptions import ( - InvalidWorkflowNameError, - NotMaintainedWorkflowError, -) +from exasol.toolbox.util.workflows.exceptions import InvalidWorkflowNameError WORKFLOW_TEMPLATES_DIRECTORY = "exasol.toolbox.templates.github.workflows" NOT_MAINTAINED_WORKFLOW_NAMES: Final[list[str]] = ["slow-checks"] @@ -21,9 +18,23 @@ def get_workflow_templates() -> Mapping[str, Path]: """ package_resources = resources.files(WORKFLOW_TEMPLATES_DIRECTORY) return { - workflow_path.name.removesuffix(".yml"): Path(str(workflow_path)) + workflow_name: Path(str(workflow_path)) for workflow_path in package_resources.iterdir() - if workflow_path.is_file() and workflow_path.name.endswith(".yml") + if workflow_path.is_file() + and workflow_path.name.endswith(".yml") + and (workflow_name := workflow_path.name.removesuffix(".yml")) + not in NOT_MAINTAINED_WORKFLOW_NAMES + } + + +def get_not_maintained_workflow_templates() -> Mapping[str, Path]: + """ + A mapping of not-maintained workflow template names to paths. + """ + package_resources = resources.files(WORKFLOW_TEMPLATES_DIRECTORY) + return { + workflow_name: Path(str(package_resources / f"{workflow_name}.yml")) + for workflow_name in NOT_MAINTAINED_WORKFLOW_NAMES } @@ -33,8 +44,6 @@ def validate_workflow_name(workflow_name: str) -> str: """ if workflow_name not in WORKFLOW_TEMPLATE_OPTIONS: raise InvalidWorkflowNameError(workflow_name, WORKFLOW_TEMPLATE_OPTIONS.keys()) - if workflow_name in NOT_MAINTAINED_WORKFLOW_NAMES: - raise NotMaintainedWorkflowError(workflow_name) return workflow_name diff --git a/exasol/toolbox/util/workflows/workflow_orchestrator.py b/exasol/toolbox/util/workflows/workflow_orchestrator.py index 646653aac..6e630b298 100644 --- a/exasol/toolbox/util/workflows/workflow_orchestrator.py +++ b/exasol/toolbox/util/workflows/workflow_orchestrator.py @@ -17,7 +17,6 @@ from exasol.toolbox.util.workflows import logger from exasol.toolbox.util.workflows.exceptions import ( InvalidWorkflowPatcherEntryError, - NotMaintainedWorkflowError, YamlKeyError, ) from exasol.toolbox.util.workflows.patch_workflow import ( @@ -27,7 +26,7 @@ from exasol.toolbox.util.workflows.templates import ( DOCUMENTATION_ONLY_WORKFLOW_NAMES, WORKFLOW_TEMPLATE_OPTIONS, - validate_workflow_name, + get_not_maintained_workflow_templates, ) from exasol.toolbox.util.workflows.workflow import Workflow @@ -79,13 +78,12 @@ def _is_new_project(self) -> bool: """ return not any(self.config.github_workflow_directory.glob("*.yml")) - def _iter_workflows(self) -> Iterator[Workflow]: - logger.info(f"Selected workflow(s) to update: {list(self.templates.keys())}") - is_new_project = self._is_new_project() - for workflow_name, template_path in self.templates.items(): + def _iter_workflows(self, templates: Mapping[str, Path]) -> Iterator[Workflow]: + logger.info(f"Selected workflow(s) to update: {list(templates.keys())}") + for workflow_name, template_path in templates.items(): patch_yaml = self._extract_workflow_patch(workflow_name=workflow_name) - if self._skip_workflow(workflow_name, is_new_project): + if self._skip_workflow(workflow_name): continue yield self._load_workflow( @@ -108,21 +106,11 @@ def _load_workflow( entry=ex.entry, ) from ex - def _skip_workflow(self, workflow_name: str, is_new_project: bool) -> bool: + def _skip_workflow(self, workflow_name: str) -> bool: """ Return ``True`` if the workflow should be skipped because it is not maintained by the PTB or not applicable to the current project, otherwise return ``False``. """ - try: - validate_workflow_name(workflow_name) - except NotMaintainedWorkflowError: - if not is_new_project: - logger.debug( - "Skipping not-maintained workflow in older project: %s", - workflow_name, - ) - return True - if ( workflow_name in DOCUMENTATION_ONLY_WORKFLOW_NAMES and not self.config.has_documentation @@ -139,7 +127,16 @@ def generate_workflows(self) -> None: """ Render the selected workflows and write them to disk. """ - for workflow in self._iter_workflows(): + # First, generate not-maintained workflows for a new project. + # This ensures proper extraction and passing of secrets & permissions + # before parent workflows such as `merge-gate.yml` and + # `periodic-validation.yml` are rendered. + if self._is_new_project() and self.workflow_choice == ALL: + not_maintainted_templates = get_not_maintained_workflow_templates() + for workflow in self._iter_workflows(not_maintainted_templates): + workflow.write_to_file() + + for workflow in self._iter_workflows(self.templates): workflow.write_to_file() def find_differing_workflows(self) -> list[str]: @@ -149,7 +146,7 @@ def find_differing_workflows(self) -> list[str]: Returns the names of the workflows that differ from the file on disk. """ outdated_workflows: list[str] = [] - for workflow in self._iter_workflows(): + for workflow in self._iter_workflows(self.templates): comparison = workflow.compare_to_file() if comparison != "": workflow_name = workflow.output_path.stem diff --git a/test/unit/config_test.py b/test/unit/config_test.py index 580b816cb..5fa06445b 100644 --- a/test/unit/config_test.py +++ b/test/unit/config_test.py @@ -51,11 +51,31 @@ def test_works_as_defined(tmp_path, test_project_config_factory): "github_workflow_patcher_yaml": None, "github_template_dict": { "custom_workflows": { - "cd-extension": {"exists": False, "secrets": ()}, - "fast-tests-extension": {"exists": False, "secrets": ()}, - "merge-gate-extension": {"exists": False, "secrets": ()}, - "slow-checks": {"exists": False, "secrets": ()}, - "merge-gate": {"exists": True, "secrets": ("SONAR_TOKEN",)}, + "cd-extension": { + "exists": False, + "secrets": (), + "permissions": {}, + }, + "fast-tests-extension": { + "exists": False, + "secrets": (), + "permissions": {}, + }, + "merge-gate-extension": { + "exists": False, + "secrets": (), + "permissions": {}, + }, + "slow-checks": { + "exists": False, + "secrets": (), + "permissions": {}, + }, + "merge-gate": { + "exists": True, + "secrets": ("SONAR_TOKEN",), + "permissions": {"contents": "read"}, + }, }, "dependency_manager_version": "2.3.0", "has_documentation": True, diff --git a/test/unit/nox/_workflow_test.py b/test/unit/nox/_workflow_test.py index b69d7f3ed..ade8112bf 100644 --- a/test/unit/nox/_workflow_test.py +++ b/test/unit/nox/_workflow_test.py @@ -182,7 +182,7 @@ def test_raises_session_quit_when_workflows_are_out_of_date( check_workflow(nox_session) assert str(exc.value) == ( - "\n14 workflows are out of date:\n" + "\n13 workflows are out of date:\n" "- build-and-publish\n" "- cd\n" "- check-release-tag\n" @@ -195,8 +195,7 @@ def test_raises_session_quit_when_workflows_are_out_of_date( "- merge-gate\n" "- periodic-validation\n" "- pr-merge\n" - "- report\n" - "- slow-checks" + "- report" ) @staticmethod @@ -221,7 +220,7 @@ def test_raises_session_quit_without_documentation_workflows( check_workflow(nox_session) assert str(exc.value) == ( - "\n12 workflows are out of date:\n" + "\n11 workflows are out of date:\n" "- build-and-publish\n" "- cd\n" "- check-release-tag\n" @@ -232,8 +231,7 @@ def test_raises_session_quit_without_documentation_workflows( "- matrix\n" "- merge-gate\n" "- periodic-validation\n" - "- report\n" - "- slow-checks" + "- report" ) diff --git a/test/unit/util/workflows/custom_workflow_extractor_test.py b/test/unit/util/workflows/custom_workflow_extractor_test.py index 608fa15c3..7295c1508 100644 --- a/test/unit/util/workflows/custom_workflow_extractor_test.py +++ b/test/unit/util/workflows/custom_workflow_extractor_test.py @@ -29,11 +29,13 @@ def test_secrets_are_normalized_on_creation(): custom_workflow_entry = CustomWorkflowEntry( exists=True, secrets=("ZETA_SECRET", "ALPHA_SECRET", "ALPHA_SECRET"), + permissions=[{"contents": "read"}], ) assert custom_workflow_entry == CustomWorkflowEntry( exists=True, secrets=("ALPHA_SECRET", "ZETA_SECRET"), + permissions=[{"contents": "read"}], ) @@ -47,6 +49,7 @@ def test_file_does_not_exist(custom_workflow_extractor): assert custom_workflow_entry == CustomWorkflowEntry( exists=False, secrets=(), + permissions=[], ) @staticmethod @@ -69,6 +72,7 @@ def test_file_does_not_contain_secrets( assert custom_workflow_entry == CustomWorkflowEntry( exists=True, secrets=(), + permissions=[{"contents": "read"}], ) @staticmethod @@ -92,6 +96,44 @@ def test_file_contains_secret(custom_workflow_extractor, workflow_directory): assert custom_workflow_entry == CustomWorkflowEntry( exists=True, secrets=("SLOW_CHECK_SECRET",), + permissions=[{"contents": "read"}], + ) + + @staticmethod + def test_file_contains_permissions(custom_workflow_extractor, workflow_directory): + workflow = "slow-checks" + workflow_path = workflow_directory / f"{workflow}.yml" + workflow_path.write_text(cleandoc(""" + name: Slow-Checks + + on: + workflow_call: + + jobs: + run-integration-tests: + permissions: + contents: read + packages: write + publish-results: + permissions: + contents: write + id-token: write + """)) + + custom_workflow_entry = custom_workflow_extractor._build_custom_workflow_entry( + workflow=workflow + ) + + assert custom_workflow_entry == CustomWorkflowEntry( + exists=True, + secrets=(), + permissions=[ + { + "contents": "write", + "packages": "write", + "id-token": "write", + } + ], ) @@ -102,10 +144,22 @@ def test_build_merge_gate_entry(custom_workflow_extractor): "merge-gate-extension": CustomWorkflowEntry( exists=True, secrets=("EXT_SECRET",), + permissions=[ + { + "contents": "read", + "packages": "read", + } + ], ), "slow-checks": CustomWorkflowEntry( exists=True, secrets=("SLOW_SECRET",), + permissions=[ + { + "packages": "write", + "id-token": "write", + } + ], ), } @@ -116,16 +170,31 @@ def test_build_merge_gate_entry(custom_workflow_extractor): assert merge_gate_entry == CustomWorkflowEntry( exists=True, secrets=("EXT_SECRET", "SLOW_SECRET", "SONAR_TOKEN"), + permissions=[ + { + "contents": "read", + "packages": "write", + "id-token": "write", + } + ], ) class TestBuildCustomWorkflowDict: default_custom_workflow_dict = { - "cd-extension": CustomWorkflowEntry(exists=False, secrets=()), - "fast-tests-extension": CustomWorkflowEntry(exists=False, secrets=()), - "merge-gate-extension": CustomWorkflowEntry(exists=False, secrets=()), - "slow-checks": CustomWorkflowEntry(exists=False, secrets=()), - "merge-gate": CustomWorkflowEntry(exists=True, secrets=("SONAR_TOKEN",)), + "cd-extension": CustomWorkflowEntry(exists=False, secrets=(), permissions=[]), + "fast-tests-extension": CustomWorkflowEntry( + exists=False, secrets=(), permissions=[] + ), + "merge-gate-extension": CustomWorkflowEntry( + exists=False, secrets=(), permissions=[] + ), + "slow-checks": CustomWorkflowEntry(exists=False, secrets=(), permissions=[]), + "merge-gate": CustomWorkflowEntry( + exists=True, + secrets=("SONAR_TOKEN",), + permissions=[{"contents": "read"}], + ), } def test_no_custom_workflows_exist(self, custom_workflow_extractor): @@ -166,6 +235,7 @@ def test_custom_workflow_written_to( expected_custom_workflow_dict[workflow] = CustomWorkflowEntry( exists=True, secrets=(secret,), + permissions=[{"contents": "read"}], ) if workflow in ("merge-gate-extension", "slow-checks"): expected_custom_workflow_dict["merge-gate"] = CustomWorkflowEntry( @@ -174,6 +244,42 @@ def test_custom_workflow_written_to( secret, "SONAR_TOKEN", ), + permissions=[{"contents": "read"}], ) assert custom_workflow_dict == expected_custom_workflow_dict + + @staticmethod + def test_merge_gate_permissions_are_aggregated_in_order(custom_workflow_extractor): + custom_workflows_dict = { + "merge-gate-extension": CustomWorkflowEntry( + exists=True, + secrets=(), + permissions=[ + { + "contents": "read", + "packages": "read", + } + ], + ), + "slow-checks": CustomWorkflowEntry( + exists=True, + secrets=(), + permissions=[ + { + "packages": "write", + "id-token": "write", + } + ], + ), + } + + merge_gate_entry = custom_workflow_extractor._build_merge_gate_entry( + custom_workflows_dict + ) + + assert list(merge_gate_entry.permissions.items()) == [ + ("contents", "read"), + ("packages", "write"), + ("id-token", "write"), + ] diff --git a/test/unit/util/workflows/custom_workflow_test.py b/test/unit/util/workflows/custom_workflow_test.py index 14fc74b53..27e2578d3 100644 --- a/test/unit/util/workflows/custom_workflow_test.py +++ b/test/unit/util/workflows/custom_workflow_test.py @@ -3,6 +3,7 @@ import pytest +from exasol.toolbox.util.workflows.custom_workflow import merge_permissions from exasol.toolbox.util.workflows.custom_workflow_extractor import CustomWorkflow @@ -24,6 +25,23 @@ class TestCustomWorkflow: required: true """) + yaml_with_permissions = cleandoc(""" + name: Build & Publish + + on: + workflow_call: + + jobs: + build: + permissions: + contents: read + packages: read + publish: + permissions: + contents: write + id-token: write + """) + yaml_without_secrets = cleandoc(""" name: Build & Publish @@ -31,6 +49,17 @@ class TestCustomWorkflow: workflow_call: """) + yaml_with_job_without_permissions = cleandoc(""" + name: Build & Publish + + on: + workflow_call: + + jobs: + build: + runs-on: ubuntu-latest + """) + def test_load_from_file(self, test_yml): test_yml.write_text(self.yaml_with_secrets) @@ -53,3 +82,46 @@ def test_extract_secrets_when_no_secrets_present(self, test_yml): secrets = custom_workflow.extract_secrets() assert secrets == () + + def test_extract_permissions_when_present(self, test_yml): + test_yml.write_text(self.yaml_with_permissions) + + custom_workflow = CustomWorkflow.load_from_file(file_path=test_yml) + permissions = custom_workflow.extract_permissions() + + assert list(permissions.items()) == [ + ("contents", "write"), + ("packages", "read"), + ("id-token", "write"), + ] + + def test_extract_permissions_when_no_jobs_present(self, test_yml): + test_yml.write_text(self.yaml_without_secrets) + + custom_workflow = CustomWorkflow.load_from_file(file_path=test_yml) + permissions = custom_workflow.extract_permissions() + + assert permissions == {} + + def test_extract_permissions_when_job_has_no_permissions(self, test_yml): + test_yml.write_text(self.yaml_with_job_without_permissions) + + custom_workflow = CustomWorkflow.load_from_file(file_path=test_yml) + permissions = custom_workflow.extract_permissions() + + assert permissions == {} + + def test_merge_permissions_keeps_most_permissive_level(self): + permissions = merge_permissions( + [ + {"contents": "read", "packages": "read"}, + {"contents": "write", "issues": "read"}, + {"packages": "read", "issues": "write"}, + ] + ) + + assert list(permissions.items()) == [ + ("contents", "write"), + ("packages", "read"), + ("issues", "write"), + ] diff --git a/test/unit/util/workflows/patch_workflow_test.py b/test/unit/util/workflows/patch_workflow_test.py index e5b69618e..f8aa21978 100644 --- a/test/unit/util/workflows/patch_workflow_test.py +++ b/test/unit/util/workflows/patch_workflow_test.py @@ -6,15 +6,11 @@ from pydantic import ValidationError from ruamel.yaml import CommentedMap -from exasol.toolbox.util.workflows.exceptions import ( - InvalidWorkflowPatcherYamlError, - NotMaintainedWorkflowError, -) +from exasol.toolbox.util.workflows.exceptions import InvalidWorkflowPatcherYamlError from exasol.toolbox.util.workflows.patch_workflow import ( ActionType, WorkflowPatcher, ) -from exasol.toolbox.util.workflows.templates import NOT_MAINTAINED_WORKFLOW_NAMES @pytest.fixture @@ -139,30 +135,3 @@ def test_raises_error_for_unknown_workflow_name( assert "Invalid workflow: unknown-workflow. Must be one of dict_keys([" in str( underlying_error ) - - @staticmethod - def test_rejects_not_maintained_workflow_name( - workflow_patcher_yaml, workflow_patcher - ): - content = f""" - workflows: - - name: "{NOT_MAINTAINED_WORKFLOW_NAMES[0]}" - remove_jobs: - - build-documentation-and-check-links - """ - workflow_patcher_yaml.write_text(cleandoc(content)) - - with pytest.raises( - InvalidWorkflowPatcherYamlError, - match="is malformed; it failed Pydantic validation", - ) as ex: - workflow_patcher.content - - underlying_error = ex.value.__cause__ - assert isinstance(underlying_error, ValidationError) - assert "workflows.0.name" in str(underlying_error) - assert "PTB-seeded workflow" in str(underlying_error) - assert isinstance( - ex.value.validation_error.errors()[0]["ctx"]["error"], - NotMaintainedWorkflowError, - ) diff --git a/test/unit/util/workflows/render_yaml_test.py b/test/unit/util/workflows/render_yaml_test.py index 7f0606f6f..61611c05e 100644 --- a/test/unit/util/workflows/render_yaml_test.py +++ b/test/unit/util/workflows/render_yaml_test.py @@ -521,6 +521,94 @@ def test_includes_extension_with_multiple_secrets(test_yml, project_config): yaml_dict = yaml_renderer.get_yaml_dict() assert yaml_renderer.get_as_string(yaml_dict) == cleandoc(expected_yaml) + @staticmethod + def test_includes_extension_with_custom_permissions(test_yml, project_config): + input_yaml = """ + jobs: + run-unit-tests: + name: Unit Tests (Python-${{ matrix.python-versions }}) + runs-on: "(( os_version ))" + permissions: + contents: read + strategy: + fail-fast: false + matrix: + python-versions: (( python_versions | tojson )) + + steps: + - name: Check out Repository + id: check-out-repository + uses: actions/checkout@v6 + + (% if custom_workflows["fast-tests-extension"].exists %) + fast-tests-extension: + name: Extension + uses: ./.github/workflows/fast-tests-extension.yml + (% if custom_workflows["fast-tests-extension"].permissions %) + permissions: + (% for permission_name, permission_level in custom_workflows["fast-tests-extension"].permissions.items() %) + (( permission_name )): (( permission_level )) + (% endfor %) + (% endif %) + (% endif %) + + """ + expected_yaml = """ + jobs: + run-unit-tests: + name: 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 + + fast-tests-extension: + name: Extension + uses: ./.github/workflows/fast-tests-extension.yml + permissions: + contents: write + packages: read + id-token: write + + """ + workflow_directory = project_config.github_workflow_directory + workflow_directory.mkdir(parents=True, exist_ok=True) + (workflow_directory / "fast-tests-extension.yml").write_text(cleandoc(""" + name: Fast-Tests-Extension + + on: + workflow_call: + + jobs: + build-artifacts: + permissions: + contents: read + packages: read + publish-artifacts: + permissions: + contents: write + id-token: write + """)) + + content = cleandoc(input_yaml) + test_yml.write_text(content) + + yaml_renderer = YamlRenderer( + github_template_dict=project_config.github_template_dict, + file_path=test_yml, + ) + + 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 = """ diff --git a/test/unit/util/workflows/templates_test.py b/test/unit/util/workflows/templates_test.py index 8969ad658..ab2679203 100644 --- a/test/unit/util/workflows/templates_test.py +++ b/test/unit/util/workflows/templates_test.py @@ -1,9 +1,6 @@ import pytest -from exasol.toolbox.util.workflows.exceptions import ( - InvalidWorkflowNameError, - NotMaintainedWorkflowError, -) +from exasol.toolbox.util.workflows.exceptions import InvalidWorkflowNameError from exasol.toolbox.util.workflows.templates import ( NOT_MAINTAINED_WORKFLOW_NAMES, WORKFLOW_TEMPLATE_OPTIONS, @@ -30,7 +27,6 @@ def test_get_workflow_templates(project_config): "periodic-validation", "pr-merge", "report", - "slow-checks", } # check only one path, as all formatted the same by convention assert ( @@ -57,8 +53,3 @@ def test_returns_valid_maintained_names(workflow_name): def test_rejects_unknown_workflow(): with pytest.raises(InvalidWorkflowNameError, match="Invalid workflow: unknown"): validate_workflow_name("unknown") - - @staticmethod - def test_rejects_not_maintained_workflow(): - with pytest.raises(NotMaintainedWorkflowError, match="PTB-seeded workflow"): - validate_workflow_name(NOT_MAINTAINED_WORKFLOW_NAMES[0]) diff --git a/test/unit/util/workflows/workflow_orchestrator_test.py b/test/unit/util/workflows/workflow_orchestrator_test.py index 63316dd5e..6758312f5 100644 --- a/test/unit/util/workflows/workflow_orchestrator_test.py +++ b/test/unit/util/workflows/workflow_orchestrator_test.py @@ -69,36 +69,9 @@ def test_returns_false_when_yml_files_exist(project_config): class TestSkipWorkflow: - @staticmethod - def test_returns_true_for_not_maintained_workflow_in_existing_project( - project_config, - ): - workflow_directory = project_config.github_workflow_directory - workflow_directory.mkdir(parents=True) - (workflow_directory / "existing.yml").touch() - - result = WorkflowOrchestrator( - workflow_choice=NOT_MAINTAINED_WORKFLOW_NAMES[0], - config=project_config, - )._skip_workflow( - workflow_name=NOT_MAINTAINED_WORKFLOW_NAMES[0], - is_new_project=False, - ) - - assert result is True - - @staticmethod - def test_returns_false_for_maintained_workflow(project_config): - result = WorkflowOrchestrator( - workflow_choice="checks", - config=project_config, - )._skip_workflow(workflow_name="checks", is_new_project=False) - - assert result is False - @staticmethod @pytest.mark.parametrize("workflow_name", DOCUMENTATION_ONLY_WORKFLOW_NAMES) - def test_returns_ralse_for_documentation_only_workflow_when_docs_enabled( + def test_returns_false_for_documentation_only_workflow_when_docs_enabled( project_config, workflow_name ): @@ -107,7 +80,6 @@ def test_returns_ralse_for_documentation_only_workflow_when_docs_enabled( config=project_config, )._skip_workflow( workflow_name=workflow_name, - is_new_project=False, ) assert result is False @@ -130,7 +102,6 @@ def has_documentation(self) -> bool: config=config, )._skip_workflow( workflow_name=workflow_name, - is_new_project=False, ) assert result is True @@ -143,10 +114,11 @@ def test_works_as_expected_without_patcher(project_config_without_patcher): project_config_without_patcher.github_workflow_directory.mkdir(parents=True) input_text = WORKFLOW_TEMPLATE_OPTIONS[workflow_name].read_text() - result = WorkflowOrchestrator( + orchestrator = WorkflowOrchestrator( workflow_choice=workflow_name, config=project_config_without_patcher, - )._iter_workflows() + ) + result = orchestrator._iter_workflows(orchestrator.templates) result = list(result) assert len(result) == 1 @@ -166,10 +138,11 @@ def test_works_as_expected_with_relevant_patcher(project_config, remove_job_yaml assert removed_job_name in remove_job_yaml assert removed_job_name in input_text - result = WorkflowOrchestrator( + orchestrator = WorkflowOrchestrator( workflow_choice=workflow_name, config=project_config, - )._iter_workflows() + ) + result = orchestrator._iter_workflows(orchestrator.templates) result = list(result) assert len(result) == 1 @@ -188,10 +161,11 @@ def test_works_as_expected_with_not_relevant_patcher( project_config.github_workflow_directory.mkdir(parents=True) input_text = WORKFLOW_TEMPLATE_OPTIONS[workflow_name].read_text() - result = WorkflowOrchestrator( + orchestrator = WorkflowOrchestrator( workflow_choice=workflow_name, config=project_config, - )._iter_workflows() + ) + result = orchestrator._iter_workflows(orchestrator.templates) result = list(result) assert len(result) == 1 @@ -201,55 +175,6 @@ def test_works_as_expected_with_not_relevant_patcher( == _remove_header(input_text)[:10] ) - @staticmethod - def test_not_maintained_workflows_added_to_new_project( - project_config_without_patcher, - ): - directory = project_config_without_patcher.github_workflow_directory - directory.mkdir(parents=True) - - result = WorkflowOrchestrator( - workflow_choice="all", - config=project_config_without_patcher, - )._iter_workflows() - - assert {f"{name}.yml" for name in NOT_MAINTAINED_WORKFLOW_NAMES}.issubset( - {workflow.output_path.name for workflow in result} - ) - - @staticmethod - @pytest.mark.parametrize("workflow_name", NOT_MAINTAINED_WORKFLOW_NAMES) - def test_not_maintained_workflows_not_modified_in_old_project( - project_config_without_patcher, workflow_name - ): - directory = project_config_without_patcher.github_workflow_directory - directory.mkdir(parents=True, exist_ok=True) - workflow = "slow-checks.yml" - (directory / workflow).touch() - - result = WorkflowOrchestrator( - workflow_choice=workflow_name, - config=project_config_without_patcher, - )._iter_workflows() - - assert list(result) == [] - - @staticmethod - @pytest.mark.parametrize("workflow_name", NOT_MAINTAINED_WORKFLOW_NAMES) - def test_not_maintained_workflows_not_added_to_old_project( - project_config_without_patcher, workflow_name - ): - directory = project_config_without_patcher.github_workflow_directory - directory.mkdir(parents=True, exist_ok=True) - (directory / "dummy.yml").touch() - - result = WorkflowOrchestrator( - workflow_choice=workflow_name, - config=project_config_without_patcher, - )._iter_workflows() - - assert list(result) == [] - @staticmethod def test_raises_invalidworkflowpatcherentryerror(project_config): patcher_yml = """ @@ -261,10 +186,11 @@ def test_raises_invalidworkflowpatcherentryerror(project_config): project_config.github_workflow_patcher_yaml.write_text(patcher_yml) with pytest.raises(InvalidWorkflowPatcherEntryError) as ex: - for _ in WorkflowOrchestrator( + orchestrator = WorkflowOrchestrator( workflow_choice="checks", config=project_config, - )._iter_workflows(): + ) + for _ in orchestrator._iter_workflows(orchestrator.templates): pass assert ( @@ -289,6 +215,23 @@ def test_writes_all_workflows_on_fresh_project(project_config_without_patcher): (directory / f"{name}.yml").exists() for name in WORKFLOW_TEMPLATE_OPTIONS ) + @staticmethod + def test_writes_not_maintained_workflows_on_fresh_project( + project_config_without_patcher, + ): + directory = project_config_without_patcher.github_workflow_directory + directory.mkdir(parents=True) + + WorkflowOrchestrator( + workflow_choice="all", + config=project_config_without_patcher, + ).generate_workflows() + + assert all( + (directory / f"{name}.yml").exists() + for name in NOT_MAINTAINED_WORKFLOW_NAMES + ) + @staticmethod def test_does_not_write_when_all_workflows_are_up_to_date( project_config_without_patcher,