From a1d6c93326db4857cc9600bf5a1f52289aea3f4d Mon Sep 17 00:00:00 2001 From: Nick Santos Date: Wed, 30 Aug 2023 17:07:49 -0400 Subject: [PATCH] cmd: clean up printer implementations (#308) fixes https://github.com/tilt-dev/ctlptl/issues/307 Signed-off-by: Nick Santos --- internal/printers/name.go | 112 ------------------------------------- pkg/api/accessors.go | 20 +++++-- pkg/cmd/apply.go | 2 +- pkg/cmd/create_cluster.go | 2 +- pkg/cmd/create_registry.go | 2 +- pkg/cmd/delete.go | 2 +- pkg/cmd/get.go | 37 +++++++++--- pkg/cmd/get_test.go | 6 +- pkg/cmd/printer.go | 23 -------- 9 files changed, 51 insertions(+), 155 deletions(-) delete mode 100644 internal/printers/name.go delete mode 100644 pkg/cmd/printer.go diff --git a/internal/printers/name.go b/internal/printers/name.go deleted file mode 100644 index 7dc6bb6..0000000 --- a/internal/printers/name.go +++ /dev/null @@ -1,112 +0,0 @@ -/* -Copyright 2017 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 printers - -import ( - "fmt" - "io" - "strings" - - "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - - "github.com/tilt-dev/ctlptl/pkg/api" -) - -// NOTE(nick): A fork of the NamePrinter that supports ctlptl types, which don't have a full object metadata - -type Named interface { - GetName() string -} - -var _ Named = &api.Cluster{} -var _ Named = &api.Registry{} - -// NamePrinter is an implementation of ResourcePrinter which outputs "resource/name" pair of an object. -type NamePrinter struct { - // ShortOutput indicates whether an operation should be - // printed along side the "resource/name" pair for an object. - ShortOutput bool - // Operation describes the name of the action that - // took place on an object, to be included in the - // finalized "successful" message. - Operation string -} - -// PrintObj is an implementation of ResourcePrinter.PrintObj which decodes the object -// and print "resource/name" pair. If the object is a List, print all items in it. -func (p *NamePrinter) PrintObj(obj runtime.Object, w io.Writer) error { - if obj.GetObjectKind().GroupVersionKind().Empty() { - return fmt.Errorf("missing apiVersion or kind; try GetObjectKind().SetGroupVersionKind() if you know the type") - } - - name := "" - if named, ok := obj.(Named); ok { - if n := named.GetName(); len(n) > 0 { - name = n - } - } - if acc, err := meta.Accessor(obj); err == nil { - if n := acc.GetName(); len(n) > 0 { - name = n - } - } - - return printObj(w, name, p.Operation, p.ShortOutput, GetObjectGroupKind(obj)) -} - -func GetObjectGroupKind(obj runtime.Object) schema.GroupKind { - if obj == nil { - return schema.GroupKind{Kind: ""} - } - groupVersionKind := obj.GetObjectKind().GroupVersionKind() - if len(groupVersionKind.Kind) > 0 { - return groupVersionKind.GroupKind() - } - - if uns, ok := obj.(*unstructured.Unstructured); ok { - if len(uns.GroupVersionKind().Kind) > 0 { - return uns.GroupVersionKind().GroupKind() - } - } - - return schema.GroupKind{Kind: ""} -} - -func printObj(w io.Writer, name string, operation string, shortOutput bool, groupKind schema.GroupKind) error { - if len(groupKind.Kind) == 0 { - return fmt.Errorf("missing kind for resource with name %v", name) - } - - if len(operation) > 0 { - operation = " " + operation - } - - if shortOutput { - operation = "" - } - - if len(groupKind.Group) == 0 { - fmt.Fprintf(w, "%s/%s%s\n", strings.ToLower(groupKind.Kind), name, operation) - return nil - } - - fmt.Fprintf(w, "%s.%s/%s%s\n", strings.ToLower(groupKind.Kind), groupKind.Group, name, operation) - return nil -} diff --git a/pkg/api/accessors.go b/pkg/api/accessors.go index a15b373..27a6659 100644 --- a/pkg/api/accessors.go +++ b/pkg/api/accessors.go @@ -1,8 +1,20 @@ package api -func (c *Cluster) GetName() string { - return c.Name +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func (c *Cluster) GetObjectMeta() metav1.Object { + return &metav1.ObjectMeta{ + Name: c.Name, + } } -func (r *Registry) GetName() string { - return r.Name + +func (r *Registry) GetObjectMeta() metav1.Object { + return &metav1.ObjectMeta{ + Name: r.Name, + } } + +var _ metav1.ObjectMetaAccessor = &Cluster{} +var _ metav1.ObjectMetaAccessor = &Registry{} diff --git a/pkg/cmd/apply.go b/pkg/cmd/apply.go index 3dc6f92..e40275e 100644 --- a/pkg/cmd/apply.go +++ b/pkg/cmd/apply.go @@ -72,7 +72,7 @@ func (o *ApplyOptions) run() error { ctx := context.TODO() - printer, err := toPrinter(o.PrintFlags) + printer, err := o.PrintFlags.ToPrinter() if err != nil { return err } diff --git a/pkg/cmd/create_cluster.go b/pkg/cmd/create_cluster.go index d054c0b..7d3f566 100644 --- a/pkg/cmd/create_cluster.go +++ b/pkg/cmd/create_cluster.go @@ -117,7 +117,7 @@ func (o *CreateClusterOptions) run(controller clusterCreator, product string) er return err } - printer, err := toPrinter(o.PrintFlags) + printer, err := o.PrintFlags.ToPrinter() if err != nil { return err } diff --git a/pkg/cmd/create_registry.go b/pkg/cmd/create_registry.go index c9e2cfe..e3091e8 100644 --- a/pkg/cmd/create_registry.go +++ b/pkg/cmd/create_registry.go @@ -99,7 +99,7 @@ func (o *CreateRegistryOptions) run(controller registryCreator, name string) err return err } - printer, err := toPrinter(o.PrintFlags) + printer, err := o.PrintFlags.ToPrinter() if err != nil { return err } diff --git a/pkg/cmd/delete.go b/pkg/cmd/delete.go index 66b895d..7aea5c4 100644 --- a/pkg/cmd/delete.go +++ b/pkg/cmd/delete.go @@ -105,7 +105,7 @@ func (o *DeleteOptions) run(args []string) error { return err } - printer, err := toPrinter(o.PrintFlags) + printer, err := o.PrintFlags.ToPrinter() if err != nil { return err } diff --git a/pkg/cmd/get.go b/pkg/cmd/get.go index c0f20d3..cfd4082 100644 --- a/pkg/cmd/get.go +++ b/pkg/cmd/get.go @@ -9,6 +9,7 @@ import ( "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/duration" @@ -144,7 +145,7 @@ func (o *GetOptions) ToPrinter() (printers.ResourcePrinter, error) { if !o.OutputFlagSpecified() { return printers.NewTablePrinter(printers.PrintOptions{}), nil } - return toPrinter(o.PrintFlags) + return o.PrintFlags.ToPrinter() } func (o *GetOptions) Print(obj runtime.Object) error { @@ -158,9 +159,31 @@ func (o *GetOptions) Print(obj runtime.Object) error { return err } - err = printer.PrintObj(o.transformForOutput(obj), o.Out) - if err != nil { - return err + if !o.OutputFlagSpecified() { + err = printer.PrintObj(o.toTable(obj), o.Out) + if err != nil { + return err + } + } else { + // Name printer only supports UnstructuredList for mysterious reasons. + _, isNamePrinter := printer.(*printers.NamePrinter) + if isNamePrinter && meta.IsListType(obj) { + items, err := meta.ExtractList(obj) + if err != nil { + return err + } + for _, item := range items { + err = printer.PrintObj(item, o.Out) + if err != nil { + return err + } + } + } else { + err = printer.PrintObj(obj, o.Out) + if err != nil { + return err + } + } } return nil } @@ -169,11 +192,7 @@ func (o *GetOptions) OutputFlagSpecified() bool { return o.PrintFlags.OutputFlagSpecified != nil && o.PrintFlags.OutputFlagSpecified() } -func (o *GetOptions) transformForOutput(obj runtime.Object) runtime.Object { - if o.OutputFlagSpecified() { - return obj - } - +func (o *GetOptions) toTable(obj runtime.Object) runtime.Object { switch r := obj.(type) { case *api.Registry: return o.registriesAsTable([]api.Registry{*r}) diff --git a/pkg/cmd/get_test.go b/pkg/cmd/get_test.go index 53598a4..ae65a80 100644 --- a/pkg/cmd/get_test.go +++ b/pkg/cmd/get_test.go @@ -83,7 +83,7 @@ func TestDefaultPrint(t *testing.T) { o.IOStreams = streams o.StartTime = startTime - err := o.Print(o.transformForOutput(clusterList)) + err := o.Print(o.toTable(clusterList)) require.NoError(t, err) assert.Equal(t, out.String(), `CURRENT NAME PRODUCT AGE REGISTRY * microk8s microk8s 3y none @@ -100,7 +100,7 @@ func TestYAML(t *testing.T) { err := o.Command().Flags().Set("output", "yaml") require.NoError(t, err) - err = o.Print(o.transformForOutput(clusterList)) + err = o.Print(clusterList) require.NoError(t, err) assert.Equal(t, `apiVersion: ctlptl.dev/v1alpha1 items: @@ -129,7 +129,7 @@ func TestRegistryPrint(t *testing.T) { o.IOStreams = streams o.StartTime = startTime - err := o.Print(o.transformForOutput(registryList)) + err := o.Print(o.toTable(registryList)) require.NoError(t, err) assert.Equal(t, `NAME HOST ADDRESS CONTAINER ADDRESS AGE ctlptl-registry 0.0.0.0:5001 172.17.0.2:5000 3y diff --git a/pkg/cmd/printer.go b/pkg/cmd/printer.go deleted file mode 100644 index 08d0a91..0000000 --- a/pkg/cmd/printer.go +++ /dev/null @@ -1,23 +0,0 @@ -package cmd - -import ( - "k8s.io/cli-runtime/pkg/genericclioptions" - "k8s.io/cli-runtime/pkg/printers" - - myprinters "github.com/tilt-dev/ctlptl/internal/printers" -) - -func toPrinter(flags *genericclioptions.PrintFlags) (printers.ResourcePrinter, error) { - p, err := flags.ToPrinter() - if err != nil { - return nil, err - } - namePrinter, ok := p.(*printers.NamePrinter) - if ok { - return &myprinters.NamePrinter{ - ShortOutput: namePrinter.ShortOutput, - Operation: namePrinter.Operation, - }, nil - } - return p, nil -}