Skip to content
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

resource template getting duplicate exec-sa-token volume mounts with automountServiceAccountToken: false #12848

Open
3 of 4 tasks
philBrown opened this issue Mar 27, 2024 · 2 comments
Assignees
Labels
area/controller Controller issues, panics P3 Low priority type/bug

Comments

@philBrown
Copy link
Contributor

Pre-requisites

  • I have double-checked my configuration
  • I can confirm the issue exists when I tested with :latest
  • I have searched existing issues and could not find a match for this bug
  • I'd like to contribute the fix myself (see contributing guide)

What happened/what did you expect to happen?

Running a workflow of workflows with automountServiceAccountToken: false results in a repeated exec-sa-token volume mount in the child pod spec.

When trying to run the child workflow, the pod spec validation fails with

Pod "my-workflow-id-sub-id" is invalid: spec.containers[1].volumeMounts[1].mountPath: Invalid value: "/var/run/secrets/kubernetes.io/serviceaccount": must be unique

It appears that the code in workflowpod.go is running multiple times for the same container in these cases.

Version

v3.5.2

Paste a small workflow that reproduces the issue. We must be able to run the workflow; don't enter a workflows that uses private images.

---
apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
  name: workflow-template-submittable
spec:
  automountServiceAccountToken: false # 👈
  executor:
    serviceAccountName: workflow-executor
  entrypoint: whalesay-template
  arguments:
    parameters:
      - name: message
        value: hello world
  templates:
    - name: whalesay-template
      inputs:
        parameters:
          - name: message
      container:
        image: docker/whalesay
        command: [cowsay]
        args: ["{{inputs.parameters.message}}"]

---
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: workflow-of-workflows-
spec:
  automountServiceAccountToken: false # 👈
  executor:
    serviceAccountName: workflow-executor
  entrypoint: main
  templates:
    - name: main
      steps:
        - - name: workflow1
            template: resource-without-argument
            arguments:
              parameters:
              - name: workflowtemplate
                value: "workflow-template-submittable"

    - name: resource-without-argument
      inputs:
        parameters:
          - name: workflowtemplate
      resource:
        action: create
        manifest: |
          apiVersion: argoproj.io/v1alpha1
          kind: Workflow
          metadata:
            generateName: workflow-of-workflows-1-
          spec:
            workflowTemplateRef:
              name: {{inputs.parameters.workflowtemplate}}
        successCondition: status.phase == Succeeded
        failureCondition: status.phase in (Failed, Error)

Logs from the workflow controller

{
  "level": "warning",
  "msg": "Non-transient error: Pod \"my-workflow-id-sub-id\" is invalid: spec.containers[1].volumeMounts[1].mountPath: Invalid value: \"/var/run/secrets/kubernetes.io/serviceaccount\": must be unique",
  "time": "..."
}

Logs from in your workflow's wait container

n/a
@tczhao tczhao added the area/controller Controller issues, panics label Mar 27, 2024
@eduardodbr eduardodbr self-assigned this Mar 28, 2024
@eduardodbr
Copy link
Member

eduardodbr commented Mar 29, 2024

Hi @philBrown,

The issue is not the workflow of workflows. The issue is being caused by using a resource template with automountServiceAccountToken: false. The following (simpler) workflow would throw the same error:

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  name: workflow-resource
  namespace: argo
spec:
  automountServiceAccountToken: false
  executor:
    serviceAccountName: default
  entrypoint: resource-create
  arguments:
    parameters:
      - name: message
        value: hello
  templates:
    - name: resource-create
      inputs:
        parameters:
          - name: message
      resource:
        action: create
        manifest: |
          apiVersion: v1
          kind: ServiceAccount
          metadata:
            name: {{inputs.parameters.message}}

Currently, the executor service account is always mounted in every container (init,wait,main) when automountServiceAccountToken is set to false. When the workflow has a step that uses the template type resource or data it generates the container spec using the newExecContainer() func that also mounts the executor service account into the main container, leading to the duplicate volume mount.

This can be fixed, but I think we should discuss it first. To understand it better, I'll give a few examples:

Init, Wait and Main will use the any SA with:

spec:
  serviceAccountName: any

Init and Wait are mounted with workflow-executorSA while main uses anywith:

spec:
  executor:
    serviceAccountName: workflow-executor
  serviceAccountName: any

Init, Wait and Main will use workflow-executor SA with:

spec:
  executor:
    serviceAccountName: workflow-executor
  serviceAccountName: any
  automountServiceAccountToken: false

IMO this PR #10945 introduced a regression. Before that change, configuring automountServiceAccountToken: false would create the main container without any SA, which I believe was the intended behavior. I believe the issue #10937 that change was trying to solve, could have been solved using a resource template (that would use the executor SA) or using a serviceAccountName with appropriate RBAC.

I'll wait for the input of the maintainers to suggest some changes.

@eduardodbr eduardodbr added the P3 Low priority label Mar 29, 2024
@philBrown philBrown changed the title Workflow of workflows getting duplicate exec-sa-token volume mounts resource template getting duplicate exec-sa-token volume mounts with automountServiceAccountToken: false Apr 2, 2024
@philBrown
Copy link
Contributor Author

Thanks for the clarification @eduardodbr

I found the error was the same whether or not the child had defined automountServiceAccountToken with true, false or not present at all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/controller Controller issues, panics P3 Low priority type/bug
Projects
None yet
Development

No branches or pull requests

3 participants