Skip to content

feat(deployments): build source images on worker nodes - #1146

Merged
dviejokfs merged 3 commits into
mainfrom
feat/worker-build-protocol
Sep 26, 2026
Merged

dviejokfs merged 3 commits into
mainfrom
feat/worker-build-protocol

Conversation

@dviejokfs

@dviejokfs dviejokfs commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Add a versioned, authenticated worker build protocol with bounded source upload and streamed events. Ordinary workers and dedicated temps.sh/role=builder nodes use the same protocol; builder-only nodes never host replicas.
  • Build and inspect on the owning worker, then stream the image to another target when needed. The control-plane profile never uses a local Docker daemon for this path.
  • Refuse generated NPM_TOKEN/NPM_RC credentials before creating .npmrc, with a second guard before archiving. Supplied environment/build-argument values are never forwarded; Dockerfiles requiring those arguments remain unsupported pending a reviewed secret protocol.

Refs #1034.

Review fixes

Finding Change
Generated credentials reach builders Reject generated package credentials before writing the context and before uploading, independently of .dockerignore.
Only one target architecture built Enumerate the full eligible target-platform set; protocol v1 explicitly refuses mixed/unknown sets before source upload, with a single-architecture targeting remedy.
Remote architecture unchecked Require recorded platform metadata, inspect the actual builder image, and verify target architecture. Cached target tags and imported images must match the builder's identity and platform. Inspection failure does not silently pass.
Remote static build cannot deploy Reject workflows requiring DeployStaticJob before a remote build starts; explain that a full-profile control plane or prebuilt static bundle is required.
Timed-out builds retain capacity Own the build future under the deadline/disconnect supervisor. Drop it before the terminal event, releasing scratch and capacity. Blocking extraction/archive work owns its cleanup guards.
Unbounded worker errors Bound error bodies during reading (4 KiB), import/inspection JSON (64 KiB), and individual event buffers before copying.
Untyped handler errors Build/export/inspection return typed Problem Details; routes remain under agent authentication and mTLS.
Existing workers ignore label changes Document first-join-only labels and emit a startup warning for already-joined workers. No destructive re-registration or volume reset.
Eligible builders miscounted Both deployment build paths pass the project slug into platform discovery, which now resolves the same operator-declared Docker-socket grant gate as placement. A non-granting worker cannot add an unusable architecture.
Handler permission guards missing Build, inspect and export use RequireAgentAuth plus the existing permission_guard! macro: deployment-create for builds and deployment-read for image reads. The handler extractor independently verifies the node token even without router middleware.

Also preserve executable source permissions while stripping privileged mode bits, so RUN ./script.sh works after upload.

Explicit protocol-v1 limitations

  • Container source builds only, for one target architecture. Mixed-architecture builds and image-based static extraction are not implemented; they now fail before uploading source rather than producing an unusable deployment.
  • No generated package credentials or unclassified build arguments. Do not weaken these guards to forward secrets.
  • Existing worker labels are persisted at registration. The dev harness does not implement in-place relabelling.

Evidence — latest eligibility/authorization fix (5e484e4b4)

cargo test -p temps-agent --lib -- --nocapture
140 passed

cargo test -p temps-deployments worker_build_platforms --lib -- --nocapture
3 passed

cargo test -p temps-deployments services::node_scheduler --lib -- --nocapture
109 passed

cargo check --lib, strict all-target/all-feature Clippy, formatting, source attribution, and commit hooks passed again. The three targeted scheduler regressions are included in the 109-test suite, not additional tests. The new authorization tests exercise the actual three image handlers without router authentication middleware, plus denied capabilities and absence of admin/secret-reading permissions.

The agent principal represents operator-issued authority for this node, not a console user/session. Its capabilities are assigned by the server only after token verification; headers and request bodies cannot claim roles or permissions. This change does not grant user/admin identities to agents or alter console API authorization.

Rebuilt and restarted this PR's three-worker mTLS test cluster on 5e484e4b4.
Live image inspection returned HTTP 200 with the correct node token and HTTP
401 with an incorrect token. A fresh POST /api/projects/1/trigger-pipeline
returned HTTP 200; deployment 7 completed all eight jobs (status 3). The
builder-only node and target worker reported the same arm64 image identity,
the control plane had no image copy, and the deployed app returned HTTP 200.
The mixed-architecture socket-grant case is covered by the scheduler regression
tests; this physical test cluster is homogeneous arm64.

Evidence — preceding hardening revision (135d11085)

cargo test -p temps-agent -p temps-deployer -p temps-deployments worker_ --lib -- --nocapture
33 passed (3 suites)

cargo test -p temps-deployer remote::tests --lib -- --nocapture
54 passed

cargo test -p temps-deployments jobs::build_image::tests --lib -- --nocapture
19 passed

cargo test -p temps-deployments ensure_image_on_remote --lib -- --nocapture
13 passed

cargo test -p temps-agent build_handler --lib -- --nocapture
5 passed

Workspace cargo check --lib, strict cargo clippy --all-targets --all-features -- -D warnings, formatting, source attribution, shell syntax and compose validation passed. The host's existing protoc installation requires DYLD_LIBRARY_PATH=/opt/homebrew/Cellar/abseil/20250814.1/lib; all commit hooks ran without bypass. The fix commit is DCO-signed.

