From f3c59cb0d15482661c7bc25bf4f964d9d6058669 Mon Sep 17 00:00:00 2001 From: Atif Ali Date: Thu, 22 Oct 2026 15:01:38 +0000 Subject: [PATCH 1/4] docs(agents): standardize additional_args for extra CLI flags Document the convention for modules that pass extra flags to a CLI: name the input additional_args, make it a map(string) of flag to value passed as written, validate keys and reject flags the module already sets (kept in sync with the script by comments), pass it base64 one argument per line, and deprecate rather than remove older names. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R2TMgCPW8SL8kUCB28L4dt --- AGENTS.md | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 4796156dc..429a479d7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -128,6 +128,40 @@ variable "api_key" { - Every `output` block must have a `description`. - Use `count = condition ? 1 : 0` for optional singleton resources. Reserve `for_each` for maps/sets where resource identity matters. +### Extra CLI flags: `additional_args` + +When a module lets users pass extra flags to a CLI it runs, name the input `additional_args` and make it a `map(string)` of flag to value. Do not use `extra_args`, `additional_arguments`, `extra_flags`, or a free-form string. + +- Each key is passed exactly as written, so the user picks `--long` or `-s`. A non-empty value follows as a separate argument; an empty value passes the flag alone. `{ "--depth" = "1", "--recurse-submodules" = "" }` becomes `--depth 1 --recurse-submodules`. +- Terraform orders map keys alphabetically, and a key can appear only once. +- Validate that every key looks like a flag and no value contains a newline. +- Add a second validation that rejects every flag the module already passes, with a comment beside both the list and the command in the script to keep them in sync. +- Pass the rendered list to the script as `base64encode(join("\n", local.additional_args))`, and read it back one argument per line into an array or `set --`. Never word-split or `eval` it. If the arguments are written into another generated script, quote each one with `printf '%q'`. +- If a module already has an input with another name for this, keep it working and add `additional_args` beside it. Mark the old input `Deprecated: use additional_args instead.` in its description, and append its arguments after `additional_args`. (`code-server` and `ttyd` predate this rule: their `additional_args` is a string, and changing its type would break existing templates.) + +```tf +variable "additional_args" { + description = "Additional flags to pass to `tool`, as a map of flag to value. Each flag is passed as written followed by its value as the next argument; an empty value passes the flag alone." + type = map(string) + default = {} + + validation { + condition = alltrue([for k, v in var.additional_args : can(regex("^--?[A-Za-z0-9][A-Za-z0-9._-]*$", k)) && !can(regex("\n", v))]) + error_message = "additional_args keys must be flags such as --name or -n, and values must not contain newlines." + } + + # The module already sets these. Keep this list in sync with the flags in run.sh. + validation { + condition = length(setintersection(keys(var.additional_args), ["--port"])) == 0 + error_message = "additional_args must not set --port; use port instead." + } +} + +locals { + additional_args = flatten([for k, v in var.additional_args : v == "" ? [k] : [k, v]]) +} +``` + ### `.tftest.hcl` test commands - Use `command = plan` only for assertions on **input-derived values** (variables, locals computed from inputs). From da2baa88d8711646a4a0859855031a7a66c3d09c Mon Sep 17 00:00:00 2001 From: Atif Ali Date: Thu, 22 Oct 2026 15:08:15 +0000 Subject: [PATCH 2/4] docs(agents): add a rule for deprecating inputs On a major bump, require Terraform >= 1.15 and use the native `deprecated` argument with a message naming the replacement; on a minor or patch bump, deprecate by description only, since Terraform 1.14 and older (bundled with Coder v2.36 and earlier) reject the argument. Point the additional_args rule at it and add both to the PR review checklist. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R2TMgCPW8SL8kUCB28L4dt --- AGENTS.md | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 429a479d7..a4ae33554 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -137,7 +137,7 @@ When a module lets users pass extra flags to a CLI it runs, name the input `addi - Validate that every key looks like a flag and no value contains a newline. - Add a second validation that rejects every flag the module already passes, with a comment beside both the list and the command in the script to keep them in sync. - Pass the rendered list to the script as `base64encode(join("\n", local.additional_args))`, and read it back one argument per line into an array or `set --`. Never word-split or `eval` it. If the arguments are written into another generated script, quote each one with `printf '%q'`. -- If a module already has an input with another name for this, keep it working and add `additional_args` beside it. Mark the old input `Deprecated: use additional_args instead.` in its description, and append its arguments after `additional_args`. (`code-server` and `ttyd` predate this rule: their `additional_args` is a string, and changing its type would break existing templates.) +- If a module already has an input with another name for this, keep it working and add `additional_args` beside it. Deprecate the old input as described in [Deprecating inputs](#deprecating-inputs), and append its arguments after `additional_args`. (`code-server` and `ttyd` predate this rule: their `additional_args` is a string, and changing its type would break existing templates.) ```tf variable "additional_args" { @@ -162,6 +162,29 @@ locals { } ``` +### Deprecating inputs + +Terraform 1.15 added a `deprecated` argument for `variable` blocks: Terraform warns whenever a caller sets the input. Terraform 1.14 and older reject the argument outright (`An argument named "deprecated" is not expected here`), even when the caller never sets the input. Coder bundles Terraform 1.15 from v2.37.0; v2.36 and older bundle 1.14. + +- **Major version bump:** set `required_version = ">= 1.15"` in the `terraform` block, and give every deprecated input a `deprecated` argument, including inputs deprecated earlier by description only. Raising the minimum Terraform version is itself a breaking change, so do it only in a major bump, and note it in the PR and README. +- **Minor or patch bump:** do not add `deprecated` or raise `required_version`. Begin the input's description with `Deprecated: use instead.` and say so in the README. +- In both cases, keep a deprecated input working until a major version removes it. + +`deprecated` takes the warning message, not a boolean: name the replacement input in it (`"Use additional_args instead."`). `deprecated = true` prints the bare message `true`, and `""` prints none. Place `deprecated` after `default`: + +```tf +terraform { + required_version = ">= 1.15" +} + +variable "extra_args" { + description = "Deprecated: use `additional_args` instead." + type = list(string) + default = [] + deprecated = "Use additional_args instead." +} +``` + ### `.tftest.hcl` test commands - Use `command = plan` only for assertions on **input-derived values** (variables, locals computed from inputs). @@ -170,7 +193,8 @@ locals { ## PR Review Checklist - Version bumped via `.github/scripts/version-bump.sh` if module changed (patch=bugfix, minor=feature, major=breaking) -- Breaking changes documented: removed inputs, changed defaults, new required variables +- Breaking changes documented: removed inputs, changed defaults, new required variables, raised minimum Terraform version +- Deprecated inputs use the `deprecated` argument and `required_version = ">= 1.15"` on a major bump, or a `Deprecated:` description on a minor or patch bump - New variables have sensible defaults to maintain backward compatibility - Tests pass (`bun run check:terraform`, `bun run check:typescript`, or `bun check` for everything); add diagnostic logging for test failures - README examples updated with new version number; tooltip/behavior changes noted From 2064aaef3678e52351bc02e6f63cd09d0727c53b Mon Sep 17 00:00:00 2001 From: Atif Ali Date: Fri, 23 Oct 2026 03:06:44 +0000 Subject: [PATCH 3/4] docs(agents): make additional_args a list of strings Per review: a map cannot repeat a flag, and Terraform silently keeps only the last value of a repeated key. The rule now uses list(string), one element per argument, passed in order, with reserved flags checked after stripping any =value. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R2TMgCPW8SL8kUCB28L4dt --- AGENTS.md | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a4ae33554..79c72d5e9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -128,38 +128,33 @@ variable "api_key" { - Every `output` block must have a `description`. - Use `count = condition ? 1 : 0` for optional singleton resources. Reserve `for_each` for maps/sets where resource identity matters. -### Extra CLI flags: `additional_args` +### Extra CLI arguments: `additional_args` -When a module lets users pass extra flags to a CLI it runs, name the input `additional_args` and make it a `map(string)` of flag to value. Do not use `extra_args`, `additional_arguments`, `extra_flags`, or a free-form string. +When a module lets users pass extra arguments to a CLI it runs, name the input `additional_args` and make it a `list(string)` with one element per argument. Do not use `extra_args`, `additional_arguments`, `extra_flags`, a map, or a free-form string. -- Each key is passed exactly as written, so the user picks `--long` or `-s`. A non-empty value follows as a separate argument; an empty value passes the flag alone. `{ "--depth" = "1", "--recurse-submodules" = "" }` becomes `--depth 1 --recurse-submodules`. -- Terraform orders map keys alphabetically, and a key can appear only once. -- Validate that every key looks like a flag and no value contains a newline. -- Add a second validation that rejects every flag the module already passes, with a comment beside both the list and the command in the script to keep them in sync. -- Pass the rendered list to the script as `base64encode(join("\n", local.additional_args))`, and read it back one argument per line into an array or `set --`. Never word-split or `eval` it. If the arguments are written into another generated script, quote each one with `printf '%q'`. +- Pass each element exactly as written and in order, as one argument. The user writes the command line as they would type it: `["--depth", "1", "--recurse-submodules"]`, or `["--depth=1"]` where the CLI accepts it. A flag can repeat (`["--header", "a", "--header", "b"]`), which a map cannot do: Terraform silently keeps only the last value of a repeated map key. +- Validate that no element is empty or contains a newline. +- Add a second validation that rejects every flag the module already passes, checking each element with any `=value` stripped so `--port=1` is caught too. Put a comment beside both the list and the command in the script to keep them in sync. +- Pass the list to the script as `base64encode(join("\n", var.additional_args))`, and read it back one argument per line into an array or `set --`. Never word-split or `eval` it. If the arguments are written into another generated script, quote each one with `printf '%q'`. - If a module already has an input with another name for this, keep it working and add `additional_args` beside it. Deprecate the old input as described in [Deprecating inputs](#deprecating-inputs), and append its arguments after `additional_args`. (`code-server` and `ttyd` predate this rule: their `additional_args` is a string, and changing its type would break existing templates.) ```tf variable "additional_args" { - description = "Additional flags to pass to `tool`, as a map of flag to value. Each flag is passed as written followed by its value as the next argument; an empty value passes the flag alone." - type = map(string) - default = {} + description = "Additional arguments to pass to `tool`, one list element per argument, in order. Each element is passed as written, so a flag can repeat and a value may contain spaces." + type = list(string) + default = [] validation { - condition = alltrue([for k, v in var.additional_args : can(regex("^--?[A-Za-z0-9][A-Za-z0-9._-]*$", k)) && !can(regex("\n", v))]) - error_message = "additional_args keys must be flags such as --name or -n, and values must not contain newlines." + condition = alltrue([for a in var.additional_args : a != "" && !can(regex("\n", a))]) + error_message = "additional_args elements must be non-empty and must not contain newlines." } # The module already sets these. Keep this list in sync with the flags in run.sh. validation { - condition = length(setintersection(keys(var.additional_args), ["--port"])) == 0 + condition = !anytrue([for a in var.additional_args : contains(["--port"], split("=", a)[0])]) error_message = "additional_args must not set --port; use port instead." } } - -locals { - additional_args = flatten([for k, v in var.additional_args : v == "" ? [k] : [k, v]]) -} ``` ### Deprecating inputs From c99ffa41a9396bbae019a77540d634c1881d8a5e Mon Sep 17 00:00:00 2001 From: Atif Ali Date: Fri, 23 Oct 2026 12:15:47 +0000 Subject: [PATCH 4/4] docs(agents): fix inconsistencies in the additional_args and deprecation rules - The coder-utils encoding table said lists go as base64 JSON; add a row for CLI argument lists, which use newline-joined base64 so scripts need no jq, and say why in the rule. - Add `deprecated` to the variable block order. - Only raise required_version on a major bump when the module has deprecated inputs. - Note that the reserved-flag check is best effort. - Note that output blocks accept `deprecated` too (Terraform 1.15+). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01R2TMgCPW8SL8kUCB28L4dt --- AGENTS.md | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 79c72d5e9..0401bf405 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,12 +59,13 @@ Store each script as a `.tftpl` file under `scripts/`. Render it at **plan time* **Encoding rules for template variables:** -| Value type | Terraform side | Template (`.tftpl`) side | -| ------------------------------------- | ----------------------------------- | ---------------------------------------------- | -| String / path | pass as-is | `ARG_FOO='${ARG_FOO}'` | -| Boolean | `tostring(var.foo)` | `ARG_FOO='${ARG_FOO}'` | -| Free-form string (may contain quotes) | `base64encode(var.foo)` | `ARG_FOO=$(echo -n '${ARG_FOO}' \| base64 -d)` | -| Object / list (JSON) | `base64encode(jsonencode(var.foo))` | `ARG_FOO=$(echo -n '${ARG_FOO}' \| base64 -d)` | +| Value type | Terraform side | Template (`.tftpl`) side | +| ------------------------------------- | ----------------------------------- | -------------------------------------------------------------------------------------- | +| String / path | pass as-is | `ARG_FOO='${ARG_FOO}'` | +| Boolean | `tostring(var.foo)` | `ARG_FOO='${ARG_FOO}'` | +| Free-form string (may contain quotes) | `base64encode(var.foo)` | `ARG_FOO=$(echo -n '${ARG_FOO}' \| base64 -d)` | +| Object / list (JSON) | `base64encode(jsonencode(var.foo))` | `ARG_FOO=$(echo -n '${ARG_FOO}' \| base64 -d)` | +| CLI argument list (`additional_args`) | `base64encode(join("\n", var.foo))` | one argument per line; see [Extra CLI arguments](#extra-cli-arguments-additional_args) | In `.tftpl` files, write literal bash `$` as `$$` (e.g., `$${HOME}`) so Terraform does not treat them as template interpolations. @@ -113,7 +114,7 @@ output "scripts" { ### Variable and output conventions -Order variable blocks: `description` → `type` → `default` → `validation` → `sensitive`. +Order variable blocks: `description` → `type` → `default` → `deprecated` → `validation` → `sensitive`. ```tf variable "api_key" { @@ -134,8 +135,8 @@ When a module lets users pass extra arguments to a CLI it runs, name the input ` - Pass each element exactly as written and in order, as one argument. The user writes the command line as they would type it: `["--depth", "1", "--recurse-submodules"]`, or `["--depth=1"]` where the CLI accepts it. A flag can repeat (`["--header", "a", "--header", "b"]`), which a map cannot do: Terraform silently keeps only the last value of a repeated map key. - Validate that no element is empty or contains a newline. -- Add a second validation that rejects every flag the module already passes, checking each element with any `=value` stripped so `--port=1` is caught too. Put a comment beside both the list and the command in the script to keep them in sync. -- Pass the list to the script as `base64encode(join("\n", var.additional_args))`, and read it back one argument per line into an array or `set --`. Never word-split or `eval` it. If the arguments are written into another generated script, quote each one with `printf '%q'`. +- Add a second validation that rejects every flag the module already passes, checking each element with any `=value` stripped so `--port=1` is caught too. Put a comment beside both the list and the command in the script to keep them in sync. The check is best effort: it misses a short flag with its value attached (`-p8080`) and also rejects a value that happens to equal a reserved flag. +- Pass the list to the script as `base64encode(join("\n", var.additional_args))`, not as JSON, so the script needs no `jq` and works in plain `sh`. Read it back one argument per line into an array or `set --`. Never word-split or `eval` it. If the arguments are written into another generated script, quote each one with `printf '%q'`. - If a module already has an input with another name for this, keep it working and add `additional_args` beside it. Deprecate the old input as described in [Deprecating inputs](#deprecating-inputs), and append its arguments after `additional_args`. (`code-server` and `ttyd` predate this rule: their `additional_args` is a string, and changing its type would break existing templates.) ```tf @@ -159,9 +160,9 @@ variable "additional_args" { ### Deprecating inputs -Terraform 1.15 added a `deprecated` argument for `variable` blocks: Terraform warns whenever a caller sets the input. Terraform 1.14 and older reject the argument outright (`An argument named "deprecated" is not expected here`), even when the caller never sets the input. Coder bundles Terraform 1.15 from v2.37.0; v2.36 and older bundle 1.14. +Terraform 1.15 added a `deprecated` argument for `variable` blocks, which warns whenever a caller sets the input, and for `output` blocks, which warns whenever a caller reads the output. The rules below apply to deprecated outputs too. Terraform 1.14 and older reject the argument outright (`An argument named "deprecated" is not expected here`), even when the caller never sets the input. Coder bundles Terraform 1.15 from v2.37.0; v2.36 and older bundle 1.14. -- **Major version bump:** set `required_version = ">= 1.15"` in the `terraform` block, and give every deprecated input a `deprecated` argument, including inputs deprecated earlier by description only. Raising the minimum Terraform version is itself a breaking change, so do it only in a major bump, and note it in the PR and README. +- **Major version bump of a module that has deprecated inputs:** set `required_version = ">= 1.15"` in the `terraform` block, and give every deprecated input a `deprecated` argument, including inputs deprecated earlier by description only. A major bump with no deprecated inputs leaves `required_version` alone. Raising the minimum Terraform version is itself a breaking change, so do it only in a major bump, and note it in the PR and README. - **Minor or patch bump:** do not add `deprecated` or raise `required_version`. Begin the input's description with `Deprecated: use instead.` and say so in the README. - In both cases, keep a deprecated input working until a major version removes it. @@ -189,7 +190,7 @@ variable "extra_args" { - Version bumped via `.github/scripts/version-bump.sh` if module changed (patch=bugfix, minor=feature, major=breaking) - Breaking changes documented: removed inputs, changed defaults, new required variables, raised minimum Terraform version -- Deprecated inputs use the `deprecated` argument and `required_version = ">= 1.15"` on a major bump, or a `Deprecated:` description on a minor or patch bump +- Deprecated inputs keep a `Deprecated:` description; on a major bump they also get the `deprecated` argument and `required_version = ">= 1.15"` - New variables have sensible defaults to maintain backward compatibility - Tests pass (`bun run check:terraform`, `bun run check:typescript`, or `bun check` for everything); add diagnostic logging for test failures - README examples updated with new version number; tooltip/behavior changes noted