github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-19 08:40:42 +00:00
Code Issues Projects Releases Wiki Activity

Add validation for revisionHistoryLimit in sts to prevent negative value (#129017)

Browse Source 
* Add validation for revisionHistoryLimit in sts to prevent negative value

* Add unit tests to verify warning messages
This commit is contained in:
Arda Güçlü 2025-01-06 22:04:31 +03:00 committed by GitHub
parent 4114a9b4e4
commit 7c887412c8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 48 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
pkg/apis/apps/validation/validation.go
View File

@ -182,15 +182,16 @@ func ValidateStatefulSetUpdate(statefulSet, oldStatefulSet *apps.StatefulSet, op
// statefulset updates aren't super common and general updates are likely to be touching spec, so we'll do this
// deep copy right away. This avoids mutating our inputs
newStatefulSetClone := statefulSet.DeepCopy()
newStatefulSetClone.Spec.Replicas = oldStatefulSet.Spec.Replicas // +k8s:verify-mutation:reason=clone
newStatefulSetClone.Spec.Template = oldStatefulSet.Spec.Template // +k8s:verify-mutation:reason=clone
newStatefulSetClone.Spec.UpdateStrategy = oldStatefulSet.Spec.UpdateStrategy // +k8s:verify-mutation:reason=clone
newStatefulSetClone.Spec.MinReadySeconds = oldStatefulSet.Spec.MinReadySeconds // +k8s:verify-mutation:reason=clone
newStatefulSetClone.Spec.Ordinals = oldStatefulSet.Spec.Ordinals // +k8s:verify-mutation:reason=clone
newStatefulSetClone.Spec.Replicas = oldStatefulSet.Spec.Replicas // +k8s:verify-mutation:reason=clone
newStatefulSetClone.Spec.Template = oldStatefulSet.Spec.Template // +k8s:verify-mutation:reason=clone
newStatefulSetClone.Spec.UpdateStrategy = oldStatefulSet.Spec.UpdateStrategy // +k8s:verify-mutation:reason=clone
newStatefulSetClone.Spec.MinReadySeconds = oldStatefulSet.Spec.MinReadySeconds // +k8s:verify-mutation:reason=clone
newStatefulSetClone.Spec.Ordinals = oldStatefulSet.Spec.Ordinals // +k8s:verify-mutation:reason=clone
newStatefulSetClone.Spec.RevisionHistoryLimit = oldStatefulSet.Spec.RevisionHistoryLimit // +k8s:verify-mutation:reason=clone
newStatefulSetClone.Spec.PersistentVolumeClaimRetentionPolicy = oldStatefulSet.Spec.PersistentVolumeClaimRetentionPolicy // +k8s:verify-mutation:reason=clone
if !apiequality.Semantic.DeepEqual(newStatefulSetClone.Spec, oldStatefulSet.Spec) {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to statefulset spec for fields other than 'replicas', 'ordinals', 'template', 'updateStrategy', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden"))
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to statefulset spec for fields other than 'replicas', 'ordinals', 'template', 'updateStrategy', 'revisionHistoryLimit', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden"))
}
return allErrs

7
pkg/registry/apps/statefulset/strategy.go
View File

@ -150,6 +150,10 @@ func (statefulSetStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Obj
for i, pvc := range newStatefulSet.Spec.VolumeClaimTemplates {
warnings = append(warnings, pvcutil.GetWarningsForPersistentVolumeClaimSpec(field.NewPath("spec", "volumeClaimTemplates").Index(i), pvc.Spec)...)
}
if newStatefulSet.Spec.RevisionHistoryLimit != nil && *newStatefulSet.Spec.RevisionHistoryLimit < 0 {
warnings = append(warnings, "spec.revisionHistoryLimit: a negative value retains all historical revisions; a value >= 0 is recommended")
}
return warnings
}
@ -182,6 +186,9 @@ func (statefulSetStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtim
for i, pvc := range newStatefulSet.Spec.VolumeClaimTemplates {
warnings = append(warnings, pvcutil.GetWarningsForPersistentVolumeClaimSpec(field.NewPath("spec", "volumeClaimTemplates").Index(i).Child("Spec"), pvc.Spec)...)
}
if newStatefulSet.Spec.RevisionHistoryLimit != nil && *newStatefulSet.Spec.RevisionHistoryLimit < 0 {
warnings = append(warnings, "spec.revisionHistoryLimit: a negative value retains all historical revisions; a value >= 0 is recommended")
}
return warnings
}

34
pkg/registry/apps/statefulset/strategy_test.go
View File

@ -20,6 +20,7 @@ import (
"testing"
"github.com/google/go-cmp/cmp"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
@ -28,6 +29,7 @@ import (
"k8s.io/kubernetes/pkg/apis/apps"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/features"
"k8s.io/utils/ptr"
)
func TestStatefulSetStrategy(t *testing.T) {
@ -137,6 +139,38 @@ func TestStatefulSetStrategy(t *testing.T) {
}
})
t.Run("StatefulSet revisionHistoryLimit field validations on creation and updation", func(t *testing.T) {
// Test creation
oldSts := &apps.StatefulSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
Spec: apps.StatefulSetSpec{
PodManagementPolicy: apps.OrderedReadyPodManagement,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
Template: validPodTemplate.Template,
UpdateStrategy: apps.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType},
RevisionHistoryLimit: ptr.To(int32(-2)),
},
}
warnings := Strategy.WarningsOnCreate(ctx, oldSts)
if len(warnings) != 1 {
t.Errorf("expected one warning got %v", warnings)
}
if warnings[0] != "spec.revisionHistoryLimit: a negative value retains all historical revisions; a value >= 0 is recommended" {
t.Errorf("unexpected warning: %v", warnings)
}
oldSts.Spec.RevisionHistoryLimit = ptr.To(int32(2))
newSts := oldSts.DeepCopy()
newSts.Spec.RevisionHistoryLimit = ptr.To(int32(-2))
warnings = Strategy.WarningsOnUpdate(ctx, newSts, oldSts)
if len(warnings) != 1 {
t.Errorf("expected one warning got %v", warnings)
}
if warnings[0] != "spec.revisionHistoryLimit: a negative value retains all historical revisions; a value >= 0 is recommended" {
t.Errorf("unexpected warning: %v", warnings)
}
})
validPs = &apps.StatefulSet{
ObjectMeta: metav1.ObjectMeta{Name: ps.Name, Namespace: ps.Namespace, ResourceVersion: "1", Generation: 1},
Spec: apps.StatefulSetSpec{