Implement Task/TaskRun Input Parameters.

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.

Gopkg.toml

@@ -62,7 +62,7 @@ required = [
 [[constraint]]
   name = "github.com/knative/build"
   # HEAD as of 2018-10-09
-  revision = "7360ec685b35802d4703b31e6c0be2cc04b4503c"
+  revision = "92a1258647b272a3a4505616b102d6e8f86be082"
 
 [prune]
   go-tests = true

docs/task-parameters.md

@@ -0,0 +1,49 @@
+## Task Parameters
+
+Tasks can declare input parameters that must be supplied to the task during a TaskRun.
+Some example use-cases of this include:
+
+* A task that needs to know what compilation flags to use when building an application.
+* A task that needs to know what to name a built artifact.
+* A task that supports several different strategies, and leaves the choice up to the other.
+
+### Usage
+
+The following example shows how Tasks can be parameterized, and these parameters can be passed to the `Task` from a `TaskRun`.
+
+Input parameters in the form of `${inputs.foo}` are replaced inside of the buildSpec.
+
+The following `Task` declares an input parameter called 'flags', and uses it in the `buildSpec.steps.args` list.
+
+```yaml
+apiVersion: pipeline.knative.dev/v1alpha1
+kind: Task
+metadata:
+  name: task-with-parameters
+spec:
+    inputs:
+        params:
+        - name: flags
+          value: string
+    buildSpec:
+        steps:
+        - name: build
+          image: my-builder
+          args: ['build', '--flags=${inputs.flags}']
+```
+
+The following `TaskRun` supplies a value for `flags`:
+
+```yaml
+apiVersion: pipeline.knative.dev/v1alpha1
+kind: TaskRun
+metadata:
+  name: run-with-parameters
+spec:
+    taskRef:
+        name: task-with-parameters
+    inputs:
+        params:
+        - name: 'flags'
+          value: 'foo=bar,baz=bat'
+```

pkg/reconciler/v1alpha1/taskrun/taskrun.go

@@ -20,6 +20,8 @@ import (
 	"fmt"
 	"reflect"
 
+	"github.com/knative/build/pkg/builder"
+
 	"github.com/knative/build-pipeline/pkg/apis/pipeline/v1alpha1"
 	"github.com/knative/build-pipeline/pkg/reconciler"
 	resources "github.com/knative/build-pipeline/pkg/reconciler/v1alpha1/taskrun/resources"
@@ -242,6 +244,9 @@ func (c *Reconciler) makeBuild(tr *v1alpha1.TaskRun, logger *zap.SugaredLogger)
 		return nil, err
 	}
 
+	// Apply parameters from the taskrun.
+	build = applyParameters(b, t, tr)
+
 	createdBuild, err := c.BuildClientSet.BuildV1alpha1().Builds(tr.Namespace).Create(build)
 	if err != nil {
 		logger.Errorf("Failed to create build for taskrun %s, %v", tr.Name, err)
@@ -258,6 +263,15 @@ func (c *Reconciler) makeBuild(tr *v1alpha1.TaskRun, logger *zap.SugaredLogger)
 		logger.Errorf("Failed to create tracker for build %s for taskrun %s: %v", buildRef, tr.Name, err)
 		return nil, err
 	}
-
 	return createdBuild, nil
 }
+
+func applyParameters(b *buildv1alpha1.Build, t *v1alpha1.Task, tr *v1alpha1.TaskRun) *buildv1alpha1.Build {
+	// This assumes that the TaskRun inputs have been validated against what the Task requests.
+	replacements := map[string]string{}
+	for _, p := range tr.Spec.Inputs.Params {
+		replacements[fmt.Sprintf("inputs.params.%s", p.Name)] = p.Value
+	}
+
+	return builder.ApplyReplacements(b, replacements)
+}

pkg/reconciler/v1alpha1/taskrun/taskrun_test.go

