From 1059dbdee185ee585094c7e564ca60912ecab969 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 22 Dec 2024 23:10:29 -0800 Subject: [PATCH] Declaratively validate minimum value of RC.Spec.MinReadySeconds # Conflicts: # staging/src/k8s.io/api/core/v1/types.go --- pkg/apis/core/v1/zz_generated.validations.go | 8 +++++- pkg/apis/core/validation/validation.go | 2 +- pkg/apis/core/validation/validation_test.go | 28 +++++++++++++++++++ .../src/k8s.io/api/core/v1/generated.proto | 1 + staging/src/k8s.io/api/core/v1/types.go | 1 + 5 files changed, 38 insertions(+), 2 deletions(-) diff --git a/pkg/apis/core/v1/zz_generated.validations.go b/pkg/apis/core/v1/zz_generated.validations.go index c5e54847792..dcfcfd172e6 100644 --- a/pkg/apis/core/v1/zz_generated.validations.go +++ b/pkg/apis/core/v1/zz_generated.validations.go @@ -98,7 +98,13 @@ func Validate_ReplicationControllerSpec(ctx context.Context, op operation.Operat return }(fldPath.Child("replicas"), obj.Replicas, safe.Field(oldObj, func(oldObj *corev1.ReplicationControllerSpec) *int32 { return oldObj.Replicas }))...) - // field corev1.ReplicationControllerSpec.MinReadySeconds has no validation + // field corev1.ReplicationControllerSpec.MinReadySeconds + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *int32) (errs field.ErrorList) { + errs = append(errs, validate.Minimum(ctx, op, fldPath, obj, oldObj, 0)...) + return + }(fldPath.Child("minReadySeconds"), &obj.MinReadySeconds, safe.Field(oldObj, func(oldObj *corev1.ReplicationControllerSpec) *int32 { return &oldObj.MinReadySeconds }))...) + // field corev1.ReplicationControllerSpec.Selector has no validation // field corev1.ReplicationControllerSpec.Template has no validation return errs diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 78abddc1522..b85c555896b 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -6319,7 +6319,7 @@ func ValidatePodTemplateSpecForRC(template *core.PodTemplateSpec, selectorMap ma // ValidateReplicationControllerSpec tests if required fields in the replication controller spec are set. func ValidateReplicationControllerSpec(spec, oldSpec *core.ReplicationControllerSpec, fldPath *field.Path, opts PodValidationOptions) field.ErrorList { allErrs := field.ErrorList{} - allErrs = append(allErrs, ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds"))...) + allErrs = append(allErrs, ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds")).MarkCoveredByDeclarative()...) allErrs = append(allErrs, ValidateNonEmptySelector(spec.Selector, fldPath.Child("selector"))...) if spec.Replicas == nil { allErrs = append(allErrs, field.Required(fldPath.Child("replicas"), "").MarkCoveredByDeclarative()) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 705b0258d31..a6e3135e898 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -16755,6 +16755,16 @@ func TestValidateReplicationControllerUpdate(t *testing.T) { update: mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.Replicas = ptr.To[int32](3) }), + }, { + old: mkValidReplicationController(func(rc *core.ReplicationController) {}), + update: mkValidReplicationController(func(rc *core.ReplicationController) { + rc.Spec.MinReadySeconds = 0 + }), + }, { + old: mkValidReplicationController(func(rc *core.ReplicationController) {}), + update: mkValidReplicationController(func(rc *core.ReplicationController) { + rc.Spec.MinReadySeconds = 3 + }), }, { old: mkValidReplicationController(func(rc *core.ReplicationController) {}), update: mkValidReplicationController(func(rc *core.ReplicationController) { @@ -16831,6 +16841,15 @@ func TestValidateReplicationControllerUpdate(t *testing.T) { field.Required(field.NewPath("spec.replicas"), ""), }, }, + "negative minReadySeconds": { + old: mkValidReplicationController(func(rc *core.ReplicationController) {}), + update: mkValidReplicationController(func(rc *core.ReplicationController) { + rc.Spec.MinReadySeconds = -1 + }), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("spec.minReadySeconds"), nil, "").WithOrigin("minimum"), + }, + }, } for k, tc := range errorCases { t.Run(k, func(t *testing.T) { @@ -16872,6 +16891,9 @@ func TestValidateReplicationController(t *testing.T) { mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.Replicas = ptr.To[int32](0) }), mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.Replicas = ptr.To[int32](1) }), mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.Replicas = ptr.To[int32](100) }), + mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.MinReadySeconds = 0 }), + mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.MinReadySeconds = 1 }), + mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.MinReadySeconds = 100 }), } for _, tc := range successCases { if errs := ValidateReplicationController(&tc, PodValidationOptions{}); len(errs) != 0 { @@ -16919,6 +16941,12 @@ func TestValidateReplicationController(t *testing.T) { field.Invalid(field.NewPath("spec.replicas"), nil, "").WithOrigin("minimum"), }, }, + "negative minReadySeconds": { + input: mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.MinReadySeconds = -1 }), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("spec.minReadySeconds"), nil, "").WithOrigin("minimum"), + }, + }, "nil replicas": { input: mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.Replicas = nil }), expectedErrs: field.ErrorList{ diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index 4f5862d2886..95bd273f1e4 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -5094,6 +5094,7 @@ message ReplicationControllerSpec { // without any of its container crashing, for it to be considered available. // Defaults to 0 (pod will be considered available as soon as it is ready) // +optional + // +k8s:minimum=0 optional int32 minReadySeconds = 4; // Selector is a label query over pods that should match the Replicas count. diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index e4dc107e665..c33ddd6f9ee 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -5116,6 +5116,7 @@ type ReplicationControllerSpec struct { // without any of its container crashing, for it to be considered available. // Defaults to 0 (pod will be considered available as soon as it is ready) // +optional + // +k8s:minimum=0 MinReadySeconds int32 `json:"minReadySeconds,omitempty" protobuf:"varint,4,opt,name=minReadySeconds"` // Selector is a label query over pods that should match the Replicas count.