Skip to content

Commit 61e4167

Browse files
vdemeestertekton-robot
authored andcommitted
Fixing nil pointer in case of no timeout ⏲
If `pr.Spec.Timeout` is nil (aka no timeout applied), dereferencing it (`*pr.Spec.Timout`) will panic. Signed-off-by: Vincent Demeester <[email protected]> (cherry picked from commit d3bf694)
1 parent 4e23d50 commit 61e4167

File tree

2 files changed

+18
-8
lines changed

2 files changed

+18
-8
lines changed

pkg/reconciler/pipelinerun/pipelinerun.go

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,9 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun)
143143
logger.Warnf("PipelineRun %s createTimestamp %s is after the pipelineRun started %s", pr.GetNamespacedName().String(), pr.CreationTimestamp, pr.Status.StartTime)
144144
pr.Status.StartTime = &pr.CreationTimestamp
145145
}
146+
146147
// start goroutine to track pipelinerun timeout only startTime is not set
147-
go c.timeoutHandler.Wait(pr.GetNamespacedName(), *pr.Status.StartTime, *pr.Spec.Timeout)
148+
go c.timeoutHandler.Wait(pr.GetNamespacedName(), *pr.Status.StartTime, getPipelineRunTimeout(ctx, pr))
148149
// Emit events. During the first reconcile the status of the PipelineRun may change twice
149150
// from not Started to Started and then to Running, so we need to sent the event here
150151
// and at the end of 'Reconcile' again.
@@ -534,7 +535,7 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1beta1.Pip
534535
}
535536
} else if !rprt.ResolvedConditionChecks.HasStarted() {
536537
for _, rcc := range rprt.ResolvedConditionChecks {
537-
rcc.ConditionCheck, err = c.makeConditionCheckContainer(rprt, rcc, pr)
538+
rcc.ConditionCheck, err = c.makeConditionCheckContainer(ctx, rprt, rcc, pr)
538539
if err != nil {
539540
recorder.Eventf(pr, corev1.EventTypeWarning, "ConditionCheckCreationFailed", "Failed to create TaskRun %q: %v", rcc.ConditionCheckName, err)
540541
return fmt.Errorf("error creating ConditionCheck container called %s for PipelineTask %s from PipelineRun %s: %w", rcc.ConditionCheckName, rprt.PipelineTask.Name, pr.Name, err)
@@ -669,7 +670,7 @@ func (c *Reconciler) createTaskRun(ctx context.Context, rprt *resources.Resolved
669670
Spec: v1beta1.TaskRunSpec{
670671
Params: rprt.PipelineTask.Params,
671672
ServiceAccountName: serviceAccountName,
672-
Timeout: getTaskRunTimeout(pr, rprt),
673+
Timeout: getTaskRunTimeout(ctx, pr, rprt),
673674
PodTemplate: podTemplate,
674675
}}
675676

@@ -818,12 +819,21 @@ func combineTaskRunAndTaskSpecAnnotations(pr *v1beta1.PipelineRun, pipelineTask
818819
return annotations
819820
}
820821

821-
func getTaskRunTimeout(pr *v1beta1.PipelineRun, rprt *resources.ResolvedPipelineRunTask) *metav1.Duration {
822+
func getPipelineRunTimeout(ctx context.Context, pr *v1beta1.PipelineRun) metav1.Duration {
823+
if pr.Spec.Timeout == nil {
824+
defaultTimeout := time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes)
825+
return metav1.Duration{Duration: defaultTimeout * time.Minute}
826+
}
827+
return *pr.Spec.Timeout
828+
}
829+
830+
func getTaskRunTimeout(ctx context.Context, pr *v1beta1.PipelineRun, rprt *resources.ResolvedPipelineRunTask) *metav1.Duration {
822831
var taskRunTimeout = &metav1.Duration{Duration: apisconfig.NoTimeoutDuration}
823832

824833
var timeout time.Duration
825834
if pr.Spec.Timeout == nil {
826-
timeout = config.DefaultTimeoutMinutes * time.Minute
835+
defaultTimeout := time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes)
836+
timeout = defaultTimeout * time.Minute
827837
} else {
828838
timeout = pr.Spec.Timeout.Duration
829839
}
@@ -874,7 +884,7 @@ func (c *Reconciler) updateLabelsAndAnnotations(pr *v1beta1.PipelineRun) (*v1bet
874884
return newPr, nil
875885
}
876886

877-
func (c *Reconciler) makeConditionCheckContainer(rprt *resources.ResolvedPipelineRunTask, rcc *resources.ResolvedConditionCheck, pr *v1beta1.PipelineRun) (*v1beta1.ConditionCheck, error) {
887+
func (c *Reconciler) makeConditionCheckContainer(ctx context.Context, rprt *resources.ResolvedPipelineRunTask, rcc *resources.ResolvedConditionCheck, pr *v1beta1.PipelineRun) (*v1beta1.ConditionCheck, error) {
878888
labels := getTaskrunLabels(pr, rprt.PipelineTask.Name)
879889
labels[pipeline.GroupName+pipeline.ConditionCheckKey] = rcc.ConditionCheckName
880890
labels[pipeline.GroupName+pipeline.ConditionNameKey] = rcc.Condition.Name
@@ -910,7 +920,7 @@ func (c *Reconciler) makeConditionCheckContainer(rprt *resources.ResolvedPipelin
910920
Resources: &v1beta1.TaskRunResources{
911921
Inputs: rcc.ToTaskResourceBindings(),
912922
},
913-
Timeout: getTaskRunTimeout(pr, rprt),
923+
Timeout: getTaskRunTimeout(ctx, pr, rprt),
914924
PodTemplate: podTemplate,
915925
}}
916926

pkg/reconciler/pipelinerun/pipelinerun_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1677,7 +1677,7 @@ func TestGetTaskRunTimeout(t *testing.T) {
16771677

16781678
for _, tc := range tcs {
16791679
t.Run(tc.name, func(t *testing.T) {
1680-
if d := cmp.Diff(getTaskRunTimeout(tc.pr, tc.rprt), tc.expected); d != "" {
1680+
if d := cmp.Diff(getTaskRunTimeout(context.TODO(), tc.pr, tc.rprt), tc.expected); d != "" {
16811681
t.Errorf("Unexpected task run timeout. Diff %s", diff.PrintWantGot(d))
16821682
}
16831683
})

0 commit comments

Comments
 (0)