diff --git a/docs/using.md b/docs/using.md index f39c349bbfb..d596d04c4e3 100644 --- a/docs/using.md +++ b/docs/using.md @@ -37,6 +37,59 @@ need. expressed explicitly using this key since a task needing a resource from a another task would have to run after. - The name used in the `providedBy` is the name of `PipelineTask` +- The name of the `PipelineResource` must correspond to a `PipelineResource` from + the `Task` that the referenced `PipelineTask` provides as an output + +For example see this `Pipeline` spec: + +```yaml + - name: build-skaffold-app + taskRef: + name: build-push + params: + - name: pathToDockerFile + value: Dockerfile + - name: pathToContext + value: /workspace/examples/microservices/leeroy-app + - name: deploy-app + taskRef: + name: demo-deploy-kubectl + resources: + - name: image + providedBy: + - build-skaffold-app +``` + +The `image` resource is expected to be provided to the `deploy-app` `Task` from the +`build-skaffold-app` `Task`. This means that the `PipelineResource` bound to the `image` +input for `deploy-app` must be bound to the same `PipelineResource` as an output from +`build-skaffold-app`. + +This is the corresponding `PipelineRun` spec: + +```yaml + - name: build-skaffold-app + ... + outputs: + - name: builtImage + resourceRef: + name: skaffold-image-leeroy-app + - name: deploy-app + ... + - name: image + resourceRef: + name: skaffold-image-leeroy-app +``` + +You can see that the `builtImage` output from `build-skaffold-app` is bound to the +`skaffold-image-leeroy-app` `PipelineResource`, and the same `PipelineResource` is bound +to `image` for `deploy-app`. + +This controls two things: + +1. The order the `Tasks` are executed in: `deploy-app` must come after `build-skaffold-app` +2. The state of the `PipelineResources`: the image provided to `deploy-app` may be changed + by `build-skaffold-app` (WIP, see [#216](https://github.com/knative/build-pipeline/issues/216)) ## Creating a Task diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_types.go b/pkg/apis/pipeline/v1alpha1/pipeline_types.go index d4b3fb08cb6..cc5c0e89f31 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_types.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_types.go @@ -90,7 +90,7 @@ type PipelineTaskParam struct { type ResourceDependency struct { // Name is the name of the Task's input that this Resource should be used for. Name string `json:"name"` - // ProvidedBy is the list of Task names that the resource has to come from. + // ProvidedBy is the list of PipelineTask names that the resource has to come from. // +optional ProvidedBy []string `json:"providedBy,omitempty"` } diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go index 417ac52a55a..7a3ecdb8257 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go @@ -197,6 +197,19 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er } return fmt.Errorf("error getting Tasks for Pipeline %s: %s", p.Name, err) } + + if err := resources.ValidateProvidedBy(pipelineState); err != nil { + // This Run has failed, so we need to mark it as failed and stop reconciling it + pr.Status.SetCondition(&duckv1alpha1.Condition{ + Type: duckv1alpha1.ConditionSucceeded, + Status: corev1.ConditionFalse, + Reason: ReasonFailedValidation, + Message: fmt.Sprintf("Pipeline %s can't be Run; it invalid input/output linkages: %s", + fmt.Sprintf("%s/%s", p.Namespace, pr.Name), err), + }) + return nil + } + for _, rprt := range pipelineState { err := taskrun.ValidateResolvedTaskResources(rprt.PipelineTask.Params, rprt.ResolvedTaskResources) if err != nil { diff --git a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go index 3fbc5a6db89..8ded24b6855 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go @@ -234,3 +234,51 @@ func GetPipelineConditionStatus(prName string, state []*ResolvedPipelineRunTask, Message: "All Tasks have completed executing", } } + +func findReferencedTask(pb string, state []*ResolvedPipelineRunTask) *ResolvedPipelineRunTask { + for _, rprtRef := range state { + if rprtRef.PipelineTask.Name == pb { + return rprtRef + } + } + return nil +} + +// ValidateProvidedBy will look at any `providedBy` clauses in the resolved PipelineRun state +// and validate it: the `providedBy` must specify an input of the current `Task`. The `PipelineTask` +// it is provided by must actually exist in the `Pipeline`. The `PipelineResource` that is bound to the input +// must be the same `PipelineResource` that was bound to the output of the previous `Task`. If the state is +// not valid, it will return an error. +func ValidateProvidedBy(state []*ResolvedPipelineRunTask) error { + for _, rprt := range state { + for _, dep := range rprt.PipelineTask.ResourceDependencies { + inputBinding, ok := rprt.ResolvedTaskResources.Inputs[dep.Name] + if !ok { + return fmt.Errorf("PipelineTask %s is trying to declare dependency for Resource %s which is not an input of PipelineTask %q", rprt.PipelineTask.Name, dep.Name, rprt.PipelineTask.Name) + } + + for _, pb := range dep.ProvidedBy { + if pb == rprt.PipelineTask.Name { + return fmt.Errorf("PipelineTask %s is trying to depend on a PipelineResource from itself", pb) + } + depTask := findReferencedTask(pb, state) + if depTask == nil { + return fmt.Errorf("pipelineTask %s is trying to depend on previous Task %q but it does not exist", rprt.PipelineTask.Name, pb) + } + + sameBindingExists := false + for _, output := range depTask.ResolvedTaskResources.Outputs { + if output.Name == inputBinding.Name { + sameBindingExists = true + } + } + if !sameBindingExists { + return fmt.Errorf("providedBy is ambiguous: input %q for PipelineTask %q is bound to %q but no outputs in PipelineTask %q are bound to same resource", + dep.Name, rprt.PipelineTask.Name, inputBinding.Name, depTask.PipelineTask.Name) + } + } + } + } + + return nil +} diff --git a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go index c347d146495..b6aa32a025f 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go @@ -561,3 +561,283 @@ func TestGetPipelineConditionStatus(t *testing.T) { }) } } + +func TestValidateProvidedBy(t *testing.T) { + r := &v1alpha1.PipelineResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "holygrail", + }, + Spec: v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeImage, + }, + } + state := []*ResolvedPipelineRunTask{{ + PipelineTask: &v1alpha1.PipelineTask{ + Name: "quest", + }, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &v1alpha1.TaskSpec{ + Outputs: &v1alpha1.Outputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "sweet-artifact", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + }, + }, + Outputs: map[string]*v1alpha1.PipelineResource{ + "sweet-artifact": r, + }, + }, + }, { + PipelineTask: &v1alpha1.PipelineTask{ + Name: "winning", + ResourceDependencies: []v1alpha1.ResourceDependency{{ + Name: "awesome-thing", + ProvidedBy: []string{"quest"}, + }}}, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &v1alpha1.TaskSpec{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "awesome-thing", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + }, + }, + Inputs: map[string]*v1alpha1.PipelineResource{ + "awesome-thing": r, + }, + }, + }} + err := ValidateProvidedBy(state) + if err != nil { + t.Fatalf("Didn't expect error when validating valid providedBy clause but got: %v", err) + } +} + +func TestValidateProvidedBy_Invalid(t *testing.T) { + r := &v1alpha1.PipelineResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "holygrail", + }, + Spec: v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeImage, + }, + } + otherR := &v1alpha1.PipelineResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "holyhandgrenade", + }, + Spec: v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeImage, + }, + } + + for _, tc := range []struct { + name string + state []*ResolvedPipelineRunTask + }{{ + name: "providedBy tries to reference input", + state: []*ResolvedPipelineRunTask{{ + PipelineTask: &v1alpha1.PipelineTask{ + Name: "quest", + }, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &v1alpha1.TaskSpec{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "sweet-artifact", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + }, + }, + Inputs: map[string]*v1alpha1.PipelineResource{ + "sweet-artifact": r, + }, + }, + }, { + PipelineTask: &v1alpha1.PipelineTask{ + Name: "winning", + ResourceDependencies: []v1alpha1.ResourceDependency{{ + Name: "awesome-thing", + ProvidedBy: []string{"quest"}, + }}}, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &v1alpha1.TaskSpec{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "awesome-thing", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + }, + }, + Inputs: map[string]*v1alpha1.PipelineResource{ + "sweet-artifact": r, + }, + }, + }}, + }, { + name: "providedBy resource doesn't exist", + state: []*ResolvedPipelineRunTask{{ + PipelineTask: &v1alpha1.PipelineTask{ + Name: "quest", + }, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &v1alpha1.TaskSpec{}, + }, + }, { + PipelineTask: &v1alpha1.PipelineTask{ + Name: "winning", + ResourceDependencies: []v1alpha1.ResourceDependency{{ + Name: "awesome-thing", + ProvidedBy: []string{"quest"}, + }}}, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &v1alpha1.TaskSpec{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "awesome-thing", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + }, + }, + Inputs: map[string]*v1alpha1.PipelineResource{ + "sweet-artifact": r, + }, + }, + }}, + }, { + name: "providedBy task doesn't exist", + state: []*ResolvedPipelineRunTask{{ + PipelineTask: &v1alpha1.PipelineTask{ + Name: "winning", + ResourceDependencies: []v1alpha1.ResourceDependency{{ + Name: "awesome-thing", + ProvidedBy: []string{"quest"}, + }}}, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &v1alpha1.TaskSpec{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "awesome-thing", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + }, + }, + Inputs: map[string]*v1alpha1.PipelineResource{ + "sweet-artifact": r, + }, + }, + }}, + }, { + name: "providedBy task refers to itself", + state: []*ResolvedPipelineRunTask{{ + PipelineTask: &v1alpha1.PipelineTask{ + Name: "winning", + ResourceDependencies: []v1alpha1.ResourceDependency{{ + Name: "awesome-thing", + ProvidedBy: []string{"winning"}, + }}}, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &v1alpha1.TaskSpec{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "awesome-thing", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + }, + }, + Inputs: map[string]*v1alpha1.PipelineResource{ + "sweet-artifact": r, + }, + }, + }}, + }, { + name: "providedBy is bound to different resource", + state: []*ResolvedPipelineRunTask{{ + PipelineTask: &v1alpha1.PipelineTask{ + Name: "quest", + }, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &v1alpha1.TaskSpec{ + Outputs: &v1alpha1.Outputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "sweet-artifact", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + }, + }, + Outputs: map[string]*v1alpha1.PipelineResource{ + "sweet-artifact": r, + }, + }, + }, { + PipelineTask: &v1alpha1.PipelineTask{ + Name: "winning", + ResourceDependencies: []v1alpha1.ResourceDependency{{ + Name: "awesome-thing", + ProvidedBy: []string{"quest"}, + }}}, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &v1alpha1.TaskSpec{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "awesome-thing", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + }, + }, + Inputs: map[string]*v1alpha1.PipelineResource{ + "sweet-artifact": otherR, + }, + }, + }}, + }, { + name: "providedBy is on output", + state: []*ResolvedPipelineRunTask{{ + PipelineTask: &v1alpha1.PipelineTask{ + Name: "quest", + }, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &v1alpha1.TaskSpec{ + Outputs: &v1alpha1.Outputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "sweet-artifact", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + }, + }, + Outputs: map[string]*v1alpha1.PipelineResource{ + "sweet-artifact": r, + }, + }, + }, { + PipelineTask: &v1alpha1.PipelineTask{ + Name: "winning", + ResourceDependencies: []v1alpha1.ResourceDependency{{ + Name: "awesome-thing", + ProvidedBy: []string{"quest"}, + }}}, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &v1alpha1.TaskSpec{ + Outputs: &v1alpha1.Outputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "awesome-thing", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + }, + }, + Outputs: map[string]*v1alpha1.PipelineResource{ + "sweet-artifact": otherR, + }, + }, + }}, + }} { + t.Run(tc.name, func(t *testing.T) { + err := ValidateProvidedBy(tc.state) + if err == nil { + t.Fatalf("Expected error when validating invalid providedBy but got none") + } + }) + } +}