argo-workflows
argo-workflows copied to clipboard
service-account defined for individual steps.
Buenos Dias :)
Our team is making research around argo-workflow project usage, and we found one potential security-issue (because of company guidelines) - we want to address it somehow, potentially contributing PR, but first I wanted to consult an idea.
Summary
Allow to define service-account on per step basis (each pod have different service-account)
Motivation
How we want use argo workflow: We want to define single workflow that will gather some data, compress it, parse and do other stuff around - all work required is defined as steps, lets say
step1: get sensitive data and anonymize that data.
step2: add metrics to already received data
step3: send to processing engine
pretty simple - but, step1 and step3 are maintained by two different teams, those steps need to access different secrets (lets say secre1 for step1 and secret3 for step3), and no other team can have access to secret used by other team (team3 cannot access secret1).
Proposal
I did already some research around code-base, and made following change (i was working on code-base commit 0a928e93):
first, Template can have its own service-account-name defined
type Template struct {
// per template service-account-name
ServiceAccountName string `json:"serviceAccountName,omitempty"`
}
then, before spawning workflowpod in createWorkflowPod function, I am using it
var desiredServiceAccountName = woc.wf.Spec.ServiceAccountName
if len(tmpl.ServiceAccountName) > 0 {
fmt.Println("GOOD")
desiredServiceAccountName = tmpl.ServiceAccountName
}
...
Spec: apiv1.PodSpec{
RestartPolicy: apiv1.RestartPolicyNever,
Containers: []apiv1.Container{
mainCtr,
},
Volumes: woc.createVolumes(),
ActiveDeadlineSeconds: tmpl.ActiveDeadlineSeconds,
ServiceAccountName: desiredServiceAccountName,
ImagePullSecrets: woc.wf.Spec.ImagePullSecrets,
},
and how I want to use this in yaml
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
generateName: coinflip-
spec:
entrypoint: coinflip
templates:
- name: coinflip
steps:
- - name: flip-coin
template: flip-coin
- - name: heads
template: heads
when: "{{steps.flip-coin.outputs.result}} == heads"
- name: tails
template: tails
when: "{{steps.flip-coin.outputs.result}} == tails"
- name: flip-coin
serviceAccountName: zarzamora
script:
image: python:alpine3.6
command: [python]
source: |
import random
import time
time.sleep(5000000)
result = "heads" if random.randint(0,1) == 0 else "tails"
print(result)
- name: heads
serviceAccountName: manzana
container:
image: alpine:3.6
command: [sh, -c]
args: ["echo \"it was heads\""]
- name: tails
serviceAccountName: aguacate
container:
image: alpine:3.6
command: [sh, -c]
args: ["echo \"it was tails\""]
pretty simple - and probably from code-base point of view just wrong, but i wanted only to test the idea.
I don't expect that you will implement it - just give feedback about idea, I am willing to provide PR.
thanks.
Message from the maintainers:
If you wish to see this enhancement implemented please add a 👍 reaction to this issue! We often sort issues this way to know what to prioritize.
Reopening since I think this was closed on error
honestly i closed it because now i have doubts if it makes sense xD but we can let this opened, maybe someone will have some idea around this ticket - atm I am trying to add "namespace" parameter to step definition to allow running workflow's steps in different namespace (maybe that is better option)
This actually totally makes sense and a much-needed feature. One of the use cases that we have is different service accounts in the namespaces have different permissions and IAM roles annotated to them. Currently, it is impossible to run steps as part of a single workflow which requires a different set of permissions
Could you do this using podSpecPatch
?
I can confirm this can be achieved by podSpecPatch
. For example:
templates:
- name: whalesay
podSpecPatch: '{"serviceAccountName": "custom-serviceaccount"}'
container:
image: docker/whalesay:latest
command: [cowsay]
args: ["hello world"]
@dinever you saved my day!
This is the way to implement executer ServiceAccounts for WorkflowTemplate too!
This enhancement make a lot of sense to me. podSpecPatch
works, but is clunky and not declarative/programmable. It would be great if someone wanted to submit a PR for this. I think it would be a good first issue.
@dinever you saved my day!
This is the way to implement executer ServiceAccounts for WorkflowTemplate too!
Being able to define a different service account for the executor (ie for artifact uploads) would help a lot on the operational side of things.
This enhancement make a lot of sense to me.
podSpecPatch
works, but is clunky and not declarative/programmable. It would be great if someone wanted to submit a PR for this. I think it would be a good first issue.
@alexec Does the approach mentioned in the issue description, by @KowalczykBartek make sense? Or is there some different approach that should be explored?
I'm taking a look at this issue and am working on a PR.
So I was looking to implement this feature, but when I went through the code (and suggestions from OP!), it seemed like this was already possible?
In workflow_types.go
, the Template
struct already has a ServiceAccountName
: https://github.com/argoproj/argo-workflows/blob/256ff72816b4314f59fe39f0a458644af0075ebe/pkg/apis/workflow/v1alpha1/workflow_types.go#L717
(meaning the WorkflowSpec
struct can specify an SA and templates within the workflow also can specify SAs)
Further, in workflowpod.go
, there is already logic to set the Pod's SA to the template SA: https://github.com/argoproj/argo-workflows/blob/256ff72816b4314f59fe39f0a458644af0075ebe/workflow/controller/workflowpod.go#L999
I haven't tried setting these myself yet though since I stumbled upon this issue beforehand, but maybe this is already possible?
This code seems to be older than this issue (per the blame
), so that makes me feel like maybe I'm missing something? or there's a bug or something? If anyone knows otherwise, please point in the right direction!
Yeah thanks for the tip @agilgur5! Removing serviceAccountName
and executor.serviceAccountName
from my WorkflowSpec
and adding executor.serviceAccountName
s with different values to my step templates worked fine for me in Argo 3.4.8. I also ensured I didn't have any service accounts configured as workflow defaults via the controller configmap, not sure how much of those extra precautions I really needed to worry about though :)
Yeah thanks for the tip @agilgur5!
Awesome! Thanks for confirming! This can be closed out then 🙂
not sure how much of those extra precautions I really needed to worry about though :)
As is k8s convention, the most specific property should be used; so the step template one should override the Workflow one which should override the default one. Or to put it visually with some pseudo-code:
step > Workflow > default
If something doesn't follow that properly, that would be a bug
Yeah thanks for the tip @agilgur5! Removing
serviceAccountName
andexecutor.serviceAccountName
from myWorkflowSpec
and addingexecutor.serviceAccountName
s with different values to my step templates worked fine for me in Argo 3.4.8. I also ensured I didn't have any service accounts configured as workflow defaults via the controller configmap, not sure how much of those extra precautions I really needed to worry about though :)
@mbaynton can you post an example workflow yaml for newbies like myself?
can you post an example workflow yaml for newbies like myself?
Taking an existing example and modifying it slightly, you can do:
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
generateName: arguments-parameters-from-configmap-
spec:
entrypoint: whalesay
templates:
- name: whalesay
serviceAccountName: argo
inputs:
parameters:
# Parameters can also be passed via configmap reference.
- name: message
valueFrom:
configMapKeyRef:
name: simple-parameters
key: msg
container:
image: argoproj/argosay:v2
args: ["echo", "{{inputs.parameters.message}}"]