Implement Task/TaskRun Input Params

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,42 @@ import (
 	ktesting "k8s.io/client-go/testing"
 )
 
-func TestReconcile(t *testing.T) {
-	taskname := "test-task"
+var simpleTask = &v1alpha1.Task{
+	ObjectMeta: metav1.ObjectMeta{
+		Name:      "test-task",
+		Namespace: "foo",
+	},
+	Spec: v1alpha1.TaskSpec{
+		BuildSpec: &buildv1alpha1.BuildSpec{
+			Steps: []corev1.Container{
+				{
+					Name:  "simple-step",
+					Image: "foo",
+				},
+			},
+		},
+	},
+}
+
+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 +80,134 @@ 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{simpleTask, templatedTask},
+	}
+	testcases := []struct {
+		name            string
+		taskRun         string
+		wantedBuildSpec buildv1alpha1.BuildSpec
+	}{
+		{
+			name:    "success",
+			taskRun: "foo/test-taskrun-run-success",
+			wantedBuildSpec: buildv1alpha1.BuildSpec{
+				Steps: []corev1.Container{
+					{
+						Name:  "simple-step",
+						Image: "foo",
+					},
 				},
-				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{
+		simpleTask,
 	}
+
 	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()