diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index e368feb6..b0176bff 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -7,6 +7,65 @@ description: "Breaking changes between flow releases, and what to do about each This page documents changes that require you to update existing flow files, config, or scripts when upgrading between major versions. +## v2.3.0 + +### Argument Values Are Literals + +**What changed:** A `$word` inside an argument value used to be expanded against flow's +environment and deleted when it did not resolve, so `flow run notify 'costs $5'` arrived as +`costs `. Argument values a user supplies are now literals, and pass through unchanged. + +Expansion still applies to the strings you write in a flow file — an argument's `default`, +and a serial or parallel step's `args:` entries. + +**How to migrate:** If you relied on a *caller* passing `$VAR` for flow to expand, expand it +in your shell instead (`flow run deploy "$TARGET"`), or move the reference into the +argument's `default`. + +--- + +### Unresolved Variables Are Left As Written + +**What changed:** A `$VAR` that does not resolve now stays in the string instead of becoming +an empty string. This affects every context flow expands: `dir` paths, `cmd` strings, an +argument's `default`, a step's `args:`, and a request's `url`, `headers` and `body`. + +**Before:** `url: "https://$API_HOST/deploy"` with no `API_HOST` requested `https:///deploy`. + +**After:** the same executable requests `https://$API_HOST/deploy`, which fails visibly. + +`$$` now produces a literal `$`, which previously had no escape. + +**How to migrate:** Nothing to change if your variables resolve. If you were depending on a +missing variable collapsing to nothing, give it an explicit empty default via a `param`. + +--- + +### Request Bodies That Are JSON Are Sent As JSON + +**What changed:** `body` was always evaluated as an Expr expression, and a JSON object is +valid Expr — it parses as a map literal and was stringified as +`map[environment:prod version:1.0]`. A body that is a JSON object or array is now sent as +written. Anything else is still an expression. + +**How to migrate:** Nothing to change. A JSON body starts working; an expression body that +returns a string keeps working. + +--- + +### Step `args:` Override Inherited Values + +**What changed:** A serial or parallel step's `args:` list is now applied to the child, and +overrides values it would otherwise inherit from the parent. It was previously ignored when +the parent declared matching `args` — its mere presence, not its contents, was what enabled +inheritance. + +**How to migrate:** Remove any placeholder `args:` entries added to force inheritance, such +as `args: ["-"]`. Inheritance no longer requires them, and a placeholder now reaches the +child as a real value. + +--- + ## v2.0.0 ### `fromFile` Import Field Removed diff --git a/docs/guides/advanced.md b/docs/guides/advanced.md index 085a01c0..0a93d4fe 100644 --- a/docs/guides/advanced.md +++ b/docs/guides/advanced.md @@ -413,8 +413,20 @@ executables: ``` > [!NOTE] -> In serial and parallel executables, `params` and `args` that are defined at the parent level will apply to all -> child executables. Argument from the parent -> child executable should use matching `EnvKey` to ensure proper resolution. +> In serial and parallel executables, `params` and `args` defined at the parent level apply to all +> child executables, whether the child runs a command, a request, or a render. An argument passed +> from parent to child is matched by `envKey`, so both must declare the same one. +> +> A step's own `args:` list overrides what it would otherwise inherit, for the arguments it sets: +> +> ```yaml +> serial: +> args: [{pos: 1, envKey: TARGET}] +> execs: +> - ref: deploy service # inherits TARGET +> - ref: deploy service +> args: ["--target=staging"] # overrides it +> ``` **Resolution example:** ```shell @@ -434,7 +446,23 @@ flow deploy app --param ENVIRONMENT=production -- --verbose=false ### Environment Variable Expansion -Environment variables are expanded in certain contexts: +`$VAR` and `${VAR}` are expanded in the parts of a flow file *you* write: + +- `dir` paths +- `cmd` strings +- A request's `url`, `headers`, and `body` +- An argument's `default` +- A serial or parallel step's `args:` entries + +They are **not** expanded in values a *user* supplies — a command-line argument, a prompt +response, or a `text` param. Those are literals, so `flow run notify 'costs $5'` sends +`costs $5`. + +Two rules apply everywhere expansion happens: + +- `$$` produces a literal `$`. +- A variable that does not resolve is left as written, so a missing `$API_HOST` shows up in + the command or URL rather than silently becoming an empty string. **Directory paths:** ```yaml diff --git a/docs/guides/executables.md b/docs/guides/executables.md index a1383a8c..2fd71f99 100644 --- a/docs/guides/executables.md +++ b/docs/guides/executables.md @@ -111,7 +111,9 @@ executables: - `secretRef`: Reference to vault secret - `prompt`: Interactive user input - `text`: Static value -- `envFile`: Load environment variables from a file +- `envFile`: Load environment variables from a file. Relative paths resolve against the + directory holding the flow file (`FLOW_DEFINITION_DIR`), not the workspace root or your + current directory — setting `dir` on the executable does not change this. ### Arguments (`args`) @@ -407,6 +409,14 @@ of the parent executable are inherited by the child executables. - `retries`: Number of times to retry failed steps - `reviewRequired`: Pause for user confirmation +Each step also accepts: +- `name`: A label for the step, shown in place of the ref in output +- `if`: An [Expr expression](./expressions) gating whether the step runs +- `interpreter`: The interpreter for a `cmd` step (a `ref` uses its own) +- `args`: Arguments to pass to a `ref`'d executable, written as you would type them + (`--flag=value` or a positional value). `$VAR` expands against the parent environment. + These override what the step would otherwise inherit. + ### parallel - Concurrent Execution Run multiple steps simultaneously: @@ -434,6 +444,14 @@ of the parent executable are inherited by the child executables. - `failFast`: Stop all operations on first failure (default: true) - `retries`: Number of times to retry failed operations +Each step also accepts: +- `name`: A label for the step, shown in place of the ref in output +- `if`: An [Expr expression](./expressions) gating whether the step runs +- `interpreter`: The interpreter for a `cmd` step (a `ref` uses its own) +- `args`: Arguments to pass to a `ref`'d executable, written as you would type them + (`--flag=value` or a positional value). `$VAR` expands against the parent environment. + These override what the step would otherwise inherit. + ### launch - Open Applications Open files, URLs, or applications: @@ -493,13 +511,32 @@ executables: - `method`: HTTP method (GET, POST, PUT, PATCH, DELETE) - `url`: Request URL (required) - `headers`: Custom headers -- `body`: Request body +- `body`: Request body — sent as written when it is JSON, otherwise an [Expr expression](./expressions) - `timeout`: Request timeout - `validStatusCodes`: Acceptable status codes - `logResponse`: Log response body - `transformResponse`: Expr expression to reshape the response before output or file save - `responseFile`: Save response to file +**Request bodies:** + +`$VAR` references in `body` are expanded first, the same as in `url` and `headers`. What +happens next depends on the result: + +- **A JSON object or array is sent as written.** This is the common case — write the body + as JSON and interpolate values with `$VAR`. +- **Anything else is evaluated as an [Expr expression](./expressions)** that must produce a + string. + +The expression form is worth reaching for when a value needs escaping. `toJSON` quotes and +escapes its argument, so a prompt containing quotes or newlines survives intact — where +pasting it into a JSON template would produce a malformed body: + +```yaml +body: > + '{"model":' + toJSON(env["MODEL"]) + ',"prompt":' + toJSON(env["PROMPT"]) + '}' +``` + **Transforming responses with `transformResponse`:** The `transformResponse` field is a single [Expr expression](./expressions) evaluated after the request completes. Its result replaces the raw response body in any output or `responseFile`. The expression has access to: @@ -524,7 +561,8 @@ transformResponse: fromJSON(body)["name"] transformResponse: upper(fromJSON(body)["status"]) # Format an array as newline-separated output -transformResponse: join(map(fromJSON(body)["items"], #["name"]), "\n") +# NOTE: quote any expression containing '#' - in YAML, a space then '#' starts a comment +transformResponse: 'join(map(fromJSON(body)["items"], #["name"]), "\n")' # Conditional with fallback transformResponse: code == 200 ? fromJSON(body)["result"] : "error " + string(code) + ": " + body @@ -561,6 +599,10 @@ executables: | `env` | `map[string]string` | Params and environment variables from the executable | | `data` | `any` | Parsed contents of `templateDataFile` (nil if not set) | +By default a `render` opens an interactive viewer. To use one non-interactively — in CI, a +script, or piped into another command — set `DISABLE_FLOW_INTERACTIVE=true`, which makes it +write plain text to stdout. See [Interactive UI](./interactive#disabling-the-tui). + `data` is typed based on the file content — a JSON object becomes a map, a JSON array becomes a slice. Access fields with bracket notation: `data["key"]` or `data[0]["field"]`. **Template file example** — given a `status-data.json`: diff --git a/docs/guides/expressions.md b/docs/guides/expressions.md index 0be04257..7e3d6366 100644 --- a/docs/guides/expressions.md +++ b/docs/guides/expressions.md @@ -5,7 +5,7 @@ description: "The Expr-based expression language flow uses for conditionals and # Expression Language -flow uses the [Expr language](https://expr-lang.org) for dynamic expressions and template logic. The same language appears in four places — learn it once and it works everywhere. +flow uses the [Expr language](https://expr-lang.org) for dynamic expressions and template logic. The same language appears in five places — learn it once and it works everywhere. ## Where Expressions Are Used @@ -13,12 +13,13 @@ flow uses the [Expr language](https://expr-lang.org) for dynamic expressions and |---------|--------|-------------------|:-----------:| | Step `if` conditions | Bare expression (no delimiters) | `os`, `arch`, `env`, `store`, `ctx` | ✓ | | `transformResponse` | Bare expression (no delimiters) | `body`, `code`, `status`, `headers` | | +| Request `body` | Bare expression, **only when the body is not JSON** | `env` | | | Template files (`.flow.tmpl`) | `{{ expression }}` delimiters | `name`, `form`, `env`, `os`, `arch`, … | | | Render templates (render `.md`) | `{{ expression }}` delimiters | `env`, `data` | | For the variables available in each surface, see the context-specific docs: - **Step conditions** — [Advanced Workflows: Conditional Execution](./advanced#conditional-execution) -- **transformResponse** — [Executables: request](./executables#request---http-requests) +- **transformResponse** and **request body** — [Executables: request](./executables#request---http-requests) - **Template files** — [Templates & Workflow Generation: Template Language](./templating#template-language) - **Render templates** — [Executables: render](./executables#render---dynamic-documentation) @@ -31,6 +32,37 @@ Expr is a sandboxed, typed, Go-native expression language. The key things that d - `if` conditions use `==`, `not`, `and`, `or` — not `eq`, `ne`, `!` - Shell execution via `$("command")` is available **only in step `if` conditions** — not in `transformResponse` or template surfaces +## Writing Expressions in a Flow File + +Expressions live inside YAML, and YAML gets a say in what reaches flow. Three things bite: + +**`#` starts a comment.** The closure shorthand used by `map()` and `filter()` is also YAML's +comment marker, so an unquoted expression is silently truncated at the first ` #`: + +```yaml +# WRONG — YAML keeps only `join(map(fromJSON(body)["items"],` +transformResponse: join(map(fromJSON(body)["items"], #["name"]), "\n") + +# RIGHT +transformResponse: 'join(map(fromJSON(body)["items"], #["name"]), "\n")' +``` + +The give-away is an error pointing past the end of what you wrote, like +`unexpected token EOF (1:32)`. + +**A leading `{` starts a flow mapping.** Quote or use a block scalar for anything beginning +with `{` or `[`. + +**A bare `: ` splits a key from a value.** Quote expressions containing one. + +Single quotes are the safest wrapper, since Expr's own string literals are usually double +quoted. When an expression needs both, reach for a block scalar: + +```yaml +if: > + env["DEPLOY_ENV"] == "production" and $("git branch --show-current") == "main" +``` + ## Core Syntax ### Operators diff --git a/docs/public/schemas/flowfile_schema.json b/docs/public/schemas/flowfile_schema.json index c5a2e505..dac57885 100644 --- a/docs/public/schemas/flowfile_schema.json +++ b/docs/public/schemas/flowfile_schema.json @@ -329,7 +329,7 @@ "type": "object", "properties": { "args": { - "description": "Arguments to pass to the executable.", + "description": "Arguments to pass to the executable, in the same form you would type them\n(`--flag=value` or a positional value). `$VAR` references are expanded against the\nparent's environment.\n\nThe parent's environment always reaches the executable; values listed here override\nwhat it would otherwise inherit for the arguments they set.\n", "type": "array", "default": [], "items": { @@ -357,7 +357,7 @@ }, "ref": { "$ref": "#/definitions/ExecutableRef", - "description": "A reference to another executable to run in serial.\nOne of `cmd` or `ref` must be set.\n", + "description": "A reference to another executable to run in parallel.\nOne of `cmd` or `ref` must be set.\n", "default": "" }, "retries": { @@ -460,7 +460,7 @@ "$ref": "#/definitions/ExecutableArgumentList" }, "body": { - "description": "The body of the request.", + "description": "The body of the request. `$VAR` references are expanded first.\n\nIf the result is a JSON object or array, it is sent as written. Otherwise it is\nevaluated as an Expr expression that must produce a string, which lets you build\na body from values that need escaping: `'{\"prompt\":' + toJSON(env[\"PROMPT\"]) + '}'`.\n", "type": "string", "default": "" }, @@ -582,7 +582,7 @@ "type": "object", "properties": { "args": { - "description": "Arguments to pass to the executable.", + "description": "Arguments to pass to the executable, in the same form you would type them\n(`--flag=value` or a positional value). `$VAR` references are expanded against the\nparent's environment.\n\nThe parent's environment always reaches the executable; values listed here override\nwhat it would otherwise inherit for the arguments they set.\n", "type": "array", "default": [], "items": { diff --git a/docs/types/flowfile.md b/docs/types/flowfile.md index 70baf640..b81c744b 100644 --- a/docs/types/flowfile.md +++ b/docs/types/flowfile.md @@ -196,12 +196,12 @@ Configuration for a parallel executable. | Field | Type | Default | Required | Description | | ----- | ---- | ------- | :------: | ----------- | -| `args` | `array` (`string`) | [] | | Arguments to pass to the executable. | +| `args` | `array` (`string`) | [] | | Arguments to pass to the executable, in the same form you would type them (`--flag=value` or a positional value). `$VAR` references are expanded against the parent's environment. The parent's environment always reaches the executable; values listed here override what it would otherwise inherit for the arguments they set. | | `cmd` | `string` | | | The command to execute. One of `cmd` or `ref` must be set. | | `if` | `string` | | | An expression that determines whether the executable should run, using the Expr language syntax. The expression is evaluated at runtime and must resolve to a boolean value. The expression has access to OS/architecture information (os, arch), environment variables (env), stored data (store), and context information (ctx) like workspace and paths. For example, `os == "darwin"` will only run on macOS, `len(store["feature"]) > 0` will run if a value exists in the store, and `env["CI"] == "true"` will run in CI environments. See the [Expr documentation](https://expr-lang.org/docs/language-definition) for more information. | | `interpreter` | [ExecutableExecInterpreter](#executableexecinterpreter) | | | The interpreter used to run `cmd` for this step. Defaults to `sh`. Only applies to `cmd`; a `ref` uses the referenced executable's own interpreter. | | `name` | `string` | | | A human-readable label for this step, used for display purposes. | -| `ref` | [ExecutableRef](#executableref) | | | A reference to another executable to run in serial. One of `cmd` or `ref` must be set. | +| `ref` | [ExecutableRef](#executableref) | | | A reference to another executable to run in parallel. One of `cmd` or `ref` must be set. | | `retries` | `integer` | 0 | | The number of times to retry the executable if it fails. | ### ExecutableParallelRefConfigList @@ -263,7 +263,7 @@ Makes an HTTP request. | Field | Type | Default | Required | Description | | ----- | ---- | ------- | :------: | ----------- | | `args` | [ExecutableArgumentList](#executableargumentlist) | | | | -| `body` | `string` | | | The body of the request. | +| `body` | `string` | | | The body of the request. `$VAR` references are expanded first. If the result is a JSON object or array, it is sent as written. Otherwise it is evaluated as an Expr expression that must produce a string, which lets you build a body from values that need escaping: `'{"prompt":' + toJSON(env["PROMPT"]) + '}'`. | | `headers` | `map` (`string` -> `string`) | map[] | | A map of headers to include in the request. | | `logResponse` | `boolean` | false | | If set to true, the response will be logged as program output. | | `method` | `string` | GET | | The HTTP method to use when making the request. | @@ -305,7 +305,7 @@ Configuration for a serial executable. | Field | Type | Default | Required | Description | | ----- | ---- | ------- | :------: | ----------- | -| `args` | `array` (`string`) | [] | | Arguments to pass to the executable. | +| `args` | `array` (`string`) | [] | | Arguments to pass to the executable, in the same form you would type them (`--flag=value` or a positional value). `$VAR` references are expanded against the parent's environment. The parent's environment always reaches the executable; values listed here override what it would otherwise inherit for the arguments they set. | | `cmd` | `string` | | | The command to execute. One of `cmd` or `ref` must be set. | | `if` | `string` | | | An expression that determines whether the executable should run, using the Expr language syntax. The expression is evaluated at runtime and must resolve to a boolean value. The expression has access to OS/architecture information (os, arch), environment variables (env), stored data (store), and context information (ctx) like workspace and paths. For example, `os == "darwin"` will only run on macOS, `len(store["feature"]) > 0` will run if a value exists in the store, and `env["CI"] == "true"` will run in CI environments. See the [Expr documentation](https://expr-lang.org/docs/language-definition) for more information. | | `interpreter` | [ExecutableExecInterpreter](#executableexecinterpreter) | | | The interpreter used to run `cmd` for this step. Defaults to `sh`. Only applies to `cmd`; a `ref` uses the referenced executable's own interpreter. | diff --git a/internal/runner/childenv.go b/internal/runner/childenv.go new file mode 100644 index 00000000..a811f10a --- /dev/null +++ b/internal/runner/childenv.go @@ -0,0 +1,58 @@ +package runner + +import ( + "maps" + + envUtils "github.com/flowexec/flow/v2/internal/utils/env" + "github.com/flowexec/flow/v2/pkg/logger" + "github.com/flowexec/flow/v2/types/executable" +) + +// ChildEnvAndArgs builds the environment and argument list for one child of a serial or +// parallel executable. The parent's resolved environment always reaches the child: it is +// the only path for a child that does not run a subprocess, since a request or render +// runner builds its own map and never reads the process environment. +// +// Arguments written on the step take precedence over inherited values. When the step +// declares none, the child's arguments are rebuilt from the parent environment by +// matching envKeys. +func ChildEnvAndArgs( + parentEnv map[string]string, + refArgs []string, + child *executable.Executable, +) (map[string]string, []string) { + childEnv := maps.Clone(parentEnv) + if childEnv == nil { + childEnv = make(map[string]string) + } + + childArgs := make([]string, 0) + execEnv := child.Env() + if execEnv == nil || len(execEnv.Args) == 0 { + if len(refArgs) > 0 { + logger.Log().Warnf( + "executable %s has no arguments defined, skipping argument processing", + child.Ref().String(), + ) + } + return childEnv, childArgs + } + + buildEnvMap := envUtils.BuildArgsEnvMap + if len(refArgs) > 0 { + for _, arg := range refArgs { + childArgs = append(childArgs, envUtils.ExpandAuthored(arg, childEnv)) + } + buildEnvMap = envUtils.BuildChildArgsEnvMap + } else { + childArgs = envUtils.BuildArgsFromEnv(execEnv.Args, childEnv) + } + + argEnv, err := buildEnvMap(execEnv.Args, childArgs, childEnv) + if err != nil { + logger.Log().WrapError(err, "unable to process arguments") + } + maps.Copy(childEnv, argEnv) + + return childEnv, childArgs +} diff --git a/internal/runner/launch/launch.go b/internal/runner/launch/launch.go index e608e472..8ba13787 100644 --- a/internal/runner/launch/launch.go +++ b/internal/runner/launch/launch.go @@ -56,7 +56,7 @@ func (r *launchRunner) Exec( if err != nil { return errors.Wrap(err, "unable to set parameters to env") } - if err := env.SetEnv(ctx.Config.CurrentVaultName(), e.Env(), inputArgs, envMap); err != nil { + if _, err := env.SetEnv(ctx.Config.CurrentVaultName(), e.Env(), inputArgs, envMap); err != nil { return errors.Wrap(err, "unable to set parameters to env") } diff --git a/internal/runner/parallel/parallel.go b/internal/runner/parallel/parallel.go index 4c619e0c..46520393 100644 --- a/internal/runner/parallel/parallel.go +++ b/internal/runner/parallel/parallel.go @@ -3,10 +3,8 @@ package parallel import ( stdCtx "context" "fmt" - "maps" "os" "path/filepath" - "strings" "github.com/flowexec/tuikit/io" "github.com/jahvon/expression" @@ -48,7 +46,8 @@ func (r *parallelRunner) Exec( inputArgs []string, ) error { parallelSpec := e.Parallel - if err := envUtils.SetEnv(ctx.Config.CurrentVaultName(), e.Env(), inputArgs, inputEnv); err != nil { + parentEnv, err := envUtils.SetEnv(ctx.Config.CurrentVaultName(), e.Env(), inputArgs, inputEnv) + if err != nil { return errors.Wrap(err, "unable to set parameters to env") } @@ -67,7 +66,7 @@ func (r *parallelRunner) Exec( } if len(parallelSpec.Execs) > 0 { - return handleExec(ctx, e, eng, parallelSpec, inputEnv) + return handleExec(ctx, e, eng, parallelSpec, parentEnv) } return fmt.Errorf("no parallel executables to run") @@ -77,7 +76,7 @@ func handleExec( ctx *context.Context, parent *executable.Executable, eng engine.Engine, parallelSpec *executable.ParallelExecutableType, - inputEnv map[string]string, + parentEnv map[string]string, ) error { groupCtx, cancel := stdCtx.WithCancel(ctx) defer cancel() @@ -102,7 +101,7 @@ func handleExec( root.WorkspacePath(), root.FlowFilePath(), ctx.ProcessTmpDir, - inputEnv, + parentEnv, ) if err != nil { return errors.Wrap(err, "unable to expand directory") @@ -145,40 +144,7 @@ func handleExec( exec := resolved[i].exec // Prepare the environment and arguments for the child executable - childEnv := make(map[string]string) - childArgs := make([]string, 0) - maps.Copy(childEnv, inputEnv) - if len(refConfig.Args) > 0 { - execEnv := exec.Env() - if execEnv == nil || execEnv.Args == nil { - logger.Log().Warnf( - "executable %s has no arguments defined, skipping argument processing", - exec.Ref().String(), - ) - } else { - for _, arg := range os.Environ() { - kv := strings.SplitN(arg, "=", 2) - if len(kv) == 2 { - childEnv[kv[0]] = kv[1] - } - } - - if parallelSpec.Args == nil { - childArgs = refConfig.Args - } else { - childArgs = envUtils.BuildArgsFromEnv(execEnv.Args, childEnv) - if len(childArgs) == 0 { - childArgs = refConfig.Args // If no resolved args, fallback to original args - } - } - - a, err := envUtils.BuildArgsEnvMap(execEnv.Args, childArgs, childEnv) - if err != nil { - logger.Log().WrapError(err, "unable to process arguments") - } - maps.Copy(childEnv, a) - } - } + childEnv, childArgs := runner.ChildEnvAndArgs(parentEnv, refConfig.Args, exec) // Set log fields and directory for the executable switch { @@ -244,7 +210,7 @@ func handleExec( return false, err } - conditionalData := runner.ExpressionEnv(ctx, parent, cacheData, inputEnv) + conditionalData := runner.ExpressionEnv(ctx, parent, cacheData, parentEnv) truthy, err := expression.IsTruthy(ifCondition, conditionalData) if err != nil { return false, err diff --git a/internal/runner/parallel/parallel_test.go b/internal/runner/parallel/parallel_test.go index 814c058f..4e8c5fea 100644 --- a/internal/runner/parallel/parallel_test.go +++ b/internal/runner/parallel/parallel_test.go @@ -162,7 +162,7 @@ var _ = Describe("ParallelRunner", func() { parentExec := &executable.Executable{ Parallel: &executable.ParallelExecutableType{ Args: executable.ArgumentList{{EnvKey: "TEST_VAR", Pos: &pos1}}, - Execs: []executable.ParallelRefConfig{{Ref: "test:child", Args: []string{"var=$TEST_VAR"}}}, + Execs: []executable.ParallelRefConfig{{Ref: "test:child", Args: []string{"--var=$TEST_VAR"}}}, }, } parentExec.SetContext("test", "/test", "test", "/test/parent.flow") diff --git a/internal/runner/render/render.go b/internal/runner/render/render.go index fc81b633..9ee00eba 100644 --- a/internal/runner/render/render.go +++ b/internal/runner/render/render.go @@ -67,7 +67,7 @@ func (r *renderRunner) Exec( inputArgs []string, ) error { renderSpec := e.Render - if err := env.SetEnv(ctx.Config.CurrentVaultName(), e.Env(), inputArgs, inputEnv); err != nil { + if _, err := env.SetEnv(ctx.Config.CurrentVaultName(), e.Env(), inputArgs, inputEnv); err != nil { return errors.Wrap(err, "unable to set parameters to env") } diff --git a/internal/runner/request/request.go b/internal/runner/request/request.go index e58c0afd..8e101c83 100644 --- a/internal/runner/request/request.go +++ b/internal/runner/request/request.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/jahvon/expression" "github.com/pkg/errors" @@ -51,9 +52,9 @@ func (r *requestRunner) Exec( return errors.Wrap(err, "unable to set parameters to env") } - url := expandEnvVars(envMap, requestSpec.URL) - body := expandEnvVars(envMap, requestSpec.Body) - if body != "" { + url := env.ExpandAuthored(requestSpec.URL, envMap) + body := env.ExpandAuthored(requestSpec.Body, envMap) + if body != "" && !isJSONDocument(body) { body, err = expression.EvaluateString(body, map[string]interface{}{"env": envMap}) if err != nil { return errors.Wrap(err, "unable to evaluate request body expression") @@ -61,7 +62,7 @@ func (r *requestRunner) Exec( } for key, value := range requestSpec.Headers { - requestSpec.Headers[key] = expandEnvVars(envMap, value) + requestSpec.Headers[key] = env.ExpandAuthored(value, envMap) } restRequest := rest.Request{ URL: url, @@ -176,11 +177,14 @@ func writeResponseToFile(resp, responseFile string, format executable.RequestRes return nil } -func expandEnvVars(envMap map[string]string, value string) string { - if envMap == nil || value == "" { - return value +// isJSONDocument reports whether the body is already a JSON object or array, in which +// case it is sent as written. Anything else is an Expr expression. Restricting this to +// objects and arrays keeps a bare JSON scalar - `"hello"` reads as a quoted string to +// JSON and as a bare one to Expr - out of the ambiguous middle. +func isJSONDocument(body string) bool { + trimmed := strings.TrimSpace(body) + if !strings.HasPrefix(trimmed, "{") && !strings.HasPrefix(trimmed, "[") { + return false } - return os.Expand(value, func(envVar string) string { - return envMap[envVar] - }) + return json.Valid([]byte(trimmed)) } diff --git a/internal/runner/request/request_test.go b/internal/runner/request/request_test.go index f077353f..85d40ac5 100644 --- a/internal/runner/request/request_test.go +++ b/internal/runner/request/request_test.go @@ -2,6 +2,7 @@ package request_test import ( stdCtx "context" + "io" "net/http" "net/http/httptest" "os" @@ -79,6 +80,58 @@ var _ = Describe("Request Runner", func() { })) }) + Describe("body", func() { + var received string + var bodyServer *httptest.Server + + BeforeEach(func() { + received = "" + bodyServer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + b, _ := io.ReadAll(r.Body) + received = string(b) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ok": true}`)) + })) + }) + + AfterEach(func() { bodyServer.Close() }) + + sendBody := func(body string, envMap map[string]string) { + exec := &executable.Executable{ + Request: &executable.RequestExecutableType{ + URL: bodyServer.URL, + Method: executable.RequestExecutableTypeMethodPOST, + Body: body, + }, + } + ctx.Logger.EXPECT().Infof(gomock.Any(), gomock.Any()).Times(1) + Expect(requestRnr.Exec(ctx.Ctx, exec, mockEngine, envMap, nil)).To(Succeed()) + } + + It("should send a JSON object body as written", func() { + sendBody( + "{\n \"environment\": \"$ENVIRONMENT\",\n \"version\": \"$VERSION\"\n}", + map[string]string{"ENVIRONMENT": "prod", "VERSION": "1.0"}, + ) + Expect(received).To(MatchJSON(`{"environment": "prod", "version": "1.0"}`)) + }) + + It("should send a nested JSON body as written", func() { + sendBody(`{"messages": [{"role": "user"}]}`, map[string]string{}) + Expect(received).To(MatchJSON(`{"messages": [{"role": "user"}]}`)) + }) + + It("should evaluate a non-JSON body as an expression", func() { + sendBody(`'{"model":' + toJSON(env["MODEL"]) + '}'`, map[string]string{"MODEL": `a "quoted" name`}) + Expect(received).To(MatchJSON(`{"model": "a \"quoted\" name"}`)) + }) + + It("should send a JSON array body as written", func() { + sendBody(`[1, 2, 3]`, map[string]string{}) + Expect(received).To(Equal(`[1, 2, 3]`)) + }) + }) + It("should send a GET request and log the response", func() { exec := &executable.Executable{ Request: &executable.RequestExecutableType{ diff --git a/internal/runner/serial/serial.go b/internal/runner/serial/serial.go index ab1f24f6..e20c54f8 100644 --- a/internal/runner/serial/serial.go +++ b/internal/runner/serial/serial.go @@ -3,7 +3,6 @@ package serial import ( "bufio" "fmt" - "maps" "os" "path/filepath" "strings" @@ -47,7 +46,8 @@ func (r *serialRunner) Exec( inputArgs []string, ) error { serialSpec := e.Serial - if err := envUtils.SetEnv(ctx.Config.CurrentVaultName(), e.Env(), inputArgs, inputEnv); err != nil { + parentEnv, err := envUtils.SetEnv(ctx.Config.CurrentVaultName(), e.Env(), inputArgs, inputEnv) + if err != nil { return errors.Wrap(err, "unable to set parameters to env") } @@ -66,7 +66,7 @@ func (r *serialRunner) Exec( } if len(serialSpec.Execs) > 0 { - return handleExec(ctx, e, eng, serialSpec, inputEnv) + return handleExec(ctx, e, eng, serialSpec, parentEnv) } return fmt.Errorf("no serial executables to run") } @@ -76,7 +76,7 @@ func handleExec( parent *executable.Executable, eng engine.Engine, serialSpec *executable.SerialExecutableType, - inputEnv map[string]string, + parentEnv map[string]string, ) error { // Expand the directory of the serial execution. The root / parent's directory is used if one is not specified. var root *executable.Executable @@ -92,7 +92,7 @@ func handleExec( root.WorkspacePath(), root.FlowFilePath(), ctx.ProcessTmpDir, - inputEnv, + parentEnv, ) if err != nil { return errors.Wrap(err, "unable to expand directory") @@ -135,40 +135,7 @@ func handleExec( exec := resolved[i].exec // Prepare the environment and arguments for the child executable - childEnv := make(map[string]string) - childArgs := make([]string, 0) - maps.Copy(childEnv, inputEnv) - if len(refConfig.Args) > 0 { - execEnv := exec.Env() - if execEnv == nil || execEnv.Args == nil { - logger.Log().Warnf( - "executable %s has no arguments defined, skipping argument processing", - exec.Ref().String(), - ) - } else { - for _, arg := range os.Environ() { - kv := strings.SplitN(arg, "=", 2) - if len(kv) == 2 { - childEnv[kv[0]] = kv[1] - } - } - - if serialSpec.Args == nil { - childArgs = refConfig.Args - } else { - childArgs = envUtils.BuildArgsFromEnv(execEnv.Args, childEnv) - if len(childArgs) == 0 { - childArgs = refConfig.Args // If no resolved args, fallback to original args - } - } - - a, err := envUtils.BuildArgsEnvMap(execEnv.Args, childArgs, childEnv) - if err != nil { - logger.Log().WrapError(err, "unable to process arguments") - } - maps.Copy(childEnv, a) - } - } + childEnv, childArgs := runner.ChildEnvAndArgs(parentEnv, refConfig.Args, exec) // Set log fields and directory for the executable switch { @@ -228,7 +195,7 @@ func handleExec( return false, err } - conditionalData := runner.ExpressionEnv(ctx, parent, cacheData, inputEnv) + conditionalData := runner.ExpressionEnv(ctx, parent, cacheData, parentEnv) truthy, err := expression.IsTruthy(ifCondition, conditionalData) if err != nil { return false, err diff --git a/internal/runner/serial/serial_test.go b/internal/runner/serial/serial_test.go index f2c9142b..f4038a38 100644 --- a/internal/runner/serial/serial_test.go +++ b/internal/runner/serial/serial_test.go @@ -25,6 +25,19 @@ func TestSerialRunner(t *testing.T) { RunSpecs(t, "Serial Runner Suite") } +// expectEngineRun stands in for the engine, running each step's function inline so the +// child runner mock sees the env and args the runner built for it. +func expectEngineRun(mockEngine *mocks.MockEngine) { + mockEngine.EXPECT(). + Execute(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(_ stdCtx.Context, execs []engine.Exec, _ ...engine.OptionFunc) engine.ResultSummary { + for _, exec := range execs { + Expect(exec.Function()).To(Succeed()) + } + return engine.ResultSummary{Results: []engine.Result{{}}} + }) +} + var _ = Describe("SerialRunner", func() { var ( ctx *testUtils.ContextWithMocks @@ -151,6 +164,121 @@ var _ = Describe("SerialRunner", func() { Expect(serialRnr.Exec(ctx.Ctx, rootExec, mockEngine, make(map[string]string), nil)).To(Succeed()) }) + It("should pass parent params to a child that declares no args", func() { + parentExec := &executable.Executable{ + Serial: &executable.SerialExecutableType{ + Params: executable.ParameterList{{EnvKey: "OUTERP", Text: "outer-param"}}, + Execs: []executable.SerialRefConfig{{Ref: "test:child"}}, + }, + } + parentExec.SetContext("test", "/test", "test", "/test/parent.flow") + + childExec := &executable.Executable{ + Request: &executable.RequestExecutableType{URL: "http://127.0.0.1/path?p=$OUTERP"}, + } + childExec.SetContext("test", "/test", "test", "/test/child.flow") + ctx.ExecutableCache.EXPECT().GetExecutableByRef(gomock.Any()).Return(childExec, nil).Times(1) + + ctx.RunnerMock.EXPECT().IsCompatible(gomock.Any()).Return(true).Times(1) + ctx.RunnerMock.EXPECT(). + Exec(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func( + _ *context.Context, + _ *executable.Executable, + _ engine.Engine, + inputEnv map[string]string, + _ []string, + ) error { + Expect(inputEnv).To(HaveKeyWithValue("OUTERP", "outer-param")) + return nil + }).Times(1) + + expectEngineRun(mockEngine) + Expect(serialRnr.Exec(ctx.Ctx, parentExec, mockEngine, make(map[string]string), nil)).To(Succeed()) + }) + + It("should pass a parent arg to a child that declares no step args", func() { + pos1 := 1 + parentExec := &executable.Executable{ + Serial: &executable.SerialExecutableType{ + Args: executable.ArgumentList{{EnvKey: "OUTER", Pos: &pos1}}, + Execs: []executable.SerialRefConfig{{Ref: "test:child"}}, + }, + } + parentExec.SetContext("test", "/test", "test", "/test/parent.flow") + + childExec := &executable.Executable{ + Exec: &executable.ExecExecutableType{ + Cmd: "echo $OUTER", + Args: executable.ArgumentList{{EnvKey: "OUTER", Pos: &pos1, Default: "(unset)"}}, + }, + } + childExec.SetContext("test", "/test", "test", "/test/child.flow") + ctx.ExecutableCache.EXPECT().GetExecutableByRef(gomock.Any()).Return(childExec, nil).Times(1) + + ctx.RunnerMock.EXPECT().IsCompatible(gomock.Any()).Return(true).Times(1) + ctx.RunnerMock.EXPECT(). + Exec(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func( + _ *context.Context, + _ *executable.Executable, + _ engine.Engine, + inputEnv map[string]string, + inputArgs []string, + ) error { + Expect(inputEnv).To(HaveKeyWithValue("OUTER", "passed-in")) + Expect(inputArgs).To(ContainElement("passed-in")) + return nil + }).Times(1) + + expectEngineRun(mockEngine) + Expect(serialRnr.Exec( + ctx.Ctx, parentExec, mockEngine, make(map[string]string), []string{"passed-in"}, + )).To(Succeed()) + }) + + It("should let args declared on the step override the inherited value", func() { + pos1 := 1 + parentExec := &executable.Executable{ + Serial: &executable.SerialExecutableType{ + Args: executable.ArgumentList{{EnvKey: "OUTER", Pos: &pos1}}, + Execs: []executable.SerialRefConfig{{ + Ref: "test:child", + Args: []string{"--var=step-value"}, + }}, + }, + } + parentExec.SetContext("test", "/test", "test", "/test/parent.flow") + + childExec := &executable.Executable{ + Exec: &executable.ExecExecutableType{ + Cmd: "echo $OUTER", + Args: executable.ArgumentList{{EnvKey: "OUTER", Flag: "var"}}, + }, + } + childExec.SetContext("test", "/test", "test", "/test/child.flow") + ctx.ExecutableCache.EXPECT().GetExecutableByRef(gomock.Any()).Return(childExec, nil).Times(1) + + ctx.RunnerMock.EXPECT().IsCompatible(gomock.Any()).Return(true).Times(1) + ctx.RunnerMock.EXPECT(). + Exec(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func( + _ *context.Context, + _ *executable.Executable, + _ engine.Engine, + inputEnv map[string]string, + _ []string, + ) error { + Expect(inputEnv).To(HaveKeyWithValue("OUTER", "step-value")) + return nil + }).Times(1) + + expectEngineRun(mockEngine) + Expect(serialRnr.Exec( + ctx.Ctx, parentExec, mockEngine, make(map[string]string), []string{"parent-value"}, + )).To(Succeed()) + }) + It("should pass environment args from parent to child executables", func() { pos1 := 1 parentExec := &executable.Executable{ @@ -158,7 +286,7 @@ var _ = Describe("SerialRunner", func() { Args: executable.ArgumentList{{EnvKey: "TEST_VAR", Pos: &pos1}}, Execs: []executable.SerialRefConfig{{ Ref: "test:child", - Args: []string{"var=$TEST_VAR"}, + Args: []string{"--var=$TEST_VAR"}, }, }, }, diff --git a/internal/utils/env/args.go b/internal/utils/env/args.go index 0fdccc14..0c9d81bd 100644 --- a/internal/utils/env/args.go +++ b/internal/utils/env/args.go @@ -1,7 +1,6 @@ package env import ( - "os" "slices" "sort" "strconv" @@ -15,11 +14,26 @@ func BuildArgsEnvMap( execArgs []string, env map[string]string, ) (map[string]string, error) { - al, err := resolveArgValues(args, execArgs, env) + al, err := resolveArgValues(args, execArgs, env, false) if err != nil { return nil, err } - return argsToEnvMap(al), nil + return argsToEnvMap(al, env), nil +} + +// BuildChildArgsEnvMap is BuildArgsEnvMap with the precedence a child step needs: a value +// passed explicitly on the step wins over one inherited from the parent environment. At +// the top level the opposite holds, so that a --param override beats a positional arg. +func BuildChildArgsEnvMap( + args executable.ArgumentList, + execArgs []string, + env map[string]string, +) (map[string]string, error) { + al, err := resolveArgValues(args, execArgs, env, true) + if err != nil { + return nil, err + } + return argsToEnvMap(al, env), nil } func parseArgs(args executable.ArgumentList, execArgs []string) (flagArgs map[string]string, posArgs []string) { @@ -62,20 +76,13 @@ func resolveArgValues( args executable.ArgumentList, execArgs []string, env map[string]string, + preferInput bool, ) (executable.ArgumentList, error) { if len(args) == 0 { return nil, nil } - if env != nil { - // Expand environment variables in arguments - for i, a := range execArgs { - execArgs[i] = os.Expand(a, func(key string) string { - return env[key] - }) - } - } flagArgs, posArgs := parseArgs(args, execArgs) - if err := setArgValues(args, flagArgs, posArgs, env); err != nil { + if err := setArgValues(args, flagArgs, posArgs, env, preferInput); err != nil { return nil, err } return args, nil @@ -86,43 +93,64 @@ func setArgValues( flagArgs map[string]string, posArgs []string, env map[string]string, + preferInput bool, ) error { - for i, arg := range args { - if arg.EnvKey != "" { - if val, found := env[arg.EnvKey]; found { - // Use the input value if provided - arg.Set(val) - args[i] = arg - continue - } + fromEnv := func(arg executable.Argument) (string, bool) { + if arg.EnvKey == "" { + return "", false } - + val, found := env[arg.EnvKey] + return val, found + } + fromInput := func(arg executable.Argument) (string, bool) { if arg.Flag != "" { - if val, ok := flagArgs[arg.Flag]; ok { + val, ok := flagArgs[arg.Flag] + return val, ok + } + if arg.Pos != nil && *arg.Pos != 0 && *arg.Pos <= len(posArgs) { + return posArgs[*arg.Pos-1], true + } + return "", false + } + + sources := []func(executable.Argument) (string, bool){fromEnv, fromInput} + if preferInput { + sources = []func(executable.Argument) (string, bool){fromInput, fromEnv} + } + for i, arg := range args { + for _, source := range sources { + if val, ok := source(arg); ok { arg.Set(val) args[i] = arg - } - } else if arg.Pos != nil && *arg.Pos != 0 { - if *arg.Pos <= len(posArgs) { - arg.Set(posArgs[*arg.Pos-1]) - args[i] = arg + break } } } return args.ValidateValues() } -func argsToEnvMap(args executable.ArgumentList) map[string]string { +func argsToEnvMap(args executable.ArgumentList, env map[string]string) map[string]string { envMap := make(map[string]string) for _, arg := range args { if arg.OutputFile != "" && arg.EnvKey == "" { continue } - envMap[arg.EnvKey] = arg.Value() + envMap[arg.EnvKey] = argValue(arg, env) } return envMap } +// argValue returns the value to use for a resolved argument. A value that was actually +// supplied - on the command line, or inherited from the parent environment - is a +// literal. Only the declared default is authored in the flow file, so only it is +// expanded. +func argValue(arg executable.Argument, env map[string]string) string { + if arg.IsSet() { + return arg.Value() + } + return ExpandAuthored(arg.Default, env) +} + func filterArgsWithOutputFile(args executable.ArgumentList) executable.ArgumentList { var outputArgs executable.ArgumentList for _, arg := range args { diff --git a/internal/utils/env/env.go b/internal/utils/env/env.go index 345b1ca3..f588c5c0 100644 --- a/internal/utils/env/env.go +++ b/internal/utils/env/env.go @@ -15,7 +15,10 @@ import ( "github.com/flowexec/flow/v2/types/executable" ) -// SetEnv sets environment variables based on the parameters and arguments defined in the executable environment. +// SetEnv sets environment variables based on the parameters and arguments defined in the +// executable environment. It returns the fully resolved environment - the input env plus +// every param and argument value - so callers can hand it to child executables, which do +// not inherit the process environment this writes to. // //nolint:gocognit func SetEnv( @@ -23,7 +26,7 @@ func SetEnv( exec *executable.ExecutableEnvironment, inputArgs []string, inputEnv map[string]string, -) error { +) (map[string]string, error) { var errs []error envMap := make(map[string]string) @@ -82,21 +85,16 @@ func SetEnv( errs = append(errs, fmt.Errorf("failed to build inputArgs env map: %w", err)) } for key, val := range argEnvMap { - val = os.Expand(val, func(key string) string { - if v, ok := envMap[key]; ok { - return v - } - return "" - }) if err := os.Setenv(key, val); err != nil { errs = append(errs, fmt.Errorf("failed to set env %s: %w", key, err)) } + envMap[key] = val } if len(errs) > 0 { - return fmt.Errorf("failed to set values for parameters: %w", errors.Join(errs...)) + return envMap, fmt.Errorf("failed to set values for parameters: %w", errors.Join(errs...)) } - return nil + return envMap, nil } // CreateTempEnvFiles creates temporary files for parameters and arguments that have an OutputFile defined. @@ -126,13 +124,13 @@ func CreateTempEnvFiles( tempFiles = append(tempFiles, dest) } - al, err := resolveArgValues(exec.Args, args, promptedEnv) + al, err := resolveArgValues(exec.Args, args, promptedEnv, false) if err != nil { errs = append(errs, err) } else { filtered := filterArgsWithOutputFile(al) for _, arg := range filtered { - dest, err := createEnvValueFile(arg.OutputFile, arg.Value(), wsPath, flowfilePath, promptedEnv) + dest, err := createEnvValueFile(arg.OutputFile, argValue(arg, promptedEnv), wsPath, flowfilePath, promptedEnv) if err != nil { errs = append(errs, err) continue @@ -215,12 +213,7 @@ func BuildEnvMap( return nil, fmt.Errorf("failed to build inputArgs env map: %w", err) } for key, val := range argEnvMap { - envMap[key] = os.Expand(val, func(key string) string { - if v, ok := envMap[key]; ok { - return v - } - return "" - }) + envMap[key] = val } if len(errs) > 0 { diff --git a/internal/utils/env/env_test.go b/internal/utils/env/env_test.go index 942eb4ec..5117a906 100644 --- a/internal/utils/env/env_test.go +++ b/internal/utils/env/env_test.go @@ -53,7 +53,7 @@ var _ = Describe("Env", func() { promptedEnv := map[string]string{ "TEST_PROMPT": "my value", } - err := env.SetEnv("demo", exec, []string{}, promptedEnv) + _, err := env.SetEnv("demo", exec, []string{}, promptedEnv) Expect(err).ToNot(HaveOccurred()) val, exists := os.LookupEnv("TEST_TEXT") Expect(exists).To(BeTrue()) @@ -82,7 +82,7 @@ TEST_ENV_VAR3=value3` }, } promptedEnv := map[string]string{} - err = env.SetEnv("", exec, []string{}, promptedEnv) + _, err = env.SetEnv("", exec, []string{}, promptedEnv) Expect(err).ToNot(HaveOccurred()) val, exists := os.LookupEnv("TEST_ENV_VAR1") @@ -111,7 +111,7 @@ TEST_ENV_VAR3=value3` }, } promptedEnv := map[string]string{} - err = env.SetEnv("", exec, []string{}, promptedEnv) + _, err = env.SetEnv("", exec, []string{}, promptedEnv) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("env key SPECIFIC_VAR not found in env file")) }) @@ -136,7 +136,7 @@ TEST_ENV_VAR3=value3` }, } promptedEnv := map[string]string{} - err = env.SetEnv("", exec, []string{}, promptedEnv) + _, err = env.SetEnv("", exec, []string{}, promptedEnv) Expect(err).ToNot(HaveOccurred()) val, exists := os.LookupEnv("TEST_ENV_VAR2") @@ -156,7 +156,7 @@ TEST_ENV_VAR3=value3` }, } promptedEnv := map[string]string{} - err := env.SetEnv("", exec, []string{}, promptedEnv) + _, err := env.SetEnv("", exec, []string{}, promptedEnv) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("failed to read .env file")) }) @@ -170,7 +170,7 @@ TEST_ENV_VAR3=value3` }, } promptedEnv := make(map[string]string) - err := env.SetEnv("", exec, []string{"test", "--flag=value"}, promptedEnv) + _, err := env.SetEnv("", exec, []string{"test", "--flag=value"}, promptedEnv) Expect(err).ToNot(HaveOccurred()) val, exists := os.LookupEnv("TEST_POS") Expect(exists).To(BeTrue()) @@ -186,7 +186,7 @@ TEST_ENV_VAR3=value3` Args: []executable.Argument{{EnvKey: "TEST_KEY", Flag: "flag"}}, } promptedEnv := map[string]string{"TEST_KEY": "input"} - err := env.SetEnv("", exec, []string{"--flag=flag"}, promptedEnv) + _, err := env.SetEnv("", exec, []string{"--flag=flag"}, promptedEnv) Expect(err).ToNot(HaveOccurred()) val, exists := os.LookupEnv("TEST_KEY") Expect(exists).To(BeTrue()) @@ -199,12 +199,55 @@ TEST_ENV_VAR3=value3` Args: []executable.Argument{{EnvKey: "TEST_KEY", Flag: "flag"}}, } promptedEnv := map[string]string{"TEST_KEY": "input"} - err := env.SetEnv("", exec, []string{"--flag=flag"}, promptedEnv) + _, err := env.SetEnv("", exec, []string{"--flag=flag"}, promptedEnv) Expect(err).ToNot(HaveOccurred()) val, exists := os.LookupEnv("TEST_KEY") Expect(exists).To(BeTrue()) Expect(val).To(Equal("input")) }) + + It("should not expand $ sequences in a user-supplied arg value", func() { + pos := 1 + exec := &executable.ExecutableEnvironment{ + Args: []executable.Argument{{EnvKey: "TEST_LITERAL", Pos: &pos}}, + } + promptedEnv := map[string]string{"KNOWN": "resolved"} + _, err := env.SetEnv("", exec, []string{"arg has $5 and $HOME and $KNOWN"}, promptedEnv) + Expect(err).ToNot(HaveOccurred()) + Expect(os.Getenv("TEST_LITERAL")).To(Equal("arg has $5 and $HOME and $KNOWN")) + }) + + It("should expand an authored default against the resolved env", func() { + pos := 1 + exec := &executable.ExecutableEnvironment{ + Params: []executable.Parameter{{EnvKey: "GREETING", Text: "hello"}}, + Args: []executable.Argument{{EnvKey: "TEST_DEFAULT", Pos: &pos, Default: "$GREETING world"}}, + } + _, err := env.SetEnv("", exec, []string{}, map[string]string{}) + Expect(err).ToNot(HaveOccurred()) + Expect(os.Getenv("TEST_DEFAULT")).To(Equal("hello world")) + }) + + It("should leave an unresolved variable in a default as written and unescape $$", func() { + pos := 1 + exec := &executable.ExecutableEnvironment{ + Args: []executable.Argument{{EnvKey: "TEST_UNRESOLVED", Pos: &pos, Default: "$$5 for $NOPE"}}, + } + _, err := env.SetEnv("", exec, []string{}, map[string]string{}) + Expect(err).ToNot(HaveOccurred()) + Expect(os.Getenv("TEST_UNRESOLVED")).To(Equal("$5 for $NOPE")) + }) + + It("should not modify the input args slice", func() { + pos := 1 + exec := &executable.ExecutableEnvironment{ + Args: []executable.Argument{{EnvKey: "TEST_INPUT", Pos: &pos}}, + } + inputArgs := []string{"$HOME"} + _, err := env.SetEnv("", exec, inputArgs, map[string]string{"HOME": "/somewhere"}) + Expect(err).ToNot(HaveOccurred()) + Expect(inputArgs).To(Equal([]string{"$HOME"})) + }) }) }) diff --git a/internal/utils/env/expand.go b/internal/utils/env/expand.go new file mode 100644 index 00000000..498e7ae6 --- /dev/null +++ b/internal/utils/env/expand.go @@ -0,0 +1,26 @@ +package env + +import "os" + +// ExpandAuthored expands $VAR and ${VAR} references in a string written in a flow file, +// resolving them against envMap. "$$" yields a literal "$". A variable that does not +// resolve is left as written, so a missing value shows up in the output instead of +// silently disappearing; note that the brace form is not preserved, so an unresolved +// "${FOO}" comes back as "$FOO". +// +// Values supplied by a user - command line arguments, prompt responses - are literals +// and must not be passed through this. +func ExpandAuthored(value string, envMap map[string]string) string { + if value == "" { + return value + } + return os.Expand(value, func(key string) string { + if key == "$" { + return "$" + } + if v, ok := envMap[key]; ok { + return v + } + return "$" + key + }) +} diff --git a/internal/validation/flowfile_schema.json b/internal/validation/flowfile_schema.json index c5a2e505..dac57885 100644 --- a/internal/validation/flowfile_schema.json +++ b/internal/validation/flowfile_schema.json @@ -329,7 +329,7 @@ "type": "object", "properties": { "args": { - "description": "Arguments to pass to the executable.", + "description": "Arguments to pass to the executable, in the same form you would type them\n(`--flag=value` or a positional value). `$VAR` references are expanded against the\nparent's environment.\n\nThe parent's environment always reaches the executable; values listed here override\nwhat it would otherwise inherit for the arguments they set.\n", "type": "array", "default": [], "items": { @@ -357,7 +357,7 @@ }, "ref": { "$ref": "#/definitions/ExecutableRef", - "description": "A reference to another executable to run in serial.\nOne of `cmd` or `ref` must be set.\n", + "description": "A reference to another executable to run in parallel.\nOne of `cmd` or `ref` must be set.\n", "default": "" }, "retries": { @@ -460,7 +460,7 @@ "$ref": "#/definitions/ExecutableArgumentList" }, "body": { - "description": "The body of the request.", + "description": "The body of the request. `$VAR` references are expanded first.\n\nIf the result is a JSON object or array, it is sent as written. Otherwise it is\nevaluated as an Expr expression that must produce a string, which lets you build\na body from values that need escaping: `'{\"prompt\":' + toJSON(env[\"PROMPT\"]) + '}'`.\n", "type": "string", "default": "" }, @@ -582,7 +582,7 @@ "type": "object", "properties": { "args": { - "description": "Arguments to pass to the executable.", + "description": "Arguments to pass to the executable, in the same form you would type them\n(`--flag=value` or a positional value). `$VAR` references are expanded against the\nparent's environment.\n\nThe parent's environment always reaches the executable; values listed here override\nwhat it would otherwise inherit for the arguments they set.\n", "type": "array", "default": [], "items": { diff --git a/tests/exec_cmd_e2e_test.go b/tests/exec_cmd_e2e_test.go index 30b32f7e..b1c71e6d 100644 --- a/tests/exec_cmd_e2e_test.go +++ b/tests/exec_cmd_e2e_test.go @@ -48,6 +48,16 @@ var _ = Describe("exec e2e", func() { Entry("request with transformation", "examples:request-with-transform"), ) + Describe("serial parent environment inheritance", func() { + It("passes the parent's args and params to a child that declares no step args", func() { + runner := utils.NewE2ECommandRunner() + stdOut := ctx.StdOut() + Expect(runner.Run(ctx.Context, "exec", "examples:serial-inherited-env", "passed-in")).To(Succeed()) + out, _ := readFileContent(stdOut) + Expect(out).To(ContainSubstring("child OUTER=[passed-in] OUTERP=[outer-param]")) + }) + }) + When("param overrides are provided", func() { It("should run the executable with the provided overrides", func() { runner := utils.NewE2ECommandRunner() diff --git a/tests/utils/builder/flowfile.go b/tests/utils/builder/flowfile.go index 49a2ec41..96e8a7b3 100644 --- a/tests/utils/builder/flowfile.go +++ b/tests/utils/builder/flowfile.go @@ -38,6 +38,8 @@ func ExamplesExecFlowFile(opts ...Option) *executable.FlowFile { ExecWithContainer(opts...), ExecWithPython(opts...), ExecWithPythonContainer(opts...), + SerialExecWithInheritedEnv(opts...), + InheritedEnvChildExec(opts...), }, } if len(opts) > 0 { diff --git a/tests/utils/builder/serial.go b/tests/utils/builder/serial.go index e5199d44..fcf7bead 100644 --- a/tests/utils/builder/serial.go +++ b/tests/utils/builder/serial.go @@ -49,3 +49,44 @@ func SerialExecWithExit(opts ...Option) *executable.Executable { } return e } + +// InheritedEnvChildExec echoes the values it resolved, so a parent's propagation can be +// asserted from the command output. It declares no step args of its own. +func InheritedEnvChildExec(opts ...Option) *executable.Executable { + pos := 1 + e := &executable.Executable{ + Verb: "run", + Name: "inherited-env-child", + Visibility: privateExecVisibility(), + Exec: &executable.ExecExecutableType{ + Args: executable.ArgumentList{{EnvKey: "OUTER", Pos: &pos, Default: "(unset)"}}, + Cmd: "echo \"child OUTER=[$OUTER] OUTERP=[$OUTERP]\"", + }, + } + if len(opts) > 0 { + vals := NewOptionValues(opts...) + e.SetContext(vals.WorkspaceName, vals.WorkspacePath, vals.NamespaceName, vals.FlowFilePath) + } + return e +} + +// SerialExecWithInheritedEnv refs a child without declaring step args, the shape in which +// a parent's args and params used to stop reaching the child entirely. +func SerialExecWithInheritedEnv(opts ...Option) *executable.Executable { + pos := 1 + e := &executable.Executable{ + Verb: "run", + Name: "serial-inherited-env", + Visibility: privateExecVisibility(), + Serial: &executable.SerialExecutableType{ + Args: executable.ArgumentList{{EnvKey: "OUTER", Pos: &pos, Default: "outer-value"}}, + Params: executable.ParameterList{{EnvKey: "OUTERP", Text: "outer-param"}}, + Execs: []executable.SerialRefConfig{{Ref: "run examples:inherited-env-child"}}, + }, + } + if len(opts) > 0 { + vals := NewOptionValues(opts...) + e.SetContext(vals.WorkspaceName, vals.WorkspacePath, vals.NamespaceName, vals.FlowFilePath) + } + return e +} diff --git a/types/executable/arguments.go b/types/executable/arguments.go index 06c1c91e..5339796b 100644 --- a/types/executable/arguments.go +++ b/types/executable/arguments.go @@ -18,6 +18,13 @@ func (a *Argument) Value() string { return a.value } +// IsSet reports whether a value was resolved for the argument, as opposed to it falling +// back to the declared default. Callers use this to tell a user-supplied literal apart +// from an author-written default that may contain $VAR references. +func (a *Argument) IsSet() bool { + return a.value != "" +} + func (a *Argument) Validate() error { if err := utils.ValidateOneOf("argument type", a.Flag, a.Pos); err != nil { return err diff --git a/types/executable/executable.gen.go b/types/executable/executable.gen.go index 6e546b7a..bd67077f 100644 --- a/types/executable/executable.gen.go +++ b/types/executable/executable.gen.go @@ -303,7 +303,15 @@ type ParallelExecutableType struct { // Configuration for a parallel executable. type ParallelRefConfig struct { - // Arguments to pass to the executable. + // Arguments to pass to the executable, in the same form you would type them + // (`--flag=value` or a positional value). `$VAR` references are expanded against + // the + // parent's environment. + // + // The parent's environment always reaches the executable; values listed here + // override + // what it would otherwise inherit for the arguments they set. + // Args []string `json:"args,omitempty" yaml:"args,omitempty" mapstructure:"args,omitempty"` // The command to execute. @@ -336,7 +344,7 @@ type ParallelRefConfig struct { // A human-readable label for this step, used for display purposes. Name string `json:"name,omitempty" yaml:"name,omitempty" mapstructure:"name,omitempty"` - // A reference to another executable to run in serial. + // A reference to another executable to run in parallel. // One of `cmd` or `ref` must be set. // Ref Ref `json:"ref,omitempty" yaml:"ref,omitempty" mapstructure:"ref,omitempty"` @@ -419,7 +427,14 @@ type RequestExecutableType struct { // Args corresponds to the JSON schema field "args". Args ArgumentList `json:"args,omitempty" yaml:"args,omitempty" mapstructure:"args,omitempty"` - // The body of the request. + // The body of the request. `$VAR` references are expanded first. + // + // If the result is a JSON object or array, it is sent as written. Otherwise it is + // evaluated as an Expr expression that must produce a string, which lets you + // build + // a body from values that need escaping: `'{"prompt":' + toJSON(env["PROMPT"]) + + // '}'`. + // Body string `json:"body,omitempty" yaml:"body,omitempty" mapstructure:"body,omitempty"` // A map of headers to include in the request. @@ -523,7 +538,15 @@ type SerialExecutableType struct { // Configuration for a serial executable. type SerialRefConfig struct { - // Arguments to pass to the executable. + // Arguments to pass to the executable, in the same form you would type them + // (`--flag=value` or a positional value). `$VAR` references are expanded against + // the + // parent's environment. + // + // The parent's environment always reaches the executable; values listed here + // override + // what it would otherwise inherit for the arguments they set. + // Args []string `json:"args,omitempty" yaml:"args,omitempty" mapstructure:"args,omitempty"` // The command to execute. diff --git a/types/executable/executable_schema.yaml b/types/executable/executable_schema.yaml index 468b9111..6e16db9d 100644 --- a/types/executable/executable_schema.yaml +++ b/types/executable/executable_schema.yaml @@ -462,7 +462,7 @@ definitions: ref: $ref: '#/definitions/Ref' description: | - A reference to another executable to run in serial. + A reference to another executable to run in parallel. One of `cmd` or `ref` must be set. default: "" if: @@ -482,7 +482,13 @@ definitions: type: array items: type: string - description: Arguments to pass to the executable. + description: | + Arguments to pass to the executable, in the same form you would type them + (`--flag=value` or a positional value). `$VAR` references are expanded against the + parent's environment. + + The parent's environment always reaches the executable; values listed here override + what it would otherwise inherit for the arguments they set. default: [] retries: type: integer @@ -583,7 +589,12 @@ definitions: default: "" body: type: string - description: The body of the request. + description: | + The body of the request. `$VAR` references are expanded first. + + If the result is a JSON object or array, it is sent as written. Otherwise it is + evaluated as an Expr expression that must produce a string, which lets you build + a body from values that need escaping: `'{"prompt":' + toJSON(env["PROMPT"]) + '}'`. default: "" headers: type: object @@ -669,7 +680,13 @@ definitions: type: array items: type: string - description: Arguments to pass to the executable. + description: | + Arguments to pass to the executable, in the same form you would type them + (`--flag=value` or a positional value). `$VAR` references are expanded against the + parent's environment. + + The parent's environment always reaches the executable; values listed here override + what it would otherwise inherit for the arguments they set. default: [] reviewRequired: type: boolean