From b4fd2285fd393d9044e799e1abfb3d5c563479b1 Mon Sep 17 00:00:00 2001 From: Ben Grewell Date: Sat, 8 Aug 2026 14:00:19 +0000 Subject: [PATCH] Resolve local paths against the suite file and widen --check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five behaviours the documentation had to apologize for, fixed instead. Local paths now follow one convention. Absolute paths are used as-is, `~` expands to the invoking user's home, and everything else resolves against the directory holding the suite file rather than the process working directory. That covers file-step sources and destinations, docker volumes, LXD disk sources, SSH keys and known_hosts, LXD certificates, and compose_file, joining dockerfile and !!load_from which already worked this way. A suite now behaves the same run from the repository root, from its own directory, or from a CI checkout elsewhere. --check gained the validation it was assumed to have: - Required fields: host on ssh, image on docker, compose_file on docker-compose. Previously an ssh node missing host passed the check and failed the run dialling :22. - Unknown option names, reported with the accepted set. Options decode through a JSON round-trip that discards unrecognised keys, so `privilaged` for `privileged` left the option at its default while the suite read as though it were set. - Cross-node constraints — duplicate names and more than one local node — which lived in the node factory and so never ran under --check. Two options that were missing: - docker `command` and `entrypoint` override the image's CMD and ENTRYPOINT, so a bare distribution image can host a node instead of exiting at once. - docker `container_name` and lxd `instance_name` decouple the platform identifier from node identity, defaulting to the node name. The node name stays what `node:` references and what reports show. --- README.md | 29 +++-- cmd/dart/main.go | 12 ++ docs/node-types.md | 148 ++++++++++++--------- docs/steps.md | 24 ++-- internal/config/config.go | 38 +++++- internal/config/paths.go | 48 +++++++ internal/config/paths_test.go | 88 +++++++++++++ internal/docker/containers.go | 18 +++ internal/docker/wrapper.go | 8 +- pkg/nodetypes/base.go | 186 +++++++++++++++++++++++---- pkg/nodetypes/docker.go | 81 +++++++----- pkg/nodetypes/docker_compose.go | 6 +- pkg/nodetypes/docker_options_test.go | 32 ++++- pkg/nodetypes/lxd.go | 119 ++++++++++------- pkg/nodetypes/lxd_devices_test.go | 8 +- pkg/nodetypes/lxd_test.go | 4 +- pkg/nodetypes/lxd_token_test.go | 2 +- pkg/nodetypes/naming_test.go | 96 ++++++++++++++ pkg/nodetypes/ssh.go | 20 ++- pkg/nodetypes/ssh_test.go | 26 ++-- pkg/steptypes/file_transfer.go | 20 +++ 21 files changed, 793 insertions(+), 220 deletions(-) create mode 100644 internal/config/paths.go create mode 100644 internal/config/paths_test.go create mode 100644 pkg/nodetypes/naming_test.go diff --git a/README.md b/README.md index ea36d70..0113249 100644 --- a/README.md +++ b/README.md @@ -195,12 +195,13 @@ unreachable endpoint. A minimal image that ships none of them is a case for Warning: a docker node's image must run a long-lived foreground process. DART creates the container from the image's own `CMD`/`ENTRYPOINT` with no -TTY and no attached stdin, and there is no `command:` option, so an image -whose default command is an interactive shell — `ubuntu`, `debian`, -`alpine` — exits the moment it starts. Node setup then polls for up to two -minutes waiting for the container to report running and fails with -`timeout waiting for container ... to become ready`. Purpose-built service -images work; bare distribution images belong on an `lxd` (or `lxd-vm`) or `ssh` +TTY and no attached stdin, so an image whose default command is an +interactive shell — `ubuntu`, `debian`, `alpine` — exits the moment it +starts. Node setup then polls for up to two minutes waiting for the +container to report running and fails with +`timeout waiting for container ... to become ready`. Give such an image a +`command:` that stays up (`command: ["sleep", "infinity"]`), use a +purpose-built service image, or put it on an `lxd` (or `lxd-vm`) or `ssh` node instead. ### Does my config deploy correctly? @@ -242,13 +243,13 @@ Both `create_dir` and `overwrite` default to false: without them the step fails when the parent directory is missing, and fails again when the destination already exists. -Note: local paths in file steps — `source` on `file_push` and -`file_template`, and `dest` on `file_fetch` — resolve against the directory -DART is invoked from, not the directory holding the suite file. Absolute -paths, or paths written relative to the runner's working directory, are the -safe form. Platform paths do not share this rule: `docker.images[].dockerfile` -and the `!!load_from` directive resolve relative to the suite file, so a -suite that mixes both cannot use one convention throughout. +Note: every local path a suite writes follows one rule — absolute paths are +used as-is, `~` is the invoking user's home directory, and anything else is +relative to the directory holding the suite file. That covers file-step +sources and destinations, docker `volumes`, LXD disk `source`s, SSH keys and +`known_hosts`, LXD certificates, `compose_file`, `docker.images[].dockerfile`, +and `!!load_from`. A suite is therefore portable: it behaves the same run from +the repository root, from its own directory, or from a CI checkout elsewhere. ### Is the package installable on a clean machine? @@ -266,7 +267,7 @@ setup: node: clean step: type: file_push - # source is read on the machine running DART, relative to its working directory + # source is read on the machine running DART, relative to the suite file options: { source: dist/myservice.deb, dest: /tmp/myservice.deb } - name: install it node: clean diff --git a/cmd/dart/main.go b/cmd/dart/main.go index d829767..f540faf 100644 --- a/cmd/dart/main.go +++ b/cmd/dart/main.go @@ -430,6 +430,18 @@ func runCheck(cfgPath, reportValue, varsValue, onlyValue, skipValue string) int return 1 } + // Constraints across the whole node list — duplicate names, more than + // one local node — are the same ones a real run enforces + if err := nodetypes.ValidateNodeSet(cfg.Nodes); err != nil { + var cfgErr *config.ConfigError + if errors.As(err, &cfgErr) { + fmt.Fprint(os.Stderr, config.RenderConfigError(cfgErr)) + } else { + fmt.Fprintf(os.Stderr, "\n%s %s\n\n", errorStyle.Sprint("Error:"), err) + } + return 1 + } + mocks := make(map[string]ifaces.Node, len(cfg.Nodes)) for _, node := range cfg.Nodes { // Unknown node types must fail --check exactly as they fail a run diff --git a/docs/node-types.md b/docs/node-types.md index ee7e188..f4a3899 100644 --- a/docs/node-types.md +++ b/docs/node-types.md @@ -9,15 +9,16 @@ DART supports several types of nodes that can be used as test targets: - **Local Node (`local`)** Execute tests on the local machine where DART is running. Invariant: at most one `local` node per suite. A second one fails configuration - with `only one local node allowed; "" is a duplicate`, reported against - that node's line in the YAML. The limit applies only to `local`; other types may + with `only one local node allowed; "" duplicates ""`, reported + against that node's line in the YAML and caught by `--check`. The limit applies only to `local`; other types may appear any number of times. Several roles on one machine are modelled with a single local node, distinguished by test and step naming rather than by separate node entries. - **Docker Node (`docker`)** Run tests inside Docker containers, with volume, environment, port, capability, - and privileged-mode options. Supports both local and remote Docker hosts. + privileged-mode, and command/entrypoint options. Supports both local and remote + Docker hosts. - **Docker Compose Node (`docker-compose`)** Manage and test services defined in Docker Compose files. Multiple nodes can target different services in the same compose stack. @@ -135,10 +136,16 @@ on the target platform. - **Docker Compose nodes:** the node name is used as the Compose project name when `project_name` is omitted. +`container_name` (docker) and `instance_name` (lxd, lxd-vm) decouple the platform +identifier from the node identity. Both default to the node name, which is what +makes a suite's containers and instances findable by the name the YAML uses; +setting one is for suites that must match an externally fixed name. The node name +remains what `node:` references, what reports and console output show, and — for +docker — what the container's hostname is set to, so node-side commands still see +the name the suite uses. + Note: name syntax is not validated by DART. A name the platform rejects surfaces as -the daemon's or LXD server's own error during node setup. There is no -`container_name` or `instance_name` option that decouples the platform identifier -from the node identity. +the daemon's or LXD server's own error during node setup. ### Node Security Defaults @@ -161,43 +168,49 @@ for an ephemeral target need not relax it for the long-lived jump host; reconnects after `reboot` route through the bastion too, and chained bastions are rejected rather than silently dropped. -`--check` validates the node options that need no connection: SSH -authentication (a key file that exists and parses, or a password), -`known_hosts` readability under the configured host-key policy, and — when a -`bastion:` block is present — that it names a host, carries usable -credentials, and is not chained. For docker nodes it checks `volumes` and -`ports` specification syntax, resolving relative volume host paths -(`./fixtures:/fixtures`) to absolute paths, since the Engine API would -otherwise treat them as *named volumes* and mount an empty one. These -breaking changes therefore surface before a run rather than during one. - -Note: `--check` does not verify that required fields are present. An `ssh` -node missing `host` passes the check and then fails the run dialling `:22`; a -`docker` node missing `image` fails at container creation; a `docker-compose` -node missing `compose_file` fails at node construction. Only the *bastion's* -host is required at check time. +`--check` validates everything about a node that needs no connection: + +- **Required fields** — `host` on `ssh`, `image` on `docker`, `compose_file` on + `docker-compose`, and a bastion's `host` when a `bastion:` block is present. +- **Option names** — any key the node type does not accept is an error naming + the accepted set (see [Unrecognised Options](#unrecognised-options)). +- **Credentials and host keys** — SSH authentication (a key file that exists and + parses, or a password), `known_hosts` readability under the configured + host-key policy, and that a bastion carries usable credentials and is not + chained. +- **Specification syntax** — docker `volumes` and `ports`, resolving relative + volume host paths (`./fixtures:/fixtures`) to absolute paths, since the Engine + API would otherwise treat them as *named volumes* and mount an empty one. +- **Cross-node constraints** — duplicate node names, and more than one `local` + node. + +What remains outside its reach is anything that needs the platform to answer: +whether an image exists, whether a host is reachable, whether a bind source +exists on the daemon. ### Unrecognised Options -Option names must match exactly. On `docker`, `docker-compose`, `ssh`, and `lxd` -nodes, `options:` is decoded by a JSON round-trip into a typed struct, so any key -that is not a recognised option is discarded without an error or a warning. A -misspelling such as `priviliged` instead of `privileged`, `hostname` instead of -`host`, or `known_host` instead of `known_hosts` leaves the option at its default -and the suite runs on. - -Only `local` nodes warn. A local node prints -`Warning: node "": option "" is not recognized and was ignored (known options: env, shell, sudo, exec_opts)` -to stderr. That warning is specific to local nodes and is not a general guarantee. - -Note: `--check` does not detect option typos on any node type. It validates the -*semantics* of recognised options that need no connection, but it decodes options -through the same round-trip that drops unknown keys, and it substitutes mock nodes -for real ones — so even the local node's warning appears only in a real run. - -A dropped SSH security key fails safe: a mistyped `insecure_skip_host_key` leaves -it `false`, and a mistyped `known_hosts` falls back to `~/.ssh/known_hosts`, so -host-key verification stays on and the symptom is a confusing connection error +Option names must match exactly. A key the node type does not accept is a +configuration error naming the offending key and the full accepted set: + +```text +Error: node "web": unknown option "privilaged" for a docker node (accepted: +capabilities, command, container_name, entrypoint, env, exec_opts, image, +networks, ports, privileged, volumes) +``` + +Rationale: `options:` is decoded by a JSON round-trip into a typed struct, which +discards anything it does not recognise. Without this check a misspelling such as +`priviliged` for `privileged` left the option at its default while the suite read +as though it were set — the assertion looked configured and tested nothing. + +`--check` reports these, so a typo surfaces before any infrastructure is created. +Local nodes additionally warn about keys misplaced inside `exec_opts`. + +Historically a dropped SSH security key failed safe: a mistyped +`insecure_skip_host_key` left it `false`, and a mistyped `known_hosts` fell back +to `~/.ssh/known_hosts`, so host-key verification stayed on and the symptom was a +confusing connection error rather than a silent downgrade. The real cost is a silently ineffective option — a `privileged` or `capabilities` typo, for example, surfaces later as an unexplained permission failure inside the container. @@ -413,6 +426,9 @@ setup, and before any setup step runs. Consequences worth knowing: | `ports` | list of `host:container[/proto]` | Published ports. | | `privileged` | bool | Opt-in full host capabilities; defaults to `false`. | | `capabilities` | list of strings | Individual Linux capabilities, for example `[NET_ADMIN]`. | +| `command` | list of strings | Overrides the image's `CMD`. Use it to give an image that would otherwise exit a process that stays in the foreground. | +| `entrypoint` | list of strings | Overrides the image's `ENTRYPOINT`. | +| `container_name` | string | The container's name on the daemon; defaults to the node name. | Note: DART does not pull Docker images. The `image:` a docker node references must already exist in the local daemon — pulled beforehand (`docker pull nginx:alpine`) @@ -422,19 +438,33 @@ node setup with `could not create container: ...` followed by the daemon's `No such image`. This applies to `type: docker` nodes only: `docker-compose` nodes pull through Compose, and LXD/Incus nodes fetch images through the LXD client. -Note: the container is created from the image's own `CMD`/`ENTRYPOINT`. DART sets -the image, hostname, environment, published ports, bind mounts, and the privilege -options from the table above, and offers no `command`, -`entrypoint`, or `tty` option, so the image must run a process that stays in the -foreground. After starting the container, node setup polls every second for up to -two minutes until the container reports `Running` and a trivial `exec` of `true` -succeeds. An image whose `CMD` exits immediately — such as bare `ubuntu:latest`, -whose `CMD` is `/bin/bash` and which exits at once because DART allocates no TTY -and attaches no stdin — never becomes ready, and setup fails after two minutes with +The container is created from the image's own `CMD`/`ENTRYPOINT` unless +`command:` or `entrypoint:` overrides them. DART allocates no TTY and attaches no +stdin, so the process it runs must stay in the foreground. After starting the +container, node setup polls every second for up to two minutes until the container +reports `Running` and a trivial `exec` of `true` succeeds. An image whose `CMD` +exits immediately — bare `ubuntu:latest`, whose `CMD` is `/bin/bash` — never +becomes ready, and setup fails after two minutes with `container not ready: timeout waiting for container ... context deadline exceeded`. -A service image (`nginx:alpine`, `postgres:16`) or a purpose-built image whose -`CMD` is a supervisor satisfies the check; `examples/docker/docker.yaml` builds -exactly such an image through the `docker.images` block. + +Three ways to satisfy the readiness check: + +- a service image whose `CMD` already stays up (`nginx:alpine`, `postgres:16`); +- a bare distribution image plus a `command:` that stays up: + + ```yaml + nodes: + - name: shellbox + type: docker + options: + image: ubuntu:24.04 + command: ["sleep", "infinity"] + ``` + +- a purpose-built image whose `CMD` is a supervisor, as + `examples/docker/docker.yaml` builds through the `docker.images` block. + +Note: there is no `tty` option. A command that requires a terminal still fails. Warning: `networks` on a `docker` node is not implemented. The option parses but is never applied — `DockerNode.Setup` does not read it, and containers are created @@ -524,8 +554,8 @@ dart -c config.yaml Warning: `volumes` host paths are resolved on the machine running DART but interpreted by the daemon. DART expands a leading `~` from the local `$HOME` and -makes any relative path absolute against DART's working directory; the result is -handed to the daemon as-is. With a remote `DOCKER_HOST`, `./fixtures:/fixtures` +makes any relative path absolute against the suite file's directory; the result +is handed to the daemon as-is. With a remote `DOCKER_HOST`, `./fixtures:/fixtures` becomes a local absolute path the daemon host probably does not have — and a bind source that does not exist is created as an empty directory rather than failing, so a test can read nothing and still pass. `--check` validates the @@ -573,6 +603,7 @@ docker compose -f -p down | `boot_wait` | map | — | Replaces the default readiness check; see [Empty VMs and ISO Boot](#empty-vms-and-iso-boot). | | `exec_opts` | map | — | Currently one key, `shell`, defaulting to `/bin/bash`. | | `project` | string | `default` | LXD project the instance is created in. Not inherited from `lxd.project`. | +| `instance_name` | string | the node name | The instance's name on the LXD/Incus server. | | `socket` | string | auto-detected | Unix socket path; used only when the suite has no top-level `lxd:` block. | | `server`, `protocol` | string | `local`, `lxd` | Image server URL and protocol; used only with a bare image alias. | | `remote_addr`, `trust_token`, `client_cert`, `client_key`, `server_cert`, `skip_verify` | — | — | Remote connection settings; used only when the suite has no top-level `lxd:` block. See [Remote LXD Support](#remote-lxd-support). | @@ -776,10 +807,11 @@ Notes: setting both `empty: true` and `image` is rejected. - `devices` accepts any LXD device configuration and is merged over the NICs generated from [`networks`](#networks), so a node can override a generated device if it needs to. -- Relative `source` paths on pool-less disk devices are made absolute against DART's working - directory — not the suite file's directory, unlike `docker.images[].dockerfile`. A disk - device that names a `pool` refers to a storage volume and is passed through untouched, as - are all sources on remote nodes, which are paths on the remote server. +- Relative `source` paths on pool-less disk devices are made absolute against the suite + file's directory, the same rule `docker.images[].dockerfile` and every other local path + follows. A disk device that names a `pool` refers to a storage volume and is passed + through untouched, as are all sources on remote nodes, which are paths on the remote + server. - `boot_wait` replaces the default readiness check: DART polls `ready_command` through the node's shell (`exec_opts.shell`, default `/bin/bash`) until it exits zero or the timeout expires. Without `ready_command`, being able to run any command at all counts as ready. An diff --git a/docs/steps.md b/docs/steps.md index d45eec1..6679e60 100644 --- a/docs/steps.md +++ b/docs/steps.md @@ -507,20 +507,18 @@ bit, and an existing destination overwritten with `overwrite: true` keeps its current mode. A following `execute` step with `chmod` covers the cases where that is not what is wanted. -Warning: local paths in these steps — `source` for `file_push` and -`file_template`, `dest` for `file_fetch` — resolve against the working directory -DART is invoked from, not against the directory holding the suite file. This -differs from `docker.images[].dockerfile` and `load_from`, which the loader -rewrites relative to the config file's directory. Running -`dart -c examples/foo/suite.yaml` from the repository root therefore looks for -`fixtures/app.conf.tmpl` at `./fixtures/app.conf.tmpl`, not at -`examples/foo/fixtures/app.conf.tmpl`. The two failures surface at different -times: a missing `file_template` source fails at step construction with +Local paths in these steps — `source` for `file_push` and `file_template`, +`dest` for `file_fetch` — follow the same rule as every other local path a +suite writes: absolute paths are used as-is, `~` expands to the invoking user's +home directory, and anything else is relative to the directory holding the +suite file. Running `dart -c examples/foo/suite.yaml` from the repository root +therefore reads `fixtures/app.conf.tmpl` at `examples/foo/fixtures/app.conf.tmpl`, +and the same command works unchanged from any directory. + +Note: a missing source still surfaces at different times by step type. A +missing `file_template` source fails at step construction with `cannot read template in step ""`, before any step runs, while a -missing `file_push` source fails mid-run with `failed to read source `, and -a `file_fetch` `dest` is simply created relative to the working directory. -Absolute paths, `{{env.*}}`/`{{var.*}}` substitution used to build them, or -always invoking DART from a fixed directory all avoid the ambiguity. +missing `file_push` source fails mid-run with `failed to read source `. Content to container and SSH nodes is written in 32 KiB base64 chunks, so files are not limited by the shell's per-argument size cap. That write is not atomic: diff --git a/internal/config/config.go b/internal/config/config.go index 1acfc66..b766a63 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -54,6 +54,12 @@ type Configuration struct { Teardown []*StepConfig `json:"teardown" yaml:"teardown"` Nodes []*NodeConfig `json:"nodes" yaml:"nodes"` Tests []*TestConfig `json:"tests" yaml:"tests"` + + // SuiteDir is the directory holding the suite file. Every local path a + // suite writes resolves against it (see ResolveLocalPath), so a suite + // behaves the same regardless of where DART is invoked from. It is + // empty for configurations built in memory. + SuiteDir string `json:"-" yaml:"-"` } // DockerConfig is the configuration for Docker @@ -78,6 +84,10 @@ type StepConfig struct { Step StepDetails `json:"step" yaml:"step"` Loc SourceLocation `json:"-" yaml:"-"` NodeLoc SourceLocation `json:"-" yaml:"-"` + // SuiteDir carries the suite file's directory to step construction, so + // local paths in options resolve against it rather than the working + // directory. + SuiteDir string `json:"-" yaml:"-"` } // StepDetails is the details of a single step @@ -95,6 +105,10 @@ type NodeConfig struct { Facts map[string]string `json:"facts,omitempty" yaml:"facts,omitempty"` Loc SourceLocation `json:"-" yaml:"-"` TypeLoc SourceLocation `json:"-" yaml:"-"` + // SuiteDir carries the suite file's directory to node construction, so + // local paths in options resolve against it rather than the working + // directory. + SuiteDir string `json:"-" yaml:"-"` } // TestConfig is the configuration for a single test @@ -259,12 +273,30 @@ func ParseConfigurationWithVars(data []byte, location string, cliVars map[string test.Order = i } - // Ensure that the Dockerfile paths that are relative to the execution point + // Local paths resolve against the suite file's directory. Stamping it + // onto each record is what lets step and node construction apply the + // same rule without reaching back for global state. + config.SuiteDir = location + for _, step := range config.Setup { + step.SuiteDir = location + } + for _, step := range config.Teardown { + step.SuiteDir = location + } + for _, node := range config.Nodes { + node.SuiteDir = location + } + if config.Docker != nil { for _, image := range config.Docker.Images { - if !filepath.IsAbs(image.Dockerfile) { - image.Dockerfile = filepath.Join(location, image.Dockerfile) + if image.Dockerfile == "" { + continue + } + resolved, err := ResolveLocalPath(location, image.Dockerfile) + if err != nil { + return nil, err } + image.Dockerfile = resolved } } diff --git a/internal/config/paths.go b/internal/config/paths.go new file mode 100644 index 0000000..427f43a --- /dev/null +++ b/internal/config/paths.go @@ -0,0 +1,48 @@ +package config + +import ( + "fmt" + "os" + "path/filepath" + "strings" +) + +// ResolveLocalPath turns a path written in a suite into an absolute path on +// the machine running DART. +// +// One convention covers every local path a suite can write: absolute paths +// are used as-is, `~` expands to the invoking user's home directory, and +// everything else is relative to the directory holding the suite file. That +// makes a suite portable — it behaves the same whether it is run from the +// repository root, from its own directory, or from a CI checkout elsewhere. +// +// suiteDir is empty for configurations built in memory rather than loaded +// from a file, in which case relative paths fall back to the process working +// directory. +func ResolveLocalPath(suiteDir, path string) (string, error) { + if path == "" { + return path, nil + } + + if strings.HasPrefix(path, "~") { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("cannot resolve %q: %w", path, err) + } + return filepath.Join(home, strings.TrimPrefix(path, "~")), nil + } + + if filepath.IsAbs(path) { + return filepath.Clean(path), nil + } + + if suiteDir != "" { + return filepath.Join(suiteDir, path), nil + } + + absolute, err := filepath.Abs(path) + if err != nil { + return "", fmt.Errorf("cannot resolve %q: %w", path, err) + } + return absolute, nil +} diff --git a/internal/config/paths_test.go b/internal/config/paths_test.go new file mode 100644 index 0000000..51de646 --- /dev/null +++ b/internal/config/paths_test.go @@ -0,0 +1,88 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestResolveLocalPath(t *testing.T) { + suite := filepath.Join(t.TempDir(), "suites") + require.NoError(t, os.MkdirAll(suite, 0o755)) + + // Relative paths belong to the suite, not to the working directory + resolved, err := ResolveLocalPath(suite, "fixtures/app.conf") + require.NoError(t, err) + assert.Equal(t, filepath.Join(suite, "fixtures/app.conf"), resolved) + + // ../ still works, relative to the suite + resolved, err = ResolveLocalPath(suite, "../shared/app.conf") + require.NoError(t, err) + assert.Equal(t, filepath.Join(filepath.Dir(suite), "shared/app.conf"), resolved) + + // Absolute stays put + resolved, err = ResolveLocalPath(suite, "/etc/hosts") + require.NoError(t, err) + assert.Equal(t, "/etc/hosts", resolved) + + // ~ is the invoking user's home, never the suite + home, err := os.UserHomeDir() + require.NoError(t, err) + resolved, err = ResolveLocalPath(suite, "~/.ssh/id_ed25519") + require.NoError(t, err) + assert.Equal(t, filepath.Join(home, ".ssh/id_ed25519"), resolved) + + // An empty path is left alone: callers use "" to mean "not set" + resolved, err = ResolveLocalPath(suite, "") + require.NoError(t, err) + assert.Equal(t, "", resolved) + + // With no suite directory — a configuration built in memory — relative + // paths fall back to the working directory + wd, err := os.Getwd() + require.NoError(t, err) + resolved, err = ResolveLocalPath("", "fixtures/app.conf") + require.NoError(t, err) + assert.Equal(t, filepath.Join(wd, "fixtures/app.conf"), resolved) +} + +// The suite directory reaches every record that can carry a local path, so +// step and node construction can apply the rule without global state. +func TestLoadStampsSuiteDir(t *testing.T) { + dir := t.TempDir() + suitePath := filepath.Join(dir, "suite.yaml") + require.NoError(t, os.WriteFile(suitePath, []byte(`suite: paths +nodes: + - name: local + type: local +setup: + - name: push a fixture + node: local + step: + type: file_push + options: + source: fixtures/app.conf + dest: /tmp/app.conf +teardown: + - name: clean up + node: local + step: + type: execute + options: + command: "true" +`), 0o644)) + + config, err := LoadConfiguration(suitePath) + require.NoError(t, err) + + assert.Equal(t, dir, config.SuiteDir) + require.Len(t, config.Setup, 1) + assert.Equal(t, dir, config.Setup[0].SuiteDir) + require.Len(t, config.Teardown, 1) + assert.Equal(t, dir, config.Teardown[0].SuiteDir) + require.Len(t, config.Nodes, 1) + assert.Equal(t, dir, config.Nodes[0].SuiteDir) +} diff --git a/internal/docker/containers.go b/internal/docker/containers.go index e28a861..a54f003 100644 --- a/internal/docker/containers.go +++ b/internal/docker/containers.go @@ -30,6 +30,8 @@ type containerOptions struct { volumes []string env []string ports []string + command []string + entrypoint []string } // WithDetach is a function that sets the detach option for creating a container. @@ -83,6 +85,22 @@ func WithPorts(ports []string) ContainerOptions { } } +// WithCommand overrides the image's CMD. An image whose default command +// exits immediately — a bare distribution image, whose CMD is an +// interactive shell — needs one that stays in the foreground instead. +func WithCommand(command []string) ContainerOptions { + return func(o *containerOptions) { + o.command = command + } +} + +// WithEntrypoint overrides the image's ENTRYPOINT. +func WithEntrypoint(entrypoint []string) ContainerOptions { + return func(o *containerOptions) { + o.entrypoint = entrypoint + } +} + // WithNetworkMode is a function that sets the network mode option for creating a container. func WithNetworkMode(networkMode string) ContainerOptions { return func(o *containerOptions) { diff --git a/internal/docker/wrapper.go b/internal/docker/wrapper.go index 48932cb..21382f5 100644 --- a/internal/docker/wrapper.go +++ b/internal/docker/wrapper.go @@ -158,9 +158,11 @@ func (w *Wrapper) CreateContainer(name, hostname, image string, options ...Conta } containerCfg := &container.Config{ - Image: image, - Hostname: hostname, - Env: c.env, + Image: image, + Hostname: hostname, + Env: c.env, + Cmd: c.command, + Entrypoint: c.entrypoint, } hostCfg := &container.HostConfig{ Privileged: c.priviliged, diff --git a/pkg/nodetypes/base.go b/pkg/nodetypes/base.go index 3c999c6..365ca80 100644 --- a/pkg/nodetypes/base.go +++ b/pkg/nodetypes/base.go @@ -3,6 +3,9 @@ package nodetypes import ( "encoding/json" "fmt" + "reflect" + "sort" + "strings" "github.com/bgrewell/dart/internal/config" "github.com/bgrewell/dart/internal/docker" @@ -26,17 +29,26 @@ func IsKnownNodeType(nodeType string) bool { return knownNodeTypes[nodeType] } -// ValidateNodeOptions checks the option shapes that can be verified -// without contacting anything, so --check catches them before a run: -// unreadable known_hosts, missing SSH credentials, malformed bastion or -// volume specifications. +// ValidateNodeOptions checks everything about a single node that can be +// verified without contacting anything, so --check catches it before a run: +// required fields, unreadable known_hosts, missing SSH credentials, and +// malformed bastion, volume, or port specifications. func ValidateNodeOptions(cfg *config.NodeConfig) error { + if err := validateOptionNames(cfg); err != nil { + return err + } + switch cfg.Type { case "ssh": var opts SshNodeOpts if err := decodeNodeOptions(cfg.Options, &opts); err != nil { return err } + // Without a host the dial goes to ":22" and fails mid-run with a + // connection error that says nothing about the real mistake + if opts.Host == "" { + return fmt.Errorf("host is required") + } if _, err := sshAuthMethods(opts.KeyFile, opts.Pass); err != nil { return err } @@ -54,12 +66,37 @@ func ValidateNodeOptions(cfg *config.NodeConfig) error { return fmt.Errorf("bastion: %w", err) } } - case "docker", "docker-compose": + case "docker": + var opts DockerNodeOpts + if err := decodeNodeOptions(cfg.Options, &opts); err != nil { + return err + } + if opts.Image == "" { + return fmt.Errorf("image is required") + } + if _, err := resolveVolumes(opts.Volumes, cfg.SuiteDir); err != nil { + return err + } + if len(opts.Ports) > 0 { + if err := docker.ValidatePortSpecs(opts.Ports); err != nil { + return err + } + } + case "docker-compose": + var composeOpts DockerComposeNodeOpts + if err := decodeNodeOptions(cfg.Options, &composeOpts); err != nil { + return err + } + if composeOpts.ComposeFile == "" { + return fmt.Errorf("compose_file is required") + } + + // A compose node also accepts the docker option shapes var opts DockerNodeOpts if err := decodeNodeOptions(cfg.Options, &opts); err != nil { return err } - if _, err := resolveVolumes(opts.Volumes); err != nil { + if _, err := resolveVolumes(opts.Volumes, cfg.SuiteDir); err != nil { return err } if len(opts.Ports) > 0 { @@ -67,10 +104,88 @@ func ValidateNodeOptions(cfg *config.NodeConfig) error { return err } } + case "lxd", "lxd-vm": + var opts LxdNodeOpts + if err := decodeNodeOptions(cfg.Options, &opts); err != nil { + return err + } + if err := opts.validate(); err != nil { + return err + } + } + return nil +} + +// optionKeysOf collects the option names a set of typed option structs +// accepts, read from their json tags — the same tags the decode uses, so +// the two cannot drift. +func optionKeysOf(targets ...interface{}) map[string]bool { + keys := map[string]bool{} + for _, target := range targets { + t := reflect.TypeOf(target) + for t.Kind() == reflect.Ptr { + t = t.Elem() + } + for i := 0; i < t.NumField(); i++ { + tag := t.Field(i).Tag.Get("json") + name, _, _ := strings.Cut(tag, ",") + if name != "" && name != "-" { + keys[name] = true + } + } + } + return keys +} + +// knownNodeOptions lists the option names each node type accepts. A key +// outside the set is a typo or a misplaced option: the decode silently +// drops it, so without this check the option reads as configured while +// nothing consumes it. +func knownNodeOptions(nodeType string) map[string]bool { + switch nodeType { + case "local": + return map[string]bool{"env": true, "shell": true, "sudo": true, "exec_opts": true} + case "ssh": + return optionKeysOf(SshNodeOpts{}) + case "docker": + return optionKeysOf(DockerNodeOpts{}) + case "docker-compose": + return optionKeysOf(DockerComposeNodeOpts{}, DockerNodeOpts{}) + case "lxd", "lxd-vm": + return optionKeysOf(LxdNodeOpts{}) } return nil } +// validateOptionNames reports the first unrecognized option name, listing +// what the type accepts so the fix is obvious. +func validateOptionNames(cfg *config.NodeConfig) error { + known := knownNodeOptions(cfg.Type) + if known == nil { + return nil + } + + unknown := make([]string, 0, len(cfg.Options)) + for key := range cfg.Options { + if !known[key] { + unknown = append(unknown, key) + } + } + if len(unknown) == 0 { + return nil + } + sort.Strings(unknown) + + accepted := make([]string, 0, len(known)) + for key := range known { + accepted = append(accepted, key) + } + sort.Strings(accepted) + + return fmt.Errorf("unknown option %q for a %s node (accepted: %s)", + unknown[0], cfg.Type, strings.Join(accepted, ", ")) +} + // decodeNodeOptions reuses the same JSON round-trip the factories use, so // validation sees exactly what construction would. func decodeNodeOptions(options map[string]interface{}, target interface{}) error { @@ -86,43 +201,62 @@ func CreateNodes(configs []*config.NodeConfig, wrapper *docker.Wrapper) (map[str return CreateNodesWithWrappers(configs, wrapper, nil) } -// CreateNodesWithWrappers creates nodes using both Docker and LXD wrappers -func CreateNodesWithWrappers(configs []*config.NodeConfig, dockerWrapper *docker.Wrapper, lxdWrapper *lxd.Wrapper) (map[string]ifaces.Node, error) { - nodes := make(map[string]ifaces.Node) - localNodeExists := false +// ValidateNodeSet checks the constraints that span the whole node list +// rather than a single node: names must be unique, and at most one node may +// be of type local. Keeping them here rather than inline in the factory is +// what lets --check report them without constructing anything. +func ValidateNodeSet(configs []*config.NodeConfig) error { + seen := make(map[string]bool, len(configs)) + localNode := "" for _, cfg := range configs { - if _, exists := nodes[cfg.Name]; exists { - return nil, &config.ConfigError{ + if seen[cfg.Name] { + return &config.ConfigError{ Message: fmt.Sprintf("duplicate node name %q", cfg.Name), Location: cfg.Loc, } } + seen[cfg.Name] = true + + if cfg.Type == "local" { + if localNode != "" { + return &config.ConfigError{ + Message: fmt.Sprintf("only one local node allowed; %q duplicates %q", cfg.Name, localNode), + Location: cfg.Loc, + } + } + localNode = cfg.Name + } + } + return nil +} +// CreateNodesWithWrappers creates nodes using both Docker and LXD wrappers +func CreateNodesWithWrappers(configs []*config.NodeConfig, dockerWrapper *docker.Wrapper, lxdWrapper *lxd.Wrapper) (map[string]ifaces.Node, error) { + if err := ValidateNodeSet(configs); err != nil { + return nil, err + } + + nodes := make(map[string]ifaces.Node) + + for _, cfg := range configs { var node ifaces.Node var err error switch cfg.Type { case "local": - if localNodeExists { - return nil, &config.ConfigError{ - Message: fmt.Sprintf("only one local node allowed; %q is a duplicate", cfg.Name), - Location: cfg.Loc, - } - } node = NewLocalNode(cfg.Name, &cfg.Options) - localNodeExists = true case "docker": - node, err = NewDockerNode(dockerWrapper, cfg.Name, &cfg.Options) + node, err = NewDockerNode(dockerWrapper, cfg.Name, &cfg.Options, cfg.SuiteDir) case "docker-compose": - node, err = NewDockerComposeNode(dockerWrapper, cfg.Name, &cfg.Options) + node, err = NewDockerComposeNode(dockerWrapper, cfg.Name, &cfg.Options, cfg.SuiteDir) case "ssh": - node, err = NewSshNode(cfg.Name, &cfg.Options) + node, err = NewSshNode(cfg.Name, &cfg.Options, cfg.SuiteDir) case "lxd": if lxdWrapper != nil { - node, err = NewLxdNodeWithWrapper(lxdWrapper, cfg.Name, &cfg.Options) + node, err = NewLxdNodeWithWrapper(lxdWrapper, cfg.Name, &cfg.Options, cfg.SuiteDir) } else { - node, err = NewLxdNode(cfg.Name, &cfg.Options) + node, err = NewLxdNode(cfg.Name, &cfg.Options, cfg.SuiteDir) } case "lxd-vm": // Alias for LXD virtual machine type. The options map is copied @@ -133,9 +267,9 @@ func CreateNodesWithWrappers(configs []*config.NodeConfig, dockerWrapper *docker } opts["instance_type"] = "virtual-machine" if lxdWrapper != nil { - node, err = NewLxdNodeWithWrapper(lxdWrapper, cfg.Name, &opts) + node, err = NewLxdNodeWithWrapper(lxdWrapper, cfg.Name, &opts, cfg.SuiteDir) } else { - node, err = NewLxdNode(cfg.Name, &opts) + node, err = NewLxdNode(cfg.Name, &opts, cfg.SuiteDir) } default: return nil, &config.ConfigError{ diff --git a/pkg/nodetypes/docker.go b/pkg/nodetypes/docker.go index a3a3b4b..eb04c66 100644 --- a/pkg/nodetypes/docker.go +++ b/pkg/nodetypes/docker.go @@ -3,10 +3,10 @@ package nodetypes import ( "encoding/json" "fmt" - "os" "path/filepath" "strings" + "github.com/bgrewell/dart/internal/config" "github.com/bgrewell/dart/internal/docker" "github.com/bgrewell/dart/internal/execution" "github.com/bgrewell/dart/internal/helpers" @@ -22,7 +22,16 @@ type DockerNetworkOpts struct { } type DockerNodeOpts struct { - Image string `yaml:"image,omitempty" json:"image"` + Image string `yaml:"image,omitempty" json:"image"` + // ContainerName decouples the Docker object's name from the node name. + // Defaults to the node name, which is what makes a suite's containers + // findable by the name the YAML uses. + ContainerName string `yaml:"container_name,omitempty" json:"container_name"` + // Command and Entrypoint override the image's CMD and ENTRYPOINT. An + // image whose default command exits immediately cannot host a node + // otherwise. + Command []string `yaml:"command,omitempty" json:"command"` + Entrypoint []string `yaml:"entrypoint,omitempty" json:"entrypoint"` ExecOptions map[string]interface{} `yaml:"exec_opts,omitempty" json:"exec_opts"` Networks []DockerNetworkOpts `yaml:"networks,omitempty" json:"networks"` // Privileged grants the container full host capabilities. It is @@ -37,7 +46,7 @@ type DockerNodeOpts struct { Capabilities []string `yaml:"capabilities,omitempty" json:"capabilities"` } -func NewDockerNode(wrapper *docker.Wrapper, name string, opts ifaces.NodeOptions) (node ifaces.Node, err error) { +func NewDockerNode(wrapper *docker.Wrapper, name string, opts ifaces.NodeOptions, suiteDir string) (node ifaces.Node, err error) { jsonData, err := json.Marshal(opts) if err != nil { @@ -51,23 +60,25 @@ func NewDockerNode(wrapper *docker.Wrapper, name string, opts ifaces.NodeOptions } return &DockerNode{ - name: name, - wrapper: wrapper, - options: nodeopts, + name: name, + wrapper: wrapper, + options: nodeopts, + suiteDir: suiteDir, }, nil } type DockerNode struct { - name string - wrapper *docker.Wrapper - options DockerNodeOpts + name string + wrapper *docker.Wrapper + options DockerNodeOpts + suiteDir string } // resolveVolumes turns relative host paths into absolute ones. The Engine // API treats a non-absolute source as a NAMED VOLUME, so "./fixtures" // would silently mount an empty volume instead of the directory — // a passing-looking test against missing data. -func resolveVolumes(volumes []string) ([]string, error) { +func resolveVolumes(volumes []string, suiteDir string) ([]string, error) { resolved := make([]string, 0, len(volumes)) for _, spec := range volumes { parts := strings.SplitN(spec, ":", 2) @@ -77,19 +88,12 @@ func resolveVolumes(volumes []string) ([]string, error) { source := parts[0] // A bare name (no separator) is a named volume by intent; a path // is anything containing a separator or starting with . or ~ - if strings.HasPrefix(source, "~") { - home, err := os.UserHomeDir() + if strings.HasPrefix(source, "~") || (strings.ContainsAny(source, "/.") && !filepath.IsAbs(source)) { + resolved, err := config.ResolveLocalPath(suiteDir, source) if err != nil { - return nil, fmt.Errorf("volume %q: cannot resolve ~: %w", spec, err) + return nil, fmt.Errorf("volume %q: %w", spec, err) } - source = filepath.Join(home, strings.TrimPrefix(source, "~")) - } - if strings.ContainsAny(source, "/.") && !filepath.IsAbs(source) { - absolute, err := filepath.Abs(source) - if err != nil { - return nil, fmt.Errorf("volume %q: cannot resolve host path: %w", spec, err) - } - source = absolute + source = resolved } resolved = append(resolved, source+":"+parts[1]) } @@ -105,7 +109,7 @@ func (d *DockerNode) Setup() error { opts = append(opts, docker.WithCapabilities(d.options.Capabilities)) } if len(d.options.Volumes) > 0 { - volumes, err := resolveVolumes(d.options.Volumes) + volumes, err := resolveVolumes(d.options.Volumes, d.suiteDir) if err != nil { return err } @@ -117,38 +121,57 @@ func (d *DockerNode) Setup() error { if len(d.options.Ports) > 0 { opts = append(opts, docker.WithPorts(d.options.Ports)) } + if len(d.options.Command) > 0 { + opts = append(opts, docker.WithCommand(d.options.Command)) + } + if len(d.options.Entrypoint) > 0 { + opts = append(opts, docker.WithEntrypoint(d.options.Entrypoint)) + } - if err := d.wrapper.CreateContainer(d.name, d.name, d.options.Image, opts...); err != nil { + // The hostname stays the node name even when the container is named + // something else, so node-side commands see the name the suite uses + if err := d.wrapper.CreateContainer(d.containerName(), d.name, d.options.Image, opts...); err != nil { return err } - if err := d.wrapper.StartContainer(d.name); err != nil { + if err := d.wrapper.StartContainer(d.containerName()); err != nil { return err } // Wait for the container to be fully ready (running and responsive) - if err := d.wrapper.WaitForContainerReady(d.name); err != nil { + if err := d.wrapper.WaitForContainerReady(d.containerName()); err != nil { return err } return nil } +// containerName is the Docker object's name. It defaults to the node name, +// so a suite's containers are findable by the name the YAML uses; +// container_name overrides it for suites that must match an externally +// fixed name. +func (d *DockerNode) containerName() string { + if d.options.ContainerName != "" { + return d.options.ContainerName + } + return d.name +} + // Teardown stops and removes the container. A container that no longer // exists (partial setup, previous cleanup, teardown-only run) counts as // already removed. func (d *DockerNode) Teardown() error { - if err := d.wrapper.StopContainer(d.name); err != nil { + if err := d.wrapper.StopContainer(d.containerName()); err != nil { if docker.IsNotFound(err) { return nil } return err } - if err := d.wrapper.RemoveContainer(d.name); err != nil && !docker.IsNotFound(err) { + if err := d.wrapper.RemoveContainer(d.containerName()); err != nil && !docker.IsNotFound(err) { return err } return nil } func (d *DockerNode) Execute(command string, options ...execution.ExecutionOption) (result *execution.ExecutionResult, err error) { - code, stdout, stderr, err := d.wrapper.ExecuteInContainerStreaming(d.name, command, execution.IsDebugMode()) + code, stdout, stderr, err := d.wrapper.ExecuteInContainerStreaming(d.containerName(), command, execution.IsDebugMode()) if err != nil { return nil, err } @@ -168,7 +191,7 @@ var _ ifaces.NetworkInspector = &DockerNode{} // without a fact command. Each attached network also yields a // per-network fact ("ipv4.test-net"). func (d *DockerNode) NetworkFacts() (map[string]string, error) { - return d.wrapper.ContainerNetworkFacts(d.name) + return d.wrapper.ContainerNetworkFacts(d.containerName()) } // Close has nothing to release: the container lifecycle is handled by diff --git a/pkg/nodetypes/docker_compose.go b/pkg/nodetypes/docker_compose.go index b4f70d8..00e4746 100644 --- a/pkg/nodetypes/docker_compose.go +++ b/pkg/nodetypes/docker_compose.go @@ -3,6 +3,7 @@ package nodetypes import ( "encoding/json" "fmt" + "github.com/bgrewell/dart/internal/config" "github.com/bgrewell/dart/internal/docker" "github.com/bgrewell/dart/internal/execution" "github.com/bgrewell/dart/internal/helpers" @@ -21,7 +22,7 @@ type DockerComposeNodeOpts struct { } // NewDockerComposeNode creates a new docker-compose node -func NewDockerComposeNode(wrapper *docker.Wrapper, name string, opts ifaces.NodeOptions) (node ifaces.Node, err error) { +func NewDockerComposeNode(wrapper *docker.Wrapper, name string, opts ifaces.NodeOptions, suiteDir string) (node ifaces.Node, err error) { jsonData, err := json.Marshal(opts) if err != nil { return nil, err @@ -37,6 +38,9 @@ func NewDockerComposeNode(wrapper *docker.Wrapper, name string, opts ifaces.Node if nodeopts.ComposeFile == "" { return nil, fmt.Errorf("compose_file is required for docker-compose node") } + if nodeopts.ComposeFile, err = config.ResolveLocalPath(suiteDir, nodeopts.ComposeFile); err != nil { + return nil, err + } return &DockerComposeNode{ name: name, diff --git a/pkg/nodetypes/docker_options_test.go b/pkg/nodetypes/docker_options_test.go index a257171..cd66ab0 100644 --- a/pkg/nodetypes/docker_options_test.go +++ b/pkg/nodetypes/docker_options_test.go @@ -15,7 +15,7 @@ import ( // non-absolute source as a NAMED VOLUME, silently mounting an empty // volume instead of the directory. func TestResolveVolumesMakesHostPathsAbsolute(t *testing.T) { - resolved, err := resolveVolumes([]string{"./fixtures:/fixtures:ro"}) + resolved, err := resolveVolumes([]string{"./fixtures:/fixtures:ro"}, "") require.NoError(t, err) require.Len(t, resolved, 1) assert.True(t, filepath.IsAbs(strings.SplitN(resolved[0], ":", 2)[0]), @@ -24,7 +24,7 @@ func TestResolveVolumesMakesHostPathsAbsolute(t *testing.T) { } func TestResolveVolumesKeepsNamedVolumes(t *testing.T) { - resolved, err := resolveVolumes([]string{"cache-data:/var/cache"}) + resolved, err := resolveVolumes([]string{"cache-data:/var/cache"}, "") require.NoError(t, err) assert.Equal(t, "cache-data:/var/cache", resolved[0], "a bare name stays a named volume") } @@ -32,28 +32,48 @@ func TestResolveVolumesKeepsNamedVolumes(t *testing.T) { func TestResolveVolumesExpandsHome(t *testing.T) { home, err := os.UserHomeDir() require.NoError(t, err) - resolved, err := resolveVolumes([]string{"~/data:/data"}) + resolved, err := resolveVolumes([]string{"~/data:/data"}, "") require.NoError(t, err) assert.True(t, strings.HasPrefix(resolved[0], home)) } func TestResolveVolumesRejectsMalformed(t *testing.T) { - _, err := resolveVolumes([]string{"/no-container-side"}) + _, err := resolveVolumes([]string{"/no-container-side"}, "") require.Error(t, err) assert.Contains(t, err.Error(), "host:container") } +// --check must catch missing required fields, so a suite does not get all +// the way to a dial or a container create before reporting the real mistake. +func TestValidateNodeOptionsRequiresRequiredFields(t *testing.T) { + err := ValidateNodeOptions(&config.NodeConfig{ + Name: "web", Type: "docker", Options: map[string]interface{}{}, + }) + assert.ErrorContains(t, err, "image is required") + + err = ValidateNodeOptions(&config.NodeConfig{ + Name: "remote", Type: "ssh", + Options: map[string]interface{}{"user": "root", "insecure_skip_host_key": true, "pass": "x"}, + }) + assert.ErrorContains(t, err, "host is required") + + err = ValidateNodeOptions(&config.NodeConfig{ + Name: "stack", Type: "docker-compose", Options: map[string]interface{}{}, + }) + assert.ErrorContains(t, err, "compose_file is required") +} + // --check must catch option problems that need no daemon or network. func TestValidateNodeOptionsCatchesLocalProblems(t *testing.T) { err := ValidateNodeOptions(&config.NodeConfig{ Name: "web", Type: "docker", - Options: map[string]interface{}{"ports": []interface{}{"not-a-port-spec:::"}}, + Options: map[string]interface{}{"image": "nginx", "ports": []interface{}{"not-a-port-spec:::"}}, }) assert.Error(t, err) err = ValidateNodeOptions(&config.NodeConfig{ Name: "web", Type: "docker", - Options: map[string]interface{}{"volumes": []interface{}{"/bad-spec"}}, + Options: map[string]interface{}{"image": "nginx", "volumes": []interface{}{"/bad-spec"}}, }) assert.ErrorContains(t, err, "host:container") diff --git a/pkg/nodetypes/lxd.go b/pkg/nodetypes/lxd.go index 7345786..066aaf9 100644 --- a/pkg/nodetypes/lxd.go +++ b/pkg/nodetypes/lxd.go @@ -13,13 +13,13 @@ import ( "fmt" "math/big" "net" - "path/filepath" "sort" "strconv" "strings" "time" "unicode" + "github.com/bgrewell/dart/internal/config" "github.com/bgrewell/dart/internal/execution" "github.com/bgrewell/dart/internal/helpers" "github.com/bgrewell/dart/internal/lxc" @@ -80,6 +80,10 @@ type LxdNodeOpts struct { SkipVerify bool `yaml:"skip_verify,omitempty" json:"skip_verify"` // Skip TLS verification (not recommended for production) // Project support Project string `yaml:"project,omitempty" json:"project"` // LXD project to use (defaults to lxd.DefaultProject) + // InstanceName decouples the LXD/Incus instance name from the node + // name. Defaults to the node name, which is what makes a suite's + // instances findable by the name the YAML uses. + InstanceName string `yaml:"instance_name,omitempty" json:"instance_name"` } // emptyInstance reports whether the instance should be created without an image. @@ -147,7 +151,7 @@ func optionValueToString(value interface{}) string { // Relative disk sources are resolved against the working directory so that test files // can reference build artifacts by their path in the repository. Sources are paths on // the LXD host, so they are left untouched when the node talks to a remote server. -func buildDevices(devices map[string]map[string]interface{}, resolvePaths bool) (map[string]map[string]string, error) { +func buildDevices(devices map[string]map[string]interface{}, resolvePaths bool, suiteDir string) (map[string]map[string]string, error) { built := make(map[string]map[string]string, len(devices)) for deviceName, device := range devices { converted := make(map[string]string, len(device)) @@ -161,7 +165,7 @@ func buildDevices(devices map[string]map[string]interface{}, resolvePaths bool) // Only plain disks reference a host path; disks backed by a storage pool name a volume if resolvePaths && converted["type"] == "disk" && converted["pool"] == "" && converted["source"] != "" { - absolute, err := filepath.Abs(converted["source"]) + absolute, err := config.ResolveLocalPath(suiteDir, converted["source"]) if err != nil { return nil, helpers.WrapError(fmt.Sprintf("device %q: unable to resolve source %q: %v", deviceName, converted["source"], err)) } @@ -175,7 +179,7 @@ func buildDevices(devices map[string]map[string]interface{}, resolvePaths bool) } // NewLxdNode creates a new LXD node without using the wrapper -func NewLxdNode(name string, opts ifaces.NodeOptions) (node ifaces.Node, err error) { +func NewLxdNode(name string, opts ifaces.NodeOptions, suiteDir string) (node ifaces.Node, err error) { jsonData, err := json.Marshal(opts) if err != nil { @@ -236,6 +240,15 @@ func NewLxdNode(name string, opts ifaces.NodeOptions) (node ifaces.Node, err err nodeopts.Protocol = protocol } + // Certificate paths belong to the machine running DART + for _, field := range []*string{&nodeopts.ClientCert, &nodeopts.ClientKey, &nodeopts.ServerCert} { + resolved, resolveErr := config.ResolveLocalPath(suiteDir, *field) + if resolveErr != nil { + return nil, resolveErr + } + *field = resolved + } + // Connect to LXD server (local or remote) var client lxdclient.InstanceServer if nodeopts.RemoteAddr != "" { @@ -340,9 +353,10 @@ func NewLxdNode(name string, opts ifaces.NodeOptions) (node ifaces.Node, err err } return &LxdNode{ - name: name, - options: nodeopts, - client: client, + name: name, + options: nodeopts, + client: client, + suiteDir: suiteDir, }, nil } @@ -351,7 +365,7 @@ func NewLxdNode(name string, opts ifaces.NodeOptions) (node ifaces.Node, err err // Note: When using a wrapper, the connection to the LXD server is managed by the wrapper itself. // Remote connection configuration in node options will be ignored. // Use NewWrapper or NewWrapperWithOptions to configure remote connections when using wrappers. -func NewLxdNodeWithWrapper(wrapper *lxd.Wrapper, name string, opts ifaces.NodeOptions) (node ifaces.Node, err error) { +func NewLxdNodeWithWrapper(wrapper *lxd.Wrapper, name string, opts ifaces.NodeOptions, suiteDir string) (node ifaces.Node, err error) { jsonData, err := json.Marshal(opts) if err != nil { @@ -406,15 +420,28 @@ func NewLxdNodeWithWrapper(wrapper *lxd.Wrapper, name string, opts ifaces.NodeOp } return &LxdNode{ - name: name, - options: nodeopts, - wrapper: wrapper, - client: client, + name: name, + options: nodeopts, + wrapper: wrapper, + client: client, + suiteDir: suiteDir, }, nil } +// instanceName is the LXD/Incus instance's name. It defaults to the node +// name, so a suite's instances are findable by the name the YAML uses; +// instance_name overrides it for suites that must match an externally +// fixed name. +func (d *LxdNode) instanceName() string { + if d.options.InstanceName != "" { + return d.options.InstanceName + } + return d.name +} + type LxdNode struct { name string + suiteDir string client lxdclient.InstanceServer wrapper *lxd.Wrapper options LxdNodeOpts @@ -456,7 +483,7 @@ func (d *LxdNode) Setup() error { // Merge in any explicitly configured devices, such as an ISO attached as boot media. // These are applied last so a node can override a generated NIC if it needs to. - configuredDevices, err := buildDevices(d.options.Devices, d.options.RemoteAddr == "") + configuredDevices, err := buildDevices(d.options.Devices, d.options.RemoteAddr == "", d.suiteDir) if err != nil { return err } @@ -483,7 +510,7 @@ func (d *LxdNode) Setup() error { // Create a request for the instance req := api.InstancesPost{ - Name: d.name, + Name: d.instanceName(), Source: source, Type: instanceType, InstancePut: api.InstancePut{ @@ -510,7 +537,7 @@ func (d *LxdNode) Setup() error { Timeout: -1, } - op, err = d.client.UpdateInstanceState(d.name, reqState, "") + op, err = d.client.UpdateInstanceState(d.instanceName(), reqState, "") if err != nil { return helpers.WrapError(fmt.Sprintf("error starting instance: %v", err)) } @@ -533,7 +560,7 @@ func (d *LxdNode) NetworkFacts() (map[string]string, error) { if d.client == nil { return nil, helpers.WrapError("lxd client not initialized") } - state, _, err := d.client.GetInstanceState(d.name) + state, _, err := d.client.GetInstanceState(d.instanceName()) if err != nil { return nil, err } @@ -578,7 +605,7 @@ func (d *LxdNode) Snapshot(name string, stateful bool) error { if d.client == nil { return helpers.WrapError("lxd client not initialized") } - return lxd.CreateInstanceSnapshot(context.Background(), d.client, d.name, name, stateful) + return lxd.CreateInstanceSnapshot(context.Background(), d.client, d.instanceName(), name, stateful) } // RestoreSnapshot rolls the instance back to a snapshot. LXD stops and @@ -592,11 +619,11 @@ func (d *LxdNode) RestoreSnapshot(name string, stateful bool) error { } wasRunning := false - if state, _, err := d.client.GetInstanceState(d.name); err == nil { + if state, _, err := d.client.GetInstanceState(d.instanceName()); err == nil { wasRunning = state.Status == "Running" } - if err := lxd.RestoreInstanceSnapshot(context.Background(), d.client, d.name, name, stateful); err != nil { + if err := lxd.RestoreInstanceSnapshot(context.Background(), d.client, d.instanceName(), name, stateful); err != nil { return err } @@ -605,8 +632,8 @@ func (d *LxdNode) RestoreSnapshot(name string, stateful bool) error { } cfg := d.options.BootWait.readinessConfig() command := d.options.BootWait.readyCommand(d.shell()) - if err := lxd.WaitForInstanceCommand(context.Background(), d.client, d.name, command, cfg); err != nil { - return helpers.WrapError(fmt.Sprintf("instance %s did not become ready after restoring snapshot %s: %v", d.name, name, err)) + if err := lxd.WaitForInstanceCommand(context.Background(), d.client, d.instanceName(), command, cfg); err != nil { + return helpers.WrapError(fmt.Sprintf("instance %s did not become ready after restoring snapshot %s: %v", d.instanceName(), name, err)) } return nil } @@ -617,7 +644,7 @@ func (d *LxdNode) DeleteSnapshot(name string) error { if d.client == nil { return helpers.WrapError("lxd client not initialized") } - if err := lxd.DeleteInstanceSnapshot(context.Background(), d.client, d.name, name); err != nil && !lxd.IsNotFound(err) { + if err := lxd.DeleteInstanceSnapshot(context.Background(), d.client, d.instanceName(), name); err != nil && !lxd.IsNotFound(err) { return err } return nil @@ -630,16 +657,16 @@ var _ ifaces.Rebooter = &LxdNode{} // which matters for crash-safety testing. The readiness wait reuses the // node's boot_wait configuration; readyCommand and timeout override it. func (d *LxdNode) Reboot(force bool, readyCommand string, timeout time.Duration) error { - op, err := d.client.UpdateInstanceState(d.name, api.InstanceStatePut{ + op, err := d.client.UpdateInstanceState(d.instanceName(), api.InstanceStatePut{ Action: "restart", Timeout: -1, Force: force, }, "") if err != nil { - return helpers.WrapError(fmt.Sprintf("error restarting instance %s: %v", d.name, err)) + return helpers.WrapError(fmt.Sprintf("error restarting instance %s: %v", d.instanceName(), err)) } if err := op.Wait(); err != nil { - return helpers.WrapError(fmt.Sprintf("error restarting instance %s: %v", d.name, err)) + return helpers.WrapError(fmt.Sprintf("error restarting instance %s: %v", d.instanceName(), err)) } cfg := d.options.BootWait.readinessConfig() @@ -650,8 +677,8 @@ func (d *LxdNode) Reboot(force bool, readyCommand string, timeout time.Duration) if readyCommand != "" { command = []string{d.shell(), "-c", readyCommand} } - if err := lxd.WaitForInstanceCommand(context.Background(), d.client, d.name, command, cfg); err != nil { - return helpers.WrapError(fmt.Sprintf("instance %s did not become ready after reboot: %v", d.name, err)) + if err := lxd.WaitForInstanceCommand(context.Background(), d.client, d.instanceName(), command, cfg); err != nil { + return helpers.WrapError(fmt.Sprintf("instance %s did not become ready after reboot: %v", d.instanceName(), err)) } return nil } @@ -675,7 +702,7 @@ func (d *LxdNode) waitForReady() error { } } command := d.options.BootWait.readyCommand(d.shell()) - if err := lxd.WaitForInstanceCommand(ctx, d.client, d.name, command, d.options.BootWait.readinessConfig()); err != nil { + if err := lxd.WaitForInstanceCommand(ctx, d.client, d.instanceName(), command, d.options.BootWait.readinessConfig()); err != nil { return helpers.WrapError(fmt.Sprintf("error waiting for instance to be ready: %v", err)) } return nil @@ -688,7 +715,7 @@ func (d *LxdNode) waitForReady() error { } // Wait for the instance to be fully ready (OS booted, networking available) - if err := lxd.WaitForInstanceReady(ctx, d.client, d.name, nil); err != nil { + if err := lxd.WaitForInstanceReady(ctx, d.client, d.instanceName(), nil); err != nil { return helpers.WrapError(fmt.Sprintf("error waiting for instance to be ready: %v", err)) } @@ -712,9 +739,9 @@ func (d *LxdNode) ejectAfterPoweroff(ctx context.Context) error { case <-waitCtx.Done(): return helpers.WrapError(fmt.Sprintf( "timeout waiting for instance %s to power off before ejecting %v: %v", - d.name, d.options.BootWait.EjectOnPoweroff, waitCtx.Err())) + d.instanceName(), d.options.BootWait.EjectOnPoweroff, waitCtx.Err())) case <-ticker.C: - state, _, err := d.client.GetInstanceState(d.name) + state, _, err := d.client.GetInstanceState(d.instanceName()) if err != nil { continue } @@ -723,32 +750,32 @@ func (d *LxdNode) ejectAfterPoweroff(ctx context.Context) error { } // Detach the install media - inst, etag, err := d.client.GetInstance(d.name) + inst, etag, err := d.client.GetInstance(d.instanceName()) if err != nil { - return helpers.WrapError(fmt.Sprintf("error getting instance %s to eject devices: %v", d.name, err)) + return helpers.WrapError(fmt.Sprintf("error getting instance %s to eject devices: %v", d.instanceName(), err)) } for _, dev := range d.options.BootWait.EjectOnPoweroff { if _, ok := inst.Devices[dev]; !ok { return helpers.WrapError(fmt.Sprintf( - "device %q in eject_on_poweroff not found on instance %s", dev, d.name)) + "device %q in eject_on_poweroff not found on instance %s", dev, d.instanceName())) } delete(inst.Devices, dev) } - op, err := d.client.UpdateInstance(d.name, inst.Writable(), etag) + op, err := d.client.UpdateInstance(d.instanceName(), inst.Writable(), etag) if err != nil { - return helpers.WrapError(fmt.Sprintf("error ejecting devices from instance %s: %v", d.name, err)) + return helpers.WrapError(fmt.Sprintf("error ejecting devices from instance %s: %v", d.instanceName(), err)) } if err := op.Wait(); err != nil { - return helpers.WrapError(fmt.Sprintf("error ejecting devices from instance %s: %v", d.name, err)) + return helpers.WrapError(fmt.Sprintf("error ejecting devices from instance %s: %v", d.instanceName(), err)) } // Boot from the installed disk - op, err = d.client.UpdateInstanceState(d.name, api.InstanceStatePut{Action: "start", Timeout: -1}, "") + op, err = d.client.UpdateInstanceState(d.instanceName(), api.InstanceStatePut{Action: "start", Timeout: -1}, "") if err != nil { - return helpers.WrapError(fmt.Sprintf("error restarting instance %s after eject: %v", d.name, err)) + return helpers.WrapError(fmt.Sprintf("error restarting instance %s after eject: %v", d.instanceName(), err)) } if err := op.Wait(); err != nil { - return helpers.WrapError(fmt.Sprintf("error restarting instance %s after eject: %v", d.name, err)) + return helpers.WrapError(fmt.Sprintf("error restarting instance %s after eject: %v", d.instanceName(), err)) } return nil } @@ -770,7 +797,7 @@ func (d *LxdNode) Teardown() error { // An instance may already be stopped, for example a VM that powered itself off at // the end of an unattended install, and stopping it again is an error - state, _, err := d.client.GetInstanceState(d.name) + state, _, err := d.client.GetInstanceState(d.instanceName()) if err != nil { // Already-removed instances (e.g. a suite teardown step deleted it as a // safety net) leave nothing to tear down @@ -788,7 +815,7 @@ func (d *LxdNode) Teardown() error { Timeout: -1, Force: true, } - op, err = d.client.UpdateInstanceState(d.name, req, "") + op, err = d.client.UpdateInstanceState(d.instanceName(), req, "") if err != nil { return helpers.WrapError(fmt.Sprintf("error stopping instance: %v", err)) } @@ -798,7 +825,7 @@ func (d *LxdNode) Teardown() error { } // Create a delete request - op, err = d.client.DeleteInstance(d.name) + op, err = d.client.DeleteInstance(d.instanceName()) if err != nil { return helpers.WrapError(fmt.Sprintf("error deleting instance: %v", err)) } @@ -818,8 +845,8 @@ func (d *LxdNode) Execute(command string, options ...execution.ExecutionOption) debugEnabled := execution.IsDebugMode() // Create TeeWriters that optionally stream to console - stdoutWriter := stream.NewTeeWriter(stream.StreamStdout, d.name, debugEnabled) - stderrWriter := stream.NewTeeWriter(stream.StreamStderr, d.name, debugEnabled) + stdoutWriter := stream.NewTeeWriter(stream.StreamStdout, d.instanceName(), debugEnabled) + stderrWriter := stream.NewTeeWriter(stream.StreamStderr, d.instanceName(), debugEnabled) execArgs := lxdclient.InstanceExecArgs{ Stdout: stdoutWriter, @@ -833,7 +860,7 @@ func (d *LxdNode) Execute(command string, options ...execution.ExecutionOption) Interactive: false, } - op, err := d.client.ExecInstance(d.name, execPost, &execArgs) + op, err := d.client.ExecInstance(d.instanceName(), execPost, &execArgs) if err != nil { return nil, helpers.WrapError(fmt.Sprintf("error executing command: %v", err)) } diff --git a/pkg/nodetypes/lxd_devices_test.go b/pkg/nodetypes/lxd_devices_test.go index 9d4421a..16b1b63 100644 --- a/pkg/nodetypes/lxd_devices_test.go +++ b/pkg/nodetypes/lxd_devices_test.go @@ -42,7 +42,7 @@ func TestBuildDevices(t *testing.T) { }, } - built, err := buildDevices(devices, true) + built, err := buildDevices(devices, true, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -97,7 +97,7 @@ func TestBuildDevicesSourceHandling(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - built, err := buildDevices(map[string]map[string]interface{}{"dev": tt.device}, tt.resolvePaths) + built, err := buildDevices(map[string]map[string]interface{}{"dev": tt.device}, tt.resolvePaths, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -111,7 +111,7 @@ func TestBuildDevicesSourceHandling(t *testing.T) { func TestBuildDevicesRequiresType(t *testing.T) { _, err := buildDevices(map[string]map[string]interface{}{ "iso": {"source": "/srv/images/boot.iso"}, - }, true) + }, true, "") if err == nil { t.Fatal("expected an error for a device without a type") @@ -148,7 +148,7 @@ func TestLxdNodeEmptyWithImageIsRejected(t *testing.T) { "instance_type": "virtual-machine", } - _, err := NewLxdNode("test-node", ifaces.NodeOptions(&opts)) + _, err := NewLxdNode("test-node", ifaces.NodeOptions(&opts), "") if err == nil { t.Fatal("expected an error when both empty and image are set") } diff --git a/pkg/nodetypes/lxd_test.go b/pkg/nodetypes/lxd_test.go index fca1014..bd68ed4 100644 --- a/pkg/nodetypes/lxd_test.go +++ b/pkg/nodetypes/lxd_test.go @@ -93,7 +93,7 @@ func TestLxdNodeRemoteValidation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := NewLxdNode("test-node", ifaces.NodeOptions(&tt.opts)) + _, err := NewLxdNode("test-node", ifaces.NodeOptions(&tt.opts), "") if tt.shouldError { if err == nil { @@ -165,7 +165,7 @@ func TestLxdNodeSocketOption(t *testing.T) { // We expect the NewLxdNode to fail when trying to connect to the socket // but we can verify the option was parsed correctly by checking the error // doesn't indicate a validation issue - _, err := NewLxdNode("test-node", ifaces.NodeOptions(&tt.opts)) + _, err := NewLxdNode("test-node", ifaces.NodeOptions(&tt.opts), "") // We expect a connection error, not a validation error if err != nil { diff --git a/pkg/nodetypes/lxd_token_test.go b/pkg/nodetypes/lxd_token_test.go index 0c464bd..fe1c91e 100644 --- a/pkg/nodetypes/lxd_token_test.go +++ b/pkg/nodetypes/lxd_token_test.go @@ -139,7 +139,7 @@ func TestLxdNodeRejectsUnusableTrustTokens(t *testing.T) { "image": "ubuntu:24.04", } - _, err := NewLxdNode("test-node", ifaces.NodeOptions(&opts)) + _, err := NewLxdNode("test-node", ifaces.NodeOptions(&opts), "") if err == nil { t.Fatal("expected an error") } diff --git a/pkg/nodetypes/naming_test.go b/pkg/nodetypes/naming_test.go new file mode 100644 index 0000000..fd51448 --- /dev/null +++ b/pkg/nodetypes/naming_test.go @@ -0,0 +1,96 @@ +package nodetypes + +import ( + "testing" + + "github.com/bgrewell/dart/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The platform object's name defaults to the node name, so a suite's +// containers and instances are findable by the name the YAML uses. +func TestPlatformNamesDefaultToNodeName(t *testing.T) { + docker := &DockerNode{name: "web"} + assert.Equal(t, "web", docker.containerName()) + + lxd := &LxdNode{name: "db"} + assert.Equal(t, "db", lxd.instanceName()) +} + +// An explicit name decouples the platform identifier from node identity, +// for suites that must match an externally fixed name. +func TestPlatformNamesCanBeOverridden(t *testing.T) { + docker := &DockerNode{ + name: "web", + options: DockerNodeOpts{ContainerName: "acme-web-01"}, + } + assert.Equal(t, "acme-web-01", docker.containerName()) + + lxd := &LxdNode{ + name: "db", + options: LxdNodeOpts{InstanceName: "acme-db-01"}, + } + assert.Equal(t, "acme-db-01", lxd.instanceName()) +} + +// command and entrypoint are what let an image whose default command exits +// immediately — a bare distribution image — host a node at all. +func TestDockerCommandAndEntrypointAccepted(t *testing.T) { + cfg := &config.NodeConfig{ + Name: "shellbox", + Type: "docker", + Options: map[string]interface{}{ + "image": "ubuntu:24.04", + "command": []interface{}{"sleep", "infinity"}, + "entrypoint": []interface{}{"/bin/sh", "-c"}, + }, + } + require.NoError(t, ValidateNodeOptions(cfg)) + + var opts DockerNodeOpts + require.NoError(t, decodeNodeOptions(cfg.Options, &opts)) + assert.Equal(t, []string{"sleep", "infinity"}, opts.Command) + assert.Equal(t, []string{"/bin/sh", "-c"}, opts.Entrypoint) +} + +// A misspelled option previously decoded to nothing and read as configured. +func TestUnknownNodeOptionIsRejected(t *testing.T) { + err := ValidateNodeOptions(&config.NodeConfig{ + Name: "web", Type: "docker", + Options: map[string]interface{}{"image": "nginx", "entrypoints": []interface{}{"/bin/sh"}}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), `unknown option "entrypoints"`) + // The message names what the type does accept, including the spelling + // the author meant + assert.Contains(t, err.Error(), "entrypoint") + + // A key that is valid on another node type is still wrong here + err = ValidateNodeOptions(&config.NodeConfig{ + Name: "web", Type: "ssh", + Options: map[string]interface{}{"host": "h", "pass": "p", "image": "nginx", "insecure_skip_host_key": true}, + }) + assert.ErrorContains(t, err, `unknown option "image"`) +} + +// The cross-node constraints must be reachable without constructing nodes, +// which is what lets --check report them. +func TestValidateNodeSet(t *testing.T) { + err := ValidateNodeSet([]*config.NodeConfig{ + {Name: "a", Type: "local"}, + {Name: "b", Type: "local"}, + }) + assert.ErrorContains(t, err, "only one local node allowed") + + err = ValidateNodeSet([]*config.NodeConfig{ + {Name: "a", Type: "docker"}, + {Name: "a", Type: "docker"}, + }) + assert.ErrorContains(t, err, `duplicate node name "a"`) + + assert.NoError(t, ValidateNodeSet([]*config.NodeConfig{ + {Name: "a", Type: "local"}, + {Name: "b", Type: "docker"}, + })) +} diff --git a/pkg/nodetypes/ssh.go b/pkg/nodetypes/ssh.go index 4b34da2..be7fd53 100644 --- a/pkg/nodetypes/ssh.go +++ b/pkg/nodetypes/ssh.go @@ -8,6 +8,7 @@ import ( "strings" "time" + dartconfig "github.com/bgrewell/dart/internal/config" "github.com/bgrewell/dart/internal/execution" "github.com/bgrewell/dart/internal/helpers" "github.com/bgrewell/dart/internal/stream" @@ -55,7 +56,7 @@ type SshBastionOpts struct { Bastion *SshBastionOpts `yaml:"bastion,omitempty" json:"bastion"` } -func NewSshNode(name string, opts ifaces.NodeOptions) (node ifaces.Node, err error) { +func NewSshNode(name string, opts ifaces.NodeOptions, suiteDir string) (node ifaces.Node, err error) { jsonData, err := json.Marshal(opts) if err != nil { @@ -74,6 +75,23 @@ func NewSshNode(name string, opts ifaces.NodeOptions) (node ifaces.Node, err err addr := fmt.Sprintf("%s:%d", nodeopts.Host, nodeopts.Port) + // Credential paths belong to the machine running DART, so they follow + // the suite-relative rule like every other local path + if nodeopts.KeyFile, err = dartconfig.ResolveLocalPath(suiteDir, nodeopts.KeyFile); err != nil { + return nil, err + } + if nodeopts.KnownHosts, err = dartconfig.ResolveLocalPath(suiteDir, nodeopts.KnownHosts); err != nil { + return nil, err + } + if nodeopts.Bastion != nil { + if nodeopts.Bastion.KeyFile, err = dartconfig.ResolveLocalPath(suiteDir, nodeopts.Bastion.KeyFile); err != nil { + return nil, err + } + if nodeopts.Bastion.KnownHosts, err = dartconfig.ResolveLocalPath(suiteDir, nodeopts.Bastion.KnownHosts); err != nil { + return nil, err + } + } + authMethods, err := sshAuthMethods(nodeopts.KeyFile, nodeopts.Pass) if err != nil { return nil, err diff --git a/pkg/nodetypes/ssh_test.go b/pkg/nodetypes/ssh_test.go index 650b464..e03758d 100644 --- a/pkg/nodetypes/ssh_test.go +++ b/pkg/nodetypes/ssh_test.go @@ -155,7 +155,7 @@ func testSSHNode(t *testing.T) ifaces.Node { "pass": "testpass", "insecure_skip_host_key": true, } - node, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts)) + node, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts), "") require.NoError(t, err) t.Cleanup(func() { node.Close() }) return node @@ -212,7 +212,7 @@ func TestSSHBadCredentials(t *testing.T) { "pass": "wrong", "insecure_skip_host_key": true, } - _, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts)) + _, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts), "") assert.Error(t, err) } @@ -227,7 +227,7 @@ func TestSSHUnknownHostKeyRefused(t *testing.T) { "host": host, "port": port, "user": "testuser", "pass": "testpass", "known_hosts": emptyKnownHosts, } - _, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts)) + _, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts), "") require.Error(t, err) assert.Contains(t, err.Error(), "knownhosts") } @@ -243,7 +243,7 @@ func TestSSHKnownHostAccepted(t *testing.T) { "host": host, "port": port, "user": "testuser", "pass": "testpass", "known_hosts": knownHostsPath, } - node, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts)) + node, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts), "") require.NoError(t, err) defer node.Close() @@ -260,7 +260,7 @@ func TestSSHMissingKnownHostsExplains(t *testing.T) { "host": host, "port": port, "user": "testuser", "pass": "testpass", "known_hosts": filepath.Join(t.TempDir(), "absent"), } - _, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts)) + _, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts), "") require.Error(t, err) assert.Contains(t, err.Error(), "insecure_skip_host_key") } @@ -271,7 +271,7 @@ func TestSSHNoCredentials(t *testing.T) { "host": host, "port": port, "user": "testuser", "insecure_skip_host_key": true, } - _, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts)) + _, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts), "") require.Error(t, err) assert.Contains(t, err.Error(), "no ssh credentials") } @@ -291,7 +291,7 @@ func TestSSHThroughBastion(t *testing.T) { "user": "testuser", "pass": "testpass", }, } - node, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts)) + node, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts), "") require.NoError(t, err) defer node.Close() @@ -308,7 +308,7 @@ func TestSSHBastionValidation(t *testing.T) { "insecure_skip_host_key": true, "bastion": map[string]interface{}{"user": "testuser", "pass": "x"}, } - _, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts)) + _, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts), "") require.Error(t, err) assert.Contains(t, err.Error(), "bastion host is required") } @@ -321,7 +321,7 @@ func TestSSHRebootAfterClosedClientDoesNotPanic(t *testing.T) { "host": host, "port": port, "user": "testuser", "pass": "testpass", "insecure_skip_host_key": true, } - node, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts)) + node, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts), "") require.NoError(t, err) sshNode := node.(*SshNode) @@ -350,7 +350,7 @@ func TestSSHHostKeyErrorsNameTheNodeAndOptions(t *testing.T) { "host": host, "port": port, "user": "testuser", "pass": "testpass", "known_hosts": emptyKnownHosts, } - _, err := NewSshNode("prod-web", ifaces.NodeOptions(&opts)) + _, err := NewSshNode("prod-web", ifaces.NodeOptions(&opts), "") require.Error(t, err) assert.Contains(t, err.Error(), "prod-web", "the failing node must be named") assert.Contains(t, err.Error(), "insecure_skip_host_key", "the way out must be named") @@ -370,7 +370,7 @@ func TestSSHHostKeyMismatchExplains(t *testing.T) { "host": host, "port": port, "user": "testuser", "pass": "testpass", "known_hosts": knownHostsPath, } - _, err := NewSshNode("prod-web", ifaces.NodeOptions(&opts)) + _, err := NewSshNode("prod-web", ifaces.NodeOptions(&opts), "") require.Error(t, err) assert.Contains(t, err.Error(), "does not match known_hosts") assert.Contains(t, err.Error(), "intercepted") @@ -386,7 +386,7 @@ func TestSSHChainedBastionRejected(t *testing.T) { "bastion": map[string]interface{}{"host": "jump2", "user": "u", "pass": "p"}, }, } - _, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts)) + _, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts), "") require.Error(t, err) assert.Contains(t, err.Error(), "chained bastions are not supported") } @@ -407,7 +407,7 @@ func TestSSHBastionKeepsOwnHostKeyPolicy(t *testing.T) { "known_hosts": emptyKnownHosts, "insecure_skip_host_key": insecure, }, } - _, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts)) + _, err := NewSshNode("ssh-test", ifaces.NodeOptions(&opts), "") require.Error(t, err, "the bastion must still verify even though the target does not") assert.Contains(t, err.Error(), "bastion") } diff --git a/pkg/steptypes/file_transfer.go b/pkg/steptypes/file_transfer.go index 6e92e19..6b18591 100644 --- a/pkg/steptypes/file_transfer.go +++ b/pkg/steptypes/file_transfer.go @@ -27,11 +27,25 @@ type FilePushStep struct { createDir bool } +// localPath resolves a path the suite wrote for the machine running DART. +// Relative paths are relative to the suite file, so a suite behaves the same +// regardless of the directory DART is invoked from. +func localPath(c *config.StepConfig, raw string) (string, error) { + resolved, err := config.ResolveLocalPath(c.SuiteDir, raw) + if err != nil { + return "", optionError(c, "%v in step %q", err, c.Name) + } + return resolved, nil +} + func newFilePushStep(c *config.StepConfig, node ifaces.Node) (ifaces.Step, error) { source, err := requiredString(c, "source", "source is required") if err != nil { return nil, err } + if source, err = localPath(c, source); err != nil { + return nil, err + } dest, err := requiredString(c, "dest", "dest is required") if err != nil { return nil, err @@ -109,6 +123,9 @@ func newFileFetchStep(c *config.StepConfig, node ifaces.Node) (ifaces.Step, erro if err != nil { return nil, err } + if dest, err = localPath(c, dest); err != nil { + return nil, err + } overwrite, err := optBool(c, "overwrite") if err != nil { return nil, err @@ -191,6 +208,9 @@ func newFileTemplateStep(c *config.StepConfig, node ifaces.Node) (ifaces.Step, e if err != nil { return nil, err } + if source, err = localPath(c, source); err != nil { + return nil, err + } dest, err := requiredString(c, "dest", "dest is required") if err != nil { return nil, err