From 615499eae5539036abf70d960beb93621f7f9a29 Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Fri, 10 Jan 2025 14:31:47 -0500 Subject: [PATCH 1/9] Adds secret env name Signed-off-by: Thomas J. Fan --- flytekit/models/security.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/flytekit/models/security.py b/flytekit/models/security.py index e210c910b7..2c7d62ec5c 100644 --- a/flytekit/models/security.py +++ b/flytekit/models/security.py @@ -17,6 +17,9 @@ class Secret(_common.FlyteIdlEntity): key is optional and can be an individual secret identifier within the secret For k8s this is required version is the version of the secret. This is an optional field mount_requirement provides a hint to the system as to how the secret should be injected + env_name is optional. Custom environment name to set the value of the secret. + If mount_requirement is ENV_VAR, then the value is the secret itself. + If mount_requirement is FILE, then the value is the path to the secret file. """ class MountType(Enum): @@ -39,6 +42,7 @@ class MountType(Enum): key: Optional[str] = None group_version: Optional[str] = None mount_requirement: MountType = MountType.ANY + env_name: Optional[str] = None def __post_init__(self): from flytekit.configuration.plugin import get_plugin @@ -56,6 +60,7 @@ def to_flyte_idl(self) -> _sec.Secret: group_version=self.group_version, key=self.key, mount_requirement=self.mount_requirement.value, + env_name=self.env_name, ) @classmethod @@ -65,6 +70,7 @@ def from_flyte_idl(cls, pb2_object: _sec.Secret) -> "Secret": group_version=pb2_object.group_version if pb2_object.group_version else None, key=pb2_object.key if pb2_object.key else None, mount_requirement=Secret.MountType(pb2_object.mount_requirement), + env_name=pb2_object.env_name if pb2_object.env_name else None, ) From 8814da92a8578dbad07550486a5aaced5785d7a5 Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Thu, 16 Jan 2025 12:45:44 -0500 Subject: [PATCH 2/9] Use env_var Signed-off-by: Thomas J. Fan --- flytekit/models/security.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/flytekit/models/security.py b/flytekit/models/security.py index 2c7d62ec5c..be1c586d54 100644 --- a/flytekit/models/security.py +++ b/flytekit/models/security.py @@ -17,7 +17,7 @@ class Secret(_common.FlyteIdlEntity): key is optional and can be an individual secret identifier within the secret For k8s this is required version is the version of the secret. This is an optional field mount_requirement provides a hint to the system as to how the secret should be injected - env_name is optional. Custom environment name to set the value of the secret. + env_var is optional. Custom environment name to set the value of the secret. If mount_requirement is ENV_VAR, then the value is the secret itself. If mount_requirement is FILE, then the value is the path to the secret file. """ @@ -42,7 +42,7 @@ class MountType(Enum): key: Optional[str] = None group_version: Optional[str] = None mount_requirement: MountType = MountType.ANY - env_name: Optional[str] = None + env_var: Optional[str] = None def __post_init__(self): from flytekit.configuration.plugin import get_plugin @@ -60,7 +60,7 @@ def to_flyte_idl(self) -> _sec.Secret: group_version=self.group_version, key=self.key, mount_requirement=self.mount_requirement.value, - env_name=self.env_name, + env_var=self.env_var, ) @classmethod @@ -70,7 +70,7 @@ def from_flyte_idl(cls, pb2_object: _sec.Secret) -> "Secret": group_version=pb2_object.group_version if pb2_object.group_version else None, key=pb2_object.key if pb2_object.key else None, mount_requirement=Secret.MountType(pb2_object.mount_requirement), - env_name=pb2_object.env_name if pb2_object.env_name else None, + env_var=pb2_object.env_var if pb2_object.env_var else None, ) From 771ba1421cefc8ea3922f3c8a129001582c6c3c7 Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Wed, 22 Jan 2025 14:51:42 -0500 Subject: [PATCH 3/9] Bump flyteidl Signed-off-by: Thomas J. Fan --- pyproject.toml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 63a2dce6a9..d4c844f318 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,7 @@ dependencies = [ "diskcache>=5.2.1", "docker>=4.0.0", "docstring-parser>=0.9.0", - "flyteidl>=1.14.1", + "flyteidl>=1.14.2", "fsspec>=2023.3.0", "gcsfs>=2023.3.0", "googleapis-common-protos>=1.57", @@ -43,7 +43,7 @@ dependencies = [ "pygments", "python-json-logger>=2.0.0", "pytimeparse>=1.1.8", - "pyyaml!=6.0.0,!=5.4.0,!=5.4.1", # pyyaml is broken with cython 3: https://github.com/yaml/pyyaml/issues/601 + "pyyaml!=6.0.0,!=5.4.0,!=5.4.1", # pyyaml is broken with cython 3: https://github.com/yaml/pyyaml/issues/601 "requests>=2.18.4", "rich", "rich_click", @@ -98,10 +98,10 @@ asyncio_default_fixture_loop_scope = "function" log_cli = false log_cli_level = 20 markers = [ - "sandbox_test: fake integration tests", # unit tests that are really integration tests that run on a sandbox environment + "sandbox_test: fake integration tests", # unit tests that are really integration tests that run on a sandbox environment "serial: tests to avoid using with pytest-xdist", "hypothesis: tests that use they hypothesis library", - "lftransfers: integration tests which involve large file transfers" + "lftransfers: integration tests which involve large file transfers", ] [tool.coverage.report] From 7e0bf7ecace7def142606adb21833e5f4a05a11a Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Wed, 22 Jan 2025 14:52:30 -0500 Subject: [PATCH 4/9] Smaller change Signed-off-by: Thomas J. Fan --- pyproject.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index d4c844f318..765d66e5e8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,7 +43,7 @@ dependencies = [ "pygments", "python-json-logger>=2.0.0", "pytimeparse>=1.1.8", - "pyyaml!=6.0.0,!=5.4.0,!=5.4.1", # pyyaml is broken with cython 3: https://github.com/yaml/pyyaml/issues/601 + "pyyaml!=6.0.0,!=5.4.0,!=5.4.1", # pyyaml is broken with cython 3: https://github.com/yaml/pyyaml/issues/601 "requests>=2.18.4", "rich", "rich_click", @@ -98,10 +98,10 @@ asyncio_default_fixture_loop_scope = "function" log_cli = false log_cli_level = 20 markers = [ - "sandbox_test: fake integration tests", # unit tests that are really integration tests that run on a sandbox environment + "sandbox_test: fake integration tests", # unit tests that are really integration tests that run on a sandbox environment "serial: tests to avoid using with pytest-xdist", "hypothesis: tests that use they hypothesis library", - "lftransfers: integration tests which involve large file transfers", + "lftransfers: integration tests which involve large file transfers" ] [tool.coverage.report] From bc9b040b4fbba977f181c49ee4b3917925ab76e1 Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Wed, 22 Jan 2025 17:39:03 -0500 Subject: [PATCH 5/9] Add integration test Signed-off-by: Thomas J. Fan --- .../integration/remote/test_remote.py | 41 +++++++++++++++++++ .../remote/workflows/basic/get_secret.py | 16 ++++++++ 2 files changed, 57 insertions(+) create mode 100644 tests/flytekit/integration/remote/workflows/basic/get_secret.py diff --git a/tests/flytekit/integration/remote/test_remote.py b/tests/flytekit/integration/remote/test_remote.py index 82c18b3c50..97f86f0a0c 100644 --- a/tests/flytekit/integration/remote/test_remote.py +++ b/tests/flytekit/integration/remote/test_remote.py @@ -899,3 +899,44 @@ def retry_operation(operation): remote.wait(execution=execution, timeout=datetime.timedelta(minutes=5)) assert execution.outputs["o0"] == {"title": "my report", "data": [1.0, 2.0, 3.0, 4.0, 5.0]} + + +@pytest.fixture +def kubectl_secret(): + secret = "abc-xyz" + # Create secret + subprocess.run([ + "kubectl", + "create", + "secret", + "-n", + "flytesnacks-development", + "generic", + "my-group", + f"--from-literal=token={secret}", + ], capture_output=True, text=True) + yield secret + + # Remove secret + subprocess.run([ + "kubectl", + "delete", + "secrets", + "-n", + "flytesnacks-development", + "my-group", + ], capture_output=True, text=True) + + +# To enable this test, kubectl must be available in the CI. +@pytest.mark.skip(reason="Waiting for flyte release that includes https://github.com/flyteorg/flyte/pull/6176") +def test_check_secret(kubectl_secret): + execution_id = run("get_secret.py", "wf") + + remote = FlyteRemote(Config.auto(config_file=CONFIG), PROJECT, DOMAIN) + execution = remote.fetch_execution(name=execution_id) + execution = remote.wait(execution=execution) + assert execution.closure.phase == WorkflowExecutionPhase.SUCCEEDED, ( + f"Execution failed with phase: {execution.closure.phase}" + ) + assert execution.outputs['o0'] == kubectl_secret diff --git a/tests/flytekit/integration/remote/workflows/basic/get_secret.py b/tests/flytekit/integration/remote/workflows/basic/get_secret.py new file mode 100644 index 0000000000..10611fccfd --- /dev/null +++ b/tests/flytekit/integration/remote/workflows/basic/get_secret.py @@ -0,0 +1,16 @@ +from flytekit import task, current_context, Secret, workflow + +secret = Secret( + group="my-group", + key="token", +) + + +@task(secret_requests=[secret]) +def get_secret() -> str: + return current_context().secrets.get(group="my-group", key="token") + + +@workflow +def wf() -> str: + return get_secret() From 27a0d3e461cca79158667b2f15f5e8b971f6271a Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Wed, 22 Jan 2025 17:39:16 -0500 Subject: [PATCH 6/9] Add integration test Signed-off-by: Thomas J. Fan --- tests/flytekit/integration/remote/test_remote.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/flytekit/integration/remote/test_remote.py b/tests/flytekit/integration/remote/test_remote.py index 97f86f0a0c..f1a8f630a2 100644 --- a/tests/flytekit/integration/remote/test_remote.py +++ b/tests/flytekit/integration/remote/test_remote.py @@ -928,7 +928,7 @@ def kubectl_secret(): ], capture_output=True, text=True) -# To enable this test, kubectl must be available in the CI. +# To enable this test, kubectl must be available. @pytest.mark.skip(reason="Waiting for flyte release that includes https://github.com/flyteorg/flyte/pull/6176") def test_check_secret(kubectl_secret): execution_id = run("get_secret.py", "wf") From daaf43e0f79af3d0074df80439337818c3b473e3 Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Wed, 22 Jan 2025 21:41:13 -0500 Subject: [PATCH 7/9] Use env_var Signed-off-by: Thomas J. Fan --- .../integration/remote/workflows/basic/get_secret.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/flytekit/integration/remote/workflows/basic/get_secret.py b/tests/flytekit/integration/remote/workflows/basic/get_secret.py index 10611fccfd..d5ad565958 100644 --- a/tests/flytekit/integration/remote/workflows/basic/get_secret.py +++ b/tests/flytekit/integration/remote/workflows/basic/get_secret.py @@ -1,14 +1,16 @@ -from flytekit import task, current_context, Secret, workflow +from flytekit import task, Secret, workflow +from os import getenv secret = Secret( group="my-group", key="token", + env_var="MY_SECRET" ) @task(secret_requests=[secret]) def get_secret() -> str: - return current_context().secrets.get(group="my-group", key="token") + return getenv("MY_SECRET") @workflow From 867bfd15a3f0cc090dc12ed3f040ef77c7e7d9fd Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Wed, 22 Jan 2025 21:52:49 -0500 Subject: [PATCH 8/9] Check for file and env_var Signed-off-by: Thomas J. Fan --- .../integration/remote/test_remote.py | 5 ++-- .../remote/workflows/basic/get_secret.py | 24 ++++++++++++------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/tests/flytekit/integration/remote/test_remote.py b/tests/flytekit/integration/remote/test_remote.py index f1a8f630a2..2aa6a09c75 100644 --- a/tests/flytekit/integration/remote/test_remote.py +++ b/tests/flytekit/integration/remote/test_remote.py @@ -930,8 +930,9 @@ def kubectl_secret(): # To enable this test, kubectl must be available. @pytest.mark.skip(reason="Waiting for flyte release that includes https://github.com/flyteorg/flyte/pull/6176") -def test_check_secret(kubectl_secret): - execution_id = run("get_secret.py", "wf") +@pytest.mark.parametrize("task", ["get_secret_env_var", "get_secret_file"]) +def test_check_secret(kubectl_secret, task): + execution_id = run("get_secret.py", task) remote = FlyteRemote(Config.auto(config_file=CONFIG), PROJECT, DOMAIN) execution = remote.fetch_execution(name=execution_id) diff --git a/tests/flytekit/integration/remote/workflows/basic/get_secret.py b/tests/flytekit/integration/remote/workflows/basic/get_secret.py index d5ad565958..a7d7ecb488 100644 --- a/tests/flytekit/integration/remote/workflows/basic/get_secret.py +++ b/tests/flytekit/integration/remote/workflows/basic/get_secret.py @@ -1,18 +1,26 @@ from flytekit import task, Secret, workflow from os import getenv -secret = Secret( +secret_env_var = Secret( group="my-group", key="token", - env_var="MY_SECRET" + env_var="MY_SECRET", + mount_requirement=Secret.MountType.ENV_VAR, +) +secret_env_file = Secret( + group="my-group", + key="token", + env_var="MY_SECRET_FILE", + mount_requirement=Secret.MountType.FILE, ) -@task(secret_requests=[secret]) -def get_secret() -> str: - return getenv("MY_SECRET") +@task(secret_requests=[secret_env_var]) +def get_secret_env_var() -> str: + return getenv("MY_SECRET", "") -@workflow -def wf() -> str: - return get_secret() +@task(secret_requests=[secret_env_file]) +def get_secret_file() -> str: + with open(getenv("MY_SECRET_FILE")) as f: + return f.read() From b2007d9e24d017495c439da2426e8c9b58bc1385 Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Thu, 23 Jan 2025 13:58:17 -0500 Subject: [PATCH 9/9] Add check for kubectl Signed-off-by: Thomas J. Fan --- tests/flytekit/integration/remote/test_remote.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/flytekit/integration/remote/test_remote.py b/tests/flytekit/integration/remote/test_remote.py index 2aa6a09c75..ba66345deb 100644 --- a/tests/flytekit/integration/remote/test_remote.py +++ b/tests/flytekit/integration/remote/test_remote.py @@ -1,4 +1,5 @@ import botocore.session +import shutil from contextlib import ExitStack, contextmanager import datetime import hashlib @@ -905,8 +906,12 @@ def retry_operation(operation): def kubectl_secret(): secret = "abc-xyz" # Create secret + kubectl = shutil.which("kubectl") + if kubectl is None: + pytest.skip("kubectl not found") + subprocess.run([ - "kubectl", + kubectl, "create", "secret", "-n", @@ -919,7 +924,7 @@ def kubectl_secret(): # Remove secret subprocess.run([ - "kubectl", + kubectl, "delete", "secrets", "-n",