feat: add containers tab to compose services#4218
Conversation
Add a Containers tab to the compose service page that lists all containers with their state, status, and container ID. Each container has a dropdown menu with lifecycle actions: View Logs, Restart, Start, Stop, and Kill. - Add containerStart, containerStop, containerKill functions to docker service - Add corresponding tRPC procedures with server ownership checks and audit logging - Update containerRestart to support remote servers via serverId - Create ShowComposeContainers component with table view and action menu - Add Containers tab between Deployments and Backups, gated by docker.read permission
dosubot
bot
added
the
size:L
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (input.serverId) { | ||
| const server = await findServerById(input.serverId); | ||
| if (server.organizationId !== ctx.session?.activeOrganizationId) { |
There was a problem hiding this comment.
killContainer
The killContainer procedure logs action: "stop", but the actual operation is a kill. This audit log will be misleading when trying to trace who forcefully killed a container vs. who gracefully stopped one.
| if (input.serverId) { | |
| const server = await findServerById(input.serverId); | |
| if (server.organizationId !== ctx.session?.activeOrganizationId) { | |
| await audit(ctx, { | |
| action: "kill", | |
| resourceType: "docker", |
| <div className="flex flex-col gap-4 pt-2.5"> | ||
| <ShowComposeContainers | ||
| serverId={data?.serverId || ""} | ||
| appName={data?.appName || ""} | ||
| appType={data?.composeType || "docker-compose"} |
There was a problem hiding this comment.
undefined instead of empty string for missing serverId
data?.serverId || "" passes an empty string when no server is associated. While if (serverId) checks in the service layer correctly treat "" as falsy (so execution falls through to local), passing "" to optional z.string().optional() fields is semantically incorrect and may cause confusion in future callsite reads.
| <div className="flex flex-col gap-4 pt-2.5"> | |
| <ShowComposeContainers | |
| serverId={data?.serverId || ""} | |
| appName={data?.appName || ""} | |
| appType={data?.composeType || "docker-compose"} | |
| <ShowComposeContainers | |
| serverId={data?.serverId || undefined} | |
| appName={data?.appName || ""} | |
| appType={data?.composeType || "docker-compose"} | |
| /> |
| action: "start", | ||
| resourceType: "docker", | ||
| resourceId: input.containerId, | ||
| resourceName: input.containerId, |
There was a problem hiding this comment.
docker.read permission
startContainer, stopContainer, and killContainer all use withPermission("docker", "read"), the same level used for read-only queries like getContainers. A user with only read access to docker can start, stop, or kill any container by calling the API directly, bypassing the intent of a read-only role. Consider requiring a write/update permission for these state-changing operations. (Note: the pre-existing restartContainer and removeContainer procedures share this pattern.)
Change restartContainer, startContainer, stopContainer, and killContainer endpoints to use service.read instead of docker.read so members with access to the compose can use container lifecycle actions.
Eliminate redundant import statement for ShowComposeContainers in the compose service page, streamlining the code and improving readability.
- Use "kill" audit action for killContainer instead of "stop" - Pass undefined instead of empty string for optional serverId
Change the audit action from "kill" to "stop" for the containerKill function to better reflect the operation being performed. This aligns the logging with the intended action and improves clarity in audit records.
1 participant
What is this PR about?
Adds a Containers tab to the compose service page, allowing users to inspect each container in a compose deployment and run basic lifecycle actions (View Logs, Restart, Start, Stop, Kill) directly from the UI.
Changes
packages/server/src/services/docker.ts— AddedcontainerStart,containerStop,containerKillfunctions with remote server support. UpdatedcontainerRestartto also supportserverId.apps/dokploy/server/api/routers/docker.ts— AddedstartContainer,stopContainer,killContainertRPC procedures with server ownership checks and audit logging. UpdatedrestartContainerto acceptserverId.apps/dokploy/components/dashboard/compose/containers/show-compose-containers.tsx— New component with a table showing container name, state (badge), status, and container ID. Each row has a dropdown menu with lifecycle actions and a View Logs dialog.apps/dokploy/pages/.../compose/[composeId].tsx— Added the Containers tab between Deployments and Backups, gated bydocker.readpermission.Checklist
canarybranch.Issues related (if applicable)
N/A
Screenshots (if applicable)
N/A
Greptile Summary
This PR adds a Containers tab to compose service pages, letting users inspect running containers and trigger lifecycle actions (start, stop, restart, kill) directly from the UI. The server-side service functions are well-structured, correctly supporting both local and remote server execution, and the old silent-error-swallowing in
containerRestartis properly fixed.killContaineraudit log recordsaction: \"stop\"instead of\"kill\", making kill events indistinguishable from stop events in the audit trail.startContainer,stopContainer, andkillContainerare gated bydocker.readpermission — consistent with the pre-existing pattern forrestartContainer/removeContainer, but worth revisiting so read-only roles cannot mutate container state via the API.Confidence Score: 4/5
Safe to merge with minor cleanup; no functional regressions introduced.
All findings are P2: the wrong audit action label for kill, the empty-string serverId, and the read-permission-for-write-mutations concern (which follows the existing codebase pattern). None block the primary user path. Score is 4 rather than 5 to flag the permission model concern as worth addressing before the pattern spreads further.
apps/dokploy/server/api/routers/docker.ts — audit action label for killContainer and permission level for mutation procedures.
Reviews (1): Last reviewed commit: "[autofix.ci] apply automated fixes" | Re-trigger Greptile