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

Feature Request: Using ConfigMap Values For Params #1744

Open
tomfrenken opened this issue Dec 12, 2019 · 32 comments
Open

Feature Request: Using ConfigMap Values For Params #1744

tomfrenken opened this issue Dec 12, 2019 · 32 comments
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@tomfrenken
Copy link
Contributor

tomfrenken commented Dec 12, 2019

Expected Behavior

When defining parameters for a Task in my Pipeline, I would like to use values for the parameters which I defined in a ConfigMap, instead of hardcoding them in the Pipeline.

A cool solution would look like the use of ConfigMaps when defining environment variables in a Pod, there I am able to do this:

        env:
        - name: SOME_ENV_VARIABLE
          valueFrom:
            configMapKeyRef:
              name: my-config
              key: my-key

if params could work the same way it would be like this:

          params:
          - name: some-parameter
            valueFrom:
              configMapKeyRef:
                name: pipeline-config
                key: some-key

Actual Behavior

If I want to use a ConfigMap I can only use it on a Step level, and I can't directly access its values, this becomes an issue in the following Task:

apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: pmd-test
spec:
  inputs:
    resources:
      - name: git-source-code
        type: git
    params:
      - name: image
        type: string
        description: The image that will be used in the test
        default: rawdee/pmd
      - name: command
        type: string
        description: The command that will be used in the test
        default: pmd
      - name: args
        type: string
        description: The args that will be used in the test
        default: -language java -dir /workspace/git-source-code -rulesets ./my-rulesets.xml
  steps:
    - name: test-pmd
      image: $(input.params.image)
      command: ["$(input.params.command)"]
      args: ['$(input.params.args)']

in this example, I want to have the possibility configure the entire Step based on the ConfigMap, but currently it would look like this:

apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: pmd-test
spec:
  volumes:
    - name: config-volume
      configMap:
        name: pipeline-config
  inputs:
    resources:
      - name: git-source-code
        type: git
  steps:
    - name: test-pmd
      volumeMounts:
        - name: config-volume
          mountPath: /my-path
      image: $(cat /my-path/my-image)
      command: ["$(cat /my-path/my-command)"]
      args: ['$(cat /my-path/my-args)]

Note that this wouldn't work for the image since I'm only able to use variable substitution there.

@vdemeester
Copy link
Member

/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 12, 2019
@imjasonh
Copy link
Member

+1, we should make this change. This makes configuring the controller more standard, and could let users update the support images to use (git-init, creds-init, etc.) without requiring a restart of the controller pod.

@imjasonh
Copy link
Member

imjasonh commented Dec 12, 2019

Thinking through this a bit more:

Luckily at least some of the groundwork is already done: support images are parsed from flags and set as fields on the pipeline.Images type.

These fields should become methods that return the current value, and some background process could monitor the ConfigMap and update the values returned by that method.

As a first step we can update one image to use the ConfigMap style, then prove it works, then update the rest. It probably wouldn't be a very hard change at all.

@imjasonh imjasonh added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Dec 12, 2019
@imjasonh
Copy link
Member

Knative configures their queue proxy image using a ConfigMap: https://github.com/knative/serving/blob/b3079512daae9708f11c7b8ea41ef54023c91725/config/config-deployment.yaml#L26

Their pkg/deployment/config.go returns a *Config with a QueueSidecarImage field that gets updated when the ConfigMap changes.

Their "deployment config" object is effectively the same as our "pipeline.Images" object.

@skaegi
Copy link
Contributor

skaegi commented Dec 13, 2019

sigh -- and after I just closed #1197 . The case can be made for this feature however I can say that I really thought I wanted this feature but in practice found I never needed it.

@imjasonh
Copy link
Member

I'm not sure I understand how these are related?

That issue seems to be about letting users declare TaskRun/PipelineRun values as coming from a ConfigMap (which I admit does sound useful?).

This issue is about letting operators specify/update controller args using ConfigMaps, which is more of an operator concern and not really exposed to end users.

Am I misunderstanding?

@skaegi
Copy link
Contributor

skaegi commented Dec 13, 2019

Hmm... maybe I'm the one misunderstanding. @tomfrenken Is this issue for something like an install time kustomization of a Task and Pipeline. e.g. not something at runtime? Can you maybe describe a bit more what you're looking for.

@tomfrenken
Copy link
Contributor Author

tomfrenken commented Dec 13, 2019

@skaegi I think @imjasonh sums it up very well, that's precisely what I was meaning
But to answer your question a little better, yes in my case I thought about customization at install time.

@ankitm123
Copy link

if no one's working on it, I can take a look @imjasonh

@mgreau
Copy link

mgreau commented Apr 22, 2020

I’m really interested in that feature. I don’t know if it’s out of scope here but I think it could be great to be able to get the default value from the ConfigMap. It would mean having a way to deploy Tasks and Pipelines which could be invoked by operators via ConfigMaps and also by user via the CLI.

@ghost
Copy link

ghost commented Apr 22, 2020

Hey folks, I'm coming late to this issue but I was wondering if someone could clearly articulate use-cases and scope of this change. The description sounds to me exactly like @skaegi 's valueFrom issue and customization at install time isn't filling in the gaps for me. For example, from the issue description the following is stated as being desirable:

          params:
          - name: some-parameter
            valueFrom:
              configMapKeyRef:
                name: pipeline-config
                key: some-key

But then we seem to says this is not what's being asked for:

That issue seems to be about letting users declare TaskRun/PipelineRun values as coming from a ConfigMap (which I admit does sound useful?).
This issue is about letting operators specify/update controller args using ConfigMaps, which is more of an operator concern and not really exposed to end users.

And now I'm really confused. I see no mention of the controller or its arguments anywhere in the initial issue description. This issue really reads to me as precisely valueFrom as described in @skaegi's linked issue. So I'm clearly missing something, either from a gap in my own understanding or a gap in the way this issue is articulated.

So before we move ahead could somebody write out one or two clearly defined user-facing use cases and a description of what's in-scope / out-of-scope? It would be excellent to see what the intended final syntax looks like as well in the context of a Task / TaskRun / Pipeline / PipelineRun. I don't mind if it's a short design doc or just a comment here with some code blocks.

@ghost
Copy link

ghost commented Apr 22, 2020

Also: the title of this issue is literally "Feature Request: Using ConfigMap Values For Params". I am so confused.

@tekton-robot
Copy link
Collaborator

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
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.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot
Copy link
Collaborator

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 14, 2020
@tekton-robot
Copy link
Collaborator

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@hanzala1234
Copy link

Any update on this?

@gnunn1
Copy link

gnunn1 commented Jun 22, 2022

I just discovered this issue, for me this comes into play when I do not control the task, i.e. it's a ClusterTask or a Task being sourced from elsewhere. This task is essentially a black box that specifies parameters x,y,z and I would like to source them from a configmap or a secret, passing environment variables here isn't helpfull since the task is specifically expecting parameters.

The only way I can see to achieve this is to write a pre-task that sources the configmap or secret and outputs the values as results which can then be referenced as parameters. This feels overly convoluted and awkward, the original syntax that is being proposed here fits the bill of what I i'm looking for, i.e.:

          params:
          - name: some-parameter
            valueFrom:
              configMapKeyRef:
                name: pipeline-config
                key: some-key

@vdemeester
Copy link
Member

/reopen
I think this is still something you may want to pursue in some way or another.

@tekton-robot tekton-robot reopened this Jun 23, 2022
@tekton-robot
Copy link
Collaborator

@vdemeester: Reopened this issue.

In response to this:

/reopen
I think this is still something you may want to pursue in some way or another.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@lbernick lbernick added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 26, 2022
@MitchelNijdam-Rockstars
Copy link

I can provide a use case we are facing that this feature would solve.

We have a pipeline Task that builds a Java application using Gradle. In order to have a speedy execution, we have built an image containing all Gradle dependencies that we use in this Task. The Task looks like this now:

apiVersion: tekton.dev/v1beta1
kind: Task
...
spec:
    steps:
        image: "our.image.registry/gradle-cached-image:1.4.5"
...

Since the dependencies of our Java application regularly change, we automatically re-build this image and update the Task definition by hand.

It would be super useful if we could just update a ConfigMap in k8s, containing the new image name, and in the Task put something like:

kind: Task
spec:
    params:
       - name: gradle-image
          valueFrom:
              configMapKeyRef:
                  name: gradle-image
                  key: image
    steps:
        image: "$(params.gradle-image)"

@shuheiktgw
Copy link

@chengjoey @vdemeester @lbernick Hi, I ran into this issue and it seems the first attempt to implement the feature stopped due to the lack of TEP. May I take it over from there and start writing a TEP? 🙂

@lbernick
Copy link
Member

Definitely, thanks @shuheiktgw! You can find more info on the TEP process and how to create one here

@lbernick
Copy link
Member

Actually, it looks like there's an existing TEP for this: tektoncd/community#868
It may be worth checking in with the author to see whether they are still working on it.

@vdemeester vdemeester removed the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Nov 16, 2023
@JohanNordlinder
Copy link

JohanNordlinder commented Apr 17, 2024

I would really like this feature, use-case as follows.

We're trying to inject a SONARQUBE_TOKEN into the default (installed by the openshift-pipelines operator) s2i-java ClusterTask as a parameter. We do not want to hard-code the token into the pipeline params as we want to keep the repo containing the pipeline free from secrets. And we also do not want to fork the ClusterTask just to be able to add a valuefrom.secretkeyref-env binding.

apiVersion: tekton.dev/v1beta1
kind: Pipeline
spec:
  tasks:
    - name: s2i-build
      params:
        - name: ENV_VARS
          value:
            - >-
              SONARQUBE_TOKEN=wouldBeCoolToFetchFromCMOrSecret
      taskRef:
        kind: ClusterTask
        name: s2i-java

@mostafaCamel
Copy link

Hi, I am new-ish to Tekton and have started looking at this issue to work on it. I have a few questions:

  • Should valueFrom be available to Task parameters only or to both Pipeline and Task parameters?
  • Should these parameters be overridable by the user if the user assigns it a value in PipelineRun/TaskRun or should the api forbid PipelineRun/TaskRun to assign values to such a parameter?
    • I am tempted to say these parameters should not be overridable but on the other hand it would break the premise that a Param value can (or even must in case default is not defined) be set during PipelineRun/ TaskRun
  • Should these values be assigned once and for all during the PipelineRun/TaskRun initialization or should the controller subscribe to ConfigMap updates in order to change to update the variable values whenever the ConfigMap changes?
    • This could be a future improvement but I guess it is worth asking whether it is needed in Tekton given the usecase (ConfigMaps are not expected to change during a CI/CD execution)

@afrittoli
Copy link
Member

I would really like this feature, use-case as follows.

We're trying to inject a SONARQUBE_TOKEN into the default (installed by the openshift-pipelines operator) s2i-java ClusterTask as a parameter. We do not want to hard-code the token into the pipeline params as we want to keep the repo containing the pipeline free from secrets. And we also do not want to fork the ClusterTask just to be able to add a valuefrom.secretkeyref-env binding.

apiVersion: tekton.dev/v1beta1
kind: Pipeline
spec:
  tasks:
    - name: s2i-build
      params:
        - name: ENV_VARS
          value:
            - >-
              SONARQUBE_TOKEN=wouldBeCoolToFetchFromCMOrSecret
      taskRef:
        kind: ClusterTask
        name: s2i-java

@JohanNordlinder I don't know if this is still relevant- the problem with passing secrets into parameters is that resolved parameters may end-up into the status. If this feature is implemented for secrets, we should take extra care about this. There was some discussion about this on the TEP tektoncd/community#868.

Would mounting the secret via a workspace work for you? This is possible today

@afrittoli
Copy link
Member

Hi, I am new-ish to Tekton and have started looking at this issue to work on it. I have a few questions:

Hi, welcome to Tekton!

  • Should valueFrom be available to Task parameters only or to both Pipeline and Task parameters?

If it is implemented for one resource, it might as well be implemented for all.
There was a TEP related to this tektoncd/community#868 but it was abandoned.
Note that the TEP, as well as some of discussion on this issue, refers to TaskRun and PipelineRun, rather than Task and Pipeline, which personally I would consider a better option.

  • Should these parameters be overridable by the user if the user assigns it a value in PipelineRun/TaskRun or should the api forbid PipelineRun/TaskRun to assign values to such a parameter?

I think it should only be possible to define a config map at runtime. but we could explore use cases for using this as Task level.

  • I am tempted to say these parameters should not be overridable but on the other hand it would break the premise that a Param value can (or even must in case default is not defined) be set during PipelineRun/ TaskRun
  • Should these values be assigned once and for all during the PipelineRun/TaskRun initialization or should the controller subscribe to ConfigMap updates in order to change to update the variable values whenever the ConfigMap changes?

Subscribing for updates does not sound like a good idea, TaskRuns and PipelineRuns are meant to be executed the way they are defined when they are started. Some parameters are defined from the results of previous tasks, but those are produced according to the DAG, so they won't change once a Task is started

  • This could be a future improvement but I guess it is worth asking whether it is needed in Tekton given the usecase (ConfigMaps are not expected to change during a CI/CD execution)

@mostafaCamel
Copy link

Thanks Andrea for the detailed answer! I have one more question.

Note that the TEP, as well as some of discussion on this issue, refers to TaskRun and PipelineRun, rather than Task and Pipeline, which personally I would consider a better option.
[...]
I think it should only be possible to define a config map at runtime. but we could explore use cases for using this as Task level

I agree with you (if I understood correctly) that it's better to define valueFrom in Task and Pipeline rather than PipelineRun and TaskRun but that the validation about the ConfigMap existence / variable existence in the ConfigMap should occur during PipelineRun/ TaskRun

My question about overridability is for the following usecase. Should the TaskRun fail because the user is trying to set the value during TaskRun or should the TaskRun not fail and allow the user to override the value that would be obtained from the configmap by the value defined in the TaskRun?

Task

apiVersion: tekton.dev/v1
kind: Task
metadata:
  name: hello-task
spec:
  params:
  - name: USERNAME
    valueFrom:
      configMapKeyRef:
        name: my-config-map
        key: username

TaskRun

apiVersion: tekton.dev/v1
kind: TaskRun
metadata:
  name: hello-task-run
spec:
  taskRef:
    name: hello-task
  params:
  - name: USERNAME
    value: "Tekton"

@JohanNordlinder
Copy link

JohanNordlinder commented Feb 25, 2025

I would really like this feature, use-case as follows.
We're trying to inject a SONARQUBE_TOKEN into the default (installed by the openshift-pipelines operator) s2i-java ClusterTask as a parameter. We do not want to hard-code the token into the pipeline params as we want to keep the repo containing the pipeline free from secrets. And we also do not want to fork the ClusterTask just to be able to add a valuefrom.secretkeyref-env binding.

apiVersion: tekton.dev/v1beta1
kind: Pipeline
spec:
  tasks:
    - name: s2i-build
      params:
        - name: ENV_VARS
          value:
            - >-
              SONARQUBE_TOKEN=wouldBeCoolToFetchFromCMOrSecret
      taskRef:
        kind: ClusterTask
        name: s2i-java

@JohanNordlinder I don't know if this is still relevant- the problem with passing secrets into parameters is that resolved parameters may end-up into the status. If this feature is implemented for secrets, we should take extra care about this. There was some discussion about this on the TEP tektoncd/community#868.

Would mounting the secret via a workspace work for you? This is possible today

Yes @afrittoli it's still relevant for us, right now I'm using the workaround proposed above by @gnunn1 with a pre-task that exposes the token as a task result which I then pass as the param value to the task.

Mounting the secret via a workspace is an interesting alternative, but I see two reasons this isn't a perfect fit;

  • Looking at the documentation it seems this can only be done in a TaskRun or PipelineRun. Our developers trigger pipelines from within the OpenShift GUI so any settings not already present in the Pipeline itself is somewhat cumbersome.
  • It's only a viable solution if you have ownership over the Task you want to reuse, it will be hard or impossible to use this method with a imported task that expect a certain set of parameters. And if you do have ownership over the tasks you might as well just use a secretRef in the step-def.

Bottom line; tasks would be so much easier to reuse if it was possible to reference a ConfigMap or Secret in the param declaration of a taskRef.

@mostafaCamel
Copy link

I agree with you (if I understood correctly) that it's better to define valueFrom in Task and Pipeline rather than PipelineRun and TaskRun but that the validation about the ConfigMap existence / variable existence in the ConfigMap should occur during PipelineRun/ TaskRun
My bad. It looks like I misunderstood Andrea's message. From reading tektoncd/community#868 and re-reading the discussion, it looks like valueFrom is rather needed in PipelineRun /TaskRun and not Pipeline/ Task

Each of PipelineRun , TaskRun , PipelineTask (Johan's usecase in the above comment) uses the type Param

type Param struct {
	Name  string     `json:"name"`
	Value ParamValue `json:"value"`
}

so I can just a add a new field to this struct (would start with ConfigMap only and not SecretKeyRef for now)

Will try to have a TEP and a PR up by end of next week

@mostafaCamel
Copy link

Apologies for the delay:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.
Projects
Status: Todo
Development

No branches or pull requests