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.

Sign up for GitHub

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

Jump to bottom

Replace $(context...) values in resolver parameters #5476

Merged
merged 1 commit into from
Sep 13, 2022
Merged

Replace $(context...) values in resolver parameters #5476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions pkg/reconciler/pipelinerun/resources/apply.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,19 @@ func paramsFromPipelineRun(ctx context.Context, pr *v1beta1.PipelineRun) (map[st
return stringReplacements, arrayReplacements, objectReplacements
}

// ApplyContexts applies the substitution from $(context.(pipelineRun|pipeline).*) with the specified values.
// Currently supports only name substitution. Uses "" as a default if name is not specified.
func ApplyContexts(ctx context.Context, spec *v1beta1.PipelineSpec, pipelineName string, pr *v1beta1.PipelineRun) *v1beta1.PipelineSpec {
replacements := map[string]string{
func getContextReplacements(pipelineName string, pr *v1beta1.PipelineRun) map[string]string {
return map[string]string{
"context.pipelineRun.name": pr.Name,
"context.pipeline.name": pipelineName,
"context.pipelineRun.namespace": pr.Namespace,
"context.pipelineRun.uid": string(pr.ObjectMeta.UID),
}
return ApplyReplacements(ctx, spec, replacements, map[string][]string{}, map[string]map[string]string{})
}

// ApplyContexts applies the substitution from $(context.(pipelineRun|pipeline).*) with the specified values.
// Currently supports only name substitution. Uses "" as a default if name is not specified.
func ApplyContexts(ctx context.Context, spec *v1beta1.PipelineSpec, pipelineName string, pr *v1beta1.PipelineRun) *v1beta1.PipelineSpec {
return ApplyReplacements(ctx, spec, getContextReplacements(pipelineName, pr), map[string][]string{}, map[string]map[string]string{})
}

// ApplyPipelineTaskContexts applies the substitution from $(context.pipelineTask.*) with the specified values.
Expand Down
3 changes: 3 additions & 0 deletions pkg/reconciler/pipelinerun/resources/pipelineref.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should - we never create an actual Pipeline, we just load the spec in to PipelineRun.Spec.PipelineSpec...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, sounds good

stringReplacements[k] = v
}
replacedParams := replaceParamValues(pr.Params, stringReplacements, arrayReplacements, objectReplacements)
for _, p := range replacedParams {
params[p.Name] = p.Value.StringVal
Expand Down
29 changes: 22 additions & 7 deletions pkg/reconciler/pipelinerun/resources/pipelineref_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,10 @@ func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) {
Resolver: "git",
Params: []v1beta1.Param{{
Name: "foo",
Value: *v1beta1.NewArrayOrString("$(params.resolver-param)"),
Value: *v1beta1.NewStructuredValues("$(params.resolver-param)"),
}, {
Name: "bar",
Value: *v1beta1.NewStructuredValues("$(context.pipelineRun.name)"),
}},
},
}
Expand All @@ -319,16 +322,22 @@ func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) {
resolved := test.NewResolvedResource([]byte(pipelineYAML), nil, nil)
requester := &test.Requester{
ResolvedResource: resolved,
Params: map[string]string{"foo": "bar"},
Params: map[string]string{
"foo": "bar",
"bar": "test-pipeline",
},
}
fn, err := resources.GetPipelineFunc(ctx, nil, nil, requester, &v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{Namespace: "default"},
ObjectMeta: metav1.ObjectMeta{
Name: "test-pipeline",
Namespace: "default",
},
Spec: v1beta1.PipelineRunSpec{
PipelineRef: pipelineRef,
ServiceAccountName: "default",
Params: []v1beta1.Param{{
Name: "resolver-param",
Value: *v1beta1.NewArrayOrString("bar"),
Value: *v1beta1.NewStructuredValues("bar"),
}},
},
})
Expand All @@ -350,19 +359,25 @@ func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) {
Resolver: "git",
Params: []v1beta1.Param{{
Name: "foo",
Value: *v1beta1.NewArrayOrString("$(params.resolver-param)"),
Value: *v1beta1.NewStructuredValues("$(params.resolver-param)"),
}, {
Name: "bar",
Value: *v1beta1.NewStructuredValues("$(context.pipelineRun.name)"),
}},
},
}

fnNotMatching, err := resources.GetPipelineFunc(ctx, nil, nil, requester, &v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{Namespace: "default"},
ObjectMeta: metav1.ObjectMeta{
Name: "other-pipeline",
Namespace: "default",
},
Spec: v1beta1.PipelineRunSpec{
PipelineRef: pipelineRefNotMatching,
ServiceAccountName: "default",
Params: []v1beta1.Param{{
Name: "resolver-param",
Value: *v1beta1.NewArrayOrString("banana"),
Value: *v1beta1.NewStructuredValues("banana"),
}},
},
})
Expand Down
13 changes: 8 additions & 5 deletions pkg/reconciler/taskrun/resources/apply.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,20 @@ func ApplyResources(spec *v1beta1.TaskSpec, resolvedResources map[string]v1beta1
return ApplyReplacements(spec, replacements, map[string][]string{})
}

