Feature/922 make permissions configurable for custom workflows#923
Feature/922 make permissions configurable for custom workflows#923ArBridgeman wants to merge 12 commits into
Conversation
10e54e9 to
3b03bca
Compare
… be handled separately
- This is already present in the CLI of the nox sessions workflow:check and workflow:generate - This is already required by the WorkflowOrchestrator which is a pydantic model which restricts WorkflowChoice and hard-code maps it to the right values.
1eae0d1 to
69e8d14
Compare
| Render the selected workflows and write them to disk. | ||
| """ | ||
| for workflow in self._iter_workflows(): | ||
| # First, generate not-maintained workflows for a new project. |
There was a problem hiding this comment.
This was actually an unidentified bug in the setup. So when we just had secrets, it went unnoticed as the default slow-checks.yml doesn't have secrets that need to be passed. However, as permissions should always be set to adhere to the best security practices; it's best that we separate the two workflow types PTB-controlled vs custom at a higher level.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69e8d1492a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…r_custom_workflows
|
|
|
||
| ## Feature | ||
|
|
||
| * #922: Extended `custom_workflows` of `github_template_dict` for automatic custom workflow permissions extraction |
There was a problem hiding this comment.
| * #922: Extended `custom_workflows` of `github_template_dict` for automatic custom workflow permissions extraction | |
| * #922: Extended `custom_workflows` of `github_template_dict` for automatic custom workflow permissions extraction |
| WRITE = 2 | ||
|
|
||
| @classmethod | ||
| def _missing_(cls, value: object) -> PermissionRank: |
There was a problem hiding this comment.
I get an error, that PermissionRank is unknown!?
| def _missing_(cls, value: object) -> PermissionRank: | |
| def _missing_(cls, value: object) -> Self: |
| * #875: Added `name` attribute to generated workflow jobs using `-extension.yml` workflows | ||
|
|
||
| ## Bug | ||
| ## Bug Fix |
There was a problem hiding this comment.
See https://github.com/exasol/python-toolbox/blob/main/exasol/toolbox/util/release/changelog.py#L34
| ## Bug Fix | |
| ## Bugfixes |
|
|
||
| ## Summary | ||
|
|
||
| ## Feature |
There was a problem hiding this comment.
See https://github.com/exasol/python-toolbox/blob/main/exasol/toolbox/util/release/changelog.py#L34
| ## Feature | |
| ## Features |
| ------- | ||
|
|
||
| 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 |
There was a problem hiding this comment.
| The PTB extracts secret names from custom workflow files and exposes them | |
| The PTB extracts the names of secrets from custom workflow files and exposes them |
| Custom Workflow Metadata | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| The PTB extracts metadata from reusable custom workflow files and exposes it |
There was a problem hiding this comment.
I like the general and technical description.
Could we, for more straight-forward and impatient users, add a small section with concrete instructions?
- What do I need to do?
- When? (In which Scenario / Use Case / Problem)
- Example, maybe a pointer to one of the PTB's workflow templates illustrating the usage?
| (% if custom_workflows["merge-gate"].permissions %) | ||
| permissions: | ||
| contents: read | ||
| (% for permission_name, permission_level in custom_workflows["merge-gate"].permissions.items() %) |
There was a problem hiding this comment.
As this pattern repeats 3-4 times - would it make sense to move the functionality into an API function
and in the jinja template only have a call like this (not sure if this is possible)?:
(% workflow_permissions("merge-gate") %)| raise ValueError(f"Unknown GitHub permission level: {value!r}") | ||
|
|
||
|
|
||
| def merge_permissions(permission_maps: list[dict[str, str]]) -> dict[str, str]: |
There was a problem hiding this comment.
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.
| 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>`__. |
There was a problem hiding this comment.
Uh - Oh - sorry for this accidental artifact!



closes #922
Checklist
Note: If any of the items in the checklist are not relevant to your PR, just check the box.
For any Pull Request
Is the following correct:
When Changes Were Made
Did you:
When Preparing a Release
Have you: