Skip to content

Commit 0380f2c

Browse files
sreeram-venkiteshsreeram-venkitesh
committed
Validation
1 parent 70540c9 commit 0380f2c

File tree

5 files changed

+353
-14
lines changed

5 files changed

+353
-14
lines changed

Diff for: ‎pkg/api/pod/testing/make.go

+6
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,12 @@ func SetContainerImage(image string) TweakContainer {
270270
}
271271
}
272272

273+
func SetContainerLifecycle(lifecycle api.Lifecycle) TweakContainer {
274+
return func(cnr *api.Container) {
275+
cnr.Lifecycle = &lifecycle
276+
}
277+
}
278+
273279
func MakeResourceRequirements(requests, limits map[string]string) api.ResourceRequirements {
274280
rr := api.ResourceRequirements{Requests: api.ResourceList{}, Limits: api.ResourceList{}}
275281
for k, v := range requests {

Diff for: ‎pkg/api/pod/util.go

+48-3
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,51 @@ func dropDisabledFields(
678678
dropPodLifecycleSleepAction(podSpec, oldPodSpec)
679679
dropImageVolumes(podSpec, oldPodSpec)
680680
dropSELinuxChangePolicy(podSpec, oldPodSpec)
681+
dropContainerStopSignals(podSpec, oldPodSpec)
682+
}
683+
684+
func dropContainerStopSignals(podSpec, oldPodSpec *api.PodSpec) {
685+
if utilfeature.DefaultFeatureGate.Enabled(features.ContainerStopSignals) || containerStopSignalsInUse(oldPodSpec) {
686+
return
687+
}
688+
689+
wipeLifecycle := func(ctr *api.Container) {
690+
if ctr.Lifecycle == nil {
691+
return
692+
}
693+
if ctr.Lifecycle.StopSignal != nil {
694+
ctr.Lifecycle.StopSignal = nil
695+
if *ctr.Lifecycle == (api.Lifecycle{}) {
696+
ctr.Lifecycle = nil
697+
}
698+
}
699+
}
700+
701+
VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
702+
if c.Lifecycle == nil {
703+
return true
704+
}
705+
wipeLifecycle(c)
706+
return true
707+
})
708+
}
709+
710+
func containerStopSignalsInUse(podSpec *api.PodSpec) bool {
711+
if podSpec == nil {
712+
return false
713+
}
714+
var inUse bool
715+
VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
716+
if c.Lifecycle == nil {
717+
return true
718+
}
719+
if c.Lifecycle.StopSignal != nil {
720+
inUse = true
721+
return false
722+
}
723+
return true
724+
})
725+
return inUse
681726
}
682727

683728
func dropDisabledPodLevelResources(podSpec, oldPodSpec *api.PodSpec) {
@@ -713,7 +758,7 @@ func dropPodLifecycleSleepAction(podSpec, oldPodSpec *api.PodSpec) {
713758
continue
714759
}
715760
adjustLifecycle(podSpec.Containers[i].Lifecycle)
716-
if podSpec.Containers[i].Lifecycle.PreStop == nil && podSpec.Containers[i].Lifecycle.PostStart == nil {
761+
if podSpec.Containers[i].Lifecycle.PreStop == nil && podSpec.Containers[i].Lifecycle.PostStart == nil && podSpec.Containers[i].Lifecycle.StopSignal == nil {
717762
podSpec.Containers[i].Lifecycle = nil
718763
}
719764
}
@@ -723,7 +768,7 @@ func dropPodLifecycleSleepAction(podSpec, oldPodSpec *api.PodSpec) {
723768
continue
724769
}
725770
adjustLifecycle(podSpec.InitContainers[i].Lifecycle)
726-
if podSpec.InitContainers[i].Lifecycle.PreStop == nil && podSpec.InitContainers[i].Lifecycle.PostStart == nil {
771+
if podSpec.InitContainers[i].Lifecycle.PreStop == nil && podSpec.InitContainers[i].Lifecycle.PostStart == nil && podSpec.InitContainers[i].Lifecycle.StopSignal == nil {
727772
podSpec.InitContainers[i].Lifecycle = nil
728773
}
729774
}
@@ -733,7 +778,7 @@ func dropPodLifecycleSleepAction(podSpec, oldPodSpec *api.PodSpec) {
733778
continue
734779
}
735780
adjustLifecycle(podSpec.EphemeralContainers[i].Lifecycle)
736-
if podSpec.EphemeralContainers[i].Lifecycle.PreStop == nil && podSpec.EphemeralContainers[i].Lifecycle.PostStart == nil {
781+
if podSpec.EphemeralContainers[i].Lifecycle.PreStop == nil && podSpec.EphemeralContainers[i].Lifecycle.PostStart == nil && podSpec.EphemeralContainers[i].Lifecycle.StopSignal == nil {
737782
podSpec.EphemeralContainers[i].Lifecycle = nil
738783
}
739784
}

Diff for: ‎pkg/api/pod/util_test.go

+142
Original file line numberDiff line numberDiff line change
@@ -3436,6 +3436,148 @@ func TestDropPodLifecycleSleepAction(t *testing.T) {
34363436
}
34373437
}
34383438

