Replace $(context...) values in resolver parameters
#5476
Conversation
fixes tektoncd#5475 I did not think of this previously, but now it works. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
tekton-robot
added
the
release-note-none
tekton-robot
added
the
size/M
|
/assign @jerop @lbernick @vdemeester |
|
The following is the coverage report on the affected files.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vdemeester The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
| @@ -74,6 +74,9 @@ func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clien | |||
| return func(ctx context.Context, name string) (v1beta1.PipelineObject, error) { | |||
| params := map[string]string{} | |||
| stringReplacements, arrayReplacements, objectReplacements := paramsFromPipelineRun(ctx, pipelineRun) | |||
| for k, v := range getContextReplacements("", pipelineRun) { | |||
There was a problem hiding this comment.
Is it possible to get the Pipeline name here? Do all resolvers need a param that is the pipeline name, or can we not make that guarantee?
There was a problem hiding this comment.
We can't make any guarantee, no, in almost all cases. In practice, the hub or bundles resolver params include what we expect will be the Pipeline name, but even then, we can't guarantee it - the actual Pipeline could have a different name than its key, e.g. The cluster resolver does know the name, but the git resolver has no idea.
There was a problem hiding this comment.
Could we add a new field to resolver for Pipeline name instead of passing it in as a param? I would imagine almost all resolvers would need it
There was a problem hiding this comment.
I don't think we should - we never create an actual Pipeline, we just load the spec in to PipelineRun.Spec.PipelineSpec...
There was a problem hiding this comment.
I don't think we should either.
There was a problem hiding this comment.
That and while the name happens to be a "key" for three of the current resolvers, there's no reason to assume that'll be the case for all or even necessarily a majority of future resolvers. Plus adding another name-related field to pipelineRef/taskRef would be pretty dang confusing.
|
/lgtm |
Changes
fixes #5475
I did not think of this previously, but now it works.
/kind bug
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes