conductor

CI task system
git clone git://git.finwo.net/app/conductor
Log | Files | Refs | README | LICENSE

commit 8ea56af54c3655289ae5291da79ad56ec139f9c3
parent f889b480e21d02da07d3d7be16820db460bf2522
Author: finwo <finwo@pm.me>
Date:   Sat, 19 Sep 2026 20:24:54 +0200

Run jobs without sharing anything with the worker

Diffstat:
M.conductor.yml | 9---------
Mdeploy/Dockerfile.worker | 21++++++++++-----------
Mdeploy/docker-compose.yml | 12+++---------
Adeploy/hub/conductor-worker.md | 107+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Adeploy/hub/conductor.md | 101+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Mdeploy/publish.sh | 63+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Mdeploy/smoke.sh | 12+++---------
Mdeploy/worker/docker-compose.yml | 10++++------
Mdocs/deployment.md | 29+++++++++++++++++++----------
Mdocs/pipeline.md | 31++++++++++++++++++++-----------
Mdocs/worker.md | 45+++++++++++++++++++++++++--------------------
Mexamples/node-app.conductor.yml | 3---
Mexamples/worker.json | 2--
Amigrations/mysql/005_drop_source_mode.sql | 5+++++
Amigrations/mysql/006_project_workdir.sql | 5+++++
Amigrations/postgres/005_drop_source_mode.sql | 5+++++
Amigrations/postgres/006_project_workdir.sql | 5+++++
Amigrations/sqlite/005_drop_source_mode.sql | 18++++++++++++++++++
Amigrations/sqlite/006_project_workdir.sql | 16++++++++++++++++
Msrc/admin-cli.js | 3---
Msrc/conductor/routes/manage.js | 14++++++++------
Msrc/conductor/routes/workers.js | 17++++++++++-------
Msrc/conductor/scheduler.js | 15++++++++++++---
Msrc/conductor/ui/pages.js | 12++++++++++--
Msrc/conductor/ui/routes.js | 11+++++++++--
Msrc/lib/pipeline/expand.js | 6------
Msrc/lib/pipeline/index.js | 3++-
Msrc/lib/pipeline/parse.js | 75+++++++++++++++++++++++++++++++++++++++++++++------------------------------
Msrc/lib/projects.js | 34++++++++++++++++++++++++++--------
Msrc/worker/agent.js | 12+++++-------
Msrc/worker/artifacts.js | 3---
Msrc/worker/config.js | 6------
Msrc/worker/docker.js | 86++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
Msrc/worker/job.js | 180+++++++++++++++++++++++++++++++++++++++++++++++--------------------------------
Msrc/worker/script.js | 14++++++++------
Msrc/worker/source.js | 88++++++++++++++++++-------------------------------------------------------------
Mtest/conductor.test.js | 113+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
Mtest/helpers/harness.js | 2+-
Mtest/ui.test.js | 52++++++++++++++++++++++++++++++++++++++++++++++++++++
Mtest/worker-docker.test.js | 96+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
Mtest/worker.test.js | 87+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------
41 files changed, 1067 insertions(+), 361 deletions(-)

