From 55ff96301797a503b6ee1d09f0eb2ffc827f01b1 Mon Sep 17 00:00:00 2001 From: "Julian V. Modesto" Date: Tue, 18 May 2021 17:28:11 -0400 Subject: [PATCH 1/2] Make validation totalAnnotationSizeLimitB public. Replace the forked totalAnnotationSizeLimitB with apimachineryvalidation.TotalAnnotationSizeLimitB. --- .../apimachinery/pkg/api/validation/objectmeta.go | 6 +++--- .../pkg/api/validation/objectmeta_test.go | 12 ++++++------ .../handlers/fieldmanager/lastappliedupdater.go | 7 +++---- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta.go b/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta.go index 3e234eb11dc..9530419db7e 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta.go @@ -33,7 +33,7 @@ import ( // FieldImmutableErrorMsg is a error message for field is immutable. const FieldImmutableErrorMsg string = `field is immutable` -const totalAnnotationSizeLimitB int = 256 * (1 << 10) // 256 kB +const TotalAnnotationSizeLimitB int = 256 * (1 << 10) // 256 kB // BannedOwners is a black list of object that are not allowed to be owners. var BannedOwners = map[schema.GroupVersionKind]struct{}{ @@ -53,8 +53,8 @@ func ValidateAnnotations(annotations map[string]string, fldPath *field.Path) fie } totalSize += (int64)(len(k)) + (int64)(len(v)) } - if totalSize > (int64)(totalAnnotationSizeLimitB) { - allErrs = append(allErrs, field.TooLong(fldPath, "", totalAnnotationSizeLimitB)) + if totalSize > (int64)(TotalAnnotationSizeLimitB) { + allErrs = append(allErrs, field.TooLong(fldPath, "", TotalAnnotationSizeLimitB)) } return allErrs } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta_test.go b/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta_test.go index ebd6c7e7c7a..349da67f8aa 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta_test.go @@ -452,10 +452,10 @@ func TestValidateAnnotations(t *testing.T) { {"1234/5678": "bar"}, {"1.2.3.4/5678": "bar"}, {"UpperCase123": "bar"}, - {"a": strings.Repeat("b", totalAnnotationSizeLimitB-1)}, + {"a": strings.Repeat("b", TotalAnnotationSizeLimitB-1)}, { - "a": strings.Repeat("b", totalAnnotationSizeLimitB/2-1), - "c": strings.Repeat("d", totalAnnotationSizeLimitB/2-1), + "a": strings.Repeat("b", TotalAnnotationSizeLimitB/2-1), + "c": strings.Repeat("d", TotalAnnotationSizeLimitB/2-1), }, } for i := range successCases { @@ -485,10 +485,10 @@ func TestValidateAnnotations(t *testing.T) { } } totalSizeErrorCases := []map[string]string{ - {"a": strings.Repeat("b", totalAnnotationSizeLimitB)}, + {"a": strings.Repeat("b", TotalAnnotationSizeLimitB)}, { - "a": strings.Repeat("b", totalAnnotationSizeLimitB/2), - "c": strings.Repeat("d", totalAnnotationSizeLimitB/2), + "a": strings.Repeat("b", TotalAnnotationSizeLimitB/2), + "c": strings.Repeat("d", TotalAnnotationSizeLimitB/2), }, } for i := range totalSizeErrorCases { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/lastappliedupdater.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/lastappliedupdater.go index ad651a2b59d..551661a0610 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/lastappliedupdater.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/lastappliedupdater.go @@ -21,12 +21,11 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" + apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" ) -const totalAnnotationSizeLimitB int64 = 256 * (1 << 10) // 256 kB - type lastAppliedUpdater struct { fieldManager Manager } @@ -126,8 +125,8 @@ func isAnnotationsValid(annotations map[string]string) error { for k, v := range annotations { totalSize += (int64)(len(k)) + (int64)(len(v)) } - if totalSize > (int64)(totalAnnotationSizeLimitB) { - return fmt.Errorf("annotations size %d is larger than limit %d", totalSize, totalAnnotationSizeLimitB) + if totalSize > (int64)(apimachineryvalidation.TotalAnnotationSizeLimitB) { + return fmt.Errorf("annotations size %d is larger than limit %d", totalSize, apimachineryvalidation.TotalAnnotationSizeLimitB) } return nil } From 2e771b8e745c4a3be0d5bae3a6dc94087284c73b Mon Sep 17 00:00:00 2001 From: "Julian V. Modesto" Date: Tue, 25 May 2021 16:01:38 -0400 Subject: [PATCH 2/2] Make a public ValidateAnnotationsSize --- .../pkg/api/validation/objectmeta.go | 17 +++++++++++++---- .../handlers/fieldmanager/lastappliedupdater.go | 13 +------------ 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta.go b/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta.go index 9530419db7e..4d9efe3f72b 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta.go @@ -46,19 +46,28 @@ var ValidateClusterName = NameIsDNS1035Label // ValidateAnnotations validates that a set of annotations are correctly defined. func ValidateAnnotations(annotations map[string]string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - var totalSize int64 - for k, v := range annotations { + for k := range annotations { for _, msg := range validation.IsQualifiedName(strings.ToLower(k)) { allErrs = append(allErrs, field.Invalid(fldPath, k, msg)) } - totalSize += (int64)(len(k)) + (int64)(len(v)) } - if totalSize > (int64)(TotalAnnotationSizeLimitB) { + if err := ValidateAnnotationsSize(annotations); err != nil { allErrs = append(allErrs, field.TooLong(fldPath, "", TotalAnnotationSizeLimitB)) } return allErrs } +func ValidateAnnotationsSize(annotations map[string]string) error { + var totalSize int64 + for k, v := range annotations { + totalSize += (int64)(len(k)) + (int64)(len(v)) + } + if totalSize > (int64)(TotalAnnotationSizeLimitB) { + return fmt.Errorf("annotations size %d is larger than limit %d", totalSize, TotalAnnotationSizeLimitB) + } + return nil +} + func validateOwnerReference(ownerReference metav1.OwnerReference, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} gvk := schema.FromAPIVersionAndKind(ownerReference.APIVersion, ownerReference.Kind) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/lastappliedupdater.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/lastappliedupdater.go index 551661a0610..7cd4eb1289b 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/lastappliedupdater.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/lastappliedupdater.go @@ -93,7 +93,7 @@ func setLastApplied(obj runtime.Object, value string) error { annotations = map[string]string{} } annotations[corev1.LastAppliedConfigAnnotation] = value - if isAnnotationsValid(annotations) != nil { + if err := apimachineryvalidation.ValidateAnnotationsSize(annotations); err != nil { delete(annotations, corev1.LastAppliedConfigAnnotation) } accessor.SetAnnotations(annotations) @@ -119,14 +119,3 @@ func buildLastApplied(obj runtime.Object) (string, error) { } return string(lastApplied), nil } - -func isAnnotationsValid(annotations map[string]string) error { - var totalSize int64 - for k, v := range annotations { - totalSize += (int64)(len(k)) + (int64)(len(v)) - } - if totalSize > (int64)(apimachineryvalidation.TotalAnnotationSizeLimitB) { - return fmt.Errorf("annotations size %d is larger than limit %d", totalSize, apimachineryvalidation.TotalAnnotationSizeLimitB) - } - return nil -}