-
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 12 commits
7a832d8
c37d96c
5d7f0ed
620fde3
24cfb7b
bea075f
92a1ae4
c3d2872
0a36b86
1ac5e10
69e8d14
30a3ac2
08f30e6
70a9f21
c1028c5
cfc71fe
29d22ac
98dbec0
3663d2b
5087874
d9b094c
85d2873
f9680a8
923c88e
4e01cdf
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 |
|---|---|---|
|
|
@@ -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?
Collaborator
Author
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 for impatient users they can use codex to summarize what's already there.
Collaborator
Author
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. |
||
| 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 | ||
|
ArBridgeman marked this conversation as resolved.
Outdated
|
||
| 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") %)
Collaborator
Author
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 implemented this separately for secrets being passed and permissions. |
||
| (( 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: | ||
|
ArBridgeman marked this conversation as resolved.
Outdated
|
||
| """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.
Collaborator
Author
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. Ok, I removed the enum and simplified it. |
||
| """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>`__. | ||
|
ArBridgeman marked this conversation as resolved.
|
||
| """ | ||
|
|
||
| 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.
|
||

Uh oh!
There was an error while loading. Please reload this page.