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

[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

fixes flyteorg/flyte#3053

Follow-up issue

NA

Linked PRs

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

[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).

[codecov]

Codecov Report

Merging #423 (9255f27) into master (b219c2a) will increase coverage by 2.55%.
The diff coverage is n/a.

❗ Current head 9255f27 differs from pull request most recent head 3e1ba6f. Consider uploading reports for the commit 3e1ba6f to get more accurate results

@@            Coverage Diff             @@
##           master     #423      +/-   ##
==========================================
+ Coverage   75.92%   78.48%   +2.55%     
==========================================
  Files          18       18              
  Lines        1458     1250     -208     
==========================================
- Hits         1107      981     -126     
+ Misses        294      212      -82     
  Partials       57       57              

Flag	Coverage Δ
unittests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 18 files with indirect coverage changes

[pingsutw]

LGTM, could you resolve a merge conflict

[wild-endeavor]

This is great. I want to call out though that this change needs to be both forward and backwards compatible. That is,

• people on newer versions of the flyte backend (like upgrade propeller/admin/etc after these PRs are merged) but old versions of flytekit should continue to work, and
• people on existing versions of flyte backend (those who do not upgrade propeller/admin/etc with these PRs) but who do upgrade flytekit, should continue to work.

[wild-endeavor]

since the existing MountType enum was only used in the mount_requirement field, it'd be effectively deprecated as well right? Can we add the flag there as well?

Also since the existing enum had an Any concept, should we add that as well to the one of?

[wild-endeavor]

Nm - let's ignore this comment

[wild-endeavor]

Suggested change

	// The name of the environment variable if the Secret is injected as environment variable. If ommitted, the default
	// The name of the environment variable if the Secret is injected as environment variable. If

[wild-endeavor]

Suggested change

	// FLYTE_SECRETS_ENV_PREFIX prefix will be used.
	// MountEnvVar is supplied with an empty string, FLYTE_SECRETS_ENV_PREFIX prefix will be used.

[wild-endeavor]

Suggested change

	// satisfy that requirement. If not provided, the default location will depend on the key management system.
	// satisfy that requirement. If provided with an empty string, the default location will depend on the key management system.