-
Notifications
You must be signed in to change notification settings - Fork 237
TEP-0125: Configmap and Secret as Param Value Source #868
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
Conversation
|
/kind tep |
|
/assign @afrittoli |
|
@dibyom: GitHub didn't allow me to assign the following users: pxp928. Note that only tektoncd members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. DetailsIn response to this:
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. |
| type Param struct { | ||
| Name string `json:"name"` | ||
| Value ArrayOrString `json:"value"` | ||
| // Additional field ValueFrom to fetch value from ConfigMap or Secret | ||
| ValueFrom *ValueSource `json:"valueFrom,omitempty" protobuf:"bytes,3,opt,name=valueFrom"` | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not what a Param looks like today - it changed with array params and object params:
type Param struct {
Name string `json:"name"`
Value ParamValue `json:"value"`
}
(...)
type ParamValue struct {
Type ParamType `json:"type"` // Represents the stored type of ParamValues.
StringVal string `json:"stringVal"`
// +listType=atomic
ArrayVal []string `json:"arrayVal"`
ObjectVal map[string]string `json:"objectVal"`
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Param struct is of PipelineRun / TaskRun
not Pipeline / Task
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The target of this TEP to specify the value from config maps and secrets directly in TaskRun / PipelineRun not in Task / Pipeline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, indeed, the TEP is about TaskRun and PipelineRun.
If you look at the specification of a PipelineRun, you can it uses a type called Param, which has a name and a ParamValue
The code in your example specifies ArrayOrString for the type of value, but that is not true anymore, value is now of type ParamValue.
type Param struct {
Name string `json:"name"`
Value ArrayOrString `json:"value"`
// Additional field ValueFrom to fetch value from ConfigMap or Secret
ValueFrom *ValueSource `json:"valueFrom,omitempty" protobuf:"bytes,3,opt,name=valueFrom"`
}It would be good to define the behaviour for parameters of type array and object
afrittoli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your proposal! - see my comments inline.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/assign |
|
/test pull-community-teps-lint |
|
@afrittoli: No presubmit jobs available for tektoncd/community@main DetailsIn response to this:
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. |
|
@rpajay thank you for the updates! There are still some open comments, let me know what you think or if there is anything I can do to help clarify them. |
|
API WG - @rpajay please take a look for another round of reviews, thanks! |
| - name: FROM_SECRET | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: pipeline-run-secret | ||
| key: HELLO_WORLD_KEY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
param values are stored in cleartext as part of the CRD. Maybe I'm missing something but does this include plans for scrubbing param values that come from secrets?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point @lbernick. If a PipelineRun parameter is passed down to a TaskRun, it could use the same valueFrom syntax, but only when passed alone (i.e. not concatenated to other strings). The TaskRun controller again could use the same syntax for pods, but that would mean that the secret value could only be available to steps as an environment variable. Using secrets with environment variables can be already done today thanks to https://github.com/tektoncd/community/blob/main/teps/0101-env-in-pod-template.md
|
API WG - under review - @rpajay please respond back if possible, appreciate it! Thanks! |
|
Issues go stale after 90d of inactivity. /lifecycle stale Send feedback to tektoncd/plumbing. |
|
API WG - @rpajay do we want to keep this proposal open? Do you need any help? |
|
Stale issues rot after 30d of inactivity. /lifecycle rotten Send feedback to tektoncd/plumbing. |
|
Rotten issues close after 30d of inactivity. /close Send feedback to tektoncd/plumbing. |
|
@tekton-robot: Closed this PR. DetailsIn response to this:
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. |
This proposal is to be able to reference `ConfigMap` as a value source for a `Param` in `TaskRun` or `PipelineRun` . This is to support Kubernetes native options (ConfigMap) as value source along with direct value passed to `TaskRun` and `PipelineRun`. Note: this proposal is basically picking up upon the closed (unmerged) [proposal](tektoncd#868). Signed-off-by: Mostafa Abdelwahab <moustapha.hicham@gmail.com>
This proposal is to be able to reference `ConfigMap` as a value source for a `Param` in `TaskRun` or `PipelineRun` . This is to support Kubernetes native options (ConfigMap) as value source along with direct value passed to `TaskRun` and `PipelineRun`. Note: this proposal is basically picking up upon the closed (unmerged) [proposal](tektoncd#868). Signed-off-by: Mostafa Abdelwahab <moustapha.hicham@gmail.com>
This proposal is to be able to reference `ConfigMap` as a value source for a `Param` in `TaskRun` or `PipelineRun` . This is to support Kubernetes native options (ConfigMap) as value source along with direct value passed to `TaskRun` and `PipelineRun`. Note: this proposal is basically picking up upon the closed (unmerged) [proposal](tektoncd#868). Signed-off-by: Mostafa Abdelwahab <moustapha.hicham@gmail.com>
This proposal is to be able to reference `ConfigMap` as a value source for a `Param` in `TaskRun` or `PipelineRun` . This is to support Kubernetes native options (ConfigMap) as value source along with direct value passed to `TaskRun` and `PipelineRun`. Note: this proposal is basically picking up upon the closed (unmerged) [proposal](tektoncd#868). Signed-off-by: Mostafa Abdelwahab <moustapha.hicham@gmail.com>
This proposal is to be able to reference `ConfigMap` as a value source for a `Param` in `TaskRun` or `PipelineRun` . This is to support Kubernetes native options (ConfigMap) as value source along with direct value passed to `TaskRun` and `PipelineRun`. Note: this proposal is basically picking up upon the closed (unmerged) [proposal](tektoncd#868). Signed-off-by: Mostafa Abdelwahab <moustapha.hicham@gmail.com>
This proposal is to be able to reference `ConfigMap` as a value source for a `Param` in `TaskRun` or `PipelineRun` . This is to support Kubernetes native options (ConfigMap) as value source along with direct value passed to `TaskRun` and `PipelineRun`. Note: this proposal is basically picking up upon the closed (unmerged) [proposal](tektoncd#868). Signed-off-by: Mostafa Abdelwahab <moustapha.hicham@gmail.com>
No description provided.