Fix Timeouts Default in v1 PipelienRun

This commit fixes the SetDefault for v1 pipelinerun.timeouts. Previously it would reset pipelinerun.timeouts fields with only timeouts.pipeline while it should have kept timeouts.tasks and timeouts.finally as expected.
tektoncd · Apr 17, 2023 · aa56ab3 · aa56ab3
1 parent 048bf9a
commit aa56ab3
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 1 deletion.
diff --git a/pkg/apis/pipeline/v1/pipelinerun_defaults.go b/pkg/apis/pipeline/v1/pipelinerun_defaults.go
@@ -37,7 +37,11 @@ func (pr *PipelineRun) SetDefaults(ctx context.Context) {
 func (prs *PipelineRunSpec) SetDefaults(ctx context.Context) {
 	cfg := config.FromContextOrDefaults(ctx)
 
-	if prs.Timeouts != nil && prs.Timeouts.Pipeline == nil {
+	if prs.Timeouts == nil {
+		prs.Timeouts = &TimeoutFields{}
+	}
+
+	if prs.Timeouts.Pipeline == nil {
 		prs.Timeouts.Pipeline = &metav1.Duration{Duration: time.Duration(cfg.Defaults.DefaultTimeoutMinutes) * time.Minute}
 	}
 

diff --git a/pkg/apis/pipeline/v1/pipelinerun_defaults_test.go b/pkg/apis/pipeline/v1/pipelinerun_defaults_test.go
@@ -19,6 +19,7 @@ package v1_test
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -37,6 +38,67 @@ func TestPipelineRunSpec_SetDefaults(t *testing.T) {
 		prs  *v1.PipelineRunSpec
 		want *v1.PipelineRunSpec
 	}{
+		{
+			desc: "timeouts is nil",
+			prs:  &v1.PipelineRunSpec{},
+			want: &v1.PipelineRunSpec{
+				TaskRunTemplate: v1.PipelineTaskRunTemplate{
+					ServiceAccountName: config.DefaultServiceAccountValue,
+				},
+				Timeouts: &v1.TimeoutFields{
+					Pipeline: &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute},
+				},
+			},
+		},
+		{
+			desc: "timeouts is not nil",
+			prs: &v1.PipelineRunSpec{
+				Timeouts: &v1.TimeoutFields{},
+			},
+			want: &v1.PipelineRunSpec{
+				TaskRunTemplate: v1.PipelineTaskRunTemplate{
+					ServiceAccountName: config.DefaultServiceAccountValue,
+				},
+				Timeouts: &v1.TimeoutFields{
+					Pipeline: &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute},
+				},
+			},
+		},
+		{
+			desc: "timeouts.pipeline is not nil",
+			prs: &v1.PipelineRunSpec{
+				Timeouts: &v1.TimeoutFields{
+					Pipeline: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes + 1) * time.Minute},
+				},
+			},
+			want: &v1.PipelineRunSpec{
+				TaskRunTemplate: v1.PipelineTaskRunTemplate{
+					ServiceAccountName: config.DefaultServiceAccountValue,
+				},
+				Timeouts: &v1.TimeoutFields{
+					Pipeline: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes + 1) * time.Minute},
+				},
+			},
+		},
+		{
+			desc: "timeouts.pipeline is nil with timeouts.tasks and timeouts.finally",
+			prs: &v1.PipelineRunSpec{
+				Timeouts: &v1.TimeoutFields{
+					Tasks:   &metav1.Duration{Duration: (config.DefaultTimeoutMinutes + 1) * time.Minute},
+					Finally: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes + 1) * time.Minute},
+				},
+			},
+			want: &v1.PipelineRunSpec{
+				TaskRunTemplate: v1.PipelineTaskRunTemplate{
+					ServiceAccountName: config.DefaultServiceAccountValue,
+				},
+				Timeouts: &v1.TimeoutFields{
+					Pipeline: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes) * time.Minute},
+					Tasks:    &metav1.Duration{Duration: (config.DefaultTimeoutMinutes + 1) * time.Minute},
+					Finally:  &metav1.Duration{Duration: (config.DefaultTimeoutMinutes + 1) * time.Minute},
+				},
+			},
+		},
 		{
 			desc: "pod template is nil",
 			prs:  &v1.PipelineRunSpec{},