Support list of "env" variables in the Build spec

[sbose78]

If the user defines the following,

kind: Build
...
spec:
  env:
    - name: "HTTP_PROXY"
      value: "http://my.enterprise.proxy"

1. The environment variables should be made available in the BuildStrategy container ( buildStep ) automatically.

2. These should, of course, not be available in the final deployable image created as a result of Support creation of lean application "runtime" images #183

3. Also, the BuildStrategy author should be able to refer to it in the container step or arg as $(build.env.HTTP_PROXY).

[qu1queee]

any reasons not to call the params similar to how is used in Tekton?

[sbose78]

It kinda brings us back to the strongly typed Vs lossely typed discussion.

The idea of having env vars for containers have a strong clear meaning in a strategy agnostic way - and hence I wanted to make them first class citizens in the Build CRD.

What do you think?

[qu1queee]

@sbose78 make sense, +1. Thanks for explaining

[otaviof]

It kinda brings us back to the strongly typed Vs lossely typed discussion.

The idea of having env vars for containers have a strong clear meaning in a strategy agnostic way - and hence I wanted to make them first class citizens in the Build CRD.

What do you think?

I like the idea of reusing common abstractions to represent the environment variables, so +1 on this.

And on supporting core-v1/EnvVar, what should we do with ValueFrom directive?

[sbose78]

We could start with it now, and then strip it down to a custom API as we deem fit? ValueFrom might be a handy thing in future anyway?

[adambkaplan]

ValueFrom is essential if you are passing credentials via an env var, such as AWS secret keys.

In general I'm OK with leaking core Kubernetes APIs into our own. "On top of k8s" is a fundamental assumption, whereas "powered by Tekton" is a current implementation.

[gabemontero]

On Wed, Jun 17, 2020 at 3:28 PM Adam Kaplan ***@***.***> wrote: ValueFrom is essential if you are passing credentials via an env var, such as AWS secret keys. In general I'm OK with leaking core Kubernetes APIs into our own. "On top of k8s" is a fundamental assumption, whereas "powered by Tekton" is a current implementation.

+1, assuming we are careful on which APIs we leak so as to not invite unintended escalation paths

…

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#224 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA3NU5EXQNKMSDT5DCIGIRTRXEKMPANCNFSM4NHJW5BA> .

[imjasonh]

I think we're all in agreement so far, but I'd like to amend this:

Also, the BuildStrategy author should be able to refer to it in the container step or arg as $(build.env.HTTP_PROXY).

I think any build.env values should just be appended to any underlying TaskRun steps' envs, overriding any BuildStrategy env values. This makes it easier for BuildStrategy authors to consume envs, since they can just be unaware of them. If they care, they can get env values using a shell, e.g.:

steps:
- image: busybox
  command: sh
  args:
  - -c
  - |
    echo 'user set HTTP_PROXY to "${HTTP_PROXY}"'

Having placeholders like $(build.env.HTTP_PROXY) means our code has to replace those placeholders, and that gets messy. We'll have to do it for parameters (#537) but we can get away without it for envs.

[gabemontero]

ValueFrom is essential if you are passing credentials via an env var, such as AWS secret keys.

In general I'm OK with leaking core Kubernetes APIs into our own. "On top of k8s" is a fundamental assumption, whereas "powered by Tekton" is a current implementation.

@adambkaplan @otaviof @sbose78 - circling back to the ValueFrom discussion from last June/July, for reference just in case anyone was unaware, here is the Tekton example of using ValueFrom: https://github.com/tektoncd/pipeline/blob/main/docs/tasks.md#using-a-secret-as-an-environment-source

You'll see in that example they leverage Tekton parameters to allow for a user supplied vs. hard coded secret name.

Correct me if I'm wrong @imjasonh but I believe supporting that pattern lines up with at lease some of your points in #224 (comment), correct ?

[gabemontero]

Some quick references I'm sure the folks already commenting here are aware of:

• https://github.com/tektoncd/pipeline/blob/main/docs/variables.md#fields-that-accept-variable-substitutions

(though I still have to refresh my memory on when in Tekton variable substitution occurs on a Task fields vs. a TaskRun.TaskSpec which is what shipwright generates)

• https://github.com/tektoncd/pipeline/blob/main/docs/tasks.md#specifying-a-step-template

(same note as above; also, the defaulting plus override notion seems valuable, but my EP will have to get into if/how to surface the existing Tekton stepTemplate into shipwright API)

• https://github.com/tektoncd/pipeline/blob/main/docs/tasks.md#using-a-secret-as-an-environment-source

(you see a direct use of params to seed the k8s valueFrom there)

[coreydaley]

/assign @coreydaley

[adambkaplan]

This work was completed in v0.6.0, under SHIP-0018.