Introduces new "env_var" and "file" fields to Secret to allow specifying name/mountPath on injection #minor

[gpgn]

TL;DR

Introduces new fields to the Secret object:

• env_var
• file

Allowing users to directly specify a name or mountPath for the Secret, without having to specify a full PodTemplate(name). The old mount_requirement can still be used. Example:

from flytekit import task, workflow, Secret
import flytekit
import os


@task(
    secret_requests=[
        Secret(
            group="user-info",
            key="user_secret",
            mount_requirement=Secret.MountType.FILE,
        ),
        Secret(
            group="user-info",
            key="user_secret",
            mount_requirement=Secret.MountType.ENV_VAR,
        ),
    ]
)
def old() -> None:
    context = flytekit.current_context()
    print("old")

    # mount_requirement=ENV_VAR
    print(context.secrets.get(env_name="_FSEC_USER-INFO_USER_SECRET"))

    # mount_requirement=FILE
    with open('/etc/flyte/secrets/user-info/user_secret', 'r') as infile:
        print(infile.readlines())
    return True

@task(
    secret_requests=[
        Secret(
            group="user-info",
            key="user_secret",
            env_var=Secret.MountEnvVar(name="foo"),
        ),
        Secret(
            group="user-info",
            key="user_secret",
            file=Secret.MountFile(path="/foo"),
        ),
    ]
)

def new() -> None:
    context = flytekit.current_context()
    print("new")

    # env_var=
    print(context.secrets.get(env_name="foo"))

    # file=
    with open('/foo/user_secret', 'r') as infile:
        print(infile.readlines())


@workflow
def training_workflow(hyperparameters: dict) -> None:
    """Put all of the steps together into a single workflow."""
    old()
    new()

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

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

Remove the 'fixes' keyword if there will be multiple PRs to fix the linked issue

fixes https://github.com/flyteorg/flyte/issues/

Follow-up issue

NA
OR
https://github.com/flyteorg/flyte/issues/

[EngHabu]

Thank you for your contribution!
I see the original issue is marked as closed as it suggests using PodTemplate. Is that not enough? or not ergonomic enough?

[EngHabu]

This only applies to ENV_VAR mount type. I would make the change as following:

1. Mark MountType mount_requirement = 4 [deprecated=true]; as deprecated.
2. Add:

     oneof MountTarget mount_target {
       MountEnvVar env_var = 5;
       MountFile file = 6;
     }
   
     message MountEnvVar {
       string name = 1;
     }
   
     message MountFile {
       string path = 1;
     }
   

3. Update secrets webhook to look at the new field first, if it's nil, it should fallback to the old field.
4. Update flytekit to always set both fields; mount_requirement and mount_target

[gpgn]

Thanks for the feedback! I'll try to add it. Could you explain what you mean with 4? Not sure I get what you mean here.

[gpgn]

Added the changes with a working initial version, I'll get to adding a number of tests as well.

[gpgn]

Tests & documentation added, let me know what you think :)

[gpgn]

I've opened PRs here for the change, I'll close this one.

• Introduces new "env_var" and "file" fields to Secret to allow specifying name/mountPath on injection flyteorg/flytekit#1726
• Introduces new "env_var" and "file" fields to Secret to allow specifying name/mountPath on injection #minor flyteorg/flytepropeller#589
• Introduces new "env_var" and "file" fields to Secret to allow specifying name/mountPath on injection flyteorg/flyteidl#423
• Adds documentation on usage for new Secret fields env_var and file flyteorg/flytesnacks#1017