scottk/kubernetes
1
0
Fork 0
mirror of https://github.com/optim-enterprises-bv/kubernetes.git synced 2025-11-02 11:18:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

Clean kube_features.go

Browse Source 
Added tests, info about new feature gate in error message, fixes from review

Added basic e2e test

Added unit tests

Ran hack/update-featuregates.sh

Tolerate updates to existing resources after disabling feature gate

Added feature gate to versioned_kube_features.go

Fixed existing tests

Use PodValidationOptions for validation instead of using feature gate directly

Relaxed validation for allowing zero in prestop hook sleep action
This commit is contained in:
Sreeram Venkitesh
2024-10-17 14:16:00 +05:30
parent 0daa75b972
commit f1f9e7b398
8 changed files with 197 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
pkg/apis/core/validation/validation.go
View File

@@ -3068,52 +3068,52 @@ func validatePodResourceClaim(podMeta *metav1.ObjectMeta, claim core.PodResource
return allErrs
}
func validateLivenessProbe(probe *core.Probe, gracePeriod *int64, fldPath *field.Path) field.ErrorList {
func validateLivenessProbe(probe *core.Probe, gracePeriod *int64, fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
allErrs := field.ErrorList{}
if probe == nil {
return allErrs
}
allErrs = append(allErrs, validateProbe(probe, gracePeriod, fldPath)...)
allErrs = append(allErrs, validateProbe(probe, gracePeriod, fldPath, opts)...)
if probe.SuccessThreshold != 1 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("successThreshold"), probe.SuccessThreshold, "must be 1"))
}
return allErrs
}
func validateReadinessProbe(probe *core.Probe, gracePeriod *int64, fldPath *field.Path) field.ErrorList {
func validateReadinessProbe(probe *core.Probe, gracePeriod *int64, fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
allErrs := field.ErrorList{}
if probe == nil {
return allErrs
}
allErrs = append(allErrs, validateProbe(probe, gracePeriod, fldPath)...)
allErrs = append(allErrs, validateProbe(probe, gracePeriod, fldPath, opts)...)
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, gracePeriod *int64, fldPath *field.Path) field.ErrorList {
func validateStartupProbe(probe *core.Probe, gracePeriod *int64, fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
allErrs := field.ErrorList{}
if probe == nil {
return allErrs
}
allErrs = append(allErrs, validateProbe(probe, gracePeriod, fldPath)...)
allErrs = append(allErrs, validateProbe(probe, gracePeriod, fldPath, opts)...)
if probe.SuccessThreshold != 1 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("successThreshold"), probe.SuccessThreshold, "must be 1"))
}
return allErrs
}
func validateProbe(probe *core.Probe, gracePeriod *int64, fldPath *field.Path) field.ErrorList {
func validateProbe(probe *core.Probe, gracePeriod *int64, fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
allErrs := field.ErrorList{}
if probe == nil {
return allErrs
}
allErrs = append(allErrs, validateHandler(handlerFromProbe(&probe.ProbeHandler), gracePeriod, fldPath)...)
allErrs = append(allErrs, validateHandler(handlerFromProbe(&probe.ProbeHandler), gracePeriod, fldPath, opts)...)
allErrs = append(allErrs, ValidateNonnegativeField(int64(probe.InitialDelaySeconds), fldPath.Child("initialDelaySeconds"))...)
allErrs = append(allErrs, ValidateNonnegativeField(int64(probe.TimeoutSeconds), fldPath.Child("timeoutSeconds"))...)
@@ -3169,14 +3169,21 @@ func handlerFromLifecycle(lh *core.LifecycleHandler) commonHandler {
}
}
func validateSleepAction(sleep *core.SleepAction, gracePeriod *int64, fldPath *field.Path) field.ErrorList {
func validateSleepAction(sleep *core.SleepAction, gracePeriod *int64, fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
allErrors := field.ErrorList{}
// We allow gracePeriod to be nil here because the pod in which this SleepAction
// is defined might have an invalid grace period defined, and we don't want to
// flag another error here when the real problem will already be flagged.
if gracePeriod != nil && 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))
if opts.AllowPodLifecycleSleepActionZeroValue {
if gracePeriod != nil && (sleep.Seconds < 0 || sleep.Seconds > *gracePeriod) {
invalidStr := fmt.Sprintf("must be non-negative and less than terminationGracePeriodSeconds (%d)", *gracePeriod)
allErrors = append(allErrors, field.Invalid(fldPath, sleep.Seconds, invalidStr))
}
} else {
if gracePeriod != nil && (sleep.Seconds <= 0 || sleep.Seconds > *gracePeriod) {
invalidStr := fmt.Sprintf("must be greater than 0 and less than terminationGracePeriodSeconds (%d). Enable AllowPodLifecycleSleepActionZeroValue feature gate for zero sleep.", *gracePeriod)
allErrors = append(allErrors, field.Invalid(fldPath, sleep.Seconds, invalidStr))
}
}
return allErrors
}
@@ -3289,7 +3296,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, gracePeriod *int64, fldPath *field.Path) field.ErrorList {
func validateHandler(handler commonHandler, gracePeriod *int64, fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
numHandlers := 0
allErrors := field.ErrorList{}
if handler.Exec != nil {
@@ -3329,7 +3336,7 @@ func validateHandler(handler commonHandler, gracePeriod *int64, fldPath *field.P
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"))...)
allErrors = append(allErrors, validateSleepAction(handler.Sleep, gracePeriod, fldPath.Child("sleep"), opts)...)
}
}
if numHandlers == 0 {
@@ -3338,13 +3345,13 @@ func validateHandler(handler commonHandler, gracePeriod *int64, fldPath *field.P
return allErrors
}
func validateLifecycle(lifecycle *core.Lifecycle, gracePeriod *int64, fldPath *field.Path) field.ErrorList {
func validateLifecycle(lifecycle *core.Lifecycle, gracePeriod *int64, fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
allErrs := field.ErrorList{}
if lifecycle.PostStart != nil {
allErrs = append(allErrs, validateHandler(handlerFromLifecycle(lifecycle.PostStart), gracePeriod, fldPath.Child("postStart"))...)
allErrs = append(allErrs, validateHandler(handlerFromLifecycle(lifecycle.PostStart), gracePeriod, fldPath.Child("postStart"), opts)...)
}
if lifecycle.PreStop != nil {
allErrs = append(allErrs, validateHandler(handlerFromLifecycle(lifecycle.PreStop), gracePeriod, fldPath.Child("preStop"))...)
allErrs = append(allErrs, validateHandler(handlerFromLifecycle(lifecycle.PreStop), gracePeriod, fldPath.Child("preStop"), opts)...)
}
return allErrs
}
@@ -3523,11 +3530,11 @@ func validateInitContainers(containers []core.Container, regularContainers []cor
switch {
case restartAlways:
if ctr.Lifecycle != nil {
allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, gracePeriod, idxPath.Child("lifecycle"))...)
allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, gracePeriod, idxPath.Child("lifecycle"), opts)...)
}
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"))...)
allErrs = append(allErrs, validateLivenessProbe(ctr.LivenessProbe, gracePeriod, idxPath.Child("livenessProbe"), opts)...)
allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, gracePeriod, idxPath.Child("readinessProbe"), opts)...)
allErrs = append(allErrs, validateStartupProbe(ctr.StartupProbe, gracePeriod, idxPath.Child("startupProbe"), opts)...)
default:
// These fields are disallowed for init containers.
@@ -3655,11 +3662,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, gracePeriod, path.Child("lifecycle"))...)
allErrs = append(allErrs, validateLifecycle(ctr.Lifecycle, gracePeriod, path.Child("lifecycle"), opts)...)
}
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"))...)
allErrs = append(allErrs, validateLivenessProbe(ctr.LivenessProbe, gracePeriod, path.Child("livenessProbe"), opts)...)
allErrs = append(allErrs, validateReadinessProbe(ctr.ReadinessProbe, gracePeriod, path.Child("readinessProbe"), opts)...)
allErrs = append(allErrs, validateStartupProbe(ctr.StartupProbe, gracePeriod, path.Child("startupProbe"), opts)...)
// These fields are disallowed for regular containers
if ctr.RestartPolicy != nil {
@@ -4049,6 +4056,8 @@ type PodValidationOptions struct {
AllowRelaxedEnvironmentVariableValidation bool
// Allow the use of a relaxed DNS search
AllowRelaxedDNSSearchValidation bool
// Allows zero value for Pod Lifecycle Sleep Action
AllowPodLifecycleSleepActionZeroValue bool
}
// validatePodMetadataAndSpec tests if required fields in the pod.metadata and pod.spec are set,

98
pkg/apis/core/validation/validation_test.go
View File

@@ -7445,7 +7445,7 @@ func TestValidateProbe(t *testing.T) {
}
for _, p := range successCases {
if errs := validateProbe(p, defaultGracePeriod, field.NewPath("field")); len(errs) != 0 {
if errs := validateProbe(p, defaultGracePeriod, field.NewPath("field"), PodValidationOptions{}); len(errs) != 0 {
t.Errorf("expected success: %v", errs)
}
}
@@ -7457,7 +7457,7 @@ func TestValidateProbe(t *testing.T) {
errorCases = append(errorCases, probe)
}
for _, p := range errorCases {
if errs := validateProbe(p, defaultGracePeriod, field.NewPath("field")); len(errs) == 0 {
if errs := validateProbe(p, defaultGracePeriod, field.NewPath("field"), PodValidationOptions{}); len(errs) == 0 {
t.Errorf("expected failure for %v", p)
}
}
@@ -7563,7 +7563,7 @@ func Test_validateProbe(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := validateProbe(tt.args.probe, defaultGracePeriod, tt.args.fldPath)
got := validateProbe(tt.args.probe, defaultGracePeriod, tt.args.fldPath, PodValidationOptions{})
if len(got) != len(tt.want) {
t.Errorf("validateProbe() = %v, want %v", got, tt.want)
return
@@ -7588,7 +7588,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), defaultGracePeriod, field.NewPath("field")); len(errs) != 0 {
if errs := validateHandler(handlerFromProbe(&h), defaultGracePeriod, field.NewPath("field"), PodValidationOptions{}); len(errs) != 0 {
t.Errorf("expected success: %v", errs)
}
}
@@ -7603,7 +7603,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), defaultGracePeriod, field.NewPath("field")); len(errs) == 0 {
if errs := validateHandler(handlerFromProbe(&h), defaultGracePeriod, field.NewPath("field"), PodValidationOptions{}); len(errs) == 0 {
t.Errorf("expected failure for %#v", h)
}
}
@@ -24147,43 +24147,109 @@ 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)
return fmt.Sprintf("must be greater than 0 and less than terminationGracePeriodSeconds (%d). Enable AllowPodLifecycleSleepActionZeroValue feature gate for zero sleep.", gracePeriod)
}
getInvalidStrWithZeroValueEnabled := func(gracePeriod int64) string {
return fmt.Sprintf("must be non-negative and less than terminationGracePeriodSeconds (%d)", gracePeriod)
}
testCases := []struct {
name string
action *core.SleepAction
gracePeriod int64
expectErr field.ErrorList
name string
action *core.SleepAction
gracePeriod int64
zeroValueEnabled bool
expectErr field.ErrorList
}{
{
name: "valid setting",
action: &core.SleepAction{
Seconds: 5,
},
gracePeriod: 30,
gracePeriod: 30,
zeroValueEnabled: false,
},
{
name: "negative seconds",
action: &core.SleepAction{
Seconds: -1,
},
gracePeriod: 30,
expectErr: field.ErrorList{field.Invalid(fldPath, -1, getInvalidStr(30))},
gracePeriod: 30,
zeroValueEnabled: false,
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))},
gracePeriod: 3,
zeroValueEnabled: false,
expectErr: field.ErrorList{field.Invalid(fldPath, 5, getInvalidStr(3))},
},
{
name: "sleep duration of zero with zero value feature gate disabled",
action: &core.SleepAction{
Seconds: 0,
},
gracePeriod: 30,
zeroValueEnabled: false,
expectErr: field.ErrorList{field.Invalid(fldPath, 0, getInvalidStr(30))},
},
{
name: "sleep duration of zero with zero value feature gate enabled",
action: &core.SleepAction{
Seconds: 0,
},
gracePeriod: 30,
zeroValueEnabled: true,
},
{
name: "invalid sleep duration (negative value) with zero value disabled",
action: &core.SleepAction{
Seconds: -1,
},
gracePeriod: 30,
zeroValueEnabled: false,
expectErr: field.ErrorList{field.Invalid(fldPath, -1, getInvalidStr(30))},
},
{
name: "invalid sleep duration (negative value) with zero value enabled",
action: &core.SleepAction{
Seconds: -1,
},
gracePeriod: 30,
zeroValueEnabled: true,
expectErr: field.ErrorList{field.Invalid(fldPath, -1, getInvalidStrWithZeroValueEnabled(30))},
},
{
name: "zero grace period duration with zero value enabled",
action: &core.SleepAction{
Seconds: 0,
},
gracePeriod: 0,
zeroValueEnabled: true,
},
{
name: "nil grace period with zero value disabled",
action: &core.SleepAction{
Seconds: 5,
},
zeroValueEnabled: false,
expectErr: field.ErrorList{field.Invalid(fldPath, 5, getInvalidStr(0))},
},
{
name: "nil grace period with zero value enabled",
action: &core.SleepAction{
Seconds: 0,
},
zeroValueEnabled: true,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
errs := validateSleepAction(tc.action, &tc.gracePeriod, fldPath)
errs := validateSleepAction(tc.action, &tc.gracePeriod, fldPath, PodValidationOptions{AllowPodLifecycleSleepActionZeroValue: tc.zeroValueEnabled})
if len(tc.expectErr) > 0 && len(errs) == 0 {
t.Errorf("Unexpected success")