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

Provide mechanism to set environment variables on TaskRuns #1606

Closed
poy opened this issue Nov 22, 2019 · 32 comments
Closed

Provide mechanism to set environment variables on TaskRuns #1606

poy opened this issue Nov 22, 2019 · 32 comments
Assignees
Labels
area/roadmap Issues that are part of the project (or organization) roadmap (usually an epic) kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@poy
Copy link
Contributor

poy commented Nov 22, 2019

Expected Behavior

Have similar API to knative-builds TemplateInstantiationSpec to provide environment variables to a TaskRun.

Actual Behavior

Use work arounds.

Steps to Reproduce the Problem

Additional Info

@vdemeester
Copy link
Member

@poy I am not entirely sure I follow but I guess, what you want is to be able to pass environment variable from the TaskRun (to the running pods) ; which is not possible as of today.

It could be something like the following…

apiVersion: tekton.dev/v1alpha1
kind: TaskRun
metadata:
  name: read-repo
spec:
  taskRef:
    name: read-task
  env:
  - name: "FOO"
    value: "bar"

… or we could enhance podTemplate to have an env field (but I am not a huge fan of that, better abstracting it from podTemplate)

/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 25, 2019
@poy
Copy link
Contributor Author

poy commented Nov 25, 2019

Sorry for being vague, but you nailed it! I also prefer your first API suggestion.

@vdemeester vdemeester added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Nov 25, 2019
@vdemeester vdemeester added this to the Pipelines 1.0/beta 🐱 milestone Nov 25, 2019
@bobcatfish
Copy link
Collaborator

hey there @poy! could you describe a bit more about the use case for this?

today you could accomplish this by:

  1. Declaring a parameter for your env var
  2. Using variable substitution to use that param to populate an environment variable

We've tried to be very deliberate about what values provide at runtime: for example, if you have a step in Task X which requires environment variable FOO, we want to make sure that is as explicit in Task X as possible - if Task X doesn't mention it, but it's expected to be provided via the TaskRun, then users of the Task won't know how to use the Task

@poy
Copy link
Contributor Author

poy commented Nov 25, 2019

@bobcatfish of course!

So we are using Buildpacks with Tekton. Traditional Buildpacks (In Cloud Foundry) allowed environment variables for settings.

We have no way of knowing what environment variables could be needed ahead of time as users can create their own Buildpacks. Therefore we need something more dynamic.

poy added a commit to poy/pipeline that referenced this issue Dec 1, 2019
Adds a `Env` field to `TaskRunSpec` to allow a user to set environment
variables on each step container. This mimics Knative-Build's
`TemplateInstantiationSpec`'s `Env` field.

Setting environment variables is useful for when a `TaskRun` author may
not know what variables are required ahead of time (e.g., Buildpacks).

fixes tektoncd#1606
poy added a commit to poy/pipeline that referenced this issue Dec 1, 2019
Adds a `Env` field to `TaskRunSpec` to allow a user to set environment
variables on each step container. This mimics Knative-Build's
`TemplateInstantiationSpec`'s `Env` field.

Setting environment variables is useful for when a `TaskRun` author may
not know what variables are required ahead of time (e.g., Buildpacks).

fixes tektoncd#1606
@poy poy mentioned this issue Dec 1, 2019
3 tasks
@poy
Copy link
Contributor Author

poy commented Dec 1, 2019

@vdemeester @bobcatfish I took a swing at a PR for this so it was easier to visualize the API change: #1657

@bobcatfish
Copy link
Collaborator

Thanks for the extra info @poy!

So we are using Buildpacks with Tekton. Traditional Buildpacks (In Cloud Foundry) allowed
environment variables for settings.

I'm not super familiar with Buildpacks, could you give an example of what an example Task + TaskRun would look like for this?

We have no way of knowing what environment variables could be needed ahead of time as users can create their own Buildpacks.

I'm a bit confused - how does the user provide the environment variables to the TaskRun if it's not known ahead of time what env variables are needed? (I think maybe an example might clear this up for me!)