Live authenticated/mTLS agent checks on the rebuilt three-worker DinD cluster:

  • GET /agent/images/inspect?image=<test-image> → HTTP 200, platform=linux/arm64.
  • POST /agent/images/build with protocol version 999 → HTTP 400, Content-Type: application/problem+json, title Invalid image request.
  • Start a build with RUN echo PR1146_CANCEL_STARTED && sleep 120; disconnect the streaming client using curl --max-time 8. Observed the started marker and expected curl exit 28. Afterwards: no sleep process, no completed cancelled image, and zero worker scratch directories.
  • Immediately submit another source build with an executable script. Observed PR1146_EXECUTABLE_SOURCE_OK and terminal type=result, build_duration_ms=2110; scratch-directory count returned to zero.

The 30-minute timeout cleanup is covered by a shortened-deadline regression test; the live test exercises client disconnection rather than waiting 30 minutes.

Full deployment rerun on this revision: POST /api/projects/1/trigger-pipeline
with {"branch":"main","environment_id":1} returned HTTP 200. Deployment 6
completed all eight jobs (each status 3). The builder-only node and separate
target worker held the same arm64 image identity; the control-plane daemon had
no copy. The deployed application returned HTTP 200. The test harness's
containerd restart stall required starting the test control-plane container
again; no volumes or unrelated Docker resources were reset.

Resource behavior

Control-plane work only, no proxy hot-path changes. One build per deployment's selected architecture; source archives are limited to 512 MiB and 100,000 entries. Shared image-operation admission bounds worker concurrency, with a 30-minute queue/upload/build deadline. Build logs use a 64-entry bounded channel and 8 KiB lines; backpressure applies at saturation, and disconnect/deadline releases build resources. Error/metadata/event buffers are bounded independently of the total transfer size.

Review status

An independent read-only security/correctness re-review found no remaining concrete blocker in the eight reported paths after the follow-up corrections. This is not a substitute for the required security sign-off on the source-transfer trust boundary. CI and current-head Greptile review must finish; no merge approval is claimed.

@github-actions

github-actions Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

📓 Changelog preview

This is what your commits will add to the generated CHANGELOG.md at release time (via git-cliff). Do not edit CHANGELOG.md by hand — it is generated from your Conventional Commit messages.

## [Unreleased]

### Added

- **deployments:** Build source images on worker nodes

### Fixed

- **deployments:** Harden worker build trust and lifecycle
- **deployments:** Align build eligibility and agent authorization

Route source builds through the authenticated agent protocol so control-plane
profiles need no local Docker daemon and build-only nodes can be reused.
Do not forward resolved build arguments; refuse Dockerfiles that consume them
until credential handling receives a dedicated security review.

Signed-off-by: David Viejo <dviejo@kfs.es>
@dviejokfs
dviejokfs force-pushed the feat/worker-build-protocol branch from 3381069 to 2da2617 Compare September 26, 2026 12:54
@dviejokfs
dviejokfs marked this pull request as ready for review September 26, 2026 13:09
@greptile-apps

greptile-apps Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[High risk] Adds distributed image building on worker nodes.

The PR appears safe to merge based on this review; no outstanding finding or new actionable issue was identified.

Summary

The PR adds authenticated worker-side source builds, image inspection and transfer, builder-only node selection, and explicit refusals for unsupported build cases. The latest revision aligns platform discovery with socket-granted placement and adds handler-level authentication and permission guards.

  • Source builds and image transfers remain on workers rather than requiring control-plane Docker.
  • The new build path checks image platform and identity before remote deployment.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  C[Control plane] -->|Source and build spec| B[Selected worker or builder-only node]
  B -->|Inspect built image| C
  B -->|Image export stream| C
  C -->|Image import stream when needed| T[Replica target worker]
  T -->|Inspect identity and platform| C
  T --> R[Run replica]
Loading

Reviews (3) · Last reviewed commit: "fix(deployments): align build eligibilit..."

Comment thread crates/temps-deployer/src/remote.rs
Comment thread crates/temps-deployments/src/services/workflow_execution_service.rs
Comment thread crates/temps-deployments/src/services/workflow_execution_service.rs
Comment thread crates/temps-agent/src/build_handler.rs Outdated
Comment thread crates/temps-deployer/src/remote.rs Outdated
Comment thread crates/temps-agent/src/build_handler.rs
Comment thread tools/dev-cluster/role-worker.sh
@greptile-apps

This comment has been minimized.

Reject generated package credentials and unsupported static or mixed-architecture builds before source upload. Verify worker image identities and platforms, bound agent responses, and cancel owned build work on deadlines and disconnects. Preserve executable source permissions and document persisted worker labels.

Signed-off-by: David Viejo <dviejo@kfs.es>
Comment thread crates/temps-deployments/src/services/workflow_execution_service.rs
Comment thread crates/temps-agent/src/build_handler.rs
Apply the operator-declared Docker socket placement gate during project build-platform discovery. Authenticate agent image handlers independently and enforce explicit deployment permissions with the shared permission guard. Cover eligibility parity, forged or missing credentials, and denied capabilities with regressions.

Signed-off-by: David Viejo <dviejo@kfs.es>
@dviejokfs
dviejokfs enabled auto-merge (squash) September 26, 2026 15:20
@dviejokfs
dviejokfs merged commit ed69c4e into main Sep 26, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant