From 610273bea09e43d638ee61cec8cb1406015b638f 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 e9bf1c17f6e..aafba9f99c0 100644
--- a/pkg/apis/pipeline/v1/taskrun_validation.go
+++ b/pkg/apis/pipeline/v1/taskrun_validation.go
@@ -216,6 +216,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(fmt.Sprintf("%s is not a valid onFailure breakpoint value, onFailure breakpoint is only allowed to be set as enabled", db.Breakpoints.OnFailure), "breakpoints.onFailure"))
 	}
diff --git a/pkg/apis/pipeline/v1/taskrun_validation_test.go b/pkg/apis/pipeline/v1/taskrun_validation_test.go
index e418256c042..5c153f8f23c 100644
--- a/pkg/apis/pipeline/v1/taskrun_validation_test.go
+++ b/pkg/apis/pipeline/v1/taskrun_validation_test.go
@@ -659,6 +659,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 alpha feature gate",
 		spec: v1.TaskRunSpec{
diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go
index e5108a6464c..a37d399e26c 100644
--- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go
+++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go
@@ -218,6 +218,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(fmt.Sprintf("%s is not a valid onFailure breakpoint value, onFailure breakpoint is only allowed to be set as enabled", db.Breakpoints.OnFailure), "breakpoints.onFailure"))
 	}
diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go
index 6e604fb6e36..3d56c014440 100644
--- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go
+++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go
@@ -647,6 +647,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: "stepOverride disallowed without alpha feature gate",
 		spec: v1beta1.TaskRunSpec{
diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go
index d63111db427..f1021d385f1 100644
--- a/pkg/pod/pod.go
+++ b/pkg/pod/pod.go
@@ -231,7 +231,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.