diff --git a/pkg/kubectl/cmd/apply/apply.go b/pkg/kubectl/cmd/apply/apply.go index ee0c39a256..1d0d8bc156 100644 --- a/pkg/kubectl/cmd/apply/apply.go +++ b/pkg/kubectl/cmd/apply/apply.go @@ -68,6 +68,7 @@ type ApplyOptions struct { ServerSideApply bool ForceConflicts bool + FieldManager string Selector string DryRun bool ServerDryRun bool @@ -196,14 +197,15 @@ func NewCmdApply(baseName string, f cmdutil.Factory, ioStreams genericclioptions func (o *ApplyOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { o.ServerSideApply = cmdutil.GetServerSideApplyFlag(cmd) o.ForceConflicts = cmdutil.GetForceConflictsFlag(cmd) + o.FieldManager = cmdutil.GetFieldManagerFlag(cmd) o.DryRun = cmdutil.GetDryRunFlag(cmd) if o.ForceConflicts && !o.ServerSideApply { - return fmt.Errorf("--force-conflicts only works with --server-side") + return fmt.Errorf("--experimental-force-conflicts only works with --experimental-server-side") } if o.DryRun && o.ServerSideApply { - return fmt.Errorf("--dry-run doesn't work with --server-side") + return fmt.Errorf("--dry-run doesn't work with --experimental-server-side (did you mean --server-dry-run instead?)") } if o.DryRun && o.ServerDryRun { @@ -393,7 +395,8 @@ func (o *ApplyOptions) Run() error { return cmdutil.AddSourceToErr("serverside-apply", info.Source, err) } options := metav1.PatchOptions{ - Force: &o.ForceConflicts, + Force: &o.ForceConflicts, + FieldManager: o.FieldManager, } if o.ServerDryRun { options.DryRun = []string{metav1.DryRunAll} diff --git a/pkg/kubectl/cmd/diff/diff.go b/pkg/kubectl/cmd/diff/diff.go index b843e997c5..177c7755d6 100644 --- a/pkg/kubectl/cmd/diff/diff.go +++ b/pkg/kubectl/cmd/diff/diff.go @@ -402,7 +402,7 @@ func (o *DiffOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { o.ServerSideApply = cmdutil.GetServerSideApplyFlag(cmd) o.ForceConflicts = cmdutil.GetForceConflictsFlag(cmd) if o.ForceConflicts && !o.ServerSideApply { - return fmt.Errorf("--force-conflicts only works with --server-side") + return fmt.Errorf("--experimental-force-conflicts only works with --experimental-server-side") } if !o.ServerSideApply { diff --git a/pkg/kubectl/cmd/util/helpers.go b/pkg/kubectl/cmd/util/helpers.go index b02cfe0e81..805111f92f 100644 --- a/pkg/kubectl/cmd/util/helpers.go +++ b/pkg/kubectl/cmd/util/helpers.go @@ -406,8 +406,9 @@ func AddDryRunFlag(cmd *cobra.Command) { } func AddServerSideApplyFlags(cmd *cobra.Command) { - cmd.Flags().Bool("server-side", false, "If true, apply runs in the server instead of the client. This is an alpha feature and flag.") - cmd.Flags().Bool("force-conflicts", false, "If true, server-side apply will force the changes against conflicts. This is an alpha feature and flag.") + cmd.Flags().Bool("experimental-server-side", false, "If true, apply runs in the server instead of the client. This is an alpha feature and flag.") + cmd.Flags().Bool("experimental-force-conflicts", false, "If true, server-side apply will force the changes against conflicts. This is an alpha feature and flag.") + cmd.Flags().String("experimental-field-manager", "kubectl", "Name of the manager used to track field ownership. This is an alpha feature and flag.") } func AddIncludeUninitializedFlag(cmd *cobra.Command) { @@ -484,11 +485,15 @@ func DumpReaderToFile(reader io.Reader, filename string) error { } func GetServerSideApplyFlag(cmd *cobra.Command) bool { - return GetFlagBool(cmd, "server-side") + return GetFlagBool(cmd, "experimental-server-side") } func GetForceConflictsFlag(cmd *cobra.Command) bool { - return GetFlagBool(cmd, "force-conflicts") + return GetFlagBool(cmd, "experimental-force-conflicts") +} + +func GetFieldManagerFlag(cmd *cobra.Command) string { + return GetFlagString(cmd, "experimental-field-manager") } func GetDryRunFlag(cmd *cobra.Command) bool { diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/apply_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/apply_test.go index 595a740075..2621d3a122 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/apply_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/apply_test.go @@ -71,6 +71,7 @@ values: result, err := rest.Patch(types.ApplyPatchType). AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural). Name("mytest"). + Param("fieldManager", "apply_test"). Body(yamlBody). DoRaw() if err != nil { @@ -90,6 +91,7 @@ values: result, err = rest.Patch(types.ApplyPatchType). AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural). Name("mytest"). + Param("fieldManager", "apply_test"). Body(yamlBody). DoRaw() if err == nil { @@ -108,6 +110,7 @@ values: AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural). Name("mytest"). Param("force", "true"). + Param("fieldManager", "apply_test"). Body(yamlBody). DoRaw() if err != nil { 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 43e2cebe74..cf668c7c81 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta.go @@ -176,6 +176,9 @@ func ValidateObjectMetaAccessor(meta metav1.Object, requiresNamespace bool, name allErrs = append(allErrs, field.Invalid(fldPath.Child("clusterName"), meta.GetClusterName(), msg)) } } + for _, entry := range meta.GetManagedFields() { + allErrs = append(allErrs, v1validation.ValidateFieldManager(entry.Manager, fldPath.Child("fieldManager"))...) + } allErrs = append(allErrs, ValidateNonnegativeField(meta.GetGeneration(), fldPath.Child("generation"))...) allErrs = append(allErrs, v1validation.ValidateLabels(meta.GetLabels(), fldPath.Child("labels"))...) allErrs = append(allErrs, ValidateAnnotations(meta.GetAnnotations(), fldPath.Child("annotations"))...) @@ -239,6 +242,9 @@ func ValidateObjectMetaAccessorUpdate(newMeta, oldMeta metav1.Object, fldPath *f allErrs = append(allErrs, field.Invalid(fldPath.Child("generation"), newMeta.GetGeneration(), "must not be decremented")) } + for _, entry := range newMeta.GetManagedFields() { + allErrs = append(allErrs, v1validation.ValidateFieldManager(entry.Manager, fldPath.Child("fieldManager"))...) + } allErrs = append(allErrs, ValidateImmutableField(newMeta.GetName(), oldMeta.GetName(), fldPath.Child("name"))...) allErrs = append(allErrs, ValidateImmutableField(newMeta.GetNamespace(), oldMeta.GetNamespace(), fldPath.Child("namespace"))...) allErrs = append(allErrs, ValidateImmutableField(newMeta.GetUID(), oldMeta.GetUID(), fldPath.Child("uid"))...) diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go index e957849ba5..23cbc2c6df 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go @@ -506,6 +506,13 @@ type CreateOptions struct { // +optional DryRun []string `json:"dryRun,omitempty" protobuf:"bytes,1,rep,name=dryRun"` // +k8s:deprecated=includeUninitialized,protobuf=2 + + // fieldManager is a name associated with the actor or entity + // that is making these changes. The value must be less than or + // 128 characters long, and only contain printable characters, + // as defined by https://golang.org/pkg/unicode/#IsPrint. + // +optional + FieldManager string `json:"fieldManager,omitempty" protobuf:"bytes,3,name=fieldManager"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -528,6 +535,16 @@ type PatchOptions struct { // flag must be unset for non-apply patch requests. // +optional Force *bool `json:"force,omitempty" protobuf:"varint,2,opt,name=force"` + + // fieldManager is a name associated with the actor or entity + // that is making these changes. The value must be less than or + // 128 characters long, and only contain printable characters, + // as defined by https://golang.org/pkg/unicode/#IsPrint. This + // field is required for apply requests + // (application/apply-patch) but optional for non-apply patch + // types (JsonPatch, MergePatch, StrategicMergePatch). + // +optional + FieldManager string `json:"fieldManager,omitempty" protobuf:"bytes,3,name=fieldManager"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -544,6 +561,13 @@ type UpdateOptions struct { // - All: all dry run stages will be processed // +optional DryRun []string `json:"dryRun,omitempty" protobuf:"bytes,1,rep,name=dryRun"` + + // fieldManager is a name associated with the actor or entity + // that is making these changes. The value must be less than or + // 128 characters long, and only contain printable characters, + // as defined by https://golang.org/pkg/unicode/#IsPrint. + // +optional + FieldManager string `json:"fieldManager,omitempty" protobuf:"bytes,2,name=fieldManager"` } // Preconditions must be fulfilled before an operation (update, delete, etc.) is carried out. diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/BUILD b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/BUILD index 072713cbb4..822d81a6ed 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/BUILD @@ -10,7 +10,11 @@ go_test( name = "go_default_test", srcs = ["validation_test.go"], embed = [":go_default_library"], - deps = ["//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library"], + deps = [ + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", + ], ) go_library( diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go index 4ce2d70d0b..2e36ee23bb 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go @@ -17,6 +17,9 @@ limitations under the License. package validation import ( + "fmt" + "unicode" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" @@ -91,22 +94,58 @@ func ValidateDeleteOptions(options *metav1.DeleteOptions) field.ErrorList { } func ValidateCreateOptions(options *metav1.CreateOptions) field.ErrorList { - return ValidateDryRun(field.NewPath("dryRun"), options.DryRun) + return append( + ValidateFieldManager(options.FieldManager, field.NewPath("fieldManager")), + ValidateDryRun(field.NewPath("dryRun"), options.DryRun)..., + ) } func ValidateUpdateOptions(options *metav1.UpdateOptions) field.ErrorList { - return ValidateDryRun(field.NewPath("dryRun"), options.DryRun) + return append( + ValidateFieldManager(options.FieldManager, field.NewPath("fieldManager")), + ValidateDryRun(field.NewPath("dryRun"), options.DryRun)..., + ) } func ValidatePatchOptions(options *metav1.PatchOptions, patchType types.PatchType) field.ErrorList { allErrs := field.ErrorList{} - if patchType != types.ApplyPatchType && options.Force != nil { - allErrs = append(allErrs, field.Forbidden(field.NewPath("force"), "may not be specified for non-apply patch")) + if patchType != types.ApplyPatchType { + if options.Force != nil { + allErrs = append(allErrs, field.Forbidden(field.NewPath("force"), "may not be specified for non-apply patch")) + } + } else { + if options.FieldManager == "" { + // This field is defaulted to "kubectl" by kubectl, but HAS TO be explicitly set by controllers. + allErrs = append(allErrs, field.Required(field.NewPath("fieldManager"), "is required for apply patch")) + } } + allErrs = append(allErrs, ValidateFieldManager(options.FieldManager, field.NewPath("fieldManager"))...) allErrs = append(allErrs, ValidateDryRun(field.NewPath("dryRun"), options.DryRun)...) return allErrs } +var FieldManagerMaxLength = 128 + +// ValidateFieldManager valides that the fieldManager is the proper length and +// only has printable characters. +func ValidateFieldManager(fieldManager string, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + // the field can not be set as a `*string`, so a empty string ("") is + // considered as not set and is defaulted by the rest of the process + // (unless apply is used, in which case it is required). + if len(fieldManager) > FieldManagerMaxLength { + allErrs = append(allErrs, field.TooLong(fldPath, fieldManager, FieldManagerMaxLength)) + } + // Verify that all characters are printable. + for i, r := range fieldManager { + if !unicode.IsPrint(r) { + allErrs = append(allErrs, field.Invalid(fldPath, fieldManager, fmt.Sprintf("invalid character %#U (at position %d)", r, i))) + } + } + + return allErrs +} + var allowedDryRunValues = sets.NewString(metav1.DryRunAll) // ValidateDryRun validates that a dryRun query param only contains allowed values. diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go index bef8510973..454d52b18b 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go @@ -21,6 +21,8 @@ import ( "strings" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -125,3 +127,117 @@ func TestInvalidDryRun(t *testing.T) { } } + +func boolPtr(b bool) *bool { + return &b +} + +func TestValidPatchOptions(t *testing.T) { + tests := []struct { + opts metav1.PatchOptions + patchType types.PatchType + }{ + { + opts: metav1.PatchOptions{ + Force: boolPtr(true), + FieldManager: "kubectl", + }, + patchType: types.ApplyPatchType, + }, + { + opts: metav1.PatchOptions{ + FieldManager: "kubectl", + }, + patchType: types.ApplyPatchType, + }, + { + opts: metav1.PatchOptions{}, + patchType: types.MergePatchType, + }, + { + opts: metav1.PatchOptions{ + FieldManager: "patcher", + }, + patchType: types.MergePatchType, + }, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%v", test.opts), func(t *testing.T) { + errs := ValidatePatchOptions(&test.opts, test.patchType) + if len(errs) != 0 { + t.Fatalf("Expected no failures, got: %v", errs) + } + }) + } +} + +func TestInvalidPatchOptions(t *testing.T) { + tests := []struct { + opts metav1.PatchOptions + patchType types.PatchType + }{ + // missing manager + { + opts: metav1.PatchOptions{}, + patchType: types.ApplyPatchType, + }, + // force on non-apply + { + opts: metav1.PatchOptions{ + Force: boolPtr(true), + }, + patchType: types.MergePatchType, + }, + // manager and force on non-apply + { + opts: metav1.PatchOptions{ + FieldManager: "kubectl", + Force: boolPtr(false), + }, + patchType: types.MergePatchType, + }, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%v", test.opts), func(t *testing.T) { + errs := ValidatePatchOptions(&test.opts, test.patchType) + if len(errs) == 0 { + t.Fatal("Expected failures, got none.") + } + }) + } +} + +func TestValidateFieldManagerValid(t *testing.T) { + tests := []string{ + "filedManager", + "你好", // Hello + "🍔", + } + + for _, test := range tests { + t.Run(test, func(t *testing.T) { + errs := ValidateFieldManager(test, field.NewPath("fieldManager")) + if len(errs) != 0 { + t.Errorf("Validation failed: %v", errs) + } + }) + } +} + +func TestValidateFieldManagerInvalid(t *testing.T) { + tests := []string{ + "field\nmanager", // Contains invalid character \n + "fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", // Has 129 chars + } + + for _, test := range tests { + t.Run(test, func(t *testing.T) { + errs := ValidateFieldManager(test, field.NewPath("fieldManager")) + if len(errs) == 0 { + t.Errorf("Validation should have failed") + } + }) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD index cb7d20abf3..a0e445f715 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/BUILD @@ -9,6 +9,7 @@ load( go_test( name = "go_default_test", srcs = [ + "create_test.go", "namer_test.go", "rest_test.go", ], 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 067675a3bb..80e781fb88 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create_test.go new file mode 100644 index 0000000000..7609472767 --- /dev/null +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go index 08c9e0eb59..f761c8b87e 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go index 82aa6a84a3..f512c98b76 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index 50592c5d7a..bc06b7acf5 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/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/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go index e55d87092f..569fcb9dfa 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go +++ b/staging/src/k8s.io/apiserver/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) } diff --git a/test/cmd/apply.sh b/test/cmd/apply.sh index b78e00cedd..9158118098 100755 --- a/test/cmd/apply.sh +++ b/test/cmd/apply.sh @@ -249,12 +249,12 @@ run_kubectl_apply_tests() { set -o errexit create_and_use_new_namespace - kube::log::status "Testing kubectl apply --server-side" + kube::log::status "Testing kubectl apply --experimental-server-side" ## kubectl apply should create the resource that doesn't exist yet # Pre-Condition: no POD exists kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' # Command: apply a pod "test-pod" (doesn't exist) should create this pod - kubectl apply --server-side -f hack/testdata/pod.yaml "${kube_flags[@]}" + kubectl apply --experimental-server-side -f hack/testdata/pod.yaml "${kube_flags[@]}" # Post-Condition: pod "test-pod" is created kube::test::get_object_assert 'pods test-pod' "{{${labels_field}.name}}" 'test-pod-label' # Clean up @@ -265,13 +265,13 @@ run_kubectl_apply_tests() { kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' # apply dry-run - kubectl apply --server-side --server-dry-run -f hack/testdata/pod.yaml "${kube_flags[@]}" + kubectl apply --experimental-server-side --server-dry-run -f hack/testdata/pod.yaml "${kube_flags[@]}" # No pod exists kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' # apply non dry-run creates the pod - kubectl apply --server-side -f hack/testdata/pod.yaml "${kube_flags[@]}" + kubectl apply --experimental-server-side -f hack/testdata/pod.yaml "${kube_flags[@]}" # apply changes - kubectl apply --server-side --server-dry-run -f hack/testdata/pod-apply.yaml "${kube_flags[@]}" + kubectl apply --experimental-server-side --server-dry-run -f hack/testdata/pod-apply.yaml "${kube_flags[@]}" # Post-Condition: label still has initial value kube::test::get_object_assert 'pods test-pod' "{{${labels_field}.name}}" 'test-pod-label' @@ -302,7 +302,7 @@ run_kubectl_apply_tests() { __EOF__ # Dry-run create the CR - kubectl "${kube_flags[@]}" apply --server-side --server-dry-run -f hack/testdata/CRD/resource.yaml "${kube_flags[@]}" + kubectl "${kube_flags[@]}" apply --experimental-server-side --server-dry-run -f hack/testdata/CRD/resource.yaml "${kube_flags[@]}" # Make sure that the CR doesn't exist ! kubectl "${kube_flags[@]}" get resource/myobj diff --git a/test/integration/apiserver/apply/BUILD b/test/integration/apiserver/apply/BUILD index a8bb1c1f59..7e1574f7f7 100644 --- a/test/integration/apiserver/apply/BUILD +++ b/test/integration/apiserver/apply/BUILD @@ -2,10 +2,15 @@ load("@io_bazel_rules_go//go:def.bzl", "go_test") go_test( name = "go_default_test", + size = "large", srcs = [ "apply_test.go", "main_test.go", ], + tags = [ + "etcd", + "integration", + ], deps = [ "//pkg/master:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", diff --git a/test/integration/apiserver/apply/apply_test.go b/test/integration/apiserver/apply/apply_test.go index 81e937dc67..4d849b7cf4 100644 --- a/test/integration/apiserver/apply/apply_test.go +++ b/test/integration/apiserver/apply/apply_test.go @@ -106,6 +106,7 @@ func TestApplyAlsoCreates(t *testing.T) { Namespace("default"). Resource(tc.resource). Name(tc.name). + Param("fieldManager", "apply_test"). Body([]byte(tc.body)). Do(). Get() @@ -132,6 +133,7 @@ func TestCreateOnApplyFailsWithUID(t *testing.T) { Namespace("default"). Resource("pods"). Name("test-pod-uid"). + Param("fieldManager", "apply_test"). Body([]byte(`{ "apiVersion": "v1", "kind": "Pod", @@ -194,6 +196,7 @@ func TestApplyUpdateApplyConflictForced(t *testing.T) { Namespace("default"). Resource("deployments"). Name("deployment"). + Param("fieldManager", "apply_test"). Body(obj).Do().Get() if err != nil { t.Fatalf("Failed to create object using Apply patch: %v", err) @@ -214,6 +217,7 @@ func TestApplyUpdateApplyConflictForced(t *testing.T) { Namespace("default"). Resource("deployments"). Name("deployment"). + Param("fieldManager", "apply_test"). Body([]byte(obj)).Do().Get() if err == nil { t.Fatalf("Expecting to get conflicts when applying object") @@ -232,6 +236,7 @@ func TestApplyUpdateApplyConflictForced(t *testing.T) { Resource("deployments"). Name("deployment"). Param("force", "true"). + Param("fieldManager", "apply_test"). Body([]byte(obj)).Do().Get() if err != nil { t.Fatalf("Failed to apply object with force: %v", err) @@ -249,6 +254,7 @@ func TestApplyManagedFields(t *testing.T) { Namespace("default"). Resource("configmaps"). Name("test-cm"). + Param("fieldManager", "apply_test"). Body([]byte(`{ "apiVersion": "v1", "kind": "ConfigMap", @@ -273,6 +279,7 @@ func TestApplyManagedFields(t *testing.T) { Namespace("default"). Resource("configmaps"). Name("test-cm"). + Param("fieldManager", "updater"). Body([]byte(`{"data":{"key": "new value"}}`)).Do().Get() if err != nil { t.Fatalf("Failed to patch object: %v", err) @@ -306,7 +313,7 @@ func TestApplyManagedFields(t *testing.T) { }, "managedFields": [ { - "manager": "apply", + "manager": "apply_test", "operation": "Apply", "apiVersion": "v1", "fields": { @@ -318,7 +325,7 @@ func TestApplyManagedFields(t *testing.T) { } }, { - "manager": "` + accessor.GetManagedFields()[1].Manager + `", + "manager": "updater", "operation": "Update", "apiVersion": "v1", "time": "` + accessor.GetManagedFields()[1].Time.UTC().Format(time.RFC3339) + `", @@ -360,6 +367,7 @@ func TestApplyRemovesEmptyManagedFields(t *testing.T) { Namespace("default"). Resource("configmaps"). Name("test-cm"). + Param("fieldManager", "apply_test"). Body(obj). Do(). Get() @@ -371,6 +379,7 @@ func TestApplyRemovesEmptyManagedFields(t *testing.T) { Namespace("default"). Resource("configmaps"). Name("test-cm"). + Param("fieldManager", "apply_test"). Body(obj).Do().Get() if err != nil { t.Fatalf("Failed to patch object: %v", err) @@ -390,3 +399,42 @@ func TestApplyRemovesEmptyManagedFields(t *testing.T) { t.Fatalf("Object contains unexpected managedFields: %v", managed) } } + +func TestApplyRequiresFieldManager(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)() + + _, client, closeFn := setup(t) + defer closeFn() + + obj := []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "test-cm", + "namespace": "default" + } + }`) + + _, err := client.CoreV1().RESTClient().Patch(types.ApplyPatchType). + Namespace("default"). + Resource("configmaps"). + Name("test-cm"). + Body(obj). + Do(). + Get() + if err == nil { + t.Fatalf("Apply should fail to create without fieldManager") + } + + _, err = client.CoreV1().RESTClient().Patch(types.ApplyPatchType). + Namespace("default"). + Resource("configmaps"). + Name("test-cm"). + Param("fieldManager", "apply_test"). + Body(obj). + Do(). + Get() + if err != nil { + t.Fatalf("Apply failed to create with fieldManager: %v", err) + } +} diff --git a/test/integration/auth/rbac_test.go b/test/integration/auth/rbac_test.go index 950b1e94e2..93374f9bfa 100644 --- a/test/integration/auth/rbac_test.go +++ b/test/integration/auth/rbac_test.go @@ -567,6 +567,9 @@ func TestRBAC(t *testing.T) { if r.verb == "PATCH" { // For patch operations, use the apply content type req.Header.Add("Content-Type", string(types.ApplyPatchType)) + q := req.URL.Query() + q.Add("fieldManager", "rbac_test") + req.URL.RawQuery = q.Encode() } if err != nil {