Separate logic to follow references from TaskRun resource validation

pkg/reconciler/v1alpha1/pipelinerun/validate.go

@@ -91,6 +91,7 @@ func validatePipelineTaskAndTask(c *Reconciler, ptask v1alpha1.PipelineTask, tas
 			}
 		}
 		for _, inputResourceParam := range task.Spec.Inputs.Params {
+			// TODO(#213): should check if the param has default values here
 			if _, ok := paramsMapping[inputResourceParam.Name]; !ok {
 				return fmt.Errorf("input param %q not provided for pipeline task %q (task %q)", inputResourceParam.Name, ptask.Name, task.Name)
 			}

pkg/reconciler/v1alpha1/taskrun/resources/taskrunresolution.go

@@ -0,0 +1,72 @@
+/*
+Copyright 2018 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either extress or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package resources
+
+import (
+	"fmt"
+
+	"github.com/knative/build-pipeline/pkg/apis/pipeline/v1alpha1"
+)
+
+// ResolvedTaskRun contains all the data that is needed to execute
+// the TaskRun: the TaskRun, it's Task and the PipelineResources it needs.
+type ResolvedTaskRun struct {
+	Task *v1alpha1.Task
+	// Inputs is a map from the name of the input required by the Task
+	// to the actual Resource to use for it
+	Inputs map[string]*v1alpha1.PipelineResource
+	// Outputs is a map from the name of the output required by the Task
+	// to the actual Resource to use for it
+	Outputs map[string]*v1alpha1.PipelineResource
+}
+
+// GetResource is a function used to retrieve PipelineResources.
+type GetResource func(string) (*v1alpha1.PipelineResource, error)
+
+// GetTask is a function used to retrieve Tasks.
+type GetTask func(string) (*v1alpha1.Task, error)
+
+// ResolveTaskRun looks up CRDs referenced by the TaskRun and returns an instance
+// of TaskRunResource with all of the relevant data populated. If referenced CRDs can't
+// be found, an error is returned.
+func ResolveTaskRun(tr *v1alpha1.TaskRunSpec, getTask GetTask, gr GetResource) (*ResolvedTaskRun, error) {
+	var err error
+	rtr := ResolvedTaskRun{
+		Inputs:  map[string]*v1alpha1.PipelineResource{},
+		Outputs: map[string]*v1alpha1.PipelineResource{},
+	}
+
+	rtr.Task, err = getTask(tr.TaskRef.Name)
+	if err != nil {
+		return nil, fmt.Errorf("couldn't retrieve referenced Task %q: %s", tr.TaskRef.Name, err)
+	}
+	for _, r := range tr.Inputs.Resources {
+		rr, err := gr(r.ResourceRef.Name)
+		if err != nil {
+			return nil, fmt.Errorf("couldn't retreive referenced input PipelineResource %q: %s", r.ResourceRef.Name, err)
+		}
+		rtr.Inputs[r.Name] = rr
+	}
+	for _, r := range tr.Outputs.Resources {
+		rr, err := gr(r.ResourceRef.Name)
+		if err != nil {
+			return nil, fmt.Errorf("couldn't retreive referenced output PipelineResource %q: %s", r.ResourceRef.Name, err)
+		}
+		rtr.Outputs[r.Name] = rr
+	}
+	return &rtr, nil
+}

pkg/reconciler/v1alpha1/taskrun/resources/taskrunresolution_test.go

@@ -0,0 +1,234 @@
+/*
+Copyright 2018 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either extress or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package resources
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/knative/build-pipeline/pkg/apis/pipeline/v1alpha1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestResolveTaskRun(t *testing.T) {
+	tr := &v1alpha1.TaskRunSpec{
+		TaskRef: v1alpha1.TaskRef{
+			Name: "orchestrate",
+		},
+		Inputs: v1alpha1.TaskRunInputs{
+			Resources: []v1alpha1.TaskRunResource{{
+				Name: "repoToBuildFrom",
+				ResourceRef: v1alpha1.PipelineResourceRef{
+					Name: "git-repo",
+				},
+			}, {
+				Name: "clusterToUse",
+				ResourceRef: v1alpha1.PipelineResourceRef{
+					Name: "k8s-cluster",
+				},
+			}},
+		},
+		Outputs: v1alpha1.TaskRunOutputs{
+			Resources: []v1alpha1.TaskRunResource{{
+				Name: "imageToBuild",
+				ResourceRef: v1alpha1.PipelineResourceRef{
+					Name: "image",
+				},
+			}, {
+				Name: "gitRepoToUpdate",
+				ResourceRef: v1alpha1.PipelineResourceRef{
+					Name: "another-git-repo",
+				},
+			}},
+		},
+	}
+
+	task := &v1alpha1.Task{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "orchestrate",
+		},
+	}
+	gt := func(n string) (*v1alpha1.Task, error) { return task, nil }
+
+	resources := []*v1alpha1.PipelineResource{{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "git-repo",
+		},
+	}, {
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "k8s-cluster",
+		},
+	}, {
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "image",
+		},
+	}, {
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "another-git-repo",
+		},
+	}}
+	resourceIndex := 0
+	gr := func(n string) (*v1alpha1.PipelineResource, error) {
+		r := resources[resourceIndex]
+		resourceIndex++
+		return r, nil
+	}
+
+	rtr, err := ResolveTaskRun(tr, gt, gr)
+	if err != nil {
+		t.Fatalf("Did not expect error trying to resolve TaskRun: %s", err)
+	}
+
+	if rtr.Task == nil || rtr.Task.Name != "orchestrate" {
+		t.Errorf("Task not resolved, expected `orchestrate` Task but got: %v", rtr.Task)
+	}
+
+	if len(rtr.Inputs) == 2 {
+		r, ok := rtr.Inputs["repoToBuildFrom"]
+		if !ok {
+			t.Errorf("Expected value present in map for `repoToBuildFrom' but it was missing")
+		} else {
+			if r.Name != "git-repo" {
+				t.Errorf("Expected to use resource `git-repo` for `repoToBuildFrom` but used %s", r.Name)
+			}
+		}
+		r, ok = rtr.Inputs["clusterToUse"]
+		if !ok {
+			t.Errorf("Expected value present in map for `clusterToUse' but it was missing")
+		} else {
+			if r.Name != "k8s-cluster" {
+				t.Errorf("Expected to use resource `k8s-cluster` for `clusterToUse` but used %s", r.Name)
+			}
+		}
+	} else {
+		t.Errorf("Expected 2 resolved inputs but instead had: %v", rtr.Inputs)
+	}
+
+	if len(rtr.Outputs) == 2 {
+		r, ok := rtr.Outputs["imageToBuild"]
+		if !ok {
+			t.Errorf("Expected value present in map for `imageToBuild' but it was missing")
+		} else {
+			if r.Name != "image" {
+				t.Errorf("Expected to use resource `image` for `imageToBuild` but used %s", r.Name)
+			}
+		}
+		r, ok = rtr.Outputs["gitRepoToUpdate"]
+		if !ok {
+			t.Errorf("Expected value present in map for `gitRepoToUpdate' but it was missing")
+		} else {
+			if r.Name != "another-git-repo" {
+				t.Errorf("Expected to use resource `another-git-repo` for `gitRepoToUpdate` but used %s", r.Name)
+			}
+		}
+	} else {
+		t.Errorf("Expected 2 resolved outputs but instead had: %v", rtr.Outputs)
+	}
+}
+func TestResolveTaskRun_missingTask(t *testing.T) {
+	tr := &v1alpha1.TaskRunSpec{
+		TaskRef: v1alpha1.TaskRef{
+			Name: "orchestrate",
+		},
+	}
+
+	gt := func(n string) (*v1alpha1.Task, error) { return nil, fmt.Errorf("nope") }
+	gr := func(n string) (*v1alpha1.PipelineResource, error) { return &v1alpha1.PipelineResource{}, nil }
+
+	_, err := ResolveTaskRun(tr, gt, gr)
+	if err == nil {
+		t.Fatalf("Expected to get error because task couldn't be resolved")
+	}
+}
+func TestResolveTaskRun_missingOutput(t *testing.T) {
+	tr := &v1alpha1.TaskRunSpec{
+		TaskRef: v1alpha1.TaskRef{
+			Name: "orchestrate",
+		},
+		Outputs: v1alpha1.TaskRunOutputs{
+			Resources: []v1alpha1.TaskRunResource{{
+				Name: "repoToUpdate",
+				ResourceRef: v1alpha1.PipelineResourceRef{
+					Name: "another-git-repo",
+				},
+			},
+			},
+		}}
+
+	gt := func(n string) (*v1alpha1.Task, error) { return &v1alpha1.Task{}, nil }
+	gr := func(n string) (*v1alpha1.PipelineResource, error) { return nil, fmt.Errorf("nope") }
+
+	_, err := ResolveTaskRun(tr, gt, gr)
+	if err == nil {
+		t.Fatalf("Expected to get error because output resource couldn't be resolved")
+	}
+}
+
+func TestResolveTaskRun_missingInput(t *testing.T) {
+	tr := &v1alpha1.TaskRunSpec{
+		TaskRef: v1alpha1.TaskRef{
+			Name: "orchestrate",
+		},
+		Inputs: v1alpha1.TaskRunInputs{
+			Resources: []v1alpha1.TaskRunResource{{
+				Name: "repoToBuildFrom",
+				ResourceRef: v1alpha1.PipelineResourceRef{
+					Name: "git-repo",
+				},
+			},
+			},
+		}}
+
+	gt := func(n string) (*v1alpha1.Task, error) { return &v1alpha1.Task{}, nil }
+	gr := func(n string) (*v1alpha1.PipelineResource, error) { return nil, fmt.Errorf("nope") }
+
+	_, err := ResolveTaskRun(tr, gt, gr)
+	if err == nil {
+		t.Fatalf("Expected to get error because input resource couldn't be resolved")
+	}
+}
+
+func TestResolveTaskRun_noResources(t *testing.T) {
+	tr := &v1alpha1.TaskRunSpec{
+		TaskRef: v1alpha1.TaskRef{
+			Name: "orchestrate",
+		},
+	}
+	task := &v1alpha1.Task{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "orchestrate",
+		},
+	}
+	gt := func(n string) (*v1alpha1.Task, error) { return task, nil }
+	gr := func(n string) (*v1alpha1.PipelineResource, error) { return &v1alpha1.PipelineResource{}, nil }
+
+	rtr, err := ResolveTaskRun(tr, gt, gr)
+	if err != nil {
+		t.Fatalf("Did not expect error trying to resolve TaskRun: %s", err)
+	}
+
+	if rtr.Task == nil || rtr.Task.Name != "orchestrate" {
+		t.Errorf("Task not resolved, expected `orchestrate` Task but got: %v", rtr.Task)
+	}
+
+	if len(rtr.Inputs) != 0 {
+		t.Errorf("Did not expect any outputs to be resolved when none specified but had %v", rtr.Inputs)
+	}
+	if len(rtr.Outputs) != 0 {
+		t.Errorf("Did not expect any outputs to be resolved when none specified but had %v", rtr.Outputs)
+	}
+}

pkg/reconciler/v1alpha1/taskrun/taskrun.go

@@ -51,8 +51,12 @@ const (
 	// Task couldn't be found
 	ReasonCouldntGetTask = "CouldntGetTask"
 
+	// ReasonFailedResolution indicated that the reason for failure status is
+	// that references within the TaskRun could not be resolved
+	ReasonFailedResolution = "TaskRunResolutionFailed"
+
 	// ReasonFailedValidation indicated that the reason for failure status is
-	// that pipelinerun failed runtime validation
+	// that taskrun failed runtime validation
 	ReasonFailedValidation = "TaskRunValidationFailed"
 
 	// ReasonRunning indicates that the reason for the inprogress status is that the TaskRun
@@ -177,7 +181,18 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 }
 
 func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error {
-	if err := validateTaskRun(c, tr); err != nil {
+	rtr, err := resources.ResolveTaskRun(&tr.Spec, c.taskLister.Tasks(tr.Namespace).Get, c.resourceLister.PipelineResources(tr.Namespace).Get)
+	if err != nil {
+		c.Logger.Error("Failed to resolve references for taskrun %s with error %v", tr.Name, err)
+		tr.Status.SetCondition(&duckv1alpha1.Condition{
+			Type:    duckv1alpha1.ConditionSucceeded,
+			Status:  corev1.ConditionFalse,
+			Reason:  ReasonFailedResolution,
+			Message: err.Error(),
+		})
+		return nil
+	}
+	if err := ValidateTaskRunAndTask(tr.Spec.Inputs.Params, rtr); err != nil {
 		c.Logger.Error("Failed to validate taskrun %s with error %v", tr.Name, err)
 		tr.Status.SetCondition(&duckv1alpha1.Condition{
 			Type:    duckv1alpha1.ConditionSucceeded,

pkg/reconciler/v1alpha1/taskrun/taskrun_test.go

@@ -615,7 +615,7 @@ func TestReconcile_InvalidTaskRuns(t *testing.T) {
 		{
 			name:    "task run with no task",
 			taskRun: taskRuns[0],
-			reason:  taskrun.ReasonFailedValidation,
+			reason:  taskrun.ReasonFailedResolution,
 		},
 	}