Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/deploy-validator-allowlist-trim.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- `wheels deploy` config validation now rejects the 13 Kamal top-level keys the runtime never reads (`boot`, `healthcheck`, `hooks`, `volumes`, `labels`, `logging`, `retain_containers`, `minimum_version`, `asset_path`, `require_destination`, `allow_empty_roles`, `run_directory`, `readiness_delay`) instead of accepting-and-ignoring them; the `unknown top-level key` error now lists the allowed keys ([#3088](https://github.com/wheels-dev/wheels/issues/3088))
1 change: 1 addition & 0 deletions changelog.d/deploy-validator-host-colons.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- `wheels deploy` host validation no longer under-counts adjacent colons: unbracketed IPv6-ish hosts like `::1:22`, `:a:b`, and `a::b` are now rejected with the documented `invalid host` error, while bracketed `[::1]:22` and single-colon `user@host:port` forms remain accepted ([#3086](https://github.com/wheels-dev/wheels/issues/3086))
20 changes: 14 additions & 6 deletions cli/lucli/services/deploy/config/Validator.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@
component {

public any function init() {
// Only keys the runtime actually reads (Config.cfc accessors + the
// commands/ consumers behind them). Keys Kamal supports but this port
// doesn't implement yet (boot, logging, retain_containers, hooks, …)
// are deliberately ABSENT so they fail loudly instead of being
// accepted-and-ignored (##3088).
variables.allowedKeys = [
"service", "image", "servers", "registry", "builder", "env",
"ssh", "proxy", "boot", "healthcheck", "hooks", "accessories",
"volumes", "labels", "logging", "retain_containers",
"minimum_version", "asset_path", "require_destination",
"allow_empty_roles", "run_directory", "readiness_delay"
"ssh", "proxy", "accessories"
];
// Pre-build a case-insensitive struct lookup so the hot path doesn't
// depend on arrayContainsNoCase (not available on every engine).
Expand All @@ -34,7 +36,10 @@ component {
$requireKey(arguments.parsed, "servers", arguments.filePath);
for (var k in arguments.parsed) {
if (!structKeyExists(variables.allowedLookup, lCase(k))) {
$raise(arguments.filePath, "unknown top-level key: '#k#'");
$raise(
arguments.filePath,
"unknown top-level key: '#k#' (allowed keys: #arrayToList(variables.allowedKeys, ', ')#)"
);
}
}
// Service / role / accessory names are interpolated raw into lock
Expand Down Expand Up @@ -69,7 +74,10 @@ component {
public void function $validateHost(required string host, required string filePath) {
// A bare host or user@host is fine; user@host:port has 1 colon; IPv6
// literals must be bracketed ([::1]:22) — anything else is ambiguous.
var colonCount = arrayLen(listToArray(arguments.host, ":", false, true)) - 1;
// Count colons directly: listToArray(includeEmptyFields=false)
// collapses adjacent/leading delimiters, so '::1:22' under-counted to
// 1 colon and slipped through (##3086).
var colonCount = len(arguments.host) - len(replace(arguments.host, ":", "", "all"));
if (colonCount > 1 && left(arguments.host, 1) != "[") {
$raise(arguments.filePath, "invalid host: '#arguments.host#'");
}
Expand Down
4 changes: 2 additions & 2 deletions cli/lucli/tests/_fixtures/deploy/configs/full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,5 @@ accessories:
port: 6379
directories:
- data:/data

readiness_delay: 0
# readiness_delay: 0 removed: the key has no runtime reader and is rejected by the
# trimmed allowlist (#3088) - unimplemented Kamal keys now fail validation loudly.
82 changes: 82 additions & 0 deletions cli/lucli/tests/specs/deploy/config/ValidatorSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,88 @@ component extends="wheels.wheelstest.system.BaseSpec" {
}
expect(state.threw).toBeTrue();
});

// ##3086 — adjacent colons must not be under-counted. listToArray with
// includeEmptyFields=false collapsed '::1:22' to ['1','22'] and let it pass.
it("rejects unbracketed multi-colon hosts, including adjacent colons", () => {
var v = new cli.lucli.services.deploy.config.Validator();
var rejects = ["::1:22", ":a:b", "a::b", "h:1:2"];
for (var host in rejects) {
var state = {threw: false, message: ""};
try {
v.validate({service: "demo", image: "a/b", servers: [host]}, "test.yml");
} catch (DeployConfigError e) {
state.threw = true;
state.message = e.message;
}
expect(state.threw).toBeTrue("expected '#host#' to be rejected");
expect(state.message).toInclude("invalid host");
}
});

it("still accepts bracketed IPv6 hosts and single-colon host:port", () => {
var v = new cli.lucli.services.deploy.config.Validator();
v.validate({service: "demo", image: "a/b", servers: ["[::1]:22"]}, "test.yml");
v.validate({service: "demo", image: "a/b", servers: ["deploy@1.2.3.4:2222"]}, "test.yml");
v.validate({service: "demo", image: "a/b", servers: ["host.example.com"]}, "test.yml");
expect(true).toBeTrue();
});

// ##3088 — keys the runtime never reads must fail validation loudly
// instead of being accepted-and-ignored.
it("rejects allowlisted-but-unimplemented top-level keys", () => {
var v = new cli.lucli.services.deploy.config.Validator();
var deadKeys = [
"boot", "healthcheck", "hooks", "volumes", "labels", "logging",
"retain_containers", "minimum_version", "asset_path",
"require_destination", "allow_empty_roles", "run_directory",
"readiness_delay"
];
for (var deadKey in deadKeys) {
var parsed = {service: "demo", image: "a/b", servers: ["1.2.3.4"]};
parsed[deadKey] = "x";
var state = {threw: false, message: ""};
try {
v.validate(parsed, "test.yml");
} catch (DeployConfigError e) {
state.threw = true;
state.message = e.message;
}
expect(state.threw).toBeTrue("expected top-level key '#deadKey#' to be rejected");
expect(state.message).toInclude("unknown top-level key");
expect(state.message).toInclude(deadKey);
}
});

it("accepts a config that uses every implemented top-level key", () => {
var v = new cli.lucli.services.deploy.config.Validator();
v.validate({
service: "demo",
image: "a/b",
servers: {web: ["1.2.3.4"]},
registry: {username: "u", password: ["KAMAL_REGISTRY_PASSWORD"]},
builder: {context: ".", dockerfile: "Dockerfile"},
env: {clear: {A: "1"}},
ssh: {user: "deploy"},
proxy: {host: "app.example.com"},
accessories: {db: {image: "postgres:16"}}
}, "test.yml");
expect(true).toBeTrue();
});

it("lists the allowed keys in the unknown-key error", () => {
var v = new cli.lucli.services.deploy.config.Validator();
var state = {threw: false, message: ""};
try {
v.validate({service: "demo", image: "a/b", servers: ["1.2.3.4"], boot: {limit: 1}}, "test.yml");
} catch (DeployConfigError e) {
state.threw = true;
state.message = e.message;
}
expect(state.threw).toBeTrue();
expect(state.message).toInclude("allowed keys:");
expect(state.message).toInclude("accessories");
});
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,14 @@ Three keys are required. Omitting any of them stops the loader with `<path to de

## All allowed top-level keys

The loader rejects any top-level key not on this list. Case-insensitive.
The loader rejects any top-level key not on this list. Case-insensitive. The allowlist contains only keys the runtime actually reads — Kamal keys that `wheels deploy` has not implemented yet are **rejected** rather than accepted-and-ignored (see [Rejected Kamal keys](#rejected-kamal-keys) below).

```text title="Allowed top-level keys (illustrative — do not type)"
service, image, servers, registry, builder, env, ssh, proxy, boot,
healthcheck, hooks, accessories, volumes, labels, logging,
retain_containers, minimum_version, asset_path, require_destination,
allow_empty_roles, run_directory, readiness_delay
service, image, servers, registry, builder, env, ssh, proxy, accessories
```

<Aside type="caution" title="Allowlisted ≠ implemented">
The validator accepts every key above, but a number of them are parsed and then **never read by the runtime**: `boot`, `logging`, `retain_containers`, `minimum_version`, `asset_path`, `require_destination`, `allow_empty_roles`, `run_directory`, `readiness_delay`, `hooks.path`, plus several sub-keys under `builder`, `ssh`, `proxy`, and role maps. Setting them has no effect today. Each one is flagged in its section below; the umbrella issue is [#3088](https://github.com/wheels-dev/wheels/issues/3088).
<Aside type="caution" title="Some sub-keys are still parsed but ignored">
Within the allowed blocks, several **sub-keys** are parsed and then never read by the runtime: `builder.arch` / `builder.args` / `builder.remote`, `ssh.proxy` / `ssh.keys_only`, `proxy.forward_headers` / `proxy.buffering`, role-map `options:` / `labels:` / `env:`, and accessory `files:`. Setting them has no effect today. Each one is flagged in its section below; the umbrella issue is [#3088](https://github.com/wheels-dev/wheels/issues/3088).
</Aside>

## `service`
Expand Down Expand Up @@ -110,7 +107,7 @@ servers:
- `options:` — parsed but **currently ignored**: no extra `docker run` flags (`memory`, `cpus`, etc.) are emitted ([#3088](https://github.com/wheels-dev/wheels/issues/3088)).
- `labels:` — parsed but **currently ignored**: containers only get the four labels Wheels adds (`service`, `role`, `destination`, `version`) ([#3088](https://github.com/wheels-dev/wheels/issues/3088)).

Host strings accept `user@host`, `host:port`, and `user@host:port`. IPv6 literals must be bracketed (`[::1]:22`). Multiple colons without brackets are meant to be rejected, but the validator currently under-counts adjacent colons, so malformed strings like `'::1:22'` slip through ([#3086](https://github.com/wheels-dev/wheels/issues/3086)) — always bracket IPv6 literals yourself.
Host strings accept `user@host`, `host:port`, and `user@host:port`. IPv6 literals must be bracketed (`[::1]:22`). Multiple colons without brackets are rejected — including adjacent colons, so an unbracketed `'::1:22'` fails validation.

## `registry`

Expand Down Expand Up @@ -251,88 +248,28 @@ Two caveats on the example shapes: `files:` is accepted but **never uploaded**

See [Accessories](/v4-0-0/deployment/accessories/) for the full walk-through.

## `volumes`, `labels`

Top-level defaults that every role inherits unless overridden.

```yaml title="config/deploy.yml (illustrative — do not type)"
volumes:
- ./storage:/rails/storage

labels:
environment: production
```

Role-level `volumes` and `labels` merge over these.

## `hooks`

Path override for the `.kamal/hooks/` directory. **Currently ignored**: the key passes validation but is never read — hook scripts are always loaded from `.kamal/hooks/` ([#3088](https://github.com/wheels-dev/wheels/issues/3088)).

```yaml title="config/deploy.yml (illustrative — do not type)"
hooks:
path: .kamal/hooks
```

See [Hooks](/v4-0-0/deployment/hooks/) for what the scripts look like.

## `healthcheck`

Legacy top-level healthcheck config (pre-kamal-proxy). New deploys should configure health checks under `proxy.healthcheck:` instead. Accepted for back-compat with Kamal's pre-2.0 config shape.

## `boot`

In Kamal, controls the rolling boot sequence (`limit:` for parallelism, `wait:` for pauses between hosts). **Currently ignored**: `wheels deploy` never reads `boot.limit` or `boot.wait` — it always deploys hosts one at a time, sequentially, with no pause ([#3088](https://github.com/wheels-dev/wheels/issues/3088); the rolling-boot orchestration work is tracked under [#2957](https://github.com/wheels-dev/wheels/issues/2957)).

```yaml title="config/deploy.yml (accepted but currently ignored)"
boot:
limit: 25%
wait: 2
```

## `logging`

In Kamal, the Docker logging driver and options, passed through as `docker run --log-driver` / `--log-opt`. **Currently ignored**: no logging flags are ever emitted ([#3088](https://github.com/wheels-dev/wheels/issues/3088)).

```yaml title="config/deploy.yml (accepted but currently ignored)"
logging:
driver: json-file
options:
max-size: 10m
max-file: "3"
```

## `retain_containers`

In Kamal, how many old containers to keep on each host for rollback. **The config key is currently ignored** ([#3088](https://github.com/wheels-dev/wheels/issues/3088)): `wheels deploy prune all` always keeps the 5 most recent containers unless you pass `--keep=<n>` on the command line.

```yaml title="config/deploy.yml (accepted but currently ignored — use prune --keep=<n> instead)"
retain_containers: 10
```

## `minimum_version`

In Kamal, requires the CLI to be at least this version. **Currently ignored**: no version gate runs, even with an impossible value like `"99.0.0"` ([#3088](https://github.com/wheels-dev/wheels/issues/3088)).

## `asset_path`

In Kamal, the in-container path used for zero-downtime JS/CSS asset rotation. **Currently ignored** ([#3088](https://github.com/wheels-dev/wheels/issues/3088)).

## `require_destination`

In Kamal, forces every command to pass `--destination=<name>`. **Currently ignored** — commands run fine without a destination regardless of this key ([#3088](https://github.com/wheels-dev/wheels/issues/3088)).

## `allow_empty_roles`

In Kamal, lets a role with an empty `hosts:` list pass validation. **Currently ignored** ([#3088](https://github.com/wheels-dev/wheels/issues/3088)).

## `run_directory`

In Kamal, the working directory inside the container. **Currently ignored** — the image's configured `WORKDIR` is always used ([#3088](https://github.com/wheels-dev/wheels/issues/3088)).

## `readiness_delay`

In Kamal, seconds to wait after `docker run` before asking the proxy to cut over. **Currently ignored** ([#3088](https://github.com/wheels-dev/wheels/issues/3088)).
## Rejected Kamal keys

Kamal accepts a number of additional top-level keys that `wheels deploy` has not implemented. They used to pass validation while doing nothing; the validator now **rejects** them with `unknown top-level key: '<name>'` so a config that looks like it works but doesn't fails loudly ([#3088](https://github.com/wheels-dev/wheels/issues/3088)).

| Key | What it does in Kamal | Status in `wheels deploy` |
|-----|-----------------------|---------------------------|
| `boot` | Rolling boot sequence (`limit:` parallelism, `wait:` pauses) | Rejected — hosts always deploy one at a time, sequentially (orchestration work tracked under [#2957](https://github.com/wheels-dev/wheels/issues/2957)) |
| `healthcheck` | Legacy pre-kamal-proxy top-level health check | Rejected — configure [`proxy.healthcheck:`](#proxy) instead |
| `hooks` | Path override for the hooks directory | Rejected — hook scripts are always loaded from `.kamal/hooks/` (see [Hooks](/v4-0-0/deployment/hooks/); the feature works, only the path override key does not exist) |
| `volumes`, `labels` | App-container volume mounts / extra labels | Rejected — containers only get the four labels Wheels adds (`service`, `role`, `destination`, `version`); only accessory `volumes:`/`directories:` are mounted |
| `logging` | `docker run --log-driver` / `--log-opt` passthrough | Rejected — no logging flags are ever emitted |
| `retain_containers` | Old-container keep count for rollback | Rejected — `wheels deploy prune all` always keeps the 5 most recent unless you pass `--keep=<n>` on the command line |
| `minimum_version` | Fail-fast CLI version gate | Rejected — no version gate runs |
| `asset_path` | Zero-downtime JS/CSS asset rotation path | Rejected |
| `require_destination` | Forces every command to pass `--destination=<name>` | Rejected |
| `allow_empty_roles` | Lets a role with an empty `hosts:` list pass validation | Rejected |
| `run_directory` | Working directory inside the container | Rejected — the image's configured `WORKDIR` is always used |
| `readiness_delay` | Seconds to wait after `docker run` before proxy cutover | Rejected |

<Aside type="note" title="Migrating a Kamal config">
If you're porting an existing Kamal `deploy.yml`, delete any of the keys above (or move `healthcheck` under `proxy:`). See [Migrating from Kamal](/v4-0-0/deployment/migrating-from-kamal/).
</Aside>

## Variable interpolation

Expand Down Expand Up @@ -388,6 +325,6 @@ One caveat: the `wheels deploy config` inspection verb currently ignores `--dest
Errors are emitted as `<absolute path to deploy.yml>: <message>` — the prefix is the full file path, and messages are never line-scoped:

- `missing required key: '<name>'` — one of `service`, `image`, `servers` is absent.
- `unknown top-level key: '<name>'` — typo or unsupported key. Cross-check against the allowlist above.
- `invalid host: '<string>'` — host string has more than one colon without IPv6 brackets. (Known gap: adjacent colons are under-counted, so strings like `'::1:22'` currently pass — [#3086](https://github.com/wheels-dev/wheels/issues/3086).)
- `unknown top-level key: '<name>' (allowed keys: …)` — typo, unsupported Kamal key, or a key from the [rejected list](#rejected-kamal-keys). The message includes the full allowlist.
- `invalid host: '<string>'` — host string has more than one colon without IPv6 brackets (adjacent colons count, so unbracketed `'::1:22'` is rejected).
- `invalid <kind> name: '<name>' (must match [a-zA-Z0-9][a-zA-Z0-9_.-]*)` — a service, role, or accessory name contains shell-unsafe characters. Develop builds only (added after 4.0.3 by [#3008](https://github.com/wheels-dev/wheels/pull/3008) for [#2956](https://github.com/wheels-dev/wheels/issues/2956)).
Loading