-
Notifications
You must be signed in to change notification settings - Fork 2
Feature/922 make permissions configurable for custom workflows #923
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
7a832d8
c37d96c
5d7f0ed
620fde3
24cfb7b
bea075f
92a1ae4
c3d2872
0a36b86
1ac5e10
69e8d14
30a3ac2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,10 @@ | |||||
|
|
||||||
| ## Summary | ||||||
|
|
||||||
| ## Feature | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See https://github.com/exasol/python-toolbox/blob/main/exasol/toolbox/util/release/changelog.py#L34
Suggested change
|
||||||
|
|
||||||
| * #922: Extended `custom_workflows` of `github_template_dict` for automatic custom workflow permissions extraction | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| ## Refactoring | ||||||
|
|
||||||
| * #924: Removed `lint:dependencies` usage from `report.yml` and added deprecation notice | ||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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 | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the general and technical description. Could we, for more straight-forward and impatient users, add a small section with concrete instructions?
|
||||||
| 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 | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| 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 | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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() %) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As this pattern repeats 3-4 times - would it make sense to move the functionality into an API function (% workflow_permissions("merge-gate") %) |
||
| (( permission_name )): (( permission_level )) | ||
| (% endfor %) | ||
| (% endif %) | ||
|
|
||
| report: | ||
| name: Report | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -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: | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I get an error, that PermissionRank is unknown!?
Suggested change
|
||||||
| """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]: | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I can get the goal of the implementation, but my stomach says there must be a simpler, shorter way. How about this approach? Permissions: TypeAlias = dict[str, str]
"""
Each permission is defined by a name as it key and a value, which is
either None, or "read" or "write".
"""
def merge_permissions(permission_maps: list[Permissions]) -> Permissions:
"""Merge permission maps to keep the greater permission."""
rank = {None: 0, "read": 1, "write": 2}
result = {}
def max_permission(perm: str, value: str) -> str:
current = result.get(perm)
return value if rank[value] > rank[current] else current
for other in permission_maps:
result |= {p: max_permission(p, v) for p, v in other.items()}
return resultNo enum required, < 70% characters, tests still green. |
||||||
| """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(): | ||||||
|
ArBridgeman marked this conversation as resolved.
|
||||||
| 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 <file:///home/chku/git/ptb/.html-documentation/user_guide/features/github_workflows/index.html#custom-workflows>`__. | ||||||
| `Custom Workflows <https://exasol.github.io/python-toolbox/main/user_guide/features/github_workflows/index.html#custom-workflows>`__. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Uh - Oh - sorry for this accidental artifact! |
||||||
| """ | ||||||
|
|
||||||
| 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) | ||||||
|
ArBridgeman marked this conversation as resolved.
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See https://github.com/exasol/python-toolbox/blob/main/exasol/toolbox/util/release/changelog.py#L34