Add validation for PR infinite timeouts

Prior to this commit, it was possible to create a PipelineRun with `timeouts.tasks` or `timeouts.finally`
set to 0 (no timeout) without setting the `timeouts.pipeline` to 0. If `timeouts.pipeline` is not set,
it defaults to the global timeout value, which is not 0. This would imply that `timeouts.tasks` or `timeouts.finally`
is longer than `timeouts.pipeline`, and it's not clear how Tekton should handle this.
Prior to this commit, Tekton in practice handles this situation by timing out the PipelineRun when `timeouts.pipeline`
is reached, but never timing out the TaskRun, which is clearly inconsistent behavior.

This commit results in an error returned to the user if they define a PipelineRun spec as described above,
and documents this behavior.

docs/pipelineruns.md

@@ -523,6 +523,7 @@ spec:
 
 If you do not specify the `timeout` value or `timeouts.pipeline` in the `PipelineRun`, the global default timeout value applies.
 If you set the `timeout` value or `timeouts.pipeline` to 0, the `PipelineRun` fails immediately upon encountering an error.
+If `timeouts.tasks` or `timeouts.finally` is set to 0, `timeouts.pipeline` must also be set to 0.
 
 The global default timeout is set to 60 minutes when you first install Tekton. You can set
 a different global default timeout value using the `default-timeout-minutes` field in

pkg/apis/pipeline/v1beta1/pipelinerun_validation.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/tektoncd/pipeline/pkg/apis/config"
+	apisconfig "github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/validate"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/pkg/apis"
@@ -169,12 +170,34 @@ func validateTimeoutDuration(field string, d *metav1.Duration) (errs *apis.Field
 }
 
 func (ps *PipelineRunSpec) validatePipelineTimeout(timeout time.Duration, errorMsg string) (errs *apis.FieldError) {
-	if ps.Timeouts.Tasks != nil && ps.Timeouts.Tasks.Duration > timeout {
-		errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s %s", ps.Timeouts.Tasks.Duration.String(), errorMsg), "timeouts.tasks"))
+	if ps.Timeouts.Tasks != nil {
+		tasksTimeoutErr := false
+		tasksTimeoutStr := ps.Timeouts.Tasks.Duration.String()
+		if ps.Timeouts.Tasks.Duration > timeout {
+			tasksTimeoutErr = true
+		}
+		if ps.Timeouts.Tasks.Duration == apisconfig.NoTimeoutDuration && timeout != apisconfig.NoTimeoutDuration {
+			tasksTimeoutErr = true
+			tasksTimeoutStr += " (no timeout)"
+		}
+		if tasksTimeoutErr {
+			errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s %s", tasksTimeoutStr, errorMsg), "timeouts.tasks"))
+		}
 	}
 
-	if ps.Timeouts.Finally != nil && ps.Timeouts.Finally.Duration > timeout {
-		errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s %s", ps.Timeouts.Finally.Duration.String(), errorMsg), "timeouts.finally"))
+	if ps.Timeouts.Finally != nil {
+		finallyTimeoutErr := false
+		finallyTimeoutStr := ps.Timeouts.Finally.Duration.String()
+		if ps.Timeouts.Finally.Duration > timeout {
+			finallyTimeoutErr = true
+		}
+		if ps.Timeouts.Finally.Duration == apisconfig.NoTimeoutDuration && timeout != apisconfig.NoTimeoutDuration {
+			finallyTimeoutErr = true
+			finallyTimeoutStr += " (no timeout)"
+		}
+		if finallyTimeoutErr {
+			errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s %s", finallyTimeoutStr, errorMsg), "timeouts.finally"))
+		}
 	}
 
 	if ps.Timeouts.Tasks != nil && ps.Timeouts.Finally != nil {

pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go

@@ -18,7 +18,6 @@ package v1beta1_test
 
 import (
 	"context"
-	"fmt"
 	"testing"
 	"time"
 
@@ -598,7 +597,77 @@ func TestPipelineRunWithAlphaFields_Invalid(t *testing.T) {
 				},
 			},
 		},
-		want: apis.ErrGeneric(fmt.Sprintf(`timeouts requires "enable-api-fields" feature gate to be "alpha" but it is "stable"`)),
+		want: apis.ErrGeneric(`timeouts requires "enable-api-fields" feature gate to be "alpha" but it is "stable"`),
+	}, {
+		name: "Tasks timeout = 0 but Pipeline timeout not set",
+		pr: v1beta1.PipelineRun{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "pipelinelinename",
+			},
+			Spec: v1beta1.PipelineRunSpec{
+				PipelineRef: &v1beta1.PipelineRef{
+					Name: "prname",
+				},
+				Timeouts: &v1beta1.TimeoutFields{
+					Tasks: &metav1.Duration{Duration: 0 * time.Minute},
+				},
+			},
+		},
+		wc:   enableAlphaAPIFields,
+		want: apis.ErrInvalidValue(`0s (no timeout) should be <= default timeout duration`, "spec.timeouts.tasks"),
+	}, {
+		name: "Tasks timeout = 0 but Pipeline timeout is not 0",
+		pr: v1beta1.PipelineRun{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "pipelinelinename",
+			},
+			Spec: v1beta1.PipelineRunSpec{
+				PipelineRef: &v1beta1.PipelineRef{
+					Name: "prname",
+				},
+				Timeouts: &v1beta1.TimeoutFields{
+					Pipeline: &metav1.Duration{Duration: 10 * time.Minute},
+					Tasks:    &metav1.Duration{Duration: 0 * time.Minute},
+				},
+			},
+		},
+		wc:   enableAlphaAPIFields,
+		want: apis.ErrInvalidValue(`0s (no timeout) should be <= pipeline duration`, "spec.timeouts.tasks"),
+	}, {
+		name: "Finally timeout = 0 but Pipeline timeout not set",
+		pr: v1beta1.PipelineRun{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "pipelinelinename",
+			},
+			Spec: v1beta1.PipelineRunSpec{
+				PipelineRef: &v1beta1.PipelineRef{
+					Name: "prname",
+				},
+				Timeouts: &v1beta1.TimeoutFields{
+					Finally: &metav1.Duration{Duration: 0 * time.Minute},
+				},
+			},
+		},
+		wc:   enableAlphaAPIFields,
+		want: apis.ErrInvalidValue(`0s (no timeout) should be <= default timeout duration`, "spec.timeouts.finally"),
+	}, {
+		name: "Finally timeout = 0 but Pipeline timeout is not 0",
+		pr: v1beta1.PipelineRun{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "pipelinelinename",
+			},
+			Spec: v1beta1.PipelineRunSpec{
+				PipelineRef: &v1beta1.PipelineRef{
+					Name: "prname",
+				},
+				Timeouts: &v1beta1.TimeoutFields{
+					Pipeline: &metav1.Duration{Duration: 10 * time.Minute},
+					Finally:  &metav1.Duration{Duration: 0 * time.Minute},
+				},
+			},
+		},
+		wc:   enableAlphaAPIFields,
+		want: apis.ErrInvalidValue(`0s (no timeout) should be <= pipeline duration`, "spec.timeouts.finally"),
 	}}
 
 	for _, tc := range tests {