From d667759a6875e1ed05b61722f538f5f77e6d3a25 Mon Sep 17 00:00:00 2001 From: parauliya Date: Fri, 11 Dec 2020 13:42:30 +0530 Subject: [PATCH 1/4] Fix #377: Removing `name` flag from the 'tink template create/update' command in tink-cli Signed-off-by: parauliya --- cmd/tink-cli/cmd/template/create.go | 22 +++++++++++++++++----- cmd/tink-cli/cmd/template/update.go | 11 ++++------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/cmd/tink-cli/cmd/template/create.go b/cmd/tink-cli/cmd/template/create.go index b19d825b8..a43404376 100644 --- a/cmd/tink-cli/cmd/template/create.go +++ b/cmd/tink-cli/cmd/template/create.go @@ -2,6 +2,7 @@ package template import ( "context" + "errors" "fmt" "io" "io/ioutil" @@ -12,21 +13,25 @@ import ( "github.com/spf13/cobra" "github.com/tinkerbell/tink/client" "github.com/tinkerbell/tink/protos/template" + "gopkg.in/yaml.v2" ) var ( fPath = "path" - fName = "name" filePath string templateName string ) +type TemplateName struct { + Name string `yaml:"name"` +} + // createCmd represents the create subcommand for template command var createCmd = &cobra.Command{ Use: "create", Short: "create a workflow template ", Example: `tink template create [flags] -cat /tmp/example.tmpl | tink template create -n example`, +cat /tmp/example.tmpl | tink template create `, PreRunE: func(c *cobra.Command, args []string) error { if !isInputFromPipe() { path, _ := c.Flags().GetString(fPath) @@ -69,8 +74,6 @@ func readAll(reader io.Reader) []byte { func addFlags() { flags := createCmd.PersistentFlags() flags.StringVarP(&filePath, "path", "p", "", "path to the template file") - flags.StringVarP(&templateName, "name", "n", "", "unique name for the template (alphanumeric and case sensitive)") - _ = createCmd.MarkPersistentFlagRequired(fName) } func tryParseTemplate(data string) error { @@ -78,7 +81,16 @@ func tryParseTemplate(data string) error { if _, err := tmpl.Parse(data); err != nil { return err } - return nil + var templ TemplateName + err := yaml.Unmarshal([]byte(data), &templ) + if err != nil { + return err + } + if templ.Name != "" { + templateName = templ.Name + return nil + } + return errors.New("Template does not have `name` field which is mandatory") } func createTemplate(data []byte) { diff --git a/cmd/tink-cli/cmd/template/update.go b/cmd/tink-cli/cmd/template/update.go index 1e07b9735..36fc2659f 100644 --- a/cmd/tink-cli/cmd/template/update.go +++ b/cmd/tink-cli/cmd/template/update.go @@ -19,9 +19,8 @@ var updateCmd = &cobra.Command{ Short: "update a template", Example: "tink template update [id] [flags]", PreRunE: func(c *cobra.Command, args []string) error { - name, _ := c.Flags().GetString(fName) path, _ := c.Flags().GetString(fPath) - if name == "" && path == "" { + if path == "" { return fmt.Errorf("%v requires at least one flag", c.UseLine()) } return nil @@ -46,19 +45,17 @@ var updateCmd = &cobra.Command{ func updateTemplate(id string) { req := template.WorkflowTemplate{Id: id} - if filePath == "" && templateName != "" { - req.Name = templateName - } else if filePath != "" && templateName == "" { + if filePath != "" { data := readTemplateData() if data != "" { if err := tryParseTemplate(data); err != nil { log.Fatal(err) } + req.Name = templateName req.Data = data } } else { - req.Name = templateName - req.Data = readTemplateData() + log.Fatal("Nothing is provided in the file path") } _, err := client.TemplateClient.UpdateTemplate(context.Background(), &req) From 7b90413d35732c1bcb8fb1d7037f49b290769aff Mon Sep 17 00:00:00 2001 From: Gaurav Gahlot Date: Wed, 23 Dec 2020 16:26:20 +0530 Subject: [PATCH 2/4] review fixes Signed-off-by: Gaurav Gahlot --- cmd/tink-cli/cmd/template/create.go | 46 ++++++++--------------------- cmd/tink-cli/cmd/template/update.go | 11 +++---- 2 files changed, 18 insertions(+), 39 deletions(-) diff --git a/cmd/tink-cli/cmd/template/create.go b/cmd/tink-cli/cmd/template/create.go index a43404376..ab514d4cd 100644 --- a/cmd/tink-cli/cmd/template/create.go +++ b/cmd/tink-cli/cmd/template/create.go @@ -8,35 +8,29 @@ import ( "io/ioutil" "log" "os" - tt "text/template" "github.com/spf13/cobra" "github.com/tinkerbell/tink/client" "github.com/tinkerbell/tink/protos/template" - "gopkg.in/yaml.v2" + "github.com/tinkerbell/tink/workflow" ) var ( - fPath = "path" - filePath string - templateName string + file = "file" + filePath string ) -type TemplateName struct { - Name string `yaml:"name"` -} - // createCmd represents the create subcommand for template command var createCmd = &cobra.Command{ Use: "create", Short: "create a workflow template ", Example: `tink template create [flags] -cat /tmp/example.tmpl | tink template create `, +cat /tmp/example.tmpl | tink template create -n example`, PreRunE: func(c *cobra.Command, args []string) error { if !isInputFromPipe() { - path, _ := c.Flags().GetString(fPath) + path, _ := c.Flags().GetString(file) if path == "" { - return fmt.Errorf("either pipe the template or provide the required '--path' flag") + return errors.New("either pipe the template or provide the required '--file' flag") } } return nil @@ -55,10 +49,11 @@ cat /tmp/example.tmpl | tink template create `, data := readAll(reader) if data != nil { - if err := tryParseTemplate(string(data)); err != nil { + wf, err := workflow.Parse(data) + if err != nil { log.Fatal(err) } - createTemplate(data) + createTemplate(wf.Name, data) } }, } @@ -73,28 +68,11 @@ func readAll(reader io.Reader) []byte { func addFlags() { flags := createCmd.PersistentFlags() - flags.StringVarP(&filePath, "path", "p", "", "path to the template file") -} - -func tryParseTemplate(data string) error { - tmpl := *tt.New("") - if _, err := tmpl.Parse(data); err != nil { - return err - } - var templ TemplateName - err := yaml.Unmarshal([]byte(data), &templ) - if err != nil { - return err - } - if templ.Name != "" { - templateName = templ.Name - return nil - } - return errors.New("Template does not have `name` field which is mandatory") + flags.StringVarP(&filePath, "file", "", "", "path to the template file") } -func createTemplate(data []byte) { - req := template.WorkflowTemplate{Name: templateName, Data: string(data)} +func createTemplate(name string, data []byte) { + req := template.WorkflowTemplate{Name: name, Data: string(data)} res, err := client.TemplateClient.CreateTemplate(context.Background(), &req) if err != nil { log.Fatal(err) diff --git a/cmd/tink-cli/cmd/template/update.go b/cmd/tink-cli/cmd/template/update.go index 36fc2659f..ff13fdadc 100644 --- a/cmd/tink-cli/cmd/template/update.go +++ b/cmd/tink-cli/cmd/template/update.go @@ -11,6 +11,7 @@ import ( "github.com/spf13/cobra" "github.com/tinkerbell/tink/client" "github.com/tinkerbell/tink/protos/template" + "github.com/tinkerbell/tink/workflow" ) // updateCmd represents the get subcommand for template command @@ -19,7 +20,7 @@ var updateCmd = &cobra.Command{ Short: "update a template", Example: "tink template update [id] [flags]", PreRunE: func(c *cobra.Command, args []string) error { - path, _ := c.Flags().GetString(fPath) + path, _ := c.Flags().GetString(file) if path == "" { return fmt.Errorf("%v requires at least one flag", c.UseLine()) } @@ -48,10 +49,11 @@ func updateTemplate(id string) { if filePath != "" { data := readTemplateData() if data != "" { - if err := tryParseTemplate(data); err != nil { + wf, err := workflow.Parse([]byte(data)) + if err != nil { log.Fatal(err) } - req.Name = templateName + req.Name = wf.Name req.Data = data } } else { @@ -81,8 +83,7 @@ func readTemplateData() string { func init() { flags := updateCmd.PersistentFlags() - flags.StringVarP(&filePath, "path", "p", "", "path to the template file") - flags.StringVarP(&templateName, "name", "n", "", "unique name for the template (alphanumeric)") + flags.StringVarP(&filePath, "file", "", "", "path to the template file") SubCommands = append(SubCommands, updateCmd) } From 7d5875519b34e82cd8344db5ac606cb1c780baf7 Mon Sep 17 00:00:00 2001 From: Gaurav Gahlot Date: Thu, 24 Dec 2020 10:54:16 +0530 Subject: [PATCH 3/4] updated command description Signed-off-by: Gaurav Gahlot --- cmd/tink-cli/cmd/template/create.go | 10 ++++++++-- cmd/tink-cli/cmd/template/update.go | 10 +++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/cmd/tink-cli/cmd/template/create.go b/cmd/tink-cli/cmd/template/create.go index ab514d4cd..c21b167b2 100644 --- a/cmd/tink-cli/cmd/template/create.go +++ b/cmd/tink-cli/cmd/template/create.go @@ -24,8 +24,14 @@ var ( var createCmd = &cobra.Command{ Use: "create", Short: "create a workflow template ", - Example: `tink template create [flags] -cat /tmp/example.tmpl | tink template create -n example`, + Long: `The create command allows you create workflow templates: + +# Pipe the file to create a template: +$ cat /tmp/example.tmpl | tink template create + +# Create template using the --file flag: +$ tink template create --file /tmp/example.tmpl +`, PreRunE: func(c *cobra.Command, args []string) error { if !isInputFromPipe() { path, _ := c.Flags().GetString(file) diff --git a/cmd/tink-cli/cmd/template/update.go b/cmd/tink-cli/cmd/template/update.go index ff13fdadc..48f5b57c2 100644 --- a/cmd/tink-cli/cmd/template/update.go +++ b/cmd/tink-cli/cmd/template/update.go @@ -16,9 +16,13 @@ import ( // updateCmd represents the get subcommand for template command var updateCmd = &cobra.Command{ - Use: "update [id] [flags]", - Short: "update a template", - Example: "tink template update [id] [flags]", + Use: "update [id] [flags]", + Short: "update a workflow template", + Long: `The update command allows you change the definition of an existing workflow template : + +# Update an existing template: +$ tink template update 614168df-45a5-11eb-b13d-0242ac120003 --file /tmp/example.tmpl +`, PreRunE: func(c *cobra.Command, args []string) error { path, _ := c.Flags().GetString(file) if path == "" { From fe503ee5fc4baedf087c1030d5250da3566651ac Mon Sep 17 00:00:00 2001 From: Gaurav Gahlot Date: Thu, 24 Dec 2020 12:00:59 +0530 Subject: [PATCH 4/4] fixed failing tests Signed-off-by: Gaurav Gahlot --- cmd/tink-cli/cmd/template/create.go | 8 ++------ cmd/tink-cli/cmd/template/update.go | 7 +++---- cmd/tink-cli/cmd/template_test.go | 26 ++++++++++++++++---------- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/cmd/tink-cli/cmd/template/create.go b/cmd/tink-cli/cmd/template/create.go index c21b167b2..7a9c85341 100644 --- a/cmd/tink-cli/cmd/template/create.go +++ b/cmd/tink-cli/cmd/template/create.go @@ -15,10 +15,7 @@ import ( "github.com/tinkerbell/tink/workflow" ) -var ( - file = "file" - filePath string -) +var filePath string // createCmd represents the create subcommand for template command var createCmd = &cobra.Command{ @@ -34,8 +31,7 @@ $ tink template create --file /tmp/example.tmpl `, PreRunE: func(c *cobra.Command, args []string) error { if !isInputFromPipe() { - path, _ := c.Flags().GetString(file) - if path == "" { + if filePath == "" { return errors.New("either pipe the template or provide the required '--file' flag") } } diff --git a/cmd/tink-cli/cmd/template/update.go b/cmd/tink-cli/cmd/template/update.go index 48f5b57c2..eecdb30d7 100644 --- a/cmd/tink-cli/cmd/template/update.go +++ b/cmd/tink-cli/cmd/template/update.go @@ -18,15 +18,14 @@ import ( var updateCmd = &cobra.Command{ Use: "update [id] [flags]", Short: "update a workflow template", - Long: `The update command allows you change the definition of an existing workflow template : + Long: `The update command allows you change the definition of an existing workflow template: # Update an existing template: $ tink template update 614168df-45a5-11eb-b13d-0242ac120003 --file /tmp/example.tmpl `, PreRunE: func(c *cobra.Command, args []string) error { - path, _ := c.Flags().GetString(file) - if path == "" { - return fmt.Errorf("%v requires at least one flag", c.UseLine()) + if filePath == "" { + return fmt.Errorf("%v requires the '--file' flag", c.UseLine()) } return nil }, diff --git a/cmd/tink-cli/cmd/template_test.go b/cmd/tink-cli/cmd/template_test.go index fdddcb82d..005d40094 100644 --- a/cmd/tink-cli/cmd/template_test.go +++ b/cmd/tink-cli/cmd/template_test.go @@ -2,6 +2,7 @@ package cmd import ( "bytes" + "fmt" "strings" "testing" @@ -44,8 +45,9 @@ func Test_templateCmd(t *testing.T) { if err := root.Execute(); err != nil { t.Error(err) } - if !strings.Contains(out.String(), "list all saved templates") { - t.Error("expected output should include list all saved templates") + want := "list all saved templates" + if !strings.Contains(out.String(), want) { + t.Error(fmt.Errorf("unexpected output, looking for %q as a substring in %q", want, out.String())) } }, }, @@ -61,8 +63,9 @@ func Test_templateCmd(t *testing.T) { if err := root.Execute(); err != nil { t.Error(err) } - if !strings.Contains(out.String(), "create a workflow template") { - t.Error("expected output should include create a workflow template") + want := "Create template using the --file flag" + if !strings.Contains(out.String(), want) { + t.Error(fmt.Errorf("unexpected output, looking for %q as a substring in %q", want, out.String())) } }, }, @@ -78,8 +81,9 @@ func Test_templateCmd(t *testing.T) { if err := root.Execute(); err != nil { t.Error(err) } - if !strings.Contains(out.String(), "delete a template") { - t.Error("expected output should include delete a template") + want := "delete a template" + if !strings.Contains(out.String(), want) { + t.Error(fmt.Errorf("unexpected output, looking for %q as a substring in %q", want, out.String())) } }, }, @@ -95,8 +99,9 @@ func Test_templateCmd(t *testing.T) { if err := root.Execute(); err != nil { t.Error(err) } - if !strings.Contains(out.String(), "get a template") { - t.Error("expected output should include get a template") + want := "get a template" + if !strings.Contains(out.String(), want) { + t.Error(fmt.Errorf("unexpected output, looking for %q as a substring in %q", want, out.String())) } }, }, @@ -112,8 +117,9 @@ func Test_templateCmd(t *testing.T) { if err := root.Execute(); err != nil { t.Error(err) } - if !strings.Contains(out.String(), "update a template") { - t.Error("expected output should include update a template") + want := "Update an existing template" + if !strings.Contains(out.String(), want) { + t.Error(fmt.Errorf("unexpected output, looking for %q as a substring in %q", want, out.String())) } }, },