diff --git a/.conductor.yml b/.conductor.yml @@ -20,9 +20,6 @@ jobs: script: - npm install --no-audit --no-fund - node --test test/ascii.test.js - cache: - key: npm - paths: [.npm] # Everything that runs without a docker socket. test: @@ -31,9 +28,6 @@ jobs: - npm test env: CONDUCTOR_TEST_NO_DOCKER: '1' - cache: - key: npm - paths: [.npm] # The same suite with the container-backed tests switched on, so the S3 # and OIDC paths are exercised against real servers rather than skipped. @@ -42,9 +36,6 @@ jobs: script: - npm install --no-audit --no-fund - npm test - cache: - key: npm - paths: [.npm] # Both images have to build before anything claims a release is possible. # Host architecture only, since this runs on every branch and the point diff --git a/deploy/Dockerfile.worker b/deploy/Dockerfile.worker @@ -10,14 +10,13 @@ # a tarball off download.docker.com, which has no riscv64 build at all. # # The worker runs each job in its own container, so it needs a docker -# socket. It does not run a daemon of its own: the containers it starts are -# siblings on the host, not children. +# socket. It does not run a daemon of its own: the containers it starts +# are siblings on the host, not children. # -# That has one consequence worth understanding. Paths in the -v flags the -# worker passes are resolved by the host daemon, not inside this container, -# so the workspace directory has to exist at the same path in both. The -# compose file in deploy/worker does that; if you run this by hand, mount -# the workspace at the path you configure rather than somewhere convenient. +# It shares nothing else with them. A job's tree is unpacked into its +# container over the docker API rather than bind mounted from here, and +# the results are read back the same way, so no directory has to mean the +# same thing on both sides and this image needs no volumes at all. ARG ALPINE_VERSION=3.23 @@ -35,10 +34,10 @@ RUN npm install --omit=dev --no-audit --no-fund yaml \ FROM alpine:${ALPINE_VERSION} -# docker-cli to start job containers on the host daemon, git for projects -# configured to clone rather than download an archive, and GNU tar because -# the busybox one is not a full substitute for reading source archives. -RUN apk add --no-cache ca-certificates docker-cli git nodejs tar +# docker-cli to drive the host daemon, and node to run the agent. No git, +# because the worker never touches a repository, and no tar, because +# docker does the unpacking on the far side of the socket. +RUN apk add --no-cache ca-certificates docker-cli nodejs WORKDIR /app diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml @@ -54,17 +54,11 @@ services: CONDUCTOR_WORKER_TOKEN: "${CONDUCTOR_WORKER_TOKEN:?set CONDUCTOR_WORKER_TOKEN, see docs/deployment.md}" CONDUCTOR_WORKER_ARCHES: "${CONDUCTOR_WORKER_ARCHES:-}" CONDUCTOR_WORKER_CONCURRENCY: "${CONDUCTOR_WORKER_CONCURRENCY:-2}" - # The path has to be identical inside and outside this container. - CONDUCTOR_WORKER_WORKSPACE: /var/lib/conductor/work - CONDUCTOR_WORKER_CACHE: /var/lib/conductor/cache volumes: - # Jobs run as sibling containers on the host daemon, so the bind - # mounts the worker asks for are resolved by the host. The workspace - # therefore has to be a host path mounted at the same location here, - # not a named volume. + # The socket, and nothing else. A job's tree is unpacked into its + # own container over this socket rather than shared from here, so + # the worker keeps no state and needs no volume of its own. - /var/run/docker.sock:/var/run/docker.sock - - /var/lib/conductor/work:/var/lib/conductor/work - - /var/lib/conductor/cache:/var/lib/conductor/cache postgres: profiles: ["postgres"] diff --git a/deploy/hub/conductor-worker.md b/deploy/hub/conductor-worker.md @@ -0,0 +1,107 @@ +# conductor-worker + +Runs jobs for a [conductor](https://hub.docker.com/r/finwo/conductor) +server. Polls for work, runs each job in its own container, streams the +log back and uploads artifacts. + +Source and issues: https://github.com/finwo/conductor + +## Tags + +| Tag | What it is | +| ---------------- | ------------------------------------------ | +| `latest` | the most recent release | +| `0.1.0` | an exact version | +| `0.1` | the latest patch of that minor series | +| `<commit>` | an exact build from main, twelve hex chars | + +Built for `linux/amd64`, `linux/arm64` and `linux/riscv64`. + +## Quick start + +A worker needs no inbound connectivity, so this works from behind NAT. +Mint a token in the interface under Workers, or with the admin CLI on the +server, then: + +```sh +docker run -d \ + --name conductor-worker \ + --restart unless-stopped \ + -e CONDUCTOR_URL=https://ci.example.com \ + -e CONDUCTOR_WORKER_TOKEN=... \ + -v /var/run/docker.sock:/var/run/docker.sock \ + finwo/conductor-worker +``` + +A worker you register is only ever offered jobs from your own projects. + +## It keeps nothing + +The socket is the only thing this image needs. A job's tree is unpacked +into the job's own container over that socket, and its artifacts are read +back the same way, so the worker shares no directory with what it runs. + +That means no volumes, nothing to persist between restarts, and no paths +that have to mean the same thing inside and outside the container. The +job containers are siblings on the host daemon rather than children, but +since nothing of the worker's filesystem is handed to them, that stops +being something you have to think about. + +Where a job runs is decided by the build, not by this machine: the +`workdir` key in the pipeline, the project's setting, or `/work`. + +## Configuration + +| Variable | Default | What it does | +| ------------------------------ | -------------- | ----------------------------------- | +| `CONDUCTOR_URL` | required | Where the conductor is. | +| `CONDUCTOR_WORKER_TOKEN` | required | Issued by the conductor, shown once. | +| `CONDUCTOR_WORKER_NAME` | the hostname | How this worker appears in runs. | +| `CONDUCTOR_WORKER_CONCURRENCY` | `1` | Jobs at once. | +| `CONDUCTOR_WORKER_ARCHES` | any | Architectures offered, comma separated. | +| `CONDUCTOR_WORKER_TOKEN_FILE` | none | Read the token from a file instead. | +| `CONDUCTOR_WORKER_DOCKER` | `docker` | Runtime CLI: docker, podman, nerdctl. | +| `CONDUCTOR_WORKER_POLL_INTERVAL` | `5` | Seconds between polls when idle. | + +Leaving `CONDUCTOR_WORKER_ARCHES` unset offers every architecture, which +is what you want unless one worker serves several. + +## Features + +A feature is a name this worker advertises, together with the local +resources a job asking for it should be given. A pipeline job asks with +`requires:`, and the conductor will not send that job to a worker which +does not advertise every name it lists. The conductor only ever learns +the names. + +Features cannot be expressed in an environment variable, because each one +carries mounts, devices and environment of its own. They need a +configuration file, JSON or YAML, mounted into the container: + +```json +{ + "conductor_url": "https://ci.example.com", + "features": { + "docker": {}, + "sign-key": { + "mounts": ["/srv/keys/build.rsa:/keys/build.rsa:ro"], + "env": { "SIGN_KEY": "/keys/build.rsa" } + } + } +} +``` + +```sh +-v /etc/conductor/worker.json:/etc/conductor/worker.json:ro +``` + +A file at that path is picked up on its own, with the token still coming +from the environment if you would rather keep it out of the file. +Nothing enforces what a name means beyond matching it, so `docker` is +simply how you say this worker's jobs may use the socket mounted above. + +## Documentation + +- [Workers](https://github.com/finwo/conductor/blob/main/docs/worker.md) +- [Pipelines](https://github.com/finwo/conductor/blob/main/docs/pipeline.md) +- [Deployment](https://github.com/finwo/conductor/blob/main/docs/deployment.md) diff --git a/deploy/hub/conductor.md b/deploy/hub/conductor.md @@ -0,0 +1,101 @@ +# conductor + +A small CI server. Accepts triggers, schedules runs, serves the worker +API and renders the interface. One service, one image. + +Source and issues: https://github.com/finwo/conductor + +## Tags + +| Tag | What it is | +| ---------------- | ------------------------------------------ | +| `latest` | the most recent release | +| `0.1.0` | an exact version | +| `0.1` | the latest patch of that minor series | +| `<commit>` | an exact build from main, twelve hex chars | + +Built for `linux/amd64`, `linux/arm64` and `linux/riscv64`. + +## Quick start + +```sh +docker run -d \ + --name conductor \ + -p 8080:8080 \ + -v conductor-data:/data \ + -e CONDUCTOR_PUBLIC_URL=http://localhost:8080 \ + finwo/conductor +``` + +The administrator password is printed once in the log on first start. +Set `CONDUCTOR_ADMIN_PASSWORD` to choose it instead. + +Jobs need a worker, which is a separate image: see +[finwo/conductor-worker](https://hub.docker.com/r/finwo/conductor-worker). +A compose file running both is in the repository under `deploy/`. + +## State + +Everything lives in `/data`, so mount a volume there: + +``` +/data/conductor.db sqlite, unless a database url is configured +/data/mirrors one bare git mirror per project +/data/logs the live log spool +/data/storage artifacts and archived logs, unless S3 is configured +``` + +## Configuration + +Every setting has a default and can come from the environment, so no +configuration file is needed. Worth setting in production: + +| Variable | Why | +| -------------------------- | ------------------------------------------------ | +| `CONDUCTOR_PUBLIC_URL` | Workers get absolute callback URLs built from it. | +| `CONDUCTOR_SESSION_SECRET` | Otherwise sessions end at every restart. | +| `CONDUCTOR_SECRET_KEY` | Otherwise stored secrets are kept in the clear. | +| `CONDUCTOR_ADMIN_PASSWORD` | Otherwise one is generated and logged once. | + +Generate the two secrets with `openssl rand -hex 32`. + +Postgres or MySQL instead of sqlite: + +```sh +-e CONDUCTOR_DATABASE_URL=postgres://conductor:secret@postgres:5432/conductor +``` + +S3 compatible storage instead of the volume: + +```sh +-e CONDUCTOR_S3_ENDPOINT=http://minio:9000 \ +-e CONDUCTOR_S3_BUCKET=conductor \ +-e CONDUCTOR_S3_ACCESS_KEY_ID=... \ +-e CONDUCTOR_S3_SECRET_ACCESS_KEY=... +``` + +Artifacts and finished logs then go to the bucket, and downloads are +handed over with a presigned redirect rather than proxied. + +## Retention + +Build output is deleted on a schedule, or a busy server fills its disk +with logs nobody will read. The defaults keep artifacts for 30 days or +the last 10 runs, and logs for 14 days, and a project may override any of +it. Upgrading an installation that has been running for a while will +delete a lot on the first sweep, so set these before starting if that +matters: + +```sh +-e CONDUCTOR_RETENTION_ARTIFACT_DAYS=0 \ +-e CONDUCTOR_RETENTION_LOG_DAYS=0 +``` + +Zero means keep forever. + +## Documentation + +- [Deployment](https://github.com/finwo/conductor/blob/main/docs/deployment.md) +- [Pipelines](https://github.com/finwo/conductor/blob/main/docs/pipeline.md) +- [Workers](https://github.com/finwo/conductor/blob/main/docs/worker.md) +- [HTTP API](https://github.com/finwo/conductor/blob/main/docs/api.md) diff --git a/deploy/publish.sh b/deploy/publish.sh @@ -193,6 +193,69 @@ docker buildx build \ --push \ "${ROOT}" +# Pushing an image does not touch the repository page, so the readme is +# sent separately or Docker Hub shows nothing at all. Kept next to the +# Dockerfiles so it is reviewed with them rather than edited in a web +# form and forgotten. +# +# Needs an API token rather than the registry session, so this is skipped +# when the script is run against an existing docker login. The release +# itself is done by then, and a stale readme is not worth failing it for. +sync_readme() { + image=$1 + file=$2 + repo=${image#*/} + namespace=${image%%/*} + + [ -f "${file}" ] || { log "no readme at ${file}, skipping"; return 0; } + + if [ -z "${REGISTRY_TOKEN:-}" ]; then + log "no REGISTRY_TOKEN, leaving the ${image} readme alone" + return 0 + fi + + if ! command -v curl >/dev/null 2>&1; then + log "curl is missing, leaving the ${image} readme alone" + return 0 + fi + + api=$(curl -fsS -X POST https://hub.docker.com/v2/auth/token \ + -H 'Content-Type: application/json' \ + -d "{\"identifier\":\"${REGISTRY_USERNAME}\",\"secret\":\"${REGISTRY_TOKEN}\"}" \ + | sed -n 's/.*"access_token":"\([^"]*\)".*/\1/p') + + [ -n "${api}" ] || { log "could not get a hub api token, leaving the readme alone"; return 0; } + + # The body is json, so the markdown has to be escaped rather than + # pasted: it is full of quotes, backslashes and newlines. + payload=$(FILE="${file}" awk ' + BEGIN { printf "{\"full_description\":\"" } + { + line = $0 + gsub(/\\/, "\\\\", line) + gsub(/"/, "\\\"", line) + gsub(/\t/, "\\t", line) + printf "%s\\n", line + } + END { printf "\"}" } + ' "${file}") + + if printf '%s' "${payload}" | curl -fsS -X PATCH \ + "https://hub.docker.com/v2/repositories/${namespace}/${repo}/" \ + -H 'Content-Type: application/json' \ + -H "Authorization: Bearer ${api}" \ + --data-binary @- >/dev/null + then + log "updated the ${image} readme" + else + log "could not update the ${image} readme" + fi +} + +log "updating the repository pages" +sync_readme "${CONDUCTOR_IMAGE}" "${ROOT}/deploy/hub/conductor.md" +sync_readme "${WORKER_IMAGE}" "${ROOT}/deploy/hub/conductor-worker.md" + log "verifying the manifests" for image in "${CONDUCTOR_IMAGE}" "${WORKER_IMAGE}"; do first=$(printf '%s' "${TAGS}" | head -1) diff --git a/deploy/smoke.sh b/deploy/smoke.sh @@ -23,9 +23,7 @@ PORT="${SMOKE_PORT:-18500}" PROJECT=conductor-smoke SECRET=smoke-trigger-secret -# Paths must match inside and outside the worker, since jobs run as -# siblings on the host daemon. -STATE=${SMOKE_STATE:-/tmp/conductor-smoke-state} + log() { printf '\n== %s\n' "$*"; } fail() { printf '\nFAILED: %s\n' "$*" >&2; exit 1; } @@ -39,7 +37,7 @@ cleanup() { fi printf '\n== cleaning up\n' SMOKE_TOKEN=unused docker compose -p "${PROJECT}" -f "${WORK}/compose.yml" down -v >/dev/null 2>&1 || true - rm -rf "${WORK}" "${STATE}" + rm -rf "${WORK}" } trap cleanup EXIT @@ -48,7 +46,7 @@ docker build -q -f "${ROOT}/deploy/Dockerfile" -t conductor:smoke "${ROOT}" >/de docker build -q -f "${ROOT}/deploy/Dockerfile.worker" -t conductor-worker:smoke "${ROOT}" >/dev/null log "preparing a repository" -mkdir -p "${WORK}/repo" "${STATE}/work" "${STATE}/cache" +mkdir -p "${WORK}/repo" cat > "${WORK}/repo/.conductor.yml" <<'PIPELINE' version: 1 visibility: public @@ -99,12 +97,8 @@ services: CONDUCTOR_WORKER_NAME: smoke-worker CONDUCTOR_WORKER_TOKEN: "\${SMOKE_TOKEN}" CONDUCTOR_WORKER_CONCURRENCY: "2" - CONDUCTOR_WORKER_WORKSPACE: ${STATE}/work - CONDUCTOR_WORKER_CACHE: ${STATE}/cache volumes: - /var/run/docker.sock:/var/run/docker.sock - - ${STATE}/work:${STATE}/work - - ${STATE}/cache:${STATE}/cache volumes: smoke-data: COMPOSE diff --git a/deploy/worker/docker-compose.yml b/deploy/worker/docker-compose.yml @@ -32,14 +32,12 @@ services: # jobs that declare none. CONDUCTOR_WORKER_ARCHES: "${CONDUCTOR_WORKER_ARCHES:-}" CONDUCTOR_WORKER_CONCURRENCY: "${CONDUCTOR_WORKER_CONCURRENCY:-1}" - # Identical inside and out, because the docker daemon resolves these - # paths on the host. - CONDUCTOR_WORKER_WORKSPACE: /var/lib/conductor/work - CONDUCTOR_WORKER_CACHE: /var/lib/conductor/cache volumes: + # The socket is all it needs. A job's tree is unpacked into the + # job's own container over this socket, and its artifacts are read + # back the same way, so the worker keeps nothing between jobs and + # shares no directory with what it runs. - /var/run/docker.sock:/var/run/docker.sock - - /var/lib/conductor/work:/var/lib/conductor/work - - /var/lib/conductor/cache:/var/lib/conductor/cache # Features are declared in a config file when a job needs something # from this machine, such as a signing key. See docs/worker.md. # - ./worker.json:/etc/conductor/worker.json:ro diff --git a/docs/deployment.md b/docs/deployment.md @@ -125,19 +125,28 @@ being proxied. The bucket must already exist. Workers ------- -A worker needs the docker socket, because it runs each job in a container. -Those containers are siblings on the host daemon, not children of the -worker, which has one consequence that catches people out: +A worker needs the docker socket, and nothing else: -**The workspace path must be identical inside and outside the worker -container.** The `-v` flags the worker passes are resolved by the host -daemon, so a path that only exists inside the worker produces an empty -workspace and a job that cannot find its own source. The compose files -mount `/var/lib/conductor/work` at the same path on both sides for exactly -this reason. A named volume will not do. +```sh +docker run -d --restart unless-stopped \ + -e CONDUCTOR_URL=https://ci.example.com \ + -e CONDUCTOR_WORKER_TOKEN=... \ + -v /var/run/docker.sock:/var/run/docker.sock \ + finwo/conductor-worker +``` + +It keeps no state and shares no directory with the jobs it runs. A job's +tree is unpacked into the job's own container over the socket, and its +artifacts are read back the same way, so there is no workspace to mount, +no path that has to mean the same thing on both sides, and nothing to +persist between restarts. + +Feature mounts are the one exception, and they are deliberate: those name +host paths an operator chose, such as a signing key, and are declared in +the worker's configuration file. See [worker.md](worker.md). To contribute capacity to somebody else's conductor, use -`deploy/worker/docker-compose.yml`; see [worker.md](worker.md). +`deploy/worker/docker-compose.yml`. Behind a reverse proxy ---------------------- diff --git a/docs/pipeline.md b/docs/pipeline.md @@ -15,6 +15,7 @@ Top level | ------------ | -------- | -------------------------------------------- | | `version` | yes | Must be `1`. | | `visibility` | no | `public` or `private`. Overrides the project. | +| `workdir` | no | Absolute path jobs run in. Overrides the project. | | `defaults` | no | Values inherited by every job. | | `jobs` | yes | Mapping of job name to job. | @@ -62,7 +63,6 @@ Jobs | `services` | list | `[]` | yes | | `env` | mapping | `{}` | merged | | `artifacts` | list or mapping | none | no | -| `cache` | list or mapping | none | yes | | `allow_failure` | boolean | `false` | yes | | `timeout` | duration | server default | yes | | `max_attempts` | integer, 1 to 10 | `1` | yes | @@ -283,18 +283,27 @@ flat and lifts the five gigabyte limit that applies to a single request. Disk images and other multi gigabyte output are fine; what they cost is storage, not resident memory. -Cache ------ +Working directory +----------------- ```yaml -cache: - key: deps-$MATRIX_PKG - paths: [.npm, vendor] +version: 1 +workdir: /usr/src/app ``` -A directory preserved between jobs on the same worker, keyed by `key`. A -cache is an optimisation, never a correctness guarantee: a job must work with -an empty one. +Where the tree is unpacked inside the job container, and the directory +every script starts in. It must be an absolute path of ordinary directory +names; it does not have to exist in the image, and is created as the tree +is unpacked. + +Three answers, most specific first: this key, the project's setting, and +failing both `/work`. The repository has the last word because the path +belongs with the code: an image that expects to build in `/usr/src/app` +knows that, and whoever registered the project should not have to. + +Nothing else is shared with the container. The tree arrives over the +docker API and artifacts leave the same way, so a worker needs no +directory in common with the jobs it runs, and no storage of its own. Environment ----------- @@ -313,8 +322,8 @@ Interpolation ------------- `${{ ... }}` is substituted before the job is stored, in `image`, `script`, -`env` values, `requires`, `services` and `cache`. Two expressions are -available: `arch` and `matrix.<name>`. +`env` values, `requires` and `services`. Two expressions are available: +`arch` and `matrix.<name>`. ```yaml build: diff --git a/docs/worker.md b/docs/worker.md @@ -6,16 +6,18 @@ inbound connectivity, so it can sit behind NAT, and it needs no repository credentials, because the conductor serves the source for the exact commit a job was handed. -The worker has no npm dependencies. Copying `src/worker` onto a host with -node, tar and a container runtime is enough. +The worker has no npm dependencies and keeps no state. Copying +`src/worker` onto a host with node and a container runtime is enough. Requirements ------------ - node 24 or newer - a docker compatible CLI: docker, podman or nerdctl - - `tar`, to unpack job sources - - `git`, only for projects configured to clone rather than download + +That is the whole list. A job's tree is unpacked into the job's own +container over the docker API, so the worker needs no `tar` of its own, +and it never touches a repository, so it needs no `git` either. Getting a token --------------- @@ -68,8 +70,10 @@ node src/worker/agent.js --config worker.json | `poll_interval` | `5` | Seconds between polls when idle. | | `docker` | `docker` | Runtime CLI. Set to `podman` or `nerdctl`. | | `shell` | `sh` | Shell used to run a job script in its image. | -| `workspace_root` | a temp directory | Where job workspaces are created. | -| `cache_root` | a temp directory | Where job caches are kept between runs. | + +There is nothing here about where jobs run or what they are given, which +is deliberate: that is a property of the build and is decided by the +pipeline or the project, not by the machine that happens to run it. Every key can also be set from the environment: `CONDUCTOR_URL`, `CONDUCTOR_WORKER_TOKEN`, `CONDUCTOR_WORKER_TOKEN_FILE`, @@ -114,11 +118,12 @@ between the job and the host. What a job gets --------------- -Each job runs in its own container, on its own network, in its own -workspace: +Each job runs in its own container, on its own network: - - the repository tree at the job's commit, unpacked into `/workspace` - - `/workspace` as the working directory + - the repository tree at the job's commit, unpacked into the working + directory, which is `/work` unless the pipeline or the project says + otherwise + - that directory as the working directory - the job's `env`, plus `ARCH` and `MATRIX_*` for a fanned out job - `CONDUCTOR_PROJECT`, `CONDUCTOR_RUN_ID`, `CONDUCTOR_RUN_NUMBER`, `CONDUCTOR_JOB`, `CONDUCTOR_JOB_ID`, `CONDUCTOR_SHA`, `CONDUCTOR_REF` @@ -126,21 +131,21 @@ workspace: - any `services`, reachable by their alias on the job network - mounts and environment from the features it requires -Output is streamed to the conductor as it happens. Artifacts are uploaded -when the job finishes, and the workspace is then removed. +Output is streamed to the conductor as it happens. Artifacts are read +back out of the container when the job finishes, and uploaded. -Caches are bind mounted from `cache_root` straight onto their path in the -workspace. A cache is an optimisation and nothing more: a job must still work -with an empty one, and a cache is never shared between workers. +Nothing else is shared. The worker mounts no directory of its own into a +job: the tree goes in over the docker API and the artifacts come back the +same way. That is why a worker needs no volumes, no matching paths +between itself and the host, and no storage at all. Operational notes ----------------- -**Cleanup.** Containers, networks and workspaces are removed whatever the -outcome. Most images run as root, so files a job creates are owned by root -while the worker usually is not. When removing the workspace fails for that -reason, the worker empties it from inside a container first. Nothing is left -behind either way. +**Cleanup.** Containers and networks are removed whatever the outcome. +There is no workspace to clean up, and so no trouble with a job having +written files as root that the worker then cannot delete: everything the +job wrote lived in its container and went with it. **Shutdown.** On SIGINT or SIGTERM the worker stops polling and lets running jobs finish. A second signal exits immediately, and the conductor will diff --git a/examples/node-app.conductor.yml b/examples/node-app.conductor.yml @@ -16,9 +16,6 @@ jobs: artifacts: paths: [node_modules/**] expire: 1h - cache: - key: npm - paths: [.npm] lint: needs: [install] diff --git a/examples/worker.json b/examples/worker.json @@ -6,8 +6,6 @@ "concurrency": 2, "poll_interval": 5, "docker": "docker", - "workspace_root": "/var/lib/conductor/work", - "cache_root": "/var/lib/conductor/cache", "features": { "dind": { "privileged": true diff --git a/migrations/mysql/005_drop_source_mode.sql b/migrations/mysql/005_drop_source_mode.sql @@ -0,0 +1,5 @@ +-- 005_drop_source_mode.sql - one way to get the source, not two +-- +-- See migrations/sqlite/005_drop_source_mode.sql for the reasoning. + +ALTER TABLE projects DROP COLUMN source_mode; diff --git a/migrations/mysql/006_project_workdir.sql b/migrations/mysql/006_project_workdir.sql @@ -0,0 +1,5 @@ +-- 006_project_workdir.sql - where a job's tree is put in its container +-- +-- See migrations/sqlite/006_project_workdir.sql for the reasoning. + +ALTER TABLE projects ADD COLUMN workdir VARCHAR(1024) NULL; diff --git a/migrations/postgres/005_drop_source_mode.sql b/migrations/postgres/005_drop_source_mode.sql @@ -0,0 +1,5 @@ +-- 005_drop_source_mode.sql - one way to get the source, not two +-- +-- See migrations/sqlite/005_drop_source_mode.sql for the reasoning. + +ALTER TABLE projects DROP COLUMN source_mode; diff --git a/migrations/postgres/006_project_workdir.sql b/migrations/postgres/006_project_workdir.sql @@ -0,0 +1,5 @@ +-- 006_project_workdir.sql - where a job's tree is put in its container +-- +-- See migrations/sqlite/006_project_workdir.sql for the reasoning. + +ALTER TABLE projects ADD COLUMN workdir TEXT; diff --git a/migrations/sqlite/005_drop_source_mode.sql b/migrations/sqlite/005_drop_source_mode.sql @@ -0,0 +1,18 @@ +-- 005_drop_source_mode.sql - one way to get the source, not two +-- +-- A project could choose between having the worker download a tarball of +-- the commit from the conductor, or clone the repository itself. Clone +-- mode is gone. +-- +-- It existed to save bandwidth on large repositories, and cost a great +-- deal for it: the worker needed git, needed credentials for the +-- repository, and could reach any ref rather than only the commit it was +-- given work for. The tarball has none of those properties, and is now +-- streamed straight into the job container, so the worker needs no +-- working directory of its own either. +-- +-- The column is dropped rather than left in place, since a setting that +-- is still offered and no longer does anything is worse than one that is +-- gone. + +ALTER TABLE projects DROP COLUMN source_mode; diff --git a/migrations/sqlite/006_project_workdir.sql b/migrations/sqlite/006_project_workdir.sql @@ -0,0 +1,16 @@ +-- 006_project_workdir.sql - where a job's tree is put in its container +-- +-- The tree used to be bind mounted from the worker at a path the worker +-- chose. It is now unpacked inside the container instead, which means the +-- path is a property of the build rather than of the machine running it, +-- and is worth being able to choose. +-- +-- Three answers, most specific first: the workdir key in the repository's +-- pipeline, this column, and failing both the server default of /work. +-- Null means the project has no opinion. +-- +-- A repository may override its project because the path belongs with the +-- code: an image that expects to build in /usr/src/app knows that, and +-- whoever registered the project should not have to. + +ALTER TABLE projects ADD COLUMN workdir TEXT; diff --git a/src/admin-cli.js b/src/admin-cli.js @@ -120,14 +120,12 @@ try { repo_url: repoUrl, default_branch: flags.branch ? String(flags.branch) : 'main', config_path: flags.config ? String(flags.config) : '.conductor.yml', - source_mode: flags.source ? String(flags.source) : 'archive', trigger_secret: secret, }); console.log(`created project ${project.id}`); console.log(` repository: ${project.repo_url}`); console.log(` pipeline: ${project.config_path}`); - console.log(` source mode: ${project.source_mode}`); console.log(` trigger url: ${cfg.server.public_url.replace(/\/+$/, '')}/api/trigger/${project.id}`); console.log(` secret: ${secret}`); break; @@ -139,7 +137,6 @@ try { repo_url: p.repo_url, branch: p.default_branch, config: p.config_path, - source: p.source_mode, enabled: p.enabled ? 'yes' : 'no', runs: p.run_counter, })); diff --git a/src/conductor/routes/manage.js b/src/conductor/routes/manage.js @@ -11,9 +11,9 @@ import crypto from 'node:crypto'; import { requireUser } from '../../lib/auth/index.js'; -import { SOURCE_MODES, VISIBILITIES, canManageProject } from '../../lib/projects.js'; +import { VISIBILITIES, canManageProject } from '../../lib/projects.js'; import { canManageWorker } from '../../lib/workers.js'; -import { PipelineError } from '../../lib/pipeline/index.js'; +import { PipelineError, DEFAULT_WORKDIR } from '../../lib/pipeline/index.js'; export default async function manageRoutes(fastify, services) { const { cfg, auth, projects, workerTokens, variables } = services; @@ -28,10 +28,12 @@ export default async function manageRoutes(fastify, services) { repo_url: p.repo_url, default_branch: p.default_branch, config_path: p.config_path, - source_mode: p.source_mode, + visibility: p.visibility, enabled: p.enabled === 1, owner_id: p.owner_id, + // Null means the repository decides, and failing that the server. + workdir: p.workdir ?? null, has_trigger_secret: Boolean(p.trigger_secret), run_count: p.run_counter, // Null means the server default applies, which the caller cannot see @@ -71,9 +73,6 @@ export default async function manageRoutes(fastify, services) { if (typeof body.repo_url !== 'string' || body.repo_url.length === 0) { return reply.code(400).send({ error: 'repo_url is required' }); } - if (body.source_mode && !SOURCE_MODES.includes(body.source_mode)) { - return reply.code(400).send({ error: `source_mode must be one of ${SOURCE_MODES.join(', ')}` }); - } if (body.visibility && !VISIBILITIES.includes(body.visibility)) { return reply.code(400).send({ error: `visibility must be one of ${VISIBILITIES.join(', ')}` }); } @@ -116,6 +115,7 @@ export default async function manageRoutes(fastify, services) { log_keep_days: cfg.retention.log_keep_days, }, sweep_interval: cfg.retention.sweep_interval, + workdir: DEFAULT_WORKDIR, })); fastify.patch('/projects/:id', async (req, reply) => { @@ -126,6 +126,8 @@ export default async function manageRoutes(fastify, services) { try { if (typeof body.enabled === 'boolean') await projects.setEnabled(project.id, body.enabled); if (typeof body.visibility === 'string') await projects.setVisibility(project.id, body.visibility); + if (Object.hasOwn(body, 'workdir')) await projects.setWorkdir(project.id, body.workdir); + const retention = {}; for (const key of ['artifact_keep_runs', 'artifact_keep_days', 'log_keep_days']) { if (Object.hasOwn(body, key)) retention[key] = body[key]; diff --git a/src/conductor/routes/workers.js b/src/conductor/routes/workers.js @@ -88,7 +88,9 @@ export default async function workerRoutes(fastify, { cfg, db, git, logs, storag env: job.env, services: job.services, artifacts: job.artifacts, - cache: job.cache, + // Where the tree is unpacked and the script runs. Resolved when + // the run was created, so it does not shift under a retry. + workdir: job.workdir, timeout: job.timeout, attempt: job.attempt, sha: job.sha, @@ -99,13 +101,11 @@ export default async function workerRoutes(fastify, { cfg, db, git, logs, storag // Derived from the server's own timeout so a worker never has to // guess how often it is expected to check in. heartbeat_interval: Math.max(5, Math.floor(cfg.scheduler.heartbeat_timeout / 4)), - // A worker fetches the tree from the conductor by default, so it - // never needs repository credentials. Projects set to clone mode - // get the URL instead. - source: project.source_mode === 'clone' - ? { mode: 'clone', repo_url: project.repo_url, sha: job.sha } - : { mode: 'archive', url: `${base}/api/workers/jobs/${encodeURIComponent(job.id)}/source.tar.gz` }, endpoints: { + // The tree comes from the conductor, so a worker never needs + // credentials for the repository and cannot reach any commit + // other than the one it was given work for. + source: `${base}/api/workers/jobs/${encodeURIComponent(job.id)}/source.tar.gz`, log: `${base}/api/workers/jobs/${encodeURIComponent(job.id)}/log`, artifact: `${base}/api/workers/jobs/${encodeURIComponent(job.id)}/artifact`, heartbeat: `${base}/api/workers/jobs/${encodeURIComponent(job.id)}/heartbeat`, @@ -123,6 +123,9 @@ export default async function workerRoutes(fastify, { cfg, db, git, logs, storag const project = await projects.get(job.project_id); if (!project) return reply.code(404).send({ error: 'unknown project' }); + // The tree rooted at the archive, with no wrapping directory: a + // worker extracts it at whatever path the job is to run in, and + // docker creates that path on the way. reply.header('content-type', 'application/gzip'); reply.header('content-disposition', `attachment; filename="${job.head_sha.slice(0, 12)}.tar.gz"`); return reply.send(git.archiveStream(project, job.head_sha)); diff --git a/src/conductor/scheduler.js b/src/conductor/scheduler.js @@ -16,7 +16,7 @@ // Claiming is a conditional UPDATE guarded on the previous state, so two // workers polling at the same instant cannot both win the same job. -import { compilePipeline, PipelineError, transitiveDependents } from '../lib/pipeline/index.js'; +import { compilePipeline, PipelineError, transitiveDependents, DEFAULT_WORKDIR } from '../lib/pipeline/index.js'; import { inClause } from '../lib/db/query.js'; import { newRunId, jobId as makeJobId } from '../lib/ids.js'; import { keys as storageKeys } from '../lib/storage/index.js'; @@ -128,6 +128,11 @@ export function createScheduler({ cfg, db, git, logs, storage, projects, variabl // legitimately change from one commit to the next. const visibility = pipeline.visibility ?? project.visibility ?? 'private'; + // Same shape of answer for where the tree is unpacked inside a job + // container: the repository knows what its build expects, the + // project can set a house default, and failing both there is one. + const workdir = pipeline.workdir ?? project.workdir ?? DEFAULT_WORKDIR; + await db.transaction(async (tx) => { const number = await projects.nextRunNumber(tx, project.id); const empty = pipeline.jobs.length === 0; @@ -180,10 +185,13 @@ export function createScheduler({ cfg, db, git, logs, storage, projects, variabl env: job.env, services: job.services, artifacts: job.artifacts, - cache: job.cache, matrix: job.matrix, needs: job.needs, depth: job.depth, + // Recorded per job rather than looked up when a worker + // polls, so a run stays reproducible after the project + // or the pipeline changes. + workdir, }), allow: job.allow_failure ? 1 : 0, attempts: job.max_attempts, @@ -298,7 +306,8 @@ export function createScheduler({ cfg, db, git, logs, storage, projects, variabl }, services: spec.services, artifacts: spec.artifacts, - cache: spec.cache, + // Runs created before this existed have no workdir recorded. + workdir: spec.workdir ?? DEFAULT_WORKDIR, }; } diff --git a/src/conductor/ui/pages.js b/src/conductor/ui/pages.js @@ -230,7 +230,7 @@ export function projectPage({ project, variables, triggerUrl, secret, user, rete <div class="row"> ${field('repository', html`<span class="mono">${project.repo_url}</span>`)} ${field('pipeline', html`<span class="mono">${project.config_path}</span>`)} - ${field('source mode', project.source_mode)} + ${field('owner', project.owner_id === null ? 'shared' : (project.owner_id === user.id ? 'you' : project.owner_id))} ${field('runs', project.run_counter)} </div> @@ -254,8 +254,16 @@ export function projectPage({ project, variables, triggerUrl, secret, user, rete <option value="false" ${project.enabled === 1 ? '' : 'selected'}>no</option> </select> </label> + <label>working directory + <input name="workdir" class="mono" placeholder="${retention.workdir}" + value="${project.workdir ?? ''}"> + </label> </div> - <p class="muted">A pipeline may override visibility per commit with a top level visibility key.</p> + <p class="muted"> + A pipeline may override visibility per commit with a top level + visibility key, and the working directory with a workdir key. + Left empty, jobs run in ${retention.workdir}. + </p> <div class="actions"><button type="submit">save</button></div> </form> </div> diff --git a/src/conductor/ui/routes.js b/src/conductor/ui/routes.js @@ -19,7 +19,7 @@ import { import { canManageProject, canViewProject, VISIBILITIES } from '../../lib/projects.js'; import { canManageWorker } from '../../lib/workers.js'; import { issueToken, sessionCookie, clearedCookie } from '../../lib/auth/index.js'; -import { PipelineError } from '../../lib/pipeline/index.js'; +import { PipelineError, DEFAULT_WORKDIR } from '../../lib/pipeline/index.js'; const LOG_TAIL_BYTES = 256 * 1024; @@ -374,7 +374,9 @@ export default async function uiRoutes(fastify, services) { secret, // What an empty field falls back to, so the form can say so // rather than looking unset. - retention: cfg.retention, + // Defaults the form shows as placeholders, so an empty field + // reads as "inherited" rather than "unset". + retention: { ...cfg.retention, workdir: DEFAULT_WORKDIR }, }), }); }); @@ -388,6 +390,11 @@ export default async function uiRoutes(fastify, services) { try { if (body.visibility) await projects.setVisibility(found.project.id, body.visibility); if (body.enabled !== undefined) await projects.setEnabled(found.project.id, body.enabled === 'true' || body.enabled === true); + // An empty field means the project has no opinion, which is a null + // rather than an empty string. + if (Object.hasOwn(body, 'workdir')) { + await projects.setWorkdir(found.project.id, String(body.workdir ?? '').trim() || null); + } } catch (e) { return fail(reply, e.message); } diff --git a/src/lib/pipeline/expand.js b/src/lib/pipeline/expand.js @@ -140,12 +140,6 @@ export function expandPipeline(pipeline, options = {}) { paths: job.artifacts.paths.map((p, i) => interpolate(problems, `${path}.artifacts.paths[${i}]`, p, context)), } : null, - cache: job.cache - ? { - key: interpolate(problems, `${path}.cache.key`, job.cache.key, context), - paths: job.cache.paths.map((p, i) => interpolate(problems, `${path}.cache.paths[${i}]`, p, context)), - } - : null, allow_failure: job.allow_failure, timeout: job.timeout ?? defaultTimeout, max_attempts: job.max_attempts ?? defaultAttempts, diff --git a/src/lib/pipeline/index.js b/src/lib/pipeline/index.js @@ -16,7 +16,7 @@ export { expandPipeline, interpolate } from './expand.js'; export { topologicalOrder, depths, transitiveDependents, runnable, CycleError } from './dag.js'; export { refMatches, jobRunsOnRef, selectForRef } from './only.js'; export { PipelineError } from './schema.js'; -export { SUPPORTED_VERSION, VISIBILITIES } from './parse.js'; +export { SUPPORTED_VERSION, VISIBILITIES, DEFAULT_WORKDIR, parseWorkdir } from './parse.js'; export function compilePipeline(text, options = {}) { const pipeline = parsePipeline(text, options); @@ -44,6 +44,7 @@ export function compilePipeline(text, options = {}) { version: pipeline.version, source: pipeline.source, visibility: pipeline.visibility, + workdir: pipeline.workdir, jobs, }; } diff --git a/src/lib/pipeline/parse.js b/src/lib/pipeline/parse.js @@ -22,28 +22,66 @@ import { NAME_PATTERN, } from './schema.js'; -const TOP_KEYS = ['version', 'visibility', 'defaults', 'jobs']; +const TOP_KEYS = ['version', 'visibility', 'workdir', 'defaults', 'jobs']; // A repository decides whether its own build results are readable without // signing in. Unset means the project's setting stands. export const VISIBILITIES = ['public', 'private']; +// Where the tree is put inside a job container, and the directory every +// script starts in. Unset means the project's setting stands, and failing +// that the server default. +export const DEFAULT_WORKDIR = '/work'; + +// An absolute path of ordinary directory names. The tree is delivered by +// unpacking an archive at the container's root, so the path is built from +// the archive's own entries: anything that could climb out of it, or that +// is not a plain name, is refused rather than sanitised. +export function parseWorkdir(problems, path, raw) { + const value = asString(problems, path, raw); + if (typeof value !== 'string') return null; + + if (!value.startsWith('/')) { + problems.add(path, `must be an absolute path, got ${JSON.stringify(value)}`); + return null; + } + + const segments = value.split('/').filter(Boolean); + if (segments.length === 0) { + problems.add(path, 'must name a directory, not the container root'); + return null; + } + + for (const segment of segments) { + if (segment === '.' || segment === '..') { + problems.add(path, `must not contain ${JSON.stringify(segment)}`); + return null; + } + if (!/^[A-Za-z0-9._-]+$/.test(segment)) { + problems.add(path, `${JSON.stringify(segment)} is not a usable directory name`); + return null; + } + } + + return `/${segments.join('/')}`; +} + const JOB_KEYS = [ 'image', 'script', 'needs', 'arch', 'matrix', 'requires', 'services', - 'env', 'artifacts', 'cache', 'allow_failure', 'timeout', 'max_attempts', + 'env', 'artifacts', 'allow_failure', 'timeout', 'max_attempts', 'only', ]; // Fields a job may inherit from defaults. const DEFAULT_KEYS = [ - 'image', 'arch', 'requires', 'env', 'services', 'cache', + 'image', 'arch', 'requires', 'env', 'services', 'allow_failure', 'timeout', 'max_attempts', ]; const ONLY_KEYS = ['refs']; const ARTIFACT_KEYS = ['paths', 'expire', 'when']; const ARTIFACT_WHEN = ['on_success', 'on_failure', 'always']; -const CACHE_KEYS = ['key', 'paths']; + const SERVICE_KEYS = ['image', 'alias', 'env', 'entrypoint', 'command']; export const SUPPORTED_VERSION = 1; @@ -110,8 +148,10 @@ export function parsePipeline(text, options = {}) { } } + const workdir = doc.workdir === undefined ? null : parseWorkdir(problems, 'workdir', doc.workdir); + problems.throwIfAny(source); - return { version: SUPPORTED_VERSION, visibility, defaults, jobs, source }; + return { version: SUPPORTED_VERSION, visibility, workdir, defaults, jobs, source }; } function parseDefaults(problems, raw) { @@ -121,7 +161,6 @@ function parseDefaults(problems, raw) { requires: [], env: {}, services: [], - cache: null, allow_failure: false, timeout: undefined, max_attempts: undefined, @@ -140,7 +179,6 @@ function parseDefaults(problems, raw) { requires: asStringList(problems, 'defaults.requires', raw.requires), env: asEnvMap(problems, 'defaults.env', raw.env), services: parseServices(problems, 'defaults.services', raw.services), - cache: parseCache(problems, 'defaults.cache', raw.cache), allow_failure: raw.allow_failure === undefined ? false : asBoolean(problems, 'defaults.allow_failure', raw.allow_failure) ?? false, timeout: raw.timeout === undefined ? undefined : asDuration(problems, 'defaults.timeout', raw.timeout), max_attempts: raw.max_attempts === undefined ? undefined : asInteger(problems, 'defaults.max_attempts', raw.max_attempts, { min: 1, max: 10 }), @@ -197,7 +235,6 @@ function parseJob(problems, path, raw, defaults) { services: raw.services === undefined ? defaults.services : parseServices(problems, `${path}.services`, raw.services), env: { ...defaults.env, ...asEnvMap(problems, `${path}.env`, raw.env) }, artifacts: parseArtifacts(problems, `${path}.artifacts`, raw.artifacts), - cache: raw.cache === undefined ? defaults.cache : parseCache(problems, `${path}.cache`, raw.cache), allow_failure: raw.allow_failure === undefined ? defaults.allow_failure : asBoolean(problems, `${path}.allow_failure`, raw.allow_failure) ?? false, @@ -401,26 +438,4 @@ function parseArtifacts(problems, path, raw) { }; } -function parseCache(problems, path, raw) { - if (raw === undefined || raw === null) return null; - const spec = Array.isArray(raw) ? { paths: raw } : raw; - if (!isPlainObject(spec)) { - problems.add(path, `expected a list of paths or a mapping, got ${typeName(raw)}`); - return null; - } - checkUnknown(problems, path, spec, CACHE_KEYS); - - const paths = asStringList(problems, `${path}.paths`, spec.paths, { max: 512 }); - if (paths.length === 0) { - problems.add(`${path}.paths`, 'must list at least one path'); - return null; - } - paths.forEach((p, i) => { - if (p.startsWith('/')) problems.add(`${path}.paths[${i}]`, 'must be relative to the workspace'); - }); - return { - key: spec.key === undefined ? 'default' : asString(problems, `${path}.key`, spec.key, { max: 128 }), - paths, - }; -} diff --git a/src/lib/projects.js b/src/lib/projects.js @@ -13,12 +13,24 @@ // original value, so it is sealed with the secret box. import { newProjectId, slugify } from './ids.js'; +import { parseWorkdir } from './pipeline/parse.js'; +import { Problems } from './pipeline/schema.js'; -export const SOURCE_MODES = ['archive', 'clone']; export const VISIBILITIES = ['public', 'private']; +// Validated with the same rule the pipeline uses, so a project and a +// repository cannot disagree about what a usable working directory is. +export function assertWorkdir(value) { + const problems = new Problems(); + const parsed = parseWorkdir(problems, 'workdir', value); + if (problems.length > 0) { + throw new Error(`workdir ${problems.items[0].message}`); + } + return parsed; +} + const COLUMNS = ` - id, name, repo_url, default_branch, config_path, source_mode, + id, name, repo_url, default_branch, config_path, workdir, trigger_secret, enabled, run_counter, owner_id, visibility, artifact_keep_runs, artifact_keep_days, log_keep_days, created_at, updated_at @@ -61,9 +73,6 @@ export function createProjects({ db, secrets }) { const id = input.id ? slugify(input.id) : (slugify(input.name) || newProjectId()); const now = Date.now(); - if (input.source_mode && !SOURCE_MODES.includes(input.source_mode)) { - throw new Error(`source_mode must be one of ${SOURCE_MODES.join(', ')}`); - } if (input.visibility && !VISIBILITIES.includes(input.visibility)) { throw new Error(`visibility must be one of ${VISIBILITIES.join(', ')}`); } @@ -73,11 +82,11 @@ export function createProjects({ db, secrets }) { await db.run( `INSERT INTO projects - (id, name, repo_url, default_branch, config_path, source_mode, + (id, name, repo_url, default_branch, config_path, workdir, trigger_secret, enabled, run_counter, owner_id, visibility, created_at, updated_at) VALUES - ({id}, {name}, {repo_url}, {branch}, {config_path}, {source_mode}, + ({id}, {name}, {repo_url}, {branch}, {config_path}, {workdir}, {secret}, {enabled}, 0, {owner}, {visibility}, {now}, {now})`, { @@ -86,7 +95,7 @@ export function createProjects({ db, secrets }) { repo_url: input.repo_url, branch: input.default_branch || 'main', config_path: input.config_path || '.conductor.yml', - source_mode: input.source_mode || 'archive', + workdir: input.workdir ? assertWorkdir(input.workdir) : null, secret: input.trigger_secret ? secrets.seal(input.trigger_secret, aad(id)) : null, enabled: input.enabled === undefined ? 1 : (input.enabled ? 1 : 0), owner: input.owner_id ?? null, @@ -155,6 +164,15 @@ export function createProjects({ db, secrets }) { ); }, + // Null hands the decision back to the repository, and failing that + // to the server default. + async setWorkdir(id, workdir) { + await db.run( + 'UPDATE projects SET workdir = {workdir}, updated_at = {now} WHERE id = {id}', + { id, workdir: workdir ? assertWorkdir(workdir) : null, now: Date.now() } + ); + }, + async setOwner(id, ownerId) { await db.run( 'UPDATE projects SET owner_id = {owner}, updated_at = {now} WHERE id = {id}', diff --git a/src/worker/agent.js b/src/worker/agent.js @@ -15,7 +15,7 @@ import { loadWorkerConfig, featureNames } from './config.js'; import { createClient } from './client.js'; import { createRuntime } from './docker.js'; -import { hasTar, hasGit } from './source.js'; + import { runJob } from './job.js'; const stamp = () => new Date().toISOString().replace('T', ' ').slice(0, 19); @@ -51,15 +51,13 @@ export async function main(argv = process.argv.slice(2)) { // Fail at startup rather than on the first job, when a missing tool would // otherwise look like a broken pipeline. + // + // The runtime is the only thing a worker needs. The tree is unpacked + // into the job container over the docker API rather than on this side, + // so there is nothing else to look for on PATH. if (!(await runtime.available())) { throw new Error(`container runtime ${JSON.stringify(cfg.docker)} is not usable; is the daemon running?`); } - if (!(await hasTar())) { - throw new Error('tar is required to unpack job sources but was not found on PATH'); - } - if (!(await hasGit())) { - logger.warn('git was not found; projects configured for clone mode will fail'); - } const features = featureNames(cfg); logger.info(`worker ${cfg.name} polling ${cfg.conductor_url}`); diff --git a/src/worker/artifacts.js b/src/worker/artifacts.js @@ -9,7 +9,6 @@ import fs from 'node:fs/promises'; import path from 'node:path'; import { Readable } from 'node:stream'; import { createReadStream } from 'node:fs'; -import { SCRIPT_DIR } from './script.js'; const MAX_FILES = 2000; @@ -33,8 +32,6 @@ export async function collectArtifacts(workspace, patterns, { logger = console } } const normalized = relative.split(path.sep).join('/'); - // The generated script lives in the workspace and is not output. - if (normalized === SCRIPT_DIR || normalized.startsWith(`${SCRIPT_DIR}/`)) continue; if (seen.has(normalized)) continue; const absolute = path.join(root, relative); diff --git a/src/worker/config.js b/src/worker/config.js @@ -29,8 +29,6 @@ const DEFAULTS = { docker: 'docker', // Shell used to run a job script inside its image. shell: 'sh', - workspace_root: null, - cache_root: null, // Applied when a job does not ask for something longer. default_timeout: 3600, }; @@ -45,8 +43,6 @@ const ENV_MAP = [ ['CONDUCTOR_WORKER_POLL_INTERVAL', 'poll_interval', toInt], ['CONDUCTOR_WORKER_DOCKER', 'docker', String], ['CONDUCTOR_WORKER_SHELL', 'shell', String], - ['CONDUCTOR_WORKER_WORKSPACE', 'workspace_root', String], - ['CONDUCTOR_WORKER_CACHE', 'cache_root', String], ]; function splitList(value) { @@ -199,8 +195,6 @@ export async function loadWorkerConfig(explicitPath) { } if (!cfg.token) problems.push('no worker token: set token, token_file, or CONDUCTOR_WORKER_TOKEN'); - cfg.workspace_root = path.resolve(cfg.workspace_root || path.join(os.tmpdir(), 'conductor-work')); - cfg.cache_root = path.resolve(cfg.cache_root || path.join(os.tmpdir(), 'conductor-cache')); cfg.conductor_url = String(cfg.conductor_url).replace(/\/+$/, ''); if (problems.length > 0) { diff --git a/src/worker/docker.js b/src/worker/docker.js @@ -39,7 +39,7 @@ export function createRuntime(cfg, { logger = console } = {}) { } // Turns a job and the worker's feature definitions into run arguments. - function buildRunArgs({ name, image, network, workspace, env = {}, mounts = [], privileged = false, devices = [], command = [], entrypoint = null, detach = false, networkAlias = null }) { + function buildRunArgs({ name, image, network, workdir = null, env = {}, mounts = [], privileged = false, devices = [], command = [], entrypoint = null, detach = false, networkAlias = null }) { const args = ['run', '--rm', '--name', name]; if (detach) args.push('--detach'); else args.push('--attach', 'stdout', '--attach', 'stderr'); @@ -51,10 +51,7 @@ export function createRuntime(cfg, { logger = console } = {}) { for (const device of devices) args.push('--device', device); for (const mount of mounts) args.push('--volume', mount); - if (workspace) { - args.push('--volume', `${workspace}:/workspace`); - args.push('--workdir', '/workspace'); - } + if (workdir) args.push('--workdir', workdir); for (const [key, value] of Object.entries(env)) { args.push('--env', `${key}=${value}`); @@ -66,8 +63,87 @@ export function createRuntime(cfg, { logger = console } = {}) { return args; } + // A job container is created rather than run, so its filesystem can be + // populated before it starts and read after it exits. Nothing from the + // worker's own filesystem is mounted into it: the source arrives over + // the docker API, which means the worker needs no shared directory with + // the host and no storage of its own. + function buildCreateArgs(options) { + const args = buildRunArgs({ ...options, detach: false }); + + // Same arguments as run, minus two things. + // + // --attach, because there is nothing to attach to until it starts. + // + // --rm, which matters more than it looks: the artifacts are read out + // of the container after it exits, and a container created with --rm + // deletes itself the moment it stops, taking them with it. Removal + // is done explicitly during cleanup instead. + const filtered = args.filter((arg, i) => { + if (arg === '--attach' || args[i - 1] === '--attach') return false; + if (arg === '--rm') return false; + return true; + }); + + filtered[0] = 'create'; + return filtered; + } + return { buildRunArgs, + buildCreateArgs, + + async create(options) { + await run(buildCreateArgs(options)); + return options.name; + }, + + // Extracts a tar stream into the container at destination. Used for + // the source tree, which is streamed from the conductor and never + // touches the worker's disk. + async copyStreamIn(name, stream, destination = '/') { + await new Promise((resolve, reject) => { + const child = spawn(bin, ['cp', '-', `${name}:${destination}`], { + stdio: ['pipe', 'ignore', 'pipe'], + }); + let stderr = ''; + child.stderr.on('data', (c) => { stderr += c.toString().slice(0, 4096); }); + child.on('error', reject); + child.on('close', (code) => { + if (code === 0) resolve(); + else reject(new Error(`${bin} cp failed: ${stderr.trim() || `exit ${code}`}`)); + }); + stream.on('error', (e) => { + child.stdin.destroy(); + reject(e); + }); + stream.pipe(child.stdin); + }); + }, + + async copyIn(name, hostPath, destination) { + await run(['cp', hostPath, `${name}:${destination}`], { timeout: 600000 }); + }, + + // Returns false when the path is simply not there, which is a normal + // outcome for an artifact pattern that matched nothing. + async copyOut(name, containerPath, hostPath) { + try { + await run(['cp', `${name}:${containerPath}`, hostPath], { timeout: 600000 }); + return true; + } catch (e) { + if (/No such container:path|not found in|no such file or directory/i.test(e.message)) return false; + throw e; + } + }, + + // Runs a created container and hands back the process, so the caller + // can stream its output as it happens. + startCreated(name) { + const args = ['start', '--attach', name]; + const child = spawn(bin, args, { stdio: ['ignore', 'pipe', 'pipe'] }); + return { child, args }; + }, async available() { try { diff --git a/src/worker/job.js b/src/worker/job.js @@ -1,20 +1,59 @@ // src/worker/job.js - running one job // -// Sequence: prepare a workspace, fetch the tree, start any services on a -// private network, run the job container, ship its output, upload -// artifacts, report the outcome, and clean up whatever was created. +// Sequence: create the container, put the tree and the script into it, +// start any services on a private network, run it, ship its output, +// take the artifacts back out, report, and clean up. +// +// Nothing is shared with the container through the filesystem. The tree +// arrives over the docker API and the artifacts leave the same way, so +// the worker needs no directory the host can also see, and therefore no +// mounts, no matching paths, and no storage of its own. What little it +// does write goes to a scratch directory that lives and dies with the +// job. // // Cleanup runs whatever happened. A worker that leaks containers or // networks stops being usable after a few dozen jobs. import fs from 'node:fs/promises'; +import os from 'node:os'; import path from 'node:path'; import { containerName } from './docker.js'; import { createLogStream } from './logstream.js'; -import { fetchSource } from './source.js'; -import { buildScript, SCRIPT_DIR, SCRIPT_NAME, SCRIPT_PATH } from './script.js'; +import { sourceIntoContainer } from './source.js'; + +// Where a job runs when the conductor did not say. The conductor resolves +// this from the pipeline and the project, so this only covers a worker +// talking to one too old to have an opinion. +const FALLBACK_WORKDIR = '/work'; +import { buildScript, SCRIPT_NAME, SCRIPT_PATH } from './script.js'; import { collectArtifacts, uploadArtifacts, shouldCollect } from './artifacts.js'; +// The part of an artifact pattern before its first wildcard, which is the +// most that can be copied out without asking the container to expand a +// glob it is no longer running to expand. An empty string means the +// pattern could match anywhere, so the whole tree has to come back. +export function patternPrefix(pattern) { + const segments = String(pattern).split('/'); + const literal = []; + for (const segment of segments) { + if (/[*?[\]{}]/.test(segment)) break; + literal.push(segment); + } + // A pattern with no wildcard at all names one path, and its last + // segment is a file rather than a directory to descend into. + return literal.join('/'); +} + +// The set of paths to copy out for a job's artifact patterns, with any +// path that is covered by another removed. +export function copyOutPaths(patterns) { + const prefixes = (patterns ?? []).map(patternPrefix); + if (prefixes.some((p) => p === '')) return ['']; + + const unique = [...new Set(prefixes)].sort(); + return unique.filter((p, i) => !unique.slice(0, i).some((other) => p === other || p.startsWith(`${other}/`))); +} + // Resolves the features a job asked for into local resources. export function resolveFeatures(job, available) { const mounts = []; @@ -38,35 +77,21 @@ export function resolveFeatures(job, available) { return { mounts, env, devices, privileged, missing }; } -// Cache directories are bind mounted straight onto their path in the -// workspace, which is cheaper than copying in and out around every job. -export async function prepareCache(cfg, job) { - if (!job.cache || !Array.isArray(job.cache.paths)) return []; - - const key = String(job.cache.key ?? 'default').replace(/[^A-Za-z0-9_.-]/g, '-').slice(0, 128); - const project = String(job.project_id ?? 'project').replace(/[^A-Za-z0-9_.-]/g, '-'); - const mounts = []; - - for (const relative of job.cache.paths) { - const normalized = relative.split('/').filter((s) => s && s !== '.' && s !== '..').join('/'); - if (!normalized) continue; - const host = path.join(cfg.cache_root, project, key, normalized); - await fs.mkdir(host, { recursive: true }); - mounts.push(`${host}:/workspace/${normalized}`); - } - - return mounts; -} - export async function runJob({ cfg, client, runtime, job, logger = console }) { - const workspace = path.join(cfg.workspace_root, containerName('ws', job.id)); const network = containerName('conductor-net', job.id); const jobContainer = containerName('conductor', job.id); + const workdir = job.workdir || FALLBACK_WORKDIR; + + // Worker local, and only ever written by the worker: the generated + // script on its way in, and artifacts on their way out. The container + // never sees it. + let scratch = null; const log = createLogStream(client, job.endpoints.log, { masked: job.masked ?? [], logger }); const services = []; let networkCreated = false; + let containerCreated = false; let heartbeat = null; let timeoutTimer = null; let cancelled = false; @@ -75,7 +100,7 @@ export async function runJob({ cfg, client, runtime, job, logger = console }) { let failure = null; try { - await fs.mkdir(workspace, { recursive: true }); + scratch = await fs.mkdtemp(path.join(os.tmpdir(), 'conductor-job-')); log.note(`[conductor] job ${job.name} on ${cfg.name}`); log.note(`[conductor] commit ${String(job.sha).slice(0, 12)} attempt ${job.attempt}`); @@ -88,13 +113,6 @@ export async function runJob({ cfg, client, runtime, job, logger = console }) { throw new Error(`worker does not provide required feature(s): ${features.missing.join(', ')}`); } - await fetchSource(client, job, workspace); - - await fs.mkdir(path.join(workspace, SCRIPT_DIR), { recursive: true }); - await fs.writeFile(path.join(workspace, SCRIPT_PATH), buildScript(job.script), { mode: 0o755 }); - - const cacheMounts = await prepareCache(cfg, job); - await runtime.createNetwork(network); networkCreated = true; @@ -113,18 +131,30 @@ export async function runJob({ cfg, client, runtime, job, logger = console }) { services.push(name); } - const { child } = runtime.start({ + // Created rather than run, so its filesystem can be filled before it + // starts and read after it exits. Feature mounts are still bind + // mounts, since those name host paths the operator chose deliberately. + await runtime.create({ name: jobContainer, image: job.image, network, - workspace, + workdir, env: { ...features.env, ...job.env }, - mounts: [...features.mounts, ...cacheMounts], + mounts: features.mounts, devices: features.devices, privileged: features.privileged, entrypoint: cfg.shell, - command: [`/workspace/${SCRIPT_PATH}`], + command: [SCRIPT_PATH], }); + containerCreated = true; + + await sourceIntoContainer({ client, runtime, job, container: jobContainer, workdir }); + + const scriptPath = path.join(scratch, SCRIPT_NAME); + await fs.writeFile(scriptPath, buildScript(job.script), { mode: 0o755 }); + await runtime.copyIn(jobContainer, scriptPath, SCRIPT_PATH); + + const { child } = runtime.startCreated(jobContainer); child.stdout.on('data', (chunk) => log.write(chunk)); child.stderr.on('data', (chunk) => log.write(chunk)); @@ -167,11 +197,13 @@ export async function runJob({ cfg, client, runtime, job, logger = console }) { const success = failure === null && exitCode === 0 && !cancelled && !timedOut; - // Artifacts are collected before teardown, and even after a failure when - // the pipeline asked for that. - if (job.artifacts && shouldCollect(job.artifacts.when, success)) { + // Artifacts are collected before teardown, and even after a failure + // when the pipeline asked for that. They come out of the stopped + // container, which still has its filesystem until it is removed. + if (containerCreated && job.artifacts && shouldCollect(job.artifacts.when, success)) { try { - const files = await collectArtifacts(workspace, job.artifacts.paths, { logger }); + const staged = await stageArtifacts(); + const files = await collectArtifacts(staged, job.artifacts.paths, { logger }); if (files.length > 0) { log.note(`[conductor] uploading ${files.length} artifact(s)`); await uploadArtifacts(client, job.endpoints.artifact, files, { logger }); @@ -210,42 +242,44 @@ export async function runJob({ cfg, client, runtime, job, logger = console }) { return { success, exitCode, error, cancelled, timedOut }; + // Copies what the artifact patterns could possibly match out of the + // stopped container, into a directory the globbing can then walk. + // + // Only the leading literal part of each pattern can be copied, since + // the container is no longer running to expand a glob. A pattern that + // could match anywhere brings the whole tree back, which is the price + // of asking for one. + async function stageArtifacts() { + const staged = path.join(scratch, 'artifacts'); + await fs.mkdir(staged, { recursive: true }); + + for (const relative of copyOutPaths(job.artifacts.paths)) { + if (relative === '') { + // The trailing /. copies the contents rather than the directory. + await runtime.copyOut(jobContainer, `${workdir}/.`, staged); + break; + } + + // Copying out of a container does need the parent to exist here, + // unlike copying in. + const parent = path.join(staged, path.dirname(relative)); + await fs.mkdir(parent, { recursive: true }); + await runtime.copyOut(jobContainer, `${workdir}/${relative}`, parent); + } + + return staged; + } + async function cleanup() { for (const name of services) await runtime.remove(name); await runtime.remove(jobContainer); if (networkCreated) await runtime.removeNetwork(network); - await removeWorkspace(); - } - // Most images run as root, so files the job created in the bind mounted - // workspace are owned by root while the worker usually is not. Removing - // them from the host then fails with EACCES. The directory itself belongs - // to the worker, so emptying it from inside a container as root and then - // removing the empty directory works without forcing every job image to - // run as the worker's uid. - async function removeWorkspace() { - try { - await fs.rm(workspace, { recursive: true, force: true }); - return; - } catch (e) { - if (e.code !== 'EACCES' && e.code !== 'EPERM' && e.code !== 'ENOTEMPTY') { - logger.warn?.(`could not remove workspace ${workspace}: ${e.message}`); - return; - } - } - - try { - // The job image is already present locally, so nothing is pulled. - await runtime.runOnce({ - name: containerName('conductor-clean', job.id), - image: job.image, - workspace, - entrypoint: cfg.shell, - command: ['-c', 'rm -rf /workspace/..?* /workspace/.[!.]* /workspace/* 2>/dev/null || true'], - }); - await fs.rm(workspace, { recursive: true, force: true }); - } catch (e) { - logger.warn?.(`could not remove workspace ${workspace}: ${e.message}`); + // Nothing here was written by the job, so there is no root owned + // output to work around: removing the container took that with it. + if (scratch) { + await fs.rm(scratch, { recursive: true, force: true }) + .catch((e) => logger.warn?.(`could not remove scratch ${scratch}: ${e.message}`)); } } } diff --git a/src/worker/script.js b/src/worker/script.js @@ -1,8 +1,11 @@ // src/worker/script.js - turning a job's commands into a shell script // -// The script is written into the workspace and run by path rather than piped -// to the shell's stdin, so that a command which itself reads stdin cannot -// swallow the rest of the script. +// The script is copied into the container and run by path rather than +// piped to the shell's stdin, so that a command which itself reads stdin +// cannot swallow the rest of the script. +// +// It lives outside the tree, so that a repository cannot shadow it and +// nothing the job does to its own working directory can lose it. // Single quoting is the only form that is safe for arbitrary text in POSIX // sh: everything inside is literal, and an embedded quote is closed, @@ -24,6 +27,5 @@ export function buildScript(commands) { return lines.join('\n'); } -export const SCRIPT_DIR = '.conductor'; -export const SCRIPT_NAME = 'script.sh'; -export const SCRIPT_PATH = `${SCRIPT_DIR}/${SCRIPT_NAME}`; +export const SCRIPT_NAME = 'entrypoint.sh'; +export const SCRIPT_PATH = `/tmp/${SCRIPT_NAME}`; diff --git a/src/worker/source.js b/src/worker/source.js @@ -1,77 +1,29 @@ -// src/worker/source.js - getting the tree into the workspace +// src/worker/source.js - getting the tree into the job container // -// Two modes, chosen per project by the conductor: +// The conductor serves a tarball of the exact commit a job was created +// for. The worker never sees a repository, holds no credentials for one, +// and cannot reach any ref other than the one it was given work for. // -// archive download a tarball of the exact commit from the conductor and -// unpack it. The worker needs no repository credentials and can -// reach no ref other than the one it was given work for. -// clone git clone the repository directly, for repositories large -// enough that a tarball per job is wasteful. +// The tarball goes straight from the conductor into the container: it is +// decompressed in flight and handed to docker cp, so it is never written +// to the worker's disk and never sits in a directory shared with the +// container. That is what lets a worker run with no mounts at all. // -// Unpacking shells out to the host tar. Both GNU and BSD tar accept these -// arguments; availability is checked once at startup. +// docker creates the destination as it extracts, including any missing +// parents, so the tree can be delivered to a path the image has never +// heard of without preparing anything first. -import { spawn, execFile } from 'node:child_process'; -import { promisify } from 'node:util'; import { Readable } from 'node:stream'; -import { pipeline } from 'node:stream/promises'; +import { createGunzip } from 'node:zlib'; -const execFileAsync = promisify(execFile); +export async function sourceIntoContainer({ client, runtime, job, container, workdir }) { + const url = job.endpoints?.source; + if (!url) throw new Error('the job carries no source endpoint'); -export async function hasTar() { - try { - await execFileAsync('tar', ['--version'], { timeout: 10000 }); - return true; - } catch { - return false; - } -} - -export async function hasGit() { - try { - await execFileAsync('git', ['--version'], { timeout: 10000 }); - return true; - } catch { - return false; - } -} - -// Streams a gzipped tar into a directory without staging it on disk. -export async function extractTarGz(stream, directory) { - const child = spawn('tar', ['-xzf', '-', '-C', directory, '--no-same-owner'], { - stdio: ['pipe', 'ignore', 'pipe'], - }); - - let stderr = ''; - child.stderr.on('data', (chunk) => { stderr += chunk.toString().slice(0, 4096); }); - - const exited = new Promise((resolve, reject) => { - child.on('error', reject); - child.on('close', (code) => { - if (code === 0) resolve(); - else reject(new Error(`tar exited ${code}: ${stderr.trim()}`)); - }); - }); - - const source = stream instanceof Readable ? stream : Readable.fromWeb(stream); - await Promise.all([pipeline(source, child.stdin), exited]); -} - -export async function fetchSource(client, job, workspace, { timeout = 600000 } = {}) { - const source = job.source ?? {}; - - if (source.mode === 'clone') { - const env = { ...process.env, GIT_TERMINAL_PROMPT: '0', GIT_ASKPASS: '' }; - await execFileAsync('git', ['clone', '--quiet', '--no-checkout', '--', source.repo_url, workspace], { timeout, env }); - await execFileAsync('git', ['-C', workspace, 'checkout', '--quiet', source.sha], { timeout, env }); - return { mode: 'clone' }; - } - - if (source.mode === 'archive') { - const body = await client.source(source.url); - await extractTarGz(body, workspace); - return { mode: 'archive' }; - } + const body = await client.source(url); + const stream = body instanceof Readable ? body : Readable.fromWeb(body); - throw new Error(`unsupported source mode: ${JSON.stringify(source.mode)}`); + // Decompressed on this side, because docker cp takes a plain tar and + // the container has no shell of its own to unpack anything with. + await runtime.copyStreamIn(container, stream.pipe(createGunzip()), workdir); } diff --git a/test/conductor.test.js b/test/conductor.test.js @@ -221,12 +221,12 @@ test('a claimed job carries everything the worker needs', async () => { assert.deepEqual(job.script, ['./build.sh $ARCH']); assert.equal(job.env.ARCH, 'x86_64'); assert.equal(job.sha, h.sha); - assert.equal(job.source.mode, 'archive'); - assert.match(job.source.url, /^http:\/\/conductor\.test\/api\/workers\/jobs\//); + assert.match(job.endpoints.source, /^http:\/\/conductor\.test\/api\/workers\/jobs\//); + assert.ok(job.endpoints.source.endsWith('/source.tar.gz')); assert.ok(job.endpoints.log.endsWith('/log')); - // The worker resolves features and cache keys from these two, so a - // missing field silently disables both. + // The worker resolves features from these, so a missing field + // silently disables them. assert.deepEqual(job.requires, []); assert.equal(job.project_id, 'demo'); assert.ok(Number.isInteger(job.heartbeat_interval) && job.heartbeat_interval > 0); @@ -675,15 +675,112 @@ test('a commit that is not in the repository is refused', async () => { }); }); -test('clone mode hands the repository url to the worker instead', async () => { - await withHarness({ sourceMode: 'clone' }, async (h) => { +test('a job is told to fetch an archive, and never the repository itself', async () => { + await withHarness({}, async (h) => { + await h.trigger({ sha: h.sha, ref: 'refs/heads/main' }); + const job = await claimNamed(h, 'lint', {}); + + assert.match(job.endpoints.source, /\/source\.tar\.gz$/); + + // A worker holds no repository credentials and can reach no ref other + // than the commit it was given work for, so the repository location + // has no business being in the payload. + assert.equal(job.source, undefined, 'there is one way to get the source, so there is no mode'); + assert.ok(!JSON.stringify(job).includes(h.repoDir), 'the repository path must not reach the worker'); + }); +}); + +test('the source archive is the bare tree, with no wrapping directory', async () => { + // The worker hands it to docker cp with the working directory as the + // destination, and docker creates that directory as it extracts. A + // wrapping directory in the archive would land one level too deep. + await withHarness({}, async (h) => { await h.trigger({ sha: h.sha, ref: 'refs/heads/main' }); const job = await claimNamed(h, 'lint', {}); - assert.equal(job.source.mode, 'clone'); - assert.equal(job.source.repo_url, h.repoDir); + + const archive = await h.app.inject({ + method: 'GET', + url: new URL(job.endpoints.source).pathname, + headers: h.auth, + }); + assert.equal(archive.statusCode, 200); + + // spawnSync rather than the promisified execFile, which has no way to + // supply stdin and simply waits for input that never comes. + const { spawnSync } = await import('node:child_process'); + const listing = spawnSync('tar', ['-tzf', '-'], { + input: archive.rawPayload, + encoding: 'utf8', + }); + assert.equal(listing.status, 0, `tar failed: ${listing.stderr}`); + + const entries = listing.stdout.split('\n').filter(Boolean); + assert.ok(entries.length > 0, 'the archive should not be empty'); + assert.ok( + entries.includes('.conductor.yml'), + `the pipeline should sit at the root of the archive, got:\n${listing.stdout}`, + ); }); }); +test('the working directory is resolved from the pipeline, then the project', async () => { + const pipeline = [ + 'version: 1', + 'workdir: /usr/src/app', + 'defaults:', + ' image: alpine:3', + 'jobs:', + ' build:', + " script: ['make']", + '', + ].join('\n'); + + await withHarness({ pipeline }, async (h) => { + await h.trigger({ sha: h.sha, ref: 'refs/heads/main' }); + const job = await claimNamed(h, 'build', {}); + assert.equal(job.workdir, '/usr/src/app', 'the repository has the final say'); + }); + + // With the repository silent, the project decides. + await withHarness({}, async (h) => { + await h.services.db.run( + 'UPDATE projects SET workdir = {dir} WHERE id = {id}', + { dir: '/srv/build', id: h.project.id } + ); + await h.trigger({ sha: h.sha, ref: 'refs/heads/main' }); + const job = await claimNamed(h, 'lint', {}); + assert.equal(job.workdir, '/srv/build'); + }); + + // With neither, there is a default. + await withHarness({}, async (h) => { + await h.trigger({ sha: h.sha, ref: 'refs/heads/main' }); + const job = await claimNamed(h, 'lint', {}); + assert.equal(job.workdir, '/work'); + }); +}); + +test('an unusable working directory is refused rather than sanitised', async () => { + for (const workdir of ['relative/path', '/', '/a/../b', '/has space']) { + const pipeline = [ + 'version: 1', + `workdir: ${JSON.stringify(workdir)}`, + 'defaults:', + ' image: alpine:3', + 'jobs:', + ' build:', + " script: ['make']", + '', + ].join('\n'); + + await withHarness({ pipeline }, async (h) => { + const res = await h.trigger({ sha: h.sha, ref: 'refs/heads/main' }); + assert.equal(res.statusCode, 422, `expected ${JSON.stringify(workdir)} to be refused`); + assert.ok(res.json().problems.some((p) => p.path === 'workdir')); + }); + } +}); + test('the trigger secret is not stored in the clear', async () => { await withHarness({}, async (h) => { const row = await h.services.db.get('SELECT trigger_secret FROM projects WHERE id = {id}', { id: 'demo' }); diff --git a/test/helpers/harness.js b/test/helpers/harness.js @@ -139,7 +139,7 @@ export async function startHarness(options = {}) { repo_url: repoDir, trigger_secret: 'test-secret', config_path: options.configPath ?? '.conductor.yml', - source_mode: options.sourceMode ?? 'archive', + // Public by default so that tests about scheduling can read runs back // without signing in. Visibility itself is covered by its own suite, // which creates private projects explicitly. diff --git a/test/ui.test.js b/test/ui.test.js @@ -454,3 +454,55 @@ test('a negative retention value is refused by the form', async () => { assert.equal(project.log_keep_days, null); }); }); + +// --- working directory --- + +test('the project page offers a working directory, defaulting to the server one', async () => { + await withUi({ allowLocalLogin: true }, async (h) => { + const session = await signIn(h); + const page = await get(h, `/projects/${h.project.id}`, session); + + assert.equal(page.statusCode, 200); + assert.match(page.body, /name="workdir"[^>]*placeholder="\/work"/); + assert.match(page.body, /name="workdir"[^>]*value=""/, 'unset means inherited'); + }); +}); + +test('saving a working directory stores it, and empty clears it', async () => { + await withUi({ allowLocalLogin: true }, async (h) => { + const session = await signIn(h); + + const saved = await h.app.inject({ + method: 'PATCH', + url: `/projects/${h.project.id}`, + ...form(session, { workdir: '/usr/src/app' }), + }); + assert.ok(saved.statusCode < 400, `unexpected ${saved.statusCode}: ${saved.body}`); + assert.equal((await h.services.projects.get(h.project.id)).workdir, '/usr/src/app'); + + const cleared = await h.app.inject({ + method: 'PATCH', + url: `/projects/${h.project.id}`, + ...form(session, { workdir: ' ' }), + }); + assert.ok(cleared.statusCode < 400); + assert.equal((await h.services.projects.get(h.project.id)).workdir, null, + 'blank means follow the repository, not a directory named blank'); + }); +}); + +test('an unusable working directory is refused by the form', async () => { + await withUi({ allowLocalLogin: true }, async (h) => { + const session = await signIn(h); + + for (const workdir of ['relative', '/', '/a/../b']) { + const res = await h.app.inject({ + method: 'PATCH', + url: `/projects/${h.project.id}`, + ...form(session, { workdir }), + }); + assert.ok(res.statusCode >= 400, `expected ${JSON.stringify(workdir)} to be refused`); + assert.equal((await h.services.projects.get(h.project.id)).workdir, null); + } + }); +}); diff --git a/test/worker-docker.test.js b/test/worker-docker.test.js @@ -54,8 +54,6 @@ async function listening(options = {}) { poll_interval: 1, docker: 'docker', shell: 'sh', - workspace_root: `${h.root}/work`, - cache_root: `${h.root}/cache`, default_timeout: 300, ...(options?.worker ?? {}), }; @@ -331,9 +329,99 @@ jobs: assert.equal(await exists('network', containerName('conductor-net', job.id)), false, 'the job network should be gone'); + // The worker keeps nothing between jobs, so there is no workspace + // left to remove: what it wrote lived in a temporary directory that + // goes with the job. const fs = await import('node:fs/promises'); - const left = await fs.readdir(`${h.root}/work`).catch(() => []); - assert.deepEqual(left, [], 'the workspace should be removed'); + const os = await import('node:os'); + const scratch = (await fs.readdir(os.tmpdir())).filter((n) => n.startsWith('conductor-job-')); + assert.deepEqual(scratch, [], 'the job scratch directory should be removed'); + } finally { + await h.stop(); + } +}); + +test('a job runs with no mounts, and nothing of the worker is shared with it', opts, async () => { + // The point of copying the tree in rather than bind mounting it: the + // container gets no path belonging to the worker, so nothing has to + // line up between the two and the worker needs no storage at all. + const pipeline = ` +version: 1 +jobs: + isolated: + image: ${IMAGE} + script: + - pwd + - cat README.md + - cat /proc/mounts +`; + const h = await listening({ pipeline }); + try { + await h.trigger({ sha: h.sha, ref: 'refs/heads/main' }); + const job = await h.client.poll({ arches: [], features: [], name: 'test' }); + + const created = h.runtime.buildCreateArgs({ + name: 'inspect', image: IMAGE, workdir: job.workdir, entrypoint: 'sh', command: ['/tmp/entrypoint.sh'], + }); + assert.ok(!created.includes('--volume'), `no volume should be passed, got: ${created.join(' ')}`); + + const result = await runJob({ cfg: h.workerCfg, client: h.client, runtime: h.runtime, job, logger: quiet }); + assert.equal(result.success, true, `job failed: ${result.error}`); + + const log = await logOf(h, job.id); + assert.match(log, /^\/work$/m, 'the job should run in the working directory'); + assert.match(log, /test repository/, 'the tree should have arrived'); + + // The proof, from inside the container: the only mounts are the ones + // docker always adds. Nothing of the worker's filesystem is there, so + // no path has to mean the same thing on both sides. + const mounts = log.split('\n').filter((line) => / \/\S+ \S+ (rw|ro)[,\s]/.test(line)); + assert.ok(mounts.length > 0, 'expected /proc/mounts in the log'); + + const shared = mounts + .map((line) => line.split(' ')[1]) + .filter((target) => !/^\/(proc|sys|dev|etc\/(hostname|hosts|resolv\.conf))/.test(target)) + .filter((target) => target !== '/'); + assert.deepEqual(shared, [], `nothing should be mounted into the job, found: ${shared.join(', ')}`); + } finally { + await h.stop(); + } +}); + +test('a pipeline chooses where its tree lands', opts, async () => { + const pipeline = ` +version: 1 +workdir: /usr/src/app +jobs: + build: + image: ${IMAGE} + script: + - pwd + - cat README.md + - mkdir -p out && printf 'made here' > out/result.txt + artifacts: + paths: [out/**] +`; + const h = await listening({ pipeline }); + try { + await h.trigger({ sha: h.sha, ref: 'refs/heads/main' }); + const job = await h.client.poll({ arches: [], features: [], name: 'test' }); + assert.equal(job.workdir, '/usr/src/app'); + + const result = await runJob({ cfg: h.workerCfg, client: h.client, runtime: h.runtime, job, logger: quiet }); + assert.equal(result.success, true, `job failed: ${result.error}`); + + const log = await logOf(h, job.id); + assert.match(log, /^\/usr\/src\/app$/m, 'the script should run there'); + assert.match(log, /test repository/, 'and the tree should be there, not at /work'); + + // Artifacts are read back out relative to the same directory. + const detail = await h.app.inject({ method: 'GET', url: `/api/jobs/${encodeURIComponent(job.id)}` }); + const artifacts = detail.json().artifacts; + assert.deepEqual(artifacts.map((a) => a.path), ['out/result.txt']); + + const download = await h.app.inject({ method: 'GET', url: `/api/artifacts/${artifacts[0].id}` }); + assert.equal(download.body, 'made here'); } finally { await h.stop(); } diff --git a/test/worker.test.js b/test/worker.test.js @@ -13,7 +13,7 @@ import { loadWorkerConfig, featureNames } from '../src/worker/config.js'; import { buildScript, shellQuote } from '../src/worker/script.js'; import { containerName, createRuntime } from '../src/worker/docker.js'; import { createLogStream } from '../src/worker/logstream.js'; -import { resolveFeatures, prepareCache } from '../src/worker/job.js'; +import { resolveFeatures, patternPrefix, copyOutPaths } from '../src/worker/job.js'; import { collectArtifacts, shouldCollect } from '../src/worker/artifacts.js'; async function tempDir(prefix = 'conductor-worker-') { @@ -143,12 +143,12 @@ test('run arguments carry mounts, env, network and privilege', () => { name: 'job1', image: 'alpine:3', network: 'net1', - workspace: '/tmp/ws', + workdir: '/work', env: { FOO: 'bar' }, mounts: ['/srv/k:/keys/k:ro'], privileged: true, entrypoint: 'sh', - command: ['/workspace/.conductor/script.sh'], + command: ['/tmp/entrypoint.sh'], }); assert.equal(args[0], 'run'); @@ -156,10 +156,47 @@ test('run arguments carry mounts, env, network and privilege', () => { assert.deepEqual(args.slice(args.indexOf('--network'), args.indexOf('--network') + 2), ['--network', 'net1']); assert.ok(args.includes('--privileged')); assert.ok(args.includes('/srv/k:/keys/k:ro')); - assert.ok(args.includes('/tmp/ws:/workspace')); + assert.deepEqual(args.slice(args.indexOf('--workdir'), args.indexOf('--workdir') + 2), ['--workdir', '/work']); assert.ok(args.includes('FOO=bar')); // The image must come before its command. - assert.ok(args.indexOf('alpine:3') < args.indexOf('/workspace/.conductor/script.sh')); + assert.ok(args.indexOf('alpine:3') < args.indexOf('/tmp/entrypoint.sh')); +}); + +test('a job container is created without sharing anything with the worker', () => { + // The whole point of copying the tree in: no path of the worker's is + // handed to the daemon, so nothing has to line up between the two. + const runtime = createRuntime({ docker: 'docker', shell: 'sh' }); + const args = runtime.buildCreateArgs({ + name: 'job1', + image: 'alpine:3', + network: 'net1', + workdir: '/work', + env: { FOO: 'bar' }, + entrypoint: 'sh', + command: ['/tmp/entrypoint.sh'], + }); + + assert.equal(args[0], 'create'); + assert.ok(!args.includes('--attach'), 'create takes no attach flags'); + assert.ok(!args.includes('--volume'), 'nothing of the worker is mounted in'); + assert.ok(!args.some((a) => a.includes(':/work')), 'the tree is copied in, never bind mounted'); + assert.deepEqual(args.slice(args.indexOf('--workdir'), args.indexOf('--workdir') + 2), ['--workdir', '/work']); + assert.ok(args.indexOf('alpine:3') < args.indexOf('/tmp/entrypoint.sh')); +}); + +test('a feature still contributes its bind mounts', () => { + // Those name host paths an operator chose deliberately, which is a + // different thing from the worker sharing its own directories. + const runtime = createRuntime({ docker: 'docker', shell: 'sh' }); + const args = runtime.buildCreateArgs({ + name: 'job1', + image: 'alpine:3', + mounts: ['/srv/keys/build.rsa:/keys/build.rsa:ro'], + entrypoint: 'sh', + command: ['/tmp/entrypoint.sh'], + }); + + assert.ok(args.includes('/srv/keys/build.rsa:/keys/build.rsa:ro')); }); test('features resolve into mounts, env and privilege', () => { @@ -179,23 +216,29 @@ test('an unknown required feature is reported rather than ignored', () => { assert.deepEqual(resolved.missing, ['nope']); }); -test('cache paths become bind mounts and are created', async () => { - const dir = await tempDir(); - const mounts = await prepareCache( - { cache_root: dir }, - { project_id: 'demo', cache: { key: 'npm', paths: ['.npm', '../escape/attempt'] } } - ); - - assert.equal(mounts.length, 2); - assert.ok(mounts[0].endsWith(':/workspace/.npm')); - // Traversal is stripped, not honoured. - assert.ok(mounts[1].endsWith(':/workspace/escape/attempt')); - for (const mount of mounts) { - const host = mount.split(':')[0]; - assert.ok((await fs.stat(host)).isDirectory()); - assert.ok(host.startsWith(dir)); - } - await fs.rm(dir, { recursive: true, force: true }); +test('an artifact pattern reduces to the part that can be copied out', () => { + // A stopped container cannot expand a glob, so only the leading + // literal part of a pattern can be asked for by name. + assert.equal(patternPrefix('out/**'), 'out'); + assert.equal(patternPrefix('build/*.tar.gz'), 'build'); + assert.equal(patternPrefix('a/b/c/**'), 'a/b/c'); + assert.equal(patternPrefix('dist/app.tar'), 'dist/app.tar'); + + // A pattern that could match anywhere has no usable prefix. + assert.equal(patternPrefix('**/*.log'), ''); + assert.equal(patternPrefix('*'), ''); +}); + +test('overlapping artifact paths are copied out once', () => { + assert.deepEqual(copyOutPaths(['out/**', 'out/deep/**', 'build/*']), ['build', 'out']); + assert.deepEqual(copyOutPaths(['out/**', 'out/**']), ['out']); + + // One pattern matching anywhere means the whole tree comes back, and + // there is no point copying anything else separately. + assert.deepEqual(copyOutPaths(['**/*.log', 'out/**']), ['']); + + // A prefix must be a whole path segment: outer must not swallow out. + assert.deepEqual(copyOutPaths(['out/**', 'outer/**']), ['out', 'outer']); }); test('artifact collection globs, and refuses to leave the workspace', async () => {