[TEP 108] Mapping workspaces

According to TEP-0108:
auto-mapping Workspaces from Pipelines to PipelineTasks
when the names of the Workspaces declared in the Pipeline
and PipelineTask are the same to reduce verbosity and improve usability of Pipelines.

docs/pipelines.md

@@ -6,8 +6,9 @@ weight: 400
 -->
 # Pipelines
 
-- [Overview](#pipelines)
-- [Configuring a `Pipeline`](#configuring-a-pipeline)
+- [Pipelines](#pipelines)
+  - [Overview](#overview)
+  - [Configuring a `Pipeline`](#configuring-a-pipeline)
   - [Specifying `Resources`](#specifying-resources)
   - [Specifying `Workspaces`](#specifying-workspaces)
   - [Specifying `Parameters`](#specifying-parameters)
@@ -177,10 +178,38 @@ spec:
           workspace: pipeline-ws1
 ```
 
+For simplicity you can also map the name of the `Workspace` in `PipelineTask` to match with 
+the `Workspace` from the `Pipeline`.
+For example:
+
+```yaml
+apiVersion: tekton.dev/v1beta1
+kind: Pipeline
+metadata:
+  name: pipeline
+spec:
+  workspaces:
+    - name: source
+  tasks:
+    - name: gen-code
+      taskRef:
+        name: gen-code # gen-code expects a Workspace named "source"
+      workspaces:
+        - name: source # <- mapping workspace name 
+    - name: commit
+      taskRef:
+        name: commit # commit expects a Workspace named "source"
+      workspaces:
+        - name: source # <- mapping workspace name 
+      runAfter:
+        - gen-code
+```
+
 For more information, see:
 - [Using `Workspaces` in `Pipelines`](workspaces.md#using-workspaces-in-pipelines)
 - The [`Workspaces` in a `PipelineRun`](../examples/v1beta1/pipelineruns/workspaces.yaml) code example
 - The [variables available in a `PipelineRun`](variables.md#variables-available-in-a-pipeline), including `workspaces.<name>.bound`.
+- [Mapping `Workspaces`](https://github.com/tektoncd/community/blob/main/teps/0108-mapping-workspaces.md)
 
 ## Specifying `Parameters`
 

pkg/apis/pipeline/v1beta1/pipeline_types.go

@@ -406,7 +406,14 @@ func validateExecutionStatusVariablesExpressions(expressions []string, ptNames s
 
 func (pt *PipelineTask) validateWorkspaces(workspaceNames sets.String) (errs *apis.FieldError) {
 	for i, ws := range pt.Workspaces {
-		if !workspaceNames.Has(ws.Workspace) {
+		if ws.Workspace == "" {
+			if !workspaceNames.Has(ws.Name) {
+				errs = errs.Also(apis.ErrInvalidValue(
+					fmt.Sprintf("pipeline task %q expects workspace with name %q but none exists in pipeline spec", pt.Name, ws.Name),
+					"",
+				).ViaFieldIndex("workspaces", i))
+			}
+		} else if !workspaceNames.Has(ws.Workspace) {
 			errs = errs.Also(apis.ErrInvalidValue(
 				fmt.Sprintf("pipeline task %q expects workspace with name %q but none exists in pipeline spec", pt.Name, ws.Workspace),
 				"",

pkg/apis/pipeline/v1beta1/pipeline_validation_test.go

@@ -1701,21 +1701,41 @@ func TestValidatePipelineWorkspacesDeclarations_Success(t *testing.T) {
 }
 
 func TestValidatePipelineWorkspacesUsage_Success(t *testing.T) {
-	desc := "unused pipeline spec workspaces do not cause an error"
-	workspaces := []PipelineWorkspaceDeclaration{{
-		Name: "foo",
+	tests := []struct {
+		name       string
+		workspaces []PipelineWorkspaceDeclaration
+		tasks      []PipelineTask
+	}{{
+		name: "unused pipeline spec workspaces do not cause an error",
+		workspaces: []PipelineWorkspaceDeclaration{{
+			Name: "foo",
+		}, {
+			Name: "bar",
+		}},
+		tasks: []PipelineTask{{
+			Name: "foo", TaskRef: &TaskRef{Name: "foo"},
+		}},
 	}, {
-		Name: "bar",
-	}}
-	tasks := []PipelineTask{{
-		Name: "foo", TaskRef: &TaskRef{Name: "foo"},
+		name: "valid mapping pipeline-task workspace name with pipeline workspace name",
+		workspaces: []PipelineWorkspaceDeclaration{{
+			Name: "pipelineWorkspaceName",
+		}},
+		tasks: []PipelineTask{{
+			Name: "foo", TaskRef: &TaskRef{Name: "foo"},
+			Workspaces: []WorkspacePipelineTaskBinding{{
+				Name:      "pipelineWorkspaceName",
+				Workspace: "",
+			}},
+		}},
 	}}
-	t.Run(desc, func(t *testing.T) {
-		err := validatePipelineWorkspacesUsage(workspaces, tasks)
-		if err != nil {
-			t.Errorf("Pipeline.validatePipelineWorkspacesUsage() returned error for valid pipeline workspaces: %v", err)
-		}
-	})
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			errs := validatePipelineWorkspacesUsage(tt.workspaces, tt.tasks).ViaField("tasks")
+			if errs != nil {
+				t.Errorf("Pipeline.validatePipelineWorkspacesUsage() returned error for valid pipeline workspaces: %v", errs)
+			}
+		})
+	}
 }
 
 func TestValidatePipelineWorkspacesDeclarations_Failure(t *testing.T) {
@@ -1786,6 +1806,22 @@ func TestValidatePipelineWorkspacesUsage_Failure(t *testing.T) {
 			Message: `invalid value: pipeline task "foo" expects workspace with name "pipelineWorkspaceName" but none exists in pipeline spec`,
 			Paths:   []string{"tasks[0].workspaces[0]"},
 		},
+	}, {
+		name: "invalid mapping workspace with different name",
+		workspaces: []PipelineWorkspaceDeclaration{{
+			Name: "pipelineWorkspaceName",
+		}},
+		tasks: []PipelineTask{{
+			Name: "foo", TaskRef: &TaskRef{Name: "foo"},
+			Workspaces: []WorkspacePipelineTaskBinding{{
+				Name:      "taskWorkspaceName",
+				Workspace: "",
+			}},
+		}},
+		expectedError: apis.FieldError{
+			Message: `invalid value: pipeline task "foo" expects workspace with name "taskWorkspaceName" but none exists in pipeline spec`,
+			Paths:   []string{"tasks[0].workspaces[0]"},
+		},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/apis/pipeline/v1beta1/swagger.json

@@ -3095,8 +3095,7 @@
       "description": "WorkspacePipelineTaskBinding describes how a workspace passed into the pipeline should be mapped to a task's declared workspace.",
       "type": "object",
       "required": [
-        "name",
-        "workspace"
+        "name"
       ],
       "properties": {
         "name": {
@@ -3110,8 +3109,7 @@
         },
         "workspace": {
           "description": "Workspace is the name of the workspace declared by the pipeline",
-          "type": "string",
-          "default": ""
+          "type": "string"
         }
       }
     },

pkg/apis/pipeline/v1beta1/workspace_types.go

@@ -105,7 +105,8 @@ type WorkspacePipelineTaskBinding struct {
 	// Name is the name of the workspace as declared by the task
 	Name string `json:"name"`
 	// Workspace is the name of the workspace declared by the pipeline
-	Workspace string `json:"workspace"`
+	// +optional
+	Workspace string `json:"workspace,omitempty"`
 	// SubPath is optionally a directory on the volume which should be used
 	// for this binding (i.e. the volume will be mounted at this sub directory).
 	// +optional

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -888,9 +888,16 @@ func getTaskrunWorkspaces(pr *v1beta1.PipelineRun, rprt *resources.ResolvedPipel
 	}
 	for _, ws := range rprt.PipelineTask.Workspaces {
 		taskWorkspaceName, pipelineTaskSubPath, pipelineWorkspaceName := ws.Name, ws.SubPath, ws.Workspace
-		if b, hasBinding := pipelineRunWorkspaces[pipelineWorkspaceName]; hasBinding {
+
+		pipelineWorkspace := pipelineWorkspaceName
+
+		if pipelineWorkspaceName == "" {
+			pipelineWorkspace = taskWorkspaceName
+		}
+
+		if b, hasBinding := pipelineRunWorkspaces[pipelineWorkspace]; hasBinding {
 			if b.PersistentVolumeClaim != nil || b.VolumeClaimTemplate != nil {
-				pipelinePVCWorkspaceName = pipelineWorkspaceName
+				pipelinePVCWorkspaceName = pipelineWorkspace
 			}
 			workspaces = append(workspaces, taskWorkspaceByWorkspaceVolumeSource(b, taskWorkspaceName, pipelineTaskSubPath, *kmeta.NewControllerRef(pr)))
 		} else {
@@ -904,9 +911,10 @@ func getTaskrunWorkspaces(pr *v1beta1.PipelineRun, rprt *resources.ResolvedPipel
 				}
 			}
 			if !workspaceIsOptional {
-				return nil, "", fmt.Errorf("expected workspace %q to be provided by pipelinerun for pipeline task %q", pipelineWorkspaceName, rprt.PipelineTask.Name)
+				return nil, "", fmt.Errorf("expected workspace %q to be provided by pipelinerun for pipeline task %q", pipelineWorkspace, rprt.PipelineTask.Name)
 			}
 		}
+
 	}
 	return workspaces, pipelinePVCWorkspaceName, nil
 }

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -7523,3 +7523,117 @@ func checkPipelineRunConditionStatusAndReason(t *testing.T, reconciledRun *v1bet
 		t.Errorf("Expected reason %s but was %s", conditionReason, condition.Reason)
 	}
 }
+
+func TestGetTaskrunWorkspaces_Failure(t *testing.T) {
+	tests := []struct {
+		name          string
+		pr            *v1beta1.PipelineRun
+		rprt          *resources.ResolvedPipelineRunTask
+		expectedError string
+	}{{
+		name: "failure declaring workspace with different name",
+		pr: parse.MustParsePipelineRun(t, `
+metadata:
+  name: pipeline
+spec:
+  workspaces:
+    - name: source`),
+		rprt: &resources.ResolvedPipelineRunTask{
+			PipelineTask: &v1beta1.PipelineTask{
+				Name: "resolved-pipelinetask",
+				Workspaces: []v1beta1.WorkspacePipelineTaskBinding{{
+					Name:      "my-task-workspace",
+					Workspace: "not-source",
+				}},
+			},
+		},
+		expectedError: `expected workspace "not-source" to be provided by pipelinerun for pipeline task "resolved-pipelinetask"`,
+	},
+		{
+			name: "failure mapping workspace with different name",
+			pr: parse.MustParsePipelineRun(t, `
+metadata:
+  name: pipeline
+spec:
+  workspaces:
+    - name: source
+ `),
+			rprt: &resources.ResolvedPipelineRunTask{
+				PipelineTask: &v1beta1.PipelineTask{
+					Name: "resolved-pipelinetask",
+					Workspaces: []v1beta1.WorkspacePipelineTaskBinding{{
+						Name:      "not-source",
+						Workspace: "",
+					}},
+				},
+			},
+			expectedError: `expected workspace "not-source" to be provided by pipelinerun for pipeline task "resolved-pipelinetask"`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, _, err := getTaskrunWorkspaces(tt.pr, tt.rprt)
+
+			if err == nil {
+				t.Errorf("Pipeline.getTaskrunWorkspaces() did not return error for invalid pipeline with finally")
+			} else if d := cmp.Diff(tt.expectedError, err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
+				t.Errorf("Pipeline.getTaskrunWorkspaces() errors diff %s", diff.PrintWantGot(d))
+			}
+		})
+	}
+
+}
+
+func TestGetTaskrunWorkspaces_Success(t *testing.T) {
+	tests := []struct {
+		name string
+		pr   *v1beta1.PipelineRun
+		rprt *resources.ResolvedPipelineRunTask
+	}{{
+		name: "valid declaration of workspace names",
+		pr: parse.MustParsePipelineRun(t, `
+metadata:
+  name: pipeline
+spec:
+  workspaces:
+    - name: source`),
+		rprt: &resources.ResolvedPipelineRunTask{
+			PipelineTask: &v1beta1.PipelineTask{
+				Name: "resolved-pipelinetask",
+				Workspaces: []v1beta1.WorkspacePipelineTaskBinding{{
+					Name:      "my-task-workspace",
+					Workspace: "source",
+				}},
+			},
+		},
+	},
+		{
+			name: "valid mapping with same workspace names",
+			pr: parse.MustParsePipelineRun(t, `
+metadata:
+  name: pipeline
+spec:
+  workspaces:
+    - name: source`),
+			rprt: &resources.ResolvedPipelineRunTask{
+				PipelineTask: &v1beta1.PipelineTask{
+					Name: "resolved-pipelinetask",
+					Workspaces: []v1beta1.WorkspacePipelineTaskBinding{{
+						Name:      "source",
+						Workspace: "",
+					}},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, _, err := getTaskrunWorkspaces(tt.pr, tt.rprt)
+
+			if err != nil {
+				t.Errorf("Pipeline.getTaskrunWorkspaces() returned error for valid pipeline with finally: %v", err)
+			}
+		})
+	}
+
+}