From fa01dfdb0a4b1627db65be0a156172ee2021eedb Mon Sep 17 00:00:00 2001 From: AxeZhan Date: Fri, 18 Aug 2023 13:35:57 +0800 Subject: [PATCH] validation --- pkg/apis/core/validation/validation.go | 73 ++++++++++++++------- pkg/apis/core/validation/validation_test.go | 73 ++++++++++++++++++--- 2 files changed, 113 insertions(+), 33 deletions(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index dadda11cd54..94978ee4452 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -2886,52 +2886,52 @@ func validatePodResourceClaimSource(claimSource core.ClaimSource, fldPath *field return allErrs } -func validateLivenessProbe(probe *core.Probe, fldPath *field.Path) field.ErrorList { +func validateLivenessProbe(probe *core.Probe, gracePeriod int64, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if probe == nil { return allErrs } - allErrs = append(allErrs, validateProbe(probe, fldPath)...) + allErrs = append(allErrs, validateProbe(probe, gracePeriod, fldPath)...) if probe.SuccessThreshold != 1 { allErrs = append(allErrs, field.Invalid(fldPath.Child("successThreshold"), probe.SuccessThreshold, "must be 1")) } return allErrs } -func validateReadinessProbe(probe *core.Probe, fldPath *field.Path) field.ErrorList { +func validateReadinessProbe(probe *core.Probe, gracePeriod int64, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if probe == nil { return allErrs } - allErrs = append(allErrs, validateProbe(probe, fldPath)...) + allErrs = append(allErrs, validateProbe(probe, gracePeriod, fldPath)...) if probe.TerminationGracePeriodSeconds != nil { allErrs = append(allErrs, field.Invalid(fldPath.Child("terminationGracePeriodSeconds"), probe.TerminationGracePeriodSeconds, "must not be set for readinessProbes")) } return allErrs } -func validateStartupProbe(probe *core.Probe, fldPath *field.Path) field.ErrorList { +func validateStartupProbe(probe *core.Probe, gracePeriod int64, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if probe == nil { return allErrs } - allErrs = append(allErrs, validateProbe(probe, fldPath)...) + allErrs = append(allErrs, validateProbe(probe, gracePeriod, fldPath)...) if probe.SuccessThreshold != 1 { allErrs = append(allErrs, field.Invalid(fldPath.Child("successThreshold"), probe.SuccessThreshold, "must be 1")) } return allErrs } -func validateProbe(probe *core.Probe, fldPath *field.Path) field.ErrorList { +func validateProbe(probe *core.Probe, gracePeriod int64, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if probe == nil { return allErrs } - allErrs = append(allErrs, validateHandler(handlerFromProbe(&probe.ProbeHandler), fldPath)...) + allErrs = append(allErrs, validateHandler(handlerFromProbe(&probe.ProbeHandler), gracePeriod, fldPath)...) allErrs = append(allErrs, ValidateNonnegativeField(int64(probe.InitialDelaySeconds), fldPath.Child("initialDelaySeconds"))...) allErrs = append(allErrs, ValidateNonnegativeField(int64(probe.TimeoutSeconds), fldPath.Child("timeoutSeconds"))...) @@ -2966,6 +2966,7 @@ type commonHandler struct { HTTPGet *core.HTTPGetAction TCPSocket *core.TCPSocketAction GRPC *core.GRPCAction + Sleep *core.SleepAction } func handlerFromProbe(ph *core.ProbeHandler) commonHandler { @@ -2982,9 +2983,19 @@ func handlerFromLifecycle(lh *core.LifecycleHandler) commonHandler { Exec: lh.Exec, HTTPGet: lh.HTTPGet, TCPSocket: lh.TCPSocket, + Sleep: lh.Sleep, } } +func validateSleepAction(sleep *core.SleepAction, gracePeriod int64, fldPath *field.Path) field.ErrorList { + allErrors := field.ErrorList{} + if sleep.Seconds <= 0 || sleep.Seconds > gracePeriod { + invalidStr := fmt.Sprintf("must be greater than 0 and less than terminationGracePeriodSeconds (%d)", gracePeriod) + allErrors = append(allErrors, field.Invalid(fldPath, sleep.Seconds, invalidStr)) + } + return allErrors +} + func validateClientIPAffinityConfig(config *core.SessionAffinityConfig, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if config == nil { @@ -3093,7 +3104,7 @@ func validateTCPSocketAction(tcp *core.TCPSocketAction, fldPath *field.Path) fie func validateGRPCAction(grpc *core.GRPCAction, fldPath *field.Path) field.ErrorList { return ValidatePortNumOrName(intstr.FromInt32(grpc.Port), fldPath.Child("port")) } -func validateHandler(handler commonHandler, fldPath *field.Path) field.ErrorList { +func validateHandler(handler commonHandler, gracePeriod int64, fldPath *field.Path) field.ErrorList { numHandlers := 0 allErrors := field.ErrorList{} if handler.Exec != nil { @@ -3128,19 +3139,27 @@ func validateHandler(handler commonHandler, fldPath *field.Path) field.ErrorList allErrors = append(allErrors, validateGRPCAction(handler.GRPC, fldPath.Child("grpc"))...) } } + if handler.Sleep != nil { + if numHandlers > 0 { + allErrors = append(allErrors, field.Forbidden(fldPath.Child("sleep"), "may not specify more than 1 handler type")) + } else { + numHandlers++ + allErrors = append(allErrors, validateSleepAction(handler.Sleep, gracePeriod, fldPath.Child("sleep"))...) + } + } if numHandlers == 0 { allErrors = append(allErrors, field.Required(fldPath, "must specify a handler type")) } return allErrors } -func validateLifecycle(lifecycle *core.Lifecycle, fldPath *field.Path) field.ErrorList { +func validateLifecycle(lifecycle *core.Lifecycle, gracePeriod int64, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if lifecycle.PostStart != nil { - allErrs = append(allErrs, validateHandler(handlerFromLifecycle(lifecycle.PostStart), fldPath.Child("postStart"))...) + allErrs = append(allErrs, validateHandler(handlerFromLifecycle(lifecycle.PostStart), gracePeriod, fldPath.Child("postStart"))...) } if lifecycle.PreStop != nil { - allErrs = append(allErrs, validateHandler(handlerFromLifecycle(lifecycle.PreStop), fldPath.Child("preStop"))...) + allErrs = append(allErrs, validateHandler(handlerFromLifecycle(lifecycle.PreStop), gracePeriod, fldPath.Child("preStop"))...) } return allErrs } @@ -3282,7 +3301,7 @@ func validateFieldAllowList(value interface{}, allowedFields map[string]bool, er } // validateInitContainers is called by pod spec and template validation to validate the list of init containers -func validateInitContainers(containers []core.Container, regularContainers []core.Container, volumes map[string]core.VolumeSource, podClaimNames sets.String, fldPath *field.Path, opts PodValidationOptions, podRestartPolicy *core.RestartPolicy) field.ErrorList { +func validateInitContainers(containers []core.Container, regularContainers []core.Container, volumes map[string]core.VolumeSource, podClaimNames sets.String, gracePeriod int64, fldPath *field.Path, opts PodValidationOptions, podRestartPolicy *core.RestartPolicy) field.ErrorList { var allErrs field.ErrorList allNames := sets.String{} @@ -3316,11 +3335,11 @@ func validateInitContainers(containers []core.Container, regularContainers []cor switch { case restartAlways: if ctr.Lifecycle != nil { - allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, idxPath.Child("lifecycle"))...) + allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, gracePeriod, idxPath.Child("lifecycle"))...) } - allErrs = append(allErrs, validateLivenessProbe(ctr.LivenessProbe, idxPath.Child("livenessProbe"))...) - allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, idxPath.Child("readinessProbe"))...) - allErrs = append(allErrs, validateStartupProbe(ctr.StartupProbe, idxPath.Child("startupProbe"))...) + allErrs = append(allErrs, validateLivenessProbe(ctr.LivenessProbe, gracePeriod, idxPath.Child("livenessProbe"))...) + allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, gracePeriod, idxPath.Child("readinessProbe"))...) + allErrs = append(allErrs, validateStartupProbe(ctr.StartupProbe, gracePeriod, idxPath.Child("startupProbe"))...) default: // These fields are disallowed for init containers. @@ -3420,7 +3439,7 @@ func validateHostUsers(spec *core.PodSpec, fldPath *field.Path) field.ErrorList } // validateContainers is called by pod spec and template validation to validate the list of regular containers. -func validateContainers(containers []core.Container, volumes map[string]core.VolumeSource, podClaimNames sets.String, fldPath *field.Path, opts PodValidationOptions, podRestartPolicy *core.RestartPolicy) field.ErrorList { +func validateContainers(containers []core.Container, volumes map[string]core.VolumeSource, podClaimNames sets.String, gracePeriod int64, fldPath *field.Path, opts PodValidationOptions, podRestartPolicy *core.RestartPolicy) field.ErrorList { allErrs := field.ErrorList{} if len(containers) == 0 { @@ -3448,11 +3467,11 @@ func validateContainers(containers []core.Container, volumes map[string]core.Vol // Regular init container and ephemeral container validation will return // field.Forbidden() for these paths. if ctr.Lifecycle != nil { - allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, path.Child("lifecycle"))...) + allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, gracePeriod, path.Child("lifecycle"))...) } - allErrs = append(allErrs, validateLivenessProbe(ctr.LivenessProbe, path.Child("livenessProbe"))...) - allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, path.Child("readinessProbe"))...) - allErrs = append(allErrs, validateStartupProbe(ctr.StartupProbe, path.Child("startupProbe"))...) + allErrs = append(allErrs, validateLivenessProbe(ctr.LivenessProbe, gracePeriod, path.Child("livenessProbe"))...) + allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, gracePeriod, path.Child("readinessProbe"))...) + allErrs = append(allErrs, validateStartupProbe(ctr.StartupProbe, gracePeriod, path.Child("startupProbe"))...) // These fields are disallowed for regular containers if ctr.RestartPolicy != nil { @@ -3974,12 +3993,18 @@ func validateHostIPs(pod *core.Pod) field.ErrorList { func ValidatePodSpec(spec *core.PodSpec, podMeta *metav1.ObjectMeta, fldPath *field.Path, opts PodValidationOptions) field.ErrorList { allErrs := field.ErrorList{} + var gracePeriod int64 + if spec.TerminationGracePeriodSeconds != nil { + // this could happen in tests + gracePeriod = *spec.TerminationGracePeriodSeconds + } + vols, vErrs := ValidateVolumes(spec.Volumes, podMeta, fldPath.Child("volumes"), opts) allErrs = append(allErrs, vErrs...) podClaimNames := gatherPodResourceClaimNames(spec.ResourceClaims) allErrs = append(allErrs, validatePodResourceClaims(podMeta, spec.ResourceClaims, fldPath.Child("resourceClaims"))...) - allErrs = append(allErrs, validateContainers(spec.Containers, vols, podClaimNames, fldPath.Child("containers"), opts, &spec.RestartPolicy)...) - allErrs = append(allErrs, validateInitContainers(spec.InitContainers, spec.Containers, vols, podClaimNames, fldPath.Child("initContainers"), opts, &spec.RestartPolicy)...) + allErrs = append(allErrs, validateContainers(spec.Containers, vols, podClaimNames, gracePeriod, fldPath.Child("containers"), opts, &spec.RestartPolicy)...) + allErrs = append(allErrs, validateInitContainers(spec.InitContainers, spec.Containers, vols, podClaimNames, gracePeriod, fldPath.Child("initContainers"), opts, &spec.RestartPolicy)...) allErrs = append(allErrs, validateEphemeralContainers(spec.EphemeralContainers, spec.Containers, spec.InitContainers, vols, podClaimNames, fldPath.Child("ephemeralContainers"), opts, &spec.RestartPolicy)...) allErrs = append(allErrs, validatePodHostNetworkDeps(spec, fldPath, opts)...) allErrs = append(allErrs, validateRestartPolicy(&spec.RestartPolicy, fldPath.Child("restartPolicy"))...) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 3b6e0af999c..30cca190da7 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -52,6 +52,7 @@ const ( dnsLabelErrMsg = "a lowercase RFC 1123 label must consist of" dnsSubdomainLabelErrMsg = "a lowercase RFC 1123 subdomain" envVarNameErrMsg = "a valid environment variable name must consist of" + defaultGracePeriod = int64(30) ) var ( @@ -6616,7 +6617,7 @@ func TestValidateProbe(t *testing.T) { } for _, p := range successCases { - if errs := validateProbe(p, field.NewPath("field")); len(errs) != 0 { + if errs := validateProbe(p, defaultGracePeriod, field.NewPath("field")); len(errs) != 0 { t.Errorf("expected success: %v", errs) } } @@ -6628,7 +6629,7 @@ func TestValidateProbe(t *testing.T) { errorCases = append(errorCases, probe) } for _, p := range errorCases { - if errs := validateProbe(p, field.NewPath("field")); len(errs) == 0 { + if errs := validateProbe(p, defaultGracePeriod, field.NewPath("field")); len(errs) == 0 { t.Errorf("expected failure for %v", p) } } @@ -6734,7 +6735,7 @@ func Test_validateProbe(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := validateProbe(tt.args.probe, tt.args.fldPath) + got := validateProbe(tt.args.probe, defaultGracePeriod, tt.args.fldPath) if len(got) != len(tt.want) { t.Errorf("validateProbe() = %v, want %v", got, tt.want) return @@ -6759,7 +6760,7 @@ func TestValidateHandler(t *testing.T) { {HTTPGet: &core.HTTPGetAction{Path: "/", Port: intstr.FromString("port"), Host: "", Scheme: "HTTP", HTTPHeaders: []core.HTTPHeader{{Name: "X-Forwarded-For", Value: "1.2.3.4"}, {Name: "X-Forwarded-For", Value: "5.6.7.8"}}}}, } for _, h := range successCases { - if errs := validateHandler(handlerFromProbe(&h), field.NewPath("field")); len(errs) != 0 { + if errs := validateHandler(handlerFromProbe(&h), defaultGracePeriod, field.NewPath("field")); len(errs) != 0 { t.Errorf("expected success: %v", errs) } } @@ -6774,7 +6775,7 @@ func TestValidateHandler(t *testing.T) { {HTTPGet: &core.HTTPGetAction{Path: "/", Port: intstr.FromString("port"), Host: "", Scheme: "HTTP", HTTPHeaders: []core.HTTPHeader{{Name: "X_Forwarded_For", Value: "foo.example.com"}}}}, } for _, h := range errorCases { - if errs := validateHandler(handlerFromProbe(&h), field.NewPath("field")); len(errs) == 0 { + if errs := validateHandler(handlerFromProbe(&h), defaultGracePeriod, field.NewPath("field")); len(errs) == 0 { t.Errorf("expected failure for %#v", h) } } @@ -7694,7 +7695,7 @@ func TestValidateContainers(t *testing.T) { var PodRestartPolicy core.RestartPolicy PodRestartPolicy = "Always" - if errs := validateContainers(successCase, volumeDevices, nil, field.NewPath("field"), PodValidationOptions{}, &PodRestartPolicy); len(errs) != 0 { + if errs := validateContainers(successCase, volumeDevices, nil, defaultGracePeriod, field.NewPath("field"), PodValidationOptions{}, &PodRestartPolicy); len(errs) != 0 { t.Errorf("expected success: %v", errs) } @@ -8308,7 +8309,7 @@ func TestValidateContainers(t *testing.T) { for _, tc := range errorCases { t.Run(tc.title+"__@L"+tc.line, func(t *testing.T) { - errs := validateContainers(tc.containers, volumeDevices, nil, field.NewPath("containers"), PodValidationOptions{}, &PodRestartPolicy) + errs := validateContainers(tc.containers, volumeDevices, nil, defaultGracePeriod, field.NewPath("containers"), PodValidationOptions{}, &PodRestartPolicy) if len(errs) == 0 { t.Fatal("expected error but received none") } @@ -8398,7 +8399,7 @@ func TestValidateInitContainers(t *testing.T) { } var PodRestartPolicy core.RestartPolicy PodRestartPolicy = "Never" - if errs := validateInitContainers(successCase, containers, volumeDevices, nil, field.NewPath("field"), PodValidationOptions{}, &PodRestartPolicy); len(errs) != 0 { + if errs := validateInitContainers(successCase, containers, volumeDevices, nil, defaultGracePeriod, field.NewPath("field"), PodValidationOptions{}, &PodRestartPolicy); len(errs) != 0 { t.Errorf("expected success: %v", errs) } @@ -8777,7 +8778,7 @@ func TestValidateInitContainers(t *testing.T) { for _, tc := range errorCases { t.Run(tc.title+"__@L"+tc.line, func(t *testing.T) { - errs := validateInitContainers(tc.initContainers, containers, volumeDevices, nil, field.NewPath("initContainers"), PodValidationOptions{}, &PodRestartPolicy) + errs := validateInitContainers(tc.initContainers, containers, volumeDevices, nil, defaultGracePeriod, field.NewPath("initContainers"), PodValidationOptions{}, &PodRestartPolicy) if len(errs) == 0 { t.Fatal("expected error but received none") } @@ -23702,3 +23703,57 @@ func TestValidateLoadBalancerStatus(t *testing.T) { }) } } + +func TestValidateSleepAction(t *testing.T) { + fldPath := field.NewPath("root") + getInvalidStr := func(gracePeriod int64) string { + return fmt.Sprintf("must be greater than 0 and less than terminationGracePeriodSeconds (%d)", gracePeriod) + } + + testCases := []struct { + name string + action *core.SleepAction + gracePeriod int64 + expectErr field.ErrorList + }{ + { + name: "valid setting", + action: &core.SleepAction{ + Seconds: 5, + }, + gracePeriod: 30, + }, + { + name: "negative seconds", + action: &core.SleepAction{ + Seconds: -1, + }, + gracePeriod: 30, + expectErr: field.ErrorList{field.Invalid(fldPath, -1, getInvalidStr(30))}, + }, + { + name: "longer than gracePeriod", + action: &core.SleepAction{ + Seconds: 5, + }, + gracePeriod: 3, + expectErr: field.ErrorList{field.Invalid(fldPath, 5, getInvalidStr(3))}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + errs := validateSleepAction(tc.action, tc.gracePeriod, fldPath) + + if len(tc.expectErr) > 0 && len(errs) == 0 { + t.Errorf("Unexpected success") + } else if len(tc.expectErr) == 0 && len(errs) != 0 { + t.Errorf("Unexpected error(s): %v", errs) + } else if len(tc.expectErr) > 0 { + if tc.expectErr[0].Error() != errs[0].Error() { + t.Errorf("Unexpected error(s): %v", errs) + } + } + }) + } +}