Implement Task/TaskRun Input Params #149
Conversation
knative-prow-robot
added
the
size/XXL
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dlorenc 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 |
knative-prow-robot
added
the
approved
|
one more thing, did you run ./hack/update-deps.sh to update vendor code? |
Thanks, redid it with that command. |
knative-prow-robot
added
do-not-merge/hold
knative-prow-robot
added
size/L
actually i screwed something up in vendor/... fixing it |
My git-fu wasn't good enough to go back and edit the initial dep commit. I squashed them for now since it's way fewer files than it was at first. |
There was a problem hiding this comment.
wooooot - pretty cool that we can re-use the functionality in knative/build
@dlorenc how do you feel about getting some docs started in our docs dir about how to use this? (i.e. user facing docs)
| @@ -107,8 +170,8 @@ func TestReconcile(t *testing.T) { | |||
| t.Errorf("Expected actions to be logged in the buildclient, got none") | |||
| } | |||
| build := client.Actions()[0].(ktesting.CreateAction).GetObject().(*buildv1alpha1.Build) | |||
| if d := cmp.Diff(build.Spec, buildSpec); d != "" { | |||
| t.Errorf("Expected created resource to be %v, but was %v", build.Spec, buildSpec) | |||
| if d := cmp.Diff(build.Spec, tc.wantedBuildSpec); d != "" { | |||
There was a problem hiding this comment.
im curious on your thoughts here @dlorenc - i feel like the set of situations this one particular test is supposed to handle are growing over time, what about creating a separate function specifically for testing the case where we have replacements in a task?
There was a problem hiding this comment.
+1 for moving this to a specific test.
There was a problem hiding this comment.
OK, cleaned up the test a bit. I actually ended up leaving this one in the same test, but reorganized the tests around it.
This test actually only had one other tc before this, but it was complicated enough to make that hard to see.
Now there's one test for times a build should have been created (with and without templating), and one for error cases.
dlorenc
force-pushed
the
params
branch
2 times, most recently
from
ed03a7a to
dd0138b
Compare
knative-prow-robot
added
size/XL
Done. |
|
/hold remove |
This adds support for Task/TaskRun Input Parameters using the 'ApplyReplacements' function from knative/build. Reusing this function ensures consistency in variable replacement between the projects. This change assumes that the supplied TaskRun.Spec.Inputs.Params have been valided against the Task.Inputs.Params elsewhere. We may want to revisit this and perform additional validation. This change does not introduce support for Resources or PipelineParams yet.
|
/hold cancel |
knative-prow-robot
removed
the
do-not-merge/hold
|
/lgtm |
| @@ -0,0 +1,49 @@ | |||
| ## Task Parameters | |||
There was a problem hiding this comment.
amaaaaazing
next time we touch this we should add a link from the main README to here, otherwise this would be hard to find
This commit builds upon tektoncd#149 and makes resource keys available for expansion inside a TaskRun. Resources are available under the namespace 'inputs.resources', and each resource is namespaced under it's own name. This commit adds to the PipelineResourceInterface, requiring types to support a Replacements method. This supplies keys to be replaced.
This commit builds upon tektoncd#149 and makes resource keys available for expansion inside a TaskRun. Resources are available under the namespace 'inputs.resources', and each resource is namespaced under it's own name. This commit adds to the PipelineResourceInterface, requiring types to support a Replacements method. This supplies keys to be replaced.
This commit builds upon tektoncd#149 and makes resource keys available for expansion inside a TaskRun. Resources are available under the namespace 'inputs.resources', and each resource is namespaced under it's own name. This commit adds to the PipelineResourceInterface, requiring types to support a Replacements method. This supplies keys to be replaced.
This commit builds upon #149 and makes resource keys available for expansion inside a TaskRun. Resources are available under the namespace 'inputs.resources', and each resource is namespaced under it's own name. This commit adds to the PipelineResourceInterface, requiring types to support a Replacements method. This supplies keys to be replaced.
Fixes tektoncd#149 Adds $(header) and $(header.X) as interpolation variables that will be substituted for header values from the event. So, header values can be used within TriggerBindings. Changes the "event" interpolation variable to "body" which nicely separates the "header" and "body" interpolation variables. Escapes double quotes in JSON strings that are substituted using variable interpolation. This fixes a bug where Triggers failed to create resources when a JSON object was passed as a parameter. The e2e test has been updated to cover this hole.
This adds support for Task/TaskRun Input Parameters using the 'ApplyReplacements' function from knative/build.
Reusing this function ensures consistency in variable replacement between the projects.
This change assumes that the supplied TaskRun.Spec.Inputs.Params have been valided against the Task.Inputs.Params elsewhere.
We may want to revisit this and perform additional validation.
This change does not introduce support for Resources or PipelineParams yet.
Vendor changes have been separated into another commit for easier reviewing.
Ref #64