Add functionality for users to retrieve template by Name as well as U…

…UID (#553)

## Description

This PR adds the ability for Tink to retrieve workflows by either UUID or by Name.
If the value after `tink template get` is not a valid UUID, Tink will default to searching for the value in the Name field of the templates.

Mostly developed by @Belchy06, thanks!
My additions are:
 - cleaned up the import blocks so its just 2 blocks
 - fix typo in help output
 - rebase on latest main
 - rework some of the logic in cmd/tink-cli/cmd/get/get.go to be clearer

## Why is this needed

This feature is required for a project @Belchy06 is working on but is also something I'd like to make use of.

## How Has This Been Tested?

This has been tested in the VM's based on the [Local Setup with Vagrant](https://docs.tinkerbell.org/setup/local-vagrant/) tutorial, as well as in a personal deployment by @Belchy06.

.golangci.yml

@@ -31,6 +31,10 @@ linters-settings:
   errorlint:
     # these are still common in Go: for instance, exit errors.
     asserts: false
+    # Forcing %w in error wrapping forces authors to make errors part of their package APIs. The decision to make
+    # an error part of a package API should be a concious decision by the author.
+    # Also see Hyrums Law.
+    errorf: false
 
   exhaustive:
     default-signifies-exhaustive: true
@@ -88,10 +92,10 @@ linters-settings:
       - name: waitgroup-by-value
 
   staticcheck:
-    go: "1.16"
+    go: "1.17"
 
   unused:
-    go: "1.16"
+    go: "1.17"
 
 output:
   sort-results: true
@@ -107,8 +111,7 @@ linters:
     - dupl
     - durationcheck
     - errcheck
-    # errname is only available in golangci-lint v1.42.0+ - wait until v1.43 is available to settle
-    #- errname
+    - errname
     - errorlint
     - exhaustive
     - exportloopref
@@ -137,7 +140,7 @@ linters:
     - nolintlint
     - predeclared
     # disabling for the initial iteration of the linting tool
-    #- promlinter
+    # - promlinter
     - revive
     - rowserrcheck
     - sqlclosecheck
@@ -188,7 +191,7 @@ issues:
       linters:
         - noctx
 
-    # kubebuilder needs the stdlib invalid `inline` json struct tag
+    # local to tink: kubebuilder needs the stdlib invalid `inline` json struct tag
     - path: pkg/apis/.*
       text: "struct-tag"
 

.yamllint

@@ -0,0 +1,16 @@
+---
+extends: default
+
+rules:
+  braces:
+    max-spaces-inside: 1
+  brackets:
+    max-spaces-inside: 1
+  comments: disable
+  comments-indentation: disable
+  document-start: disable
+  line-length:
+    level: warning
+    max: 160
+    allow-non-breakable-inline-mappings: true
+  truthy: disable

cmd/tink-cli/cmd/get/get.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 
+	"github.com/google/uuid"
 	"github.com/jedib0t/go-pretty/table"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -19,6 +20,8 @@ type Options struct {
 	RetrieveData func(context.Context, *client.FullClient) ([]interface{}, error)
 	// RetrieveByID is used when a get command has a list of arguments
 	RetrieveByID func(context.Context, *client.FullClient, string) (interface{}, error)
+	// RetrieveByName is used when a get command has a list of arguments
+	RetrieveByName func(context.Context, *client.FullClient, string) (interface{}, error)
 	// PopulateTable populates a table with the data retrieved with the RetrieveData function.
 	PopulateTable func([]interface{}, table.Writer) error
 
@@ -62,17 +65,11 @@ func NewGetCommand(opt Options) *cobra.Command {
 
 			client := clientctx.Get(cmd.Context())
 			if len(args) != 0 {
-				if opt.RetrieveByID == nil {
-					return errors.New("option RetrieveByID is not implemented for this resource yet. Please have a look at the issue in GitHub or open a new one")
-				}
-				for _, requestedID := range args {
-					s, err := opt.RetrieveByID(cmd.Context(), client, requestedID)
-					if err != nil {
-						continue
-					}
-					data = append(data, s)
-				}
+				data, err = retrieveMulti(cmd.Context(), opt, client, args)
 			} else {
+				if opt.RetrieveData == nil {
+					return errors.New("get-all-data is not implemented for this resource yet, please have a look at the issue in GitHub or open a new one")
+				}
 				data, err = opt.RetrieveData(cmd.Context(), client)
 			}
 			if err != nil {
@@ -117,3 +114,31 @@ func NewGetCommand(opt Options) *cobra.Command {
 	cmd.PersistentFlags().BoolVar(&opt.NoHeaders, "no-headers", false, "Table contains an header with the columns' name. You can disable it from being printed out")
 	return cmd
 }
+
+func retrieveMulti(ctx context.Context, opt Options, fc *client.FullClient, args []string) ([]interface{}, error) {
+	var data []interface{}
+	for _, arg := range args {
+		var retriever func(context.Context, *client.FullClient, string) (interface{}, error)
+		if _, err := uuid.Parse(arg); err != nil {
+			// arg is invalid UUID, search for arg in `name` field of db
+			if opt.RetrieveByName == nil {
+				return nil, errors.New("get by Name is not implemented for this resource yet, please have a look at the issue in GitHub or open a new one")
+			}
+			retriever = opt.RetrieveByName
+		} else {
+			// arg is a valid UUID, search for arg in `id` field of db
+			if opt.RetrieveByID == nil {
+				return nil, errors.New("get by ID is not implemented for this resource yet, please have a look at the issue in GitHub or open a new one")
+			}
+			retriever = opt.RetrieveByID
+		}
+
+		s, err := retriever(ctx, fc, arg)
+		if err != nil {
+			continue
+		}
+		data = append(data, s)
+	}
+
+	return data, nil
+}

cmd/tink-cli/cmd/get/get_test.go

@@ -3,12 +3,14 @@ package get
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/jedib0t/go-pretty/table"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/tinkerbell/tink/client"
 	"github.com/tinkerbell/tink/cmd/tink-cli/cmd/internal/clientctx"
@@ -18,6 +20,7 @@ func TestNewGetCommand(t *testing.T) {
 	tests := []struct {
 		Name         string
 		ExpectStdout string
+		ExpectError  error
 		Args         []string
 		Opt          Options
 		Skip         string
@@ -51,20 +54,20 @@ func TestNewGetCommand(t *testing.T) {
 		},
 		{
 			Name: "get-by-id",
-			Args: []string{"30"},
-			ExpectStdout: `+------+-------+
-| NAME | ID    |
-+------+-------+
-| 30   | hello |
-+------+-------+
+			Args: []string{"e0ffbf50-ae7c-4c92-bc7f-34e0de25a989"},
+			ExpectStdout: `+-------+--------------------------------------+
+| NAME  | ID                                   |
++-------+--------------------------------------+
+| hello | e0ffbf50-ae7c-4c92-bc7f-34e0de25a989 |
++-------+--------------------------------------+
 `,
 			Opt: Options{
 				Headers: []string{"name", "id"},
 				RetrieveByID: func(ctx context.Context, cl *client.FullClient, arg string) (interface{}, error) {
-					if arg != "30" {
-						t.Errorf("expected 30 as arg got %s", arg)
+					if arg != "e0ffbf50-ae7c-4c92-bc7f-34e0de25a989" {
+						t.Errorf("expected e0ffbf50-ae7c-4c92-bc7f-34e0de25a989 as arg got %s", arg)
 					}
-					return []string{"30", "hello"}, nil
+					return []string{"hello", "e0ffbf50-ae7c-4c92-bc7f-34e0de25a989"}, nil
 				},
 				PopulateTable: func(data []interface{}, w table.Writer) error {
 					for _, v := range data {
@@ -76,6 +79,43 @@ func TestNewGetCommand(t *testing.T) {
 				},
 			},
 		},
+		{
+			Name:        "get-by-id but no retriever",
+			Args:        []string{"e0ffbf50-ae7c-4c92-bc7f-34e0de25a989"},
+			ExpectError: errors.New("get by ID is not implemented for this resource yet, please have a look at the issue in GitHub or open a new one"),
+		},
+		{
+			Name: "get-by-name",
+			Args: []string{"hello"},
+			ExpectStdout: `+-------+--------------------------------------+
+| NAME  | ID                                   |
++-------+--------------------------------------+
+| hello | e0ffbf50-ae7c-4c92-bc7f-34e0de25a989 |
++-------+--------------------------------------+
+`,
+			Opt: Options{
+				Headers: []string{"name", "id"},
+				RetrieveByName: func(ctx context.Context, cl *client.FullClient, arg string) (interface{}, error) {
+					if arg != "hello" {
+						t.Errorf("expected hello as arg got %s", arg)
+					}
+					return []string{"hello", "e0ffbf50-ae7c-4c92-bc7f-34e0de25a989"}, nil
+				},
+				PopulateTable: func(data []interface{}, w table.Writer) error {
+					for _, v := range data {
+						if vv, ok := v.([]string); ok {
+							w.AppendRow(table.Row{vv[0], vv[1]})
+						}
+					}
+					return nil
+				},
+			},
+		},
+		{
+			Name:        "get-by-name but no retriever",
+			Args:        []string{"hello"},
+			ExpectError: errors.New("get by Name is not implemented for this resource yet, please have a look at the issue in GitHub or open a new one"),
+		},
 		{
 			Name: "happy-path-no-headers",
 			ExpectStdout: `+----+-------+
@@ -174,25 +214,33 @@ func TestNewGetCommand(t *testing.T) {
 				},
 			},
 		},
+		{
+			Name:        "no opts",
+			ExpectError: errors.New("get-all-data is not implemented for this resource yet, please have a look at the issue in GitHub or open a new one"),
+		},
 	}
 
 	for _, s := range tests {
 		t.Run(s.Name, func(t *testing.T) {
 			if s.Skip != "" {
 				t.Skip(s.Skip)
 			}
-			stdout := bytes.NewBufferString("")
+			stdout := &bytes.Buffer{}
 			cmd := NewGetCommand(s.Opt)
+			cmd.SilenceErrors = true
 			cmd.SetOut(stdout)
 			cmd.SetArgs(s.Args)
 			err := cmd.ExecuteContext(clientctx.Set(context.Background(), &client.FullClient{}))
-			if err != nil {
-				t.Error(err)
+			if fmt.Sprint(err) != fmt.Sprint(s.ExpectError) {
+				t.Errorf("unexpected error: want=%v, got=%v", s.ExpectError, err)
 			}
 			out, err := ioutil.ReadAll(stdout)
 			if err != nil {
 				t.Error(err)
 			}
+			if s.ExpectError != nil {
+				s.ExpectStdout = cmd.UsageString() + "\n"
+			}
 			if diff := cmp.Diff(string(out), s.ExpectStdout); diff != "" {
 				t.Fatal(diff)
 			}

cmd/tink-cli/cmd/template/get.go

@@ -26,19 +26,18 @@ var GetCmd = &cobra.Command{
 		if len(args) == 0 {
 			return fmt.Errorf("%v requires an argument", c.UseLine())
 		}
-		for _, arg := range args {
-			if _, err := uuid.Parse(arg); err != nil {
-				return fmt.Errorf("invalid uuid: %s", arg)
-			}
-		}
 		return nil
 	},
 	Run: func(c *cobra.Command, args []string) {
 		for _, arg := range args {
-			req := template.GetRequest{
-				GetBy: &template.GetRequest_Id{
-					Id: arg,
-				},
+			req := template.GetRequest{}
+			// Parse arg[0] to see if it is a UUID
+			if _, err := uuid.Parse(arg); err == nil {
+				// UUID
+				req.GetBy = &template.GetRequest_Id{Id: arg}
+			} else {
+				// String (Name)
+				req.GetBy = &template.GetRequest_Name{Name: arg}
 			}
 			t, err := client.TemplateClient.GetTemplate(context.Background(), &req)
 			if err != nil {
@@ -61,6 +60,14 @@ func (h *getTemplate) RetrieveByID(ctx context.Context, cl *client.FullClient, r
 	})
 }
 
+func (h *getTemplate) RetrieveByName(_ context.Context, cl *client.FullClient, requestName string) (interface{}, error) {
+	return cl.TemplateClient.GetTemplate(context.Background(), &template.GetRequest{
+		GetBy: &template.GetRequest_Name{
+			Name: requestName,
+		},
+	})
+}
+
 func (h *getTemplate) RetrieveData(ctx context.Context, cl *client.FullClient) ([]interface{}, error) {
 	list, err := cl.TemplateClient.ListTemplates(ctx, &template.ListRequest{
 		FilterBy: &template.ListRequest_Name{
@@ -98,9 +105,10 @@ func (h *getTemplate) PopulateTable(data []interface{}, t table.Writer) error {
 func NewGetOptions() get.Options {
 	h := getTemplate{}
 	return get.Options{
-		Headers:       []string{"ID", "Name", "Created At", "Updated At"},
-		RetrieveByID:  h.RetrieveByID,
-		RetrieveData:  h.RetrieveData,
-		PopulateTable: h.PopulateTable,
+		Headers:        []string{"ID", "Name", "Created At", "Updated At"},
+		RetrieveByID:   h.RetrieveByID,
+		RetrieveByName: h.RetrieveByName,
+		RetrieveData:   h.RetrieveData,
+		PopulateTable:  h.PopulateTable,
 	}
 }

db/hardware_test.go

@@ -320,7 +320,7 @@ func TestGetByID_WithNonExistingID(t *testing.T) {
 
 	// TODO: use errors.Is here
 	if !strings.Contains(err.Error(), want) {
-		t.Error(fmt.Errorf("unexpected output, looking for %q as a substring in %q", want, err.Error())) //nolint:errorlint // non-wrapping format verb for fmt.Errorf
+		t.Error(fmt.Errorf("unexpected output, looking for %q as a substring in %q", want, err.Error()))
 	}
 }
 
@@ -433,7 +433,7 @@ func TestGetByIP_WithNonExistingIP(t *testing.T) {
 
 	want := "no rows in result set"
 	if !strings.Contains(err.Error(), want) {
-		t.Error(fmt.Errorf("unexpected output, looking for %q as a substring in %q", want, err.Error())) //nolint:errorlint // non-wrapping format verb for fmt.Errorf
+		t.Error(fmt.Errorf("unexpected output, looking for %q as a substring in %q", want, err.Error()))
 	}
 }
 
@@ -544,7 +544,7 @@ func TestGetByMAC_WithNonExistingMAC(t *testing.T) {
 
 	want := "no rows in result set"
 	if !strings.Contains(err.Error(), want) {
-		t.Error(fmt.Errorf("unexpected output, looking for %q as a substring in %q", want, err.Error())) //nolint:errorlint // non-wrapping format verb for fmt.Errorf
+		t.Error(fmt.Errorf("unexpected output, looking for %q as a substring in %q", want, err.Error()))
 	}
 }
 

db/template_test.go

@@ -362,7 +362,7 @@ func TestGetTemplateWithInvalidID(t *testing.T) {
 	want := "no rows in result set"
 	// TODO: replace with errors.Is
 	if !strings.Contains(err.Error(), want) {
-		t.Error(fmt.Errorf("unexpected output, looking for %q as a substring in %q", want, err.Error())) // nolint:errorlint // non-wrapping format verb for fmt.Errorf
+		t.Error(fmt.Errorf("unexpected output, looking for %q as a substring in %q", want, err.Error()))
 	}
 }