From ad5cda4d21974104db01732474130d10c001d3f2 Mon Sep 17 00:00:00 2001 From: Alexander Zielenski Date: Tue, 14 Mar 2023 15:08:00 -0700 Subject: [PATCH] remove checks for fieldmanager from handlers it should never be nil --- .../apiserver/pkg/endpoints/handlers/create.go | 13 ++++++------- .../apiserver/pkg/endpoints/handlers/patch.go | 13 ++++--------- .../apiserver/pkg/endpoints/handlers/update.go | 18 ++++++++---------- 3 files changed, 18 insertions(+), 26 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go index 78c1d2f52a7..120d3f665bd 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go @@ -191,14 +191,13 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int // Dedup owner references before updating managed fields dedupOwnerReferencesAndAddWarning(obj, req.Context(), false) result, err := finisher.FinishRequest(ctx, func() (runtime.Object, error) { - if scope.FieldManager != nil { - liveObj, err := scope.Creater.New(scope.Kind) - if err != nil { - return nil, fmt.Errorf("failed to create new object (Create for %v): %v", scope.Kind, err) - } - obj = scope.FieldManager.UpdateNoErrors(liveObj, obj, managerOrUserAgent(options.FieldManager, req.UserAgent())) - admit = fieldmanager.NewManagedFieldsValidatingAdmissionController(admit) + liveObj, err := scope.Creater.New(scope.Kind) + if err != nil { + return nil, fmt.Errorf("failed to create new object (Create for %v): %v", scope.Kind, err) } + obj = scope.FieldManager.UpdateNoErrors(liveObj, obj, managerOrUserAgent(options.FieldManager, req.UserAgent())) + admit = fieldmanager.NewManagedFieldsValidatingAdmissionController(admit) + if mutatingAdmission, ok := admit.(admission.MutationInterface); ok && mutatingAdmission.Handles(admission.Create) { if err := mutatingAdmission.Admit(ctx, admissionAttributes, scope); err != nil { return nil, err diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index f743e7c5d10..ca561192f50 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -177,9 +177,8 @@ func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interfac userInfo, ) - if scope.FieldManager != nil { - admit = fieldmanager.NewManagedFieldsValidatingAdmissionController(admit) - } + admit = fieldmanager.NewManagedFieldsValidatingAdmissionController(admit) + mutatingAdmission, _ := admit.(admission.MutationInterface) createAuthorizerAttributes := authorizer.AttributesRecord{ User: userInfo, @@ -345,9 +344,7 @@ func (p *jsonPatcher) applyPatchToCurrentObject(requestContext context.Context, } } - if p.fieldManager != nil { - objToUpdate = p.fieldManager.UpdateNoErrors(currentObject, objToUpdate, managerOrUserAgent(p.options.FieldManager, p.userAgent)) - } + objToUpdate = p.fieldManager.UpdateNoErrors(currentObject, objToUpdate, managerOrUserAgent(p.options.FieldManager, p.userAgent)) return objToUpdate, nil } @@ -441,9 +438,7 @@ func (p *smpPatcher) applyPatchToCurrentObject(requestContext context.Context, c return nil, err } - if p.fieldManager != nil { - newObj = p.fieldManager.UpdateNoErrors(currentObject, newObj, managerOrUserAgent(p.options.FieldManager, p.userAgent)) - } + newObj = p.fieldManager.UpdateNoErrors(currentObject, newObj, managerOrUserAgent(p.options.FieldManager, p.userAgent)) return newObj, nil } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go index 20288863981..4b76ef97e07 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go @@ -156,15 +156,13 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa // allows skipping managedFields update if the resulting object is too big shouldUpdateManagedFields := true - if scope.FieldManager != nil { - admit = fieldmanager.NewManagedFieldsValidatingAdmissionController(admit) - transformers = append(transformers, func(_ context.Context, newObj, liveObj runtime.Object) (runtime.Object, error) { - if shouldUpdateManagedFields { - return scope.FieldManager.UpdateNoErrors(liveObj, newObj, managerOrUserAgent(options.FieldManager, req.UserAgent())), nil - } - return newObj, nil - }) - } + admit = fieldmanager.NewManagedFieldsValidatingAdmissionController(admit) + transformers = append(transformers, func(_ context.Context, newObj, liveObj runtime.Object) (runtime.Object, error) { + if shouldUpdateManagedFields { + return scope.FieldManager.UpdateNoErrors(liveObj, newObj, managerOrUserAgent(options.FieldManager, req.UserAgent())), nil + } + return newObj, nil + }) if mutatingAdmission, ok := admit.(admission.MutationInterface); ok { transformers = append(transformers, func(ctx context.Context, newObj, oldObj runtime.Object) (runtime.Object, error) { @@ -228,7 +226,7 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa result, err := requestFunc() // If the object wasn't committed to storage because it's serialized size was too large, // it is safe to remove managedFields (which can be large) and try again. - if isTooLargeError(err) && scope.FieldManager != nil { + if isTooLargeError(err) { if accessor, accessorErr := meta.Accessor(obj); accessorErr == nil { accessor.SetManagedFields(nil) shouldUpdateManagedFields = false