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

Allow variables in jobspec #15384

Closed
SamMousa opened this issue Nov 25, 2022 · 12 comments
Closed

Allow variables in jobspec #15384

SamMousa opened this issue Nov 25, 2022 · 12 comments

Comments

@SamMousa
Copy link
Contributor

SamMousa commented Nov 25, 2022

Introduction

This request aims to solve a problem similar to the one described in #15332.
When using the docker driver with authentication it would be nice to be able to fill it from variables.

Currently this is possible from a template, but this is a security risk. We MUST push the docker credentials into the container environment, but we shouldn't have to. The workload has nothing to do with pulling the container so it shouldn't have access to the credentials.
Whereas Vault is an external service, this might be easier to implement for Nomad native variables, since they are internal.

Proposal

Make all variables available via qualified names. Similiar to how you can use this:

env {
  SECRET_DIR = NOMAD_SECRETS_DIR
  NODE_NAME = node.unique.name

fyi: the above code works, but according to docs I should be using:

env {
  SECRET_DIR = "${NOMAD_SECRETS_DIR}"
  NODE_NAME = "${node.unique.name}"

So the syntax could just be a variable path:

env {
  SECRET_DIR = "${variables.jobs.jobname.groupname.taskname}"
}

It should be explicitly noted that the goal here is to have static value embedding. Changing the variable will not affect any running jobs.

Use-cases

Use variables at a job configuration level without making their values available to the workload.

Alternative solution

Alternatively we could solve this using templates by adding some more flags.
Currently we have env which pushes it into the environment for the task.
We could add another envOrchestratorOnly flag. When set to true it will remove the variables after initializing the workload, before actually starting it.
This would allow you to add it in a backward compatible manner; however I'm not a fan of this cluttering of boolean options.

It is already a bit weird having to specify a target file when the goal is to just put it in the env. This will get worse of we keep adding on boolean flags.
I'd argue a cleaner syntax would be to use destination in a more dynamic way:

destination = "nomad://env"
destination = "nomad://driver"
@lgfa29
Copy link
Contributor

lgfa29 commented Nov 25, 2022

Hi @SamMousa 👋

Thanks for the proposal. Like #15332 I think this would help simplify job files, but I'm not sure I follow the use case you mentioned:

When using the docker driver with authentication it would be nice to be able to fill it from variables.

There are currently three ways to provide auth when pulling Docker images. One of them defines the values in the job itself using the auth task driver config. Is this the value you were thinking about?

I would also like to clarify this part:

Currently this is possible from a template, but this is a security risk. We MUST push the docker credentials into the container environment, but we shouldn't have to. The workload has nothing to do with pulling the container so it shouldn't have access to the credentials.

Tasks have access to env values the same way as a template with env = true, so I'm not sure how this proposal would fix this.

@SamMousa
Copy link
Contributor Author

Apologies for not getting back to this sooner.

Is this the value you were thinking about?

Yes, I'm talking about the task driver config in the job.

I'm not sure how this proposal would fix this.

Well the issue is there's not enough "stages". The task driver goes through several stages:

  1. Bootstrap; pull the docker image, create the container
  2. Run the container
  3. Cleanup

Currently there is just one environment for both of these steps. But these steps don't operate on the same security level:

  • The bootstrapping happens by trusted code with full access to the host machine
  • The running container runs in a more limited environment without full access

The variables inside the job spec are replaced during bootstrapping. Separating these 2 phases would be simple for the docker runner since it is probably explicitly adding env variables to the docker container. So it could just not add variables to the container.

@lgfa29
Copy link
Contributor

lgfa29 commented Dec 1, 2022

Ah I see. So I think what you're looking for is described in #9740, and not really related to environment variables since those are always set in the container.

I will close this one as a duplicate of #9740, but let me know if I missing something and we can reopen it.

Feel free to 👍 and add any additional thoughts you may have there.

@lgfa29 lgfa29 closed this as not planned Won't fix, can't repro, duplicate, stale Dec 1, 2022
@SamMousa
Copy link
Contributor Author

SamMousa commented Dec 1, 2022

No, this isn't related to vault.
It's about passing parameters to the task driver and not the task. As you say env is always passed to the container, which is what I'm proposing to change. Allow the jobspec to indicate whether an env template is rendered to driver, workload or both

@lgfa29
Copy link
Contributor

lgfa29 commented Dec 2, 2022

Ah yeah, the Vault aspect is more of an implementation detail, whatever we build for Vault should support Nomad Variables as well.

Are there any other configuration values other than authentication that could benefit from this? I can't think of any from the top of my head, but I may be missing some 🤔

@SamMousa
Copy link
Contributor Author

SamMousa commented Dec 2, 2022

Maybe things like artifact URLs?

@lgfa29
Copy link
Contributor

lgfa29 commented Dec 2, 2022

Artifacts are already downloaded before the task starts. You can even use them to load Docker images as TAR files 😄
https://developer.hashicorp.com/nomad/docs/drivers/docker#load

@SamMousa
Copy link
Contributor Author

SamMousa commented Dec 2, 2022

I know that; but what if my artifact URL contains a secret? Like a random token so not anyone can simply download it.
How can I pass that? If I need to use env it means the workload itself can also access it.

Edit: of course in that case it would be another form of authentication. But it's use case is just not limited to docker auth

@lgfa29
Copy link
Contributor

lgfa29 commented Dec 2, 2022

Ah for sure. I think this is covered in option 2 here? #3854 (comment)

As before, this is an old proposal so Vault was the only option available for secrets management, but anything we implement in this area would work with Nomad Variables as well.

@SamMousa
Copy link
Contributor Author

SamMousa commented Dec 3, 2022

Yes it is, maybe time to rewrite that solution as a new proposal to get more eyes on it?

@lgfa29
Copy link
Contributor

lgfa29 commented Dec 5, 2022

I think it would take a while for us to write a new proposal, so for the meantime I updated the issue title and tags and added a comment summarizing your thoughts here. Feel free to add more things there if I missed anything 👍

@github-actions
Copy link

github-actions bot commented Apr 5, 2023

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

No branches or pull requests

2 participants