feat(deployments): build source images on worker nodes - #1146
Merged
Merged
Conversation
📓 Changelog previewThis is what your commits will add to the generated ## [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
force-pushed
the
feat/worker-build-protocol
branch
from
September 26, 2026 12:54
3381069 to
2da2617
Compare
dviejokfs
marked this pull request as ready for review
September 26, 2026 13:09
|
This comment has been minimized.
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>
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
enabled auto-merge (squash)
September 26, 2026 15:20
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
temps.sh/role=buildernodes use the same protocol; builder-only nodes never host replicas.NPM_TOKEN/NPM_RCcredentials 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
.dockerignore.DeployStaticJobbefore a remote build starts; explain that a full-profile control plane or prebuilt static bundle is required.RequireAgentAuthplus the existingpermission_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.shworks after upload.Explicit protocol-v1 limitations
Evidence — latest eligibility/authorization fix (
5e484e4b4)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-pipelinereturned 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)Workspace
cargo check --lib, strictcargo clippy --all-targets --all-features -- -D warnings, formatting, source attribution, shell syntax and compose validation passed. The host's existingprotocinstallation requiresDYLD_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/buildwith protocol version 999 → HTTP 400,Content-Type: application/problem+json, titleInvalid image request.RUN echo PR1146_CANCEL_STARTED && sleep 120; disconnect the streaming client usingcurl --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.PR1146_EXECUTABLE_SOURCE_OKand terminaltype=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-pipelinewith
{"branch":"main","environment_id":1}returned HTTP 200. Deployment 6completed 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.