Another couple ideas of how you could do this:

  1. I think maybe you could have this same functionality if your Task had an array parameter which you could use to populate the environment variables (tho you'd have to do it for every step)
  2. You could use the stepTemplate to set the environment varaiables

@bobcatfish
Copy link
Collaborator

You could use the stepTemplate to set the environment varaiables

I think this has the same magic problem that is making me hesitate to agree with adding env vars on TaskRuns - but at least it's a feature we already have :S

btw I'm not sure if you've already been but if you're able to attend our working group could be a great place to discuss a feature request / design like this in real time

@bobcatfish
Copy link
Collaborator

(p.s. @vdemeester I noticed you put this into the beta milestone so we might be on slightly different pages about this?)

@vdemeester
Copy link
Member

@bobcatfish ah 😅 I did put it in the beta milestone just so that we discuss it while discussing beta, doesn't really mean I think it should go in for beta or not 👼

@skaegi
Copy link
Contributor

skaegi commented Dec 2, 2019

We use a well-known "named" ConfigMap to pass environment variable values into Steps. This works well when you know the environment variable name.

If you do not know the environment variable name in advance you can use envFrom or mount the ConfigMap as a volume and read and process the names and values as the first command in your Step. A similar approach is to mount an _env volume possibly generated in a previous Task and then in your Steps eval that before running your command.

@bobcatfish
Copy link
Collaborator

You could use the stepTemplate to set the environment varaiables

Just realized this is in the Task not the TaskRun so no help after all 🤦‍♀️ but @skaegi 's suggestions sound legit! and/or array param maybe?

@vdemeester
Copy link
Member

also, with embedded TaskSpec and PipelineSpec in the Run object & co, it's "kinda" covered, as you could "copy" a Task and embedded it with some changes 👼

@poy
Copy link
Contributor Author

poy commented Dec 3, 2019

@skaegi and @bobcatfish We actually use a secret today, however we were hoping to get away from managing the lifecycle of another object, however this is a valid option.

@vdemeester It seems weird to me that there are different API options if I use a Task vs embed it... But it also doesn't seem very clean to do so.

@vdemeester
Copy link
Member

@vdemeester It seems weird to me that there are different API options if I use a Task vs embed it... But it also doesn't seem very clean to do so.

There is not a different API options depending of if you use a Task or embedded it. But as this is "hidden" from the user (aka your tool handle that), you could do something like the following (in go code) :

  • get the task you want (in memory) — or even have your own defined in go code (using the Task struct), depends on how much you show the user / allow the user to modify/view.
  • add environments in there (in memory).
  • create a taskrun with embedded task, and use the task definition you just modify in the taskSpec.

You would use a given task as a "template" for your taskrun ; but all your taskrun would be "self-descriptive" and would not depend on any "task" per-se.

Not sure how clean or not it is 👼 it's just a possibility and a valid use-case I think 😉

@ghost
Copy link

ghost commented Dec 5, 2019

Just to try and move the conversation forward a bit more on this, there's another use case for the feature that I think is worth considering. Node.js libraries and apps commonly support a debug env var (e.g. DEBUG=express:router* means "print verbose logs from express:router modules") which a user may want to toggle on for specific runs of their tasks while they triage issues in their pipelines. I believe this is also true in Java land where env vars can be used to optionally tell an app to expose debugging ports while running.

I don't think it's great that in order to support those env vars every Java or node.js Task in the universe would need to individually expose their own debug param. This usage may not quite push the needle on deciding whether to add the feature or not but I wanted to point out that there are definitely use cases for custom runtime env vars beyond buildpack conf.

I'd also argue that, while it may not be awesome to allow runtime configuration outside of a task's declared params / artifacts, so long as the TaskRun stores the env vars that were passed in there will at least be a clear record of their usage and they aren't totally silent.

And, finally, just to throw some more options out there, this could conceivably be configurable on the Task:

kind: Task
allowRuntimeEnvVars: true # but defaults to false!

Having that would make runtime env var support explicitly opt-in and catalog tasks could totally leave it out so we don't propagate an anti-pattern.

@skaegi
Copy link
Contributor

skaegi commented Dec 5, 2019

I think at some point we have to recognize a Task author cannot make everything parameterizable. Not strictly for CI/CD and Tekton but when we hit these sorts of "debug" and "instrumentation" cases Kustomize is a pretty good solution.

@bobcatfish
Copy link
Collaborator

I just want to double check: are we sure that the array param won't work for @poy's use case?

@poy
Copy link
Contributor Author

poy commented Dec 13, 2019

I think we could duct-tape this solution in, however it comes with two major drawbacks (as far as I can tell)... We would have to require the entrypoint to be a shell. This would allow us to prepend the command with the given array. BUT:

  1. While I'm sure most containers will have a shell available in the path, I don't know that its guaranteed (containers generated from ko for example).

  2. It also implies we have to "crack open" the container a bit to invoke the default entrypoint/command so we can prepend the environment variables.

Overall, while I agree this would likely cover many use cases, it doesn't feel clean and certainly doesn't cover all of them.

@cccfeng
Copy link
Contributor

cccfeng commented Dec 14, 2019

@vdemeester It seems weird to me that there are different API options if I use a Task vs embed it... But it also doesn't seem very clean to do so.

There is not a different API options depending of if you use a Task or embedded it. But as this is "hidden" from the user (aka your tool handle that), you could do something like the following (in go code) :

  • get the task you want (in memory) — or even have your own defined in go code (using the Task struct), depends on how much you show the user / allow the user to modify/view.
  • add environments in there (in memory).
  • create a taskrun with embedded task, and use the task definition you just modify in the taskSpec.

You would use a given task as a "template" for your taskrun ; but all your taskrun would be "self-descriptive" and would not depend on any "task" per-se.

Not sure how clean or not it is 👼 it's just a possibility and a valid use-case I think 😉

This is really a way of implementation, but if I want to pull up taskRun or pipelineRun through tektoncd/trigger project, it seems to be a little difficult. IIRC resourceTemplates can only pull an instance like pipelineResource or pipelineRun, but can't dynamically modify the definition of task step's env. I agree with @sbwsg that the template writer cannot enumerate the parameters/envs that the template instance will pass in

@bobcatfish bobcatfish removed this from the Pipelines 1.0/beta 🐱 milestone Feb 18, 2020
@bobcatfish
Copy link
Collaborator

Thanks for the follow up @poy!

We would have to require the entrypoint to be a shell.

What about if the params could be a map or list of lists? So you could do something like:

apiVersion: tekton.dev/v1beta1
kind: TaskRun
spec:
    params:
    - name: settings
       value:
       - name: MY_ENV_VAR
          value: foo
---
apiVersion: tekton.dev/v1beta1
kind: Task
spec:
    params:
    - name: settings
       type: list-of-lists # probably something more complex with "schema" see #1393 
    steps:
    - image: my-image-with-no-shell
      envValueFrom: params.settings[*]

(Completely handwaving about the syntax above! I think that @skaegi is going to simultaneously hate the above syntax and have way better ideas)

If something like this would work for you @poy I'm happy to open another issue to explore further

@poy
Copy link
Contributor Author

poy commented Mar 13, 2020

If those settings are made available to the underlying container via environment variables, that sounds great!

piyush-garg added a commit to piyush-garg/pipeline that referenced this issue Nov 30, 2020
This will help to specificy env default, env at pipelinerun
and taskrun level. Env in pod template will take
precedence over one defined in step and stepTemplate

Env at pipelinerun and taskrun level will take precedence over
the one in default podtemplate

Fix tektoncd#1606
piyush-garg added a commit to piyush-garg/pipeline that referenced this issue Dec 8, 2020
This will help to specificy env default, env at pipelinerun
and taskrun level. Env in pod template at taskrun and pipelinerun
level will take precedence over one defined in step and stepTemplate

Env at pipelinerun and taskrun level will override the
default podtemplate

Fix tektoncd#1606
piyush-garg added a commit to piyush-garg/pipeline that referenced this issue Dec 8, 2020
This will help to specificy env default, env at pipelinerun
and taskrun level. Env in pod template at taskrun and pipelinerun
level will take precedence over one defined in step and stepTemplate

Env at pipelinerun and taskrun level will override the
default podtemplate

Fix tektoncd#1606
piyush-garg added a commit to piyush-garg/pipeline that referenced this issue Dec 8, 2020
This will help to specificy env default, env at pipelinerun
and taskrun level. Env in pod template at taskrun and pipelinerun
level will take precedence over one defined in step and stepTemplate

Env at pipelinerun and taskrun level will override the
default podtemplate

Fix tektoncd#1606
piyush-garg added a commit to piyush-garg/pipeline that referenced this issue Dec 8, 2020
This will help to specificy env default, env at pipelinerun
and taskrun level. Env in pod template at taskrun and pipelinerun
level will take precedence over one defined in step and stepTemplate

Env at pipelinerun and taskrun level will override the
default podtemplate

Fix tektoncd#1606
piyush-garg added a commit to piyush-garg/pipeline that referenced this issue Dec 8, 2020
This will help to specificy env default, env at pipelinerun
and taskrun level. Env in pod template at taskrun and pipelinerun
level will take precedence over one defined in step and stepTemplate

Env at pipelinerun and taskrun level will override the
default podtemplate

Fix tektoncd#1606
@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 9, 2021
@bobcatfish bobcatfish added the area/roadmap Issues that are part of the project (or organization) roadmap (usually an epic) label Feb 24, 2021
@bobcatfish
Copy link
Collaborator

This is (about to be) in our roadmap (WIP roadmap)

/lifecycle frozen

@tekton-robot tekton-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Feb 24, 2021
@devholic
Copy link
Contributor

Any updates on this issue? 🧐

Udiknedormin referenced this issue in rafalbigaj/community Dec 21, 2021
This document proposes support for environment variables configuration in `podTemplate`. That allows to exclude common variables to the global level as well as overwrite defaults specified on the particular step level.
@xchapter7x xchapter7x moved this to In Progress in Tekton Community Roadmap Sep 6, 2022
@jerop
Copy link
Member

jerop commented Feb 17, 2023

This issue was addressed in TEP-0101: Env in POD template - feel free to reopen if there are any further requests about this feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/roadmap Issues that are part of the project (or organization) roadmap (usually an epic) kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.