@@ -18,6 +18,8 @@ import (
 	"testing"
 	"time"
 
+	corev1 "k8s.io/api/core/v1"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/knative/build-pipeline/pkg/apis/pipeline/v1alpha1"
 	fakepipelineclientset "github.com/knative/build-pipeline/pkg/client/clientset/versioned/fake"
@@ -26,7 +28,6 @@ import (
 	buildv1alpha1 "github.com/knative/build/pkg/apis/build/v1alpha1"
 	fakebuildclientset "github.com/knative/build/pkg/client/clientset/versioned/fake"
 	buildinformers "github.com/knative/build/pkg/client/informers/externalversions"
-
 	"github.com/knative/pkg/controller"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zaptest/observer"
@@ -35,8 +36,49 @@ import (
 	ktesting "k8s.io/client-go/testing"
 )
 
-func TestReconcile(t *testing.T) {
-	taskname := "test-task"
+var kanikoTask = &v1alpha1.Task{
+	ObjectMeta: metav1.ObjectMeta{
+		Name:      "test-task",
+		Namespace: "foo",
+	},
+	Spec: v1alpha1.TaskSpec{
+		BuildSpec: &buildv1alpha1.BuildSpec{
+			Template: &buildv1alpha1.TemplateInstantiationSpec{
+				Name: "kaniko",
+				Arguments: []buildv1alpha1.ArgumentSpec{
+					buildv1alpha1.ArgumentSpec{
+						Name:  "DOCKERFILE",
+						Value: "${PATH_TO_DOCKERFILE}",
+					},
+					buildv1alpha1.ArgumentSpec{
+						Name:  "REGISTRY",
+						Value: "${REGISTRY}",
+					},
+				},
+			},
+		},
+	},
+}
+
+var templatedTask = &v1alpha1.Task{
+	ObjectMeta: metav1.ObjectMeta{
+		Name:      "task-with-templating",
+		Namespace: "foo",
+	},
+	Spec: v1alpha1.TaskSpec{
+		BuildSpec: &buildv1alpha1.BuildSpec{
+			Steps: []corev1.Container{
+				{
+					Name:  "mycontainer",
+					Image: "myimage",
+					Args:  []string{"--my-arg=${inputs.params.myarg}"},
+				},
+			},
+		},
+	},
+}
+
+func TestReconcileBuildsCreated(t *testing.T) {
 	taskruns := []*v1alpha1.TaskRun{
 		{
 			ObjectMeta: metav1.ObjectMeta{
@@ -45,77 +87,141 @@ func TestReconcile(t *testing.T) {
 			},
 			Spec: v1alpha1.TaskRunSpec{
 				TaskRef: v1alpha1.TaskRef{
-					Name:       taskname,
+					Name:       "test-task",
+					APIVersion: "a1",
+				},
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test-taskrun-templating",
+				Namespace: "foo",
+			},
+			Spec: v1alpha1.TaskRunSpec{
+				TaskRef: v1alpha1.TaskRef{
+					Name:       "task-with-templating",
 					APIVersion: "a1",
 				},
-			}},
+				Inputs: v1alpha1.TaskRunInputs{
+					Params: []v1alpha1.Param{
+						{
+							Name:  "myarg",
+							Value: "foo",
+						},
+					},
+				},
+			},
+		},
 	}
 
-	buildSpec := buildv1alpha1.BuildSpec{
-		Template: &buildv1alpha1.TemplateInstantiationSpec{
-			Name: "kaniko",
-			Arguments: []buildv1alpha1.ArgumentSpec{
-				buildv1alpha1.ArgumentSpec{
-					Name:  "DOCKERFILE",
-					Value: "${PATH_TO_DOCKERFILE}",
+	d := testData{
+		taskruns: taskruns,
+		tasks:    []*v1alpha1.Task{kanikoTask, templatedTask},
+	}
+	testcases := []struct {
+		name            string
+		taskRun         string
+		wantedBuildSpec buildv1alpha1.BuildSpec
+	}{
+		{
+			name:    "success",
+			taskRun: "foo/test-taskrun-run-success",
+			wantedBuildSpec: buildv1alpha1.BuildSpec{
+				Template: &buildv1alpha1.TemplateInstantiationSpec{
+					Name: "kaniko",
+					Arguments: []buildv1alpha1.ArgumentSpec{
+						buildv1alpha1.ArgumentSpec{
+							Name:  "DOCKERFILE",
+							Value: "${PATH_TO_DOCKERFILE}",
+						},
+						buildv1alpha1.ArgumentSpec{
+							Name:  "REGISTRY",
+							Value: "${REGISTRY}",
+						},
+					},
 				},
-				buildv1alpha1.ArgumentSpec{
-					Name:  "REGISTRY",
-					Value: "${REGISTRY}",
+			},
+		},
+		{
+			name:    "params",
+			taskRun: "foo/test-taskrun-templating",
+			wantedBuildSpec: buildv1alpha1.BuildSpec{
+				Steps: []corev1.Container{
+					{
+						Name:  "mycontainer",
+						Image: "myimage",
+						Args:  []string{"--my-arg=foo"},
+					},
 				},
-			}},
+			},
+		},
 	}
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			c, _, client := getController(d)
+			if err := c.Reconciler.Reconcile(context.Background(), tc.taskRun); err != nil {
+				t.Errorf("expected no error. Got error %v", err)
+			}
 
-	tasks := []*v1alpha1.Task{
-		{
+			if len(client.Actions()) == 0 {
+				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, tc.wantedBuildSpec); d != "" {
+				t.Errorf("buildspec doesn't match, diff: %s", d)
+			}
+		})
+	}
+}
+
+func TestReconcileBuildCreationErrors(t *testing.T) {
+	taskRuns := []*v1alpha1.TaskRun{
+		&v1alpha1.TaskRun{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      taskname,
+				Name:      "notaskrun",
 				Namespace: "foo",
 			},
-			Spec: v1alpha1.TaskSpec{
-				BuildSpec: &buildSpec,
-			}},
+			Spec: v1alpha1.TaskRunSpec{
+				TaskRef: v1alpha1.TaskRef{
+					Name:       "notask",
+					APIVersion: "a1",
+				},
+			},
+		},
+	}
+
+	tasks := []*v1alpha1.Task{
+		kanikoTask,
 	}
+
 	d := testData{
-		taskruns: taskruns,
+		taskruns: taskRuns,
 		tasks:    tasks,
 	}
+
 	testcases := []struct {
-		name         string
-		taskRun      string
-		shdErr       bool
-		shdMakebuild bool
-		log          string
+		name    string
+		taskRun string
 	}{
-		{"success", "foo/test-taskrun-run-success", false, true, ""},
+		{
+			name:    "task run with no task",
+			taskRun: "foo/notaskrun",
+		},
 	}
+
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
-			c, logs, client := getController(d)
-			err := c.Reconciler.Reconcile(context.Background(), tc.taskRun)
-			if tc.shdErr != (err != nil) {
-				t.Errorf("expected to see error %t. Got error %v", tc.shdErr, err)
+			c, _, client := getController(d)
+			if err := c.Reconciler.Reconcile(context.Background(), tc.taskRun); err == nil {
+				t.Error("Expected reconcile to error. Got nil")
 			}
-			if tc.log == "" && logs.Len() > 0 {
-				t.Errorf("expected to see no error log. However found errors in logs: %v", logs)
-			} else if tc.log != "" && logs.FilterMessage(tc.log).Len() == 0 {
-				m := getLogMessages(logs)
-				t.Errorf("Log lines diff %s", cmp.Diff(tc.log, m))
-			} else if tc.shdMakebuild {
-				if err == nil {
-					if len(client.Actions()) == 0 {
-						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 len(client.Actions()) != 0 {
+				t.Errorf("expected no actions to be created by the reconciler, got %v", client.Actions())
 			}
 		})
 	}
-}
 
+}
 func getController(d testData) (*controller.Impl, *observer.ObservedLogs, *fakebuildclientset.Clientset) {
 	pipelineClient := fakepipelineclientset.NewSimpleClientset()
 	buildClient := fakebuildclientset.NewSimpleClientset()