Validate PipelineRuns without reconciler

This way we can test the validation without needing an entire
reconciler, and we can also eventually call the same resource validation
logic used by TaskRuns (#213)

pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go

@@ -153,7 +153,12 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 }
 
 func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) error {
- p, serviceAccount, verr := validatePipelineRun(c, pr)
+ p, serviceAccount, verr := ValidatePipelineRun(
+ pr,
+ c.pipelineLister.Pipelines(pr.Namespace).Get,
+ c.taskLister.Tasks(pr.Namespace).Get,
+ c.resourceLister.PipelineResources(pr.Namespace).Get,
+ )
  if verr != nil {
  c.Logger.Error("Failed to validate pipelinerun %s with error %v", pr.Name, verr)
  pr.Status.SetCondition(&duckv1alpha1.Condition{

pkg/reconciler/v1alpha1/pipelinerun/validate.go

@@ -22,21 +22,30 @@ import (
  "github.com/knative/build-pipeline/pkg/apis/pipeline/v1alpha1"
 )
 
-// validate all references in pipelinerun exist at runtime
-func validatePipelineRun(c *Reconciler, pr *v1alpha1.PipelineRun) (*v1alpha1.Pipeline, string, error) {
+// GetPipeline is used to retrieve existing instance of Pipeline called name
+type GetPipeline func(name string) (*v1alpha1.Pipeline, error)
+
+// GetResource is a function used to retrieve PipelineResources.
+type GetResource func(string) (*v1alpha1.PipelineResource, error)
+
+// GetTask is a function that will retrieve the task called name.
+type GetTask func(name string) (*v1alpha1.Task, error)
+
+// ValidatePipelineRun validates all references in pipelinerun exist at runtime
+func ValidatePipelineRun(pr *v1alpha1.PipelineRun, getPipeline GetPipeline, getTask GetTask, getResource GetResource) (*v1alpha1.Pipeline, string, error) {
  sa := pr.Spec.ServiceAccount
  // verify pipeline reference and all corresponding tasks exist
- p, err := c.pipelineLister.Pipelines(pr.Namespace).Get(pr.Spec.PipelineRef.Name)
+ p, err := getPipeline(pr.Spec.PipelineRef.Name)
  if err != nil {
  return nil, sa, fmt.Errorf("error listing pipeline ref %s: %v", pr.Spec.PipelineRef.Name, err)
  }
- if err := validatePipelineTask(c, p.Spec.Tasks, pr.Spec.PipelineTaskResources, pr.Namespace); err != nil {
+ if err := validatePipelineTask(getTask, getResource, p.Spec.Tasks, pr.Spec.PipelineTaskResources, pr.Namespace); err != nil {
  return nil, sa, fmt.Errorf("error validating tasks in pipeline %s: %v", p.Name, err)
  }
  return p, sa, nil
 }
 
-func validatePipelineTaskAndTask(c *Reconciler, ptask v1alpha1.PipelineTask, task *v1alpha1.Task, inputs []v1alpha1.TaskResourceBinding, outputs []v1alpha1.TaskResourceBinding, ns string) error {
+func validatePipelineTaskAndTask(getResource GetResource, ptask v1alpha1.PipelineTask, task *v1alpha1.Task, inputs []v1alpha1.TaskResourceBinding, outputs []v1alpha1.TaskResourceBinding, ns string) error {
  // a map from the name of the input resource to the type
  inputMapping := map[string]v1alpha1.PipelineResourceType{}
  // a map from the name of the output resource to the type
@@ -52,7 +61,7 @@ func validatePipelineTaskAndTask(c *Reconciler, ptask v1alpha1.PipelineTask, tas
  inputMapping[r.Name] = ""
  // TODO(#213): if this is the empty string should it be an error? or maybe let the lookup fail?
  if r.ResourceRef.Name != "" {
- rr, err := c.resourceLister.PipelineResources(ns).Get(r.ResourceRef.Name)
+ rr, err := getResource(r.ResourceRef.Name)
  if err != nil {
  return fmt.Errorf("error listing input pipeline resource for task %s: %v ", ptask.Name, err)
  }
@@ -64,7 +73,7 @@ func validatePipelineTaskAndTask(c *Reconciler, ptask v1alpha1.PipelineTask, tas
  outputMapping[r.Name] = ""
  // TODO(#213): if this is the empty string should it be an error? or maybe let the lookup fail?
  if r.ResourceRef.Name != "" {
- rr, err := c.resourceLister.PipelineResources(ns).Get(r.ResourceRef.Name)
+ rr, err := getResource(r.ResourceRef.Name)
  if err != nil {
  return fmt.Errorf("error listing output pipeline resource for task %s: %v ", ptask.Name, err)
  }
@@ -104,17 +113,17 @@ func validatePipelineTaskAndTask(c *Reconciler, ptask v1alpha1.PipelineTask, tas
  return nil
 }
 
-func validatePipelineTask(c *Reconciler, tasks []v1alpha1.PipelineTask, resources []v1alpha1.PipelineTaskResource, ns string) error {
+func validatePipelineTask(getTask GetTask, getResource GetResource, tasks []v1alpha1.PipelineTask, resources []v1alpha1.PipelineTaskResource, ns string) error {
  for _, pt := range tasks {
- t, err := c.taskLister.Tasks(ns).Get(pt.TaskRef.Name)
+ t, err := getTask(pt.TaskRef.Name)
  if err != nil {
  return fmt.Errorf("couldn't get task %q: %s", pt.TaskRef.Name, err)
  }
 
  found := false
  for _, tr := range resources {
  if tr.Name == pt.Name {
- if err := validatePipelineTaskAndTask(c, pt, t, tr.Inputs, tr.Outputs, ns); err != nil {
+ if err := validatePipelineTaskAndTask(getResource, pt, t, tr.Inputs, tr.Outputs, ns); err != nil {
  return fmt.Errorf("pipelineTask %q is invalid: %s", pt.Name, err)
  }
  found = true

pkg/reconciler/v1alpha1/pipelinerun/validate_test.go

@@ -1,13 +1,11 @@
 package pipelinerun_test
 
 import (
- "context"
  "testing"
 
  "github.com/knative/build-pipeline/pkg/apis/pipeline/v1alpha1"
- "github.com/knative/build-pipeline/test"
- duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1"
- corev1 "k8s.io/api/core/v1"
+ "github.com/knative/build-pipeline/pkg/reconciler/v1alpha1/pipelinerun"
+ "k8s.io/apimachinery/pkg/api/errors"
  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -310,22 +308,33 @@ func Test_InvalidPipelineTask(t *testing.T) {
 
  for _, tc := range tcs {
  t.Run(tc.name, func(t *testing.T) {
- d := test.Data{
- PipelineRuns: prs,
- Pipelines: ps,
- Tasks: ts,
- PipelineResources: rr,
+ getPipeline := func(name string) (*v1alpha1.Pipeline, error) {
+ for _, p := range ps {
+ if p.Name == name {
+ return p, nil
+ }
+ }
+ return nil, errors.NewNotFound(v1alpha1.Resource("pipeline"), name)
  }
-
- c, _, _ := test.GetPipelineRunController(d)
- err := c.Reconciler.Reconcile(context.Background(), "foo/"+tc.pipelineRun.Name)
-
- if err != nil {
- t.Errorf("Did not expect to see error when reconciling invalid PipelineRun but saw %q", err)
+ getTask := func(name string) (*v1alpha1.Task, error) {
+ for _, t := range ts {
+ if t.Name == name {
+ return t, nil
+ }
+ }
+ return nil, errors.NewNotFound(v1alpha1.Resource("task"), name)
+ }
+ getResource := func(name string) (*v1alpha1.PipelineResource, error) {
+ for _, r := range rr {
+ if r.Name == name {
+ return r, nil
+ }
+ }
+ return nil, errors.NewNotFound(v1alpha1.Resource("pipelineResource"), name)
  }
- condition := tc.pipelineRun.Status.GetCondition(duckv1alpha1.ConditionSucceeded)
- if condition == nil || condition.Status != corev1.ConditionFalse {
- t.Errorf("Expected status to be failed on invalid PipelineRun %s but was: %v", tc.pipeline.Name, condition)
+ _, _, err := pipelinerun.ValidatePipelineRun(tc.pipelineRun, getPipeline, getTask, getResource)
+ if err == nil {
+ t.Errorf("Expected error from validating invalid PipelineRun but was none")
  }
  })
  }