From 8442a118ea91c1ccb6a4b2d1505359732608b5cf Mon Sep 17 00:00:00 2001 From: Waseem Ahmad Date: Tue, 23 May 2017 08:17:20 +0530 Subject: [PATCH] Denote if a printer is generic. This fixes #38779. This allows us to avoid case in which printers.GetStandardPrinter returns nil for both printer and err removing any potential panics that may arise throughout kubectl commands. Please see #38779 and #38112 for complete context. Add comment explaining adding handlers to printers.HumanReadablePrinter also remove an unnecessary conversion of printers.HumanReadablePrinter to printers.ResourcePrinter. --- pkg/kubectl/cmd/apply_test.go | 3 +- pkg/kubectl/cmd/cmd_test.go | 9 +++- pkg/kubectl/cmd/config/view.go | 2 +- pkg/kubectl/cmd/convert.go | 2 +- pkg/kubectl/cmd/create_clusterrole_test.go | 4 ++ pkg/kubectl/cmd/create_role_test.go | 4 ++ pkg/kubectl/cmd/get.go | 10 ++-- pkg/kubectl/cmd/get_test.go | 28 +++++----- pkg/kubectl/cmd/patch_test.go | 3 +- pkg/kubectl/cmd/testing/fake.go | 10 ++-- pkg/kubectl/cmd/util/factory.go | 6 +-- pkg/kubectl/cmd/util/factory_builder.go | 54 +++++++++++-------- pkg/kubectl/cmd/util/printing.go | 14 ++--- pkg/kubectl/sorting_printer.go | 6 ++- pkg/printers/customcolumn.go | 4 ++ pkg/printers/humanreadable.go | 4 ++ pkg/printers/interface.go | 6 +++ pkg/printers/internalversion/printers_test.go | 31 +++++++---- pkg/printers/json.go | 8 +++ pkg/printers/jsonpath.go | 4 ++ pkg/printers/name.go | 4 ++ pkg/printers/printers.go | 54 +++++++++++-------- pkg/printers/template.go | 4 ++ pkg/printers/versioned.go | 4 ++ 24 files changed, 182 insertions(+), 96 deletions(-) diff --git a/pkg/kubectl/cmd/apply_test.go b/pkg/kubectl/cmd/apply_test.go index 23f17179c46ef..75a050371e392 100644 --- a/pkg/kubectl/cmd/apply_test.go +++ b/pkg/kubectl/cmd/apply_test.go @@ -456,8 +456,7 @@ func TestApplyObjectOutput(t *testing.T) { } f, tf, _, _ := cmdtesting.NewAPIFactory() - tf.CommandPrinter = &printers.YAMLPrinter{} - tf.GenericPrinter = true + tf.Printer = &printers.YAMLPrinter{} tf.UnstructuredClient = &fake.RESTClient{ APIRegistry: api.Registry, NegotiatedSerializer: unstructuredSerializer, diff --git a/pkg/kubectl/cmd/cmd_test.go b/pkg/kubectl/cmd/cmd_test.go index 6fec9f56fa4d1..bca86c6496188 100644 --- a/pkg/kubectl/cmd/cmd_test.go +++ b/pkg/kubectl/cmd/cmd_test.go @@ -80,8 +80,9 @@ func defaultClientConfigForVersion(version *schema.GroupVersion) *restclient.Con } type testPrinter struct { - Objects []runtime.Object - Err error + Objects []runtime.Object + Err error + GenericPrinter bool } func (t *testPrinter) PrintObj(obj runtime.Object, out io.Writer) error { @@ -99,6 +100,10 @@ func (t *testPrinter) AfterPrint(output io.Writer, res string) error { return nil } +func (t *testPrinter) IsGeneric() bool { + return t.GenericPrinter +} + type testDescriber struct { Name, Namespace string Settings printers.DescriberSettings diff --git a/pkg/kubectl/cmd/config/view.go b/pkg/kubectl/cmd/config/view.go index 4c7cdb665d346..9f6bf449b103e 100644 --- a/pkg/kubectl/cmd/config/view.go +++ b/pkg/kubectl/cmd/config/view.go @@ -81,7 +81,7 @@ func NewCmdConfigView(out, errOut io.Writer, ConfigAccess clientcmd.ConfigAccess cmd.Flags().Set("output", defaultOutputFormat) } - printer, _, err := cmdutil.PrinterForCommand(cmd, meta.NewDefaultRESTMapper(nil, nil), latest.Scheme, []runtime.Decoder{latest.Codec}) + printer, err := cmdutil.PrinterForCommand(cmd, meta.NewDefaultRESTMapper(nil, nil), latest.Scheme, nil, []runtime.Decoder{latest.Codec}, printers.PrintOptions{}) cmdutil.CheckErr(err) printer = printers.NewVersionedPrinter(printer, latest.Scheme, latest.ExternalVersion) diff --git a/pkg/kubectl/cmd/convert.go b/pkg/kubectl/cmd/convert.go index 64e992240d2f1..c9bae684c63fe 100644 --- a/pkg/kubectl/cmd/convert.go +++ b/pkg/kubectl/cmd/convert.go @@ -164,7 +164,7 @@ func (o *ConvertOptions) Complete(f cmdutil.Factory, out io.Writer, cmd *cobra.C cmd.Flags().Set("output", outputFormat) } o.encoder = f.JSONEncoder() - o.printer, _, err = f.PrinterForCommand(cmd) + o.printer, err = f.PrinterForCommand(cmd, printers.PrintOptions{}) if err != nil { return err } diff --git a/pkg/kubectl/cmd/create_clusterrole_test.go b/pkg/kubectl/cmd/create_clusterrole_test.go index 750c12acbc9d3..299fc85892fa5 100644 --- a/pkg/kubectl/cmd/create_clusterrole_test.go +++ b/pkg/kubectl/cmd/create_clusterrole_test.go @@ -46,6 +46,10 @@ func (t *testClusterRolePrinter) HandledResources() []string { return []string{} } +func (t *testClusterRolePrinter) IsGeneric() bool { + return true +} + func TestCreateClusterRole(t *testing.T) { clusterRoleName := "my-cluster-role" diff --git a/pkg/kubectl/cmd/create_role_test.go b/pkg/kubectl/cmd/create_role_test.go index a15ef075cc4ab..59e86b8cb346d 100644 --- a/pkg/kubectl/cmd/create_role_test.go +++ b/pkg/kubectl/cmd/create_role_test.go @@ -46,6 +46,10 @@ func (t *testRolePrinter) HandledResources() []string { return []string{} } +func (t *testRolePrinter) IsGeneric() bool { + return true +} + func TestCreateRole(t *testing.T) { roleName := "my-role" diff --git a/pkg/kubectl/cmd/get.go b/pkg/kubectl/cmd/get.go index a8fe9cda165b8..385eca6286903 100644 --- a/pkg/kubectl/cmd/get.go +++ b/pkg/kubectl/cmd/get.go @@ -302,7 +302,7 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ return err } - printer, generic, err := f.PrinterForCommand(cmd) + printer, err := f.PrinterForCommand(cmd, printers.PrintOptions{}) if err != nil { return err } @@ -313,7 +313,7 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ r.IgnoreErrors(kapierrors.IsNotFound) } - if generic { + if printer.IsGeneric() { // we flattened the data from the builder, so we have individual items, but now we'd like to either: // 1. if there is more than one item, combine them all into a single list // 2. if there is a single item and that item is a list, leave it as its specific list @@ -436,7 +436,7 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ mapping = infos[ix].Mapping original = infos[ix].Object } - if printer == nil || lastMapping == nil || mapping == nil || mapping.Resource != lastMapping.Resource { + if shouldGetNewPrinterForMapping(printer, lastMapping, mapping) { if printer != nil { w.Flush() } @@ -513,3 +513,7 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ cmdutil.PrintFilterCount(errOut, len(objs), filteredResourceCount, len(allErrs), filterOpts, options.IgnoreNotFound) return utilerrors.NewAggregate(allErrs) } + +func shouldGetNewPrinterForMapping(printer printers.ResourcePrinter, lastMapping, mapping *meta.RESTMapping) bool { + return printer == nil || lastMapping == nil || mapping == nil || mapping.Resource != lastMapping.Resource +} diff --git a/pkg/kubectl/cmd/get_test.go b/pkg/kubectl/cmd/get_test.go index 265fa0e08ca77..48e99b9e0145d 100644 --- a/pkg/kubectl/cmd/get_test.go +++ b/pkg/kubectl/cmd/get_test.go @@ -220,15 +220,16 @@ func TestGetObjectsFiltered(t *testing.T) { second := &pods.Items[1] testCases := []struct { - args []string - resp runtime.Object - flags map[string]string - expect []runtime.Object + args []string + resp runtime.Object + flags map[string]string + expect []runtime.Object + genericPrinter bool }{ - {args: []string{"pods", "foo"}, resp: first, expect: []runtime.Object{first}}, - {args: []string{"pods", "foo"}, flags: map[string]string{"show-all": "false"}, resp: first, expect: []runtime.Object{first}}, + {args: []string{"pods", "foo"}, resp: first, expect: []runtime.Object{first}, genericPrinter: true}, + {args: []string{"pods", "foo"}, flags: map[string]string{"show-all": "false"}, resp: first, expect: []runtime.Object{first}, genericPrinter: true}, {args: []string{"pods"}, flags: map[string]string{"show-all": "true"}, resp: pods, expect: []runtime.Object{first, second}}, - {args: []string{"pods/foo"}, resp: first, expect: []runtime.Object{first}}, + {args: []string{"pods/foo"}, resp: first, expect: []runtime.Object{first}, genericPrinter: true}, {args: []string{"pods"}, flags: map[string]string{"output": "yaml"}, resp: pods, expect: []runtime.Object{second}}, {args: []string{}, flags: map[string]string{"filename": "../../../examples/storage/cassandra/cassandra-controller.yaml"}, resp: pods, expect: []runtime.Object{first, second}}, @@ -240,7 +241,7 @@ func TestGetObjectsFiltered(t *testing.T) { for i, test := range testCases { t.Logf("%d", i) f, tf, codec, _ := cmdtesting.NewAPIFactory() - tf.Printer = &testPrinter{} + tf.Printer = &testPrinter{GenericPrinter: test.genericPrinter} tf.UnstructuredClient = &fake.RESTClient{ APIRegistry: api.Registry, NegotiatedSerializer: unstructuredSerializer, @@ -281,7 +282,7 @@ func TestGetObjectIgnoreNotFound(t *testing.T) { } f, tf, codec, _ := cmdtesting.NewAPIFactory() - tf.Printer = &testPrinter{} + tf.Printer = &testPrinter{GenericPrinter: true} tf.UnstructuredClient = &fake.RESTClient{ APIRegistry: api.Registry, NegotiatedSerializer: unstructuredSerializer, @@ -393,7 +394,7 @@ func TestGetObjectsIdentifiedByFile(t *testing.T) { pods, _, _ := testData() f, tf, codec, _ := cmdtesting.NewAPIFactory() - tf.Printer = &testPrinter{} + tf.Printer = &testPrinter{GenericPrinter: true} tf.UnstructuredClient = &fake.RESTClient{ APIRegistry: api.Registry, NegotiatedSerializer: unstructuredSerializer, @@ -561,8 +562,7 @@ func TestGetMultipleTypeObjectsAsList(t *testing.T) { pods, svc, _ := testData() f, tf, codec, _ := cmdtesting.NewAPIFactory() - tf.CommandPrinter = &testPrinter{} - tf.GenericPrinter = true + tf.Printer = &testPrinter{GenericPrinter: true} tf.UnstructuredClient = &fake.RESTClient{ APIRegistry: api.Registry, NegotiatedSerializer: unstructuredSerializer, @@ -589,7 +589,7 @@ func TestGetMultipleTypeObjectsAsList(t *testing.T) { cmd.Flags().Set("output", "json") cmd.Run(cmd, []string{"pods,services"}) - actual := tf.CommandPrinter.(*testPrinter).Objects + actual := tf.Printer.(*testPrinter).Objects fn := func(obj runtime.Object) unstructured.Unstructured { data, err := runtime.Encode(api.Codecs.LegacyCodec(schema.GroupVersion{Version: "v1"}), obj) if err != nil { @@ -716,7 +716,7 @@ func TestGetByFormatForcesFlag(t *testing.T) { pods, _, _ := testData() f, tf, codec, _ := cmdtesting.NewAPIFactory() - tf.Printer = &testPrinter{} + tf.Printer = &testPrinter{GenericPrinter: true} tf.UnstructuredClient = &fake.RESTClient{ APIRegistry: api.Registry, NegotiatedSerializer: unstructuredSerializer, diff --git a/pkg/kubectl/cmd/patch_test.go b/pkg/kubectl/cmd/patch_test.go index 5908afbe0d263..6c23d126fbd39 100644 --- a/pkg/kubectl/cmd/patch_test.go +++ b/pkg/kubectl/cmd/patch_test.go @@ -161,8 +161,7 @@ func TestPatchObjectFromFileOutput(t *testing.T) { svcCopy.Labels["post-patch"] = "post-patch-value" f, tf, codec, _ := cmdtesting.NewAPIFactory() - tf.CommandPrinter = &printers.YAMLPrinter{} - tf.GenericPrinter = true + tf.Printer = &printers.YAMLPrinter{} tf.UnstructuredClient = &fake.RESTClient{ APIRegistry: api.Registry, NegotiatedSerializer: unstructuredSerializer, diff --git a/pkg/kubectl/cmd/testing/fake.go b/pkg/kubectl/cmd/testing/fake.go index 646c0d1efb2aa..d940ec0bf4375 100644 --- a/pkg/kubectl/cmd/testing/fake.go +++ b/pkg/kubectl/cmd/testing/fake.go @@ -221,13 +221,11 @@ type TestFactory struct { UnstructuredClient kubectl.RESTClient Describer printers.Describer Printer printers.ResourcePrinter - CommandPrinter printers.ResourcePrinter Validator validation.Schema Namespace string ClientConfig *restclient.Config Err error Command string - GenericPrinter bool TmpDir string ClientForMappingFunc func(mapping *meta.RESTMapping) (resource.RESTClient, error) @@ -338,8 +336,8 @@ func (f *FakeFactory) Describer(*meta.RESTMapping) (printers.Describer, error) { return f.tf.Describer, f.tf.Err } -func (f *FakeFactory) PrinterForCommand(cmd *cobra.Command) (printers.ResourcePrinter, bool, error) { - return f.tf.CommandPrinter, f.tf.GenericPrinter, f.tf.Err +func (f *FakeFactory) PrinterForCommand(cmd *cobra.Command, options printers.PrintOptions) (printers.ResourcePrinter, error) { + return f.tf.Printer, f.tf.Err } func (f *FakeFactory) Printer(mapping *meta.RESTMapping, options printers.PrintOptions) (printers.ResourcePrinter, error) { @@ -619,8 +617,8 @@ func (f *fakeAPIFactory) UnstructuredClientForMapping(m *meta.RESTMapping) (reso return f.tf.UnstructuredClient, f.tf.Err } -func (f *fakeAPIFactory) PrinterForCommand(cmd *cobra.Command) (printers.ResourcePrinter, bool, error) { - return f.tf.CommandPrinter, f.tf.GenericPrinter, f.tf.Err +func (f *fakeAPIFactory) PrinterForCommand(cmd *cobra.Command, options printers.PrintOptions) (printers.ResourcePrinter, error) { + return f.tf.Printer, f.tf.Err } func (f *fakeAPIFactory) Describer(*meta.RESTMapping) (printers.Describer, error) { diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 23ce0c04342a5..8c3d8f1c34f1f 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -230,10 +230,10 @@ type ObjectMappingFactory interface { // Generally they depend upon client mapper functions type BuilderFactory interface { // PrinterForCommand returns the default printer for the command. It requires that certain options - // are declared on the command (see AddPrinterFlags). Returns a printer, true if the printer is - // generic (is not internal), or an error if a printer could not be found. + // are declared on the command (see AddPrinterFlags). Returns a printer, or an error if a printer + // could not be found. // TODO: Break the dependency on cmd here. - PrinterForCommand(cmd *cobra.Command) (printers.ResourcePrinter, bool, error) + PrinterForCommand(cmd *cobra.Command, options printers.PrintOptions) (printers.ResourcePrinter, error) // PrinterForMapping returns a printer suitable for displaying the provided resource type. // Requires that printer flags have been added to cmd (see AddPrinterFlags). PrinterForMapping(cmd *cobra.Command, mapping *meta.RESTMapping, withNamespace bool) (printers.ResourcePrinter, error) diff --git a/pkg/kubectl/cmd/util/factory_builder.go b/pkg/kubectl/cmd/util/factory_builder.go index bc56994cbdf37..0d122843dc6ea 100644 --- a/pkg/kubectl/cmd/util/factory_builder.go +++ b/pkg/kubectl/cmd/util/factory_builder.go @@ -31,6 +31,7 @@ import ( "k8s.io/kubernetes/pkg/kubectl/plugins" "k8s.io/kubernetes/pkg/kubectl/resource" "k8s.io/kubernetes/pkg/printers" + printersinternal "k8s.io/kubernetes/pkg/printers/internalversion" ) type ring2Factory struct { @@ -47,24 +48,41 @@ func NewBuilderFactory(clientAccessFactory ClientAccessFactory, objectMappingFac return f } -func (f *ring2Factory) PrinterForCommand(cmd *cobra.Command) (printers.ResourcePrinter, bool, error) { +func (f *ring2Factory) PrinterForCommand(cmd *cobra.Command, options printers.PrintOptions) (printers.ResourcePrinter, error) { mapper, typer, err := f.objectMappingFactory.UnstructuredObject() if err != nil { - return nil, false, err + return nil, err } // TODO: used by the custom column implementation and the name implementation, break this dependency decoders := []runtime.Decoder{f.clientAccessFactory.Decoder(true), unstructured.UnstructuredJSONScheme} - return PrinterForCommand(cmd, mapper, typer, decoders) + encoder := f.clientAccessFactory.JSONEncoder() + return PrinterForCommand(cmd, mapper, typer, encoder, decoders, options) } func (f *ring2Factory) PrinterForMapping(cmd *cobra.Command, mapping *meta.RESTMapping, withNamespace bool) (printers.ResourcePrinter, error) { - printer, generic, err := f.PrinterForCommand(cmd) + // Some callers do not have "label-columns" so we can't use the GetFlagStringSlice() helper + columnLabel, err := cmd.Flags().GetStringSlice("label-columns") + if err != nil { + columnLabel = []string{} + } + + options := printers.PrintOptions{ + NoHeaders: GetFlagBool(cmd, "no-headers"), + WithNamespace: withNamespace, + Wide: GetWideFlag(cmd), + ShowAll: GetFlagBool(cmd, "show-all"), + ShowLabels: GetFlagBool(cmd, "show-labels"), + AbsoluteTimestamps: isWatch(cmd), + ColumnLabels: columnLabel, + } + + printer, err := f.PrinterForCommand(cmd, options) if err != nil { return nil, err } // Make sure we output versioned data for generic printers - if generic { + if printer.IsGeneric() { if mapping == nil { return nil, fmt.Errorf("no serialization format found") } @@ -74,25 +92,17 @@ func (f *ring2Factory) PrinterForMapping(cmd *cobra.Command, mapping *meta.RESTM } printer = printers.NewVersionedPrinter(printer, mapping.ObjectConvertor, version, mapping.GroupVersionKind.GroupVersion()) + } else { - // Some callers do not have "label-columns" so we can't use the GetFlagStringSlice() helper - columnLabel, err := cmd.Flags().GetStringSlice("label-columns") - if err != nil { - columnLabel = []string{} - } - printer, err = f.clientAccessFactory.Printer(mapping, printers.PrintOptions{ - NoHeaders: GetFlagBool(cmd, "no-headers"), - WithNamespace: withNamespace, - Wide: GetWideFlag(cmd), - ShowAll: GetFlagBool(cmd, "show-all"), - ShowLabels: GetFlagBool(cmd, "show-labels"), - AbsoluteTimestamps: isWatch(cmd), - ColumnLabels: columnLabel, - }) - if err != nil { - return nil, err + // We add handlers to the printer in case it is printers.HumanReadablePrinter. + // printers.AddHandlers expects concrete type of printers.HumanReadablePrinter + // as its parameter because of this we have to do a type check on printer and + // extract out concrete HumanReadablePrinter from it. We are then able to attach + // handlers on it. + if humanReadablePrinter, ok := printer.(*printers.HumanReadablePrinter); ok { + printersinternal.AddHandlers(humanReadablePrinter) + printer = humanReadablePrinter } - printer = maybeWrapSortingPrinter(cmd, printer) } return printer, nil diff --git a/pkg/kubectl/cmd/util/printing.go b/pkg/kubectl/cmd/util/printing.go index 62729316249ee..90817c26b4f18 100644 --- a/pkg/kubectl/cmd/util/printing.go +++ b/pkg/kubectl/cmd/util/printing.go @@ -107,7 +107,7 @@ func ValidateOutputArgs(cmd *cobra.Command) error { // PrinterForCommand returns the default printer for this command. // Requires that printer flags have been added to cmd (see AddPrinterFlags). -func PrinterForCommand(cmd *cobra.Command, mapper meta.RESTMapper, typer runtime.ObjectTyper, decoders []runtime.Decoder) (printers.ResourcePrinter, bool, error) { +func PrinterForCommand(cmd *cobra.Command, mapper meta.RESTMapper, typer runtime.ObjectTyper, encoder runtime.Encoder, decoders []runtime.Decoder, options printers.PrintOptions) (printers.ResourcePrinter, error) { outputFormat := GetFlagString(cmd, "output") // templates are logically optional for specifying a format. @@ -133,15 +133,15 @@ func PrinterForCommand(cmd *cobra.Command, mapper meta.RESTMapper, typer runtime if cmd.Flags().Lookup("allow-missing-template-keys") != nil { allowMissingTemplateKeys = GetFlagBool(cmd, "allow-missing-template-keys") } - printer, generic, err := printers.GetStandardPrinter( + printer, err := printers.GetStandardPrinter( outputFormat, templateFile, GetFlagBool(cmd, "no-headers"), allowMissingTemplateKeys, - mapper, typer, decoders, + mapper, typer, encoder, decoders, options, ) if err != nil { - return nil, generic, err + return nil, err } - return maybeWrapSortingPrinter(cmd, printer), generic, nil + return maybeWrapSortingPrinter(cmd, printer), nil } // PrintResourceInfoForCommand receives a *cobra.Command and a *resource.Info and @@ -149,11 +149,11 @@ func PrinterForCommand(cmd *cobra.Command, mapper meta.RESTMapper, typer runtime // object passed is non-generic, it attempts to print the object using a HumanReadablePrinter. // Requires that printer flags have been added to cmd (see AddPrinterFlags). func PrintResourceInfoForCommand(cmd *cobra.Command, info *resource.Info, f Factory, out io.Writer) error { - printer, generic, err := f.PrinterForCommand(cmd) + printer, err := f.PrinterForCommand(cmd, printers.PrintOptions{}) if err != nil { return err } - if !generic || printer == nil { + if !printer.IsGeneric() { printer, err = f.PrinterForMapping(cmd, nil, false) if err != nil { return err diff --git a/pkg/kubectl/sorting_printer.go b/pkg/kubectl/sorting_printer.go index bd8f5f973f951..f0001ea431304 100644 --- a/pkg/kubectl/sorting_printer.go +++ b/pkg/kubectl/sorting_printer.go @@ -60,10 +60,14 @@ func (s *SortingPrinter) PrintObj(obj runtime.Object, out io.Writer) error { } // TODO: implement HandledResources() -func (p *SortingPrinter) HandledResources() []string { +func (s *SortingPrinter) HandledResources() []string { return []string{} } +func (s *SortingPrinter) IsGeneric() bool { + return s.Delegate.IsGeneric() +} + func (s *SortingPrinter) sortObj(obj runtime.Object) error { objs, err := meta.ExtractList(obj) if err != nil { diff --git a/pkg/printers/customcolumn.go b/pkg/printers/customcolumn.go index ac533b2cdd965..c0676962a3c4a 100644 --- a/pkg/printers/customcolumn.go +++ b/pkg/printers/customcolumn.go @@ -239,3 +239,7 @@ func (s *CustomColumnsPrinter) printOneObject(obj runtime.Object, parsers []*jso func (s *CustomColumnsPrinter) HandledResources() []string { return []string{} } + +func (s *CustomColumnsPrinter) IsGeneric() bool { + return true +} diff --git a/pkg/printers/humanreadable.go b/pkg/printers/humanreadable.go index 8517ef7ce3dbe..ba27c2e4e0552 100644 --- a/pkg/printers/humanreadable.go +++ b/pkg/printers/humanreadable.go @@ -152,6 +152,10 @@ func (h *HumanReadablePrinter) AfterPrint(output io.Writer, res string) error { return nil } +func (h *HumanReadablePrinter) IsGeneric() bool { + return false +} + func (h *HumanReadablePrinter) unknown(data []byte, w io.Writer) error { _, err := fmt.Fprintf(w, "Unknown object: %s", string(data)) return err diff --git a/pkg/printers/interface.go b/pkg/printers/interface.go index aaadecc62b7d7..ff2a609aa33ee 100644 --- a/pkg/printers/interface.go +++ b/pkg/printers/interface.go @@ -31,6 +31,8 @@ type ResourcePrinter interface { //Can be used to print out warning/clarifications if needed //after all objects were printed AfterPrint(io.Writer, string) error + // Identify if it is a generic printer + IsGeneric() bool } // ResourcePrinterFunc is a function that can print objects @@ -50,6 +52,10 @@ func (fn ResourcePrinterFunc) AfterPrint(io.Writer, string) error { return nil } +func (fn ResourcePrinterFunc) IsGeneric() bool { + return true +} + type PrintOptions struct { NoHeaders bool WithNamespace bool diff --git a/pkg/printers/internalversion/printers_test.go b/pkg/printers/internalversion/printers_test.go index ff940fb278e2f..15c03c7ab5197 100644 --- a/pkg/printers/internalversion/printers_test.go +++ b/pkg/printers/internalversion/printers_test.go @@ -81,12 +81,22 @@ func TestVersionedPrinter(t *testing.T) { } func TestPrintDefault(t *testing.T) { - printer, found, err := printers.GetStandardPrinter("", "", false, false, nil, nil, nil) - if err != nil { - t.Fatalf("unexpected error: %#v", err) + printerTests := []struct { + Name string + Format string + }{ + {"test wide", "wide"}, + {"test blank format", ""}, } - if found { - t.Errorf("no printer should have been found: %#v / %v", printer, err) + + for _, test := range printerTests { + printer, err := printers.GetStandardPrinter(test.Format, "", false, false, nil, nil, api.Codecs.LegacyCodec(api.Registry.EnabledVersions()...), []runtime.Decoder{api.Codecs.UniversalDecoder(), unstructured.UnstructuredJSONScheme}, printers.PrintOptions{}) + if err != nil { + t.Errorf("in %s, unexpected error: %#v", test.Name, err) + } + if printer.IsGeneric() { + t.Errorf("in %s, printer should not be generic: %#v", test.Name, printer) + } } } @@ -136,11 +146,11 @@ func TestPrinter(t *testing.T) { } for _, test := range printerTests { buf := bytes.NewBuffer([]byte{}) - printer, generic, err := printers.GetStandardPrinter(test.Format, test.FormatArgument, false, true, api.Registry.RESTMapper(api.Registry.EnabledVersions()...), api.Scheme, []runtime.Decoder{api.Codecs.UniversalDecoder(), unstructured.UnstructuredJSONScheme}) + printer, err := printers.GetStandardPrinter(test.Format, test.FormatArgument, false, true, api.Registry.RESTMapper(api.Registry.EnabledVersions()...), api.Scheme, api.Codecs.LegacyCodec(api.Registry.EnabledVersions()...), []runtime.Decoder{api.Codecs.UniversalDecoder(), unstructured.UnstructuredJSONScheme}, printers.PrintOptions{}) if err != nil { t.Errorf("in %s, unexpected error: %#v", test.Name, err) } - if generic && len(test.OutputVersions) > 0 { + if printer.IsGeneric() && len(test.OutputVersions) > 0 { printer = printers.NewVersionedPrinter(printer, api.Scheme, test.OutputVersions...) } if err := printer.PrintObj(test.Input, buf); err != nil { @@ -164,9 +174,10 @@ func TestBadPrinter(t *testing.T) { {"bad template", "template", "{{ .Name", fmt.Errorf("error parsing template {{ .Name, template: output:1: unclosed action\n")}, {"bad templatefile", "templatefile", "", fmt.Errorf("templatefile format specified but no template file given")}, {"bad jsonpath", "jsonpath", "{.Name", fmt.Errorf("error parsing jsonpath {.Name, unclosed action\n")}, + {"unknown format", "anUnknownFormat", "", fmt.Errorf("output format \"anUnknownFormat\" not recognized")}, } for _, test := range badPrinterTests { - _, _, err := printers.GetStandardPrinter(test.Format, test.FormatArgument, false, false, api.Registry.RESTMapper(api.Registry.EnabledVersions()...), api.Scheme, []runtime.Decoder{api.Codecs.UniversalDecoder(), unstructured.UnstructuredJSONScheme}) + _, err := printers.GetStandardPrinter(test.Format, test.FormatArgument, false, false, api.Registry.RESTMapper(api.Registry.EnabledVersions()...), api.Scheme, api.Codecs.LegacyCodec(api.Registry.EnabledVersions()...), []runtime.Decoder{api.Codecs.UniversalDecoder(), unstructured.UnstructuredJSONScheme}, printers.PrintOptions{}) if err == nil || err.Error() != test.Error.Error() { t.Errorf("in %s, expect %s, got %s", test.Name, test.Error, err) } @@ -359,7 +370,7 @@ func TestNamePrinter(t *testing.T) { }, "pods/foo\npods/bar\n"}, } - printer, _, _ := printers.GetStandardPrinter("name", "", false, false, api.Registry.RESTMapper(api.Registry.EnabledVersions()...), api.Scheme, []runtime.Decoder{api.Codecs.UniversalDecoder(), unstructured.UnstructuredJSONScheme}) + printer, _ := printers.GetStandardPrinter("name", "", false, false, api.Registry.RESTMapper(api.Registry.EnabledVersions()...), api.Scheme, api.Codecs.LegacyCodec(api.Registry.EnabledVersions()...), []runtime.Decoder{api.Codecs.UniversalDecoder(), unstructured.UnstructuredJSONScheme}, printers.PrintOptions{}) for name, item := range tests { buff := &bytes.Buffer{} err := printer.PrintObj(item.obj, buff) @@ -2235,7 +2246,7 @@ func TestAllowMissingKeys(t *testing.T) { } for _, test := range tests { buf := bytes.NewBuffer([]byte{}) - printer, _, err := printers.GetStandardPrinter(test.Format, test.Template, false, test.AllowMissingTemplateKeys, api.Registry.RESTMapper(api.Registry.EnabledVersions()...), api.Scheme, []runtime.Decoder{api.Codecs.UniversalDecoder(), unstructured.UnstructuredJSONScheme}) + printer, err := printers.GetStandardPrinter(test.Format, test.Template, false, test.AllowMissingTemplateKeys, api.Registry.RESTMapper(api.Registry.EnabledVersions()...), api.Scheme, api.Codecs.LegacyCodec(api.Registry.EnabledVersions()...), []runtime.Decoder{api.Codecs.UniversalDecoder(), unstructured.UnstructuredJSONScheme}, printers.PrintOptions{}) if err != nil { t.Errorf("in %s, unexpected error: %#v", test.Name, err) } diff --git a/pkg/printers/json.go b/pkg/printers/json.go index 19c734e5bf7a1..1547d2d481d35 100644 --- a/pkg/printers/json.go +++ b/pkg/printers/json.go @@ -63,6 +63,10 @@ func (p *JSONPrinter) HandledResources() []string { return []string{} } +func (p *JSONPrinter) IsGeneric() bool { + return true +} + // YAMLPrinter is an implementation of ResourcePrinter which outputs an object as YAML. // The input object is assumed to be in the internal version of an API and is converted // to the given version first. @@ -99,3 +103,7 @@ func (p *YAMLPrinter) PrintObj(obj runtime.Object, w io.Writer) error { func (p *YAMLPrinter) HandledResources() []string { return []string{} } + +func (p *YAMLPrinter) IsGeneric() bool { + return true +} diff --git a/pkg/printers/jsonpath.go b/pkg/printers/jsonpath.go index 404648a77573e..f1739dcd6ad54 100644 --- a/pkg/printers/jsonpath.go +++ b/pkg/printers/jsonpath.go @@ -155,3 +155,7 @@ func (j *JSONPathPrinter) PrintObj(obj runtime.Object, w io.Writer) error { func (p *JSONPathPrinter) HandledResources() []string { return []string{} } + +func (p *JSONPathPrinter) IsGeneric() bool { + return true +} diff --git a/pkg/printers/name.go b/pkg/printers/name.go index d1605c63c9739..c720ca583d4fc 100644 --- a/pkg/printers/name.go +++ b/pkg/printers/name.go @@ -87,3 +87,7 @@ func (p *NamePrinter) PrintObj(obj runtime.Object, w io.Writer) error { func (p *NamePrinter) HandledResources() []string { return []string{} } + +func (p *NamePrinter) IsGeneric() bool { + return true +} diff --git a/pkg/printers/printers.go b/pkg/printers/printers.go index 6272ebc4574f1..220fa3980947c 100644 --- a/pkg/printers/printers.go +++ b/pkg/printers/printers.go @@ -25,92 +25,102 @@ import ( "k8s.io/apimachinery/pkg/runtime" ) -// GetStandardPrinter takes a format type, an optional format argument. It will return true -// if the format is generic (untyped), otherwise it will return false. The printer -// is agnostic to schema versions, so you must send arguments to PrintObj in the -// version you wish them to be shown using a VersionedPrinter (typically when -// generic is true). -func GetStandardPrinter(format, formatArgument string, noHeaders, allowMissingTemplateKeys bool, mapper meta.RESTMapper, typer runtime.ObjectTyper, decoders []runtime.Decoder) (ResourcePrinter, bool, error) { +// GetStandardPrinter takes a format type, an optional format argument. It will return +// a printer or an error. The printer is agnostic to schema versions, so you must +// send arguments to PrintObj in the version you wish them to be shown using a +// VersionedPrinter (typically when generic is true). +func GetStandardPrinter(format, formatArgument string, noHeaders, allowMissingTemplateKeys bool, mapper meta.RESTMapper, typer runtime.ObjectTyper, encoder runtime.Encoder, decoders []runtime.Decoder, options PrintOptions) (ResourcePrinter, error) { var printer ResourcePrinter switch format { + case "json": printer = &JSONPrinter{} + case "yaml": printer = &YAMLPrinter{} + case "name": printer = &NamePrinter{ Typer: typer, Decoders: decoders, Mapper: mapper, } + case "template", "go-template": if len(formatArgument) == 0 { - return nil, false, fmt.Errorf("template format specified but no template given") + return nil, fmt.Errorf("template format specified but no template given") } templatePrinter, err := NewTemplatePrinter([]byte(formatArgument)) if err != nil { - return nil, false, fmt.Errorf("error parsing template %s, %v\n", formatArgument, err) + return nil, fmt.Errorf("error parsing template %s, %v\n", formatArgument, err) } templatePrinter.AllowMissingKeys(allowMissingTemplateKeys) printer = templatePrinter + case "templatefile", "go-template-file": if len(formatArgument) == 0 { - return nil, false, fmt.Errorf("templatefile format specified but no template file given") + return nil, fmt.Errorf("templatefile format specified but no template file given") } data, err := ioutil.ReadFile(formatArgument) if err != nil { - return nil, false, fmt.Errorf("error reading template %s, %v\n", formatArgument, err) + return nil, fmt.Errorf("error reading template %s, %v\n", formatArgument, err) } templatePrinter, err := NewTemplatePrinter(data) if err != nil { - return nil, false, fmt.Errorf("error parsing template %s, %v\n", string(data), err) + return nil, fmt.Errorf("error parsing template %s, %v\n", string(data), err) } templatePrinter.AllowMissingKeys(allowMissingTemplateKeys) printer = templatePrinter + case "jsonpath": if len(formatArgument) == 0 { - return nil, false, fmt.Errorf("jsonpath template format specified but no template given") + return nil, fmt.Errorf("jsonpath template format specified but no template given") } jsonpathPrinter, err := NewJSONPathPrinter(formatArgument) if err != nil { - return nil, false, fmt.Errorf("error parsing jsonpath %s, %v\n", formatArgument, err) + return nil, fmt.Errorf("error parsing jsonpath %s, %v\n", formatArgument, err) } jsonpathPrinter.AllowMissingKeys(allowMissingTemplateKeys) printer = jsonpathPrinter + case "jsonpath-file": if len(formatArgument) == 0 { - return nil, false, fmt.Errorf("jsonpath file format specified but no template file file given") + return nil, fmt.Errorf("jsonpath file format specified but no template file file given") } data, err := ioutil.ReadFile(formatArgument) if err != nil { - return nil, false, fmt.Errorf("error reading template %s, %v\n", formatArgument, err) + return nil, fmt.Errorf("error reading template %s, %v\n", formatArgument, err) } jsonpathPrinter, err := NewJSONPathPrinter(string(data)) if err != nil { - return nil, false, fmt.Errorf("error parsing template %s, %v\n", string(data), err) + return nil, fmt.Errorf("error parsing template %s, %v\n", string(data), err) } jsonpathPrinter.AllowMissingKeys(allowMissingTemplateKeys) printer = jsonpathPrinter + case "custom-columns": var err error if printer, err = NewCustomColumnsPrinterFromSpec(formatArgument, decoders[0], noHeaders); err != nil { - return nil, false, err + return nil, err } + case "custom-columns-file": file, err := os.Open(formatArgument) if err != nil { - return nil, false, fmt.Errorf("error reading template %s, %v\n", formatArgument, err) + return nil, fmt.Errorf("error reading template %s, %v\n", formatArgument, err) } defer file.Close() if printer, err = NewCustomColumnsPrinterFromTemplate(file, decoders[0]); err != nil { - return nil, false, err + return nil, err } + case "wide": fallthrough case "": - return nil, false, nil + + printer = NewHumanReadablePrinter(encoder, decoders[0], options) default: - return nil, false, fmt.Errorf("output format %q not recognized", format) + return nil, fmt.Errorf("output format %q not recognized", format) } - return printer, true, nil + return printer, nil } diff --git a/pkg/printers/template.go b/pkg/printers/template.go index 9dbc33ce8fcab..c54c511a20893 100644 --- a/pkg/printers/template.go +++ b/pkg/printers/template.go @@ -88,6 +88,10 @@ func (p *TemplatePrinter) HandledResources() []string { return []string{} } +func (p *TemplatePrinter) IsGeneric() bool { + return true +} + // safeExecute tries to execute the template, but catches panics and returns an error // should the template engine panic. func (p *TemplatePrinter) safeExecute(w io.Writer, obj interface{}) error { diff --git a/pkg/printers/versioned.go b/pkg/printers/versioned.go index e8330992c30cc..5e3a6cdac29ed 100644 --- a/pkg/printers/versioned.go +++ b/pkg/printers/versioned.go @@ -61,3 +61,7 @@ func (p *VersionedPrinter) PrintObj(obj runtime.Object, w io.Writer) error { func (p *VersionedPrinter) HandledResources() []string { return []string{} } + +func (p *VersionedPrinter) IsGeneric() bool { + return p.printer.IsGeneric() +}