Skip to content

fix(docs): add RBAC and user to executor plugins example #13019

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.

Sign up for GitHub

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

Open
dgolja wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(docs): add RBAC and user to executor plugins example #13019

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c9aaf5
fix(doc): Update executor plugins doc with additional RBAB informatio…
dgolja May 8, 2024
ea4c775
refactor the permissions section and improve grammar.
dgolja May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion docs/executor_plugins.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ controller:
value: "true"
```

In case you want to modify a running `workflow-controller`:

```bash
kubectl patch deployment \
workflow-controller \
--namespace argo \
-p '{"spec":{"template":{"spec":{"containers":[{"name":"workflow-controller","env":[{"name":"ARGO_EXECUTOR_PLUGINS","value":"true"}]}]}}}}'
```

## Template Executor

This is a plugin that runs custom "plugin" templates, e.g. for non-pod tasks such as Tekton builds, Spark jobs, sending
Expand Down Expand Up @@ -162,7 +171,7 @@ spec:
- containerPort: 4355
securityContext:
runAsNonRoot: true
runAsUser: 65534 # nobody
runAsUser: 1000
resources:
requests:
memory: "64Mi"
Expand Down Expand Up @@ -202,6 +211,30 @@ spec:

You'll see the workflow complete successfully.

**Note**: The service account running the workflow needs at least the following permissions. If <= v3.2 you must replace `workflowtasksets/status` with `workflowtasksets`.
Copy link

@agilgur5 agilgur5 May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this block should go in the ## Configuration section above a subsection ### Permissions. It's more important than a "Note" -- this is required

Copy link

@agilgur5 agilgur5 May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Argo "agent" is mentioned in the HTTP template docs as well. Ideally we'd unify them and have a page on the Agent that both link to with regard to permissions / more information. The Agent is poorly documented; I didn't even know much about it until a few months ago

Potentially a section in the existing Workflow RBAC page might make sense, which is what the quick-start manifest already links to

Copy link

@agilgur5 agilgur5 May 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also leave the "If <= v3.2" out of this and leave it in the Workflow description annotation like the quick start manifest. Let's keep the two identical.

Even better would be to make that an in-line comment on the workflowtasksets/status line in both

Copy link
Author

@dgolja dgolja May 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know what you think about the latest change. I added the ### Permissions section with some relevant links and removed the Role definition. Not every user may experience permission issues, because it depends on how they installed WF. I think this new section letting them know they may need to adjust the permissions are adequate for now.

I agree that the documentation for the agent and permissions, in general, could be better. Let me think about it a bit more, and for now, I'll provide this as a stopgap. I may create a follow-up PR to improve the permissions documentation, but first, I need to dig more into the code to fully understand it.

Copy link

@agilgur5 agilgur5 May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this new section letting them know they may need to adjust the permissions are adequate for now.

Yea we'd still leave this section or something similar if we move the actual Agent docs to its own page; this section would just link to that, something like "Plugins use the Argo Agent, ensure you have appropriate RBAC for your Workflow".

Let me know what you think about the latest change.

It looks like the current text is mostly just a copy of the HTTP Template doc? That section causes confusion as-is the way it is written so if we're going to add it, I'd prefer to unify in the Workflow RBAC page. We can just have a short description there and show the RBAC. Can improve that in later PRs

I can help guide that if you're ok relying on my knowledge.


```yaml
---
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: agent
rules:
- apiGroups:
- argoproj.io
resources:
- workflowtasksets
verbs:
- list
- watch
- apiGroups:
- argoproj.io
resources:
- workflowtasksets/status
verbs:
- patch
```

### Discovery

When a workflow is run, plugins are loaded from:
Expand Down