From ecafae333ba299ca8f4cfdf71ca1764e4982e07d Mon Sep 17 00:00:00 2001 From: Sagar Muchhal Date: Mon, 24 Aug 2020 16:47:12 -0700 Subject: [PATCH] Adds args-file flag to run command This patch adds a new flag `--args-file` to the run command which takes the path to a file containing key=value pairs of arguments which can be passed to the Starlark diagnostics script. The values for the keys passed via the `--args-file` flag can be overridden by passing a diff value for the same key via the `--args` flag. Signed-off-by: Sagar Muchhal --- cmd/cmd_suite_test.go | 16 ++++++++ cmd/run.go | 46 +++++++++++++++++++---- cmd/run_test.go | 42 +++++++++++++++++++++ util/args.go | 43 +++++++++++++++++++++ util/args_test.go | 83 +++++++++++++++++++++++++++++++++++++++++ util/util_suite_test.go | 16 ++++++++ 6 files changed, 238 insertions(+), 8 deletions(-) create mode 100644 cmd/cmd_suite_test.go create mode 100644 cmd/run_test.go create mode 100644 util/args.go create mode 100644 util/args_test.go create mode 100644 util/util_suite_test.go diff --git a/cmd/cmd_suite_test.go b/cmd/cmd_suite_test.go new file mode 100644 index 00000000..e5c7f738 --- /dev/null +++ b/cmd/cmd_suite_test.go @@ -0,0 +1,16 @@ +// Copyright (c) 2020 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestCmd(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Cmd Suite") +} diff --git a/cmd/run.go b/cmd/run.go index cf70982b..dba212ec 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -4,17 +4,24 @@ package cmd import ( - "fmt" "os" "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/vmware-tanzu/crash-diagnostics/exec" + "github.com/vmware-tanzu/crash-diagnostics/util" ) +type runFlags struct { + args map[string]string + argsFile string +} + // newRunCommand creates a command to run the Diagnostics script a file func newRunCommand() *cobra.Command { - scriptArgs := make(map[string]string) + flags := &runFlags{ + args: make(map[string]string), + } cmd := &cobra.Command{ Args: cobra.ExactArgs(1), @@ -22,24 +29,47 @@ func newRunCommand() *cobra.Command { Short: "Executes a diagnostics script file", Long: "Executes a diagnostics script and collects its output as an archive bundle", RunE: func(cmd *cobra.Command, args []string) error { - return run(scriptArgs, args[0]) + return run(flags, args[0]) }, } - cmd.Flags().StringToStringVar(&scriptArgs, "args", scriptArgs, "comma-separated key=value arguments to pass to the diagnostics file") + cmd.Flags().StringToStringVar(&flags.args, "args", flags.args, "comma-separated key=value arguments to pass to the diagnostics file") + cmd.Flags().StringVar(&flags.argsFile, "args-file", flags.argsFile, "path to the file having key=value arguments to pass to the diagnostics file") return cmd } -func run(scriptArgs map[string]string, path string) error { +func run(flags *runFlags, path string) error { file, err := os.Open(path) if err != nil { - return errors.Wrap(err, fmt.Sprintf("script file not found: %s", path)) + return errors.Wrapf(err, "script file not found: %s", path) } - defer file.Close() + scriptArgs, err := processScriptArguments(flags) + if err != nil { + return err + } + if err := exec.ExecuteFile(file, scriptArgs); err != nil { - return errors.Wrap(err, fmt.Sprintf("execution failed for %s", file.Name())) + return errors.Wrapf(err, "execution failed for %s", file.Name()) } return nil } + +// prepares a map of key-value strings to be passed to the execution script +// It builds the map from the args-file as well as the args flag passed to +// the run command. +func processScriptArguments(flags *runFlags) (map[string]string, error) { + // read inputs from the scriptArgs-file + scriptArgs, err := util.ReadArgsFile(flags.argsFile) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse scriptArgs file: %s", flags.argsFile) + } + + // values specified by the args flag override the values from the args-file flag + for k, v := range flags.args { + scriptArgs[k] = v + } + + return scriptArgs, nil +} diff --git a/cmd/run_test.go b/cmd/run_test.go new file mode 100644 index 00000000..3ff7f661 --- /dev/null +++ b/cmd/run_test.go @@ -0,0 +1,42 @@ +// Copyright (c) 2020 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "io/ioutil" + "os" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" +) + +var _ = Describe("Run", func() { + + Context("With args-file and args both", func() { + + DescribeTable("processScriptArguments", func(argsFileContent string, args map[string]string, size int) { + f, err := ioutil.TempFile(os.TempDir(), "") + Expect(err).NotTo(HaveOccurred()) + + err = ioutil.WriteFile(f.Name(), []byte(argsFileContent), 0644) + Expect(err).NotTo(HaveOccurred()) + + defer f.Close() + + flags := &runFlags{ + args: args, + argsFile: f.Name(), + } + scriptArgs, err := processScriptArguments(flags) + Expect(err).NotTo(HaveOccurred()) + Expect(scriptArgs).To(HaveLen(size)) + }, + Entry("no overlapping keys", "key=value", map[string]string{"a": "b"}, 2), + Entry("overlapping keys", "key=value", map[string]string{"key": "b"}, 1), + Entry("file with no keys", "", map[string]string{"key": "b"}, 1), + Entry("with file and without args", "key=value", map[string]string{}, 1), + ) + }) +}) diff --git a/util/args.go b/util/args.go new file mode 100644 index 00000000..76e51ce7 --- /dev/null +++ b/util/args.go @@ -0,0 +1,43 @@ +// Copyright (c) 2020 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package util + +import ( + "bufio" + "fmt" + "log" + "os" + "strings" + + "github.com/pkg/errors" + "github.com/sirupsen/logrus" +) + +func ReadArgsFile(path string) (map[string]string, error) { + file, err := os.Open(path) + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("args file not found: %s", path)) + } + defer file.Close() + + scanner := bufio.NewScanner(file) + if err := scanner.Err(); err != nil { + log.Fatal(err) + return nil, err + } + + args := map[string]string{} + for scanner.Scan() { + line := scanner.Text() + if !strings.HasPrefix(line, "#") && len(strings.TrimSpace(line)) != 0 { + if pair := strings.Split(line, "="); len(pair) == 2 { + args[strings.TrimSpace(pair[0])] = strings.TrimSpace(pair[1]) + } else { + logrus.Warnf("unknown entry in args file: %s", line) + } + } + } + + return args, nil +} diff --git a/util/args_test.go b/util/args_test.go new file mode 100644 index 00000000..6593724c --- /dev/null +++ b/util/args_test.go @@ -0,0 +1,83 @@ +// Copyright (c) 2020 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package util + +import ( + "io/ioutil" + "os" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + "github.com/onsi/gomega/gbytes" + "github.com/sirupsen/logrus" +) + +var _ = Describe("ReadArgsFile", func() { + + It("returns an error when an invalid file name is passed", func() { + _, err := ReadArgsFile("/foo/blah") + Expect(err).To(HaveOccurred()) + }) + + Context("with valid file", func() { + DescribeTable("length of args map", func(input string, size int, warnMsgPresent bool) { + f := writeContentToFile(input) + defer f.Close() + + warnBuffer := gbytes.NewBuffer() + logrus.SetOutput(warnBuffer) + + args, _ := ReadArgsFile(f.Name()) + Expect(args).To(HaveLen(size)) + + if warnMsgPresent { + Expect(warnBuffer).To(gbytes.Say("unknown entry in args file")) + } + }, + Entry("valid with no spaces", ` +key=value +foo=bar +`, 2, false), + Entry("valid with spaces", ` +# key represents earth is round +key = value +foo= bar +bloop =blah + `, 3, false), + Entry("valid with empty values", ` +key = +foo= bar +bloop= + `, 3, false), + Entry("invalid", ` +key value +foo +bar +`, 0, true)) + }) + + It("accepts comments in the args file", func() { + f := writeContentToFile(`# key represents A +key = value +# foo represents B +foo= bar`) + defer f.Close() + + args, err := ReadArgsFile(f.Name()) + Expect(err).NotTo(HaveOccurred()) + Expect(args).To(HaveLen(2)) + }) + +}) + +var writeContentToFile = func(content string) *os.File { + f, err := ioutil.TempFile(os.TempDir(), "read_file_args") + Expect(err).NotTo(HaveOccurred()) + + err = ioutil.WriteFile(f.Name(), []byte(content), 0644) + Expect(err).NotTo(HaveOccurred()) + + return f +} diff --git a/util/util_suite_test.go b/util/util_suite_test.go new file mode 100644 index 00000000..68a720af --- /dev/null +++ b/util/util_suite_test.go @@ -0,0 +1,16 @@ +// Copyright (c) 2020 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package util + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestUtil(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Util Suite") +}