From c824bae4fae6611c6d6bf0c757ccd7b0f08da6de Mon Sep 17 00:00:00 2001 From: Josh Berry Date: Mon, 16 Oct 2023 13:57:34 -0700 Subject: [PATCH 1/2] Allow passing multiple `--input-file`s to workflow commands [#351] The documentation for how to pass multiple arguments was just wrong--`json.Unmarshal()` does not support parsing multi-line JSON inputs as separate values. Fix this by accepting multiple `--input-file` flags, parsing each one separately, and turning each file into a distinct argument. Also, check and throw an error if the user tries to use both `--input` and `--input-file`, since the old behavior--silently ignoring `--input` if `--input-file` is specified--was surprising. --- common/defs-flags.go | 4 +- common/flags.go | 4 +- common/util.go | 24 +++++--- tests/workflow_test.go | 76 ++++++++++++++++++++++++ tests/workflows/encodejson/encodejson.go | 35 +++++++++++ 5 files changed, 132 insertions(+), 11 deletions(-) create mode 100644 tests/workflows/encodejson/encodejson.go diff --git a/common/defs-flags.go b/common/defs-flags.go index b3a7550e..92334a81 100644 --- a/common/defs-flags.go +++ b/common/defs-flags.go @@ -44,8 +44,8 @@ const ( "\t│ │ │ │ │ \n" + "\t* * * * *" FlagWorkflowIdReusePolicyDefinition = "Allows the same Workflow Id to be used in a new Workflow Execution. Options are: AllowDuplicate, AllowDuplicateFailedOnly, RejectDuplicate, TerminateIfRunning." - FlagInputDefinition = "Optional JSON input to provide to the Workflow. Pass \"null\" for null values." - FlagInputFileDefinition = "Passes optional input for the Workflow from a JSON file. If there are multiple JSON files, concatenate them and separate by space or newline. Input from the command line will overwrite file input." + FlagInputDefinition = "JSON value to provide to the Workflow or Query. You may use --input multiple times to pass multiple arguments. May not be combined with --input-file." + FlagInputFileDefinition = "Reads a JSON file and provides the JSON as input to the Workflow or Query. The file must contain a single JSON value (typically an object). Each file is passed as a separate argument; you may use --input-file multiple times to pass multiple arguments. May not be combined with --input." FlagSearchAttributeDefinition = "Passes Search Attribute in key=value format. Use valid JSON formats for value." FlagMemoDefinition = "Passes a memo in key=value format. Use valid JSON formats for value." FlagMemoFileDefinition = "Passes a memo as file input, with each line following key=value format. Use valid JSON formats for value." diff --git a/common/flags.go b/common/flags.go index e9a12d18..44ed1393 100644 --- a/common/flags.go +++ b/common/flags.go @@ -339,7 +339,7 @@ var FlagsForStartWorkflowT = []cli.Flag{ Usage: FlagInputDefinition, Category: CategoryMain, }, - &cli.StringFlag{ + &cli.StringSliceFlag{ Name: FlagInputFile, Usage: FlagInputFileDefinition, Category: CategoryMain, @@ -387,7 +387,7 @@ var FlagsForStackTraceQuery = append(FlagsForExecution, []cli.Flag{ Usage: FlagInputDefinition, Category: CategoryMain, }, - &cli.StringFlag{ + &cli.StringSliceFlag{ Name: FlagInputFile, Usage: FlagInputFileDefinition, Category: CategoryMain, diff --git a/common/util.go b/common/util.go index ac8a913b..913594d1 100644 --- a/common/util.go +++ b/common/util.go @@ -409,6 +409,10 @@ func ProcessJSONInput(c *cli.Context) (*commonpb.Payloads, error) { // read multiple inputs presented in json format func readJSONInputs(c *cli.Context) ([][]byte, error) { + if c.IsSet(FlagInput) && c.IsSet(FlagInputFile) { + return nil, fmt.Errorf("you may not combine --input and --input-file; please use one or the other") + } + if c.IsSet(FlagInput) { inputsG := c.Generic(FlagInput) @@ -432,14 +436,20 @@ func readJSONInputs(c *cli.Context) ([][]byte, error) { return inputsRaw, nil } else if c.IsSet(FlagInputFile) { - inputFile := c.String(FlagInputFile) - // This method is purely used to parse input from the CLI. The input comes from a trusted user - // #nosec - data, err := os.ReadFile(inputFile) - if err != nil { - return nil, fmt.Errorf("unable to read input file: %w", err) + inputFiles := c.StringSlice(FlagInputFile) + + args := [][]byte{} + for _, inputFile := range(inputFiles) { + // This method is purely used to parse input from the CLI. The input + // comes from a trusted user #nosec + data, err := os.ReadFile(inputFile) + if err != nil { + return nil, fmt.Errorf("unable to read input file: %w", err) + } + args = append(args, data) } - return [][]byte{data}, nil + + return args, nil } return nil, nil } diff --git a/tests/workflow_test.go b/tests/workflow_test.go index 41329453..0e7409e8 100644 --- a/tests/workflow_test.go +++ b/tests/workflow_test.go @@ -5,11 +5,13 @@ import ( "fmt" "math/rand" "os" + "path/filepath" "strconv" "time" "github.com/pborman/uuid" "github.com/temporalio/cli/tests/workflows/awaitsignal" + "github.com/temporalio/cli/tests/workflows/encodejson" "github.com/temporalio/cli/tests/workflows/helloworld" "github.com/temporalio/cli/tests/workflows/update" "go.temporal.io/api/enums/v1" @@ -23,6 +25,80 @@ const ( testNamespace = "default" ) +func (s *e2eSuite) TestWorkflowExecute_Input() { + s.T().Parallel() + + server, cli, _ := s.setUpTestEnvironment() + defer func() { _ = server.Stop() }() + + client := server.Client() + + worker := s.newWorker(server, testTq, func(r worker.Registry) { + r.RegisterWorkflow(encodejson.Workflow) + }) + defer worker.Stop() + + // Run the workflow to completion using the CLI. (TODO: We unfortunately + // don't have a way to check the CLI output directly to make sure it prints + // the right result...) + err := cli.Run([]string{"", "workflow", "execute", + "--input", "1", "--input", "\"two\"", "--input", "{\"three\": 3}", + "--input", "[\"a\", \"b\", \"c\"]", + "--type", "Workflow", "--task-queue", testTq, "--workflow-id", "test"}) + s.NoError(err) + + // Check that the workflow produced the result we expect--if it did, that + // means the CLI passed the arguments correctly. + var result interface{} + wf := client.GetWorkflow(context.Background(), "test", "") + err = wf.Get(context.Background(), &result) + s.NoError(err) + + s.Assert().Equal("[1,\"two\",{\"three\":3},[\"a\",\"b\",\"c\"]]", result) +} + +func (s *e2eSuite) TestWorkflowExecute_InputFile() { + s.T().Parallel() + + tempDir := s.T().TempDir() + argFiles := []string{ + filepath.Join(tempDir, "arg1.json"), filepath.Join(tempDir, "arg2.json"), + filepath.Join(tempDir, "arg3.json"), filepath.Join(tempDir, "arg4.json"), + } + s.NoError(os.WriteFile(argFiles[0], []byte("1"), 0700)) + s.NoError(os.WriteFile(argFiles[1], []byte("\"two\""), 0700)) + s.NoError(os.WriteFile(argFiles[2], []byte("{\"three\": 3}"), 0700)) + s.NoError(os.WriteFile(argFiles[3], []byte("[\"a\", \"b\", \"c\"]"), 0700)) + + server, cli, _ := s.setUpTestEnvironment() + defer func() { _ = server.Stop() }() + + client := server.Client() + + worker := s.newWorker(server, testTq, func(r worker.Registry) { + r.RegisterWorkflow(encodejson.Workflow) + }) + defer worker.Stop() + + // Run the workflow to completion using the CLI. (TODO: We unfortunately + // don't have a way to check the CLI output directly to make sure it prints + // the right result...) + err := cli.Run([]string{"", "workflow", "execute", + "--input-file", argFiles[0], "--input-file", argFiles[1], + "--input-file", argFiles[2], "--input-file", argFiles[3], + "--type", "Workflow", "--task-queue", testTq, "--workflow-id", "test"}) + s.NoError(err) + + // Check that the workflow produced the result we expect--if it did, that + // means the CLI passed the arguments correctly. + var result interface{} + wf := client.GetWorkflow(context.Background(), "test", "") + err = wf.Get(context.Background(), &result) + s.NoError(err) + + s.Assert().Equal("[1,\"two\",{\"three\":3},[\"a\",\"b\",\"c\"]]", result) +} + func (s *e2eSuite) TestWorkflowShow_ReplayableHistory() { s.T().Parallel() diff --git a/tests/workflows/encodejson/encodejson.go b/tests/workflows/encodejson/encodejson.go new file mode 100644 index 00000000..8f4f88f4 --- /dev/null +++ b/tests/workflows/encodejson/encodejson.go @@ -0,0 +1,35 @@ +package encodejson + +import ( + "encoding/json" + "time" + + "go.temporal.io/sdk/workflow" + + // TODO(cretz): Remove when tagged + _ "go.temporal.io/sdk/contrib/tools/workflowcheck/determinism" +) + +// Workflow is a Hello World workflow definition. (Ordinarily I would define +// this as a variadic function, but that's not supported currently--see +// https://github.com/temporalio/sdk-go/issues/1114) +func Workflow(ctx workflow.Context, a, b, c, d interface{}) (string, error) { + args := []interface{}{a, b, c, d} + + ao := workflow.ActivityOptions{ + StartToCloseTimeout: 10 * time.Second, + } + ctx = workflow.WithActivityOptions(ctx, ao) + + logger := workflow.GetLogger(ctx) + logger.Info("EncodeJSON workflow started", a, b, c, d) + + result, err := json.Marshal(args) + if err != nil { + return "", err + } + + logger.Info("EncodeJSON workflow completed", result) + + return string(result), nil +} From d483cb860864ada7eae0bf816c08f8a283c68f9f Mon Sep 17 00:00:00 2001 From: Josh Berry Date: Mon, 16 Oct 2023 14:19:21 -0700 Subject: [PATCH 2/2] fix lints --- common/util.go | 2 +- tests/workflow_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/common/util.go b/common/util.go index 913594d1..e67ef1a8 100644 --- a/common/util.go +++ b/common/util.go @@ -439,7 +439,7 @@ func readJSONInputs(c *cli.Context) ([][]byte, error) { inputFiles := c.StringSlice(FlagInputFile) args := [][]byte{} - for _, inputFile := range(inputFiles) { + for _, inputFile := range inputFiles { // This method is purely used to parse input from the CLI. The input // comes from a trusted user #nosec data, err := os.ReadFile(inputFile) diff --git a/tests/workflow_test.go b/tests/workflow_test.go index 0e7409e8..0e0ff569 100644 --- a/tests/workflow_test.go +++ b/tests/workflow_test.go @@ -33,10 +33,10 @@ func (s *e2eSuite) TestWorkflowExecute_Input() { client := server.Client() - worker := s.newWorker(server, testTq, func(r worker.Registry) { + w := s.newWorker(server, testTq, func(r worker.Registry) { r.RegisterWorkflow(encodejson.Workflow) }) - defer worker.Stop() + defer w.Stop() // Run the workflow to completion using the CLI. (TODO: We unfortunately // don't have a way to check the CLI output directly to make sure it prints @@ -75,10 +75,10 @@ func (s *e2eSuite) TestWorkflowExecute_InputFile() { client := server.Client() - worker := s.newWorker(server, testTq, func(r worker.Registry) { + w := s.newWorker(server, testTq, func(r worker.Registry) { r.RegisterWorkflow(encodejson.Workflow) }) - defer worker.Stop() + defer w.Stop() // Run the workflow to completion using the CLI. (TODO: We unfortunately // don't have a way to check the CLI output directly to make sure it prints