From 37d5327fddb23238361835d7db61e887a5302d88 Mon Sep 17 00:00:00 2001 From: Christie Wilson Date: Tue, 24 Sep 2019 17:13:54 -0400 Subject: [PATCH 1/2] Resolve all PipelineResources first before continuing As part of #1184 I need to call `GetSetup` on all PipelineResources early on in PipelineRun execution. Since PipelineRuns declare all their resource up front, I wanted to be able to resolve all of them at once, then call `GetSetup` on all of them. Also, as Pipelines got more complex (we added Conditions) it turned out we were retrieving the resources in a few different places. Also in #1324 @pritidesai is making it so that these Resources can be provided by spec. By resolving all of this up front at once, we can simplify the logic later on. And you can see in this commit that we are able to reduce the responsibilities of ResolvePipelineRun a bit too! --- pkg/reconciler/pipelinerun/pipelinerun.go | 24 +- .../pipelinerun/pipelinerun_test.go | 4 +- .../resources/pipelinerunresolution.go | 164 ++++------- .../resources/pipelinerunresolution_test.go | 255 +++++++----------- .../taskrun/resources/input_resources.go | 21 -- .../resources/taskresourceresolution.go | 22 ++ .../resources/taskresourceresolution_test.go | 31 +++ 7 files changed, 218 insertions(+), 303 deletions(-) diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 31fe2ee17ed..e93b87dea05 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -242,8 +242,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er }) return nil } - providedResources, err := resources.GetResourcesFromBindings(p, pr) - if err != nil { + if err := resources.ValidateResourceBindings(p, pr); err != nil { // This Run has failed, so we need to mark it as failed and stop reconciling it pr.Status.SetCondition(&apis.Condition{ Type: apis.ConditionSucceeded, @@ -254,6 +253,18 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er }) return nil } + providedResources, err := resources.GetResourcesFromBindings(pr, c.resourceLister.PipelineResources(pr.Namespace).Get) + if err != nil { + // This Run has failed, so we need to mark it as failed and stop reconciling it + pr.Status.SetCondition(&apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionFalse, + Reason: ReasonCouldntGetResource, + Message: fmt.Sprintf("PipelineRun %s can't be Run; it tries to bind Resources that don't exist: %s", + fmt.Sprintf("%s/%s", p.Namespace, pr.Name), err), + }) + return nil + } // Ensure that the parameters from the PipelineRun are overriding Pipeline parameters with the same type. // Weird substitution issues can occur if this is not validated (ApplyParameters() does not verify type). @@ -284,7 +295,6 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er func(name string) (v1alpha1.TaskInterface, error) { return c.clusterTaskLister.Get(name) }, - c.resourceLister.PipelineResources(pr.Namespace).Get, func(name string) (*v1alpha1.Condition, error) { return c.conditionLister.Conditions(pr.Namespace).Get(name) }, @@ -301,14 +311,6 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er Message: fmt.Sprintf("Pipeline %s can't be Run; it contains Tasks that don't exist: %s", fmt.Sprintf("%s/%s", p.Namespace, p.Name), err), }) - case *resources.ResourceNotFoundError: - pr.Status.SetCondition(&apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionFalse, - Reason: ReasonCouldntGetResource, - Message: fmt.Sprintf("PipelineRun %s can't be Run; it tries to bind Resources that don't exist: %s", - fmt.Sprintf("%s/%s", p.Namespace, pr.Name), err), - }) case *resources.ConditionNotFoundError: pr.Status.SetCondition(&apis.Condition{ Type: apis.ConditionSucceeded, diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index ef7645d75e3..c6f7bfa43f7 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -225,8 +225,8 @@ func TestReconcile(t *testing.T) { ) // ignore IgnoreUnexported ignore both after and before steps fields - if d := cmp.Diff(actual, expectedTaskRun, cmpopts.SortSlices(func(x, y v1alpha1.TaskResourceBinding) bool { return x.Name < y.Name })); d != "" { - t.Errorf("expected to see TaskRun %v created. Diff %s", expectedTaskRun, d) + if d := cmp.Diff(expectedTaskRun, actual, cmpopts.SortSlices(func(x, y v1alpha1.TaskResourceBinding) bool { return x.Name < y.Name })); d != "" { + t.Errorf("expected to see TaskRun %v created. Diff (-want, +got): %s", expectedTaskRun, d) } // test taskrun is able to recreate correct pipeline-pvc-name if expectedTaskRun.GetPipelineRunPVCName() != "test-pipeline-run-success-pvc" { diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index eddfebb9a00..a181fe3eee2 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -24,7 +24,6 @@ import ( "golang.org/x/xerrors" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" @@ -97,7 +96,7 @@ func (t ResolvedPipelineRunTask) IsSuccessful() bool { return false } -// IsFailed returns true only if the taskrun itself has failed +// IsFailure returns true only if the taskrun itself has failed func (t ResolvedPipelineRunTask) IsFailure() bool { if t.TaskRun == nil { return false @@ -170,12 +169,24 @@ func (state PipelineRunState) SuccessfulPipelineTaskNames() []string { // GetTaskRun is a function that will retrieve the TaskRun name. type GetTaskRun func(name string) (*v1alpha1.TaskRun, error) -// GetResourcesFromBindings will validate that all PipelineResources declared in Pipeline p are bound in PipelineRun pr -// and if so, will return a map from the declared name of the PipelineResource (which is how the PipelineResource will -// be referred to in the PipelineRun) to the ResourceRef. -func GetResourcesFromBindings(p *v1alpha1.Pipeline, pr *v1alpha1.PipelineRun) (map[string]v1alpha1.PipelineResourceBinding, error) { - resources := map[string]v1alpha1.PipelineResourceBinding{} +// GetResourcesFromBindings will retreive all Resources bound in PipelineRun pr and return a map +// from the declared name of the PipelineResource (which is how the PipelineResource will +// be referred to in the PipelineRun) to the PipelineResource, obtained via getResource. +func GetResourcesFromBindings(pr *v1alpha1.PipelineRun, getResource resources.GetResource) (map[string]*v1alpha1.PipelineResource, error) { + resources := map[string]*v1alpha1.PipelineResource{} + for _, resource := range pr.Spec.Resources { + r, err := getResource(resource.ResourceRef.Name) + if err != nil { + return resources, xerrors.Errorf("Error following resource reference for %s: %w", resource.Name, err) + } + resources[resource.Name] = r + } + return resources, nil +} + +// ValidateResourceBindings validate that the PipelineResources declared in Pipeline p are bound in PipelineRun. +func ValidateResourceBindings(p *v1alpha1.Pipeline, pr *v1alpha1.PipelineRun) error { required := make([]string, 0, len(p.Spec.Resources)) for _, resource := range p.Spec.Resources { required = append(required, resource.Name) @@ -184,44 +195,10 @@ func GetResourcesFromBindings(p *v1alpha1.Pipeline, pr *v1alpha1.PipelineRun) (m for _, resource := range pr.Spec.Resources { provided = append(provided, resource.Name) } - err := list.IsSame(required, provided) - if err != nil { - return resources, xerrors.Errorf("PipelineRun bound resources didn't match Pipeline: %w", err) + if err := list.IsSame(required, provided); err != nil { + return xerrors.Errorf("PipelineRun bound resources didn't match Pipeline: %w", err) } - - for _, resource := range pr.Spec.Resources { - resources[resource.Name] = resource - } - return resources, nil -} - -func getPipelineRunTaskResources(pt v1alpha1.PipelineTask, providedResources map[string]v1alpha1.PipelineResourceBinding) ([]v1alpha1.TaskResourceBinding, []v1alpha1.TaskResourceBinding, error) { - inputs, outputs := []v1alpha1.TaskResourceBinding{}, []v1alpha1.TaskResourceBinding{} - if pt.Resources != nil { - for _, taskInput := range pt.Resources.Inputs { - resource, ok := providedResources[taskInput.Resource] - if !ok { - return inputs, outputs, xerrors.Errorf("pipelineTask tried to use input resource %s not present in declared resources", taskInput.Resource) - } - inputs = append(inputs, v1alpha1.TaskResourceBinding{ - Name: taskInput.Name, - ResourceRef: resource.ResourceRef, - ResourceSpec: resource.ResourceSpec, - }) - } - for _, taskOutput := range pt.Resources.Outputs { - resource, ok := providedResources[taskOutput.Resource] - if !ok { - return outputs, outputs, xerrors.Errorf("pipelineTask tried to use output resource %s not present in declared resources", taskOutput.Resource) - } - outputs = append(outputs, v1alpha1.TaskResourceBinding{ - Name: taskOutput.Name, - ResourceRef: resource.ResourceRef, - ResourceSpec: resource.ResourceSpec, - }) - } - } - return inputs, outputs, nil + return nil } // TaskNotFoundError indicates that the resolution failed because a referenced Task couldn't be retrieved @@ -234,15 +211,6 @@ func (e *TaskNotFoundError) Error() string { return fmt.Sprintf("Couldn't retrieve Task %q: %s", e.Name, e.Msg) } -// ResourceNotFoundError indicates that the resolution failed because a referenced PipelineResource couldn't be retrieved -type ResourceNotFoundError struct { - Msg string -} - -func (e *ResourceNotFoundError) Error() string { - return fmt.Sprintf("Couldn't retrieve PipelineResource: %s", e.Msg) -} - type ConditionNotFoundError struct { Name string Msg string @@ -255,17 +223,15 @@ func (e *ConditionNotFoundError) Error() string { // ResolvePipelineRun retrieves all Tasks instances which are reference by tasks, getting // instances from getTask. If it is unable to retrieve an instance of a referenced Task, it // will return an error, otherwise it returns a list of all of the Tasks retrieved. -// It will retrieve the Resources needed for the TaskRun as well using getResource and the mapping -// of providedResources. +// It will retrieve the Resources needed for the TaskRun using the mapping of providedResources. func ResolvePipelineRun( pipelineRun v1alpha1.PipelineRun, getTask resources.GetTask, getTaskRun resources.GetTaskRun, getClusterTask resources.GetClusterTask, - getResource resources.GetResource, getCondition GetCondition, tasks []v1alpha1.PipelineTask, - providedResources map[string]v1alpha1.PipelineResourceBinding, + providedResources map[string]*v1alpha1.PipelineResource, ) (PipelineRunState, error) { state := []*ResolvedPipelineRunTask{} @@ -292,17 +258,10 @@ func ResolvePipelineRun( } } - // Get all the resources that this task will be using, if any - inputs, outputs, err := getPipelineRunTaskResources(pt, providedResources) - if err != nil { - return nil, xerrors.Errorf("unexpected error which should have been caught by Pipeline webhook: %w", err) - } - spec := t.TaskSpec() - rtr, err := resources.ResolveTaskResources(&spec, t.TaskMetadata().Name, pt.TaskRef.Kind, inputs, outputs, getResource) - + rtr, err := ResolvePipelineTaskResources(pt, &spec, t.TaskMetadata().Name, pt.TaskRef.Kind, providedResources) if err != nil { - return nil, &ResourceNotFoundError{Msg: err.Error()} + return nil, xerrors.Errorf("couldn't match referenced resources with declared resources: %w", err) } rprt.ResolvedTaskResources = rtr @@ -319,7 +278,7 @@ func ResolvePipelineRun( // Get all conditions that this pipelineTask will be using, if any if len(pt.Conditions) > 0 { - rcc, err := resolveConditionChecks(&pt, pipelineRun.Status.TaskRuns, rprt.TaskRunName, getTaskRun, getCondition, getResource, providedResources) + rcc, err := resolveConditionChecks(&pt, pipelineRun.Status.TaskRuns, rprt.TaskRunName, getTaskRun, getCondition, providedResources) if err != nil { return nil, err } @@ -365,7 +324,6 @@ func GetPipelineConditionStatus(pr *v1alpha1.PipelineRun, state PipelineRunState // 2. Any one TaskRun has failed - >Failed. This should change with #1020 and #1023 // 3. All tasks are done or are skipped (i.e. condition check failed).-> Success // 4. A Task or Condition is running right now or there are things left to run -> Running - if pr.IsTimedOut() { return &apis.Condition{ Type: apis.ConditionSucceeded, @@ -494,10 +452,7 @@ func ValidateFrom(state PipelineRunState) error { return nil } -func resolveConditionChecks(pt *v1alpha1.PipelineTask, - taskRunStatus map[string]*v1alpha1.PipelineRunTaskRunStatus, - taskRunName string, getTaskRun resources.GetTaskRun, getCondition GetCondition, - getResource resources.GetResource, providedResources map[string]v1alpha1.PipelineResourceBinding) ([]*ResolvedConditionCheck, error) { +func resolveConditionChecks(pt *v1alpha1.PipelineTask, taskRunStatus map[string]*v1alpha1.PipelineRunTaskRunStatus, taskRunName string, getTaskRun resources.GetTaskRun, getCondition GetCondition, providedResources map[string]*v1alpha1.PipelineResource) ([]*ResolvedConditionCheck, error) { rccs := []*ResolvedConditionCheck{} for _, ptc := range pt.Conditions { cName := ptc.ConditionRef @@ -515,20 +470,21 @@ func resolveConditionChecks(pt *v1alpha1.PipelineTask, return nil, xerrors.Errorf("error retrieving ConditionCheck %s for taskRun name %s : %w", conditionCheckName, taskRunName, err) } } + conditionResources := map[string]*v1alpha1.PipelineResource{} + for _, declared := range ptc.Resources { + r, ok := providedResources[declared.Resource] + if !ok { + return nil, xerrors.Errorf("resources %s missing for condition %s in pipeline task %s", declared.Resource, cName, pt.Name) + } + conditionResources[declared.Name] = r + } rcc := ResolvedConditionCheck{ Condition: c, ConditionCheckName: conditionCheckName, ConditionCheck: v1alpha1.NewConditionCheck(cctr), PipelineTaskCondition: &ptc, - } - - if len(ptc.Resources) > 0 { - r, err := resolveConditionResources(ptc.Resources, getResource, providedResources) - if err != nil { - return nil, xerrors.Errorf("cloud not resolve resources for condition %s in pipeline task %s: %w", cName, pt.Name, err) - } - rcc.ResolvedResources = r + ResolvedResources: conditionResources, } rccs = append(rccs, &rcc) @@ -536,33 +492,31 @@ func resolveConditionChecks(pt *v1alpha1.PipelineTask, return rccs, nil } -func resolveConditionResources(prc []v1alpha1.PipelineConditionResource, - getResource resources.GetResource, - providedResources map[string]v1alpha1.PipelineResourceBinding, -) (map[string]*v1alpha1.PipelineResource, error) { - rr := make(map[string]*v1alpha1.PipelineResource) - for _, r := range prc { - // First get a ref to actual resource name from its bound name - resourceBinding, ok := providedResources[r.Resource] - if !ok { - return nil, xerrors.Errorf("resource %s not present in declared resources", r.Resource) - } - // Next, fetch the actual resource definition - if resourceBinding.ResourceRef.Name != "" { - gotResource, err := getResource(resourceBinding.ResourceRef.Name) - if err != nil { - return nil, xerrors.Errorf("could not retrieve resource %s: %w", r.Name, err) +// ResolvePipelineTaskResources matches PipelineResources referenced by pt inputs and outputs with the +// providedResources and returns an instance of ResolvedTaskResources. +func ResolvePipelineTaskResources(pt v1alpha1.PipelineTask, ts *v1alpha1.TaskSpec, taskName string, kind v1alpha1.TaskKind, providedResources map[string]*v1alpha1.PipelineResource) (*resources.ResolvedTaskResources, error) { + rtr := resources.ResolvedTaskResources{ + TaskName: taskName, + TaskSpec: ts, + Kind: kind, + Inputs: map[string]*v1alpha1.PipelineResource{}, + Outputs: map[string]*v1alpha1.PipelineResource{}, + } + if pt.Resources != nil { + for _, taskInput := range pt.Resources.Inputs { + resource, ok := providedResources[taskInput.Resource] + if !ok { + return nil, xerrors.Errorf("pipelineTask tried to use input resource %s not present in declared resources", taskInput.Resource) } - // Finally add it to the resolved resources map - rr[r.Name] = gotResource - } else if resourceBinding.ResourceSpec != nil && resourceBinding.ResourceSpec.Type != "" { - rr[r.Name] = &v1alpha1.PipelineResource{ - ObjectMeta: metav1.ObjectMeta{ - Name: resourceBinding.Name, - }, - Spec: *resourceBinding.ResourceSpec, + rtr.Inputs[taskInput.Name] = resource + } + for _, taskOutput := range pt.Resources.Outputs { + resource, ok := providedResources[taskOutput.Resource] + if !ok { + return nil, xerrors.Errorf("pipelineTask tried to use output resource %s not present in declared resources", taskOutput.Resource) } + rtr.Outputs[taskOutput.Name] = resource } } - return rr, nil + return &rtr, nil } diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 0bdfb11e24c..8532cddd434 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -17,6 +17,7 @@ limitations under the License. package resources import ( + "fmt" "strings" "testing" @@ -1031,52 +1032,36 @@ func TestGetPipelineConditionStatus(t *testing.T) { } func TestGetResourcesFromBindings(t *testing.T) { - p := tb.Pipeline("pipelines", "namespace", tb.PipelineSpec( - tb.PipelineDeclaredResource("git-resource", "git"), - )) pr := tb.PipelineRun("pipelinerun", "namespace", tb.PipelineRunSpec("pipeline", tb.PipelineRunResourceBinding("git-resource", tb.PipelineResourceBindingRef("sweet-resource")), )) - m, err := GetResourcesFromBindings(p, pr) + r := tb.PipelineResource("sweet-resource", "namespace") + getResource := func(name string) (*v1alpha1.PipelineResource, error) { + if name != "sweet-resource" { + return nil, fmt.Errorf("Request for unexpected resource %s", name) + } + return r, nil + } + m, err := GetResourcesFromBindings(pr, getResource) if err != nil { t.Fatalf("didn't expect error getting resources from bindings but got: %v", err) } - expectedResources := map[string]v1alpha1.PipelineResourceBinding{ - "git-resource": { - Name: "git-resource", - ResourceRef: v1alpha1.PipelineResourceRef{Name: "sweet-resource"}, - }, - } + expectedResources := map[string]*v1alpha1.PipelineResource{"git-resource": r} if d := cmp.Diff(expectedResources, m); d != "" { t.Fatalf("Expected resources didn't match actual -want, +got: %v", d) } } -func TestGetResourcesFromBindings_Missing(t *testing.T) { - p := tb.Pipeline("pipelines", "namespace", tb.PipelineSpec( - tb.PipelineDeclaredResource("git-resource", "git"), - tb.PipelineDeclaredResource("image-resource", "image"), - )) +func TestGetResourcesFromBindings_ErrorGettingResource(t *testing.T) { pr := tb.PipelineRun("pipelinerun", "namespace", tb.PipelineRunSpec("pipeline", tb.PipelineRunResourceBinding("git-resource", tb.PipelineResourceBindingRef("sweet-resource")), )) - _, err := GetResourcesFromBindings(p, pr) - if err == nil { - t.Fatalf("Expected error indicating `image-resource` was missing but got no error") + getResource := func(name string) (*v1alpha1.PipelineResource, error) { + return nil, xerrors.Errorf("IT HAS ALL GONE WRONG") } -} - -func TestGetResourcesFromBindings_Extra(t *testing.T) { - p := tb.Pipeline("pipelines", "namespace", tb.PipelineSpec( - tb.PipelineDeclaredResource("git-resource", "git"), - )) - pr := tb.PipelineRun("pipelinerun", "namespace", tb.PipelineRunSpec("pipeline", - tb.PipelineRunResourceBinding("git-resource", tb.PipelineResourceBindingRef("sweet-resource")), - tb.PipelineRunResourceBinding("image-resource", tb.PipelineResourceBindingRef("sweet-resource2")), - )) - _, err := GetResourcesFromBindings(p, pr) + _, err := GetResourcesFromBindings(pr, getResource) if err == nil { - t.Fatalf("Expected error indicating `image-resource` was extra but got no error") + t.Fatalf("Expected error indicating resource couldnt be retrieved but got no error") } } @@ -1095,14 +1080,6 @@ func TestResolvePipelineRun(t *testing.T) { tb.PipelineTaskOutputResource("output1", "git-resource"), ), )) - providedResources := map[string]v1alpha1.PipelineResourceBinding{ - "git-resource": { - Name: "someresource", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "someresource", - }, - }, - } r := &v1alpha1.PipelineResource{ ObjectMeta: metav1.ObjectMeta{ @@ -1112,6 +1089,7 @@ func TestResolvePipelineRun(t *testing.T) { Type: v1alpha1.PipelineResourceTypeGit, }, } + providedResources := map[string]*v1alpha1.PipelineResource{"git-resource": r} pr := v1alpha1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{ Name: "pipelinerun", @@ -1122,10 +1100,9 @@ func TestResolvePipelineRun(t *testing.T) { getTask := func(name string) (v1alpha1.TaskInterface, error) { return task, nil } getTaskRun := func(name string) (*v1alpha1.TaskRun, error) { return nil, nil } getClusterTask := func(name string) (v1alpha1.TaskInterface, error) { return nil, nil } - getResource := func(name string) (*v1alpha1.PipelineResource, error) { return r, nil } getCondition := func(name string) (*v1alpha1.Condition, error) { return nil, nil } - pipelineState, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getResource, getCondition, p.Spec.Tasks, providedResources) + pipelineState, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getCondition, p.Spec.Tasks, providedResources) if err != nil { t.Fatalf("Error getting tasks for fake pipeline %s: %s", p.ObjectMeta.Name, err) } @@ -1167,8 +1144,8 @@ func TestResolvePipelineRun(t *testing.T) { }, }} - if d := cmp.Diff(pipelineState, expectedState, cmpopts.IgnoreUnexported(v1alpha1.TaskRunSpec{})); d != "" { - t.Errorf("Expected to get current pipeline state %v, but actual differed: %s", expectedState, d) + if d := cmp.Diff(expectedState, pipelineState, cmpopts.IgnoreUnexported(v1alpha1.TaskRunSpec{})); d != "" { + t.Errorf("Expected to get current pipeline state %v, but actual differed (-want, +got): %s", expectedState, d) } } @@ -1183,21 +1160,18 @@ func TestResolvePipelineRun_PipelineTaskHasNoResources(t *testing.T) { Name: "mytask3", TaskRef: v1alpha1.TaskRef{Name: "task"}, }} - providedResources := map[string]v1alpha1.PipelineResourceBinding{} + providedResources := map[string]*v1alpha1.PipelineResource{} getTask := func(name string) (v1alpha1.TaskInterface, error) { return task, nil } getTaskRun := func(name string) (*v1alpha1.TaskRun, error) { return &trs[0], nil } getClusterTask := func(name string) (v1alpha1.TaskInterface, error) { return clustertask, nil } - getResource := func(name string) (*v1alpha1.PipelineResource, error) { - return nil, xerrors.New("should not get called") - } getCondition := func(name string) (*v1alpha1.Condition, error) { return nil, nil } pr := v1alpha1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{ Name: "pipelinerun", }, } - pipelineState, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getResource, getCondition, pts, providedResources) + pipelineState, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getCondition, pts, providedResources) if err != nil { t.Fatalf("Did not expect error when resolving PipelineRun without Resources: %v", err) } @@ -1223,7 +1197,7 @@ func TestResolvePipelineRun_TaskDoesntExist(t *testing.T) { Name: "mytask1", TaskRef: v1alpha1.TaskRef{Name: "task"}, }} - providedResources := map[string]v1alpha1.PipelineResourceBinding{} + providedResources := map[string]*v1alpha1.PipelineResource{} // Return an error when the Task is retrieved, as if it didn't exist getTask := func(name string) (v1alpha1.TaskInterface, error) { @@ -1236,9 +1210,6 @@ func TestResolvePipelineRun_TaskDoesntExist(t *testing.T) { getTaskRun := func(name string) (*v1alpha1.TaskRun, error) { return nil, errors.NewNotFound(v1alpha1.Resource("taskrun"), name) } - getResource := func(name string) (*v1alpha1.PipelineResource, error) { - return nil, xerrors.New("should not get called") - } getCondition := func(name string) (*v1alpha1.Condition, error) { return nil, nil } @@ -1247,7 +1218,7 @@ func TestResolvePipelineRun_TaskDoesntExist(t *testing.T) { Name: "pipelinerun", }, } - _, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getResource, getCondition, pts, providedResources) + _, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getCondition, pts, providedResources) switch err := err.(type) { case nil: t.Fatalf("Expected error getting non-existent Tasks for Pipeline %s but got none", p.Name) @@ -1277,14 +1248,11 @@ func TestResolvePipelineRun_ResourceBindingsDontExist(t *testing.T) { ), )), }} - providedResources := map[string]v1alpha1.PipelineResourceBinding{} + providedResources := map[string]*v1alpha1.PipelineResource{} getTask := func(name string) (v1alpha1.TaskInterface, error) { return task, nil } getTaskRun := func(name string) (*v1alpha1.TaskRun, error) { return &trs[0], nil } getClusterTask := func(name string) (v1alpha1.TaskInterface, error) { return clustertask, nil } - getResource := func(name string) (*v1alpha1.PipelineResource, error) { - return nil, xerrors.New("shouldnt be called") - } getCondition := func(name string) (*v1alpha1.Condition, error) { return nil, nil } @@ -1296,7 +1264,7 @@ func TestResolvePipelineRun_ResourceBindingsDontExist(t *testing.T) { Name: "pipelinerun", }, } - _, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getResource, getCondition, tt.p.Spec.Tasks, providedResources) + _, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getCondition, tt.p.Spec.Tasks, providedResources) if err == nil { t.Fatalf("Expected error when bindings are in incorrect state for Pipeline %s but got none", p.Name) } @@ -1304,61 +1272,6 @@ func TestResolvePipelineRun_ResourceBindingsDontExist(t *testing.T) { } } -func TestResolvePipelineRun_ResourcesDontExist(t *testing.T) { - tests := []struct { - name string - p *v1alpha1.Pipeline - }{{ - name: "input doesnt exist", - p: tb.Pipeline("pipelines", "namespace", tb.PipelineSpec( - tb.PipelineTask("mytask1", "task", - tb.PipelineTaskInputResource("input1", "git-resource"), - ), - )), - }, { - name: "output doesnt exist", - p: tb.Pipeline("pipelines", "namespace", tb.PipelineSpec( - tb.PipelineTask("mytask1", "task", - tb.PipelineTaskOutputResource("input1", "git-resource"), - ), - )), - }} - providedResources := map[string]v1alpha1.PipelineResourceBinding{ - "git-resource": { - Name: "doesnt-exist", - }, - } - - getTask := func(name string) (v1alpha1.TaskInterface, error) { return task, nil } - getTaskRun := func(name string) (*v1alpha1.TaskRun, error) { return &trs[0], nil } - getClusterTask := func(name string) (v1alpha1.TaskInterface, error) { return clustertask, nil } - getResource := func(name string) (*v1alpha1.PipelineResource, error) { - return nil, errors.NewNotFound(v1alpha1.Resource("pipelineresource"), name) - } - getCondition := func(name string) (*v1alpha1.Condition, error) { - return nil, nil - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - pr := v1alpha1.PipelineRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pipelinerun", - }, - } - _, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getResource, getCondition, tt.p.Spec.Tasks, providedResources) - switch err := err.(type) { - case nil: - t.Fatalf("Expected error getting non-existent Resources for Pipeline %s but got none", p.Name) - case *ResourceNotFoundError: - // expected error - default: - t.Fatalf("Expected specific error type returned by func for non-existent Resource for Pipeline %s but got %s", p.Name, err) - } - }) - } -} - func TestValidateFrom(t *testing.T) { r := tb.PipelineResource("holygrail", namespace, tb.PipelineResourceSpec(v1alpha1.PipelineResourceTypeImage)) state := []*ResolvedPipelineRunTask{{ @@ -1543,14 +1456,6 @@ func TestResolvePipelineRun_withExistingTaskRuns(t *testing.T) { tb.PipelineTaskInputResource("input1", "git-resource"), ), )) - providedResources := map[string]v1alpha1.PipelineResourceBinding{ - "git-resource": { - Name: "someresource", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "someresource", - }, - }, - } r := &v1alpha1.PipelineResource{ ObjectMeta: metav1.ObjectMeta{ @@ -1560,6 +1465,7 @@ func TestResolvePipelineRun_withExistingTaskRuns(t *testing.T) { Type: v1alpha1.PipelineResourceTypeGit, }, } + providedResources := map[string]*v1alpha1.PipelineResource{"git-resource": r} taskrunStatus := map[string]*v1alpha1.PipelineRunTaskRunStatus{} taskrunStatus["pipelinerun-mytask-with-a-really-long-name-to-trigger-tru-9l9zj"] = &v1alpha1.PipelineRunTaskRunStatus{ PipelineTaskName: "mytask-with-a-really-long-name-to-trigger-truncation", @@ -1580,9 +1486,8 @@ func TestResolvePipelineRun_withExistingTaskRuns(t *testing.T) { getTask := func(name string) (v1alpha1.TaskInterface, error) { return task, nil } getClusterTask := func(name string) (v1alpha1.TaskInterface, error) { return nil, nil } getTaskRun := func(name string) (*v1alpha1.TaskRun, error) { return nil, nil } - getResource := func(name string) (*v1alpha1.PipelineResource, error) { return r, nil } getCondition := func(name string) (*v1alpha1.Condition, error) { return nil, nil } - pipelineState, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getResource, getCondition, p.Spec.Tasks, providedResources) + pipelineState, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getCondition, p.Spec.Tasks, providedResources) if err != nil { t.Fatalf("Error getting tasks for fake pipeline %s: %s", p.ObjectMeta.Name, err) } @@ -1624,13 +1529,10 @@ func TestResolveConditionChecks(t *testing.T) { TaskRef: v1alpha1.TaskRef{Name: "task"}, Conditions: []v1alpha1.PipelineTaskCondition{ptc}, }} - providedResources := map[string]v1alpha1.PipelineResourceBinding{} + providedResources := map[string]*v1alpha1.PipelineResource{} getTask := func(name string) (v1alpha1.TaskInterface, error) { return task, nil } getClusterTask := func(name string) (v1alpha1.TaskInterface, error) { return nil, xerrors.New("should not get called") } - getResource := func(name string) (*v1alpha1.PipelineResource, error) { - return nil, xerrors.New("should not get called") - } getCondition := func(name string) (*v1alpha1.Condition, error) { return &condition, nil } pr := v1alpha1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{ @@ -1658,6 +1560,7 @@ func TestResolveConditionChecks(t *testing.T) { Condition: &condition, ConditionCheck: v1alpha1.NewConditionCheck(cc), PipelineTaskCondition: &ptc, + ResolvedResources: providedResources, }}, }, { @@ -1674,19 +1577,20 @@ func TestResolveConditionChecks(t *testing.T) { ConditionCheckName: "pipelinerun-mytask1-mssqb-always-true-78c5n", Condition: &condition, PipelineTaskCondition: &ptc, + ResolvedResources: providedResources, }}, }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - pipelineState, err := ResolvePipelineRun(pr, getTask, tc.getTaskRun, getClusterTask, getResource, getCondition, pts, providedResources) + pipelineState, err := ResolvePipelineRun(pr, getTask, tc.getTaskRun, getClusterTask, getCondition, pts, providedResources) if err != nil { t.Fatalf("Did not expect error when resolving PipelineRun without Conditions: %v", err) } - if d := cmp.Diff(pipelineState[0].ResolvedConditionChecks, tc.expectedConditionCheck, cmpopts.IgnoreUnexported(v1alpha1.TaskRunSpec{})); d != "" { - t.Fatalf("ConditionChecks did not resolve as expected for case %s : %s", tc.name, d) + if d := cmp.Diff(tc.expectedConditionCheck, pipelineState[0].ResolvedConditionChecks, cmpopts.IgnoreUnexported(v1alpha1.TaskRunSpec{})); d != "" { + t.Fatalf("ConditionChecks did not resolve as expected for case %s (-want, +got) : %s", tc.name, d) } }) } @@ -1704,7 +1608,7 @@ func TestResolveConditionChecks_ConditionDoesNotExist(t *testing.T) { ConditionRef: "does-not-exist", }}, }} - providedResources := map[string]v1alpha1.PipelineResourceBinding{} + providedResources := map[string]*v1alpha1.PipelineResource{} getTask := func(name string) (v1alpha1.TaskInterface, error) { return task, nil } getTaskRun := func(name string) (*v1alpha1.TaskRun, error) { @@ -1716,9 +1620,6 @@ func TestResolveConditionChecks_ConditionDoesNotExist(t *testing.T) { return nil, xerrors.Errorf("getTaskRun called with unexpected name %s", name) } getClusterTask := func(name string) (v1alpha1.TaskInterface, error) { return nil, xerrors.New("should not get called") } - getResource := func(name string) (*v1alpha1.PipelineResource, error) { - return nil, xerrors.New("should not get called") - } getCondition := func(name string) (*v1alpha1.Condition, error) { return nil, errors.NewNotFound(v1alpha1.Resource("condition"), name) } @@ -1728,7 +1629,7 @@ func TestResolveConditionChecks_ConditionDoesNotExist(t *testing.T) { }, } - _, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getResource, getCondition, pts, providedResources) + _, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getCondition, pts, providedResources) switch err := err.(type) { case nil: @@ -1762,7 +1663,7 @@ func TestResolveConditionCheck_UseExistingConditionCheckName(t *testing.T) { TaskRef: v1alpha1.TaskRef{Name: "task"}, Conditions: []v1alpha1.PipelineTaskCondition{ptc}, }} - providedResources := map[string]v1alpha1.PipelineResourceBinding{} + providedResources := map[string]*v1alpha1.PipelineResource{} getTask := func(name string) (v1alpha1.TaskInterface, error) { return task, nil } getTaskRun := func(name string) (*v1alpha1.TaskRun, error) { @@ -1774,9 +1675,6 @@ func TestResolveConditionCheck_UseExistingConditionCheckName(t *testing.T) { return nil, xerrors.Errorf("getTaskRun called with unexpected name %s", name) } getClusterTask := func(name string) (v1alpha1.TaskInterface, error) { return nil, xerrors.New("should not get called") } - getResource := func(name string) (*v1alpha1.PipelineResource, error) { - return nil, xerrors.New("should not get called") - } getCondition := func(name string) (*v1alpha1.Condition, error) { return &condition, nil } ccStatus := make(map[string]*v1alpha1.PipelineRunConditionCheckStatus) @@ -1798,7 +1696,7 @@ func TestResolveConditionCheck_UseExistingConditionCheckName(t *testing.T) { }, } - pipelineState, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getResource, getCondition, pts, providedResources) + pipelineState, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getCondition, pts, providedResources) if err != nil { t.Fatalf("Did not expect error when resolving PipelineRun without Conditions: %v", err) } @@ -1807,10 +1705,11 @@ func TestResolveConditionCheck_UseExistingConditionCheckName(t *testing.T) { Condition: &condition, ConditionCheck: v1alpha1.NewConditionCheck(cc), PipelineTaskCondition: &ptc, + ResolvedResources: providedResources, }} - if d := cmp.Diff(pipelineState[0].ResolvedConditionChecks, expectedConditionChecks, cmpopts.IgnoreUnexported(v1alpha1.TaskRunSpec{})); d != "" { - t.Fatalf("ConditionChecks did not resolve as expected : %s", d) + if d := cmp.Diff(expectedConditionChecks, pipelineState[0].ResolvedConditionChecks, cmpopts.IgnoreUnexported(v1alpha1.TaskRunSpec{})); d != "" { + t.Fatalf("ConditionChecks did not resolve as expected (-want, +got): %s", d) } } @@ -1841,12 +1740,6 @@ func TestResolvedConditionCheck_WithResources(t *testing.T) { getTask := func(name string) (v1alpha1.TaskInterface, error) { return task, nil } getTaskRun := func(name string) (*v1alpha1.TaskRun, error) { return nil, nil } getClusterTask := func(name string) (v1alpha1.TaskInterface, error) { return nil, xerrors.New("should not get called") } - getResource := func(name string) (*v1alpha1.PipelineResource, error) { - if name == "some-repo" { - return gitResource, nil - } - return nil, xerrors.Errorf("getResource called with unexpected name: %s", name) - } getCondition := func(name string) (*v1alpha1.Condition, error) { return condition, nil } @@ -1859,29 +1752,22 @@ func TestResolvedConditionCheck_WithResources(t *testing.T) { tcs := []struct { name string - providedResources map[string]v1alpha1.PipelineResourceBinding + providedResources map[string]*v1alpha1.PipelineResource wantErr bool expected map[string]*v1alpha1.PipelineResource }{{ - name: "resource exists", - providedResources: map[string]v1alpha1.PipelineResourceBinding{ - "blah": { - Name: "some-repo", - }, - }, - expected: map[string]*v1alpha1.PipelineResource{}, + name: "resource exists", + providedResources: map[string]*v1alpha1.PipelineResource{"blah": gitResource}, + expected: map[string]*v1alpha1.PipelineResource{"workspace": gitResource}, }, { - name: "undeclared resource", - providedResources: map[string]v1alpha1.PipelineResourceBinding{ - "foo": { - Name: "some-repo", - }}, - wantErr: true, + name: "resource does not exist", + providedResources: map[string]*v1alpha1.PipelineResource{}, + wantErr: true, }} for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - pipelineState, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getResource, getCondition, pts, tc.providedResources) + pipelineState, err := ResolvePipelineRun(pr, getTask, getTaskRun, getClusterTask, getCondition, pts, tc.providedResources) if tc.wantErr { if err == nil { @@ -1897,10 +1783,51 @@ func TestResolvedConditionCheck_WithResources(t *testing.T) { PipelineTaskCondition: &ptc, ResolvedResources: tc.expected, }} - if d := cmp.Diff(pipelineState[0].ResolvedConditionChecks, expectedConditionChecks, cmpopts.IgnoreUnexported(v1alpha1.TaskRunSpec{})); d != "" { - t.Fatalf("ConditionChecks did not resolve as expected : %s", d) + if d := cmp.Diff(expectedConditionChecks, pipelineState[0].ResolvedConditionChecks, cmpopts.IgnoreUnexported(v1alpha1.TaskRunSpec{})); d != "" { + t.Fatalf("ConditionChecks did not resolve as expected (-want, +got): %s", d) } } }) } } + +func TestValidateResourceBindings(t *testing.T) { + p := tb.Pipeline("pipelines", "namespace", tb.PipelineSpec( + tb.PipelineDeclaredResource("git-resource", "git"), + )) + pr := tb.PipelineRun("pipelinerun", "namespace", tb.PipelineRunSpec("pipeline", + tb.PipelineRunResourceBinding("git-resource", tb.PipelineResourceBindingRef("sweet-resource")), + )) + err := ValidateResourceBindings(p, pr) + if err != nil { + t.Fatalf("didn't expect error getting resources from bindings but got: %v", err) + } +} + +func TestValidateResourceBindings_Missing(t *testing.T) { + p := tb.Pipeline("pipelines", "namespace", tb.PipelineSpec( + tb.PipelineDeclaredResource("git-resource", "git"), + tb.PipelineDeclaredResource("image-resource", "image"), + )) + pr := tb.PipelineRun("pipelinerun", "namespace", tb.PipelineRunSpec("pipeline", + tb.PipelineRunResourceBinding("git-resource", tb.PipelineResourceBindingRef("sweet-resource")), + )) + err := ValidateResourceBindings(p, pr) + if err == nil { + t.Fatalf("Expected error indicating `image-resource` was missing but got no error") + } +} + +func TestGetResourcesFromBindings_Extra(t *testing.T) { + p := tb.Pipeline("pipelines", "namespace", tb.PipelineSpec( + tb.PipelineDeclaredResource("git-resource", "git"), + )) + pr := tb.PipelineRun("pipelinerun", "namespace", tb.PipelineRunSpec("pipeline", + tb.PipelineRunResourceBinding("git-resource", tb.PipelineResourceBindingRef("sweet-resource")), + tb.PipelineRunResourceBinding("image-resource", tb.PipelineResourceBindingRef("sweet-resource2")), + )) + err := ValidateResourceBindings(p, pr) + if err == nil { + t.Fatalf("Expected error indicating `image-resource` was extra but got no error") + } +} diff --git a/pkg/reconciler/taskrun/resources/input_resources.go b/pkg/reconciler/taskrun/resources/input_resources.go index 7223e0f1ab8..44a3005bca7 100644 --- a/pkg/reconciler/taskrun/resources/input_resources.go +++ b/pkg/reconciler/taskrun/resources/input_resources.go @@ -25,7 +25,6 @@ import ( "go.uber.org/zap" "golang.org/x/xerrors" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" ) @@ -122,26 +121,6 @@ func AddInputResource( return taskSpec, nil } -func getResource(r *v1alpha1.TaskResourceBinding, getter GetResource) (*v1alpha1.PipelineResource, error) { - // Check both resource ref or resource Spec are not present. Taskrun webhook should catch this in validation error. - if r.ResourceRef.Name != "" && r.ResourceSpec != nil { - return nil, xerrors.New("Both ResourseRef and ResourceSpec are defined. Expected only one") - } - - if r.ResourceRef.Name != "" { - return getter(r.ResourceRef.Name) - } - if r.ResourceSpec != nil { - return &v1alpha1.PipelineResource{ - ObjectMeta: metav1.ObjectMeta{ - Name: r.Name, - }, - Spec: *r.ResourceSpec, - }, nil - } - return nil, xerrors.New("Neither ResourseRef not ResourceSpec is defined") -} - func destinationPath(name, path string) string { if path == "" { return filepath.Join(workspaceDir, name) diff --git a/pkg/reconciler/taskrun/resources/taskresourceresolution.go b/pkg/reconciler/taskrun/resources/taskresourceresolution.go index 9273524f0e6..b877b95f10c 100644 --- a/pkg/reconciler/taskrun/resources/taskresourceresolution.go +++ b/pkg/reconciler/taskrun/resources/taskresourceresolution.go @@ -19,6 +19,7 @@ package resources import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "golang.org/x/xerrors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // ResolvedTaskResources contains all the data that is needed to execute @@ -70,3 +71,24 @@ func ResolveTaskResources(ts *v1alpha1.TaskSpec, taskName string, kind v1alpha1. } return &rtr, nil } + +// getResource will return an instance of a PipelineResource to use for r, either by getting it with getter or by +// instantiating it from the embedded spec. +func getResource(r *v1alpha1.TaskResourceBinding, getter GetResource) (*v1alpha1.PipelineResource, error) { + if r.ResourceRef.Name != "" && r.ResourceSpec != nil { + return nil, xerrors.New("Both ResourseRef and ResourceSpec are defined. Expected only one") + } + + if r.ResourceRef.Name != "" { + return getter(r.ResourceRef.Name) + } + if r.ResourceSpec != nil { + return &v1alpha1.PipelineResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: r.Name, + }, + Spec: *r.ResourceSpec, + }, nil + } + return nil, xerrors.New("Neither ResourseRef nor ResourceSpec is defined") +} diff --git a/pkg/reconciler/taskrun/resources/taskresourceresolution_test.go b/pkg/reconciler/taskrun/resources/taskresourceresolution_test.go index 075ee221784..4fe83fd9f26 100644 --- a/pkg/reconciler/taskrun/resources/taskresourceresolution_test.go +++ b/pkg/reconciler/taskrun/resources/taskresourceresolution_test.go @@ -220,3 +220,34 @@ func TestResolveTaskRun_noResources(t *testing.T) { t.Errorf("Did not expect any outputs to be resolved when none specified but had %v", rtr.Outputs) } } + +func TestResolveTaskRun_InvalidBothSpecified(t *testing.T) { + inputs := []v1alpha1.TaskResourceBinding{{ + Name: "repoToBuildFrom", + // Can't specify both ResourceRef and ResourceSpec + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "git-repo", + }, + ResourceSpec: &v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeGit, + }, + }} + gr := func(n string) (*v1alpha1.PipelineResource, error) { return &v1alpha1.PipelineResource{}, nil } + + _, err := ResolveTaskResources(&v1alpha1.TaskSpec{}, "orchestrate", v1alpha1.NamespacedTaskKind, inputs, []v1alpha1.TaskResourceBinding{}, gr) + if err == nil { + t.Fatalf("Expected to get error because both ref and spec were used") + } +} + +func TestResolveTaskRun_InvalidNeitherSpecified(t *testing.T) { + inputs := []v1alpha1.TaskResourceBinding{{ + Name: "repoToBuildFrom", + }} + gr := func(n string) (*v1alpha1.PipelineResource, error) { return &v1alpha1.PipelineResource{}, nil } + + _, err := ResolveTaskResources(&v1alpha1.TaskSpec{}, "orchestrate", v1alpha1.NamespacedTaskKind, inputs, []v1alpha1.TaskResourceBinding{}, gr) + if err == nil { + t.Fatalf("Expected to get error because neither spec or ref were used") + } +} From aa4fb0aeaf70bf343f6a21066afa6ad4473c251e Mon Sep 17 00:00:00 2001 From: Christie Wilson Date: Fri, 27 Sep 2019 16:23:50 -0400 Subject: [PATCH 2/2] Use the same logic to resolve spec vs ref in Pipelines + Tasks In #1324 we updated PipelineRuns to allow for embedding ResourceSpecs in PipelineRuns. This commit makes it so that the logic for resolving (i.e. deciding if PipelineResources are specified by Spec or Ref) is shared by PipelineRuns + TaskRuns. This is done by making it so that the "binding" uses the same type underneath. The only reason they can't be the exact same type is that TaskRuns additionally need the "path" attribute, which is actually only used for PVC copying, which will be removed in #1284, and then we should be able to remove paths entirely and the type can be the same. Also added some additional comments around the use of `SelfLink`, and made sure it was well covered in the reconciler test. --- pkg/apis/pipeline/v1alpha1/taskrun_types.go | 14 +- .../v1alpha1/taskrun_validation_test.go | 86 +++++--- .../v1alpha1/zz_generated.deepcopy.go | 7 +- .../pipelinerun/pipelinerun_test.go | 34 +++- .../resources/conditionresolution.go | 4 +- .../resources/conditionresolution_test.go | 4 +- .../resources/input_output_steps.go | 12 +- .../resources/input_output_steps_test.go | 128 +++++++----- .../resources/pipelinerunresolution.go | 11 +- .../resources/pipelinerunresolution_test.go | 28 ++- .../taskrun/resources/image_exporter.go | 2 +- .../taskrun/resources/image_exporter_test.go | 96 +++++---- .../taskrun/resources/input_resource_test.go | 154 +++++++++------ .../taskrun/resources/output_resource_test.go | 152 ++++++++------ pkg/reconciler/taskrun/resources/pod_test.go | 6 +- .../resources/taskresourceresolution.go | 9 +- .../resources/taskresourceresolution_test.go | 185 +++++++++++++++--- test/builder/task.go | 8 +- test/builder/task_test.go | 24 ++- test/cluster_resource_test.go | 23 +-- 20 files changed, 650 insertions(+), 337 deletions(-) diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_types.go b/pkg/apis/pipeline/v1alpha1/taskrun_types.go index 1273e72f6dd..0c5e542c874 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_types.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_types.go @@ -79,16 +79,12 @@ type TaskRunInputs struct { } // TaskResourceBinding points to the PipelineResource that -// will be used for the Task input or output called Name. The optional Path field -// corresponds to a path on disk at which the Resource can be found (used when providing -// the resource via mounted volume, overriding the default logic to fetch the Resource). +// will be used for the Task input or output called Name. type TaskResourceBinding struct { - Name string `json:"name"` - // no more than one of the ResourceRef and ResourceSpec may be specified. - // +optional - ResourceRef PipelineResourceRef `json:"resourceRef,omitempty"` - // +optional - ResourceSpec *PipelineResourceSpec `json:"resourceSpec,omitempty"` + PipelineResourceBinding + // Paths will probably be removed in #1284, and then PipelineResourceBinding can be used instead. + // The optional Path field corresponds to a path on disk at which the Resource can be found + // (used when providing the resource via mounted volume, overriding the default logic to fetch the Resource). // +optional Paths []string `json:"paths,omitempty"` } diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go b/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go index d86e0cdaef5..1dc63fba9b7 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go @@ -172,10 +172,12 @@ func TestInput_Validate(t *testing.T) { Value: *builder.ArrayOrString("value"), }}, Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "testresource", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "testresource", + }, + Name: "workspace", }, - Name: "workspace", }}, } if err := i.Validate(context.Background(), "spec.inputs"); err != nil { @@ -192,15 +194,19 @@ func TestInput_Invalidate(t *testing.T) { name: "duplicate task inputs", inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "testresource1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "testresource1", + }, + Name: "workspace", }, - Name: "workspace", }, { - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "testresource2", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "testresource2", + }, + Name: "workspace", }, - Name: "workspace", }}, }, wantErr: apis.ErrMultipleOneOf("spec.Inputs.Resources.Name"), @@ -208,10 +214,12 @@ func TestInput_Invalidate(t *testing.T) { name: "invalid task input params", inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "testresource", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "testresource", + }, + Name: "resource", }, - Name: "resource", }}, Params: []v1alpha1.Param{{ Name: "name", @@ -226,13 +234,15 @@ func TestInput_Invalidate(t *testing.T) { name: "duplicate resource ref and resource spec", inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "testresource", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "testresource", + }, + ResourceSpec: &v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeGit, + }, + Name: "resource-dup", }, - ResourceSpec: &v1alpha1.PipelineResourceSpec{ - Type: v1alpha1.PipelineResourceTypeGit, - }, - Name: "resource-dup", }}, }, wantErr: apis.ErrDisallowedFields("spec.Inputs.Resources.Name.ResourceRef", "spec.Inputs.Resources.Name.ResourceSpec"), @@ -240,10 +250,12 @@ func TestInput_Invalidate(t *testing.T) { name: "invalid resource spec", inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceSpec: &v1alpha1.PipelineResourceSpec{ - Type: "non-existent", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceSpec: &v1alpha1.PipelineResourceSpec{ + Type: "non-existent", + }, + Name: "resource-inv", }, - Name: "resource-inv", }}, }, wantErr: apis.ErrInvalidValue("spec.type", "non-existent"), @@ -251,7 +263,9 @@ func TestInput_Invalidate(t *testing.T) { name: "no resource ref and resource spec", inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "resource", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "resource", + }, }}, }, wantErr: apis.ErrMissingField("spec.Inputs.Resources.Name.ResourceRef", "spec.Inputs.Resources.Name.ResourceSpec"), @@ -269,10 +283,12 @@ func TestInput_Invalidate(t *testing.T) { func TestOutput_Validate(t *testing.T) { i := v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "testresource", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "testresource", + }, + Name: "someimage", }, - Name: "someimage", }}, } if err := i.Validate(context.Background(), "spec.outputs"); err != nil { @@ -288,15 +304,19 @@ func TestOutput_Invalidate(t *testing.T) { name: "duplicated task outputs", outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "testresource1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "testresource1", + }, + Name: "workspace", }, - Name: "workspace", }, { - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "testresource2", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "testresource2", + }, + Name: "workspace", }, - Name: "workspace", }}, }, wantErr: apis.ErrMultipleOneOf("spec.Outputs.Resources.Name"), @@ -304,7 +324,9 @@ func TestOutput_Invalidate(t *testing.T) { name: "no output resource with resource spec nor resource ref", outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "workspace", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "workspace", + }, }}, }, wantErr: apis.ErrMissingField("spec.Outputs.Resources.Name.ResourceSpec", "spec.Outputs.Resources.Name.ResourceRef"), diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index f179da54b5f..7aa1d1bc8f1 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -1602,12 +1602,7 @@ func (in *TaskResource) DeepCopy() *TaskResource { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TaskResourceBinding) DeepCopyInto(out *TaskResourceBinding) { *out = *in - out.ResourceRef = in.ResourceRef - if in.ResourceSpec != nil { - in, out := &in.ResourceSpec, &out.ResourceSpec - *out = new(PipelineResourceSpec) - (*in).DeepCopyInto(*out) - } + in.PipelineResourceBinding.DeepCopyInto(&out.PipelineResourceBinding) if in.Paths != nil { in, out := &in.Paths, &out.Paths *out = make([]string, len(*in)) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index c6f7bfa43f7..282e9794f0e 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -80,7 +80,15 @@ func TestReconcile(t *testing.T) { tb.PipelineRunSpec("test-pipeline", tb.PipelineRunServiceAccount("test-sa"), tb.PipelineRunResourceBinding("git-repo", tb.PipelineResourceBindingRef("some-repo")), - tb.PipelineRunResourceBinding("best-image", tb.PipelineResourceBindingRef("some-image")), + tb.PipelineRunResourceBinding("best-image", tb.PipelineResourceBindingResourceSpec( + &v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeImage, + Params: []v1alpha1.ResourceParam{{ + Name: "url", + Value: "gcr.io/sven", + }}, + }, + )), tb.PipelineRunParam("bar", "somethingmorefun"), ), ), @@ -160,11 +168,13 @@ func TestReconcile(t *testing.T) { v1alpha1.PipelineResourceTypeGit, tb.PipelineResourceSpecParam("url", "https://github.com/kristoff/reindeer"), )), - tb.PipelineResource("some-image", "foo", tb.PipelineResourceSpec( - v1alpha1.PipelineResourceTypeImage, - tb.PipelineResourceSpecParam("url", "gcr.io/sven"), - )), } + + // When PipelineResources are created in the cluster, Kubernetes will add a SelfLink. We + // are using this to differentiate between Resources that we are referencing by Spec or by Ref + // after we have resolved them. + rs[0].SelfLink = "some/link" + d := test.Data{ PipelineRuns: prs, Pipelines: ps, @@ -211,13 +221,21 @@ func TestReconcile(t *testing.T) { tb.TaskRunInputsParam("foo", "somethingfun"), tb.TaskRunInputsParam("bar", "somethingmorefun"), tb.TaskRunInputsParam("templatedparam", "$(inputs.workspace.revision)"), - tb.TaskRunInputsResource("workspace", tb.TaskResourceBindingResourceSpec(&rs[0].Spec)), + tb.TaskRunInputsResource("workspace", tb.TaskResourceBindingRef("some-repo")), ), tb.TaskRunOutputs( - tb.TaskRunOutputsResource("image-to-use", tb.TaskResourceBindingResourceSpec(&rs[1].Spec), + tb.TaskRunOutputsResource("image-to-use", tb.TaskResourceBindingResourceSpec( + &v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeImage, + Params: []v1alpha1.ResourceParam{{ + Name: "url", + Value: "gcr.io/sven", + }}, + }, + ), tb.TaskResourceBindingPaths("/pvc/unit-test-1/image-to-use"), ), - tb.TaskRunOutputsResource("workspace", tb.TaskResourceBindingResourceSpec(&rs[0].Spec), + tb.TaskRunOutputsResource("workspace", tb.TaskResourceBindingRef("some-repo"), tb.TaskResourceBindingPaths("/pvc/unit-test-1/workspace"), ), ), diff --git a/pkg/reconciler/pipelinerun/resources/conditionresolution.go b/pkg/reconciler/pipelinerun/resources/conditionresolution.go index 9a8691b1126..e0cc9774f47 100644 --- a/pkg/reconciler/pipelinerun/resources/conditionresolution.go +++ b/pkg/reconciler/pipelinerun/resources/conditionresolution.go @@ -175,7 +175,9 @@ func (rcc *ResolvedConditionCheck) ToTaskResourceBindings() []v1alpha1.TaskResou for name, r := range rcc.ResolvedResources { tr := v1alpha1.TaskResourceBinding{ - Name: name, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: name, + }, } if r.SelfLink != "" { tr.ResourceRef = v1alpha1.PipelineResourceRef{ diff --git a/pkg/reconciler/pipelinerun/resources/conditionresolution_test.go b/pkg/reconciler/pipelinerun/resources/conditionresolution_test.go index 83b3a43f30d..b1822ad4b0c 100644 --- a/pkg/reconciler/pipelinerun/resources/conditionresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/conditionresolution_test.go @@ -289,7 +289,9 @@ func TestResolvedConditionCheck_ToTaskResourceBindings(t *testing.T) { } expected := []v1alpha1.TaskResourceBinding{{ - Name: "git-resource", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "git-resource", + }, }} if d := cmp.Diff(expected, rcc.ToTaskResourceBindings()); d != "" { diff --git a/pkg/reconciler/pipelinerun/resources/input_output_steps.go b/pkg/reconciler/pipelinerun/resources/input_output_steps.go index f75d721b786..4c7a78856db 100644 --- a/pkg/reconciler/pipelinerun/resources/input_output_steps.go +++ b/pkg/reconciler/pipelinerun/resources/input_output_steps.go @@ -28,9 +28,13 @@ func GetOutputSteps(outputs map[string]*v1alpha1.PipelineResource, taskName, sto for name, outputResource := range outputs { taskOutputResource := v1alpha1.TaskResourceBinding{ - Name: name, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: name, + }, Paths: []string{filepath.Join(storageBasePath, taskName, name)}, } + // SelfLink is being checked there to determine if this PipelineResource is an instance that + // exists in the cluster (in which case Kubernetes will populate this field) or is specified by Spec if outputResource.SelfLink != "" { taskOutputResource.ResourceRef = v1alpha1.PipelineResourceRef{ Name: outputResource.Name, @@ -55,8 +59,12 @@ func GetInputSteps(inputs map[string]*v1alpha1.PipelineResource, pt *v1alpha1.Pi for name, inputResource := range inputs { taskInputResource := v1alpha1.TaskResourceBinding{ - Name: name, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: name, + }, } + // SelfLink is being checked there to determine if this PipelineResource is an instance that + // exists in the cluster (in which case Kubernetes will populate this field) or is specified by Spec if inputResource.SelfLink != "" { taskInputResource.ResourceRef = v1alpha1.PipelineResourceRef{ Name: inputResource.Name, diff --git a/pkg/reconciler/pipelinerun/resources/input_output_steps_test.go b/pkg/reconciler/pipelinerun/resources/input_output_steps_test.go index 8f398d12165..84c664d7cd5 100644 --- a/pkg/reconciler/pipelinerun/resources/input_output_steps_test.go +++ b/pkg/reconciler/pipelinerun/resources/input_output_steps_test.go @@ -60,9 +60,11 @@ func TestGetOutputSteps(t *testing.T) { name: "single output", outputs: map[string]*v1alpha1.PipelineResource{"test-output": r1}, expectedtaskOuputResources: []v1alpha1.TaskResourceBinding{{ - Name: "test-output", - ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, - Paths: []string{"/pvc/test-taskname/test-output"}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "test-output", + ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, + }, + Paths: []string{"/pvc/test-taskname/test-output"}, }}, pipelineTaskName: "test-taskname", }, { @@ -72,22 +74,28 @@ func TestGetOutputSteps(t *testing.T) { "test-output-2": r2, }, expectedtaskOuputResources: []v1alpha1.TaskResourceBinding{{ - Name: "test-output", - ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, - Paths: []string{"/pvc/test-multiple-outputs/test-output"}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "test-output", + ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, + }, + Paths: []string{"/pvc/test-multiple-outputs/test-output"}, }, { - Name: "test-output-2", - ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource2"}, - Paths: []string{"/pvc/test-multiple-outputs/test-output-2"}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "test-output-2", + ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource2"}, + }, + Paths: []string{"/pvc/test-multiple-outputs/test-output-2"}, }}, pipelineTaskName: "test-multiple-outputs", }, { name: "single output with resource spec", outputs: map[string]*v1alpha1.PipelineResource{"test-output": r3}, expectedtaskOuputResources: []v1alpha1.TaskResourceBinding{{ - Name: "test-output", - ResourceSpec: &r3.Spec, - Paths: []string{"/pvc/test-taskname/test-output"}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "test-output", + ResourceSpec: &r3.Spec, + }, + Paths: []string{"/pvc/test-taskname/test-output"}, }}, pipelineTaskName: "test-taskname", }, { @@ -97,13 +105,17 @@ func TestGetOutputSteps(t *testing.T) { "test-output-2": r3, }, expectedtaskOuputResources: []v1alpha1.TaskResourceBinding{{ - Name: "test-output-1", - ResourceSpec: &r3.Spec, - Paths: []string{"/pvc/test-multiple-outputs-with-resource-spec/test-output-1"}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "test-output-1", + ResourceSpec: &r3.Spec, + }, + Paths: []string{"/pvc/test-multiple-outputs-with-resource-spec/test-output-1"}, }, { - Name: "test-output-2", - ResourceSpec: &r3.Spec, - Paths: []string{"/pvc/test-multiple-outputs-with-resource-spec/test-output-2"}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "test-output-2", + ResourceSpec: &r3.Spec, + }, + Paths: []string{"/pvc/test-multiple-outputs-with-resource-spec/test-output-2"}, }}, pipelineTaskName: "test-multiple-outputs-with-resource-spec", }} @@ -153,16 +165,20 @@ func TestGetInputSteps(t *testing.T) { }, }, expectedtaskInputResources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, - Name: "test-input", - Paths: []string{"/pvc/prev-task-1/test-input"}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, + Name: "test-input", + }, + Paths: []string{"/pvc/prev-task-1/test-input"}, }}, }, { name: "task-with-no-input-constraint", inputs: map[string]*v1alpha1.PipelineResource{"test-input": r1}, expectedtaskInputResources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, - Name: "test-input", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, + Name: "test-input", + }, }}, pipelineTask: &v1alpha1.PipelineTask{ Name: "sample-test-task", @@ -179,9 +195,11 @@ func TestGetInputSteps(t *testing.T) { }, }, expectedtaskInputResources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, - Name: "test-input", - Paths: []string{"/pvc/prev-task-1/test-input", "/pvc/prev-task-2/test-input"}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, + Name: "test-input", + }, + Paths: []string{"/pvc/prev-task-1/test-input", "/pvc/prev-task-2/test-input"}, }}, }, { name: "task-with-a-constraint-with-resource-spec", @@ -195,16 +213,20 @@ func TestGetInputSteps(t *testing.T) { }, }, expectedtaskInputResources: []v1alpha1.TaskResourceBinding{{ - ResourceSpec: &r2.Spec, - Name: "test-input", - Paths: []string{"/pvc/prev-task-1/test-input"}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceSpec: &r2.Spec, + Name: "test-input", + }, + Paths: []string{"/pvc/prev-task-1/test-input"}, }}, }, { name: "task-with-no-input-constraint-but-with-resource-spec", inputs: map[string]*v1alpha1.PipelineResource{"test-input": r2}, expectedtaskInputResources: []v1alpha1.TaskResourceBinding{{ - ResourceSpec: &r2.Spec, - Name: "test-input", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceSpec: &r2.Spec, + Name: "test-input", + }, }}, pipelineTask: &v1alpha1.PipelineTask{ Name: "sample-test-task", @@ -221,9 +243,11 @@ func TestGetInputSteps(t *testing.T) { }, }, expectedtaskInputResources: []v1alpha1.TaskResourceBinding{{ - ResourceSpec: &r2.Spec, - Name: "test-input", - Paths: []string{"/pvc/prev-task-1/test-input", "/pvc/prev-task-2/test-input"}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceSpec: &r2.Spec, + Name: "test-input", + }, + Paths: []string{"/pvc/prev-task-1/test-input", "/pvc/prev-task-2/test-input"}, }}, }, } @@ -280,24 +304,34 @@ func TestWrapSteps(t *testing.T) { resources.WrapSteps(taskRunSpec, pt, inputs, outputs, pvcDir) expectedtaskInputResources := []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, - Name: "test-input", - Paths: []string{"/pvc/prev-task/test-input"}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, + Name: "test-input", + }, + Paths: []string{"/pvc/prev-task/test-input"}, }, { - ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, - Name: "test-input-2", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, + Name: "test-input-2", + }, }, { - ResourceSpec: &r2.Spec, - Name: "test-input-3", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceSpec: &r2.Spec, + Name: "test-input-3", + }, }} expectedtaskOuputResources := []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, - Name: "test-output", - Paths: []string{"/pvc/test-task/test-output"}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{Name: "resource1"}, + Name: "test-output", + }, + Paths: []string{"/pvc/test-task/test-output"}, }, { - ResourceSpec: &r2.Spec, - Name: "test-output-2", - Paths: []string{"/pvc/test-task/test-output-2"}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceSpec: &r2.Spec, + Name: "test-output-2", + }, + Paths: []string{"/pvc/test-task/test-output-2"}, }} sort.SliceStable(expectedtaskInputResources, func(i, j int) bool { return expectedtaskInputResources[i].Name < expectedtaskInputResources[j].Name }) diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index a181fe3eee2..d617e38c3a5 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -173,16 +173,15 @@ type GetTaskRun func(name string) (*v1alpha1.TaskRun, error) // from the declared name of the PipelineResource (which is how the PipelineResource will // be referred to in the PipelineRun) to the PipelineResource, obtained via getResource. func GetResourcesFromBindings(pr *v1alpha1.PipelineRun, getResource resources.GetResource) (map[string]*v1alpha1.PipelineResource, error) { - resources := map[string]*v1alpha1.PipelineResource{} - + rs := map[string]*v1alpha1.PipelineResource{} for _, resource := range pr.Spec.Resources { - r, err := getResource(resource.ResourceRef.Name) + r, err := resources.GetResourceFromBinding(&resource, getResource) if err != nil { - return resources, xerrors.Errorf("Error following resource reference for %s: %w", resource.Name, err) + return rs, xerrors.Errorf("Error following resource reference for %s: %w", resource.Name, err) } - resources[resource.Name] = r + rs[resource.Name] = r } - return resources, nil + return rs, nil } // ValidateResourceBindings validate that the PipelineResources declared in Pipeline p are bound in PipelineRun. diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 8532cddd434..5e78e7c6204 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -1034,6 +1034,15 @@ func TestGetPipelineConditionStatus(t *testing.T) { func TestGetResourcesFromBindings(t *testing.T) { pr := tb.PipelineRun("pipelinerun", "namespace", tb.PipelineRunSpec("pipeline", tb.PipelineRunResourceBinding("git-resource", tb.PipelineResourceBindingRef("sweet-resource")), + tb.PipelineRunResourceBinding("image-resource", tb.PipelineResourceBindingResourceSpec( + &v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeImage, + Params: []v1alpha1.ResourceParam{{ + Name: "url", + Value: "gcr.io/sven", + }}, + }), + ), )) r := tb.PipelineResource("sweet-resource", "namespace") getResource := func(name string) (*v1alpha1.PipelineResource, error) { @@ -1046,9 +1055,22 @@ func TestGetResourcesFromBindings(t *testing.T) { if err != nil { t.Fatalf("didn't expect error getting resources from bindings but got: %v", err) } - expectedResources := map[string]*v1alpha1.PipelineResource{"git-resource": r} - if d := cmp.Diff(expectedResources, m); d != "" { - t.Fatalf("Expected resources didn't match actual -want, +got: %v", d) + + r1, ok := m["git-resource"] + if !ok { + t.Errorf("Missing expected resource git-resource: %v", m) + } else if d := cmp.Diff(r, r1); d != "" { + t.Errorf("Expected resources didn't match actual -want, +got: %v", d) + } + + r2, ok := m["image-resource"] + if !ok { + t.Errorf("Missing expected resource image-resource: %v", m) + } else if r2.Spec.Type != v1alpha1.PipelineResourceTypeImage || + len(r2.Spec.Params) != 1 || + r2.Spec.Params[0].Name != "url" || + r2.Spec.Params[0].Value != "gcr.io/sven" { + t.Errorf("Did not get expected image resource, got %v", r2.Spec) } } diff --git a/pkg/reconciler/taskrun/resources/image_exporter.go b/pkg/reconciler/taskrun/resources/image_exporter.go index 0f7be8b8750..95dde6110fe 100644 --- a/pkg/reconciler/taskrun/resources/image_exporter.go +++ b/pkg/reconciler/taskrun/resources/image_exporter.go @@ -47,7 +47,7 @@ func AddOutputImageDigestExporter( return xerrors.Errorf("Failed to get bound resource: %w while adding output image digest exporter", err) } - resource, err := getResource(boundResource, gr) + resource, err := GetResourceFromBinding(&boundResource.PipelineResourceBinding, gr) if err != nil { return xerrors.Errorf("Failed to get output pipeline Resource for taskRun %q resource %v; error: %w while adding output image digest exporter", tr.Name, boundResource, err) } diff --git a/pkg/reconciler/taskrun/resources/image_exporter_test.go b/pkg/reconciler/taskrun/resources/image_exporter_test.go index 8fa354e04f6..c4e8e9be314 100644 --- a/pkg/reconciler/taskrun/resources/image_exporter_test.go +++ b/pkg/reconciler/taskrun/resources/image_exporter_test.go @@ -73,17 +73,21 @@ func TestAddOutputImageDigestExporter(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-image", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-image-1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-image", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image-1", + }, }, }}, }, Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-image", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-image-1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-image", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image-1", + }, }, }}, }, @@ -139,17 +143,21 @@ func TestAddOutputImageDigestExporter(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-image", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-image-1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-image", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image-1", + }, }, }}, }, Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-image", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-image-1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-image", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image-1", + }, }, }}, }, @@ -221,17 +229,21 @@ func TestUpdateTaskRunStatus_withValidJson(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-image", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-image-1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-image", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image-1", + }, }, }}, }, Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-image", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-image-1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-image", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image-1", + }, }, }}, }, @@ -276,17 +288,21 @@ func TestUpdateTaskRunStatus_withInvalidJson(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-image", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-image-1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-image", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image-1", + }, }, }}, }, Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-image", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-image-1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-image", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image-1", + }, }, }}, }, @@ -362,17 +378,21 @@ func TestTaskRunHasOutputImageResource(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-image", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-image-1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-image", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image-1", + }, }, }}, }, Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-image", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-image-1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-image", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image-1", + }, }, }}, }, @@ -411,17 +431,21 @@ func TestTaskRunHasOutputImageResource(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-git-1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-git-1", + }, }, }}, }, Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-git-1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-git-1", + }, }, }}, }, diff --git a/pkg/reconciler/taskrun/resources/input_resource_test.go b/pkg/reconciler/taskrun/resources/input_resource_test.go index 0e329263abb..090e14bcc70 100644 --- a/pkg/reconciler/taskrun/resources/input_resource_test.go +++ b/pkg/reconciler/taskrun/resources/input_resource_test.go @@ -246,10 +246,12 @@ func TestAddResourceToTask(t *testing.T) { }, Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "the-git", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "the-git", + }, + Name: "gitspace", }, - Name: "gitspace", }}, }, }, @@ -290,10 +292,12 @@ func TestAddResourceToTask(t *testing.T) { }, Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "the-git-with-branch", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "the-git-with-branch", + }, + Name: "gitspace", }, - Name: "gitspace", }}, }, }, @@ -323,15 +327,19 @@ func TestAddResourceToTask(t *testing.T) { }, Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "the-git-with-branch", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "the-git-with-branch", + }, + Name: "gitspace", }, - Name: "gitspace", }, { - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "the-git-with-branch", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "the-git-with-branch", + }, + Name: "git-duplicate-space", }, - Name: "git-duplicate-space", }}, }, }, @@ -367,10 +375,12 @@ func TestAddResourceToTask(t *testing.T) { }, Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "the-git", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "the-git", + }, + Name: "gitspace", }, - Name: "gitspace", }}, }, }, @@ -400,10 +410,12 @@ func TestAddResourceToTask(t *testing.T) { }, Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "the-git-with-branch", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "the-git-with-branch", + }, + Name: "gitspace", }, - Name: "gitspace", }}, }, }, @@ -434,10 +446,12 @@ func TestAddResourceToTask(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "the-git", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "the-git", + }, + Name: "gitspace", }, - Name: "gitspace", Paths: []string{"prev-task-path"}, }}, }, @@ -476,10 +490,12 @@ func TestAddResourceToTask(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "storage1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "storage1", + }, + Name: "workspace", }, - Name: "workspace", }}, }, }, @@ -514,10 +530,12 @@ func TestAddResourceToTask(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "storage1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "storage1", + }, + Name: "workspace", }, - Name: "workspace", Paths: []string{"prev-task-path"}, }}, }, @@ -556,10 +574,12 @@ func TestAddResourceToTask(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "storage-gcs-invalid", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "storage-gcs-invalid", + }, + Name: "workspace", }, - Name: "workspace", }}, }, }, @@ -576,10 +596,12 @@ func TestAddResourceToTask(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "storage-gcs-invalid", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "storage-gcs-invalid", + }, + Name: "workspace", }, - Name: "workspace", }}, }, }, @@ -626,9 +648,11 @@ func TestAddResourceToTask(t *testing.T) { }, Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "target-cluster", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "cluster3", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "target-cluster", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "cluster3", + }, }, }}, }, @@ -668,9 +692,11 @@ func TestAddResourceToTask(t *testing.T) { }, Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "target-cluster", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "cluster2", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "target-cluster", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "cluster2", + }, }, }}, }, @@ -753,7 +779,9 @@ func TestStorageInputResource(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "gcs-input-resource", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "gcs-input-resource", + }, }}, }, }, @@ -774,16 +802,18 @@ func TestStorageInputResource(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "gcs-input-resource", - ResourceSpec: &v1alpha1.PipelineResourceSpec{ - Type: v1alpha1.PipelineResourceTypeStorage, - Params: []v1alpha1.ResourceParam{{ - Name: "Location", - Value: "gs://fake-bucket/rules.zip", - }, { - Name: "Type", - Value: "gcs", - }}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "gcs-input-resource", + ResourceSpec: &v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeStorage, + Params: []v1alpha1.ResourceParam{{ + Name: "Location", + Value: "gs://fake-bucket/rules.zip", + }, { + Name: "Type", + Value: "gcs", + }}, + }, }, }}, }, @@ -839,9 +869,11 @@ func TestStorageInputResource(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "gcs-input-resource", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "storage-gcs-keys", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "gcs-input-resource", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "storage-gcs-keys", + }, }, }}, }, @@ -931,10 +963,12 @@ func TestAddStepsToTaskWithBucketFromConfigMap(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "the-git", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "the-git", + }, + Name: "gitspace", }, - Name: "gitspace", Paths: []string{"prev-task-path"}, }}, }, @@ -969,10 +1003,12 @@ func TestAddStepsToTaskWithBucketFromConfigMap(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "storage1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "storage1", + }, + Name: "workspace", }, - Name: "workspace", Paths: []string{"prev-task-path"}, }}, }, diff --git a/pkg/reconciler/taskrun/resources/output_resource_test.go b/pkg/reconciler/taskrun/resources/output_resource_test.go index e9925795f77..c393bfe0b3b 100644 --- a/pkg/reconciler/taskrun/resources/output_resource_test.go +++ b/pkg/reconciler/taskrun/resources/output_resource_test.go @@ -121,17 +121,21 @@ func TestValidOutputResources(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-git", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-git", + }, }, }}, }, Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-git", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-git", + }, }, Paths: []string{"pipeline-task-name"}, }}, @@ -199,9 +203,11 @@ func TestValidOutputResources(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-git", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-git", + }, }, Paths: []string{"pipeline-task-name"}, }}, @@ -262,9 +268,11 @@ func TestValidOutputResources(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-image", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image", + }, }, Paths: []string{"pipeline-task-name"}, }}, @@ -304,9 +312,11 @@ func TestValidOutputResources(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-git", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-git", + }, }, }}, }, @@ -348,17 +358,21 @@ func TestValidOutputResources(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-gcs", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-gcs", + }, }, }}, }, Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-gcs", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-gcs", + }, }, Paths: []string{"pipeline-task-path"}, }}, @@ -445,9 +459,11 @@ func TestValidOutputResources(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-gcs", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-gcs", + }, }, Paths: []string{"pipeline-task-path"}, }}, @@ -520,9 +536,11 @@ func TestValidOutputResources(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-gcs", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-gcs", + }, }, Paths: []string{"pipeline-task-path"}, }}, @@ -581,9 +599,11 @@ func TestValidOutputResources(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-gcs", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-gcs", + }, }, }}, }, @@ -645,9 +665,11 @@ func TestValidOutputResources(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-image", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image", + }, }, }}, }, @@ -689,9 +711,11 @@ func TestValidOutputResources(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-image", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image", + }, }, }}, }, @@ -729,9 +753,11 @@ func TestValidOutputResources(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-image", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image", + }, }, }}, }, @@ -816,17 +842,21 @@ func TestValidOutputResourcesWithBucketStorage(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-git", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-git", + }, }, }}, }, Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-git", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-git", + }, }, Paths: []string{"pipeline-task-name"}, }}, @@ -881,9 +911,11 @@ func TestValidOutputResourcesWithBucketStorage(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-git", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-git", + }, }, Paths: []string{"pipeline-task-name"}, }}, @@ -927,9 +959,11 @@ func TestValidOutputResourcesWithBucketStorage(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-git", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-git", + }, }, }}, }, @@ -1039,9 +1073,11 @@ func TestInvalidOutputResources(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "source-gcs", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-gcs", + }, }, Paths: []string{"test-path"}, }}, @@ -1087,9 +1123,11 @@ func TestInvalidOutputResources(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "source-workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "invalid-source-storage", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "invalid-source-storage", + }, }, }}, }, diff --git a/pkg/reconciler/taskrun/resources/pod_test.go b/pkg/reconciler/taskrun/resources/pod_test.go index d927e593ebe..c002ed84c84 100644 --- a/pkg/reconciler/taskrun/resources/pod_test.go +++ b/pkg/reconciler/taskrun/resources/pod_test.go @@ -551,9 +551,11 @@ func TestInitOutputResourcesDefaultDir(t *testing.T) { trs := v1alpha1.TaskRunSpec{ Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "outputimage", - ResourceRef: v1alpha1.PipelineResourceRef{ + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ Name: "outputimage", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "outputimage", + }, }, }}, }, diff --git a/pkg/reconciler/taskrun/resources/taskresourceresolution.go b/pkg/reconciler/taskrun/resources/taskresourceresolution.go index b877b95f10c..0084d11d6f9 100644 --- a/pkg/reconciler/taskrun/resources/taskresourceresolution.go +++ b/pkg/reconciler/taskrun/resources/taskresourceresolution.go @@ -52,7 +52,7 @@ func ResolveTaskResources(ts *v1alpha1.TaskSpec, taskName string, kind v1alpha1. } for _, r := range inputs { - rr, err := getResource(&r, gr) + rr, err := GetResourceFromBinding(&r.PipelineResourceBinding, gr) if err != nil { return nil, xerrors.Errorf("couldn't retrieve referenced input PipelineResource %q: %w", r.ResourceRef.Name, err) } @@ -61,7 +61,7 @@ func ResolveTaskResources(ts *v1alpha1.TaskSpec, taskName string, kind v1alpha1. } for _, r := range outputs { - rr, err := getResource(&r, gr) + rr, err := GetResourceFromBinding(&r.PipelineResourceBinding, gr) if err != nil { return nil, xerrors.Errorf("couldn't retrieve referenced output PipelineResource %q: %w", r.ResourceRef.Name, err) @@ -72,13 +72,12 @@ func ResolveTaskResources(ts *v1alpha1.TaskSpec, taskName string, kind v1alpha1. return &rtr, nil } -// getResource will return an instance of a PipelineResource to use for r, either by getting it with getter or by +// GetResourceFromBinding will return an instance of a PipelineResource to use for r, either by getting it with getter or by // instantiating it from the embedded spec. -func getResource(r *v1alpha1.TaskResourceBinding, getter GetResource) (*v1alpha1.PipelineResource, error) { +func GetResourceFromBinding(r *v1alpha1.PipelineResourceBinding, getter GetResource) (*v1alpha1.PipelineResource, error) { if r.ResourceRef.Name != "" && r.ResourceSpec != nil { return nil, xerrors.New("Both ResourseRef and ResourceSpec are defined. Expected only one") } - if r.ResourceRef.Name != "" { return getter(r.ResourceRef.Name) } diff --git a/pkg/reconciler/taskrun/resources/taskresourceresolution_test.go b/pkg/reconciler/taskrun/resources/taskresourceresolution_test.go index 4fe83fd9f26..da92ba8cac1 100644 --- a/pkg/reconciler/taskrun/resources/taskresourceresolution_test.go +++ b/pkg/reconciler/taskrun/resources/taskresourceresolution_test.go @@ -17,6 +17,7 @@ limitations under the License. package resources import ( + "fmt" "testing" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" @@ -27,36 +28,48 @@ import ( func TestResolveTaskRun(t *testing.T) { inputs := []v1alpha1.TaskResourceBinding{{ - Name: "repoToBuildFrom", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "git-repo", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "repoToBuildFrom", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "git-repo", + }, }, }, { - Name: "clusterToUse", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "k8s-cluster", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "clusterToUse", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "k8s-cluster", + }, }, }, { - Name: "clusterspecToUse", - ResourceSpec: &v1alpha1.PipelineResourceSpec{ - Type: v1alpha1.PipelineResourceTypeCluster, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "clusterspecToUse", + ResourceSpec: &v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeCluster, + }, }, }} outputs := []v1alpha1.TaskResourceBinding{{ - Name: "imageToBuild", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "image", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "imageToBuild", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "image", + }, }, }, { - Name: "gitRepoToUpdate", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "another-git-repo", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "gitRepoToUpdate", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "another-git-repo", + }, }, }, { - Name: "gitspecToUse", - ResourceSpec: &v1alpha1.PipelineResourceSpec{ - Type: v1alpha1.PipelineResourceTypeGit, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "gitspecToUse", + ResourceSpec: &v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeGit, + }, }, }} @@ -166,9 +179,11 @@ func TestResolveTaskRun(t *testing.T) { func TestResolveTaskRun_missingOutput(t *testing.T) { outputs := []v1alpha1.TaskResourceBinding{{ - Name: "repoToUpdate", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "another-git-repo", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "repoToUpdate", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "another-git-repo", + }, }}} gr := func(n string) (*v1alpha1.PipelineResource, error) { return nil, xerrors.New("nope") } @@ -180,9 +195,11 @@ func TestResolveTaskRun_missingOutput(t *testing.T) { func TestResolveTaskRun_missingInput(t *testing.T) { inputs := []v1alpha1.TaskResourceBinding{{ - Name: "repoToBuildFrom", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "git-repo", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "repoToBuildFrom", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "git-repo", + }, }}} gr := func(n string) (*v1alpha1.PipelineResource, error) { return nil, xerrors.New("nope") } @@ -223,13 +240,15 @@ func TestResolveTaskRun_noResources(t *testing.T) { func TestResolveTaskRun_InvalidBothSpecified(t *testing.T) { inputs := []v1alpha1.TaskResourceBinding{{ - Name: "repoToBuildFrom", - // Can't specify both ResourceRef and ResourceSpec - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "git-repo", - }, - ResourceSpec: &v1alpha1.PipelineResourceSpec{ - Type: v1alpha1.PipelineResourceTypeGit, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "repoToBuildFrom", + // Can't specify both ResourceRef and ResourceSpec + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "git-repo", + }, + ResourceSpec: &v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeGit, + }, }, }} gr := func(n string) (*v1alpha1.PipelineResource, error) { return &v1alpha1.PipelineResource{}, nil } @@ -242,7 +261,9 @@ func TestResolveTaskRun_InvalidBothSpecified(t *testing.T) { func TestResolveTaskRun_InvalidNeitherSpecified(t *testing.T) { inputs := []v1alpha1.TaskResourceBinding{{ - Name: "repoToBuildFrom", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "repoToBuildFrom", + }, }} gr := func(n string) (*v1alpha1.PipelineResource, error) { return &v1alpha1.PipelineResource{}, nil } @@ -251,3 +272,103 @@ func TestResolveTaskRun_InvalidNeitherSpecified(t *testing.T) { t.Fatalf("Expected to get error because neither spec or ref were used") } } + +func TestGetResourceFromBinding_Ref(t *testing.T) { + r := &v1alpha1.PipelineResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "git-repo", + }, + } + binding := &v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "foo-resource", + }, + } + gr := func(n string) (*v1alpha1.PipelineResource, error) { + return r, nil + } + + rr, err := GetResourceFromBinding(binding, gr) + if err != nil { + t.Fatalf("Did not expect error trying to get resource from binding: %s", err) + } + if rr != r { + t.Errorf("Didn't get expected resource, got %v", rr) + } +} + +func TestGetResourceFromBinding_Spec(t *testing.T) { + binding := &v1alpha1.PipelineResourceBinding{ + ResourceSpec: &v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeGit, + Params: []v1alpha1.ResourceParam{{ + Name: "url", + Value: "github.com/mycoolorg/mycoolrepo", + }}, + }, + } + gr := func(n string) (*v1alpha1.PipelineResource, error) { + return nil, fmt.Errorf("shouldnt be called! but was for %s", n) + } + + rr, err := GetResourceFromBinding(binding, gr) + if err != nil { + t.Fatalf("Did not expect error trying to get resource from binding: %s", err) + } + if rr.Spec.Type != v1alpha1.PipelineResourceTypeGit { + t.Errorf("Got %s instead of expected resource type", rr.Spec.Type) + } + if len(rr.Spec.Params) != 1 || rr.Spec.Params[0].Name != "url" || rr.Spec.Params[0].Value != "github.com/mycoolorg/mycoolrepo" { + t.Errorf("Got unexpected params %v", rr.Spec.Params) + } +} + +func TestGetResourceFromBinding_NoNameOrSpec(t *testing.T) { + binding := &v1alpha1.PipelineResourceBinding{} + gr := func(n string) (*v1alpha1.PipelineResource, error) { + return nil, nil + } + + _, err := GetResourceFromBinding(binding, gr) + if err == nil { + t.Fatalf("Expected error when no name or spec but got none") + } +} + +func TestGetResourceFromBinding_NameAndSpec(t *testing.T) { + binding := &v1alpha1.PipelineResourceBinding{ + ResourceSpec: &v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeGit, + Params: []v1alpha1.ResourceParam{{ + Name: "url", + Value: "github.com/mycoolorg/mycoolrepo", + }}, + }, + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "foo-resource", + }, + } + gr := func(n string) (*v1alpha1.PipelineResource, error) { + return nil, nil + } + + _, err := GetResourceFromBinding(binding, gr) + if err == nil { + t.Fatalf("Expected error when no name or spec but got none") + } +} + +func TestGetResourceFromBinding_ErrorGettingResource(t *testing.T) { + binding := &v1alpha1.PipelineResourceBinding{ + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "foo-resource", + }, + } + gr := func(n string) (*v1alpha1.PipelineResource, error) { + return nil, fmt.Errorf("it has all gone wrong") + } + _, err := GetResourceFromBinding(binding, gr) + if err == nil { + t.Fatalf("Expected error when error retrieving resource but got none") + } +} diff --git a/test/builder/task.go b/test/builder/task.go index 566aca5dcc5..daca7cbff9f 100644 --- a/test/builder/task.go +++ b/test/builder/task.go @@ -542,7 +542,9 @@ func TaskRunInputsParam(name, value string, additionalValues ...string) TaskRunI func TaskRunInputsResource(name string, ops ...TaskResourceBindingOp) TaskRunInputsOp { return func(i *v1alpha1.TaskRunInputs) { binding := &v1alpha1.TaskResourceBinding{ - Name: name, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: name, + }, } for _, op := range ops { op(binding) @@ -596,7 +598,9 @@ func TaskRunOutputs(ops ...TaskRunOutputsOp) TaskRunSpecOp { func TaskRunOutputsResource(name string, ops ...TaskResourceBindingOp) TaskRunOutputsOp { return func(i *v1alpha1.TaskRunOutputs) { binding := &v1alpha1.TaskResourceBinding{ - Name: name, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: name, + }, } for _, op := range ops { op(binding) diff --git a/test/builder/task_test.go b/test/builder/task_test.go index f032159fe0b..0f7836ce9c4 100644 --- a/test/builder/task_test.go +++ b/test/builder/task_test.go @@ -188,16 +188,20 @@ func TestTaskRunWithTaskRef(t *testing.T) { Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "git-resource", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "my-git", - APIVersion: "a1", + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "git-resource", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "my-git", + APIVersion: "a1", + }, }, Paths: []string{"source-folder"}, }, { - Name: "another-git-resource", - ResourceSpec: &v1alpha1.PipelineResourceSpec{Type: v1alpha1.PipelineResourceType("cluster")}, - Paths: []string{"source-folder"}, + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ + Name: "another-git-resource", + ResourceSpec: &v1alpha1.PipelineResourceSpec{Type: v1alpha1.PipelineResourceType("cluster")}, + }, + Paths: []string{"source-folder"}, }}, Params: []v1alpha1.Param{{ Name: "iparam", @@ -209,9 +213,11 @@ func TestTaskRunWithTaskRef(t *testing.T) { }, Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ - Name: "git-resource", - ResourceRef: v1alpha1.PipelineResourceRef{ + PipelineResourceBinding: v1alpha1.PipelineResourceBinding{ Name: "git-resource", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "git-resource", + }, }, Paths: []string{"output-folder"}, }}, diff --git a/test/cluster_resource_test.go b/test/cluster_resource_test.go index 2991d998973..416cbca877e 100644 --- a/test/cluster_resource_test.go +++ b/test/cluster_resource_test.go @@ -121,25 +121,10 @@ func getClusterResourceTask(namespace, name, resName, configName string) *v1alph } func getClusterResourceTaskRun(namespace, name, taskName, resName string) *v1alpha1.TaskRun { - return &v1alpha1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: namespace, - Name: name, - }, - Spec: v1alpha1.TaskRunSpec{ - TaskRef: &v1alpha1.TaskRef{ - Name: taskName, - }, - Inputs: v1alpha1.TaskRunInputs{ - Resources: []v1alpha1.TaskResourceBinding{{ - Name: "target-cluster", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: resName, - }, - }}, - }, - }, - } + return tb.TaskRun(name, namespace, tb.TaskRunSpec( + tb.TaskRunTaskRef(taskName), + tb.TaskRunInputs(tb.TaskRunInputsResource("target-cluster", tb.TaskResourceBindingRef(resName))), + )) } func getClusterConfigMap(namespace, name string) *corev1.ConfigMap {