3439+
func TestDropContainerStopSignals(t *testing.T) {
3440+
makeContainer := func(lifecycle *api.Lifecycle) api.Container {
3441+
container := api.Container{Name: "foo"}
3442+
if lifecycle != nil {
3443+
container.Lifecycle = lifecycle
3444+
}
3445+
return container
3446+
}
3447+
3448+
makeEphemeralContainer := func(lifecycle *api.Lifecycle) api.EphemeralContainer {
3449+
container := api.EphemeralContainer{
3450+
EphemeralContainerCommon: api.EphemeralContainerCommon{Name: "foo"},
3451+
}
3452+
if lifecycle != nil {
3453+
container.Lifecycle = lifecycle
3454+
}
3455+
return container
3456+
}
3457+
3458+
makePod := func(os api.OSName, containers []api.Container, initContainers []api.Container, ephemeralContainers []api.EphemeralContainer) *api.PodSpec {
3459+
return &api.PodSpec{
3460+
OS: &api.PodOS{Name: os},
3461+
Containers: containers,
3462+
InitContainers: initContainers,
3463+
EphemeralContainers: ephemeralContainers,
3464+
}
3465+
}
3466+
3467+
testCases := []struct {
3468+
featuregateEnabled bool
3469+
oldLifecycle *api.Lifecycle
3470+
newLifecycle *api.Lifecycle
3471+
expectedLifecycle *api.Lifecycle
3472+
}{
3473+
// feature gate is turned on and stopsignal is not in use - Lifecycle stays nil
3474+
{
3475+
featuregateEnabled: true,
3476+
oldLifecycle: nil,
3477+
newLifecycle: nil,
3478+
expectedLifecycle: nil,
3479+
},
3480+
// feature gate is turned off and StopSignal is in use - StopSignal is not dropped
3481+
{
3482+
featuregateEnabled: false,
3483+
oldLifecycle: &api.Lifecycle{StopSignal: ptr.To(api.SIGTERM)},
3484+
newLifecycle: &api.Lifecycle{StopSignal: ptr.To(api.SIGTERM)},
3485+
expectedLifecycle: &api.Lifecycle{StopSignal: ptr.To(api.SIGTERM)},
3486+
},
3487+
// feature gate is turned off and StopSignal is not in use - Entire lifecycle is dropped
3488+
{
3489+
featuregateEnabled: false,
3490+
oldLifecycle: &api.Lifecycle{StopSignal: nil},
3491+
newLifecycle: &api.Lifecycle{StopSignal: ptr.To(api.SIGTERM)},
3492+
expectedLifecycle: nil,
3493+
},
3494+
// feature gate is turned on and StopSignal is in use - StopSignal is not dropped
3495+
{
3496+
featuregateEnabled: true,
3497+
oldLifecycle: &api.Lifecycle{StopSignal: nil},
3498+
newLifecycle: &api.Lifecycle{StopSignal: ptr.To(api.SIGTERM)},
3499+
expectedLifecycle: &api.Lifecycle{StopSignal: ptr.To(api.SIGTERM)},
3500+
},
3501+
// feature gate is turned off and PreStop is in use - StopSignal alone is dropped
3502+
{
3503+
featuregateEnabled: false,
3504+
oldLifecycle: &api.Lifecycle{StopSignal: nil, PreStop: &api.LifecycleHandler{
3505+
Exec: &api.ExecAction{Command: []string{"foo"}},
3506+
}},
3507+
newLifecycle: &api.Lifecycle{StopSignal: ptr.To(api.SIGTERM), PreStop: &api.LifecycleHandler{
3508+
Exec: &api.ExecAction{Command: []string{"foo"}},
3509+
}},
3510+
expectedLifecycle: &api.Lifecycle{StopSignal: nil, PreStop: &api.LifecycleHandler{
3511+
Exec: &api.ExecAction{Command: []string{"foo"}},
3512+
}},
3513+
},
3514+
// feature gate is turned on and PreStop is in use - StopSignal is not dropped
3515+
{
3516+
featuregateEnabled: true,
3517+
oldLifecycle: &api.Lifecycle{StopSignal: nil, PreStop: &api.LifecycleHandler{
3518+
Exec: &api.ExecAction{Command: []string{"foo"}},
3519+
}},
3520+
newLifecycle: &api.Lifecycle{StopSignal: ptr.To(api.SIGTERM), PreStop: &api.LifecycleHandler{
3521+
Exec: &api.ExecAction{Command: []string{"foo"}},
3522+
}},
3523+
expectedLifecycle: &api.Lifecycle{StopSignal: ptr.To(api.SIGTERM), PreStop: &api.LifecycleHandler{
3524+
Exec: &api.ExecAction{Command: []string{"foo"}},
3525+
}},
3526+
},
3527+
// feature gate is turned off and PreStop and StopSignal are in use - nothing is dropped
3528+
{
3529+
featuregateEnabled: true,
3530+
oldLifecycle: &api.Lifecycle{StopSignal: ptr.To(api.SIGTERM), PreStop: &api.LifecycleHandler{
3531+
Exec: &api.ExecAction{Command: []string{"foo"}},
3532+
}},
3533+
newLifecycle: &api.Lifecycle{StopSignal: ptr.To(api.SIGTERM), PreStop: &api.LifecycleHandler{
3534+
Exec: &api.ExecAction{Command: []string{"foo"}},
3535+
}},
3536+
expectedLifecycle: &api.Lifecycle{StopSignal: ptr.To(api.SIGTERM), PreStop: &api.LifecycleHandler{
3537+
Exec: &api.ExecAction{Command: []string{"foo"}},
3538+
}},
3539+
},
3540+
}
3541+
3542+
for i, tc := range testCases {
3543+
t.Run(fmt.Sprintf("test_%d", i), func(t *testing.T) {
3544+
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ContainerStopSignals, tc.featuregateEnabled)
3545+
// Containers
3546+
{
3547+
oldPod := makePod(api.Linux, []api.Container{makeContainer(tc.oldLifecycle.DeepCopy())}, nil, nil)
3548+
newPod := makePod(api.Linux, []api.Container{makeContainer(tc.newLifecycle.DeepCopy())}, nil, nil)
3549+
expectedPod := makePod(api.Linux, []api.Container{makeContainer(tc.expectedLifecycle.DeepCopy())}, nil, nil)
3550+
dropDisabledFields(newPod, nil, oldPod, nil)
3551+
3552+
if diff := cmp.Diff(expectedPod, newPod); diff != "" {
3553+
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
3554+
}
3555+
}
3556+
// InitContainers
3557+
{
3558+
oldPod := makePod(api.Linux, nil, []api.Container{makeContainer(tc.oldLifecycle.DeepCopy())}, nil)
3559+
newPod := makePod(api.Linux, nil, []api.Container{makeContainer(tc.newLifecycle.DeepCopy())}, nil)
3560+
expectPod := makePod(api.Linux, nil, []api.Container{makeContainer(tc.expectedLifecycle.DeepCopy())}, nil)
3561+
dropDisabledFields(newPod, nil, oldPod, nil)
3562+
if diff := cmp.Diff(expectPod, newPod); diff != "" {
3563+
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
3564+
}
3565+
}
3566+
// EphemeralContainers
3567+
{
3568+
oldPod := makePod(api.Linux, nil, nil, []api.EphemeralContainer{makeEphemeralContainer(tc.oldLifecycle.DeepCopy())})
3569+
newPod := makePod(api.Linux, nil, nil, []api.EphemeralContainer{makeEphemeralContainer(tc.newLifecycle.DeepCopy())})
3570+
expectPod := makePod(api.Linux, nil, nil, []api.EphemeralContainer{makeEphemeralContainer(tc.expectedLifecycle.DeepCopy())})
3571+
dropDisabledFields(newPod, nil, oldPod, nil)
3572+
if diff := cmp.Diff(expectPod, newPod); diff != "" {
3573+
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
3574+
}
3575+
}
3576+
3577+
})
3578+
}
3579+
}
3580+
34393581
func TestDropSupplementalGroupsPolicy(t *testing.T) {
34403582
supplementalGroupsPolicyMerge := api.SupplementalGroupsPolicyMerge
34413583
podWithSupplementalGroupsPolicy := func() *api.Pod {

Diff for: ‎pkg/apis/core/validation/validation.go

+51-7
Original file line numberDiff line numberDiff line change
@@ -3343,14 +3343,58 @@ func validateHandler(handler commonHandler, gracePeriod *int64, fldPath *field.P
33433343
return allErrors
33443344
}
33453345

3346-
func validateLifecycle(lifecycle *core.Lifecycle, gracePeriod *int64, fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
3346+
var supportedStopSignalsLinux = sets.New(
3347+
core.SIGABRT, core.SIGALRM, core.SIGBUS, core.SIGCHLD,
3348+
core.SIGCLD, core.SIGCONT, core.SIGFPE, core.SIGHUP,
3349+
core.SIGILL, core.SIGINT, core.SIGIO, core.SIGIOT,
3350+
core.SIGKILL, core.SIGPIPE, core.SIGPOLL, core.SIGPROF,
3351+
core.SIGPWR, core.SIGQUIT, core.SIGSEGV, core.SIGSTKFLT,
3352+
core.SIGSTOP, core.SIGSYS, core.SIGTERM, core.SIGTRAP,
3353+
core.SIGTSTP, core.SIGTTIN, core.SIGTTOU, core.SIGURG,
3354+
core.SIGUSR1, core.SIGUSR2, core.SIGVTALRM, core.SIGWINCH,
3355+
core.SIGXCPU, core.SIGXFSZ, core.SIGRTMIN, core.SIGRTMINPLUS1,
3356+
core.SIGRTMINPLUS2, core.SIGRTMINPLUS3, core.SIGRTMINPLUS4,
3357+
core.SIGRTMINPLUS5, core.SIGRTMINPLUS6, core.SIGRTMINPLUS7,
3358+
core.SIGRTMINPLUS8, core.SIGRTMINPLUS9, core.SIGRTMINPLUS10,
3359+
core.SIGRTMINPLUS11, core.SIGRTMINPLUS12, core.SIGRTMINPLUS13,
3360+
core.SIGRTMINPLUS14, core.SIGRTMINPLUS15, core.SIGRTMAXMINUS14,
3361+
core.SIGRTMAXMINUS13, core.SIGRTMAXMINUS12, core.SIGRTMAXMINUS11,
3362+
core.SIGRTMAXMINUS10, core.SIGRTMAXMINUS9, core.SIGRTMAXMINUS8,
3363+
core.SIGRTMAXMINUS7, core.SIGRTMAXMINUS6, core.SIGRTMAXMINUS5,
3364+
core.SIGRTMAXMINUS4, core.SIGRTMAXMINUS3, core.SIGRTMAXMINUS2,
3365+
core.SIGRTMAXMINUS1, core.SIGRTMAX)
3366+
3367+
var supportedStopSignalsWindows = sets.New(core.SIGKILL, core.SIGTERM)
3368+
3369+
func validateStopSignal(stopSignal *core.Signal, fldPath *field.Path, os *core.PodOS) field.ErrorList {
3370+
allErrors := field.ErrorList{}
3371+
3372+
if os == nil {
3373+
allErrors = append(allErrors, field.Forbidden(fldPath, "may not be set for containers with empty `spec.os.name`"))
3374+
} else if os.Name == core.Windows {
3375+
if !supportedStopSignalsWindows.Has(*stopSignal) {
3376+
allErrors = append(allErrors, field.NotSupported(fldPath, stopSignal, sets.List(supportedStopSignalsWindows)))
3377+
}
3378+
} else if os.Name == core.Linux {
3379+
if !supportedStopSignalsLinux.Has(*stopSignal) {
3380+
allErrors = append(allErrors, field.NotSupported(fldPath, stopSignal, sets.List(supportedStopSignalsLinux)))
3381+
}
3382+
}
3383+
3384+
return allErrors
3385+
}
3386+
3387+
func validateLifecycle(lifecycle *core.Lifecycle, gracePeriod *int64, fldPath *field.Path, opts PodValidationOptions, os *core.PodOS) field.ErrorList {
33473388
allErrs := field.ErrorList{}
33483389
if lifecycle.PostStart != nil {
33493390
allErrs = append(allErrs, validateHandler(handlerFromLifecycle(lifecycle.PostStart), gracePeriod, fldPath.Child("postStart"), opts)...)
33503391
}
33513392
if lifecycle.PreStop != nil {
33523393
allErrs = append(allErrs, validateHandler(handlerFromLifecycle(lifecycle.PreStop), gracePeriod, fldPath.Child("preStop"), opts)...)
33533394
}
3395+
if lifecycle.StopSignal != nil {
3396+
allErrs = append(allErrs, validateStopSignal(lifecycle.StopSignal, fldPath.Child("stopSignal"), os)...)
3397+
}
33543398
return allErrs
33553399
}
33563400

@@ -3494,7 +3538,7 @@ func validateFieldAllowList(value interface{}, allowedFields map[string]bool, er
34943538
}
34953539

34963540
// validateInitContainers is called by pod spec and template validation to validate the list of init containers
3497-
func validateInitContainers(containers []core.Container, regularContainers []core.Container, volumes map[string]core.VolumeSource, podClaimNames sets.Set[string], gracePeriod *int64, fldPath *field.Path, opts PodValidationOptions, podRestartPolicy *core.RestartPolicy, hostUsers bool) field.ErrorList {
3541+
func validateInitContainers(containers []core.Container, os *core.PodOS, regularContainers []core.Container, volumes map[string]core.VolumeSource, podClaimNames sets.Set[string], gracePeriod *int64, fldPath *field.Path, opts PodValidationOptions, podRestartPolicy *core.RestartPolicy, hostUsers bool) field.ErrorList {
34983542
var allErrs field.ErrorList
34993543

35003544
allNames := sets.Set[string]{}
@@ -3528,7 +3572,7 @@ func validateInitContainers(containers []core.Container, regularContainers []cor
35283572
switch {
35293573
case restartAlways:
35303574
if ctr.Lifecycle != nil {
3531-
allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, gracePeriod, idxPath.Child("lifecycle"), opts)...)
3575+
allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, gracePeriod, idxPath.Child("lifecycle"), opts, os)...)
35323576
}
35333577
allErrs = append(allErrs, validateLivenessProbe(ctr.LivenessProbe, gracePeriod, idxPath.Child("livenessProbe"), opts)...)
35343578
allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, gracePeriod, idxPath.Child("readinessProbe"), opts)...)
@@ -3632,7 +3676,7 @@ func validateHostUsers(spec *core.PodSpec, fldPath *field.Path) field.ErrorList
36323676
}
36333677

36343678
// validateContainers is called by pod spec and template validation to validate the list of regular containers.
3635-
func validateContainers(containers []core.Container, volumes map[string]core.VolumeSource, podClaimNames sets.Set[string], gracePeriod *int64, fldPath *field.Path, opts PodValidationOptions, podRestartPolicy *core.RestartPolicy, hostUsers bool) field.ErrorList {
3679+
func validateContainers(containers []core.Container, os *core.PodOS, volumes map[string]core.VolumeSource, podClaimNames sets.Set[string], gracePeriod *int64, fldPath *field.Path, opts PodValidationOptions, podRestartPolicy *core.RestartPolicy, hostUsers bool) field.ErrorList {
36363680
allErrs := field.ErrorList{}
36373681

36383682
if len(containers) == 0 {
@@ -3660,7 +3704,7 @@ func validateContainers(containers []core.Container, volumes map[string]core.Vol
36603704
// Regular init container and ephemeral container validation will return
36613705
// field.Forbidden() for these paths.
36623706
if ctr.Lifecycle != nil {
3663-
allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, gracePeriod, path.Child("lifecycle"), opts)...)
3707+
allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, gracePeriod, path.Child("lifecycle"), opts, os)...)
36643708
}
36653709
allErrs = append(allErrs, validateLivenessProbe(ctr.LivenessProbe, gracePeriod, path.Child("livenessProbe"), opts)...)
36663710
allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, gracePeriod, path.Child("readinessProbe"), opts)...)
@@ -4207,8 +4251,8 @@ func ValidatePodSpec(spec *core.PodSpec, podMeta *metav1.ObjectMeta, fldPath *fi
42074251
allErrs = append(allErrs, vErrs...)
42084252
podClaimNames := gatherPodResourceClaimNames(spec.ResourceClaims)
42094253
allErrs = append(allErrs, validatePodResourceClaims(podMeta, spec.ResourceClaims, fldPath.Child("resourceClaims"))...)
4210-
allErrs = append(allErrs, validateContainers(spec.Containers, vols, podClaimNames, gracePeriod, fldPath.Child("containers"), opts, &spec.RestartPolicy, hostUsers)...)
4211-
allErrs = append(allErrs, validateInitContainers(spec.InitContainers, spec.Containers, vols, podClaimNames, gracePeriod, fldPath.Child("initContainers"), opts, &spec.RestartPolicy, hostUsers)...)
4254+
allErrs = append(allErrs, validateContainers(spec.Containers, spec.OS, vols, podClaimNames, gracePeriod, fldPath.Child("containers"), opts, &spec.RestartPolicy, hostUsers)...)
4255+
allErrs = append(allErrs, validateInitContainers(spec.InitContainers, spec.OS, spec.Containers, vols, podClaimNames, gracePeriod, fldPath.Child("initContainers"), opts, &spec.RestartPolicy, hostUsers)...)
42124256
allErrs = append(allErrs, validateEphemeralContainers(spec.EphemeralContainers, spec.Containers, spec.InitContainers, vols, podClaimNames, fldPath.Child("ephemeralContainers"), opts, &spec.RestartPolicy, hostUsers)...)
42134257

42144258
if opts.PodLevelResourcesEnabled {

0 commit comments

Comments
 (0)