diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/admission.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/admission.go index 751c4753d72..fdc95aa9331 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/admission.go @@ -67,7 +67,7 @@ func (admit *managedFieldsValidatingAdmissionController) Admit(ctx context.Conte return err } managedFieldsAfterAdmission := objectMeta.GetManagedFields() - if err := ValidateManagedFields(managedFieldsAfterAdmission); err != nil { + if _, err := DecodeManagedFields(managedFieldsAfterAdmission); err != nil { objectMeta.SetManagedFields(managedFieldsBeforeAdmission) warning.AddWarning(ctx, "", fmt.Sprintf(InvalidManagedFieldsAfterMutatingAdmissionWarningFormat, diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/validation.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/validation.go deleted file mode 100644 index 324c23b5553..00000000000 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/validation.go +++ /dev/null @@ -1,38 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package fieldmanager - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" - "k8s.io/apimachinery/pkg/util/validation/field" -) - -// ValidateManagedFields by checking the integrity of every entry and trying to decode -// them to the internal format -func ValidateManagedFields(managedFields []metav1.ManagedFieldsEntry) error { - validationErrs := v1validation.ValidateManagedFields(managedFields, field.NewPath("metadata").Child("managedFields")) - if len(validationErrs) > 0 { - return validationErrs.ToAggregate() - } - - if _, err := DecodeManagedFields(managedFields); err != nil { - return err - } - - return nil -} diff --git a/test/integration/apiserver/admissionwebhook/invalid_managedFields_test.go b/test/integration/apiserver/admissionwebhook/invalid_managedFields_test.go index e6f9a9250f2..9a5ba8e8df3 100644 --- a/test/integration/apiserver/admissionwebhook/invalid_managedFields_test.go +++ b/test/integration/apiserver/admissionwebhook/invalid_managedFields_test.go @@ -34,7 +34,9 @@ import ( admissionv1 "k8s.io/api/admissionregistration/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager" clientset "k8s.io/client-go/kubernetes" @@ -127,7 +129,7 @@ func TestMutatingWebhookResetsInvalidManagedFields(t *testing.T) { if err != nil { return false, err } - if err := fieldmanager.ValidateManagedFields(pod.ManagedFields); err != nil { + if err := validateManagedFieldsAndDecode(pod.ManagedFields); err != nil { lastErr = err return false, nil } @@ -152,7 +154,7 @@ func TestMutatingWebhookResetsInvalidManagedFields(t *testing.T) { if err != nil { t.Fatal(err) } - if err := fieldmanager.ValidateManagedFields(pod.ManagedFields); err != nil { + if err := validateManagedFieldsAndDecode(pod.ManagedFields); err != nil { t.Error(err) } if warningWriter.WarningCount() != 2 { @@ -164,6 +166,16 @@ func TestMutatingWebhookResetsInvalidManagedFields(t *testing.T) { } } +// validate against both decoding and validation to make sure we use the hardest rule between the both to reset +// with decoding being as strict as it gets, only using it should be enough in admission +func validateManagedFieldsAndDecode(managedFields []metav1.ManagedFieldsEntry) error { + if _, err := fieldmanager.DecodeManagedFields(managedFields); err != nil { + return err + } + validationErrs := v1validation.ValidateManagedFields(managedFields, field.NewPath("metadata").Child("managedFields")) + return validationErrs.ToAggregate() +} + func newInvalidManagedFieldsWebhookHandler(t *testing.T) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close()