From 46d98f52ff64823ed489c5ab38480e0843b0a106 Mon Sep 17 00:00:00 2001 From: Antoine Pelisse Date: Sat, 16 Feb 2019 20:16:11 -0800 Subject: [PATCH] Add "fieldManager" to flag to PATCH/CREATE/UPDATE And add a corresponding flag in kubectl (for apply), even though the value is defaulted in kubectl with "kubectl". The flag is required for Apply patch-type, and optional for other PATCH, CREATE and UPDATE (in which case we fallback on the user-agent). Kubernetes-commit: eb904d8fa89da491f400614f99458ed3f0d529fb --- pkg/endpoints/handlers/create.go | 34 ++++++++++- pkg/endpoints/handlers/create_test.go | 60 +++++++++++++++++++ .../handlers/fieldmanager/fieldmanager.go | 6 +- .../fieldmanager/fieldmanager_test.go | 5 +- pkg/endpoints/handlers/patch.go | 6 +- pkg/endpoints/handlers/update.go | 2 +- 6 files changed, 100 insertions(+), 13 deletions(-) create mode 100644 pkg/endpoints/handlers/create_test.go diff --git a/pkg/endpoints/handlers/create.go b/pkg/endpoints/handlers/create.go index 067675a3b..80e781fb8 100644 --- a/pkg/endpoints/handlers/create.go +++ b/pkg/endpoints/handlers/create.go @@ -17,11 +17,14 @@ limitations under the License. package handlers import ( + "bytes" "context" "fmt" "net/http" "strings" "time" + "unicode" + "unicode/utf8" "k8s.io/apimachinery/pkg/api/errors" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" @@ -141,7 +144,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, admit admission.Inte return } - obj, err = scope.FieldManager.Update(liveObj, obj, prefixFromUserAgent(req.UserAgent())) + obj, err = scope.FieldManager.Update(liveObj, obj, managerOrUserAgent(options.FieldManager, req.UserAgent())) if err != nil { scope.err(fmt.Errorf("failed to update object (Create for %v) managed fields: %v", scope.Kind, err), w, req) return @@ -193,6 +196,31 @@ func (c *namedCreaterAdapter) Create(ctx context.Context, name string, obj runti return c.Creater.Create(ctx, obj, createValidatingAdmission, options) } -func prefixFromUserAgent(u string) string { - return strings.Split(u, "/")[0] +// manager is assumed to be already a valid value, we need to make +// userAgent into a valid value too. +func managerOrUserAgent(manager, userAgent string) string { + if manager != "" { + return manager + } + return prefixFromUserAgent(userAgent) +} + +// prefixFromUserAgent takes the characters preceding the first /, quote +// unprintable character and then trim what's beyond the +// FieldManagerMaxLength limit. +func prefixFromUserAgent(u string) string { + m := strings.Split(u, "/")[0] + buf := bytes.NewBuffer(nil) + for _, r := range m { + // Ignore non-printable characters + if !unicode.IsPrint(r) { + continue + } + // Only append if we have room for it + if buf.Len()+utf8.RuneLen(r) > validation.FieldManagerMaxLength { + break + } + buf.WriteRune(r) + } + return buf.String() } diff --git a/pkg/endpoints/handlers/create_test.go b/pkg/endpoints/handlers/create_test.go new file mode 100644 index 000000000..760947276 --- /dev/null +++ b/pkg/endpoints/handlers/create_test.go @@ -0,0 +1,60 @@ +/* +Copyright 2019 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 handlers + +import ( + "fmt" + "testing" +) + +func TestManagerOrUserAgent(t *testing.T) { + tests := []struct { + manager string + userAgent string + expected string + }{ + { + manager: "", + userAgent: "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.121 Safari/537.36", + expected: "Mozilla", + }, + { + manager: "", + userAgent: "fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff/Something", + expected: "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + }, + { + manager: "", + userAgent: "🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔", + expected: "🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔🍔", + }, + { + manager: "manager", + userAgent: "userAgent", + expected: "manager", + }, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%v-%v", test.manager, test.userAgent), func(t *testing.T) { + got := managerOrUserAgent(test.manager, test.userAgent) + if got != test.expected { + t.Errorf("Wanted %#v, got %#v", test.expected, got) + } + }) + } +} diff --git a/pkg/endpoints/handlers/fieldmanager/fieldmanager.go b/pkg/endpoints/handlers/fieldmanager/fieldmanager.go index 08c9e0eb5..f761c8b87 100644 --- a/pkg/endpoints/handlers/fieldmanager/fieldmanager.go +++ b/pkg/endpoints/handlers/fieldmanager/fieldmanager.go @@ -30,8 +30,6 @@ import ( "sigs.k8s.io/structured-merge-diff/merge" ) -const applyManager = "apply" - // FieldManager updates the managed fields and merge applied // configurations. type FieldManager struct { @@ -141,7 +139,7 @@ func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (r // Apply is used when server-side apply is called, as it merges the // object and update the managed fields. -func (f *FieldManager) Apply(liveObj runtime.Object, patch []byte, force bool) (runtime.Object, error) { +func (f *FieldManager) Apply(liveObj runtime.Object, patch []byte, fieldManager string, force bool) (runtime.Object, error) { // If the object doesn't have metadata, apply isn't allowed. if _, err := meta.Accessor(liveObj); err != nil { return nil, fmt.Errorf("couldn't get accessor: %v", err) @@ -168,7 +166,7 @@ func (f *FieldManager) Apply(liveObj runtime.Object, patch []byte, force bool) ( if err != nil { return nil, fmt.Errorf("failed to create typed live object: %v", err) } - manager, err := f.buildManagerInfo(applyManager, metav1.ManagedFieldsOperationApply) + manager, err := f.buildManagerInfo(fieldManager, metav1.ManagedFieldsOperationApply) if err != nil { return nil, fmt.Errorf("failed to build manager identifier: %v", err) } diff --git a/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go b/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go index 82aa6a84a..f512c98b7 100644 --- a/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go +++ b/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go @@ -96,7 +96,7 @@ func TestApplyStripsFields(t *testing.T) { }], "resourceVersion": "b" } - }`), false) + }`), "fieldmanager_test", false) if err != nil { t.Fatalf("failed to apply object: %v", err) } @@ -110,6 +110,7 @@ func TestApplyStripsFields(t *testing.T) { t.Fatalf("fields did not get stripped on apply: %v", m) } } + func TestApplyDoesNotStripLabels(t *testing.T) { f := NewTestFieldManager(t) @@ -123,7 +124,7 @@ func TestApplyDoesNotStripLabels(t *testing.T) { "a": "b" }, } - }`), false) + }`), "fieldmanager_test", false) if err != nil { t.Fatalf("failed to apply object: %v", err) } diff --git a/pkg/endpoints/handlers/patch.go b/pkg/endpoints/handlers/patch.go index 50592c5d7..bc06b7acf 100644 --- a/pkg/endpoints/handlers/patch.go +++ b/pkg/endpoints/handlers/patch.go @@ -320,7 +320,7 @@ func (p *jsonPatcher) applyPatchToCurrentObject(currentObject runtime.Object) (r } if p.fieldManager != nil { - if objToUpdate, err = p.fieldManager.Update(currentObject, objToUpdate, prefixFromUserAgent(p.userAgent)); err != nil { + if objToUpdate, err = p.fieldManager.Update(currentObject, objToUpdate, managerOrUserAgent(p.options.FieldManager, p.userAgent)); err != nil { return nil, fmt.Errorf("failed to update object (json PATCH for %v) managed fields: %v", p.kind, err) } } @@ -387,7 +387,7 @@ func (p *smpPatcher) applyPatchToCurrentObject(currentObject runtime.Object) (ru } if p.fieldManager != nil { - if newObj, err = p.fieldManager.Update(currentObject, newObj, prefixFromUserAgent(p.userAgent)); err != nil { + if newObj, err = p.fieldManager.Update(currentObject, newObj, managerOrUserAgent(p.options.FieldManager, p.userAgent)); err != nil { return nil, fmt.Errorf("failed to update object (smp PATCH for %v) managed fields: %v", p.kind, err) } } @@ -414,7 +414,7 @@ func (p *applyPatcher) applyPatchToCurrentObject(obj runtime.Object) (runtime.Ob if p.fieldManager == nil { panic("FieldManager must be installed to run apply") } - return p.fieldManager.Apply(obj, p.patch, force) + return p.fieldManager.Apply(obj, p.patch, p.options.FieldManager, force) } func (p *applyPatcher) createNewObject() (runtime.Object, error) { diff --git a/pkg/endpoints/handlers/update.go b/pkg/endpoints/handlers/update.go index e55d87092..569fcb9df 100644 --- a/pkg/endpoints/handlers/update.go +++ b/pkg/endpoints/handlers/update.go @@ -124,7 +124,7 @@ func UpdateResource(r rest.Updater, scope RequestScope, admit admission.Interfac transformers := []rest.TransformFunc{} if scope.FieldManager != nil { transformers = append(transformers, func(_ context.Context, newObj, liveObj runtime.Object) (runtime.Object, error) { - obj, err := scope.FieldManager.Update(liveObj, newObj, prefixFromUserAgent(req.UserAgent())) + obj, err := scope.FieldManager.Update(liveObj, newObj, managerOrUserAgent(options.FieldManager, req.UserAgent())) if err != nil { return nil, fmt.Errorf("failed to update object (Update for %v) managed fields: %v", scope.Kind, err) }