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

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

Closed
3 tasks done
EmilRex opened this issue Oct 14, 2022 · 7 comments · Fixed by #15888
Closed
3 tasks done

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

EmilRex opened this issue Oct 14, 2022 · 7 comments · Fixed by #15888
Assignees
Labels
enhancement An improvement of an existing feature security Related to application or infrastructure security

Comments

@EmilRex
Copy link
Contributor

EmilRex commented Oct 14, 2022

First check

  • I added a descriptive title to this issue.
  • I used the GitHub search to find a similar request and didn't find it.
  • I searched the Prefect documentation for this feature.

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.

@EmilRex EmilRex added enhancement An improvement of an existing feature status:triage labels Oct 14, 2022
@zanieb
Copy link
Contributor

zanieb commented Oct 14, 2022

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
Copy link
Contributor Author

EmilRex commented Oct 14, 2022

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

@zanieb
Copy link
Contributor

zanieb commented Oct 14, 2022

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
Copy link

mikeoconnor0308 commented Oct 20, 2022

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
Copy link
Contributor

zanieb commented Feb 6, 2023

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.

@zanieb zanieb mentioned this issue Feb 6, 2023
4 tasks
@zanieb zanieb added the security Related to application or infrastructure security label Feb 6, 2023
@ghost
Copy link

ghost commented Apr 4, 2023

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
Copy link

ghost commented Apr 11, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature security Related to application or infrastructure security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants