flyteidl icon indicating copy to clipboard operation
flyteidl copied to clipboard

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

Open gpgn opened this issue 2 years ago • 2 comments

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
  • [x] Feature
  • [ ] Plugin

Are all requirements met?

  • [x] Code completed
  • [x] Smoke tested
  • [x] Unit tests added
  • [x] Code documentation added
  • [x] 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 https://github.com/flyteorg/flyte/issues/3053

Follow-up issue

NA

Linked PRs

  • https://github.com/flyteorg/flytekit/pull/1726
  • https://github.com/flyteorg/flytepropeller/pull/589
  • https://github.com/flyteorg/flyteidl/pull/423
  • https://github.com/flyteorg/flytesnacks/pull/1017

gpgn avatar Jul 08 '23 16:07 gpgn

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

welcome[bot] avatar Jul 08 '23 16:07 welcome[bot]

Codecov Report

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

:exclamation: 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

codecov[bot] avatar Jul 12 '23 15:07 codecov[bot]