Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add functionality for users to retrieve template by Name as well as UUID #553

Merged
merged 4 commits into from
May 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -137,7 +140,7 @@ linters:
- nolintlint
- predeclared
# disabling for the initial iteration of the linting tool
#- promlinter
# - promlinter
- revive
- rowserrcheck
- sqlclosecheck
Expand Down Expand Up @@ -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"

Expand Down
16 changes: 16 additions & 0 deletions .yamllint
Original file line number Diff line number Diff line change
@@ -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
45 changes: 35 additions & 10 deletions cmd/tink-cli/cmd/get/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
72 changes: 60 additions & 12 deletions cmd/tink-cli/cmd/get/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -18,6 +20,7 @@ func TestNewGetCommand(t *testing.T) {
tests := []struct {
Name string
ExpectStdout string
ExpectError error
Args []string
Opt Options
Skip string
Expand Down Expand Up @@ -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 {
Expand All @@ -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: `+----+-------+
Expand Down Expand Up @@ -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)
}
Expand Down
34 changes: 21 additions & 13 deletions cmd/tink-cli/cmd/template/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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{
Expand Down Expand Up @@ -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,
}
}
6 changes: 3 additions & 3 deletions db/hardware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}
}

Expand Down Expand Up @@ -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()))
}
}

Expand Down Expand Up @@ -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()))
}
}

Expand Down
2 changes: 1 addition & 1 deletion db/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}
}

Expand Down
Loading