Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds secret env_var #3048

Merged
merged 10 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions flytekit/models/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -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_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.
"""

class MountType(Enum):
Expand All @@ -39,6 +42,7 @@ class MountType(Enum):
key: Optional[str] = None
group_version: Optional[str] = None
mount_requirement: MountType = MountType.ANY
env_var: Optional[str] = None

def __post_init__(self):
from flytekit.configuration.plugin import get_plugin
Expand All @@ -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_var=self.env_var,
)

@classmethod
Expand All @@ -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_var=pb2_object.env_var if pb2_object.env_var else None,
)


Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
42 changes: 42 additions & 0 deletions tests/flytekit/integration/remote/test_remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -899,3 +899,45 @@ 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
eapolinario marked this conversation as resolved.
Show resolved Hide resolved
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add error handling for kubectl commands

Consider handling potential errors from subprocess.run() calls when creating and deleting the secret. The command could fail if kubectl is not installed or user lacks permissions.

Code suggestion
Check the AI-generated fix before applying
 @@ -908,11 +908,14 @@
     result = subprocess.run([
         "kubectl", 
         "create",
         "secret",
         "-n",
         "flytesnacks-development", 
         "generic",
         "my-group",
         f"--from-literal=token={secret}",
     ],  capture_output=True, text=True)
 +    if result.returncode != 0:
 +        raise RuntimeError(f"Failed to create secret: {result.stderr}")
     yield secret

     # Remove secret
     result = subprocess.run([
         "kubectl",
         "delete", 
         "secrets",
         "-n",
         "flytesnacks-development",
         "my-group",
     ],  capture_output=True, text=True)
 +    if result.returncode != 0:
 +        raise RuntimeError(f"Failed to delete secret: {result.stderr}")

Code Review Run #7b0105


Is this a valid issue, or was it incorrectly flagged by the Agent?

  • it was incorrectly flagged



# 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")
@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)
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
26 changes: 26 additions & 0 deletions tests/flytekit/integration/remote/workflows/basic/get_secret.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
from flytekit import task, Secret, workflow
from os import getenv

secret_env_var = Secret(
group="my-group",
key="token",
env_var="MY_SECRET",
mount_requirement=Secret.MountType.ENV_VAR,
)
secret_env_file = Secret(
group="my-group",
key="token",
env_var="MY_SECRET_FILE",
eapolinario marked this conversation as resolved.
Show resolved Hide resolved
mount_requirement=Secret.MountType.FILE,
)


@task(secret_requests=[secret_env_var])
def get_secret_env_var() -> str:
return getenv("MY_SECRET", "")
thomasjpfan marked this conversation as resolved.
Show resolved Hide resolved


@task(secret_requests=[secret_env_file])
def get_secret_file() -> str:
with open(getenv("MY_SECRET_FILE")) as f:
return f.read()
thomasjpfan marked this conversation as resolved.
Show resolved Hide resolved
Loading