From 118e162dbd30fb8dba10046f8d5e760a72f7b9dc Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Mon, 27 Jan 2020 18:06:48 +0100 Subject: [PATCH] =?UTF-8?q?Port=20Workspace=20in=20pipeline=20support=20to?= =?UTF-8?q?=20v1alpha2=20=F0=9F=8E=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Vincent Demeester --- pkg/apis/pipeline/v1alpha2/pipeline_types.go | 9 ++++ .../pipeline/v1alpha2/pipeline_validation.go | 33 +++++++++++++ .../v1alpha2/pipeline_validation_test.go | 46 +++++++++++++++++++ .../v1alpha2/zz_generated.deepcopy.go | 10 ++++ 4 files changed, 98 insertions(+) diff --git a/pkg/apis/pipeline/v1alpha2/pipeline_types.go b/pkg/apis/pipeline/v1alpha2/pipeline_types.go index ae862be94fd..c87d73dbbf5 100644 --- a/pkg/apis/pipeline/v1alpha2/pipeline_types.go +++ b/pkg/apis/pipeline/v1alpha2/pipeline_types.go @@ -73,6 +73,10 @@ type PipelineSpec struct { // Params declares a list of input parameters that must be supplied when // this Pipeline is run. Params []ParamSpec `json:"params,omitempty"` + // Workspaces declares a set of named workspaces that are expected to be + // provided by a PipelineRun. + // +optional + Workspaces []WorkspacePipelineDeclaration `json:"workspaces,omitempty"` } // PipelineTask defines a task in a Pipeline, passing inputs from both @@ -111,6 +115,11 @@ type PipelineTask struct { // Parameters declares parameters passed to this task. // +optional Params []Param `json:"params,omitempty"` + + // Workspaces maps workspaces from the pipeline spec to the workspaces + // declared in the Task. + // +optional + Workspaces []WorkspacePipelineTaskBinding `json:"workspaces,omitempty"` } func (pt PipelineTask) HashKey() string { diff --git a/pkg/apis/pipeline/v1alpha2/pipeline_validation.go b/pkg/apis/pipeline/v1alpha2/pipeline_validation.go index 11130a05b0e..48c5af717c2 100644 --- a/pkg/apis/pipeline/v1alpha2/pipeline_validation.go +++ b/pkg/apis/pipeline/v1alpha2/pipeline_validation.go @@ -189,6 +189,39 @@ func (ps *PipelineSpec) Validate(ctx context.Context) *apis.FieldError { return err } + // Validate the pipeline's workspaces. + if err := validatePipelineWorkspaces(ps.Workspaces, ps.Tasks); err != nil { + return err + } + + return nil +} + +func validatePipelineWorkspaces(wss []WorkspacePipelineDeclaration, pts []PipelineTask) *apis.FieldError { + // Workspace names must be non-empty and unique. + wsTable := make(map[string]struct{}) + for i, ws := range wss { + if ws.Name == "" { + return apis.ErrInvalidValue(fmt.Sprintf("workspace %d has empty name", i), "spec.workspaces") + } + if _, ok := wsTable[ws.Name]; ok { + return apis.ErrInvalidValue(fmt.Sprintf("workspace with name %q appears more than once", ws.Name), "spec.workspaces") + } + wsTable[ws.Name] = struct{}{} + } + + // Any workspaces used in PipelineTasks should have their name declared in the Pipeline's + // Workspaces list. + for ptIdx, pt := range pts { + for wsIdx, ws := range pt.Workspaces { + if _, ok := wsTable[ws.Workspace]; !ok { + return apis.ErrInvalidValue( + fmt.Sprintf("pipeline task %q expects workspace with name %q but none exists in pipeline spec", pt.Name, ws.Workspace), + fmt.Sprintf("spec.tasks[%d].workspaces[%d]", ptIdx, wsIdx), + ) + } + } + } return nil } diff --git a/pkg/apis/pipeline/v1alpha2/pipeline_validation_test.go b/pkg/apis/pipeline/v1alpha2/pipeline_validation_test.go index 360bb07908a..99f1f484cc6 100644 --- a/pkg/apis/pipeline/v1alpha2/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1alpha2/pipeline_validation_test.go @@ -562,6 +562,52 @@ func TestPipeline_Validate(t *testing.T) { }, }, failureExpected: true, + }, { + name: "unused pipeline spec workspaces do not cause an error", + p: &v1alpha2.Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: v1alpha2.PipelineSpec{ + Tasks: []v1alpha2.PipelineTask{{ + Name: "foo", TaskRef: &v1alpha2.TaskRef{Name: "foo"}, + }}, + Workspaces: []v1alpha2.WorkspacePipelineDeclaration{{ + Name: "foo", + }, { + Name: "bar", + }}, + }, + }, + failureExpected: false, + }, { + name: "workspace bindings relying on a non-existent pipeline workspace cause an error", + p: &v1alpha2.Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: v1alpha2.PipelineSpec{ + Tasks: []v1alpha2.PipelineTask{{ + Name: "foo", TaskRef: &v1alpha2.TaskRef{Name: "foo"}, + Workspaces: []v1alpha2.WorkspacePipelineTaskBinding{{ + Name: "taskWorkspaceName", + Workspace: "pipelineWorkspaceName", + }}, + }}, + Workspaces: []v1alpha2.WorkspacePipelineDeclaration{{ + Name: "foo", + }}, + }, + }, + failureExpected: true, + }, { + name: "multiple workspaces sharing the same name are not allowed", + p: &v1alpha2.Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, Spec: v1alpha2.PipelineSpec{ + Workspaces: []v1alpha2.WorkspacePipelineDeclaration{{ + Name: "foo", + }, { + Name: "foo", + }}, + }, + }, + failureExpected: true, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1alpha2/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha2/zz_generated.deepcopy.go index b944a8cd68d..27ec355669a 100644 --- a/pkg/apis/pipeline/v1alpha2/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha2/zz_generated.deepcopy.go @@ -322,6 +322,11 @@ func (in *PipelineSpec) DeepCopyInto(out *PipelineSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Workspaces != nil { + in, out := &in.Workspaces, &out.Workspaces + *out = make([]WorkspacePipelineDeclaration, len(*in)) + copy(*out, *in) + } return } @@ -388,6 +393,11 @@ func (in *PipelineTask) DeepCopyInto(out *PipelineTask) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Workspaces != nil { + in, out := &in.Workspaces, &out.Workspaces + *out = make([]WorkspacePipelineTaskBinding, len(*in)) + copy(*out, *in) + } return }