From d63abc6bd3be441fe93b6b4edf6b80125b42330b Mon Sep 17 00:00:00 2001 From: Priti Desai Date: Mon, 20 Jan 2020 15:14:12 -0800 Subject: [PATCH] Enabling Pipeline Resources to be marked as Optional Pipeline inputs and outputs are considered required, there is no way today to mark them optional. This change introduces a new field called optional as part of the PipelineDeclaredResource similar to previous PR #1601, by default optional is set to false and a resource is required. To mark any resource optional, set optional to true: apiVersion: tekton.dev/v1alpha1 kind: Pipeline metadata: name: pipeline-build-image spec: resources: - name: workspace type: git optional: true tasks: - name: check-workspace Closes #1710 --- docs/resources.md | 23 +++- .../pipelineruns/demo-optional-resources.yaml | 128 ++++++++++++++++++ pkg/apis/pipeline/v1alpha2/pipeline_types.go | 4 + .../resources/pipelinerunresolution.go | 62 +++++++-- .../resources/pipelinerunresolution_test.go | 87 ++++++++++++ 5 files changed, 287 insertions(+), 17 deletions(-) create mode 100644 examples/pipelineruns/demo-optional-resources.yaml diff --git a/docs/resources.md b/docs/resources.md index 69136117598..300a3cdcf23 100644 --- a/docs/resources.md +++ b/docs/resources.md @@ -229,7 +229,7 @@ resourcesResult: ### Optional Resources -By default, a resource is declared as mandatory unless `optional` is set `true` +By default, a resource is declared as mandatory unless `optional` is set to `true` for that resource. Resources declared as `optional` in a `Task` does not have be specified in `TaskRun`. @@ -246,12 +246,31 @@ spec: optional: true ``` +Similarly, resources declared as `optional` in a `Pipeline` does not have to be +specified in `PipelineRun`. + +```yaml +apiVersion: tekton.dev/v1alpha1 +kind: Pipeline +metadata: + name: pipeline-build-image +spec: + resources: + - name: workspace + type: git + optional: true + tasks: + - name: check-workspace +... +``` + You can refer to different examples demonstrating usage of optional resources in -`Task` and `Condition`: +`Task`, `Condition`, and `Pipeline`: - [Task](../examples/taskruns/optional-resources.yaml) - [Cluster Task](../examples/taskruns/optional-resources-with-clustertask.yaml) - [Condition](../examples/pipelineruns/conditional-pipelinerun-with-optional-resources.yaml) +- [Pipeline](../examples/pipelineruns/demo-optional-resources.yaml) ## Resource Types diff --git a/examples/pipelineruns/demo-optional-resources.yaml b/examples/pipelineruns/demo-optional-resources.yaml new file mode 100644 index 00000000000..e584453a58f --- /dev/null +++ b/examples/pipelineruns/demo-optional-resources.yaml @@ -0,0 +1,128 @@ +apiVersion: tekton.dev/v1alpha1 +kind: Condition +metadata: + name: check-git-pipeline-resource +spec: + params: + - name: "path" + resources: + - name: git-repo + type: git + optional: true + check: + image: alpine + script: 'test -f $(resources.git-repo.path)/$(params.path)' +--- + +apiVersion: tekton.dev/v1alpha1 +kind: Condition +metadata: + name: check-image-pipeline-resource +spec: + resources: + - name: built-image + type: image + optional: true + check: + image: alpine + script: 'test ! -z $(resources.built-image.url)' +--- + +apiVersion: tekton.dev/v1alpha1 +kind: Task +metadata: + name: build-an-image +spec: + inputs: + resources: + - name: git-repo + type: git + optional: true + params: + - name: DOCKERFILE + description: The path to the dockerfile to build from GitHub Repo + default: "Dockerfile" + outputs: + resources: + - name: built-image + type: image + optional: true + steps: + - name: build-an-image + image: "gcr.io/kaniko-project/executor:latest" + command: + - /kaniko/executor + args: + - --dockerfile=$(inputs.params.DOCKERFILE) + - --destination=$(outputs.resources.built-image.url) +--- + +apiVersion: tekton.dev/v1alpha1 +kind: Pipeline +metadata: + name: demo-pipeline-to-build-an-image +spec: + resources: + - name: source-repo + type: git + optional: true + - name: web-image + type: image + optional: true + params: + - name: "path" + default: "README.md" + tasks: + - name: build-an-image + taskRef: + name: build-an-image + conditions: + - conditionRef: "check-git-pipeline-resource" + params: + - name: "path" + value: "$(params.path)" + resources: + - name: git-repo + resource: source-repo + - conditionRef: "check-image-pipeline-resource" + resources: + - name: built-image + resource: web-image + resources: + inputs: + - name: git-repo + resource: source-repo + outputs: + - name: built-image + resource: web-image + +--- + +apiVersion: tekton.dev/v1alpha1 +kind: PipelineRun +metadata: + name: demo-pipeline-to-build-an-image-without-resources +spec: + pipelineRef: + name: demo-pipeline-to-build-an-image + serviceAccountName: 'default' +--- + +apiVersion: tekton.dev/v1alpha1 +kind: PipelineRun +metadata: + name: demo-pipeline-to-build-an-image-without-image-resource +spec: + pipelineRef: + name: demo-pipeline-to-build-an-image + serviceAccountName: 'default' + resources: + - name: source-repo + resourceSpec: + type: git + params: + - name: revision + value: master + - name: url + value: https://github.com/tektoncd/pipeline +--- diff --git a/pkg/apis/pipeline/v1alpha2/pipeline_types.go b/pkg/apis/pipeline/v1alpha2/pipeline_types.go index ae862be94fd..4fb0109f0a6 100644 --- a/pkg/apis/pipeline/v1alpha2/pipeline_types.go +++ b/pkg/apis/pipeline/v1alpha2/pipeline_types.go @@ -175,6 +175,10 @@ type PipelineDeclaredResource struct { Name string `json:"name"` // Type is the type of the PipelineResource. Type PipelineResourceType `json:"type"` + // Optional declares the resource as optional. + // optional: true - the resource is considered optional + // optional: false - the resource is considered required (default/equivalent of not specifying it) + Optional bool `json:"optional,omitempty"` } // PipelineTaskResources allows a Pipeline to declare how its DeclaredPipelineResources diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 128813c32a3..79b25160f1a 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -184,15 +184,29 @@ func GetResourcesFromBindings(pr *v1alpha1.PipelineRun, getResource resources.Ge // ValidateResourceBindings validate that the PipelineResources declared in Pipeline p are bound in PipelineRun. func ValidateResourceBindings(p *v1alpha1.PipelineSpec, pr *v1alpha1.PipelineRun) error { required := make([]string, 0, len(p.Resources)) + optional := make([]string, 0, len(p.Resources)) for _, resource := range p.Resources { - required = append(required, resource.Name) + if resource.Optional { + // create a list of optional resources + optional = append(optional, resource.Name) + } else { + // create a list of required resources + required = append(required, resource.Name) + } } provided := make([]string, 0, len(pr.Spec.Resources)) for _, resource := range pr.Spec.Resources { provided = append(provided, resource.Name) } - if err := list.IsSame(required, provided); err != nil { - return fmt.Errorf("pipelineRun bound resources didn't match Pipeline: %w", err) + // verify that the list of required resources exists in the provided resources + missing := list.DiffLeft(required, provided) + if len(missing) > 0 { + return fmt.Errorf("Pipeline's declared required resources are missing from the PipelineRun: %s", missing) + } + // verify that the list of provided resources does not have any extra resources (outside of required and optional resources combined) + extra := list.DiffLeft(provided, append(required, optional...)) + if len(extra) > 0 { + return fmt.Errorf("PipelineRun's declared resources didn't match usage in Pipeline: %s", extra) } return nil } @@ -449,11 +463,15 @@ func resolveConditionChecks(pt *v1alpha1.PipelineTask, taskRunStatus map[string] } conditionResources := map[string]*v1alpha1.PipelineResource{} for _, declared := range ptc.Resources { - r, ok := providedResources[declared.Resource] - if !ok { - return nil, fmt.Errorf("resources %s missing for condition %s in pipeline task %s", declared.Resource, cName, pt.Name) + if r, ok := providedResources[declared.Resource]; ok { + conditionResources[declared.Name] = r + } else { + for _, resource := range c.Spec.Resources { + if declared.Name == resource.Name && !resource.Optional { + return nil, fmt.Errorf("resources %s missing for condition %s in pipeline task %s", declared.Resource, cName, pt.Name) + } + } } - conditionResources[declared.Name] = r } rcc := ResolvedConditionCheck{ @@ -481,18 +499,32 @@ func ResolvePipelineTaskResources(pt v1alpha1.PipelineTask, ts *v1alpha1.TaskSpe } if pt.Resources != nil { for _, taskInput := range pt.Resources.Inputs { - resource, ok := providedResources[taskInput.Resource] - if !ok { - return nil, fmt.Errorf("pipelineTask tried to use input resource %s not present in declared resources", taskInput.Resource) + if resource, ok := providedResources[taskInput.Resource]; ok { + rtr.Inputs[taskInput.Name] = resource + } else { + if ts.Inputs == nil { + return nil, fmt.Errorf("pipelineTask tried to use input resource %s not present in declared resources", taskInput.Resource) + } + for _, r := range ts.Inputs.Resources { + if r.Name == taskInput.Name && !r.Optional { + return nil, fmt.Errorf("pipelineTask tried to use input resource %s not present in declared resources", taskInput.Resource) + } + } } - rtr.Inputs[taskInput.Name] = resource } for _, taskOutput := range pt.Resources.Outputs { - resource, ok := providedResources[taskOutput.Resource] - if !ok { - return nil, fmt.Errorf("pipelineTask tried to use output resource %s not present in declared resources", taskOutput.Resource) + if resource, ok := providedResources[taskOutput.Resource]; ok { + rtr.Outputs[taskOutput.Name] = resource + } else { + if ts.Outputs == nil { + return nil, fmt.Errorf("pipelineTask tried to use output resource %s not present in declared resources", taskOutput.Resource) + } + for _, r := range ts.Outputs.Resources { + if r.Name == taskOutput.Name && !r.Optional { + return nil, fmt.Errorf("pipelineTask tried to use output resource %s not present in declared resources", taskOutput.Resource) + } + } } - rtr.Outputs[taskOutput.Name] = resource } } return &rtr, nil diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 0cd08dc161f..8469c841f25 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -452,6 +452,38 @@ var taskCancelled = PipelineRunState{{ }, }} +var taskWithOptionalResources = &v1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task", + }, + Spec: v1alpha1.TaskSpec{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ResourceDeclaration: v1alpha1.ResourceDeclaration{ + Name: "optional-input", + Type: "git", + Optional: true, + }}, {ResourceDeclaration: v1alpha1.ResourceDeclaration{ + Name: "required-input", + Type: "git", + Optional: false, + }}}}, + Outputs: &v1alpha1.Outputs{ + Resources: []v1alpha1.TaskResource{{ResourceDeclaration: v1alpha1.ResourceDeclaration{ + Name: "optional-output", + Type: "git", + Optional: true, + }}, {ResourceDeclaration: v1alpha1.ResourceDeclaration{ + Name: "required-output", + Type: "git", + Optional: false, + }}}, + }, + Steps: []v1alpha1.Step{{Container: corev1.Container{ + Name: "step1", + }}}, + }, +} + func DagFromState(state PipelineRunState) (*dag.Graph, error) { pts := []v1alpha1.PipelineTask{} for _, rprt := range state { @@ -1392,6 +1424,61 @@ func TestResolvePipelineRun_withExistingTaskRuns(t *testing.T) { } } +func TestResolvedPipelineRun_PipelineTaskHasOptionalResources(t *testing.T) { + names.TestingSeed() + p := tb.Pipeline("pipelines", "namespace", tb.PipelineSpec( + tb.PipelineDeclaredResource("git-resource", "git"), + tb.PipelineTask("mytask1", "task", + tb.PipelineTaskInputResource("required-input", "git-resource"), + tb.PipelineTaskOutputResource("required-output", "git-resource"), + ), + )) + + r := &v1alpha1.PipelineResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "someresource", + }, + Spec: v1alpha1.PipelineResourceSpec{ + Type: v1alpha1.PipelineResourceTypeGit, + }, + } + providedResources := map[string]*v1alpha1.PipelineResource{"git-resource": r} + pr := v1alpha1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipelinerun", + }, + } + + getTask := func(name string) (v1alpha1.TaskInterface, error) { return taskWithOptionalResources, nil } + getTaskRun := func(name string) (*v1alpha1.TaskRun, error) { return nil, nil } + getClusterTask := func(name string) (v1alpha1.TaskInterface, error) { return nil, nil } + getCondition := func(name string) (*v1alpha1.Condition, error) { return nil, nil } + + 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) + } + expectedState := PipelineRunState{{ + PipelineTask: &p.Spec.Tasks[0], + TaskRunName: "pipelinerun-mytask1-9l9zj", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskName: taskWithOptionalResources.Name, + TaskSpec: &taskWithOptionalResources.Spec, + Inputs: map[string]*v1alpha1.PipelineResource{ + "required-input": r, + }, + Outputs: map[string]*v1alpha1.PipelineResource{ + "required-output": r, + }, + }, + }} + + 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) + } +} + func TestResolveConditionChecks(t *testing.T) { names.TestingSeed() ccName := "pipelinerun-mytask1-9l9zj-always-true-mz4c7"