[pipeline-to-taskrun] Support optional workspace ⚙️

Initially I left this out just to reduce the number of cases I needed to
cover but it was actually pretty easy to add. The functionality will be:
- When an optional workspace is provided, we do the same remapping we do
  for other workspace
- When an optional workspace is not provided, we don't do any remapping
  and just declare the same workspace as optional in the merged Task

I ran into this b/c I was trying to use pipeline to taskrun with the
latest git clone task in the catalog (0.4) which declares an optional
ssh-directory and I was frustrated that it didn't work.

pipeline-to-taskrun/README.md

@@ -83,6 +83,7 @@ Currently supported features:
 * Sequential tasks (specified using [`runAfter`](https://github.com/tektoncd/pipeline/blob/main/docs/pipelines.md#using-the-runafter-parameter))
 * [String params](https://github.com/tektoncd/pipeline/blob/main/docs/pipelines.md#specifying-parameters)
 * [Workspaces](https://github.com/tektoncd/pipeline/blob/main/docs/pipelines.md#specifying-workspaces)
+  * Including [optional workspaces](https://github.com/tektoncd/pipeline/blob/main/docs/workspaces.md#optional-workspaces)
 
 ### Potential future features
 
@@ -102,7 +103,6 @@ These features may be added in the future:
 * Workspace features:
   * [mountPaths](https://github.com/tektoncd/pipeline/blob/main/docs/workspaces.md#using-workspaces-in-tasks)
   * [subPaths](https://github.com/tektoncd/pipeline/blob/main/docs/workspaces.md#using-workspaces-in-pipelines)
-  * [optional](https://github.com/tektoncd/pipeline/blob/main/docs/workspaces.md#optional-workspaces)
   * [readOnly](https://github.com/tektoncd/pipeline/blob/main/docs/workspaces.md#using-workspaces-in-tasks)
   * [isolated](https://github.com/tektoncd/pipeline/blob/main/docs/workspaces.md#isolating-workspaces-to-specific-steps-or-sidecars)
   * In addition further thought will have to be given to support workspaces that combine

pipeline-to-taskrun/pkg/reconciler/pipelinetotaskrun/pipelinetotaskrun_test.go

@@ -655,29 +655,6 @@ spec:
       persistentVolumeClaim:
         claimName: pvc
 `),
-	}, {
-		name:            "optional workspaces - TODO(community#447)",
-		expectedErrText: []string{"optional"},
-		pipeline: test.MustParsePipeline(t, `
-metadata:
-  name: pipeline
-  namespace: foo
-spec:
-  tasks:
-  - name: use-workspace
-    taskSpec:
-      steps:
-      - image: ubuntu
-      workspaces:
-      - name: task-workspace
-        optional: true
-    workspaces:
-    - name: task-workspace
-      workspace: pipeline-workspace
-  workspaces:
-  - name: pipeline-workspace
-`),
-		run: test.MustParseRun(t, run),
 	}, {
 		name:            "embedded tasks with labels and annotations - TODO(community#447)",
 		expectedErrText: []string{"label", "annotation"},

pipeline-to-taskrun/pkg/reconciler/pipelinetotaskrun/steps.go

@@ -165,6 +165,11 @@ func (pti PipelineTaskInfo) RenameWorkspaces(newMapping map[string]string) Pipel
 	// create a mapping of the replacements that can be used to update the steps
 	replacements := map[string]string{}
 	for oldName, newName := range newMapping {
+		// this value will be blank for optional workspaces that aren't provided
+		// for those we shouldn't do any replacement
+		if newName == "" {
+			continue
+		}
 		// we need to explicitly replace every known workspace variable
 		for _, variable := range []string{"path", "bound", "claim", "volume"} {
 			// this is the format that ApplyReplacements expects the replacements to arrive in; it infers the surrounding

pipeline-to-taskrun/pkg/reconciler/pipelinetotaskrun/steps_test.go

@@ -412,6 +412,7 @@ func TestRenameWorkspaces(t *testing.T) {
   - name: grab-source-clone
     image: some-git-image
     script: |
+      echo $(workspaces.optional.bound)
       echo $(workspaces.foobar.bound)
       echo $(workspaces.foobar.claim)
       echo $(workspaces.foobar.volume)
@@ -430,6 +431,7 @@ func TestRenameWorkspaces(t *testing.T) {
   - name: grab-source-clone
     image: some-git-image
     script: |
+      echo $(workspaces.optional.bound)
       echo $(workspaces.the-ultimate-volume.bound)
       echo $(workspaces.the-ultimate-volume.claim)
       echo $(workspaces.the-ultimate-volume.volume)
@@ -438,7 +440,7 @@ func TestRenameWorkspaces(t *testing.T) {
   - name: commit
     description: "The precise commit SHA that was fetched by this Task"
 `)
-	newMapping := map[string]string{"foobar": "the-ultimate-volume"}
+	newMapping := map[string]string{"foobar": "the-ultimate-volume", "optional": ""}
 	updatedPti := pti.RenameWorkspaces(newMapping)
 	if d := cmp.Diff(expected, updatedPti); d != "" {
 		t.Errorf("didn't get expected updated info. Diff: %s", diff.PrintWantGot(d))

pipeline-to-taskrun/pkg/reconciler/pipelinetotaskrun/taskrun.go

@@ -37,6 +37,20 @@ func getMergedTaskRun(run *v1alpha1.Run, pSpec *v1beta1.PipelineSpec, taskSpecs
 		})
 	}
 
+	// if an optional workspace isn't provided, we don't need to remap it but we still need to declare it
+	// in order for any variable interpolation to work
+	optionalWS, err := getUnboundOptionalWorkspaces(taskSpecs, newWorkspaceMapping)
+	if err != nil {
+		return nil, fmt.Errorf("invalid workspace binding for %s wasn't caught by validation: %v", run.Name, err)
+	}
+	for _, ws := range optionalWS {
+		tr.Spec.TaskSpec.Workspaces = append(tr.Spec.TaskSpec.Workspaces, v1beta1.WorkspaceDeclaration{
+			Name:        ws.Name,
+			Description: ws.Description,
+			Optional:    ws.Optional,
+		})
+	}
+
 	for _, pTask := range sequenceWithAppliedParams {
 		pti, err := NewPipelineTaskInfo(pTask, taskSpecs)
 		if err != nil {

pipeline-to-taskrun/pkg/reconciler/pipelinetotaskrun/testdata/expected-taskrun.yaml

@@ -143,6 +143,8 @@ spec:
       # when the script block is in the middle of the file and absent when at the end of the file
       script: |-
         #!/bin/sh
+        echo "$(workspaces.ssh-directory.bound)"
+
         set -eu -o pipefail
         if [[ "$(params.grab-source-verbose)" == "true" ]] ; then
           set -x
@@ -233,6 +235,8 @@ spec:
     workspaces:
     - name: where-it-all-happens
     - name: gcs-creds
+    - name: ssh-directory
+      optional: true
   workspaces:
   - name: where-it-all-happens
     persistentVolumeClaim:

pipeline-to-taskrun/pkg/reconciler/pipelinetotaskrun/testdata/gcs-upload.yaml

@@ -18,6 +18,8 @@ spec:
   workspaces:
     - name: credentials
       description: A secret with a service account key to use as GOOGLE_APPLICATION_CREDENTIALS.
+      # The task won't work without this workspace, but we're setting it to optional to make sure we test the case where an optional workspace is provided
+      optional: true
     - name: source
       description: A workspace where files will be uploaded from.
   params:

pipeline-to-taskrun/pkg/reconciler/pipelinetotaskrun/testdata/git-clone.yaml

@@ -22,6 +22,8 @@ spec:
   workspaces:
     - name: output
       description: The git repo will be cloned onto the volume backing this workspace
+    - name: ssh-directory
+      optional: true
   params:
     - name: url
       description: git url to clone
@@ -89,6 +91,8 @@ spec:
       # when the script block is in the middle of the file and absent when at the end of the file
       script: |-
         #!/bin/sh
+        echo "$(workspaces.ssh-directory.bound)"
+
         set -eu -o pipefail
         if [[ "$(params.verbose)" == "true" ]] ; then
           set -x

pipeline-to-taskrun/pkg/reconciler/pipelinetotaskrun/validate.go

@@ -99,9 +99,6 @@ func validateTaskSpec(taskSpec *v1beta1.TaskSpec) error {
 		if w.ReadOnly {
 			return fmt.Errorf("readOnly workspaces are not supported but %s is readOnly", w.Name)
 		}
-		if w.Optional {
-			return fmt.Errorf("optional workspaces are not supported but %s is optional", w.Name)
-		}
 	}
 	for _, p := range taskSpec.Params {
 		if p.Type == v1beta1.ParamTypeArray {

pipeline-to-taskrun/pkg/reconciler/pipelinetotaskrun/workspaces.go

@@ -17,13 +17,14 @@ limitations under the License.
 package pipelinetotaskrun
 
 import (
+	"fmt"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 )
 
 type PipelineTaskToWorkspaces map[string]map[string]string
 
 // getNewWorkspaceMapping will create an object that maps from the mapped workspaces in each pipeline task in pTasks to
-// the pipeline level workspace that is actually used. It returns a map where they keys are pipeline task names, and
+// the pipeline level workspace that is actually used. It returns a map where the keys are pipeline task names, and
 // the values are dictionaries that map each of the task's declared workspaces to the actual workspaces used.
 func getNewWorkspaceMapping(pTasks []v1beta1.PipelineTask) PipelineTaskToWorkspaces {
 	mapping := PipelineTaskToWorkspaces{}
@@ -37,3 +38,21 @@ func getNewWorkspaceMapping(pTasks []v1beta1.PipelineTask) PipelineTaskToWorkspa
 
 	return mapping
 }
+
+// getUnboundOptionalWorkspaces returns a list of all the optional workspaces that are declared in taskSpecs but not actually
+// bound in newWorkspaceMapping, or an error if an unbound workspace is not optional.
+func getUnboundOptionalWorkspaces(taskSpecs map[string]*v1beta1.TaskSpec, newWorkspaceMapping PipelineTaskToWorkspaces) ([]v1beta1.WorkspaceDeclaration, error) {
+	optionalWS := []v1beta1.WorkspaceDeclaration{}
+	for pt, mappings := range newWorkspaceMapping {
+		taskSpec := taskSpecs[pt]
+		for _, ws := range taskSpec.Workspaces {
+			if _, ok := mappings[ws.Name]; !ok {
+				if !ws.Optional {
+					return nil, fmt.Errorf("workspace %s is not bound in %s but is not optional", ws.Name, pt)
+				}
+				optionalWS = append(optionalWS, ws)
+			}
+		}
+	}
+	return optionalWS, nil
+}

pipeline-to-taskrun/pkg/reconciler/pipelinetotaskrun/workspaces_test.go

@@ -36,6 +36,8 @@ spec:
     workspaces:
     - name: source
       workspace: where-it-all-happens
+    - name: secret
+      workspace: gcs-creds
   - name: upload-results
     workspaces:
     - name: source
@@ -53,6 +55,7 @@ spec:
 		},
 		"run-tests": {
 			"source": "where-it-all-happens",
+			"secret": "gcs-creds",
 		},
 		"upload-results": {
 			"source":      "where-it-all-happens",
@@ -91,3 +94,79 @@ spec:
 		t.Errorf("Did not get expected workspace mapping: %v", diff.PrintWantGot(d))
 	}
 }
+
+func TestGetUnboundOptionalWorkspaces(t *testing.T) {
+	mapping := PipelineTaskToWorkspaces{
+		"grab-source": {
+			"output": "where-it-all-happens",
+		},
+		"run-tests": {
+			"source": "where-it-all-happens",
+			"secret": "gcs-creds",
+		},
+		"upload-results": {
+			"source":      "where-it-all-happens",
+			"credentials": "gcs-creds",
+		},
+	}
+	tasks := []*v1beta1.Task{
+		test.MustParseTask(t, `
+spec:
+  workspaces:
+  - name: output
+  - name: ssh-directory
+    optional: true
+`),
+		test.MustParseTask(t, `
+spec:
+  workspaces:
+  - name: source
+  - name: secret
+    optional: true
+`),
+		test.MustParseTask(t, `
+spec:
+  workspaces:
+  - name: source
+  - name: credentials
+`),
+	}
+	taskSpecs := map[string]*v1beta1.TaskSpec{
+		"grab-source":    &tasks[0].Spec,
+		"run-tests":      &tasks[1].Spec,
+		"upload-results": &tasks[2].Spec,
+	}
+	expectedOptionalWS := []v1beta1.WorkspaceDeclaration{
+		taskSpecs["grab-source"].Workspaces[1],
+	}
+
+	optionalWS, err := getUnboundOptionalWorkspaces(taskSpecs, mapping)
+	if err != nil {
+		t.Fatalf("Did not expect error when getting optional workspaces but got %v", err)
+	}
+
+	if d := cmp.Diff(expectedOptionalWS, optionalWS); d != "" {
+		t.Errorf("Did not get expected optional workspaces: %v", diff.PrintWantGot(d))
+	}
+}
+
+func TestGetUnboundOptionalWorkspacesInvalid(t *testing.T) {
+	mapping := PipelineTaskToWorkspaces{
+		"grab-source": {},
+	}
+	tasks := []*v1beta1.Task{
+		test.MustParseTask(t, `
+spec:
+  workspaces:
+  - name: output
+`),
+	}
+	taskSpecs := map[string]*v1beta1.TaskSpec{
+		"grab-source": &tasks[0].Spec,
+	}
+
+	_, err := getUnboundOptionalWorkspaces(taskSpecs, mapping)
+	if err == nil {
+		t.Fatalf("Expected error when required workspace is not bound but got none")
+	}
+}