// ApplyContexts applies the substitution from $(context.(taskRun|task).*) with the specified values.
// Uses "" as a default if a value is not available.
func ApplyContexts(spec *v1beta1.TaskSpec, taskName string, tr *v1beta1.TaskRun) *v1beta1.TaskSpec {
replacements := map[string]string{
func getContextReplacements(taskName string, tr *v1beta1.TaskRun) map[string]string {
return map[string]string{
"context.taskRun.name": tr.Name,
"context.task.name": taskName,
"context.taskRun.namespace": tr.Namespace,
"context.taskRun.uid": string(tr.ObjectMeta.UID),
"context.task.retry-count": strconv.Itoa(len(tr.Status.RetriesStatus)),
}
return ApplyReplacements(spec, replacements, map[string][]string{})
}

// ApplyContexts applies the substitution from $(context.(taskRun|task).*) with the specified values.
// Uses "" as a default if a value is not available.
func ApplyContexts(spec *v1beta1.TaskSpec, taskName string, tr *v1beta1.TaskRun) *v1beta1.TaskSpec {
return ApplyReplacements(spec, getContextReplacements(taskName, tr), map[string][]string{})
}

// ApplyWorkspaces applies the substitution from paths that the workspaces in declarations mounted to, the
Expand Down
3 changes: 3 additions & 0 deletions pkg/reconciler/taskrun/resources/taskref.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset
var replacedParams []v1beta1.Param
if ownerAsTR, ok := owner.(*v1beta1.TaskRun); ok {
stringReplacements, arrayReplacements := paramsFromTaskRun(ctx, ownerAsTR)
for k, v := range getContextReplacements("", ownerAsTR) {
stringReplacements[k] = v
}
for _, p := range tr.Params {
p.Value.ApplyReplacements(stringReplacements, arrayReplacements, nil)
replacedParams = append(replacedParams, p)
Expand Down
29 changes: 22 additions & 7 deletions pkg/reconciler/taskrun/resources/taskref_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,10 @@ func TestGetTaskFunc_RemoteResolution_ReplacedParams(t *testing.T) {
Resolver: "git",
Params: []v1beta1.Param{{
Name: "foo",
Value: *v1beta1.NewArrayOrString("$(params.resolver-param)"),
Value: *v1beta1.NewStructuredValues("$(params.resolver-param)"),
}, {
Name: "bar",
Value: *v1beta1.NewStructuredValues("$(context.taskRun.name)"),
}},
},
}
Expand All @@ -565,16 +568,22 @@ func TestGetTaskFunc_RemoteResolution_ReplacedParams(t *testing.T) {
resolved := test.NewResolvedResource([]byte(taskYAML), nil, nil)
requester := &test.Requester{
ResolvedResource: resolved,
Params: map[string]string{"foo": "bar"},
Params: map[string]string{
"foo": "bar",
"bar": "test-task",
},
}
tr := &v1beta1.TaskRun{
ObjectMeta: metav1.ObjectMeta{Namespace: "default"},
ObjectMeta: metav1.ObjectMeta{
Name: "test-task",
Namespace: "default",
},
Spec: v1beta1.TaskRunSpec{
TaskRef: taskRef,
ServiceAccountName: "default",
Params: []v1beta1.Param{{
Name: "resolver-param",
Value: *v1beta1.NewArrayOrString("bar"),
Value: *v1beta1.NewStructuredValues("bar"),
}},
},
}
Expand All @@ -597,19 +606,25 @@ func TestGetTaskFunc_RemoteResolution_ReplacedParams(t *testing.T) {
Resolver: "git",
Params: []v1beta1.Param{{
Name: "foo",
Value: *v1beta1.NewArrayOrString("$(params.resolver-param)"),
Value: *v1beta1.NewStructuredValues("$(params.resolver-param)"),
}, {
Name: "bar",
Value: *v1beta1.NewStructuredValues("$(context.taskRun.name)"),
}},
},
}

trNotMatching := &v1beta1.TaskRun{
ObjectMeta: metav1.ObjectMeta{Namespace: "default"},
ObjectMeta: metav1.ObjectMeta{
Name: "other-task",
Namespace: "default",
},
Spec: v1beta1.TaskRunSpec{
TaskRef: taskRefNotMatching,
ServiceAccountName: "default",
Params: []v1beta1.Param{{
Name: "resolver-param",
Value: *v1beta1.NewArrayOrString("banana"),
Value: *v1beta1.NewStructuredValues("banana"),
}},
},
}
Expand Down
8 changes: 7 additions & 1 deletion test/resolution.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package test

import (
"context"
"errors"
"fmt"
"strings"

resolution "github.com/tektoncd/pipeline/pkg/resolution/resource"
)
Expand Down Expand Up @@ -54,11 +56,15 @@ func (r *Requester) Submit(ctx context.Context, resolverName resolution.Resolver
reqParams[k] = v
}

var wrongParams []string
for k, v := range r.Params {
if reqValue, ok := reqParams[k]; !ok || reqValue != v {
return nil, fmt.Errorf("expected %s param to be %s, but was %s", k, v, reqValue)
wrongParams = append(wrongParams, fmt.Sprintf("expected %s param to be %s, but was %s", k, v, reqValue))
}
}
if len(wrongParams) > 0 {
return nil, errors.New(strings.Join(wrongParams, "; "))
}

return r.ResolvedResource, r.SubmitErr
}
Expand Down