From 5038eb2a3fa322878c22aff199995f00c30ee125 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Mon, 13 Nov 2017 23:01:51 -0500 Subject: [PATCH] Remove use of VersionedObject and simplify builder in generic methods Reduce all uses of Unstructured to the simpler form, and avoid asking for mapper or typer unless it is required. Use Typed() for places that previously used VersionedObject, and remove paths for versioned objects from code that is now using unstructured. --- pkg/kubectl/apply.go | 92 +++++----------- pkg/kubectl/cmd/annotate.go | 30 ++--- pkg/kubectl/cmd/apply.go | 23 ++-- pkg/kubectl/cmd/apply_set_last_applied.go | 24 ++-- pkg/kubectl/cmd/apply_test.go | 86 ++++++++------- pkg/kubectl/cmd/apply_view_last_applied.go | 7 +- pkg/kubectl/cmd/convert.go | 2 +- pkg/kubectl/cmd/create.go | 8 +- pkg/kubectl/cmd/delete.go | 10 +- pkg/kubectl/cmd/describe.go | 14 +-- pkg/kubectl/cmd/diff.go | 12 +- pkg/kubectl/cmd/label.go | 19 ++-- pkg/kubectl/cmd/patch.go | 10 +- pkg/kubectl/cmd/replace.go | 27 ++--- pkg/kubectl/cmd/resource/BUILD | 1 + pkg/kubectl/cmd/rollingupdate.go | 4 +- pkg/kubectl/cmd/set/BUILD | 1 - pkg/kubectl/cmd/set/helper.go | 11 +- pkg/kubectl/cmd/set/set_env.go | 17 ++- pkg/kubectl/cmd/set/set_image.go | 15 +-- pkg/kubectl/cmd/set/set_resources.go | 15 +-- pkg/kubectl/cmd/set/set_resources_test.go | 104 +++++++++--------- pkg/kubectl/cmd/set/set_selector.go | 15 +-- pkg/kubectl/cmd/set/set_serviceaccount.go | 15 +-- .../cmd/set/set_serviceaccount_test.go | 54 ++++----- pkg/kubectl/cmd/set/set_subject.go | 29 +++-- .../3.original | 2 +- pkg/kubectl/cmd/testing/BUILD | 1 + pkg/kubectl/cmd/util/editor/editoptions.go | 7 +- pkg/kubectl/cmd/util/factory.go | 1 + 30 files changed, 263 insertions(+), 393 deletions(-) diff --git a/pkg/kubectl/apply.go b/pkg/kubectl/apply.go index 784fc9b25c..fc716369a1 100644 --- a/pkg/kubectl/apply.go +++ b/pkg/kubectl/apply.go @@ -72,59 +72,33 @@ func GetModifiedConfiguration(info *resource.Info, annotate bool, codec runtime. // First serialize the object without the annotation to prevent recursion, // then add that serialization to it as the annotation and serialize it again. var modified []byte - if info.VersionedObject != nil { - // If an object was read from input, use that version. - accessor, err := meta.Accessor(info.VersionedObject) - if err != nil { - return nil, err - } - // Get the current annotations from the object. - annots := accessor.GetAnnotations() - if annots == nil { - annots = map[string]string{} - } + // Otherwise, use the server side version of the object. + accessor := info.Mapping.MetadataAccessor + // Get the current annotations from the object. + annots, err := accessor.Annotations(info.Object) + if err != nil { + return nil, err + } - original := annots[api.LastAppliedConfigAnnotation] - delete(annots, api.LastAppliedConfigAnnotation) - accessor.SetAnnotations(annots) - // TODO: this needs to be abstracted - there should be no assumption that versioned object - // can be marshalled to JSON. - modified, err = runtime.Encode(codec, info.VersionedObject) - if err != nil { - return nil, err - } + if annots == nil { + annots = map[string]string{} + } - if annotate { - annots[api.LastAppliedConfigAnnotation] = string(modified) - accessor.SetAnnotations(annots) - // TODO: this needs to be abstracted - there should be no assumption that versioned object - // can be marshalled to JSON. - modified, err = runtime.Encode(codec, info.VersionedObject) - if err != nil { - return nil, err - } - } + original := annots[api.LastAppliedConfigAnnotation] + delete(annots, api.LastAppliedConfigAnnotation) + if err := accessor.SetAnnotations(info.Object, annots); err != nil { + return nil, err + } - // Restore the object to its original condition. - annots[api.LastAppliedConfigAnnotation] = original - accessor.SetAnnotations(annots) - } else { - // Otherwise, use the server side version of the object. - accessor := info.Mapping.MetadataAccessor - // Get the current annotations from the object. - annots, err := accessor.Annotations(info.Object) - if err != nil { - return nil, err - } + modified, err = runtime.Encode(codec, info.Object) + if err != nil { + return nil, err + } - if annots == nil { - annots = map[string]string{} - } - - original := annots[api.LastAppliedConfigAnnotation] - delete(annots, api.LastAppliedConfigAnnotation) - if err := accessor.SetAnnotations(info.Object, annots); err != nil { + if annotate { + annots[api.LastAppliedConfigAnnotation] = string(modified) + if err := info.Mapping.MetadataAccessor.SetAnnotations(info.Object, annots); err != nil { return nil, err } @@ -132,24 +106,12 @@ func GetModifiedConfiguration(info *resource.Info, annotate bool, codec runtime. if err != nil { return nil, err } + } - if annotate { - annots[api.LastAppliedConfigAnnotation] = string(modified) - if err := info.Mapping.MetadataAccessor.SetAnnotations(info.Object, annots); err != nil { - return nil, err - } - - modified, err = runtime.Encode(codec, info.Object) - if err != nil { - return nil, err - } - } - - // Restore the object to its original condition. - annots[api.LastAppliedConfigAnnotation] = original - if err := info.Mapping.MetadataAccessor.SetAnnotations(info.Object, annots); err != nil { - return nil, err - } + // Restore the object to its original condition. + annots[api.LastAppliedConfigAnnotation] = original + if err := info.Mapping.MetadataAccessor.SetAnnotations(info.Object, annots); err != nil { + return nil, err } return modified, nil diff --git a/pkg/kubectl/cmd/annotate.go b/pkg/kubectl/cmd/annotate.go index d53b50144a..d80071ea6d 100644 --- a/pkg/kubectl/cmd/annotate.go +++ b/pkg/kubectl/cmd/annotate.go @@ -189,28 +189,22 @@ func (o AnnotateOptions) RunAnnotate(f cmdutil.Factory, cmd *cobra.Command) erro changeCause := f.Command(cmd, false) includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false) - b := f.NewBuilder(). - ContinueOnError(). + b := f.NewBuilder() + if o.local { + b = b.Local() + } else { + b = b.Unstructured() + } + b = b.ContinueOnError(). NamespaceParam(namespace).DefaultNamespace(). FilenameParam(enforceNamespace, &o.FilenameOptions). IncludeUninitialized(includeUninitialized). Flatten() if !o.local { - // call this method here, as it requires an api call - // and will cause the command to fail when there is - // no connection to a server - mapper, typer, err := f.UnstructuredObject() - if err != nil { - return err - } - b = b.LabelSelectorParam(o.selector). - Unstructured(f.UnstructuredClientForMapping, mapper, typer). ResourceTypeOrNameArgs(o.all, o.resources...). Latest() - } else { - b = b.Local(f.ClientForMapping) } r := b.Do() @@ -287,15 +281,7 @@ func (o AnnotateOptions) RunAnnotate(f cmdutil.Factory, cmd *cobra.Command) erro } } - var mapper meta.RESTMapper - if o.local { - mapper, _ = f.Object() - } else { - mapper, _, err = f.UnstructuredObject() - if err != nil { - return err - } - } + mapper := r.Mapper().RESTMapper if len(o.outputFormat) > 0 { return f.PrintObject(cmd, o.local, mapper, outputObj, o.out) } diff --git a/pkg/kubectl/cmd/apply.go b/pkg/kubectl/cmd/apply.go index a1ab8e6f5a..38d176345a 100644 --- a/pkg/kubectl/cmd/apply.go +++ b/pkg/kubectl/cmd/apply.go @@ -198,12 +198,12 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, opti return err } - mapper, typer, err := f.UnstructuredObject() - if err != nil { - return err - } - if options.Prune { + mapper, _, err := f.UnstructuredObject() + if err != nil { + return err + } + options.PruneResources, err = parsePruneResources(mapper, cmdutil.GetFlagStringArray(cmd, "prune-whitelist")) if err != nil { return err @@ -214,7 +214,7 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, opti // unless explicitly set --include-uninitialized=false includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, options.Prune) r := f.NewBuilder(). - Unstructured(f.UnstructuredClientForMapping, mapper, typer). + Unstructured(). Schema(schema). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). @@ -223,8 +223,7 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, opti IncludeUninitialized(includeUninitialized). Flatten(). Do() - err = r.Err() - if err != nil { + if err := r.Err(); err != nil { return err } @@ -234,14 +233,13 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, opti encoder := f.JSONEncoder() decoder := f.Decoder(false) + mapper := r.Mapper().RESTMapper visitedUids := sets.NewString() visitedNamespaces := sets.NewString() count := 0 err = r.Visit(func(info *resource.Info, err error) error { - // In this method, info.Object contains the object retrieved from the server - // and info.VersionedObject contains the object decoded from the input source. if err != nil { return err } @@ -252,10 +250,7 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, opti // Add change-cause annotation to resource info if it should be recorded if cmdutil.ShouldRecord(cmd, info) { - recordInObj := info.VersionedObject - if info.VersionedObject == nil { - recordInObj = info.Object - } + recordInObj := info.Object if err := cmdutil.RecordChangeCause(recordInObj, f.Command(cmd, false)); err != nil { glog.V(4).Infof("error recording current command: %v", err) } diff --git a/pkg/kubectl/cmd/apply_set_last_applied.go b/pkg/kubectl/cmd/apply_set_last_applied.go index 58328dea83..39ea7468a9 100644 --- a/pkg/kubectl/cmd/apply_set_last_applied.go +++ b/pkg/kubectl/cmd/apply_set_last_applied.go @@ -122,29 +122,19 @@ func (o *SetLastAppliedOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) } func (o *SetLastAppliedOptions) Validate(f cmdutil.Factory, cmd *cobra.Command) error { - mapper, typer, err := f.UnstructuredObject() - if err != nil { - return err - } - r := f.NewBuilder(). - Unstructured(f.UnstructuredClientForMapping, mapper, typer). + Unstructured(). NamespaceParam(o.Namespace).DefaultNamespace(). FilenameParam(o.EnforceNamespace, &o.FilenameOptions). Latest(). Flatten(). Do() - err = r.Err() - if err != nil { - return err - } - err = r.Visit(func(info *resource.Info, err error) error { + err := r.Visit(func(info *resource.Info, err error) error { if err != nil { return err } - - patchBuf, diffBuf, patchType, err := editor.GetApplyPatch(info.VersionedObject, o.Codec) + patchBuf, diffBuf, patchType, err := editor.GetApplyPatch(info.Object, o.Codec) if err != nil { return err } @@ -157,16 +147,16 @@ func (o *SetLastAppliedOptions) Validate(f cmdutil.Factory, cmd *cobra.Command) return cmdutil.AddSourceToErr(fmt.Sprintf("retrieving current configuration of:\n%v\nfrom server for:", info), info.Source, err) } } - oringalBuf, err := kubectl.GetOriginalConfiguration(info.Mapping, info.Object) + originalBuf, err := kubectl.GetOriginalConfiguration(info.Mapping, info.Object) if err != nil { return cmdutil.AddSourceToErr(fmt.Sprintf("retrieving current configuration of:\n%v\nfrom server for:", info), info.Source, err) } - if oringalBuf == nil && !o.CreateAnnotation { + if originalBuf == nil && !o.CreateAnnotation { return cmdutil.UsageErrorf(cmd, "no last-applied-configuration annotation found on resource: %s, to create the annotation, run the command with --create-annotation", info.Name) } //only add to PatchBufferList when changed - if !bytes.Equal(cmdutil.StripComments(oringalBuf), cmdutil.StripComments(diffBuf)) { + if !bytes.Equal(cmdutil.StripComments(originalBuf), cmdutil.StripComments(diffBuf)) { p := PatchBuffer{Patch: patchBuf, PatchType: patchType} o.PatchBufferList = append(o.PatchBufferList, p) o.InfoList = append(o.InfoList, info) @@ -234,7 +224,7 @@ func (o *SetLastAppliedOptions) getPatch(info *resource.Info) ([]byte, []byte, e objMap := map[string]map[string]map[string]string{} metadataMap := map[string]map[string]string{} annotationsMap := map[string]string{} - localFile, err := runtime.Encode(o.Codec, info.VersionedObject) + localFile, err := runtime.Encode(o.Codec, info.Object) if err != nil { return nil, localFile, err } diff --git a/pkg/kubectl/cmd/apply_test.go b/pkg/kubectl/cmd/apply_test.go index 84eaa967a3..c7f7eaa597 100644 --- a/pkg/kubectl/cmd/apply_test.go +++ b/pkg/kubectl/cmd/apply_test.go @@ -1014,51 +1014,53 @@ func TestRunApplySetLastApplied(t *testing.T) { }, } for _, test := range tests { - f, tf, codec, _ := cmdtesting.NewAPIFactory() - tf.Printer = &testPrinter{} - tf.UnstructuredClient = &fake.RESTClient{ - GroupVersion: schema.GroupVersion{Version: "v1"}, - NegotiatedSerializer: unstructuredSerializer, - Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { - switch p, m := req.URL.Path, req.Method; { - case p == pathRC && m == "GET": - bodyRC := ioutil.NopCloser(bytes.NewReader(currentRC)) - return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: bodyRC}, nil - case p == noAnnotationPath && m == "GET": - bodyRC := ioutil.NopCloser(bytes.NewReader(noAnnotationRC)) - return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: bodyRC}, nil - case p == noExistPath && m == "GET": - return &http.Response{StatusCode: 404, Header: defaultHeader(), Body: objBody(codec, &api.Pod{})}, nil - case p == pathRC && m == "PATCH": - checkPatchString(t, req) - bodyRC := ioutil.NopCloser(bytes.NewReader(currentRC)) - return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: bodyRC}, nil - case p == "/api/v1/namespaces/test" && m == "GET": - return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, &api.Namespace{})}, nil - default: - t.Fatalf("unexpected request: %#v\n%#v", req.URL, req) - return nil, nil - } - }), - } - tf.Namespace = "test" - tf.ClientConfig = defaultClientConfig() - buf, errBuf := bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{}) + t.Run(test.name, func(t *testing.T) { + f, tf, codec, _ := cmdtesting.NewAPIFactory() + tf.Printer = &testPrinter{} + tf.UnstructuredClient = &fake.RESTClient{ + GroupVersion: schema.GroupVersion{Version: "v1"}, + NegotiatedSerializer: unstructuredSerializer, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + switch p, m := req.URL.Path, req.Method; { + case p == pathRC && m == "GET": + bodyRC := ioutil.NopCloser(bytes.NewReader(currentRC)) + return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: bodyRC}, nil + case p == noAnnotationPath && m == "GET": + bodyRC := ioutil.NopCloser(bytes.NewReader(noAnnotationRC)) + return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: bodyRC}, nil + case p == noExistPath && m == "GET": + return &http.Response{StatusCode: 404, Header: defaultHeader(), Body: objBody(codec, &api.Pod{})}, nil + case p == pathRC && m == "PATCH": + checkPatchString(t, req) + bodyRC := ioutil.NopCloser(bytes.NewReader(currentRC)) + return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: bodyRC}, nil + case p == "/api/v1/namespaces/test" && m == "GET": + return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, &api.Namespace{})}, nil + default: + t.Fatalf("unexpected request: %#v\n%#v", req.URL, req) + return nil, nil + } + }), + } + tf.Namespace = "test" + tf.ClientConfig = defaultClientConfig() + buf, errBuf := bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{}) - cmdutil.BehaviorOnFatal(func(str string, code int) { - if str != test.expectedErr { - t.Errorf("%s: unexpected error: %s\nexpected: %s", test.name, str, test.expectedErr) + cmdutil.BehaviorOnFatal(func(str string, code int) { + if str != test.expectedErr { + t.Errorf("%s: unexpected error: %s\nexpected: %s", test.name, str, test.expectedErr) + } + }) + + cmd := NewCmdApplySetLastApplied(f, buf, errBuf) + cmd.Flags().Set("filename", test.filePath) + cmd.Flags().Set("output", test.output) + cmd.Run(cmd, []string{}) + + if buf.String() != test.expectedOut { + t.Fatalf("%s: unexpected output: %s\nexpected: %s", test.name, buf.String(), test.expectedOut) } }) - - cmd := NewCmdApplySetLastApplied(f, buf, errBuf) - cmd.Flags().Set("filename", test.filePath) - cmd.Flags().Set("output", test.output) - cmd.Run(cmd, []string{}) - - if buf.String() != test.expectedOut { - t.Fatalf("%s: unexpected output: %s\nexpected: %s", test.name, buf.String(), test.expectedOut) - } } cmdutil.BehaviorOnFatal(func(str string, code int) {}) } diff --git a/pkg/kubectl/cmd/apply_view_last_applied.go b/pkg/kubectl/cmd/apply_view_last_applied.go index 8692cffde1..7282ae6265 100644 --- a/pkg/kubectl/cmd/apply_view_last_applied.go +++ b/pkg/kubectl/cmd/apply_view_last_applied.go @@ -87,13 +87,8 @@ func (o *ViewLastAppliedOptions) Complete(f cmdutil.Factory, args []string) erro return err } - mapper, typer, err := f.UnstructuredObject() - if err != nil { - return err - } - r := f.NewBuilder(). - Unstructured(f.UnstructuredClientForMapping, mapper, typer). + Unstructured(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, &o.FilenameOptions). ResourceTypeOrNameArgs(enforceNamespace, args...). diff --git a/pkg/kubectl/cmd/convert.go b/pkg/kubectl/cmd/convert.go index c37ad0003c..51ffe60366 100644 --- a/pkg/kubectl/cmd/convert.go +++ b/pkg/kubectl/cmd/convert.go @@ -136,7 +136,7 @@ func (o *ConvertOptions) Complete(f cmdutil.Factory, out io.Writer, cmd *cobra.C } o.builder = o.builder.Schema(schema) } else { - o.builder = o.builder.Local(f.ClientForMapping) + o.builder = o.builder.Local() } cmdNamespace, _, err := f.DefaultNamespace() diff --git a/pkg/kubectl/cmd/create.go b/pkg/kubectl/cmd/create.go index 5b60874d71..e3340bcf22 100644 --- a/pkg/kubectl/cmd/create.go +++ b/pkg/kubectl/cmd/create.go @@ -184,13 +184,8 @@ func RunCreate(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, opt return err } - mapper, typer, err := f.UnstructuredObject() - if err != nil { - return err - } - r := f.NewBuilder(). - Unstructured(f.UnstructuredClientForMapping, mapper, typer). + Unstructured(). Schema(schema). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). @@ -205,6 +200,7 @@ func RunCreate(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, opt dryRun := cmdutil.GetFlagBool(cmd, "dry-run") output := cmdutil.GetFlagString(cmd, "output") + mapper := r.Mapper().RESTMapper count := 0 err = r.Visit(func(info *resource.Info, err error) error { diff --git a/pkg/kubectl/cmd/delete.go b/pkg/kubectl/cmd/delete.go index be087c5808..80a9bd4992 100644 --- a/pkg/kubectl/cmd/delete.go +++ b/pkg/kubectl/cmd/delete.go @@ -169,16 +169,9 @@ func (o *DeleteOptions) Complete(f cmdutil.Factory, out, errOut io.Writer, args return err } - // Set up client based support. - mapper, typer, err := f.UnstructuredObject() - if err != nil { - return err - } - - o.Mapper = mapper includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false) r := f.NewBuilder(). - Unstructured(f.UnstructuredClientForMapping, mapper, typer). + Unstructured(). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, &o.FilenameOptions). @@ -193,6 +186,7 @@ func (o *DeleteOptions) Complete(f cmdutil.Factory, out, errOut io.Writer, args return err } o.Result = r + o.Mapper = r.Mapper().RESTMapper o.f = f // Set up writer diff --git a/pkg/kubectl/cmd/describe.go b/pkg/kubectl/cmd/describe.go index 0f69c73164..e2343de17a 100644 --- a/pkg/kubectl/cmd/describe.go +++ b/pkg/kubectl/cmd/describe.go @@ -24,7 +24,6 @@ import ( "github.com/spf13/cobra" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kubernetes/pkg/kubectl" @@ -116,16 +115,11 @@ func RunDescribe(f cmdutil.Factory, out, cmdErr io.Writer, cmd *cobra.Command, a return cmdutil.UsageErrorf(cmd, "Required resource not specified.") } - mapper, typer, err := f.UnstructuredObject() - if err != nil { - return err - } - // include the uninitialized objects by default // unless user explicitly set --include-uninitialized=false includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, true) r := f.NewBuilder(). - Unstructured(f.UnstructuredClientForMapping, mapper, typer). + Unstructured(). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace().AllNamespaces(allNamespaces). FilenameParam(enforceNamespace, options). @@ -182,11 +176,7 @@ func RunDescribe(f cmdutil.Factory, out, cmdErr io.Writer, cmd *cobra.Command, a } func DescribeMatchingResources(f cmdutil.Factory, namespace, rsrc, prefix string, describerSettings *printers.DescriberSettings, out io.Writer, originalError error) error { - mapper, typer, err := f.UnstructuredObject() - if err != nil { - return err - } - r := resource.NewBuilder(mapper, f.CategoryExpander(), typer, resource.ClientMapperFunc(f.UnstructuredClientForMapping), unstructured.UnstructuredJSONScheme). + r := f.NewBuilder().Unstructured(). NamespaceParam(namespace).DefaultNamespace(). ResourceTypeOrNameArgs(true, rsrc). SingleResourceType(). diff --git a/pkg/kubectl/cmd/diff.go b/pkg/kubectl/cmd/diff.go index fb4fa2baa1..f70303328b 100644 --- a/pkg/kubectl/cmd/diff.go +++ b/pkg/kubectl/cmd/diff.go @@ -280,7 +280,7 @@ func (obj InfoObject) toMap(data []byte) (map[string]interface{}, error) { } func (obj InfoObject) Local() (map[string]interface{}, error) { - data, err := runtime.Encode(obj.Encoder, obj.Info.VersionedObject) + data, err := runtime.Encode(obj.Encoder, obj.Info.Object) if err != nil { return nil, err } @@ -408,24 +408,18 @@ func RunDiff(f cmdutil.Factory, diff *DiffProgram, options *DiffOptions, from, t printer := Printer{} - mapper, typer, err := f.UnstructuredObject() - if err != nil { - return err - } - cmdNamespace, enforceNamespace, err := f.DefaultNamespace() if err != nil { return err } r := f.NewBuilder(). - Unstructured(f.UnstructuredClientForMapping, mapper, typer). + Unstructured(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, &options.FilenameOptions). Flatten(). Do() - err = r.Err() - if err != nil { + if err := r.Err(); err != nil { return err } diff --git a/pkg/kubectl/cmd/label.go b/pkg/kubectl/cmd/label.go index 4ba688d0e1..2244e9d297 100644 --- a/pkg/kubectl/cmd/label.go +++ b/pkg/kubectl/cmd/label.go @@ -191,7 +191,13 @@ func (o *LabelOptions) RunLabel(f cmdutil.Factory, cmd *cobra.Command) error { changeCause := f.Command(cmd, false) includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false) - b := f.NewBuilder(). + b := f.NewBuilder() + if o.local { + b = b.Local() + } else { + b = b.Unstructured() + } + b = b. ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, &o.FilenameOptions). @@ -199,20 +205,9 @@ func (o *LabelOptions) RunLabel(f cmdutil.Factory, cmd *cobra.Command) error { Flatten() if !o.local { - // call this method here, as it requires an api call - // and will cause the command to fail when there is - // no connection to a server - mapper, typer, err := f.UnstructuredObject() - if err != nil { - return err - } - b = b.LabelSelectorParam(o.selector). - Unstructured(f.UnstructuredClientForMapping, mapper, typer). ResourceTypeOrNameArgs(o.all, o.resources...). Latest() - } else { - b = b.Local(f.ClientForMapping) } one := false diff --git a/pkg/kubectl/cmd/patch.go b/pkg/kubectl/cmd/patch.go index b60ca83a35..4ce2897be9 100644 --- a/pkg/kubectl/cmd/patch.go +++ b/pkg/kubectl/cmd/patch.go @@ -153,13 +153,8 @@ func RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin return fmt.Errorf("unable to parse %q: %v", patch, err) } - mapper, typer, err := f.UnstructuredObject() - if err != nil { - return err - } - r := f.NewBuilder(). - Unstructured(f.UnstructuredClientForMapping, mapper, typer). + Unstructured(). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, &options.FilenameOptions). @@ -196,7 +191,6 @@ func RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin // Copy the resource info and update with the result of applying the user's patch infoCopy := *info infoCopy.Object = patchedObj - infoCopy.VersionedObject = patchedObj if patch, patchType, err := cmdutil.ChangeResourcePatch(&infoCopy, f.Command(cmd, true)); err == nil { if recordedObj, err := helper.Patch(info.Namespace, info.Name, patchType, patch); err != nil { glog.V(4).Infof("error recording reason: %v", err) @@ -244,7 +238,7 @@ func RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin count++ - originalObjJS, err := runtime.Encode(unstructured.UnstructuredJSONScheme, info.VersionedObject) + originalObjJS, err := runtime.Encode(unstructured.UnstructuredJSONScheme, info.Object) if err != nil { return err } diff --git a/pkg/kubectl/cmd/replace.go b/pkg/kubectl/cmd/replace.go index 2ad34c99e7..dd8194bf1c 100644 --- a/pkg/kubectl/cmd/replace.go +++ b/pkg/kubectl/cmd/replace.go @@ -27,7 +27,6 @@ import ( "github.com/golang/glog" "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" @@ -120,24 +119,20 @@ func RunReplace(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str return fmt.Errorf("--timeout must have --force specified") } - mapper, typer, err := f.UnstructuredObject() - if err != nil { - return err - } - r := f.NewBuilder(). - Unstructured(f.UnstructuredClientForMapping, mapper, typer). + Unstructured(). Schema(schema). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, options). Flatten(). Do() - err = r.Err() - if err != nil { + if err := r.Err(); err != nil { return err } + mapper := r.Mapper().RESTMapper + return r.Visit(func(info *resource.Info, err error) error { if err != nil { return err @@ -193,21 +188,19 @@ func forceReplace(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []s } } - mapper, typer, err := f.UnstructuredObject() - if err != nil { - return err - } - r := resource.NewBuilder(mapper, f.CategoryExpander(), typer, resource.ClientMapperFunc(f.UnstructuredClientForMapping), unstructured.UnstructuredJSONScheme). + r := f.NewBuilder().Unstructured(). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, options). ResourceTypeOrNameArgs(false, args...).RequireObject(false). Flatten(). Do() - err = r.Err() - if err != nil { + if err := r.Err(); err != nil { return err } + + mapper := r.Mapper().RESTMapper + //Replace will create a resource if it doesn't exist already, so ignore not found error ignoreNotFound := true timeout := cmdutil.GetFlagDuration(cmd, "timeout") @@ -250,7 +243,7 @@ func forceReplace(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []s } r = f.NewBuilder(). - Unstructured(f.UnstructuredClientForMapping, mapper, typer). + Unstructured(). Schema(schema). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). diff --git a/pkg/kubectl/cmd/resource/BUILD b/pkg/kubectl/cmd/resource/BUILD index fce9d65f79..9c97f41d24 100644 --- a/pkg/kubectl/cmd/resource/BUILD +++ b/pkg/kubectl/cmd/resource/BUILD @@ -56,6 +56,7 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer/json:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer/streaming:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//vendor/k8s.io/apimachinery/pkg/watch:go_default_library", "//vendor/k8s.io/client-go/dynamic:go_default_library", "//vendor/k8s.io/client-go/rest:go_default_library", diff --git a/pkg/kubectl/cmd/rollingupdate.go b/pkg/kubectl/cmd/rollingupdate.go index 0aa23ab926..26ac3d3e71 100644 --- a/pkg/kubectl/cmd/rollingupdate.go +++ b/pkg/kubectl/cmd/rollingupdate.go @@ -395,11 +395,11 @@ func findNewName(args []string, oldRc *api.ReplicationController) string { } func isReplicasDefaulted(info *resource.Info) bool { - if info == nil || info.VersionedObject == nil { + if info == nil { // was unable to recover versioned info return false } - switch t := info.VersionedObject.(type) { + switch t := info.AsVersioned().(type) { case *v1.ReplicationController: return t.Spec.Replicas == nil } diff --git a/pkg/kubectl/cmd/set/BUILD b/pkg/kubectl/cmd/set/BUILD index 5e9c8e5895..f0f1848d89 100644 --- a/pkg/kubectl/cmd/set/BUILD +++ b/pkg/kubectl/cmd/set/BUILD @@ -19,7 +19,6 @@ go_library( importpath = "k8s.io/kubernetes/pkg/kubectl/cmd/set", visibility = ["//build/visible_to:pkg_kubectl_cmd_set_CONSUMERS"], deps = [ - "//pkg/api/legacyscheme:go_default_library", "//pkg/apis/rbac:go_default_library", "//pkg/kubectl:go_default_library", "//pkg/kubectl/cmd/templates:go_default_library", diff --git a/pkg/kubectl/cmd/set/helper.go b/pkg/kubectl/cmd/set/helper.go index 6f8bdfee80..5d6bed200d 100644 --- a/pkg/kubectl/cmd/set/helper.go +++ b/pkg/kubectl/cmd/set/helper.go @@ -126,14 +126,7 @@ type patchFn func(*resource.Info) ([]byte, error) // the changes in the object. Encoder must be able to encode the info into the appropriate destination type. // This function returns whether the mutation function made any change in the original object. func CalculatePatch(patch *Patch, encoder runtime.Encoder, mutateFn patchFn) bool { - versioned, err := patch.Info.Mapping.ConvertToVersion(patch.Info.Object, patch.Info.Mapping.GroupVersionKind.GroupVersion()) - if err != nil { - patch.Err = err - return true - } - patch.Info.VersionedObject = versioned - - patch.Before, patch.Err = runtime.Encode(encoder, patch.Info.VersionedObject) + patch.Before, patch.Err = runtime.Encode(encoder, patch.Info.Object) patch.After, patch.Err = mutateFn(patch.Info) if patch.Err != nil { return true @@ -142,7 +135,7 @@ func CalculatePatch(patch *Patch, encoder runtime.Encoder, mutateFn patchFn) boo return false } - patch.Patch, patch.Err = strategicpatch.CreateTwoWayMergePatch(patch.Before, patch.After, patch.Info.VersionedObject) + patch.Patch, patch.Err = strategicpatch.CreateTwoWayMergePatch(patch.Before, patch.After, patch.Info.Object) return true } diff --git a/pkg/kubectl/cmd/set/set_env.go b/pkg/kubectl/cmd/set/set_env.go index d4895119fd..8378b97edd 100644 --- a/pkg/kubectl/cmd/set/set_env.go +++ b/pkg/kubectl/cmd/set/set_env.go @@ -247,7 +247,7 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error { ResourceTypeOrNameArgs(o.All, o.From). Latest() } else { - b = b.Local(f.ClientForMapping) + b = b.Local() } infos, err := b.Do().Infos() @@ -315,7 +315,7 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error { ResourceTypeOrNameArgs(o.All, o.Resources...). Latest() } else { - b = b.Local(f.ClientForMapping) + b = b.Local() } o.Infos, err = b.Do().Infos() @@ -323,7 +323,8 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error { return err } patches := CalculatePatches(o.Infos, o.Encoder, func(info *resource.Info) ([]byte, error) { - _, err := o.UpdatePodSpecForObject(info.VersionedObject, func(spec *v1.PodSpec) error { + info.Object = info.AsVersioned() + _, err := o.UpdatePodSpecForObject(info.Object, func(spec *v1.PodSpec) error { resolutionErrorsEncountered := false containers, _ := selectContainers(spec.Containers, o.ContainerSelector) if len(containers) == 0 { @@ -389,7 +390,7 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error { }) if err == nil { - return runtime.Encode(o.Encoder, info.VersionedObject) + return runtime.Encode(o.Encoder, info.Object) } return nil, err }) @@ -413,7 +414,7 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error { } if o.PrintObject != nil && (o.Local || o.DryRun) { - if err := o.PrintObject(o.Cmd, o.Local, o.Mapper, patch.Info.VersionedObject, o.Out); err != nil { + if err := o.PrintObject(o.Cmd, o.Local, o.Mapper, patch.Info.AsVersioned(), o.Out); err != nil { return err } continue @@ -433,11 +434,7 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error { } if len(o.Output) > 0 { - versionedObject, err := patch.Info.Mapping.ConvertToVersion(obj, patch.Info.Mapping.GroupVersionKind.GroupVersion()) - if err != nil { - return err - } - if err := o.PrintObject(o.Cmd, o.Local, o.Mapper, versionedObject, o.Out); err != nil { + if err := o.PrintObject(o.Cmd, o.Local, o.Mapper, info.AsVersioned(), o.Out); err != nil { return err } continue diff --git a/pkg/kubectl/cmd/set/set_image.go b/pkg/kubectl/cmd/set/set_image.go index 4ddc30a04c..e6636620ed 100644 --- a/pkg/kubectl/cmd/set/set_image.go +++ b/pkg/kubectl/cmd/set/set_image.go @@ -162,7 +162,7 @@ func (o *ImageOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st return resource.LocalResourceError } - builder = builder.Local(f.ClientForMapping) + builder = builder.Local() } o.Infos, err = builder.Do().Infos() @@ -191,7 +191,8 @@ func (o *ImageOptions) Run() error { patches := CalculatePatches(o.Infos, o.Encoder, func(info *resource.Info) ([]byte, error) { transformed := false - _, err := o.UpdatePodSpecForObject(info.VersionedObject, func(spec *v1.PodSpec) error { + info.Object = info.AsVersioned() + _, err := o.UpdatePodSpecForObject(info.Object, func(spec *v1.PodSpec) error { for name, image := range o.ContainerImages { var ( containerFound bool @@ -228,7 +229,7 @@ func (o *ImageOptions) Run() error { return nil }) if transformed && err == nil { - return runtime.Encode(o.Encoder, info.VersionedObject) + return runtime.Encode(o.Encoder, info.Object) } return nil, err }) @@ -246,7 +247,7 @@ func (o *ImageOptions) Run() error { } if o.PrintObject != nil && (o.Local || o.DryRun) { - if err := o.PrintObject(o.Cmd, o.Local, o.Mapper, patch.Info.VersionedObject, o.Out); err != nil { + if err := o.PrintObject(o.Cmd, o.Local, o.Mapper, patch.Info.AsVersioned(), o.Out); err != nil { return err } continue @@ -272,11 +273,7 @@ func (o *ImageOptions) Run() error { info.Refresh(obj, true) if len(o.Output) > 0 { - versionedObject, err := patch.Info.Mapping.ConvertToVersion(obj, patch.Info.Mapping.GroupVersionKind.GroupVersion()) - if err != nil { - return err - } - if err := o.PrintObject(o.Cmd, o.Local, o.Mapper, versionedObject, o.Out); err != nil { + if err := o.PrintObject(o.Cmd, o.Local, o.Mapper, info.AsVersioned(), o.Out); err != nil { return err } continue diff --git a/pkg/kubectl/cmd/set/set_resources.go b/pkg/kubectl/cmd/set/set_resources.go index a2ea3e4659..bfa1fbfa02 100644 --- a/pkg/kubectl/cmd/set/set_resources.go +++ b/pkg/kubectl/cmd/set/set_resources.go @@ -164,7 +164,7 @@ func (o *ResourcesOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args return resource.LocalResourceError } - builder = builder.Local(f.ClientForMapping) + builder = builder.Local() } o.Infos, err = builder.Do().Infos() @@ -192,7 +192,8 @@ func (o *ResourcesOptions) Run() error { allErrs := []error{} patches := CalculatePatches(o.Infos, o.Encoder, func(info *resource.Info) ([]byte, error) { transformed := false - _, err := o.UpdatePodSpecForObject(info.VersionedObject, func(spec *v1.PodSpec) error { + info.Object = info.AsVersioned() + _, err := o.UpdatePodSpecForObject(info.Object, func(spec *v1.PodSpec) error { containers, _ := selectContainers(spec.Containers, o.ContainerSelector) if len(containers) != 0 { for i := range containers { @@ -217,7 +218,7 @@ func (o *ResourcesOptions) Run() error { return nil }) if transformed && err == nil { - return runtime.Encode(o.Encoder, info.VersionedObject) + return runtime.Encode(o.Encoder, info.Object) } return nil, err }) @@ -236,7 +237,7 @@ func (o *ResourcesOptions) Run() error { } if o.Local || cmdutil.GetDryRunFlag(o.Cmd) { - if err := o.PrintObject(o.Cmd, o.Local, o.Mapper, patch.Info.VersionedObject, o.Out); err != nil { + if err := o.PrintObject(o.Cmd, o.Local, o.Mapper, patch.Info.AsVersioned(), o.Out); err != nil { return err } continue @@ -261,11 +262,7 @@ func (o *ResourcesOptions) Run() error { shortOutput := o.Output == "name" if len(o.Output) > 0 && !shortOutput { - versionedObject, err := patch.Info.Mapping.ConvertToVersion(obj, patch.Info.Mapping.GroupVersionKind.GroupVersion()) - if err != nil { - return err - } - if err := o.PrintObject(o.Cmd, o.Local, o.Mapper, versionedObject, o.Out); err != nil { + if err := o.PrintObject(o.Cmd, o.Local, o.Mapper, info.AsVersioned(), o.Out); err != nil { return err } continue diff --git a/pkg/kubectl/cmd/set/set_resources_test.go b/pkg/kubectl/cmd/set/set_resources_test.go index b82aa28e01..306a43379c 100644 --- a/pkg/kubectl/cmd/set/set_resources_test.go +++ b/pkg/kubectl/cmd/set/set_resources_test.go @@ -442,57 +442,59 @@ func TestSetResourcesRemote(t *testing.T) { args: []string{"replicationcontroller", "nginx"}, }, } - for _, input := range inputs { - groupVersion := schema.GroupVersion{Group: input.apiGroup, Version: input.apiVersion} - testapi.Default = testapi.Groups[input.testAPIGroup] - f, tf, _, ns := cmdtesting.NewAPIFactory() - codec := scheme.Codecs.CodecForVersions(scheme.Codecs.LegacyCodec(groupVersion), scheme.Codecs.UniversalDecoder(groupVersion), groupVersion, groupVersion) - mapper, typer := f.Object() - tf.Printer = &printers.NamePrinter{Decoders: []runtime.Decoder{testapi.Default.Codec()}, Typer: typer, Mapper: mapper} - tf.Namespace = "test" - tf.CategoryExpander = categories.LegacyCategoryExpander - tf.Client = &fake.RESTClient{ - GroupVersion: groupVersion, - NegotiatedSerializer: ns, - Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { - resourcePath := testapi.Default.ResourcePath(input.args[0]+"s", tf.Namespace, input.args[1]) - switch p, m := req.URL.Path, req.Method; { - case p == resourcePath && m == http.MethodGet: - return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: objBody(codec, input.object)}, nil - case p == resourcePath && m == http.MethodPatch: - stream, err := req.GetBody() - if err != nil { - return nil, err + for i, input := range inputs { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + groupVersion := schema.GroupVersion{Group: input.apiGroup, Version: input.apiVersion} + testapi.Default = testapi.Groups[input.testAPIGroup] + f, tf, _, ns := cmdtesting.NewAPIFactory() + codec := scheme.Codecs.CodecForVersions(scheme.Codecs.LegacyCodec(groupVersion), scheme.Codecs.UniversalDecoder(groupVersion), groupVersion, groupVersion) + mapper, typer := f.Object() + tf.Printer = &printers.NamePrinter{Decoders: []runtime.Decoder{testapi.Default.Codec()}, Typer: typer, Mapper: mapper} + tf.Namespace = "test" + tf.CategoryExpander = categories.LegacyCategoryExpander + tf.Client = &fake.RESTClient{ + GroupVersion: groupVersion, + NegotiatedSerializer: ns, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + resourcePath := testapi.Default.ResourcePath(input.args[0]+"s", tf.Namespace, input.args[1]) + switch p, m := req.URL.Path, req.Method; { + case p == resourcePath && m == http.MethodGet: + return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: objBody(codec, input.object)}, nil + case p == resourcePath && m == http.MethodPatch: + stream, err := req.GetBody() + if err != nil { + return nil, err + } + bytes, err := ioutil.ReadAll(stream) + if err != nil { + return nil, err + } + assert.Contains(t, string(bytes), "200m", fmt.Sprintf("resources not updated for %#v", input.object)) + return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: objBody(codec, input.object)}, nil + default: + t.Errorf("%s: unexpected request: %s %#v\n%#v", "resources", req.Method, req.URL, req) + return nil, fmt.Errorf("unexpected request") } - bytes, err := ioutil.ReadAll(stream) - if err != nil { - return nil, err - } - assert.Contains(t, string(bytes), "200m", fmt.Sprintf("resources not updated for %#v", input.object)) - return &http.Response{StatusCode: http.StatusOK, Header: defaultHeader(), Body: objBody(codec, input.object)}, nil - default: - t.Errorf("%s: unexpected request: %s %#v\n%#v", "resources", req.Method, req.URL, req) - return nil, fmt.Errorf("unexpected request") - } - }), - VersionedAPIPath: path.Join(input.apiPrefix, testapi.Default.GroupVersion().String()), - } - buf := new(bytes.Buffer) - cmd := NewCmdResources(f, buf, buf) - cmd.SetOutput(buf) - cmd.Flags().Set("output", "yaml") - opts := ResourcesOptions{ - Out: buf, - Local: true, - Limits: "cpu=200m,memory=512Mi", - ContainerSelector: "*"} - err := opts.Complete(f, cmd, input.args) - if err == nil { - err = opts.Validate() - } - if err == nil { - err = opts.Run() - } - assert.NoError(t, err) + }), + VersionedAPIPath: path.Join(input.apiPrefix, testapi.Default.GroupVersion().String()), + } + buf := new(bytes.Buffer) + cmd := NewCmdResources(f, buf, buf) + cmd.SetOutput(buf) + cmd.Flags().Set("output", "yaml") + opts := ResourcesOptions{ + Out: buf, + Local: true, + Limits: "cpu=200m,memory=512Mi", + ContainerSelector: "*"} + err := opts.Complete(f, cmd, input.args) + if err == nil { + err = opts.Validate() + } + if err == nil { + err = opts.Run() + } + assert.NoError(t, err) + }) } } diff --git a/pkg/kubectl/cmd/set/set_selector.go b/pkg/kubectl/cmd/set/set_selector.go index c8873afed9..f4c5009b1c 100644 --- a/pkg/kubectl/cmd/set/set_selector.go +++ b/pkg/kubectl/cmd/set/set_selector.go @@ -147,7 +147,7 @@ func (o *SelectorOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [ return resource.LocalResourceError } - o.builder = o.builder.Local(f.ClientForMapping) + o.builder = o.builder.Local() } o.PrintObject = func(obj runtime.Object) error { @@ -181,15 +181,12 @@ func (o *SelectorOptions) RunSelector() error { return r.Visit(func(info *resource.Info, err error) error { patch := &Patch{Info: info} CalculatePatch(patch, o.encoder, func(info *resource.Info) ([]byte, error) { - versioned, err := info.Mapping.ConvertToVersion(info.Object, info.Mapping.GroupVersionKind.GroupVersion()) - if err != nil { - return nil, err - } - patch.Info.VersionedObject = versioned - selectErr := updateSelectorForObject(info.VersionedObject, *o.selector) + versioned := info.AsVersioned() + patch.Info.Object = versioned + selectErr := updateSelectorForObject(info.Object, *o.selector) if selectErr == nil { - return runtime.Encode(o.encoder, info.VersionedObject) + return runtime.Encode(o.encoder, info.Object) } return nil, selectErr }) @@ -198,7 +195,7 @@ func (o *SelectorOptions) RunSelector() error { return patch.Err } if o.local || o.dryrun { - return o.PrintObject(info.VersionedObject) + return o.PrintObject(info.Object) } patched, err := resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, types.StrategicMergePatchType, patch.Patch) diff --git a/pkg/kubectl/cmd/set/set_serviceaccount.go b/pkg/kubectl/cmd/set/set_serviceaccount.go index dec21af892..b499074965 100644 --- a/pkg/kubectl/cmd/set/set_serviceaccount.go +++ b/pkg/kubectl/cmd/set/set_serviceaccount.go @@ -138,7 +138,7 @@ func (saConfig *serviceAccountConfig) Complete(f cmdutil.Factory, cmd *cobra.Com builder.ResourceTypeOrNameArgs(saConfig.all, resources...). Latest() } else { - builder = builder.Local(f.ClientForMapping) + builder = builder.Local() } saConfig.infos, err = builder.Do().Infos() if err != nil { @@ -151,11 +151,12 @@ func (saConfig *serviceAccountConfig) Complete(f cmdutil.Factory, cmd *cobra.Com func (saConfig *serviceAccountConfig) Run() error { patchErrs := []error{} patchFn := func(info *resource.Info) ([]byte, error) { - saConfig.updatePodSpecForObject(info.VersionedObject, func(podSpec *v1.PodSpec) error { + info.Object = info.AsVersioned() + saConfig.updatePodSpecForObject(info.Object, func(podSpec *v1.PodSpec) error { podSpec.ServiceAccountName = saConfig.serviceAccountName return nil }) - return runtime.Encode(saConfig.encoder, info.VersionedObject) + return runtime.Encode(saConfig.encoder, info.Object) } patches := CalculatePatches(saConfig.infos, saConfig.encoder, patchFn) for _, patch := range patches { @@ -165,7 +166,7 @@ func (saConfig *serviceAccountConfig) Run() error { continue } if saConfig.local || saConfig.dryRun { - if err := saConfig.PrintObject(saConfig.cmd, saConfig.local, saConfig.mapper, patch.Info.VersionedObject, saConfig.out); err != nil { + if err := saConfig.PrintObject(saConfig.cmd, saConfig.local, saConfig.mapper, patch.Info.AsVersioned(), saConfig.out); err != nil { return err } continue @@ -184,11 +185,7 @@ func (saConfig *serviceAccountConfig) Run() error { } } if len(saConfig.output) > 0 { - versionedObject, err := patch.Info.Mapping.ConvertToVersion(patched, patch.Info.Mapping.GroupVersionKind.GroupVersion()) - if err != nil { - return err - } - if err := saConfig.PrintObject(saConfig.cmd, saConfig.local, saConfig.mapper, versionedObject, saConfig.out); err != nil { + if err := saConfig.PrintObject(saConfig.cmd, saConfig.local, saConfig.mapper, info.AsVersioned(), saConfig.out); err != nil { return err } continue diff --git a/pkg/kubectl/cmd/set/set_serviceaccount_test.go b/pkg/kubectl/cmd/set/set_serviceaccount_test.go index 11810cc18e..713ba1b5b3 100644 --- a/pkg/kubectl/cmd/set/set_serviceaccount_test.go +++ b/pkg/kubectl/cmd/set/set_serviceaccount_test.go @@ -66,32 +66,34 @@ func TestSetServiceAccountLocal(t *testing.T) { {yaml: "../../../../test/fixtures/doc-yaml/user-guide/deployment.yaml", apiGroup: "extensions"}, } - f, tf, _, _ := cmdtesting.NewAPIFactory() - tf.Client = &fake.RESTClient{ - GroupVersion: schema.GroupVersion{Version: "v1"}, - Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { - t.Fatalf("unexpected request: %s %#v\n%#v", req.Method, req.URL, req) - return nil, nil - }), - } - tf.Namespace = "test" - out := new(bytes.Buffer) - cmd := NewCmdServiceAccount(f, out, out) - cmd.SetOutput(out) - cmd.Flags().Set("output", "yaml") - cmd.Flags().Set("local", "true") - for _, input := range inputs { - testapi.Default = testapi.Groups[input.apiGroup] - tf.Printer = printers.NewVersionedPrinter(&printers.YAMLPrinter{}, testapi.Default.Converter(), *testapi.Default.GroupVersion()) - saConfig := serviceAccountConfig{fileNameOptions: resource.FilenameOptions{ - Filenames: []string{input.yaml}}, - out: out, - local: true} - err := saConfig.Complete(f, cmd, []string{serviceAccount}) - assert.NoError(t, err) - err = saConfig.Run() - assert.NoError(t, err) - assert.Contains(t, out.String(), "serviceAccountName: "+serviceAccount, fmt.Sprintf("serviceaccount not updated for %s", input.yaml)) + for i, input := range inputs { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + f, tf, _, _ := cmdtesting.NewAPIFactory() + tf.Client = &fake.RESTClient{ + GroupVersion: schema.GroupVersion{Version: "v1"}, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + t.Fatalf("unexpected request: %s %#v\n%#v", req.Method, req.URL, req) + return nil, nil + }), + } + tf.Namespace = "test" + out := new(bytes.Buffer) + cmd := NewCmdServiceAccount(f, out, out) + cmd.SetOutput(out) + cmd.Flags().Set("output", "yaml") + cmd.Flags().Set("local", "true") + testapi.Default = testapi.Groups[input.apiGroup] + tf.Printer = printers.NewVersionedPrinter(&printers.YAMLPrinter{}, testapi.Default.Converter(), *testapi.Default.GroupVersion()) + saConfig := serviceAccountConfig{fileNameOptions: resource.FilenameOptions{ + Filenames: []string{input.yaml}}, + out: out, + local: true} + err := saConfig.Complete(f, cmd, []string{serviceAccount}) + assert.NoError(t, err) + err = saConfig.Run() + assert.NoError(t, err) + assert.Contains(t, out.String(), "serviceAccountName: "+serviceAccount, fmt.Sprintf("serviceaccount not updated for %s", input.yaml)) + }) } } diff --git a/pkg/kubectl/cmd/set/set_subject.go b/pkg/kubectl/cmd/set/set_subject.go index 4a80fb613f..a76fbf507f 100644 --- a/pkg/kubectl/cmd/set/set_subject.go +++ b/pkg/kubectl/cmd/set/set_subject.go @@ -28,7 +28,6 @@ import ( "k8s.io/apimachinery/pkg/types" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/rbac" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" @@ -126,7 +125,17 @@ func (o *SubjectOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [] } includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false) - builder := f.NewBuilder(). + builder := f.NewBuilder() + if o.Local { + // if a --local flag was provided, and a resource was specified in the form + // /, fail immediately as --local cannot query the api server + // for the specified resource. + if len(args) > 0 { + return resource.LocalResourceError + } + builder = builder.Local() + } + builder = builder. ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, &o.FilenameOptions). @@ -138,15 +147,6 @@ func (o *SubjectOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [] LabelSelectorParam(o.Selector). ResourceTypeOrNameArgs(o.All, args...). Latest() - } else { - // if a --local flag was provided, and a resource was specified in the form - // /, fail immediately as --local cannot query the api server - // for the specified resource. - if len(args) > 0 { - return resource.LocalResourceError - } - - builder = builder.Local(f.ClientForMapping) } o.Infos, err = builder.Do().Infos() @@ -219,9 +219,8 @@ func (o *SubjectOptions) Run(f cmdutil.Factory, fn updateSubjects) error { transformed, err := updateSubjectForObject(info.Object, subjects, fn) if transformed && err == nil { - // TODO: switch UpdatePodSpecForObject to work on v1.PodSpec, use info.VersionedObject, and avoid conversion completely - versionedEncoder := legacyscheme.Codecs.EncoderForVersion(o.Encoder, info.Mapping.GroupVersionKind.GroupVersion()) - return runtime.Encode(versionedEncoder, info.Object) + // TODO: switch UpdatePodSpecForObject to work on v1.PodSpec + return runtime.Encode(o.Encoder, info.AsVersioned()) } return nil, err }) @@ -256,7 +255,7 @@ func (o *SubjectOptions) Run(f cmdutil.Factory, fn updateSubjects) error { shortOutput := o.Output == "name" if len(o.Output) > 0 && !shortOutput { - return o.PrintObject(o.Mapper, info.Object, o.Out) + return o.PrintObject(o.Mapper, info.AsVersioned(), o.Out) } f.PrintSuccess(o.Mapper, shortOutput, o.Out, info.Mapping.Resource, info.Name, false, "subjects updated") } diff --git a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/3.original b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/3.original index 28250a2099..ede23048bd 100644 --- a/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/3.original +++ b/pkg/kubectl/cmd/testdata/edit/testcase-apply-edit-last-applied-list-fail/3.original @@ -1,7 +1,7 @@ # Please edit the 'last-applied-configuration' annotations below. # Lines beginning with a '#' will be ignored, and an empty file will abort the edit. # -# The edited file had a syntax error: unable to get type info from the object "*unstructured.Unstructured": Object 'apiVersion' is missing in 'unstructured object has no version' +# The edited file had a syntax error: unable to get type info from the object "*unstructured.Unstructured": Object 'apiVersion' is missing in 'object has no apiVersion field' # apiVersion: v1 items: diff --git a/pkg/kubectl/cmd/testing/BUILD b/pkg/kubectl/cmd/testing/BUILD index b46a4a5b66..90fe0a0872 100644 --- a/pkg/kubectl/cmd/testing/BUILD +++ b/pkg/kubectl/cmd/testing/BUILD @@ -32,6 +32,7 @@ go_library( "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", diff --git a/pkg/kubectl/cmd/util/editor/editoptions.go b/pkg/kubectl/cmd/util/editor/editoptions.go index 1fada70ae8..0a7eab192e 100644 --- a/pkg/kubectl/cmd/util/editor/editoptions.go +++ b/pkg/kubectl/cmd/util/editor/editoptions.go @@ -107,12 +107,12 @@ func (o *EditOptions) Complete(f cmdutil.Factory, out, errOut io.Writer, args [] if err != nil { return err } - mapper, typer, err := f.UnstructuredObject() + mapper, _, err := f.UnstructuredObject() if err != nil { return err } - b := f.NewBuilder().Unstructured(f.UnstructuredClientForMapping, mapper, typer) + b := f.NewBuilder().Unstructured() if o.EditMode == NormalEditMode || o.EditMode == ApplyEditMode { // when do normal edit or apply edit we need to always retrieve the latest resource from server b = b.ResourceTypeOrNameArgs(true, args...).Latest() @@ -132,7 +132,8 @@ func (o *EditOptions) Complete(f cmdutil.Factory, out, errOut io.Writer, args [] o.updatedResultGetter = func(data []byte) *resource.Result { // resource builder to read objects from edited data - return resource.NewBuilder(mapper, f.CategoryExpander(), typer, resource.ClientMapperFunc(f.UnstructuredClientForMapping), unstructured.UnstructuredJSONScheme). + return f.NewBuilder(). + Unstructured(). Stream(bytes.NewReader(data), "edited-file"). IncludeUninitialized(includeUninitialized). ContinueOnError(). diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 037a0351a2..ffa7f2344e 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -193,6 +193,7 @@ type ObjectMappingFactory interface { // runtime.Unstructured. This performs API calls to discover types. UnstructuredObject() (meta.RESTMapper, runtime.ObjectTyper, error) // Returns interface for expanding categories like `all`. + // TODO: this should probably return an error if the full expander can't be loaded. CategoryExpander() categories.CategoryExpander // Returns a RESTClient for working with the specified RESTMapping or an error. This is intended // for working with arbitrary resources and is not guaranteed to point to a Kubernetes APIServer.