From 0f6c9733bd5fd9c2db852d47ea3f4180855ade72 Mon Sep 17 00:00:00 2001
From: Kewei Yang <cug.ykw@gmail.com>
Date: Thu, 21 Mar 2024 14:19:22 +0800
Subject: [PATCH] fix:when debug.breakpoints.onFailure is an empty string,
 redundant volumes appear

---
 pkg/apis/pipeline/v1/taskrun_validation.go         |  5 +++++
 pkg/apis/pipeline/v1/taskrun_validation_test.go    | 14 ++++++++++++++
 pkg/apis/pipeline/v1beta1/taskrun_validation.go    |  5 +++++
 .../pipeline/v1beta1/taskrun_validation_test.go    | 14 ++++++++++++++
 pkg/pod/pod.go                                     |  2 +-
 5 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/pkg/apis/pipeline/v1/taskrun_validation.go b/pkg/apis/pipeline/v1/taskrun_validation.go
index f2aafa23f74..ef4dcb5b6f5 100644
--- a/pkg/apis/pipeline/v1/taskrun_validation.go
+++ b/pkg/apis/pipeline/v1/taskrun_validation.go
@@ -224,6 +224,11 @@ func validateDebug(db *TaskRunDebug) (errs *apis.FieldError) {
 	if db == nil || db.Breakpoints == nil {
 		return errs
 	}
+
+	if db.Breakpoints.OnFailure == "" {
+		errs = errs.Also(apis.ErrInvalidValue("onFailure breakpoint is empty, it is only allowed to be set as enabled", "breakpoints.onFailure"))
+	}
+
 	if db.Breakpoints.OnFailure != "" && db.Breakpoints.OnFailure != EnabledOnFailureBreakpoint {
 		errs = errs.Also(apis.ErrInvalidValue(db.Breakpoints.OnFailure+" is not a valid onFailure breakpoint value, onFailure breakpoint is only allowed to be set as enabled", "breakpoints.onFailure"))
 	}
diff --git a/pkg/apis/pipeline/v1/taskrun_validation_test.go b/pkg/apis/pipeline/v1/taskrun_validation_test.go
index 33689e325ec..4241e27fe86 100644
--- a/pkg/apis/pipeline/v1/taskrun_validation_test.go
+++ b/pkg/apis/pipeline/v1/taskrun_validation_test.go
@@ -703,6 +703,20 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
 		},
 		wantErr: apis.ErrInvalidValue("turnOn is not a valid onFailure breakpoint value, onFailure breakpoint is only allowed to be set as enabled", "debug.breakpoints.onFailure"),
 		wc:      cfgtesting.EnableAlphaAPIFields,
+	}, {
+		name: "empty onFailure breakpoint",
+		spec: v1.TaskRunSpec{
+			TaskRef: &v1.TaskRef{
+				Name: "my-task",
+			},
+			Debug: &v1.TaskRunDebug{
+				Breakpoints: &v1.TaskBreakpoints{
+					OnFailure: "",
+				},
+			},
+		},
+		wantErr: apis.ErrInvalidValue("onFailure breakpoint is empty, it is only allowed to be set as enabled", "debug.breakpoints.onFailure"),
+		wc:      cfgtesting.EnableAlphaAPIFields,
 	}, {
 		name: "stepSpecs disallowed without beta feature gate",
 		spec: v1.TaskRunSpec{
diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go
index a82996439f7..bc0a2639875 100644
--- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go
+++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go
@@ -224,6 +224,11 @@ func validateDebug(db *TaskRunDebug) (errs *apis.FieldError) {
 	if db == nil || db.Breakpoints == nil {
 		return errs
 	}
+
+	if db.Breakpoints.OnFailure == "" {
+		errs = errs.Also(apis.ErrInvalidValue("onFailure breakpoint is empty, it is only allowed to be set as enabled", "breakpoints.onFailure"))
+	}
+
 	if db.Breakpoints.OnFailure != "" && db.Breakpoints.OnFailure != EnabledOnFailureBreakpoint {
 		errs = errs.Also(apis.ErrInvalidValue(db.Breakpoints.OnFailure+" is not a valid onFailure breakpoint value, onFailure breakpoint is only allowed to be set as enabled", "breakpoints.onFailure"))
 	}
diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go
index 9bbf77036b9..f594f420401 100644
--- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go
+++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go
@@ -698,6 +698,20 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
 		},
 		wantErr: apis.ErrInvalidValue("turnOn is not a valid onFailure breakpoint value, onFailure breakpoint is only allowed to be set as enabled", "debug.breakpoints.onFailure"),
 		wc:      cfgtesting.EnableAlphaAPIFields,
+	}, {
+		name: "empty onFailure breakpoint",
+		spec: v1beta1.TaskRunSpec{
+			TaskRef: &v1beta1.TaskRef{
+				Name: "my-task",
+			},
+			Debug: &v1beta1.TaskRunDebug{
+				Breakpoints: &v1beta1.TaskBreakpoints{
+					OnFailure: "",
+				},
+			},
+		},
+		wantErr: apis.ErrInvalidValue("onFailure breakpoint is empty, it is only allowed to be set as enabled", "debug.breakpoints.onFailure"),
+		wc:      cfgtesting.EnableAlphaAPIFields,
 	}, {
 		name: "duplicate stepOverride names",
 		spec: v1beta1.TaskRunSpec{
diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go
index 8e808cb6abc..792e2143e11 100644
--- a/pkg/pod/pod.go
+++ b/pkg/pod/pod.go
@@ -241,7 +241,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta
 		initContainers = append(initContainers, *scriptsInit)
 		volumes = append(volumes, scriptsVolume)
 	}
-	if alphaAPIEnabled && taskRun.Spec.Debug != nil {
+	if alphaAPIEnabled && taskRun.Spec.Debug != nil && taskRun.Spec.Debug.NeedsDebug() {
 		volumes = append(volumes, debugScriptsVolume, debugInfoVolume)
 	}
 	// Initialize any workingDirs under /workspace.