Fix pod creation issue when having multiple secret from same secret group

[pradithya]

TL;DR

Fix invalid pod manifest created by flyte-pod-webhook when a task within workflow request multiple secret from the same secret group. This change is specific to secret mount using ANY or FILE.
This can be reproduced by executing task user_info_task in this example

Type

• Bug Fix
• Feature
• Plugin

Are all requirements met?

• Code completed
• Smoke tested
• Unit tests added
• Code documentation added
• Any pending items have an associated Issue

Complete description

Executing user_info_task in in this example produce following error in the flyte-propeller

The operation was attempted but failed due to reason(s) other than insufficient resource: [Pod \"l73xooe1z6-n1-0\" is invalid: spec.containers[0].volumeMounts[2].mountPath: Invalid value: \"/etc/flyte/secrets/user-info\": must be unique]\n","ts":"2021-08-19T04:57:21Z"

The issue is due to the fact that pod mutator will create 2 volume mounts with the same mount path which is considered invalid by kube-api server.
There are a few modification within this PR to fix this issue:

1. Create only 1 volume for the same secret group (k8s secret), and add each secret keys as new items. There is a change in volume name generation to exclude the secret name.
2. Create only 1 volume mount for the same secret group.

Tracking Issue

Follow-up issue

NA

[welcome]

Thank you for opening this pull request! 🙌
These tips will help get your PR across the finish line:

• Most of the repos have a PR template; if not, fill it out to the best of your knowledge. - Sign off your commits (Reference: DCO Guide).

[pradithya]

Smoke test was performed with following workflow

import os
import flytekit

from flytekit import Secret, task, workflow
from flytekit.testing import SecretsManager

SECRET_GROUP = "user-info"
SECRET_NAME = "user_secret"
USERNAME_SECRET = "username"
PASSWORD_SECRET = "password"

@task(secret_requests=[Secret(group=SECRET_GROUP, key=SECRET_NAME)])
def secret_task() -> str:
    secret_val = flytekit.current_context().secrets.get(SECRET_GROUP, SECRET_NAME)
    # Please do not print the secret value, we are doing so just as a demonstration
    print(secret_val)
    return secret_val

@task(
    secret_requests=[Secret(key=USERNAME_SECRET, group=SECRET_GROUP), Secret(key=PASSWORD_SECRET, group=SECRET_GROUP)])
def user_info_task() -> (str, str):
    secret_username = flytekit.current_context().secrets.get(SECRET_GROUP, USERNAME_SECRET)
    secret_pwd = flytekit.current_context().secrets.get(SECRET_GROUP, PASSWORD_SECRET)
    # Please do not print the secret value, this is just a demonstration.
    print(f"{secret_username}={secret_pwd}")
    return secret_username, secret_pwd

@task(secret_requests=[Secret(group=SECRET_GROUP, key=SECRET_NAME, mount_requirement=Secret.MountType.FILE)])
def secret_file_task() -> (str, str):
    # SM here is a handle to the secrets manager
    sm = flytekit.current_context().secrets
    f = sm.get_secrets_file(SECRET_GROUP, SECRET_NAME)
    secret_val = sm.get(SECRET_GROUP, SECRET_NAME)
    # returning the filename and the secret_val
    return f, secret_val

@workflow
def my_secret_workflow() -> (str, str, str, str, str):
    x = secret_task()
    y, z = user_info_task()
    f, s = secret_file_task()
    return x, y, z, f, s

if __name__ == "__main__":
    sec = SecretsManager()
    os.environ[sec.get_secrets_env_var(SECRET_GROUP, SECRET_NAME)] = "value"
    os.environ[sec.get_secrets_env_var(SECRET_GROUP, USERNAME_SECRET)] = "username_value"
    os.environ[sec.get_secrets_env_var(SECRET_GROUP, PASSWORD_SECRET)] = "password_value"
    x, y, z, f, s = my_secret_workflow()
    assert x == "value"
    assert y == "username_value"
    assert z == "password_value"
    assert f == sec.get_secrets_file(SECRET_GROUP, SECRET_NAME)
    assert s == "value"

Pod manifest

apiVersion: v1
kind: Pod
metadata:
  annotations:
    cluster-autoscaler.kubernetes.io/safe-to-evict: "false"
    flyte.secrets/s0: m4zg54lqhiqce4ltmvzc11lomzxsectlmv3tuibcovzwk3tomfwwkiqk
    flyte.secrets/s1: m4zg54lqhiqce4ltmvzc11lomzxsectlmv3tuibcobqxg32xn4zgiiqk
  creationTimestamp: "2021-08-19T05:46:18Z"
  labels:
    execution-id: cqy12bw3gy
    inject-flyte-secrets: "true"
    interruptible: "false"
    node-id: n1
    task-name: secret-workflows-example-user-info-task
    workflow-name: secret-workflows-example-my-secret-workflow
  name: cqy12bw3gy-n1-0
  namespace: flyteexamples-development
  ownerReferences:
  - apiVersion: flyte.lyft.com/v1alpha1
    blockOwnerDeletion: true
    controller: true
    kind: flyteworkflow
    name: cqy12bw3gy
  resourceVersion: "426574933"
spec:
  containers:
  - args:
    - pyflyte-fast-execute
    - --additional-distribution
    - gs://d-gods-flyte/fast/v4-fast0dd557c64bff1af32e2e83cc9813a828.tar.gz
    - --dest-dir
    - /root
    - --
    - pyflyte-execute
    - --inputs
    - gs://d-gods-flyte/metadata/propeller/flyteexamples-development-cqy12bw3gy/n1/data/inputs.pb
    - --output-prefix
    - gs://d-gods-flyte/metadata/propeller/flyteexamples-development-cqy12bw3gy/n1/data/0
    - --raw-output-data-prefix
    - gs://d-gods-flyte/cm/cqy12bw3gy-n1-0
    - --resolver
    - flytekit.core.python_auto_container.default_task_resolver
    - --
    - task-module
    - secret.workflows.example
    - task-name
    - user_info_task
    env:
    - name: FLYTE_INTERNAL_IMAGE
      value: asia.gcr.io/gods-dev/aria:1.0.1
    - name: FLYTE_INTERNAL_EXECUTION_WORKFLOW
      value: flyteexamples:development:secret.workflows.example.my_secret_workflow
    - name: FLYTE_INTERNAL_EXECUTION_ID
      value: cqy12bw3gy
    - name: FLYTE_INTERNAL_EXECUTION_PROJECT
      value: flyteexamples
    - name: FLYTE_INTERNAL_EXECUTION_DOMAIN
      value: development
    - name: FLYTE_ATTEMPT_NUMBER
      value: "0"
    - name: FLYTE_INTERNAL_TASK_PROJECT
      value: flyteexamples
    - name: FLYTE_INTERNAL_TASK_DOMAIN
      value: development
    - name: FLYTE_INTERNAL_TASK_NAME
      value: secret.workflows.example.user_info_task
    - name: FLYTE_INTERNAL_TASK_VERSION
      value: v4
    - name: FLYTE_INTERNAL_PROJECT
      value: flyteexamples
    - name: FLYTE_INTERNAL_DOMAIN
      value: development
    - name: FLYTE_INTERNAL_NAME
      value: secret.workflows.example.user_info_task
    - name: FLYTE_INTERNAL_VERSION
      value: v4
    - name: FLYTE_SECRETS_DEFAULT_DIR
      value: /etc/flyte/secrets
    - name: FLYTE_SECRETS_FILE_PREFIX
    image: asia.gcr.io/gods-dev/aria:1.0.1
    imagePullPolicy: IfNotPresent
    name: cqy12bw3gy-n1-0
    resources:
      limits:
        cpu: "2"
        memory: 1Gi
      requests:
        cpu: 100m
        memory: 100Mi
    volumeMounts:
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: default-token-62hqz
      readOnly: true
    - mountPath: /etc/flyte/secrets/user-info
      name: ovzwk3rnnfxgm216
      readOnly: true
  volumes:
  - name: default-token-62hqz
    secret:
      defaultMode: 420
      secretName: default-token-62hqz
  - name: ovzwk3rnnfxgm216
    secret:
      defaultMode: 420
      items:
      - key: username
        path: username
      - key: password
        path: password
      secretName: user-info

[kumare3]

cc @EngHabu

[codecov]

Codecov Report

Merging #306 (42715d0) into master (536ab9a) will increase coverage by 0.02%.
The diff coverage is 100.00%.

[EngHabu]

Thank you for your contribution and the detailed PR!

[EngHabu]

Do we need this file merged?

[kumare3]

@pradithya drop the file?

[pradithya]

Woops, I deleted it.

[kumare3]

this is embarassing @pradithya. Thank you for the fix. After you delete the one file we can merge this!

[welcome]

Congrats on merging your first pull request! 🎉