PrefectHQ / prefect

Prefect is a workflow orchestration framework for building resilient data pipelines in Python.
https://prefect.io
Apache License 2.0
15.85k stars 1.55k forks source link

Agent should pass API key to KubernetesJob as a secret #7185

Open EmilRex opened 1 year ago

EmilRex commented 1 year ago

First check

Prefect Version

2.x

Describe the current behavior

When an agent launches a new KuberenetesJob it passes the Prefect Cloud API key as a regular environment variable.

Describe the proposed behavior

The agent should pass the Prefect Cloud API key as a secretRef as described here.

Example Use

No response

Additional context

In the Prefect helm chart, the API key is passed as a secret.

zanieb commented 1 year ago

I think this will depend on detecting that it was a secret-ref in the first place? I do not think we want to create a new secret-ref for each job.

EmilRex commented 1 year ago

@madkinsz Do you see any good options for letting the user specify their own secretRef?

zanieb commented 1 year ago

Oh totally, you can always override our behavior with a customization e.g.

from prefect.infrastructure import KubernetesJob
from prefect.settings import temporary_settings, PREFECT_API_KEY

with temporary_settings({PREFECT_API_KEY: "foo"}):
    job = KubernetesJob(
        command=["bash", "-c", "echo $PREFECT_API_KEY"],
        # Do not use the API key from the settings
        env={"PREFECT_API_KEY": None},
        customizations=[
            {
                "op": "add",
                "path": "/spec/template/spec/containers/0/env/-",
                "value": {
                    "name": "PREFECT_API_KEY",
                    "valueFrom": {
                        "secretKeyRef": {
                            "name": "prefect-cloud-key",
                            "key": "value",
                        }
                    },
                },
            }
        ],
    )
    print(job.preview())
mikeoconnor0308 commented 1 year ago

Thanks for the workaround, I think we can make that work.

I would argue that given that the Prefect agent helm chart encourages use of a secretRef for the key, a kubernetes job specification should likewise require that to be specified. That makes the system feel more consistent.

Generally, a lot of configuration for a job in real-world settings comes from secrets, so it feels like a natural extension of the above is to make environment from secrets an incorporated feature rather than a non-trivial JSONPatch customization, something like:

    job = KubernetesJob(
        command=[],
        env={"PREFECT_API_KEY": None},
        env_secrets={"PREFECT_API_KEY": {"name": "prefect-cloud-key", "key": "value" }
    )

I appreciate the balance here between general customization vs ease is tricky.

zanieb commented 1 year ago

One thing we could do here is default to using an API key secret with a hard-coded name if it exists. So like.. if we see the secret "prefect-api-key" has a valid value we will inject that into the job by default? I'm a little hesitant to do something magical here though. I think this will be more natural in our upcoming improvements to agents.

ghost commented 1 year ago

IMHO this should be blocker priority, yet there hasn't been any activity for months. While I like a lot of prefects concepts, I can't use it until security is taken as a serious aspect.

ghost commented 1 year ago

EDIT: just noticed that applying customizations to workpools doesn't seem to work. I wonder if there is currently any way to make this work with workpools.