feat(compose): support registry images and remote prebuilt deploys - #5159
feat(compose): support registry images and remote prebuilt deploys#5159designly1 wants to merge 5 commits into
Conversation
Adds compose registry configuration, build pipeline updates, and migration for stored image metadata. Co-authored-by: Cursor <cursoragent@cursor.com>
Ensure deploy log directories are created on the deploy server when build and deploy run on different hosts, append deploy logs on failure, and prefix chained prebuilt compose commands with `docker compose`. Move onlyBuiltDependencies to pnpm-workspace.yaml per pnpm v10 convention. Co-authored-by: Cursor <cursoragent@cursor.com>
| const { composeId, ...rest } = input; | ||
| const updated = await updateCompose(composeId, rest); |
There was a problem hiding this comment.
Unscoped build registry credentials
When a user submits another organization's registry ID while updating an accessible Compose service, this mutation persists it without validating registry access. A later remote deployment resolves the registry by ID and uses its credentials in Docker login commands, causing cross-tenant credential use.
How this was verified: The accepted registry ID flows from the update mutation through an organization-unscoped credential lookup into the generated Docker login commands.
Knowledge Base Used: Compose Deployment Flow
| const logServerId = compose.buildServerId || compose.serverId; | ||
| const { LOGS_PATH } = paths(!!logServerId); | ||
| const formattedDateTime = format(new Date(), "yyyy-MM-dd:HH:mm:ss"); | ||
| const fileName = `${compose.appName}-${formattedDateTime}.log`; | ||
| const logFilePath = path.join(LOGS_PATH, compose.appName, fileName); | ||
|
|
||
| if (compose.serverId) { | ||
| const server = await findServerById(compose.serverId); | ||
| if (logServerId) { |
There was a problem hiding this comment.
Build-server logs escape cleanup
When a Compose service has distinct build and deploy servers, this code creates its main log on the build server, while individual and service-level deletion paths still target deployment.serverId or compose.serverId. Deleting deployment records or the service therefore leaves the corresponding logs on the build server, causing orphaned disk usage.
Knowledge Base Used: Application Deployment Flow
Reject buildRegistryId from other organizations on compose update, and delete deployment logs from buildServerId when present so split build/deploy stacks do not leave orphaned files. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks @greptile-apps — both P1s were valid and are fixed in 38bcee3:
Note: applications already have a similar |
|
Both fixes look correct. Registry ACL ( Log cleanup ( The pre-existing application gap note is fair — it predates this PR and is a separate surface. Both P1s here are properly addressed. Tip: You can customize Greptile's behavior for this repo with |
| if (input.buildRegistryId) { | ||
| const reg = await findRegistryById(input.buildRegistryId); | ||
| if (reg.organizationId !== ctx.session.activeOrganizationId) { |
There was a problem hiding this comment.
Cross-tenant registry association persists
If a session's accessed-services list contains a Compose service from another organization, this mutation validates the submitted registry against only the active organization and then persists it on that foreign Compose service. A subsequent remote deployment uses the associated registry credentials for Docker login, causing cross-tenant credential use.
How this was verified: The permission check does not establish the Compose service's active-organization ownership before the organization-scoped registry ID reaches updateCompose.
Knowledge Base Used: Compose Deployment Flow
Name server-side compose relations to match compose.buildServerId and count both deploy-target and build-server references in haveActiveServices so remote server deletion no longer fails with ambiguous Drizzle relations. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Re-submitted with an additional hardening commit ( What changed since last review:
PR description updated with a Hardening section covering relation disambiguation, delete guardrails, registry org validation, and split build/deploy log cleanup. Validated on gpc3 ( |
| ...(compose.buildServerId && { | ||
| buildServerId: compose.buildServerId, | ||
| }), |
There was a problem hiding this comment.
Split logs read from wrong server
When a Compose deployment uses distinct build and deploy servers, this code records the build server as the main log host, but the log reader ignores deployment.buildServerId and tails deployment.compose.serverId, causing the deployment viewer to display empty logs.
Knowledge Base Used: Application Deployment Flow
Require compose updates to target a service in the active organization before persisting build registry credentials, and read deployment logs from buildServerId when split compose builds write logs on the build host. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Re-submitted with Greptile follow-up fixes in Addressed since last review:
Prior hardening commits remain: server-relation disambiguation ( Please re-review. |
Summary
pull+up --no-build) when a build server is configured, with registry login on both build and deploy hosts.docker compose.Hardening
This feature introduced a second FK from
composetoserver(buildServerId, alongside existingserverId). Several server-management paths were updated to handle that safely:Drizzle relation disambiguation
composeRelationsalready named its two server links (composeServer,composeBuildServer), butserverRelationsstill had an unnamedcompose: many(compose). Any query that loaded a server withwith: { compose: ... }— including remote server delete — failed with:Fix: mirror the application pattern on the server side:
compose→relationName: "composeServer"(deploy target)buildCompose→relationName: "composeBuildServer"(build host)Server delete guardrails
haveActiveServices()now counts both deploy-target and build-server references before allowing deletion:applications+buildApplicationscompose+buildComposePreviously only the deploy-target side was checked. A server used only as a build host could appear deletable (
totalSum === 0in the UI) even while compose stacks still referenced it viabuildServerId.Registry authorization (compose update)
compose.updatenow requires the target compose to belong to the active organization (matchingone/delete) before any fields are persisted. Registry and build-server IDs are then validated against the same active org. This closes the cross-org credential association Greptile flagged whenaccessedServicesspans organizations.Split build/deploy log cleanup and read path
buildServerIdis set, deployment log directories are removed from the build server on cleanup, preventing orphaned log files after split build/deploy runs.deployment.readLogsnow resolves the log host asdeployment.buildServerId || deployment.serverId || …, matching wherecreateDeploymentComposewrites the main log file on split build/deploy runs.Database migration
0186_sharp_arclight.sqladdscompose.buildServerIdandcompose.buildRegistryId(nullable FKs toserverandregistry).Test plan
builddirectives on the same server (no build server) — behavior unchanged--buildpnpm test apps/dokploy/__test__/compose/create-command-prebuilt.test.tspnpm test apps/dokploy/__test__/compose/build-compose-command.test.tspnpm test apps/dokploy/__test__/compose/build-registry-images.test.tsGreptile Summary
This PR adds remote build-server and registry support for Compose deployments, allowing images to be built and pushed on one host before deployment on another.
Confidence Score: 5/5
The PR appears safe to merge because no blocking failure remains.
No blocking failure remains.
Reviews (4): Last reviewed commit: "fix(compose): tighten update ACL and dep..." | Re-trigger Greptile
Context used (3)