From 3c9a1fda6cba32b08178b953d1ee1e0e7aa754f6 Mon Sep 17 00:00:00 2001
From: Chitrang Patel <chitrang@google.com>
Date: Tue, 4 Jun 2024 11:52:48 -0400
Subject: [PATCH] Fix: Identify workspace usage in a Task

Prior to this, when identifying whether a Task used a workspace,
we limited the check to command, args and scripts in steps,
stepTemplates and sidecars. However, the workspace path could also
be used as a param to a StepAction or env cariables in steps and
sidecars and also workingDirs. This PR fixes that.

Fixes https://github.com/tektoncd/pipeline/issues/8008.
---
 pkg/workspace/apply.go      | 21 ++++++++
 pkg/workspace/apply_test.go | 98 +++++++++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)

diff --git a/pkg/workspace/apply.go b/pkg/workspace/apply.go
index e4b1581d2af..40d2b2827ac 100644
--- a/pkg/workspace/apply.go
+++ b/pkg/workspace/apply.go
@@ -242,6 +242,10 @@ func findWorkspaceSubstitutionLocationsInSidecars(sidecars []v1.Sidecar) sets.St
 		for i := range sidecar.Command {
 			locationsToCheck.Insert(sidecar.Command[i])
 		}
+		locationsToCheck.Insert(sidecar.WorkingDir)
+		for _, e := range sidecar.Env {
+			locationsToCheck.Insert(e.Value)
+		}
 	}
 	return locationsToCheck
 }
@@ -258,6 +262,18 @@ func findWorkspaceSubstitutionLocationsInSteps(steps []v1.Step) sets.String {
 		for i := range step.Command {
 			locationsToCheck.Insert(step.Command[i])
 		}
+
+		locationsToCheck.Insert(step.WorkingDir)
+		for _, e := range step.Env {
+			locationsToCheck.Insert(e.Value)
+		}
+		for _, p := range step.Params {
+			locationsToCheck.Insert(p.Value.ArrayVal...)
+			for k := range p.Value.ObjectVal {
+				locationsToCheck.Insert(p.Value.ObjectVal[k])
+			}
+			locationsToCheck.Insert(p.Value.StringVal)
+		}
 	}
 	return locationsToCheck
 }
@@ -272,6 +288,11 @@ func findWorkspaceSubstitutionLocationsInStepTemplate(stepTemplate *v1.StepTempl
 		for i := range stepTemplate.Command {
 			locationsToCheck.Insert(stepTemplate.Command[i])
 		}
+
+		locationsToCheck.Insert(stepTemplate.WorkingDir)
+		for _, e := range stepTemplate.Env {
+			locationsToCheck.Insert(e.Value)
+		}
 	}
 	return locationsToCheck
 }
diff --git a/pkg/workspace/apply_test.go b/pkg/workspace/apply_test.go
index 01d5811f11a..b00ae904c1a 100644
--- a/pkg/workspace/apply_test.go
+++ b/pkg/workspace/apply_test.go
@@ -1266,6 +1266,104 @@ func TestFindWorkspacesUsedByTask(t *testing.T) {
 			"steptemplate-args",
 			"steptemplate-command",
 		),
+	}, {
+		name: "workspace used in step env",
+		ts: &v1.TaskSpec{
+			Steps: []v1.Step{{
+				Name:  "step-name",
+				Image: "step-image",
+				Env: []corev1.EnvVar{{
+					Name:  "path",
+					Value: "$(workspaces.env-ws.path)",
+				}},
+				Command: []string{"ls"},
+			}},
+		},
+		want: sets.NewString(
+			"env-ws",
+		),
+	}, {
+		name: "workspace used in step workingDir",
+		ts: &v1.TaskSpec{
+			Steps: []v1.Step{{
+				Name:       "step-name",
+				Image:      "step-image",
+				WorkingDir: "$(workspaces.shared.path)",
+				Command:    []string{"ls"},
+			}},
+		},
+		want: sets.NewString(
+			"shared",
+		),
+	}, {
+		name: "workspace used in step Params for stepactions",
+		ts: &v1.TaskSpec{
+			Steps: []v1.Step{{
+				Name: "step-name",
+				Params: []v1.Param{{
+					Name:  "path",
+					Value: *v1.NewStructuredValues("$(workspaces.shared.path)"),
+				}},
+				Ref: &v1.Ref{
+					Name: "step-action",
+				},
+			}},
+		},
+		want: sets.NewString(
+			"shared",
+		),
+	}, {
+		name: "workspace used in sidecar env",
+		ts: &v1.TaskSpec{
+			Sidecars: []v1.Sidecar{{
+				Name:  "step-name",
+				Image: "step-image",
+				Env: []corev1.EnvVar{{
+					Name:  "path",
+					Value: "$(workspaces.env-ws.path)",
+				}},
+				Command: []string{"ls"},
+			}},
+		},
+		want: sets.NewString(
+			"env-ws",
+		),
+	}, {
+		name: "workspace used in sidecar workingDir",
+		ts: &v1.TaskSpec{
+			Sidecars: []v1.Sidecar{{
+				Name:       "step-name",
+				Image:      "step-image",
+				WorkingDir: "$(workspaces.shared.path)",
+				Command:    []string{"ls"},
+			}},
+		},
+		want: sets.NewString(
+			"shared",
+		),
+	}, {
+		name: "workspace used in stepTemplate env",
+		ts: &v1.TaskSpec{
+			StepTemplate: &v1.StepTemplate{
+				Env: []corev1.EnvVar{{
+					Name:  "path",
+					Value: "$(workspaces.env-ws.path)",
+				}},
+			},
+		},
+		want: sets.NewString(
+			"env-ws",
+		),
+	}, {
+		name: "workspace used in stepTemplate workingDir",
+		ts: &v1.TaskSpec{
+			StepTemplate: &v1.StepTemplate{
+				WorkingDir: "$(workspaces.shared.path)",
+			},
+		},
+		want: sets.NewString(
+			"shared",
+		),
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {