From f63359efb03705f795510551a2aba987bae1150d Mon Sep 17 00:00:00 2001 From: Alex Petrov Date: Sat, 1 Feb 2025 12:21:21 -0500 Subject: [PATCH] fix(pod/util): typos in getting pod validation options Before, containers with the PostStart sleep lifecycle hook would cause null pointer panics due to a typo in the field name being checked. This commit fixes that. The check also needs to be done on the oldPodSpec, rather than the podSpec, so that existing workloads which use the zero value continue functioning in the same way. --- pkg/api/pod/util.go | 4 +- pkg/api/pod/util_test.go | 89 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index a00c82cc083..06a2573fcd2 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -418,7 +418,7 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po } } - opts.AllowPodLifecycleSleepActionZeroValue = opts.AllowPodLifecycleSleepActionZeroValue || podLifecycleSleepActionZeroValueInUse(podSpec) + opts.AllowPodLifecycleSleepActionZeroValue = opts.AllowPodLifecycleSleepActionZeroValue || podLifecycleSleepActionZeroValueInUse(oldPodSpec) // If oldPod has resize policy set on the restartable init container, we must allow it opts.AllowSidecarResizePolicy = opts.AllowSidecarResizePolicy || hasRestartableInitContainerResizePolicy(oldPodSpec) } @@ -775,7 +775,7 @@ func podLifecycleSleepActionZeroValueInUse(podSpec *api.PodSpec) bool { inUse = true return false } - if c.Lifecycle.PostStart != nil && c.Lifecycle.PostStart.Sleep != nil && c.Lifecycle.PreStop.Sleep.Seconds == 0 { + if c.Lifecycle.PostStart != nil && c.Lifecycle.PostStart.Sleep != nil && c.Lifecycle.PostStart.Sleep.Seconds == 0 { inUse = true return false } diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index 81897f4c484..42678e29b8b 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -4657,3 +4657,92 @@ func TestValidateInvalidLabelValueInNodeSelectorOption(t *testing.T) { }) } } + +func TestValidateAllowPodLifecycleSleepActionZeroValue(t *testing.T) { + testCases := []struct { + name string + podSpec *api.PodSpec + expectAllowPodLifecycleSleepActionZeroValue bool + }{ + { + name: "no lifecycle hooks", + podSpec: &api.PodSpec{}, + expectAllowPodLifecycleSleepActionZeroValue: false, + }, + { + name: "Prestop with non-zero second duration", + podSpec: &api.PodSpec{ + Containers: []api.Container{ + { + Lifecycle: &api.Lifecycle{ + PreStop: &api.LifecycleHandler{ + Sleep: &api.SleepAction{ + Seconds: 1, + }, + }, + }, + }, + }, + }, + expectAllowPodLifecycleSleepActionZeroValue: false, + }, + { + name: "PostStart with non-zero second duration", + podSpec: &api.PodSpec{ + Containers: []api.Container{ + { + Lifecycle: &api.Lifecycle{ + PostStart: &api.LifecycleHandler{ + Sleep: &api.SleepAction{ + Seconds: 1, + }, + }, + }, + }, + }, + }, + expectAllowPodLifecycleSleepActionZeroValue: false, + }, + { + name: "PreStop with zero seconds", + podSpec: &api.PodSpec{ + Containers: []api.Container{ + { + Lifecycle: &api.Lifecycle{ + PreStop: &api.LifecycleHandler{ + Sleep: &api.SleepAction{ + Seconds: 0, + }, + }, + }, + }, + }, + }, + expectAllowPodLifecycleSleepActionZeroValue: true, + }, + { + name: "PostStart with zero seconds", + podSpec: &api.PodSpec{ + Containers: []api.Container{ + { + Lifecycle: &api.Lifecycle{ + PostStart: &api.LifecycleHandler{ + Sleep: &api.SleepAction{ + Seconds: 0, + }, + }, + }, + }, + }, + }, + expectAllowPodLifecycleSleepActionZeroValue: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gotOptions := GetValidationOptionsFromPodSpecAndMeta(&api.PodSpec{}, tc.podSpec, nil, nil) + assert.Equal(t, tc.expectAllowPodLifecycleSleepActionZeroValue, gotOptions.AllowPodLifecycleSleepActionZeroValue, "AllowPodLifecycleSleepActionZeroValue") + }) + } +}