diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 88bebdf1..27c8cac0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -78,9 +78,9 @@ jobs: - name: Qlty check (all plugins) uses: nick-fields/retry@ce71cc2ab81d554ebbe88c79ab5975992d79ba08 # v3.0.2 with: - timeout_minutes: 5 + timeout_minutes: 8 max_attempts: 3 - retry_on: error + retry_on: any command: qlty check --all --no-progress test: diff --git a/.gitignore b/.gitignore index 1d43056a..9d6042a0 100644 --- a/.gitignore +++ b/.gitignore @@ -148,6 +148,7 @@ apps/web/content/docs/ # Storybook build output storybook-static/ -# Claude Code project instructions +# Claude Code project instructions and local config CLAUDE.md +.claude/ artifacts/ diff --git a/CHANGELOG.md b/CHANGELOG.md index d60db55c..49002a65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **Compose trigger uses Docker Engine API** — Compose-managed container updates now use the Docker Engine API directly (pull, stop, recreate) instead of shelling out to `docker compose` / `docker-compose` CLI. Eliminates `spawn docker ENOENT` errors in environments without Docker CLI binaries installed. +- **Compose self-update delegates to parent orchestrator** — Self-update for compose-managed Drydock containers now uses the parent Docker trigger's helper-container transition with health gates and rollback, instead of direct stop/recreate. +- **Compose runtime refresh extracted** — Shared `refreshComposeServiceWithDockerApi()` helper eliminates the recursive `recreateContainer` → `updateContainerWithCompose` call chain. Both code paths now converge on the same explicit, non-recursive method. +- **Compose-file-once batch mode re-enabled** — `COMPOSEFILEONCE=true` now works with the Docker Engine API runtime. First container per service gets a full runtime refresh; subsequent containers sharing the same service skip the refresh. - **Self-update controller testable entrypoint** — Extracted process-level entry logic to a separate entrypoint module, making the controller independently testable without triggering process-exit side effects. - **Dockercompose YAML patching simplified** — Removed redundant type guards and dead-code branches from compose file patching helpers, reducing code paths and improving maintainability. - **Dashboard fetches recent-status from backend** — Dashboard now fetches pre-computed container statuses from `/api/containers/recent-status` instead of scanning the raw audit log client-side. @@ -118,6 +122,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Action buttons disable and show spinner during in-progress actions** — Container action buttons (Stop, Start, Restart, Update, Delete) now show a disabled state with a spinner while the action runs in the background, providing clear visual feedback. The confirm dialog closes immediately on accept instead of blocking the UI. +- **Command palette clears stale filter on navigation** — Navigating to a container via Ctrl+K search now clears the active `filterKind`, preventing stale filter state from hiding the navigated container. +- **Manual update button works with compose triggers** — The update container endpoint now searches for both `docker` and `dockercompose` trigger types, matching the existing preview endpoint behavior. Previously, users with only a compose trigger saw "No docker trigger found for this container". +- **CI: qlty retry on timeout** — Changed `retry_on` from `error` to `any` so qlty timeouts trigger retries. Increased timeout from 5 to 8 minutes. +- **OIDC docs clarified `DD_PUBLIC_URL` requirement** — OIDC documentation now explicitly marks `DD_PUBLIC_URL` as required and includes it in all provider example configurations (Authelia, Auth0, Authentik, Dex). Without this variable, the OIDC provider fails to register at startup. +- **Compose trigger docs updated for Engine API** — Removed stale references to `docker compose config --quiet` CLI validation and `docker compose pull`/`up` commands. Docs now reflect in-process YAML validation and Docker Engine API runtime. Added callout that Docker Compose CLI is not required. +- **Container actions docs updated for compose triggers** — Update action documentation now mentions both Docker and Docker Compose trigger support. - **Compose trigger rejects compose files mounted outside app directory** — Removed overly strict working-directory boundary enforcement from `runComposeCommand` that rejected compose files bind-mounted outside `/home/node/app`, breaking documented mount patterns like `/drydock/docker-compose.yml`. Compose file paths are operator-configured and already validated during resolution. - **Compose trigger uses host paths instead of container paths** — Docker label auto-detection (`com.docker.compose.project.config_files`) now remaps host-side paths to container-internal paths using Drydock's own bind mount information. Previously, host paths like `/mnt/volume1/docker/stacks/monitoring/compose.yaml` were used directly inside the container where they don't exist, causing "does not exist" errors even when the file was properly mounted. - **Compose trigger logs spurious warnings for unrelated containers** — When multiple compose triggers are configured, each trigger now silently skips containers whose resolved compose files don't match the trigger's configured `FILE` path, eliminating noisy cross-stack "does not exist" warnings. diff --git a/README.md b/README.md index a11e71c7..be47edfa 100644 --- a/README.md +++ b/README.md @@ -204,7 +204,7 @@ Docker Hub, GHCR, ECR, GCR, GAR, GitLab, Quay, Harbor, Artifactory, Nexus, and m

Docker Compose Updates

-Auto-pull and recreate services via docker-compose with service-scoped compose image patching +Auto-pull and recreate services via Docker Engine API with YAML-preserving service-scoped image patching

Distributed Agents

diff --git a/app/api/backup.test.ts b/app/api/backup.test.ts index fa3ed84f..209b132b 100644 --- a/app/api/backup.test.ts +++ b/app/api/backup.test.ts @@ -306,6 +306,55 @@ describe('Backup Router', () => { ); }); + test('should rollback successfully with a dockercompose trigger', async () => { + const handler = getHandler('post', '/:id/rollback'); + const container = { + id: 'c1', + name: 'nginx', + image: { registry: { name: 'hub' } }, + }; + const latestBackup = { + id: 'b1', + containerId: 'c1', + imageName: 'library/nginx', + imageTag: '1.24', + }; + + mockGetContainer.mockReturnValue(container); + mockGetBackupsByName.mockReturnValue([latestBackup]); + + const mockCurrentContainer = {}; + const mockContainerSpec = { State: { Running: true } }; + const composeTrigger = { + type: 'dockercompose', + getWatcher: vi.fn(() => ({ dockerApi: {} })), + pullImage: vi.fn().mockResolvedValue(undefined), + getCurrentContainer: vi.fn().mockResolvedValue(mockCurrentContainer), + inspectContainer: vi.fn().mockResolvedValue(mockContainerSpec), + stopAndRemoveContainer: vi.fn().mockResolvedValue(undefined), + recreateContainer: vi.fn().mockResolvedValue(undefined), + }; + mockGetState.mockReturnValue({ + trigger: { 'dockercompose.default': composeTrigger }, + registry: { hub: { getAuthPull: vi.fn().mockResolvedValue({}) } }, + }); + + const req = createMockRequest({ params: { id: 'c1' } }); + const res = createMockResponse(); + await handler(req, res); + + expect(composeTrigger.pullImage).toHaveBeenCalled(); + expect(composeTrigger.stopAndRemoveContainer).toHaveBeenCalled(); + expect(composeTrigger.recreateContainer).toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Container rolled back successfully', + backup: latestBackup, + }), + ); + }); + test('should rollback successfully when a valid backupId is provided', async () => { const handler = getHandler('post', '/:id/rollback'); const container = { diff --git a/app/api/container-actions.test.ts b/app/api/container-actions.test.ts index 034ac37d..42c805b5 100644 --- a/app/api/container-actions.test.ts +++ b/app/api/container-actions.test.ts @@ -460,6 +460,34 @@ describe('Container Actions Router', () => { ); }); + test('should update container successfully with a dockercompose trigger', async () => { + const container = { + id: 'c1', + name: 'nginx', + image: { name: 'nginx' }, + updateAvailable: true, + }; + const updatedContainer = { ...container, image: { name: 'nginx:latest' } }; + mockGetContainer.mockReturnValueOnce(container).mockReturnValueOnce(updatedContainer); + const mockTriggerFn = vi.fn().mockResolvedValue(undefined); + const trigger = { type: 'dockercompose', trigger: mockTriggerFn }; + mockGetState.mockReturnValue({ trigger: { 'dockercompose.default': trigger } }); + + const handler = getHandler('post', '/:id/update'); + const req = createMockRequest({ params: { id: 'c1' } }); + const res = createMockResponse(); + await handler(req, res); + + expect(mockTriggerFn).toHaveBeenCalledWith(container); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Container updated successfully', + result: updatedContainer, + }), + ); + }); + test('should return 404 when container not found', async () => { mockGetContainer.mockReturnValue(undefined); diff --git a/app/api/container-actions.ts b/app/api/container-actions.ts index a1863d09..7bb8778c 100644 --- a/app/api/container-actions.ts +++ b/app/api/container-actions.ts @@ -163,7 +163,9 @@ async function updateContainer(req: Request, res: Response) { return; } - const trigger = findDockerTriggerForContainer(registry.getState().trigger, container); + const trigger = findDockerTriggerForContainer(registry.getState().trigger, container, { + triggerTypes: ['docker', 'dockercompose'], + }); if (!trigger) { sendErrorResponse(res, 404, NO_DOCKER_TRIGGER_FOUND_ERROR); return; diff --git a/app/api/docker-trigger.test.ts b/app/api/docker-trigger.test.ts index ba0182d1..82db59d9 100644 --- a/app/api/docker-trigger.test.ts +++ b/app/api/docker-trigger.test.ts @@ -26,7 +26,7 @@ describe('docker-trigger helper', () => { expect(result).toBeUndefined(); }); - test('ignores compose triggers by default', () => { + test('includes compose triggers by default', () => { const composeTrigger = { type: 'dockercompose' }; const result = findDockerTriggerForContainer( @@ -36,10 +36,10 @@ describe('docker-trigger helper', () => { { id: 'c1' }, ); - expect(result).toBeUndefined(); + expect(result).toBe(composeTrigger); }); - test('can include compose triggers when requested', () => { + test('can limit trigger types when requested', () => { const composeTrigger = { type: 'dockercompose' }; const result = findDockerTriggerForContainer( @@ -47,10 +47,10 @@ describe('docker-trigger helper', () => { 'dockercompose.default': composeTrigger, }, { id: 'c1' }, - { triggerTypes: ['docker', 'dockercompose'] }, + { triggerTypes: ['docker'] }, ); - expect(result).toBe(composeTrigger); + expect(result).toBeUndefined(); }); test('skips docker triggers with a different agent than the container', () => { diff --git a/app/api/docker-trigger.ts b/app/api/docker-trigger.ts index bb36d37f..c5116f47 100644 --- a/app/api/docker-trigger.ts +++ b/app/api/docker-trigger.ts @@ -3,7 +3,7 @@ import type Docker from '../triggers/providers/docker/Docker.js'; import type Trigger from '../triggers/providers/Trigger.js'; export const NO_DOCKER_TRIGGER_FOUND_ERROR = 'No docker trigger found for this container'; -const DEFAULT_TRIGGER_TYPES = ['docker']; +const DEFAULT_TRIGGER_TYPES = ['docker', 'dockercompose']; interface FindDockerTriggerForContainerOptions { triggerTypes?: string[]; diff --git a/app/api/webhook.test.ts b/app/api/webhook.test.ts index e722f5f9..6da66220 100644 --- a/app/api/webhook.test.ts +++ b/app/api/webhook.test.ts @@ -1051,6 +1051,28 @@ describe('Webhook Router', () => { }); }); + test('should trigger update and return 200 with a dockercompose trigger', async () => { + const container = { name: 'my-nginx', image: { name: 'nginx' } }; + mockGetContainers.mockReturnValue([container]); + const mockTrigger = vi.fn().mockResolvedValue(undefined); + mockGetState.mockReturnValue({ + watcher: {}, + trigger: { 'dockercompose.default': { type: 'dockercompose', trigger: mockTrigger } }, + }); + + const handler = getHandler('post', '/update/:containerName'); + const req = createMockRequest({ params: { containerName: 'my-nginx' } }); + const res = createMockResponse(); + await handler(req, res); + + expect(mockTrigger).toHaveBeenCalledWith(container); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ + message: 'Update triggered for container my-nginx', + result: { container: 'my-nginx' }, + }); + }); + test('should sanitize reflected containerName in successful update response', async () => { const containerName = '\u001b[31mmy-nginx\u001b[0m\nnext'; const container = { name: containerName, image: { name: 'nginx' } }; diff --git a/app/authentications/providers/oidc/Oidc.test.ts b/app/authentications/providers/oidc/Oidc.test.ts index 6fb0a40c..58d89aa5 100644 --- a/app/authentications/providers/oidc/Oidc.test.ts +++ b/app/authentications/providers/oidc/Oidc.test.ts @@ -1,3 +1,6 @@ +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; import express from 'express'; import { ClientSecretPost, Configuration } from 'openid-client'; import * as configuration from '../../../configuration/index.js'; @@ -13,6 +16,18 @@ const configurationValid = { timeout: 5000, }; +async function createTemporaryCaFile( + contents = '-----BEGIN CERTIFICATE-----\nTEST\n-----END CERTIFICATE-----\n', +) { + const tempDirectory = await mkdtemp(path.join(os.tmpdir(), 'oidc-ca-')); + const caPath = path.join(tempDirectory, 'ca.pem'); + await writeFile(caPath, contents); + return { + caPath, + cleanup: async () => rm(tempDirectory, { recursive: true, force: true }), + }; +} + // --- Factory helpers for repeated test fixtures --- function createRes(overrides = {}) { @@ -101,6 +116,7 @@ beforeEach(() => { fetchUserInfo: vi.fn(), skipSubjectCheck: Symbol('skip-subject-check'), ClientSecretPost: vi.fn(), + customFetch: Symbol('customFetch'), discovery: vi.fn(), buildEndSessionUrl: vi.fn(), }; @@ -126,7 +142,10 @@ test('validateConfiguration should return validated configuration when valid', a configuration.ddEnvVars.DD_PUBLIC_URL = 'https://dd.example.com'; try { const validatedConfiguration = oidc.validateConfiguration(configurationValid); - expect(validatedConfiguration).toStrictEqual(configurationValid); + expect(validatedConfiguration).toStrictEqual({ + ...configurationValid, + insecure: false, + }); } finally { if (previousPublicUrl === undefined) { delete configuration.ddEnvVars.DD_PUBLIC_URL; @@ -168,7 +187,10 @@ test('validateConfiguration should allow optional logouturl override', async () logouturl: 'https://idp.example.com/logout', }; const validatedConfiguration = oidc.validateConfiguration(configWithLogoutUrl); - expect(validatedConfiguration).toStrictEqual(configWithLogoutUrl); + expect(validatedConfiguration).toStrictEqual({ + ...configWithLogoutUrl, + insecure: false, + }); } finally { if (previousPublicUrl === undefined) { delete configuration.ddEnvVars.DD_PUBLIC_URL; @@ -197,6 +219,29 @@ test('validateConfiguration should reject non-http logouturl schemes', async () } }); +test('validateConfiguration should allow cafile and insecure TLS options', async () => { + const previousPublicUrl = configuration.ddEnvVars.DD_PUBLIC_URL; + configuration.ddEnvVars.DD_PUBLIC_URL = 'https://dd.example.com'; + try { + const validatedConfiguration = oidc.validateConfiguration({ + ...configurationValid, + cafile: '/certs/private-ca.pem', + insecure: true, + }); + expect(validatedConfiguration).toStrictEqual({ + ...configurationValid, + cafile: '/certs/private-ca.pem', + insecure: true, + }); + } finally { + if (previousPublicUrl === undefined) { + delete configuration.ddEnvVars.DD_PUBLIC_URL; + } else { + configuration.ddEnvVars.DD_PUBLIC_URL = previousPublicUrl; + } + } +}); + test('getStrategy should return an Authentication strategy', async () => { const strategy = oidc.getStrategy(app); expect(strategy.name).toEqual('oidc'); @@ -310,6 +355,24 @@ test('maskConfiguration should include configured logouturl', async () => { }); }); +test('maskConfiguration should mask configured cafile', async () => { + oidc.configuration = { + ...configurationValid, + cafile: '/etc/ssl/private/oidc-ca.pem', + insecure: true, + }; + + expect(oidc.maskConfiguration()).toEqual({ + clientid: '[REDACTED]', + clientsecret: '[REDACTED]', + discovery: 'https://idp/.well-known/openid-configuration', + redirect: false, + cafile: '[REDACTED]', + insecure: true, + timeout: 5000, + }); +}); + test('getStrategyDescription should return strategy description', async () => { oidc.logoutUrl = 'https://idp/logout'; expect(oidc.getStrategyDescription()).toEqual({ @@ -1147,6 +1210,7 @@ test('initAuthentication should discover and configure client', async () => { const callArgs = openidClientMock.discovery.mock.calls[0]; expect(callArgs[4].execute).toEqual([]); + expect(callArgs[4][openidClientMock.customFetch]).toBeUndefined(); expect(oidc.logoutUrl).toBe('https://idp/logout'); }); @@ -1212,6 +1276,74 @@ test('initAuthentication should handle non-Error end session url failure', async ); }); +test('initAuthentication should configure custom fetch when cafile is set', async () => { + const { caPath, cleanup } = await createTemporaryCaFile(); + const fetchSpy = vi + .spyOn(globalThis, 'fetch') + .mockResolvedValue(new Response(null, { status: 200 }) as Response); + oidc.configuration = { + ...configurationValid, + cafile: caPath, + }; + const mockClient = {}; + openidClientMock.discovery = vi.fn().mockResolvedValue(mockClient); + openidClientMock.buildEndSessionUrl = vi.fn().mockReturnValue(new URL('https://idp/logout')); + + try { + await oidc.initAuthentication(); + + const callArgs = openidClientMock.discovery.mock.calls[0]; + const customFetch = callArgs[4][openidClientMock.customFetch]; + expect(typeof customFetch).toBe('function'); + + await customFetch('https://idp.example.com/.well-known/openid-configuration', { + method: 'GET', + }); + expect(fetchSpy).toHaveBeenCalledWith( + 'https://idp.example.com/.well-known/openid-configuration', + expect.objectContaining({ dispatcher: expect.anything() }), + ); + } finally { + fetchSpy.mockRestore(); + await cleanup(); + } +}); + +test('initAuthentication should configure custom fetch and warn when insecure TLS is enabled', async () => { + const fetchSpy = vi + .spyOn(globalThis, 'fetch') + .mockResolvedValue(new Response(null, { status: 200 }) as Response); + oidc.configuration = { + ...configurationValid, + insecure: true, + }; + const mockClient = {}; + openidClientMock.discovery = vi.fn().mockResolvedValue(mockClient); + openidClientMock.buildEndSessionUrl = vi.fn().mockReturnValue(new URL('https://idp/logout')); + + try { + await oidc.initAuthentication(); + + expect(oidc.log.warn).toHaveBeenCalledWith( + 'TLS certificate verification disabled for OIDC - do not use in production', + ); + + const callArgs = openidClientMock.discovery.mock.calls[0]; + const customFetch = callArgs[4][openidClientMock.customFetch]; + expect(typeof customFetch).toBe('function'); + + await customFetch('https://idp.example.com/.well-known/openid-configuration', { + method: 'GET', + }); + expect(fetchSpy).toHaveBeenCalledWith( + 'https://idp.example.com/.well-known/openid-configuration', + expect.objectContaining({ dispatcher: expect.anything() }), + ); + } finally { + fetchSpy.mockRestore(); + } +}); + test('initAuthentication should use configured logouturl when end session url is unsupported', async () => { oidc.configuration = { ...configurationValid, diff --git a/app/authentications/providers/oidc/Oidc.ts b/app/authentications/providers/oidc/Oidc.ts index 6b704f9c..0846d734 100644 --- a/app/authentications/providers/oidc/Oidc.ts +++ b/app/authentications/providers/oidc/Oidc.ts @@ -1,8 +1,12 @@ +import { readFile } from 'node:fs/promises'; +import type { ConnectionOptions } from 'node:tls'; import type { Request, Response } from 'express'; import rateLimit from 'express-rate-limit'; import * as openidClientLibrary from 'openid-client'; +import { Agent } from 'undici'; import { v4 as uuid } from 'uuid'; import { ddEnvVars, getPublicUrl, getServerConfiguration } from '../../../configuration/index.js'; +import { resolveConfiguredPath } from '../../../runtime/paths.js'; import { getErrorMessage } from '../../../util/error.js'; import { enforceConcurrentSessionLimit } from '../../../util/session-limit.js'; import Authentication from '../Authentication.js'; @@ -316,6 +320,8 @@ class Oidc extends Authentication { discovery: this.joi.string().uri().required(), clientid: this.joi.string().required(), clientsecret: this.joi.string().required(), + cafile: this.joi.string(), + insecure: this.joi.boolean().default(false), redirect: this.joi.boolean().default(false), logouturl: this.joi.string().uri({ scheme: ['http', 'https'] }), timeout: this.joi.number().greater(500).default(5000), @@ -341,6 +347,10 @@ class Oidc extends Authentication { discovery: this.configuration.discovery, clientid: Oidc.mask(this.configuration.clientid), clientsecret: Oidc.mask(this.configuration.clientsecret), + ...(this.configuration.cafile ? { cafile: Oidc.mask(this.configuration.cafile) } : {}), + ...(typeof this.configuration.insecure === 'boolean' + ? { insecure: this.configuration.insecure } + : {}), redirect: this.configuration.redirect, ...(this.configuration.logouturl ? { logouturl: this.configuration.logouturl } : {}), timeout: this.configuration.timeout, @@ -359,15 +369,39 @@ class Oidc extends Authentication { ); execute = [openidClient.allowInsecureRequests]; } + const discoveryOptions: openidClientLibrary.DiscoveryRequestOptions = { + timeout: timeoutSeconds, + execute, + }; + if (this.configuration.cafile || this.configuration.insecure) { + const connectOptions: ConnectionOptions = {}; + if (this.configuration.cafile) { + const caFilePath = resolveConfiguredPath(this.configuration.cafile, { + label: 'OIDC CA certificate path', + }); + connectOptions.ca = await readFile(caFilePath); + } + if (this.configuration.insecure) { + this.log.warn('TLS certificate verification disabled for OIDC - do not use in production'); + connectOptions.rejectUnauthorized = false; + } + const dispatcher = new Agent({ connect: connectOptions }); + const oidcFetch: openidClientLibrary.CustomFetch = (input, init) => + fetch( + input as RequestInfo | URL, + { + ...(init as unknown as RequestInit), + dispatcher, + } as RequestInit & { dispatcher: Agent }, + ); + discoveryOptions[openidClient.customFetch] = oidcFetch; + } this.client = await openidClient.discovery( discoveryUrl, this.configuration.clientid, this.configuration.clientsecret, openidClient.ClientSecretPost(this.configuration.clientsecret), - { - timeout: timeoutSeconds, - execute, - }, + discoveryOptions, ); this.logoutUrl = this.configuration.logouturl; try { diff --git a/app/package-lock.json b/app/package-lock.json index 696e4a51..f54f4485 100644 --- a/app/package-lock.json +++ b/app/package-lock.json @@ -51,6 +51,7 @@ "semver": "7.7.4", "set-value": "4.1.0", "sort-es": "1.7.18", + "undici": "^7.22.0", "uuid": "^13.0.0", "yaml": "2.8.2" }, @@ -7437,6 +7438,15 @@ "dev": true, "license": "MIT" }, + "node_modules/undici": { + "version": "7.22.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-7.22.0.tgz", + "integrity": "sha512-RqslV2Us5BrllB+JeiZnK4peryVTndy9Dnqq62S3yYRRTj0tFQCwEniUy2167skdGOy3vqRzEvl1Dm4sV2ReDg==", + "license": "MIT", + "engines": { + "node": ">=20.18.1" + } + }, "node_modules/undici-types": { "version": "7.18.2", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.18.2.tgz", diff --git a/app/package.json b/app/package.json index f2b2a407..c54c5d8f 100644 --- a/app/package.json +++ b/app/package.json @@ -10,7 +10,6 @@ "scripts": { "build": "tsc && node scripts/copy-static-assets.mjs", "start": "nodemon --exec \"ts-node --esm\" index.ts", - "doc": ".docsify serve ./docs", "format": "biome format --write .", "lint:fix": "biome check --fix .", "lint": "biome check .", @@ -63,6 +62,7 @@ "semver": "7.7.4", "set-value": "4.1.0", "sort-es": "1.7.18", + "undici": "^7.22.0", "uuid": "^13.0.0", "yaml": "2.8.2" }, diff --git a/app/runtime/child-process-env.test.ts b/app/runtime/child-process-env.test.ts index 9171a001..942272dd 100644 --- a/app/runtime/child-process-env.test.ts +++ b/app/runtime/child-process-env.test.ts @@ -1,12 +1,8 @@ -import { - buildComposeCommandEnvironment, - buildHookCommandEnvironment, -} from './child-process-env.js'; +import { buildHookCommandEnvironment } from './child-process-env.js'; describe('runtime/child-process-env', () => { - test('returns empty env objects when parent env is empty', () => { + test('returns empty env object when parent env is empty', () => { expect(buildHookCommandEnvironment({}, {})).toEqual({}); - expect(buildComposeCommandEnvironment({})).toEqual({}); }); test('buildHookCommandEnvironment keeps only base allowlisted vars and applies overrides', () => { @@ -43,59 +39,14 @@ describe('runtime/child-process-env', () => { }); }); - test('buildComposeCommandEnvironment keeps compose and docker env vars only', () => { + test('buildHookCommandEnvironment can inherit values from allowlisted prefixes', () => { const parentEnv = { - PATH: '/usr/bin', - DOCKER_HOST: 'unix:///var/run/docker.sock', - COMPOSE_FILE: 'compose.yaml', - COMPOSE_PROJECT_NAME: 'drydock', - CUSTOM_VAR: 'nope', - HTTP_PROXY: 'http://proxy', - NO_PROXY: undefined, - }; - - expect(buildComposeCommandEnvironment(parentEnv)).toEqual({ - PATH: '/usr/bin', - DOCKER_HOST: 'unix:///var/run/docker.sock', - COMPOSE_FILE: 'compose.yaml', - COMPOSE_PROJECT_NAME: 'drydock', - HTTP_PROXY: 'http://proxy', - }); - }); - - test('buildComposeCommandEnvironment matches COMPOSE_ prefix strictly at the start', () => { - const parentEnv = { - COMPOSE_FILE: 'compose.yaml', - COMPOSE_: 'edge', - COMPOSE__EXTRA: 'double-underscore', - XCOMPOSE_FILE: 'nope', - compose_FILE: 'nope', - COMPOSE: 'nope', - }; - - expect(buildComposeCommandEnvironment(parentEnv)).toEqual({ - COMPOSE_FILE: 'compose.yaml', - COMPOSE_: 'edge', - COMPOSE__EXTRA: 'double-underscore', - }); - }); - - test('buildComposeCommandEnvironment handles large parent env input', () => { - const largeParentEnv: Record = { - PATH: '/usr/bin', - DOCKER_HOST: 'unix:///var/run/docker.sock', - COMPOSE_PROJECT_NAME: 'drydock', + DD_HOOK_CUSTOM: 'custom-value', SECRET_TOKEN: 'should-not-leak', }; - for (let index = 0; index < 5_000; index += 1) { - largeParentEnv[`UNRELATED_${index}`] = `value-${index}`; - } - - expect(buildComposeCommandEnvironment(largeParentEnv)).toEqual({ - PATH: '/usr/bin', - DOCKER_HOST: 'unix:///var/run/docker.sock', - COMPOSE_PROJECT_NAME: 'drydock', + expect(buildHookCommandEnvironment({}, parentEnv, ['DD_HOOK_'])).toEqual({ + DD_HOOK_CUSTOM: 'custom-value', }); }); }); diff --git a/app/runtime/child-process-env.ts b/app/runtime/child-process-env.ts index 13fef377..86e5e060 100644 --- a/app/runtime/child-process-env.ts +++ b/app/runtime/child-process-env.ts @@ -17,24 +17,6 @@ const BASE_ALLOWLISTED_ENV_KEYS = new Set([ 'USER', ]); -const COMPOSE_ALLOWLISTED_ENV_KEYS = new Set([ - ...BASE_ALLOWLISTED_ENV_KEYS, - 'DOCKER_BUILDKIT', - 'DOCKER_CERT_PATH', - 'DOCKER_CONFIG', - 'DOCKER_CONTEXT', - 'DOCKER_HOST', - 'DOCKER_TLS_VERIFY', - 'HTTPS_PROXY', - 'HTTP_PROXY', - 'NO_PROXY', - 'https_proxy', - 'http_proxy', - 'no_proxy', -]); - -const COMPOSE_ALLOWLISTED_ENV_PREFIXES = ['COMPOSE_']; - type ChildProcessEnv = Record; function buildAllowlistedEnvironment( @@ -61,19 +43,10 @@ function buildAllowlistedEnvironment( export function buildHookCommandEnvironment( overrides: Record = {}, parentEnv: NodeJS.ProcessEnv = process.env, + allowlistedPrefixes: readonly string[] = [], ): ChildProcessEnv { return { - ...buildAllowlistedEnvironment(parentEnv, BASE_ALLOWLISTED_ENV_KEYS), + ...buildAllowlistedEnvironment(parentEnv, BASE_ALLOWLISTED_ENV_KEYS, allowlistedPrefixes), ...overrides, }; } - -export function buildComposeCommandEnvironment( - parentEnv: NodeJS.ProcessEnv = process.env, -): ChildProcessEnv { - return buildAllowlistedEnvironment( - parentEnv, - COMPOSE_ALLOWLISTED_ENV_KEYS, - COMPOSE_ALLOWLISTED_ENV_PREFIXES, - ); -} diff --git a/app/triggers/providers/dockercompose/Dockercompose.test.ts b/app/triggers/providers/dockercompose/Dockercompose.test.ts index fe79d24c..4ccf34eb 100644 --- a/app/triggers/providers/dockercompose/Dockercompose.test.ts +++ b/app/triggers/providers/dockercompose/Dockercompose.test.ts @@ -1,4 +1,3 @@ -import { execFile } from 'node:child_process'; import { EventEmitter } from 'node:events'; import { watch } from 'node:fs'; import fs from 'node:fs/promises'; @@ -64,10 +63,6 @@ vi.mock('../../../store/container.js', () => ({ vi.mock('../../hooks/HookRunner.js', () => ({ runHook: vi.fn() })); vi.mock('../docker/HealthMonitor.js', () => ({ startHealthMonitor: vi.fn() })); -vi.mock('node:child_process', () => ({ - execFile: vi.fn(), -})); - vi.mock('node:fs', async (importOriginal) => { const actual = await importOriginal(); return { @@ -209,6 +204,37 @@ function makeExecMocks({ return { startStream, mockExec, recreatedContainer }; } +function makeDockerContainerHandle({ + running = true, + image = 'nginx:1.0.0', + id = 'container-id', + name = 'nginx', + autoRemove = false, +} = {}) { + return { + id, + inspect: vi.fn().mockResolvedValue({ + Id: id, + Name: `/${name}`, + Config: { + Image: image, + Env: [], + Labels: {}, + }, + HostConfig: { + AutoRemove: autoRemove, + }, + NetworkSettings: { + Networks: {}, + }, + State: { Running: running }, + }), + stop: vi.fn().mockResolvedValue(undefined), + remove: vi.fn().mockResolvedValue(undefined), + wait: vi.fn().mockResolvedValue(undefined), + }; +} + /** * Set up the common spies used by processComposeFile tests that exercise * the write / trigger / hooks path. @@ -290,11 +316,23 @@ describe('Dockercompose Trigger', () => { mockDockerApi = { modem: { socketPath: '/var/run/docker.sock', - }, - getContainer: vi.fn().mockReturnValue({ - inspect: vi.fn().mockResolvedValue({ - State: { Running: true }, + followProgress: vi.fn((_stream, onDone, onProgress) => { + onProgress?.({ + status: 'Pulling fs layer', + id: 'layer-1', + progressDetail: { current: 1, total: 1 }, + }); + onDone?.(null, [{ status: 'Pull complete', id: 'layer-1' }]); }), + }, + pull: vi.fn().mockResolvedValue({}), + createContainer: vi.fn().mockResolvedValue({ + start: vi.fn().mockResolvedValue(undefined), + inspect: vi.fn().mockResolvedValue({ State: { Running: true } }), + }), + getContainer: vi.fn().mockReturnValue(makeDockerContainerHandle()), + getNetwork: vi.fn().mockReturnValue({ + connect: vi.fn().mockResolvedValue(undefined), }), }; @@ -314,11 +352,6 @@ describe('Dockercompose Trigger', () => { }, }, }); - - execFile.mockImplementation((_command, _args, _options, callback) => { - callback(null, '', ''); - return {}; - }); }); // ----------------------------------------------------------------------- @@ -444,6 +477,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/portainer.yml', 'portainer', container, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -476,11 +514,21 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'nginx', tagContainer, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); expect(composeUpdateSpy).toHaveBeenCalledWith( '/opt/drydock/test/stack.yml', 'redis', digestContainer, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -509,6 +557,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'redis', container, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -538,6 +591,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'redis', container, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -565,6 +623,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'nginx', container, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -624,6 +687,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'filebrowser', container, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -1045,6 +1113,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'nginx', container1, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -1082,6 +1155,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'nginx', containerInProject, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -1191,6 +1269,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'nginx', container1, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -1198,74 +1281,62 @@ describe('Dockercompose Trigger', () => { // compose command execution // ----------------------------------------------------------------------- - test('updateContainerWithCompose should skip compose commands in dry-run mode', async () => { + test('updateContainerWithCompose should skip Docker API calls in dry-run mode', async () => { trigger.configuration.dryrun = true; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - const container = { name: 'nginx', watcher: 'local' }; + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const container = makeContainer({ name: 'nginx' }); await trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container); - expect(runComposeCommandSpy).not.toHaveBeenCalled(); + expect(pullImageSpy).not.toHaveBeenCalled(); expect(mockLog.child).toHaveBeenCalledWith({ container: 'nginx' }); expect(mockLog.info).toHaveBeenCalledWith(expect.stringContaining('dry-run mode is enabled')); }); - test('updateContainerWithCompose should run pull then up for the target service', async () => { + test('updateContainerWithCompose should pull and recreate the target service via Docker API', async () => { trigger.configuration.dryrun = false; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - mockDockerApi.getContainer.mockReturnValueOnce({ - inspect: vi.fn().mockResolvedValue({ - State: { Running: true }, - }), - }); - const container = { name: 'nginx', watcher: 'local' }; + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const stopContainerSpy = vi.spyOn(trigger, 'stopContainer').mockResolvedValue(); + const removeContainerSpy = vi.spyOn(trigger, 'removeContainer').mockResolvedValue(); + const createContainerSpy = vi.spyOn(trigger, 'createContainer').mockResolvedValue({ + start: vi.fn().mockResolvedValue(undefined), + } as any); + const startContainerSpy = vi.spyOn(trigger, 'startContainer').mockResolvedValue(); + const container = makeContainer({ name: 'nginx' }); await trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 1, - '/opt/drydock/test/stack.yml', - ['pull', 'nginx'], - mockLog, - ); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 2, - '/opt/drydock/test/stack.yml', - ['up', '-d', '--no-deps', 'nginx'], - mockLog, - ); + expect(pullImageSpy).toHaveBeenCalledTimes(1); + expect(stopContainerSpy).toHaveBeenCalledTimes(1); + expect(removeContainerSpy).toHaveBeenCalledTimes(1); + expect(createContainerSpy).toHaveBeenCalledTimes(1); + expect(startContainerSpy).toHaveBeenCalledTimes(1); }); test('updateContainerWithCompose should preserve stopped runtime state', async () => { trigger.configuration.dryrun = false; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - mockDockerApi.getContainer.mockReturnValueOnce({ - inspect: vi.fn().mockResolvedValue({ - State: { Running: false }, + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const startContainerSpy = vi.spyOn(trigger, 'startContainer').mockResolvedValue(); + vi.spyOn(trigger, 'getCurrentContainer').mockResolvedValue( + makeDockerContainerHandle({ + running: false, }), - }); - const container = { name: 'nginx', watcher: 'local' }; + ); + const container = makeContainer({ name: 'nginx' }); await trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 1, - '/opt/drydock/test/stack.yml', - ['pull', 'nginx'], - mockLog, - ); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 2, - '/opt/drydock/test/stack.yml', - ['up', '--no-start', '--no-deps', 'nginx'], - mockLog, - ); + expect(pullImageSpy).toHaveBeenCalledTimes(1); + expect(startContainerSpy).not.toHaveBeenCalled(); }); - test('updateContainerWithCompose should add force-recreate argument when requested', async () => { + test('updateContainerWithCompose should skip pull when requested and still recreate', async () => { trigger.configuration.dryrun = false; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - const container = { name: 'nginx', watcher: 'local' }; + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const createContainerSpy = vi.spyOn(trigger, 'createContainer').mockResolvedValue({ + start: vi.fn().mockResolvedValue(undefined), + } as any); + const container = makeContainer({ name: 'nginx' }); await trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container, { shouldStart: true, @@ -1273,18 +1344,14 @@ describe('Dockercompose Trigger', () => { forceRecreate: true, }); - expect(runComposeCommandSpy).toHaveBeenCalledTimes(1); - expect(runComposeCommandSpy).toHaveBeenCalledWith( - '/opt/drydock/test/stack.yml', - ['up', '-d', '--no-deps', '--force-recreate', 'nginx'], - mockLog, - ); + expect(pullImageSpy).not.toHaveBeenCalled(); + expect(createContainerSpy).toHaveBeenCalledTimes(1); }); - test('updateContainerWithCompose should pass compose file chain to compose commands', async () => { + test('updateContainerWithCompose should ignore compose file chain and use Docker API path', async () => { trigger.configuration.dryrun = false; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - const container = { name: 'nginx', watcher: 'local' }; + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const container = makeContainer({ name: 'nginx' }); const composeFiles = ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml']; await trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container, { @@ -1293,11 +1360,134 @@ describe('Dockercompose Trigger', () => { composeFiles, }); - expect(runComposeCommandSpy).toHaveBeenCalledWith( - '/opt/drydock/test/stack.yml', - ['up', '-d', '--no-deps', 'nginx'], - mockLog, - composeFiles, + expect(pullImageSpy).not.toHaveBeenCalled(); + }); + + test('updateContainerWithCompose should reuse runtime context without resolving registry manager', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ name: 'nginx' }); + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const resolveRegistryManagerSpy = vi.spyOn(trigger, 'resolveRegistryManager'); + const getWatcherSpy = vi.spyOn(trigger, 'getWatcher'); + const runtimeContext = { + dockerApi: mockDockerApi, + auth: { from: 'context' }, + newImage: 'nginx:9.9.9', + }; + + await trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container, { + runtimeContext, + }); + + expect(resolveRegistryManagerSpy).not.toHaveBeenCalled(); + expect(getWatcherSpy).not.toHaveBeenCalled(); + expect(pullImageSpy).toHaveBeenCalledWith( + runtimeContext.dockerApi, + runtimeContext.auth, + runtimeContext.newImage, + expect.anything(), + ); + }); + + test('updateContainerWithCompose should fetch auth when runtime context provides newImage without auth', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ name: 'nginx' }); + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const resolveRegistryManagerSpy = vi.spyOn(trigger, 'resolveRegistryManager'); + const getNewImageFullNameSpy = vi.spyOn(trigger, 'getNewImageFullName'); + const registryGetAuthPull = vi.fn().mockResolvedValue({ from: 'registry-auth' }); + const runtimeContext = { + dockerApi: mockDockerApi, + newImage: 'nginx:9.9.9', + registry: { + getAuthPull: registryGetAuthPull, + }, + }; + + await trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container, { + runtimeContext, + }); + + expect(resolveRegistryManagerSpy).not.toHaveBeenCalled(); + expect(getNewImageFullNameSpy).not.toHaveBeenCalled(); + expect(registryGetAuthPull).toHaveBeenCalledTimes(1); + expect(pullImageSpy).toHaveBeenCalledWith( + runtimeContext.dockerApi, + { from: 'registry-auth' }, + runtimeContext.newImage, + expect.anything(), + ); + }); + + test('updateContainerWithCompose should throw when current container cannot be resolved', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ name: 'nginx' }); + vi.spyOn(trigger, 'getCurrentContainer').mockResolvedValue(undefined); + + await expect( + trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container), + ).rejects.toThrow( + 'Unable to refresh compose service nginx from /opt/drydock/test/stack.yml because container nginx no longer exists', + ); + }); + + test('updateContainerWithCompose should surface pullImage failures and stop before recreation', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ name: 'nginx' }); + vi.spyOn(trigger, 'pullImage').mockRejectedValue(new Error('pull failed')); + const stopContainerSpy = vi.spyOn(trigger, 'stopContainer').mockResolvedValue(); + const createContainerSpy = vi.spyOn(trigger, 'createContainer').mockResolvedValue({ + start: vi.fn().mockResolvedValue(undefined), + } as any); + + await expect( + trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container), + ).rejects.toThrow('pull failed'); + + expect(stopContainerSpy).not.toHaveBeenCalled(); + expect(createContainerSpy).not.toHaveBeenCalled(); + }); + + test('updateContainerWithCompose should surface stopAndRemoveContainer failures and skip recreation', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ name: 'nginx' }); + vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + vi.spyOn(trigger, 'stopContainer').mockRejectedValue(new Error('stop failed')); + const createContainerSpy = vi.spyOn(trigger, 'createContainer').mockResolvedValue({ + start: vi.fn().mockResolvedValue(undefined), + } as any); + + await expect( + trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container), + ).rejects.toThrow('stop failed'); + + expect(createContainerSpy).not.toHaveBeenCalled(); + }); + + test('updateContainerWithCompose should surface recreateContainer failures', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ name: 'nginx' }); + vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + vi.spyOn(trigger, 'stopContainer').mockResolvedValue(); + vi.spyOn(trigger, 'removeContainer').mockResolvedValue(); + vi.spyOn(trigger, 'createContainer').mockRejectedValue(new Error('create failed')); + + await expect( + trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container), + ).rejects.toThrow('create failed'); + }); + + test('updateContainerWithCompose should throw when inspectContainer returns malformed runtime state', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ name: 'nginx' }); + vi.spyOn(trigger, 'inspectContainer').mockResolvedValue({ + Config: { Image: 'nginx:1.0.0' }, + } as any); + + await expect( + trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container), + ).rejects.toThrow( + 'Unable to refresh compose service nginx from /opt/drydock/test/stack.yml because Docker inspection data is missing runtime state', ); }); @@ -1309,7 +1499,7 @@ describe('Dockercompose Trigger', () => { ); }); - test('recreateContainer should rewrite compose service image and recreate via compose lifecycle', async () => { + test('recreateContainer should rewrite compose service image without routing through updateContainerWithCompose', async () => { const container = makeContainer({ name: 'nginx', labels: { @@ -1326,7 +1516,7 @@ describe('Dockercompose Trigger', () => { ].join('\n'); vi.spyOn(trigger, 'getComposeFile').mockResolvedValue(Buffer.from(composeFileContent)); const writeComposeFileSpy = vi.spyOn(trigger, 'writeComposeFile').mockResolvedValue(); - const composeUpdateSpy = vi.spyOn(trigger, 'updateContainerWithCompose').mockResolvedValue(); + const composeUpdateSpy = vi.spyOn(trigger, 'updateContainerWithCompose'); vi.spyOn(trigger, 'getComposeFileAsObject').mockResolvedValue( makeCompose({ nginx: { image: 'nginx:1.1.0' } }), ); @@ -1350,16 +1540,7 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', expect.stringContaining('# existing comment'), ); - expect(composeUpdateSpy).toHaveBeenCalledWith( - '/opt/drydock/test/stack.yml', - 'nginx', - container, - { - shouldStart: false, - skipPull: true, - forceRecreate: true, - }, - ); + expect(composeUpdateSpy).not.toHaveBeenCalled(); }); test('recreateContainer should fallback to registry-derived image when current spec image is missing', async () => { @@ -1393,7 +1574,7 @@ describe('Dockercompose Trigger', () => { expect(resolveContextSpy).toHaveBeenCalledWith(container, 'nginx:1.0.0'); }); - test('recreateContainer integration should update compose image and run force-recreate without pull', async () => { + test('recreateContainer integration should update compose image and recreate via Docker API without pull', async () => { trigger.configuration.dryrun = false; const container = makeContainer({ name: 'nginx', @@ -1408,7 +1589,10 @@ describe('Dockercompose Trigger', () => { vi.spyOn(trigger, 'getComposeFileAsObject').mockResolvedValue( makeCompose({ nginx: { image: 'nginx:1.1.0' } }), ); - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const createContainerSpy = vi.spyOn(trigger, 'createContainer').mockResolvedValue({ + start: vi.fn().mockResolvedValue(undefined), + } as any); await trigger.recreateContainer( mockDockerApi, @@ -1425,15 +1609,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', expect.stringContaining('nginx:1.0.0'), ); - expect(runComposeCommandSpy).toHaveBeenCalledTimes(1); - expect(runComposeCommandSpy).toHaveBeenCalledWith( - '/opt/drydock/test/stack.yml', - ['up', '-d', '--no-deps', '--force-recreate', 'nginx'], - mockLog, - ); + expect(pullImageSpy).not.toHaveBeenCalled(); + expect(createContainerSpy).toHaveBeenCalledTimes(1); }); - test('executeSelfUpdate should run compose-native self-update strategy', async () => { + test('executeSelfUpdate should delegate to parent self-update transition with hydrated runtime context', async () => { trigger.configuration.dryrun = false; const container = makeContainer({ name: 'drydock', @@ -1448,8 +1628,25 @@ describe('Dockercompose Trigger', () => { service: 'drydock', serviceDefinition: {}, }; + const currentContainer = makeDockerContainerHandle(); + const currentContainerSpec = { + Id: 'current-id', + Name: '/drydock', + State: { Running: true }, + HostConfig: { + Binds: ['/var/run/docker.sock:/var/run/docker.sock'], + }, + }; - const insertBackupSpy = vi.spyOn(trigger, 'insertContainerImageBackup'); + const getCurrentContainerSpy = vi + .spyOn(trigger, 'getCurrentContainer') + .mockResolvedValue(currentContainer); + const inspectContainerSpy = vi + .spyOn(trigger, 'inspectContainer') + .mockResolvedValue(currentContainerSpec as any); + const orchestratorExecuteSpy = vi + .spyOn(trigger.selfUpdateOrchestrator, 'execute') + .mockResolvedValue(true); const composeUpdateSpy = vi.spyOn(trigger, 'updateContainerWithCompose').mockResolvedValue(); const hooksSpy = vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); @@ -1469,13 +1666,148 @@ describe('Dockercompose Trigger', () => { ); expect(updated).toBe(true); - expect(insertBackupSpy).toHaveBeenCalledTimes(1); - expect(composeUpdateSpy).toHaveBeenCalledWith( - '/opt/drydock/test/stack.yml', - 'drydock', + expect(getCurrentContainerSpy).toHaveBeenCalledWith(mockDockerApi, container); + expect(inspectContainerSpy).toHaveBeenCalledWith(currentContainer, mockLog); + expect(orchestratorExecuteSpy).toHaveBeenCalledWith( + expect.objectContaining({ + currentContainer, + currentContainerSpec, + }), + container, + mockLog, + undefined, + ); + expect(composeUpdateSpy).not.toHaveBeenCalled(); + expect(hooksSpy).not.toHaveBeenCalled(); + }); + + test('executeSelfUpdate should reuse current container and inspection from context when available', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ + name: 'drydock', + imageName: 'codeswhat/drydock', + labels: { + 'dd.compose.file': '/opt/drydock/test/stack.yml', + 'com.docker.compose.service': 'drydock', + }, + }); + const composeContext = { + composeFile: '/opt/drydock/test/stack.yml', + service: 'drydock', + serviceDefinition: {}, + }; + const currentContainer = makeDockerContainerHandle({ id: 'context-container-id' }); + const currentContainerSpec = { + Id: 'context-id', + Name: '/drydock', + State: { Running: true }, + HostConfig: { + Binds: ['/var/run/docker.sock:/var/run/docker.sock'], + }, + }; + + const getCurrentContainerSpy = vi + .spyOn(trigger, 'getCurrentContainer') + .mockResolvedValue(makeDockerContainerHandle({ id: 'fetched-id' })); + const inspectContainerSpy = vi.spyOn(trigger, 'inspectContainer').mockResolvedValue({ + Id: 'fetched-id', + State: { Running: true }, + } as any); + const orchestratorExecuteSpy = vi + .spyOn(trigger.selfUpdateOrchestrator, 'execute') + .mockResolvedValue(true); + + const updated = await trigger.executeSelfUpdate( + { + dockerApi: mockDockerApi, + registry: getState().registry.hub, + auth: {}, + newImage: 'codeswhat/drydock:1.1.0', + currentContainer, + currentContainerSpec, + }, + container, + mockLog, + 'op-self-update-context', + composeContext, + ); + + expect(updated).toBe(true); + expect(getCurrentContainerSpy).not.toHaveBeenCalled(); + expect(inspectContainerSpy).not.toHaveBeenCalled(); + expect(orchestratorExecuteSpy).toHaveBeenCalledWith( + expect.objectContaining({ + currentContainer, + currentContainerSpec, + }), + container, + mockLog, + 'op-self-update-context', + ); + }); + + test('executeSelfUpdate should inspect context current container when inspection is missing', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ + name: 'drydock', + imageName: 'codeswhat/drydock', + labels: { + 'dd.compose.file': '/opt/drydock/test/stack.yml', + 'com.docker.compose.service': 'drydock', + }, + }); + const composeContext = { + composeFile: '/opt/drydock/test/stack.yml', + service: 'drydock', + serviceDefinition: {}, + }; + const currentContainer = makeDockerContainerHandle({ id: 'context-container-id' }); + const currentContainerSpec = { + Id: 'context-inspected-id', + Name: '/drydock', + State: { Running: true }, + HostConfig: { + Binds: ['/var/run/docker.sock:/var/run/docker.sock'], + }, + }; + + const getCurrentContainerSpy = vi + .spyOn(trigger, 'getCurrentContainer') + .mockResolvedValue(makeDockerContainerHandle({ id: 'fetched-id' })); + const inspectContainerSpy = vi + .spyOn(trigger, 'inspectContainer') + .mockResolvedValue(currentContainerSpec as any); + const orchestratorExecuteSpy = vi + .spyOn(trigger.selfUpdateOrchestrator, 'execute') + .mockResolvedValue(true); + + const updated = await trigger.executeSelfUpdate( + { + dockerApi: mockDockerApi, + registry: getState().registry.hub, + auth: {}, + newImage: 'codeswhat/drydock:1.1.0', + currentContainer, + currentContainerSpec: null, + }, + container, + mockLog, + undefined, + composeContext, + ); + + expect(updated).toBe(true); + expect(getCurrentContainerSpy).not.toHaveBeenCalled(); + expect(inspectContainerSpy).toHaveBeenCalledWith(currentContainer, mockLog); + expect(orchestratorExecuteSpy).toHaveBeenCalledWith( + expect.objectContaining({ + currentContainer, + currentContainerSpec, + }), container, + mockLog, + undefined, ); - expect(hooksSpy).toHaveBeenCalledWith(container, 'drydock', {}); }); test('performContainerUpdate should throw when compose context is missing', async () => { @@ -1517,6 +1849,12 @@ describe('Dockercompose Trigger', () => { }; const composeUpdateSpy = vi.spyOn(trigger, 'updateContainerWithCompose').mockResolvedValue(); const hooksSpy = vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); + const getCurrentContainerSpy = vi + .spyOn(trigger, 'getCurrentContainer') + .mockResolvedValue(makeDockerContainerHandle()); + const orchestratorExecuteSpy = vi + .spyOn(trigger.selfUpdateOrchestrator, 'execute') + .mockResolvedValue(true); const updated = await trigger.executeSelfUpdate( { @@ -1538,77 +1876,13 @@ describe('Dockercompose Trigger', () => { expect(updated).toBe(false); expect(composeUpdateSpy).not.toHaveBeenCalled(); expect(hooksSpy).not.toHaveBeenCalled(); + expect(getCurrentContainerSpy).not.toHaveBeenCalled(); + expect(orchestratorExecuteSpy).not.toHaveBeenCalled(); expect(mockLog.info).toHaveBeenCalledWith( 'Do not replace the existing container because dry-run mode is enabled', ); }); - test('runComposeCommand should use docker compose when available', async () => { - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const composeFilePath = path.resolve(process.cwd(), 'test', 'stack.yml'); - - await trigger.runComposeCommand(composeFilePath, ['pull', 'nginx'], logContainer); - - expect(execFile).toHaveBeenCalledWith( - 'docker', - ['compose', '-f', composeFilePath, 'pull', 'nginx'], - expect.objectContaining({ - cwd: path.dirname(composeFilePath), - }), - expect.any(Function), - ); - expect(logContainer.warn).not.toHaveBeenCalled(); - }); - - test('runComposeCommand should include ordered compose file chain when provided', async () => { - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const composeFilePath = path.resolve(process.cwd(), 'test', 'stack.yml'); - const composeOverridePath = path.resolve(process.cwd(), 'test', 'stack.override.yml'); - - await trigger.runComposeCommand(composeFilePath, ['pull', 'nginx'], logContainer, [ - composeFilePath, - composeOverridePath, - ]); - - expect(execFile).toHaveBeenCalledWith( - 'docker', - ['compose', '-f', composeFilePath, '-f', composeOverridePath, 'pull', 'nginx'], - expect.objectContaining({ - cwd: path.dirname(composeFilePath), - }), - expect.any(Function), - ); - }); - - test('runComposeCommand should not forward non-allowlisted parent environment variables', async () => { - const secretKey = 'DRYDOCK_TEST_COMPOSE_SECRET'; - const originalSecret = process.env[secretKey]; - const originalPath = process.env.PATH; - process.env[secretKey] = 'top-secret'; - process.env.PATH = '/tmp/drydock-compose-path'; - const composeFilePath = path.resolve(process.cwd(), 'test', 'stack.yml'); - - try { - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - await trigger.runComposeCommand(composeFilePath, ['pull', 'nginx'], logContainer); - - const commandOptions = execFile.mock.calls[0][2]; - expect(commandOptions.env.PATH).toBe('/tmp/drydock-compose-path'); - expect(commandOptions.env[secretKey]).toBeUndefined(); - } finally { - if (originalSecret === undefined) { - delete process.env[secretKey]; - } else { - process.env[secretKey] = originalSecret; - } - if (originalPath === undefined) { - delete process.env.PATH; - } else { - process.env.PATH = originalPath; - } - } - }); - test('resolveComposeFilePath should allow absolute compose files while blocking relative traversal when boundary is enforced', () => { const composeFilePathOutsideWorkingDirectory = path.resolve( process.cwd(), @@ -1637,187 +1911,6 @@ describe('Dockercompose Trigger', () => { ).not.toThrow(); }); - test('runComposeCommand should fall back to docker-compose when docker compose plugin is missing', async () => { - execFile - .mockImplementationOnce((_command, _args, _options, callback) => { - const error = new Error('compose plugin missing'); - error.stderr = "docker: 'compose' is not a docker command."; - callback(error, '', error.stderr); - return {}; - }) - .mockImplementationOnce((_command, _args, _options, callback) => { - callback(null, '', ''); - return {}; - }); - - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const composeFilePath = path.resolve(process.cwd(), 'test', 'stack.yml'); - - await trigger.runComposeCommand(composeFilePath, ['pull', 'nginx'], logContainer); - - expect(execFile).toHaveBeenNthCalledWith( - 1, - 'docker', - ['compose', '-f', composeFilePath, 'pull', 'nginx'], - expect.objectContaining({ cwd: path.dirname(composeFilePath) }), - expect.any(Function), - ); - expect(execFile).toHaveBeenNthCalledWith( - 2, - 'docker-compose', - ['-f', composeFilePath, 'pull', 'nginx'], - expect.objectContaining({ cwd: path.dirname(composeFilePath) }), - expect.any(Function), - ); - expect(logContainer.warn).toHaveBeenCalledWith( - expect.stringContaining('trying docker-compose'), - ); - }); - - test('runComposeCommand should accept compose files outside the working directory', async () => { - execFile.mockImplementationOnce((_command, _args, _options, callback) => { - callback(null, '', ''); - return {}; - }); - - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const composeFilePath = path.resolve(process.cwd(), '..', 'external', 'compose.yaml'); - - await trigger.runComposeCommand(composeFilePath, ['up', '-d'], logContainer); - - expect(execFile).toHaveBeenCalledWith( - 'docker', - ['compose', '-f', composeFilePath, 'up', '-d'], - expect.objectContaining({ cwd: path.dirname(composeFilePath) }), - expect.any(Function), - ); - }); - - test('runComposeCommand should throw when compose command fails', async () => { - execFile.mockImplementationOnce((_command, _args, _options, callback) => { - callback(new Error('boom'), '', 'boom'); - return {}; - }); - - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const composeFilePath = path.resolve(process.cwd(), 'test', 'stack.yml'); - - await expect( - trigger.runComposeCommand(composeFilePath, ['pull', 'nginx'], logContainer), - ).rejects.toThrow(`Error when running docker compose pull nginx for ${composeFilePath} (boom)`); - - expect(execFile).toHaveBeenCalledTimes(1); - }); - - test('runComposeCommand should handle failures without stderr payload', async () => { - execFile.mockImplementationOnce((_command, _args, _options, callback) => { - callback(new Error('boom-without-stderr'), '', undefined); - return {}; - }); - - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const composeFilePath = path.resolve(process.cwd(), 'test', 'stack.yml'); - - await expect( - trigger.runComposeCommand(composeFilePath, ['pull', 'nginx'], logContainer), - ).rejects.toThrow( - `Error when running docker compose pull nginx for ${composeFilePath} (boom-without-stderr)`, - ); - }); - - test('runComposeCommand should log stdout and stderr output', async () => { - execFile.mockImplementationOnce((_command, _args, _options, callback) => { - callback(null, 'pulled image\n', 'minor warning\n'); - return {}; - }); - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const composeFilePath = path.resolve(process.cwd(), 'test', 'stack.yml'); - - await trigger.runComposeCommand(composeFilePath, ['pull', 'nginx'], logContainer); - - expect(logContainer.debug).toHaveBeenCalledWith( - expect.stringContaining('docker compose pull nginx stdout:\npulled image'), - ); - expect(logContainer.debug).toHaveBeenCalledWith( - expect.stringContaining('docker compose pull nginx stderr:\nminor warning'), - ); - }); - - test('runComposeCommand should resolve relative compose file paths outside working directory', async () => { - execFile.mockImplementationOnce((_command, _args, _options, callback) => { - callback(null, '', ''); - return {}; - }); - - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const expectedPath = path.resolve(process.cwd(), '../outside/stack.yml'); - - await trigger.runComposeCommand('../outside/stack.yml', ['pull', 'nginx'], logContainer); - - expect(execFile).toHaveBeenCalledWith( - 'docker', - ['compose', '-f', expectedPath, 'pull', 'nginx'], - expect.objectContaining({ cwd: path.dirname(expectedPath) }), - expect.any(Function), - ); - }); - - test('getContainerRunningState should assume running when inspect fails', async () => { - const logContainer = { warn: vi.fn() }; - mockDockerApi.getContainer.mockReturnValueOnce({ - inspect: vi.fn().mockRejectedValue(new Error('inspect failed')), - }); - - const running = await trigger.getContainerRunningState( - { - name: 'nginx', - watcher: 'local', - }, - logContainer, - ); - - expect(running).toBe(true); - expect(logContainer.warn).toHaveBeenCalledWith( - 'Unable to inspect running state for nginx; assuming running (inspect failed)', - ); - }); - - test('getContainerRunningState should assume running when watcher dockerApi is unavailable', async () => { - const logContainer = { warn: vi.fn() }; - - const running = await trigger.getContainerRunningState( - { - name: 'nginx', - watcher: 'missing', - }, - logContainer, - ); - - expect(running).toBe(true); - }); - - test('getContainerRunningState should assume running when watcher dockerApi is not an object', async () => { - const logContainer = { warn: vi.fn() }; - getState.mockReturnValue({ - registry: getState().registry, - watcher: { - 'docker.local': { - dockerApi: 'invalid', - }, - }, - }); - - const running = await trigger.getContainerRunningState( - { - name: 'nginx', - watcher: 'local', - }, - logContainer, - ); - - expect(running).toBe(true); - }); - test('resolveComposeServiceContext should throw when no compose file is configured', async () => { trigger.configuration.file = undefined; @@ -2582,43 +2675,17 @@ describe('Dockercompose Trigger', () => { expect(writeSpy).not.toHaveBeenCalled(); }); - test('validateComposeConfiguration should fall back to docker-compose when docker compose plugin is missing', async () => { - const executeCommandSpy = vi - .spyOn(trigger, 'executeCommand') - .mockRejectedValueOnce( - Object.assign(new Error('compose plugin missing'), { - stderr: "docker: 'compose' is not a docker command.", - }), - ) - .mockResolvedValueOnce({ stdout: '', stderr: '' }); - + test('validateComposeConfiguration should validate compose text in-process without shell commands', async () => { await trigger.validateComposeConfiguration( '/opt/drydock/test/compose.yml', 'services:\n nginx:\n image: nginx:1.1.0\n', ); - - expect(executeCommandSpy).toHaveBeenNthCalledWith( - 1, - 'docker', - ['compose', '-f', expect.stringContaining('.compose.yml.validate-'), 'config', '--quiet'], - expect.objectContaining({ - cwd: '/opt/drydock/test', - }), - ); - expect(executeCommandSpy).toHaveBeenNthCalledWith( - 2, - 'docker-compose', - ['-f', expect.stringContaining('.compose.yml.validate-'), 'config', '--quiet'], - expect.objectContaining({ - cwd: '/opt/drydock/test', - }), - ); }); test('validateComposeConfiguration should validate updated file against full compose file chain', async () => { - const executeCommandSpy = vi - .spyOn(trigger, 'executeCommand') - .mockResolvedValueOnce({ stdout: '', stderr: '' }); + const getComposeFileAsObjectSpy = vi + .spyOn(trigger, 'getComposeFileAsObject') + .mockResolvedValue(makeCompose({ base: { image: 'busybox:1.0.0' } })); await trigger.validateComposeConfiguration( '/opt/drydock/test/stack.override.yml', @@ -2628,21 +2695,7 @@ describe('Dockercompose Trigger', () => { }, ); - expect(executeCommandSpy).toHaveBeenCalledWith( - 'docker', - [ - 'compose', - '-f', - '/opt/drydock/test/stack.yml', - '-f', - expect.stringContaining('.stack.override.yml.validate-'), - 'config', - '--quiet', - ], - expect.objectContaining({ - cwd: '/opt/drydock/test', - }), - ); + expect(getComposeFileAsObjectSpy).toHaveBeenCalledWith('/opt/drydock/test/stack.yml'); }); test('updateComposeServiceImageInText should throw when compose document has parse errors', () => { @@ -3583,7 +3636,7 @@ describe('Dockercompose Trigger', () => { expect(parseDocumentSpy).toHaveBeenCalledTimes(1); }); - test('processComposeFile should batch compose pull/up by file when composeFileOnce mode is enabled', async () => { + test('processComposeFile should refresh each distinct service in compose-file-once mode', async () => { trigger.configuration.dryrun = false; trigger.configuration.prune = false; trigger.configuration.composeFileOnce = true; @@ -3618,7 +3671,9 @@ describe('Dockercompose Trigger', () => { ), ); vi.spyOn(trigger, 'writeComposeFile').mockResolvedValue(); - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); + const runContainerUpdateLifecycleSpy = vi + .spyOn(trigger, 'runContainerUpdateLifecycle') + .mockResolvedValue(); vi.spyOn(trigger, 'maybeScanAndGateUpdate').mockResolvedValue(); vi.spyOn(trigger, 'runPreUpdateHook').mockResolvedValue(); vi.spyOn(trigger, 'runPostUpdateHook').mockResolvedValue(); @@ -3630,18 +3685,92 @@ describe('Dockercompose Trigger', () => { redisContainer, ]); - expect(runComposeCommandSpy).toHaveBeenCalledTimes(2); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( + expect(runContainerUpdateLifecycleSpy).toHaveBeenCalledTimes(2); + expect(runContainerUpdateLifecycleSpy).toHaveBeenNthCalledWith( 1, - '/opt/drydock/test/stack.yml', - ['pull', 'nginx', 'redis'], - expect.anything(), + nginxContainer, + expect.objectContaining({ + service: 'nginx', + composeFileOnceApplied: false, + }), ); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( + expect(runContainerUpdateLifecycleSpy).toHaveBeenNthCalledWith( 2, + redisContainer, + expect.objectContaining({ + service: 'redis', + composeFileOnceApplied: false, + }), + ); + }); + + test('processComposeFile should pre-pull distinct services and skip per-service pull in compose-file-once mode', async () => { + trigger.configuration.dryrun = false; + trigger.configuration.prune = false; + trigger.configuration.composeFileOnce = true; + + const nginxContainer = makeContainer({ + labels: { 'com.docker.compose.service': 'nginx' }, + }); + const redisContainer = makeContainer({ + name: 'redis', + imageName: 'redis', + tagValue: '7.0.0', + remoteValue: '7.1.0', + labels: { 'com.docker.compose.service': 'redis' }, + }); + + vi.spyOn(trigger, 'getComposeFileAsObject').mockResolvedValue( + makeCompose({ + nginx: { image: 'nginx:1.0.0' }, + redis: { image: 'redis:7.0.0' }, + }), + ); + vi.spyOn(trigger, 'getComposeFile').mockResolvedValue( + Buffer.from( + [ + 'services:', + ' nginx:', + ' image: nginx:1.0.0', + ' redis:', + ' image: redis:7.0.0', + '', + ].join('\n'), + ), + ); + vi.spyOn(trigger, 'writeComposeFile').mockResolvedValue(); + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const updateContainerWithComposeSpy = vi + .spyOn(trigger, 'updateContainerWithCompose') + .mockResolvedValue(); + vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); + vi.spyOn(trigger, 'maybeScanAndGateUpdate').mockResolvedValue(); + vi.spyOn(trigger, 'runPreUpdateHook').mockResolvedValue(); + vi.spyOn(trigger, 'runPostUpdateHook').mockResolvedValue(); + vi.spyOn(trigger, 'cleanupOldImages').mockResolvedValue(); + vi.spyOn(trigger, 'maybeStartAutoRollbackMonitor').mockResolvedValue(); + + await trigger.processComposeFile('/opt/drydock/test/stack.yml', [ + nginxContainer, + redisContainer, + ]); + + expect(pullImageSpy).toHaveBeenCalledTimes(2); + expect(updateContainerWithComposeSpy).toHaveBeenCalledWith( '/opt/drydock/test/stack.yml', - ['up', '-d', '--no-deps', 'nginx', 'redis'], - expect.anything(), + 'nginx', + nginxContainer, + expect.objectContaining({ + skipPull: true, + }), + ); + expect(updateContainerWithComposeSpy).toHaveBeenCalledWith( + '/opt/drydock/test/stack.yml', + 'redis', + redisContainer, + expect.objectContaining({ + skipPull: true, + }), ); }); @@ -4464,91 +4593,15 @@ describe('Dockercompose Trigger', () => { expect(composeFile).toBe('/opt/drydock/test/stack.yml'); }); - test('validateComposeConfiguration should throw a compose-command error for non-fallback failures', async () => { - vi.spyOn(trigger, 'executeCommand').mockRejectedValueOnce(new Error('invalid compose file')); - + test('validateComposeConfiguration should throw when the updated compose text is invalid YAML', async () => { await expect( trigger.validateComposeConfiguration( '/opt/drydock/test/compose.yml', - 'services:\n nginx:\n image: nginx:broken\n', + 'services:\n nginx: [\n', ), ).rejects.toThrow('Error when validating compose configuration'); }); - test('updateComposeServicesWithCompose should return immediately when no services are provided', async () => { - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - - await trigger.updateComposeServicesWithCompose('/opt/drydock/test/stack.yml', []); - - expect(runComposeCommandSpy).not.toHaveBeenCalled(); - }); - - test('updateComposeServicesWithCompose should no-op in dry-run mode', async () => { - trigger.configuration.dryrun = true; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - - await trigger.updateComposeServicesWithCompose('/opt/drydock/test/stack.yml', ['nginx']); - - expect(runComposeCommandSpy).not.toHaveBeenCalled(); - expect(mockLog.info).toHaveBeenCalledWith(expect.stringContaining('dry-run mode is enabled')); - }); - - test('updateComposeServicesWithCompose should run chain-aware pull/up for started and stopped services', async () => { - trigger.configuration.dryrun = false; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - const composeFiles = ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml']; - - await trigger.updateComposeServicesWithCompose( - '/opt/drydock/test/stack.yml', - ['nginx', 'redis'], - { - composeFiles, - serviceRunningStates: new Map([ - ['nginx', true], - ['redis', false], - ]), - }, - ); - - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 1, - '/opt/drydock/test/stack.yml', - ['pull', 'nginx', 'redis'], - expect.anything(), - composeFiles, - ); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 2, - '/opt/drydock/test/stack.yml', - ['up', '-d', '--no-deps', 'nginx'], - expect.anything(), - composeFiles, - ); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 3, - '/opt/drydock/test/stack.yml', - ['up', '--no-start', '--no-deps', 'redis'], - expect.anything(), - composeFiles, - ); - }); - - test('updateComposeServicesWithCompose should keep stopped services stopped without compose-file chain', async () => { - trigger.configuration.dryrun = false; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - - await trigger.updateComposeServicesWithCompose('/opt/drydock/test/stack.yml', ['nginx'], { - serviceRunningStates: new Map([['nginx', false]]), - }); - - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 2, - '/opt/drydock/test/stack.yml', - ['up', '--no-start', '--no-deps', 'nginx'], - expect.anything(), - ); - }); - test('mutateComposeFile should validate compose chain when multiple compose files are provided', async () => { vi.spyOn(trigger, 'getComposeFile').mockResolvedValue( Buffer.from('services:\n nginx:\n image: nginx:1.0.0\n'), @@ -4606,16 +4659,150 @@ describe('Dockercompose Trigger', () => { ); }); - test('executeSelfUpdate should pass compose chain to compose refresh when multiple files are present', async () => { + test('performContainerUpdate should pass runtime context to per-service update when available', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ + name: 'nginx', + }); + const updateContainerWithComposeSpy = vi + .spyOn(trigger, 'updateContainerWithCompose') + .mockResolvedValue(); + vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); + const runtimeContext = { + dockerApi: mockDockerApi, + auth: { from: 'context' }, + newImage: 'nginx:9.9.9', + registry: getState().registry.hub, + }; + + const updated = await trigger.performContainerUpdate( + runtimeContext as any, + container as any, + mockLog, + { + composeFile: '/opt/drydock/test/stack.override.yml', + composeFiles: ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml'], + service: 'nginx', + serviceDefinition: {}, + composeFileOnceApplied: false, + } as any, + ); + + expect(updated).toBe(true); + expect(updateContainerWithComposeSpy).toHaveBeenCalledWith( + '/opt/drydock/test/stack.override.yml', + 'nginx', + container, + { + composeFiles: ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml'], + runtimeContext, + }, + ); + }); + + test('performContainerUpdate should pass skipPull in multi-file compose context', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ + name: 'nginx', + }); + const updateContainerWithComposeSpy = vi + .spyOn(trigger, 'updateContainerWithCompose') + .mockResolvedValue(); + vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); + + const updated = await trigger.performContainerUpdate({} as any, container as any, mockLog, { + composeFile: '/opt/drydock/test/stack.override.yml', + composeFiles: ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml'], + service: 'nginx', + serviceDefinition: {}, + composeFileOnceApplied: false, + skipPull: true, + } as any); + + expect(updated).toBe(true); + expect(updateContainerWithComposeSpy).toHaveBeenCalledWith( + '/opt/drydock/test/stack.override.yml', + 'nginx', + container, + { + composeFiles: ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml'], + skipPull: true, + }, + ); + }); + + test('performContainerUpdate should avoid passing runtime context when none is available in single-file path', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ + name: 'nginx', + }); + const updateContainerWithComposeSpy = vi + .spyOn(trigger, 'updateContainerWithCompose') + .mockResolvedValue(); + vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); + + const updated = await trigger.performContainerUpdate({} as any, container as any, mockLog, { + composeFile: '/opt/drydock/test/stack.yml', + service: 'nginx', + serviceDefinition: {}, + composeFileOnceApplied: false, + } as any); + + expect(updated).toBe(true); + expect(updateContainerWithComposeSpy).toHaveBeenCalledWith( + '/opt/drydock/test/stack.yml', + 'nginx', + container, + {}, + ); + }); + + test('performContainerUpdate should skip per-service refresh when compose-file-once is already applied', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ + name: 'nginx', + }); + const updateContainerWithComposeSpy = vi + .spyOn(trigger, 'updateContainerWithCompose') + .mockResolvedValue(); + const hooksSpy = vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); + + const updated = await trigger.performContainerUpdate({} as any, container as any, mockLog, { + composeFile: '/opt/drydock/test/stack.yml', + service: 'nginx', + serviceDefinition: {}, + composeFileOnceApplied: true, + } as any); + + expect(updated).toBe(true); + expect(updateContainerWithComposeSpy).not.toHaveBeenCalled(); + expect(hooksSpy).toHaveBeenCalledWith(container, 'nginx', {}); + expect(mockLog.info).toHaveBeenCalledWith( + expect.stringContaining('Skip per-service compose refresh for nginx'), + ); + }); + + test('executeSelfUpdate should forward operation id to parent self-update transition', async () => { trigger.configuration.dryrun = false; const container = makeContainer({ name: 'drydock', imageName: 'codeswhat/drydock', }); + const currentContainer = makeDockerContainerHandle(); + const currentContainerSpec = { + Id: 'current-id', + Name: '/drydock', + State: { Running: true }, + HostConfig: { + Binds: ['/var/run/docker.sock:/var/run/docker.sock'], + }, + }; + vi.spyOn(trigger, 'getCurrentContainer').mockResolvedValue(currentContainer); + vi.spyOn(trigger, 'inspectContainer').mockResolvedValue(currentContainerSpec as any); + const executeSpy = vi.spyOn(trigger.selfUpdateOrchestrator, 'execute').mockResolvedValue(true); const updateContainerWithComposeSpy = vi .spyOn(trigger, 'updateContainerWithCompose') .mockResolvedValue(); - vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); const updated = await trigger.executeSelfUpdate( { @@ -4628,7 +4815,7 @@ describe('Dockercompose Trigger', () => { }, container, mockLog, - undefined, + 'op-self-update-123', { composeFile: '/opt/drydock/test/stack.override.yml', composeFiles: ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml'], @@ -4638,17 +4825,19 @@ describe('Dockercompose Trigger', () => { ); expect(updated).toBe(true); - expect(updateContainerWithComposeSpy).toHaveBeenCalledWith( - '/opt/drydock/test/stack.override.yml', - 'drydock', + expect(executeSpy).toHaveBeenCalledWith( + expect.objectContaining({ + currentContainer, + currentContainerSpec, + }), container, - { - composeFiles: ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml'], - }, + mockLog, + 'op-self-update-123', ); + expect(updateContainerWithComposeSpy).not.toHaveBeenCalled(); }); - test('processComposeFile should fetch running state once per service in compose-file-once mode', async () => { + test('processComposeFile should mark repeated compose services as already refreshed in compose-file-once mode', async () => { trigger.configuration.dryrun = false; trigger.configuration.composeFileOnce = true; const firstContainer = makeContainer({ @@ -4669,16 +4858,82 @@ describe('Dockercompose Trigger', () => { Buffer.from(['services:', ' nginx:', ' image: nginx:1.0.0', ''].join('\n')), ); vi.spyOn(trigger, 'writeComposeFile').mockResolvedValue(); - const runningStateSpy = vi.spyOn(trigger, 'getContainerRunningState').mockResolvedValue(true); - vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - vi.spyOn(trigger, 'runContainerUpdateLifecycle').mockResolvedValue(); + const runContainerUpdateLifecycleSpy = vi + .spyOn(trigger, 'runContainerUpdateLifecycle') + .mockResolvedValue(); await trigger.processComposeFile('/opt/drydock/test/stack.yml', [ firstContainer, secondContainer, ]); - expect(runningStateSpy).toHaveBeenCalledTimes(1); + expect(runContainerUpdateLifecycleSpy).toHaveBeenCalledTimes(2); + expect(runContainerUpdateLifecycleSpy).toHaveBeenNthCalledWith( + 1, + firstContainer, + expect.objectContaining({ + service: 'nginx', + composeFileOnceApplied: false, + }), + ); + expect(runContainerUpdateLifecycleSpy).toHaveBeenNthCalledWith( + 2, + secondContainer, + expect.objectContaining({ + service: 'nginx', + composeFileOnceApplied: true, + }), + ); + }); + + test('processComposeFile should pre-pull once for repeated compose services in compose-file-once mode', async () => { + trigger.configuration.dryrun = false; + trigger.configuration.prune = false; + trigger.configuration.composeFileOnce = true; + const firstContainer = makeContainer({ + name: 'nginx-a', + labels: { 'com.docker.compose.service': 'nginx' }, + }); + const secondContainer = makeContainer({ + name: 'nginx-b', + labels: { 'com.docker.compose.service': 'nginx' }, + }); + + vi.spyOn(trigger, 'getComposeFileAsObject').mockResolvedValue( + makeCompose({ + nginx: { image: 'nginx:1.0.0' }, + }), + ); + vi.spyOn(trigger, 'getComposeFile').mockResolvedValue( + Buffer.from(['services:', ' nginx:', ' image: nginx:1.0.0', ''].join('\n')), + ); + vi.spyOn(trigger, 'writeComposeFile').mockResolvedValue(); + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const updateContainerWithComposeSpy = vi + .spyOn(trigger, 'updateContainerWithCompose') + .mockResolvedValue(); + vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); + vi.spyOn(trigger, 'maybeScanAndGateUpdate').mockResolvedValue(); + vi.spyOn(trigger, 'runPreUpdateHook').mockResolvedValue(); + vi.spyOn(trigger, 'runPostUpdateHook').mockResolvedValue(); + vi.spyOn(trigger, 'cleanupOldImages').mockResolvedValue(); + vi.spyOn(trigger, 'maybeStartAutoRollbackMonitor').mockResolvedValue(); + + await trigger.processComposeFile('/opt/drydock/test/stack.yml', [ + firstContainer, + secondContainer, + ]); + + expect(pullImageSpy).toHaveBeenCalledTimes(1); + expect(updateContainerWithComposeSpy).toHaveBeenCalledTimes(1); + expect(updateContainerWithComposeSpy).toHaveBeenCalledWith( + '/opt/drydock/test/stack.yml', + 'nginx', + firstContainer, + expect.objectContaining({ + skipPull: true, + }), + ); }); test('preview should passthrough base preview errors without compose metadata', async () => { @@ -4758,9 +5013,9 @@ describe('Dockercompose Trigger', () => { } }); - test('updateContainerWithCompose should pass compose file chain to pull command', async () => { + test('updateContainerWithCompose should use Docker API pull regardless of compose file chain', async () => { trigger.configuration.dryrun = false; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); const composeFiles = ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml']; const container = makeContainer({ name: 'nginx', @@ -4772,13 +5027,7 @@ describe('Dockercompose Trigger', () => { skipPull: false, }); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 1, - '/opt/drydock/test/stack.yml', - ['pull', 'nginx'], - expect.anything(), - composeFiles, - ); + expect(pullImageSpy).toHaveBeenCalledTimes(1); }); test('recreateContainer should include compose file chain when compose service is defined in overrides', async () => { @@ -4795,7 +5044,9 @@ describe('Dockercompose Trigger', () => { service: 'nginx', } as any); vi.spyOn(trigger, 'mutateComposeFile').mockResolvedValue(true); - const composeUpdateSpy = vi.spyOn(trigger, 'updateContainerWithCompose').mockResolvedValue(); + const refreshComposeServiceSpy = vi + .spyOn(trigger as any, 'refreshComposeServiceWithDockerApi') + .mockResolvedValue(); await trigger.recreateContainer( mockDockerApi, @@ -4808,7 +5059,7 @@ describe('Dockercompose Trigger', () => { mockLog, ); - expect(composeUpdateSpy).toHaveBeenCalledWith( + expect(refreshComposeServiceSpy).toHaveBeenCalledWith( '/opt/drydock/test/stack.override.yml', 'nginx', container, @@ -4834,9 +5085,9 @@ describe('Dockercompose Trigger', () => { }); test('validateComposeConfiguration should append target compose file when compose chain omits it', async () => { - const executeCommandSpy = vi - .spyOn(trigger, 'executeCommand') - .mockResolvedValueOnce({ stdout: '', stderr: '' }); + const getComposeFileAsObjectSpy = vi + .spyOn(trigger, 'getComposeFileAsObject') + .mockResolvedValue(makeCompose({ base: { image: 'busybox:1.0.0' } })); await trigger.validateComposeConfiguration( '/opt/drydock/test/stack.override.yml', @@ -4846,20 +5097,6 @@ describe('Dockercompose Trigger', () => { }, ); - expect(executeCommandSpy).toHaveBeenCalledWith( - 'docker', - [ - 'compose', - '-f', - '/opt/drydock/test/stack.yml', - '-f', - expect.stringContaining('.stack.override.yml.validate-'), - 'config', - '--quiet', - ], - expect.objectContaining({ - cwd: '/opt/drydock/test', - }), - ); + expect(getComposeFileAsObjectSpy).toHaveBeenCalledWith('/opt/drydock/test/stack.yml'); }); }); diff --git a/app/triggers/providers/dockercompose/Dockercompose.ts b/app/triggers/providers/dockercompose/Dockercompose.ts index 95959772..e32ca2cb 100644 --- a/app/triggers/providers/dockercompose/Dockercompose.ts +++ b/app/triggers/providers/dockercompose/Dockercompose.ts @@ -1,18 +1,14 @@ -import { execFile } from 'node:child_process'; import { constants as fsConstants } from 'node:fs'; import fs from 'node:fs/promises'; import path from 'node:path'; import yaml, { type Pair, type ParsedNode } from 'yaml'; import type { ContainerImage } from '../../../model/container.js'; import { getState } from '../../../registry/index.js'; -import { buildComposeCommandEnvironment } from '../../../runtime/child-process-env.js'; import { resolveConfiguredPath, resolveConfiguredPathWithinBase } from '../../../runtime/paths.js'; import { sleep } from '../../../util/sleep.js'; import Docker from '../docker/Docker.js'; import ComposeFileLockManager from './ComposeFileLockManager.js'; -const COMPOSE_COMMAND_TIMEOUT_MS = 60_000; -const COMPOSE_COMMAND_MAX_BUFFER_BYTES = 10 * 1024 * 1024; const YAML_MAX_ALIAS_COUNT = 10_000; const COMPOSE_RENAME_MAX_RETRIES = 5; const COMPOSE_RENAME_RETRY_MS = 200; @@ -92,6 +88,19 @@ type RegistryImageContainerReference = { }; }; +type ComposeRuntimeRefreshOptions = { + shouldStart?: boolean; + skipPull?: boolean; + forceRecreate?: boolean; + composeFiles?: string[]; + runtimeContext?: { + dockerApi?: unknown; + auth?: unknown; + newImage?: string; + registry?: unknown; + }; +}; + function getDockerApiFromWatcher(watcher: unknown): DockerApiLike | undefined { if (!watcher || typeof watcher !== 'object') { return undefined; @@ -1084,148 +1093,25 @@ class Dockercompose extends Docker { const effectiveComposeFileChain = composeFileChain.includes(composeFilePath) ? composeFileChain : [...composeFileChain, composeFilePath]; - - const composeDirectory = path.dirname(composeFilePath); - const composeFileName = path.basename(composeFilePath); - const validationFilePath = path.join( - composeDirectory, - `.${composeFileName}.validate-${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}`, - ); - - await fs.writeFile(validationFilePath, composeFileText); - const validationArguments = [ - ...effectiveComposeFileChain.flatMap((composeFile) => [ - '-f', - composeFile === composeFilePath ? validationFilePath : composeFile, - ]), - 'config', - '--quiet', - ]; - const commandsToTry = [ - { - command: 'docker', - args: ['compose', ...validationArguments], - label: 'docker compose', - }, - { - command: 'docker-compose', - args: validationArguments, - label: 'docker-compose', - }, - ]; - try { - for (const composeCommand of commandsToTry) { - try { - await this.executeCommand(composeCommand.command, composeCommand.args, { - cwd: composeDirectory, - env: buildComposeCommandEnvironment(), - }); - return; - } catch (e) { - const stderr = `${e?.stderr || ''}`; - const dockerComposePluginMissing = - composeCommand.command === 'docker' && - /docker: ['"]?compose['"]? is not a docker command/i.test(stderr); - const executableMissing = e?.code === 'ENOENT'; - - if ( - composeCommand.command === 'docker' && - (dockerComposePluginMissing || executableMissing) - ) { - this.log.warn( - `Cannot use docker compose for compose validation on ${composeFilePath} (${e.message}); trying docker-compose`, - ); - continue; - } - - throw new Error( - `Error when validating compose configuration for ${composeFilePath} using ${composeCommand.label} (${e.message})`, + const composeByFile = new Map(); + for (const composeFile of effectiveComposeFileChain) { + if (composeFile === composeFilePath) { + composeByFile.set( + composeFile, + yaml.parse(composeFileText, { + maxAliasCount: YAML_MAX_ALIAS_COUNT, + }), ); + continue; } + composeByFile.set(composeFile, await this.getComposeFileAsObject(composeFile)); } - } finally { - await this.cleanupComposeTemporaryFile(validationFilePath); - } - } - - async updateComposeServicesWithCompose(composeFile, services, options = {}) { - if (services.length === 0) { - return; - } - - const { composeFiles = [composeFile], serviceRunningStates = new Map() } = - options as { - composeFiles?: string[]; - serviceRunningStates?: Map; - }; - const composeFileChain = this.normalizeComposeFileChain(composeFile, composeFiles); - const runWithComposeFileChain = composeFileChain.length > 1; - - const logContainer = this.log.child({ - composeFile, - services, - }); - - if (this.configuration.dryrun) { - logContainer.info( - `Do not refresh compose services ${services.join(', ')} from ${composeFile} because dry-run mode is enabled`, - ); - return; - } - - if (runWithComposeFileChain) { - await this.runComposeCommand( - composeFile, - ['pull', ...services], - logContainer, - composeFileChain, + await this.getComposeFileChainAsObject(effectiveComposeFileChain, composeByFile); + } catch (e) { + throw new Error( + `Error when validating compose configuration for ${composeFilePath} (${e.message})`, ); - } else { - await this.runComposeCommand(composeFile, ['pull', ...services], logContainer); - } - - const servicesToStart: string[] = []; - const servicesToKeepStopped: string[] = []; - for (const service of services) { - if (serviceRunningStates.get(service) === false) { - servicesToKeepStopped.push(service); - } else { - servicesToStart.push(service); - } - } - - if (servicesToStart.length > 0) { - if (runWithComposeFileChain) { - await this.runComposeCommand( - composeFile, - ['up', '-d', '--no-deps', ...servicesToStart], - logContainer, - composeFileChain, - ); - } else { - await this.runComposeCommand( - composeFile, - ['up', '-d', '--no-deps', ...servicesToStart], - logContainer, - ); - } - } - if (servicesToKeepStopped.length > 0) { - if (runWithComposeFileChain) { - await this.runComposeCommand( - composeFile, - ['up', '--no-start', '--no-deps', ...servicesToKeepStopped], - logContainer, - composeFileChain, - ); - } else { - await this.runComposeCommand( - composeFile, - ['up', '--no-start', '--no-deps', ...servicesToKeepStopped], - logContainer, - ); - } } } @@ -1257,10 +1143,28 @@ class Dockercompose extends Docker { } /** - * Override: compose doesn't need to inspect the existing container - * (compose CLI handles the container lifecycle). Lighter context. + * Override: provide shared runtime dependencies once per lifecycle run. + * Runtime container state is still resolved on demand per service refresh. */ - async createTriggerContext(container, logContainer, _composeContext) { + async createTriggerContext(container, logContainer, composeContext) { + const runtimeContext = (composeContext as { runtimeContext?: unknown } | undefined) + ?.runtimeContext as ComposeRuntimeRefreshOptions['runtimeContext'] | undefined; + if ( + runtimeContext?.dockerApi && + runtimeContext?.registry && + runtimeContext?.auth !== undefined && + runtimeContext?.newImage + ) { + return { + dockerApi: runtimeContext.dockerApi, + registry: runtimeContext.registry, + auth: runtimeContext.auth, + newImage: runtimeContext.newImage, + currentContainer: null, + currentContainerSpec: null, + }; + } + const watcher = this.getWatcher(container); const { dockerApi } = watcher; const registry = getState().registry[container.image.registry.name]; @@ -1277,12 +1181,29 @@ class Dockercompose extends Docker { } /** - * Override: use compose CLI for pull/recreate instead of Docker API. + * Override: apply compose-specific hooks while performing runtime refresh + * through the Docker Engine API. */ - async performContainerUpdate(_context, container, _logContainer, composeCtx) { + async performContainerUpdate(context, container, _logContainer, composeCtx) { if (!composeCtx) { throw new Error(`Missing compose context for container ${container.name}`); } + const composeRuntimeContext = (composeCtx as { runtimeContext?: unknown })?.runtimeContext as + | ComposeRuntimeRefreshOptions['runtimeContext'] + | undefined; + const runtimeContext = { + dockerApi: context?.dockerApi, + auth: context?.auth, + newImage: context?.newImage, + registry: context?.registry, + ...(composeRuntimeContext || {}), + }; + const hasRuntimeContext = + runtimeContext.dockerApi !== undefined || + runtimeContext.auth !== undefined || + runtimeContext.newImage !== undefined || + runtimeContext.registry !== undefined; + if (composeCtx.composeFileOnceApplied === true) { const logContainer = this.log.child({ container: container.name, @@ -1298,6 +1219,8 @@ class Dockercompose extends Docker { container, { composeFiles: composeCtx.composeFiles, + ...(composeCtx.skipPull === true ? { skipPull: true } : {}), + ...(hasRuntimeContext ? { runtimeContext } : {}), }, ); } else { @@ -1305,6 +1228,10 @@ class Dockercompose extends Docker { composeCtx.composeFile, composeCtx.service, container, + { + ...(composeCtx.skipPull === true ? { skipPull: true } : {}), + ...(hasRuntimeContext ? { runtimeContext } : {}), + }, ); } } @@ -1329,10 +1256,11 @@ class Dockercompose extends Docker { } /** - * Self-update for compose-managed Drydock service. This must stay in compose - * lifecycle instead of Docker API recreate to preserve compose ownership. + * Self-update for compose-managed Drydock service. Delegate to the parent + * self-update transition so the helper container can enforce startup/health + * gates and rollback before retiring the old process. */ - async executeSelfUpdate(context, container, logContainer, _operationId, composeCtx) { + async executeSelfUpdate(context, container, logContainer, operationId, composeCtx) { if (!composeCtx) { throw new Error(`Missing compose context for self-update container ${container.name}`); } @@ -1342,20 +1270,19 @@ class Dockercompose extends Docker { return false; } - this.insertContainerImageBackup(context, container); - if (Array.isArray(composeCtx.composeFiles) && composeCtx.composeFiles.length > 1) { - await this.updateContainerWithCompose(composeCtx.composeFile, composeCtx.service, container, { - composeFiles: composeCtx.composeFiles, - }); - } else { - await this.updateContainerWithCompose(composeCtx.composeFile, composeCtx.service, container); - } - await this.runServicePostStartHooks( - container, - composeCtx.service, - composeCtx.serviceDefinition, - ); - return true; + const currentContainer = + context?.currentContainer ?? (await this.getCurrentContainer(context.dockerApi, container)); + const currentContainerSpec = + context?.currentContainerSpec ?? + (await this.inspectContainer(currentContainer, logContainer)); + + const selfUpdateContext = { + ...context, + currentContainer, + currentContainerSpec, + }; + + return super.executeSelfUpdate(selfUpdateContext, container, logContainer, operationId); } /** @@ -1572,161 +1499,72 @@ class Dockercompose extends Docker { } } - let composeFileOnceHandledServices = new Set(); - if ( - this.configuration.composeFileOnce === true && - this.configuration.dryrun !== true && - mappingsNeedingRuntimeUpdate.length > 1 - ) { - const serviceRunningStates = new Map(); - const servicesToRefresh: string[] = []; - for (const { container, service } of mappingsNeedingRuntimeUpdate) { - if (serviceRunningStates.has(service)) { - continue; + const composeFileOnceHandledServices = new Set(); + const composeFileOnceEnabled = + this.configuration.composeFileOnce === true && this.configuration.dryrun !== true; + const composeFileOnceRuntimeContextByService = new Map< + string, + NonNullable + >(); + if (composeFileOnceEnabled) { + const firstContainerByService = new Map< + string, + (typeof mappingsNeedingRuntimeUpdate)[number] + >(); + for (const mapping of mappingsNeedingRuntimeUpdate) { + if (!firstContainerByService.has(mapping.service)) { + firstContainerByService.set(mapping.service, mapping); } - const runningStateLogger = this.log.child({ - container: container.name, - }); - serviceRunningStates.set( - service, - await this.getContainerRunningState(container, runningStateLogger), - ); - servicesToRefresh.push(service); } - await this.updateComposeServicesWithCompose(composeFile, servicesToRefresh, { - composeFiles: composeFileChain, - serviceRunningStates, - }); - composeFileOnceHandledServices = new Set(servicesToRefresh); - this.log.info( - `Compose-file-once mode refreshed ${servicesToRefresh.length} service(s) for ${composeFileChainSummary}`, + await Promise.all( + [...firstContainerByService.entries()].map(async ([service, mapping]) => { + const runtimeContainer = mapping.container; + const logContainer = this.log.child({ + container: runtimeContainer.name, + }); + const watcher = this.getWatcher(runtimeContainer); + const { dockerApi } = watcher; + const registry = this.resolveRegistryManager(runtimeContainer, logContainer, { + allowAnonymousFallback: true, + }); + const auth = await registry.getAuthPull(); + const newImage = this.getNewImageFullName(registry, runtimeContainer); + composeFileOnceRuntimeContextByService.set(service, { + dockerApi, + registry, + auth, + newImage, + }); + await this.pullImage(dockerApi, auth, newImage, logContainer); + }), ); } // Refresh all containers requiring a runtime update via the shared // lifecycle orchestrator (security gate, hooks, prune/backup, events). for (const { container, service } of mappingsNeedingRuntimeUpdate) { + const composeFileOnceApplied = + composeFileOnceEnabled && composeFileOnceHandledServices.has(service); + const composeFileOnceRuntimeContext = composeFileOnceRuntimeContextByService.get(service); const composeContext = { composeFile, composeFiles: composeFileChain, service, serviceDefinition: compose.services[service], - composeFileOnceApplied: composeFileOnceHandledServices.has(service), + composeFileOnceApplied, + skipPull: + composeFileOnceEnabled && + composeFileOnceApplied !== true && + composeFileOnceRuntimeContext !== undefined, + runtimeContext: composeFileOnceRuntimeContext, }; await this.runContainerUpdateLifecycle(container, composeContext); - } - } - - async executeCommand(command, args, options = {}) { - return new Promise((resolve, reject) => { - execFile( - command, - args, - { - ...options, - timeout: COMPOSE_COMMAND_TIMEOUT_MS, - maxBuffer: COMPOSE_COMMAND_MAX_BUFFER_BYTES, - }, - (error, stdout, stderr) => { - if (error) { - error.stdout = stdout; - error.stderr = stderr; - reject(error); - return; - } - resolve({ - stdout: stdout || '', - stderr: stderr || '', - }); - }, - ); - }); - } - - async runComposeCommand(composeFile, composeArgs, logContainer, composeFiles = [composeFile]) { - const composeFileChain = this.normalizeComposeFileChain(composeFile, composeFiles); - const composeFilePaths = composeFileChain.map((composeFilePathToResolve) => - this.resolveComposeFilePath(composeFilePathToResolve), - ); - const composeFileArgs = composeFilePaths.flatMap((composeFilePath) => ['-f', composeFilePath]); - const composeWorkingDirectory = path.dirname(composeFilePaths[0]); - const composeFilePathSummary = composeFilePaths.join(', '); - const commandsToTry = [ - { - command: 'docker', - args: ['compose', ...composeFileArgs, ...composeArgs], - label: 'docker compose', - }, - { - command: 'docker-compose', - args: [...composeFileArgs, ...composeArgs], - label: 'docker-compose', - }, - ]; - - for (const composeCommand of commandsToTry) { - try { - const { stdout, stderr } = (await this.executeCommand( - composeCommand.command, - composeCommand.args, - { - cwd: composeWorkingDirectory, - env: buildComposeCommandEnvironment(), - }, - )) as { stdout: string; stderr: string }; - if (stdout.trim()) { - logContainer.debug( - `${composeCommand.label} ${composeArgs.join(' ')} stdout:\n${stdout.trim()}`, - ); - } - if (stderr.trim()) { - logContainer.debug( - `${composeCommand.label} ${composeArgs.join(' ')} stderr:\n${stderr.trim()}`, - ); - } - return; - } catch (e) { - const stderr = `${e?.stderr || ''}`; - const dockerComposePluginMissing = - composeCommand.command === 'docker' && - /docker: ['"]?compose['"]? is not a docker command/i.test(stderr); - const executableMissing = e?.code === 'ENOENT'; - - if ( - composeCommand.command === 'docker' && - (dockerComposePluginMissing || executableMissing) - ) { - logContainer.warn( - `Cannot use docker compose for ${composeFilePathSummary} (${e.message}); trying docker-compose`, - ); - continue; - } - - throw new Error( - `Error when running ${composeCommand.label} ${composeArgs.join(' ')} for ${composeFilePathSummary} (${e.message})`, - ); + if (composeFileOnceEnabled && !composeFileOnceApplied) { + composeFileOnceHandledServices.add(service); } } } - async getContainerRunningState(container, logContainer) { - try { - const watcher = this.getWatcher(container); - const dockerApi = getDockerApiFromWatcher(watcher); - if (!dockerApi) { - return true; - } - const containerToInspect = dockerApi.getContainer(container.name); - const containerState = await containerToInspect.inspect(); - return containerState?.State?.Running !== false; - } catch (e) { - logContainer.warn( - `Unable to inspect running state for ${container.name}; assuming running (${e.message})`, - ); - return true; - } - } - async resolveComposeServiceContext(container, currentImage) { const composeFiles = await this.resolveComposeFilesForContainer(container); if (composeFiles.length === 0) { @@ -1814,25 +1652,33 @@ class Dockercompose extends Docker { } async updateContainerWithCompose(composeFile, service, container, options = {}) { + await this.refreshComposeServiceWithDockerApi(composeFile, service, container, options); + } + + private ensureComposeRuntimeState(currentContainerSpec, composeFile, service): void { + if (typeof currentContainerSpec?.State?.Running !== 'boolean') { + throw new Error( + `Unable to refresh compose service ${service} from ${composeFile} because Docker inspection data is missing runtime state`, + ); + } + } + + /** + * Refresh one compose-managed service by using the Docker Engine API + * directly. Shared by updateContainerWithCompose() and recreateContainer() + * to keep the runtime recreation path explicit and non-recursive. + */ + private async refreshComposeServiceWithDockerApi( + composeFile, + service, + container, + options: ComposeRuntimeRefreshOptions = {}, + ) { const logContainer = this.log.child({ container: container.name, }); - const { - shouldStart = undefined, - skipPull = false, - forceRecreate = false, - } = options as { - shouldStart?: boolean; - skipPull?: boolean; - forceRecreate?: boolean; - composeFiles?: string[]; - }; - const composeFileChain = this.normalizeComposeFileChain( - composeFile, - (options as { composeFiles?: string[] })?.composeFiles, - ); - const runWithComposeFileChain = composeFileChain.length > 1; + const { shouldStart = undefined, skipPull = false, forceRecreate = false } = options; if (this.configuration.dryrun) { logContainer.info( @@ -1841,45 +1687,82 @@ class Dockercompose extends Docker { return; } - const serviceShouldStart = - shouldStart !== undefined - ? shouldStart - : await this.getContainerRunningState(container, logContainer); + const runtimeContext = options.runtimeContext || {}; + const dockerApi = runtimeContext.dockerApi || this.getWatcher(container)?.dockerApi; + let auth = runtimeContext.auth; + let newImage = runtimeContext.newImage; - logContainer.info(`Refresh compose service ${service} from ${composeFile}`); - if (!skipPull) { - if (runWithComposeFileChain) { - await this.runComposeCommand( - composeFile, - ['pull', service], - logContainer, - composeFileChain, - ); - } else { - await this.runComposeCommand(composeFile, ['pull', service], logContainer); + if (!newImage || (!skipPull && auth === undefined)) { + const registry = + runtimeContext.registry || + this.resolveRegistryManager(container, logContainer, { + allowAnonymousFallback: true, + }); + if (!newImage) { + newImage = this.getNewImageFullName(registry, container); + } + if (!skipPull && auth === undefined) { + auth = await registry.getAuthPull(); } - } else { - logContainer.debug(`Skip compose pull for ${service} from ${composeFile}`); } + const currentContainer = await this.getCurrentContainer(dockerApi, container); + if (!currentContainer) { + throw new Error( + `Unable to refresh compose service ${service} from ${composeFile} because container ${container.name} no longer exists`, + ); + } + const currentContainerSpec = await this.inspectContainer(currentContainer, logContainer); + this.ensureComposeRuntimeState(currentContainerSpec, composeFile, service); + const serviceShouldStart = + shouldStart !== undefined ? shouldStart : currentContainerSpec.State.Running; - const upArgs = ['up']; - if (serviceShouldStart) { - upArgs.push('-d'); + logContainer.info( + `Refresh compose service ${service} from ${composeFile} using Docker Engine API`, + ); + if (!skipPull) { + await this.pullImage(dockerApi, auth, newImage, logContainer); } else { - upArgs.push('--no-start'); + logContainer.debug(`Skip image pull for ${service} from ${composeFile}`); } - upArgs.push('--no-deps'); if (forceRecreate) { - upArgs.push('--force-recreate'); - } - upArgs.push(service); - if (runWithComposeFileChain) { - await this.runComposeCommand(composeFile, upArgs, logContainer, composeFileChain); - } else { - await this.runComposeCommand(composeFile, upArgs, logContainer); + logContainer.debug( + `Force recreate requested for ${service}; Docker Engine API path always recreates containers`, + ); } + + const recreationContainerSpec = { + ...currentContainerSpec, + State: { + ...currentContainerSpec.State, + Running: serviceShouldStart, + }, + }; + // Intentionally bypass Dockercompose.stopAndRemoveContainer() no-op: this + // internal Engine API refresh path must perform the real stop/remove. + await super.stopAndRemoveContainer( + currentContainer, + currentContainerSpec, + container, + logContainer, + ); + await super.recreateContainer( + dockerApi, + recreationContainerSpec, + newImage, + container, + logContainer, + ); } + /** + * No-op for generic callers that invoke stop/remove and recreate as two + * separate steps (for example health-monitor rollback paths). In compose + * mode, recreateContainer() owns the full mutation + runtime refresh and + * would otherwise duplicate stop/remove work. + * + * When a compose refresh must actually stop/remove, we bypass this override + * via super.stopAndRemoveContainer() in refreshComposeServiceWithDockerApi(). + */ async stopAndRemoveContainer(_currentContainer, _currentContainerSpec, container, logContainer) { logContainer.info( `Skip direct stop/remove for compose-managed container ${container.name}; using compose lifecycle`, @@ -1920,17 +1803,17 @@ class Dockercompose extends Docker { shouldStart: currentContainerSpec?.State?.Running === true, skipPull: true, forceRecreate: true, - } as { - shouldStart: boolean; - skipPull: boolean; - forceRecreate: boolean; - composeFiles?: string[]; - }; + } as ComposeRuntimeRefreshOptions; if (composeFiles.length > 1) { composeUpdateOptions.composeFiles = composeFiles; } - await this.updateContainerWithCompose(composeFile, service, container, composeUpdateOptions); + await this.refreshComposeServiceWithDockerApi( + composeFile, + service, + container, + composeUpdateOptions, + ); } async runServicePostStartHooks(container, serviceKey, service) { diff --git a/app/triggers/providers/mqtt/Mqtt.test.ts b/app/triggers/providers/mqtt/Mqtt.test.ts index 2fb40e83..fbf20f30 100644 --- a/app/triggers/providers/mqtt/Mqtt.test.ts +++ b/app/triggers/providers/mqtt/Mqtt.test.ts @@ -32,6 +32,10 @@ const configurationValid = { enabled: false, prefix: 'homeassistant', attributes: 'full', + filter: { + include: '', + exclude: '', + }, }, tls: { clientkey: undefined, @@ -112,6 +116,10 @@ test('validateConfiguration should default hass.discovery to true when hass.enab prefix: 'homeassistant', discovery: true, attributes: 'full', + filter: { + include: '', + exclude: '', + }, }); }); @@ -157,6 +165,11 @@ test('initTrigger should init Mqtt client', async () => { enabled: true, discovery: true, prefix: 'homeassistant', + attributes: 'full', + filter: { + include: '', + exclude: '', + }, }, }; const spy = vi.spyOn(mqttClient, 'connectAsync'); @@ -176,7 +189,13 @@ test.each(containerData)('trigger should format json message payload as expected mqtt.configuration = { topic: 'dd/container', exclude: '', - hass: { attributes: 'full' }, + hass: { + attributes: 'full', + filter: { + include: '', + exclude: '', + }, + }, }; const container = { id: '31a61a8305ef1fc9a71fa4f20a68d7ec88b28e32303bbc4a5f192e851165b816', @@ -225,7 +244,16 @@ test('initTrigger should read TLS files when configured', async () => { cachain: '/path/to/ca.pem', rejectunauthorized: false, }, - hass: { enabled: false }, + hass: { + enabled: false, + discovery: false, + prefix: 'homeassistant', + attributes: 'full', + filter: { + include: '', + exclude: '', + }, + }, }; await mqtt.initTrigger(); @@ -259,11 +287,31 @@ test('handleContainerEvent should log when trigger fails', async () => { expect(debugSpy).toHaveBeenCalledWith(expect.any(Error)); }); +test('handleContainerEvent should skip trigger when mustTrigger is false', async () => { + const mustTriggerSpy = vi.spyOn(mqtt, 'mustTrigger').mockReturnValue(false); + const triggerSpy = vi.spyOn(mqtt, 'trigger').mockResolvedValue(undefined); + + mqtt.handleContainerEvent({ name: 'ignored', watcher: 'local' }); + await Promise.resolve(); + + expect(mustTriggerSpy).toHaveBeenCalledWith({ name: 'ignored', watcher: 'local' }); + expect(triggerSpy).not.toHaveBeenCalled(); +}); + test('initTrigger should execute registered container event callbacks', async () => { mqtt.configuration = { ...configurationValid, clientid: 'dd', - hass: { enabled: false, discovery: false, prefix: 'homeassistant' }, + hass: { + enabled: false, + discovery: false, + prefix: 'homeassistant', + attributes: 'full', + filter: { + include: '', + exclude: '', + }, + }, }; vi.spyOn(mqttClient, 'connectAsync').mockResolvedValue({ publish: vi.fn().mockResolvedValue(undefined), @@ -283,7 +331,16 @@ test('deregister then initTrigger should not duplicate container event callbacks mqtt.configuration = { ...configurationValid, clientid: 'dd', - hass: { enabled: false, discovery: false, prefix: 'homeassistant' }, + hass: { + enabled: false, + discovery: false, + prefix: 'homeassistant', + attributes: 'full', + filter: { + include: '', + exclude: '', + }, + }, }; vi.spyOn(mqttClient, 'connectAsync').mockResolvedValue({ publish: vi.fn().mockResolvedValue(undefined), @@ -329,6 +386,36 @@ describe('hass.attributes validation', () => { }); }); +describe('hass.filter validation', () => { + test('should default hass.filter include and exclude to empty strings', () => { + const validated = mqtt.validateConfiguration({ + url: configurationValid.url, + clientid: 'dd', + }); + expect(validated.hass.filter).toStrictEqual({ + include: '', + exclude: '', + }); + }); + + test('should accept hass.filter include and exclude', () => { + const validated = mqtt.validateConfiguration({ + url: configurationValid.url, + clientid: 'dd', + hass: { + filter: { + include: 'name,image_name,result_tag', + exclude: 'security_sbom_documents_0_spdx_version', + }, + }, + }); + expect(validated.hass.filter).toStrictEqual({ + include: 'name,image_name,result_tag', + exclude: 'security_sbom_documents_0_spdx_version', + }); + }); +}); + describe('exclude validation', () => { test('should accept exclude as comma-separated string', () => { const validated = mqtt.validateConfiguration({ @@ -392,7 +479,13 @@ describe('trigger filtering', () => { mqtt.configuration = { topic: 'dd/container', exclude: '', - hass: { attributes: 'short' }, + hass: { + attributes: 'short', + filter: { + include: '', + exclude: '', + }, + }, }; await mqtt.trigger(containerWithSecurity); @@ -409,7 +502,13 @@ describe('trigger filtering', () => { mqtt.configuration = { topic: 'dd/container', exclude: '', - hass: { attributes: 'full' }, + hass: { + attributes: 'full', + filter: { + include: '', + exclude: '', + }, + }, }; await mqtt.trigger(containerWithSecurity); @@ -423,7 +522,13 @@ describe('trigger filtering', () => { mqtt.configuration = { topic: 'dd/container', exclude: 'details', - hass: { attributes: 'short' }, + hass: { + attributes: 'short', + filter: { + include: '', + exclude: '', + }, + }, }; await mqtt.trigger(containerWithSecurity); @@ -434,11 +539,59 @@ describe('trigger filtering', () => { expect(publishedPayload).toHaveProperty('security_scan_vulnerabilities_0_id', 'CVE-2024-0001'); }); + test('should use hass.filter.include over all other filters when set', async () => { + mqtt.configuration = { + topic: 'dd/container', + exclude: 'details', + hass: { + attributes: 'short', + filter: { + include: 'name,image_name,result_tag', + exclude: 'security_scan_vulnerabilities_0_id', + }, + }, + }; + await mqtt.trigger(containerWithSecurity); + + const publishedPayload = JSON.parse(mqtt.client.publish.mock.calls[0][1]); + expect(publishedPayload).toEqual({ + name: 'filtered-test', + image_name: 'nginx', + result_tag: '1.26', + }); + }); + + test('should use hass.filter.exclude over legacy exclude and hass.attributes', async () => { + mqtt.configuration = { + topic: 'dd/container', + exclude: 'details', + hass: { + attributes: 'short', + filter: { + include: '', + exclude: 'security_sbom_documents_0_spdx_version', + }, + }, + }; + await mqtt.trigger(containerWithSecurity); + + const publishedPayload = JSON.parse(mqtt.client.publish.mock.calls[0][1]); + expect(publishedPayload).not.toHaveProperty('security_sbom_documents_0_spdx_version'); + expect(publishedPayload).toHaveProperty('details_ports_0', '80/tcp'); + expect(publishedPayload).toHaveProperty('security_scan_vulnerabilities_0_id', 'CVE-2024-0001'); + }); + test('should publish full container when both are default', async () => { mqtt.configuration = { topic: 'dd/container', exclude: '', - hass: { attributes: 'full' }, + hass: { + attributes: 'full', + filter: { + include: '', + exclude: '', + }, + }, }; await mqtt.trigger(containerWithSecurity); @@ -447,4 +600,22 @@ describe('trigger filtering', () => { expect(publishedPayload).toHaveProperty('details_ports_0', '80/tcp'); expect(publishedPayload).toHaveProperty('labels_com_docker_compose_project', 'app'); }); + + test('should default hass.attributes to full when not provided in runtime config', async () => { + mqtt.configuration = { + topic: 'dd/container', + exclude: '', + hass: { + filter: { + include: '', + exclude: '', + }, + }, + }; + await mqtt.trigger(containerWithSecurity); + + const publishedPayload = JSON.parse(mqtt.client.publish.mock.calls[0][1]); + expect(publishedPayload).toHaveProperty('security_scan_vulnerabilities_0_id', 'CVE-2024-0001'); + expect(publishedPayload).toHaveProperty('details_ports_0', '80/tcp'); + }); }); diff --git a/app/triggers/providers/mqtt/Mqtt.ts b/app/triggers/providers/mqtt/Mqtt.ts index a13d6c38..50e80ca9 100644 --- a/app/triggers/providers/mqtt/Mqtt.ts +++ b/app/triggers/providers/mqtt/Mqtt.ts @@ -7,6 +7,7 @@ import { resolveConfiguredPath } from '../../../runtime/paths.js'; import Trigger, { type TriggerConfiguration } from '../Trigger.js'; import { filterContainer, + filterContainerInclude, HASS_ATTRIBUTE_PRESET_VALUES, HASS_ATTRIBUTE_PRESETS, type HassAttributePreset, @@ -43,6 +44,10 @@ interface MqttConfiguration extends TriggerConfiguration { prefix: string; discovery: boolean; attributes: HassAttributePreset; + filter: { + include: string; + exclude: string; + }; }; tls: { clientkey?: string; @@ -52,6 +57,22 @@ interface MqttConfiguration extends TriggerConfiguration { }; } +interface MqttFilterConfig { + mode: 'include' | 'exclude'; + stage: 'container' | 'flattened'; + paths: string[]; +} + +function splitFilterPaths(value: string | undefined): string[] { + if (!value) { + return []; + } + return value + .split(',') + .map((path) => path.trim()) + .filter(Boolean); +} + /** * MQTT Trigger implementation */ @@ -66,6 +87,10 @@ class Mqtt extends Trigger { prefix: hassDefaultPrefix, discovery: false, attributes: 'full', + filter: { + include: '', + exclude: '', + }, }, tls: { rejectunauthorized: true, @@ -85,6 +110,9 @@ class Mqtt extends Trigger { } handleContainerEvent(container) { + if (!this.mustTrigger(container)) { + return; + } void this.trigger(container).catch((error) => { this.log.warn(`Error (${error.message})`); this.log.debug(error); @@ -117,12 +145,25 @@ class Mqtt extends Trigger { .string() .valid(...HASS_ATTRIBUTE_PRESET_VALUES) .default('full'), + filter: this.joi + .object({ + include: this.joi.string().allow('').default(''), + exclude: this.joi.string().allow('').default(''), + }) + .default({ + include: '', + exclude: '', + }), }) .default({ enabled: false, prefix: hassDefaultPrefix, discovery: false, attributes: 'full', + filter: { + include: '', + exclude: '', + }, }), tls: this.joi .object({ @@ -214,14 +255,39 @@ class Mqtt extends Trigger { await super.deregisterComponent(); } - getExcludePaths(): string[] { - if (this.configuration.exclude) { - return this.configuration.exclude - .split(',') - .map((s) => s.trim()) - .filter(Boolean); + getFilterConfig(): MqttFilterConfig { + const includePaths = splitFilterPaths(this.configuration.hass?.filter?.include); + if (includePaths.length > 0) { + return { + mode: 'include', + stage: 'flattened', + paths: includePaths, + }; } - return HASS_ATTRIBUTE_PRESETS[this.configuration.hass.attributes]; + + const hassExcludePaths = splitFilterPaths(this.configuration.hass?.filter?.exclude); + if (hassExcludePaths.length > 0) { + return { + mode: 'exclude', + stage: 'flattened', + paths: hassExcludePaths, + }; + } + + const legacyExcludePaths = splitFilterPaths(this.configuration.exclude); + if (legacyExcludePaths.length > 0) { + return { + mode: 'exclude', + stage: 'container', + paths: legacyExcludePaths, + }; + } + + return { + mode: 'exclude', + stage: 'container', + paths: HASS_ATTRIBUTE_PRESETS[this.configuration.hass?.attributes ?? 'full'], + }; } /** @@ -236,11 +302,21 @@ class Mqtt extends Trigger { container, }); - const excludePaths = this.getExcludePaths(); - const containerToPublish = filterContainer(container, excludePaths); + const filterConfig = this.getFilterConfig(); + const containerToPublish = + filterConfig.stage === 'container' + ? filterContainer(container, filterConfig.paths) + : container; + const flattenedContainer = flatten(containerToPublish); + const containerToPublishFlattened = + filterConfig.stage === 'flattened' + ? filterConfig.mode === 'include' + ? filterContainerInclude(flattenedContainer, filterConfig.paths) + : filterContainer(flattenedContainer, filterConfig.paths) + : flattenedContainer; this.log.debug(`Publish container result to ${containerTopic}`); - return this.client.publish(containerTopic, JSON.stringify(flatten(containerToPublish)), { + return this.client.publish(containerTopic, JSON.stringify(containerToPublishFlattened), { retain: true, }); } diff --git a/app/triggers/providers/mqtt/filter.test.ts b/app/triggers/providers/mqtt/filter.test.ts index d8d7edf6..365fc41f 100644 --- a/app/triggers/providers/mqtt/filter.test.ts +++ b/app/triggers/providers/mqtt/filter.test.ts @@ -1,4 +1,9 @@ -import { filterContainer, HASS_ATTRIBUTE_PRESET_VALUES, HASS_ATTRIBUTE_PRESETS } from './filter.js'; +import { + filterContainer, + filterContainerInclude, + HASS_ATTRIBUTE_PRESET_VALUES, + HASS_ATTRIBUTE_PRESETS, +} from './filter.js'; describe('filterContainer', () => { const container = { @@ -178,6 +183,63 @@ describe('filterContainer', () => { }); }); +describe('filterContainerInclude', () => { + const flattenedContainer = { + name: 'test', + watcher: 'local', + image_name: 'nginx', + result_tag: '1.26', + security_scan_status: 'passed', + security_scan_vulnerabilities_0_id: 'CVE-2024-0001', + }; + + test('returns container unchanged (same reference) when includePaths is empty', () => { + const result = filterContainerInclude(flattenedContainer, []); + expect(result).toBe(flattenedContainer); + }); + + test('returns primitive input unchanged when includePaths are requested', () => { + const result = filterContainerInclude('container-as-string', ['name']); + expect(result).toBe('container-as-string'); + }); + + test('keeps only included top-level keys', () => { + const result = filterContainerInclude(flattenedContainer, ['name', 'image_name', 'result_tag']); + expect(result).toEqual({ + name: 'test', + image_name: 'nginx', + result_tag: '1.26', + }); + }); + + test('ignores include keys that do not exist', () => { + const result = filterContainerInclude(flattenedContainer, ['name', 'does_not_exist']); + expect(result).toEqual({ + name: 'test', + }); + }); + + test('does not mutate the original container', () => { + const original = JSON.parse(JSON.stringify(flattenedContainer)); + filterContainerInclude(flattenedContainer, ['name']); + expect(flattenedContainer).toEqual(original); + }); + + test('preserves symbol keys while filtering string keys', () => { + const secret = Symbol('secret'); + const flattenedWithSymbol = { + ...flattenedContainer, + [secret]: 'value', + }; + + const result = filterContainerInclude(flattenedWithSymbol, ['name']); + expect(result).toEqual({ + name: 'test', + [secret]: 'value', + }); + }); +}); + describe('HASS_ATTRIBUTE_PRESETS', () => { test('full preset has empty exclude list', () => { expect(HASS_ATTRIBUTE_PRESETS.full).toEqual([]); diff --git a/app/triggers/providers/mqtt/filter.ts b/app/triggers/providers/mqtt/filter.ts index f102dd04..59fc64d7 100644 --- a/app/triggers/providers/mqtt/filter.ts +++ b/app/triggers/providers/mqtt/filter.ts @@ -121,3 +121,31 @@ export function filterContainer(container: T, excludePaths: string[]): T { return clone as T; } + +/** + * Filter a flat container object by keeping only the given top-level keys. + * Returns the original container when includePaths is empty (zero overhead). + */ +export function filterContainerInclude(container: T, includePaths: string[]): T { + if (includePaths.length === 0) { + return container; + } + if (!isRecord(container)) { + return container; + } + + const sourceRoot = container as Record; + const clone = cloneObjectShallow(sourceRoot); + const includeSet = new Set(includePaths); + + for (const key of Reflect.ownKeys(sourceRoot)) { + if (typeof key !== 'string') { + continue; + } + if (!includeSet.has(key)) { + delete clone[key]; + } + } + + return clone as T; +} diff --git a/app/watchers/providers/docker/Docker.test.ts b/app/watchers/providers/docker/Docker.test.ts index c7cd0387..55d4f274 100644 --- a/app/watchers/providers/docker/Docker.test.ts +++ b/app/watchers/providers/docker/Docker.test.ts @@ -5526,6 +5526,59 @@ describe('Docker Watcher', () => { }), ); }); + + test('pruneOldContainers should delete stale entries when a same-name replacement exists', async () => { + const dockerApi = { + getContainer: vi.fn(), + }; + + await testable_pruneOldContainers( + [ + { + id: 'new-1', + watcher: 'docker', + name: 'app', + }, + ] as any, + [ + { + id: 'old-1', + watcher: 'docker', + name: 'app', + }, + ] as any, + dockerApi as any, + ); + + expect(dockerApi.getContainer).not.toHaveBeenCalled(); + expect(storeContainer.deleteContainer).toHaveBeenCalledWith('old-1'); + }); + + test('pruneOldContainers should treat missing watcher as an empty watcher key', async () => { + const dockerApi = { + getContainer: vi.fn(), + }; + + await testable_pruneOldContainers( + [ + { + id: 'new-1', + name: 'app', + }, + ] as any, + [ + { + id: 'old-1', + watcher: '', + name: 'app', + }, + ] as any, + dockerApi as any, + ); + + expect(dockerApi.getContainer).not.toHaveBeenCalled(); + expect(storeContainer.deleteContainer).toHaveBeenCalledWith('old-1'); + }); }); describe('Additional Coverage - maintenance queue internals', () => { diff --git a/app/watchers/providers/docker/Docker.ts b/app/watchers/providers/docker/Docker.ts index a21afe7d..c62c96f8 100644 --- a/app/watchers/providers/docker/Docker.ts +++ b/app/watchers/providers/docker/Docker.ts @@ -320,7 +320,21 @@ async function pruneOldContainers( dockerApi: DockerApiContainerInspector, ) { const containersToRemove = getOldContainers(newContainers, containersFromTheStore); + const newContainerNameKeys = new Set( + newContainers + .filter((container) => typeof container.name === 'string' && container.name !== '') + .map((container) => `${container.watcher || ''}::${container.name}`), + ); for (const containerToRemove of containersToRemove) { + const staleContainerNameKey = `${containerToRemove.watcher || ''}::${containerToRemove.name || ''}`; + if ( + typeof containerToRemove.name === 'string' && + containerToRemove.name !== '' && + newContainerNameKeys.has(staleContainerNameKey) + ) { + storeContainer.deleteContainer(containerToRemove.id); + continue; + } try { const inspectResult = await dockerApi.getContainer(containerToRemove.id).inspect(); const newStatus = inspectResult?.State?.Status; diff --git a/app/watchers/providers/docker/container-event-update.test.ts b/app/watchers/providers/docker/container-event-update.test.ts index 5f99d533..6340a911 100644 --- a/app/watchers/providers/docker/container-event-update.test.ts +++ b/app/watchers/providers/docker/container-event-update.test.ts @@ -86,6 +86,27 @@ describe('container event update helpers', () => { ); }); + test('processDockerEvent schedules a debounced watch for rename events when container is not in store', async () => { + const watchCronDebounced = vi.fn().mockResolvedValue(undefined); + + await processDockerEvent( + { Action: 'rename', id: 'container123' }, + { + watchCronDebounced, + ensureRemoteAuthHeaders: vi.fn().mockResolvedValue(undefined), + inspectContainer: vi.fn().mockResolvedValue({ + Name: '/renamed-container', + State: { Status: 'running' }, + }), + getContainerFromStore: vi.fn().mockReturnValue(undefined), + updateContainerFromInspect: vi.fn(), + debug: vi.fn(), + }, + ); + + expect(watchCronDebounced).toHaveBeenCalledTimes(1); + }); + test('updateContainerFromInspect updates status/name/displayName and persists', () => { const container = createMockContainer({ status: 'stopped', diff --git a/app/watchers/providers/docker/container-event-update.ts b/app/watchers/providers/docker/container-event-update.ts index d2f89018..317eb1f4 100644 --- a/app/watchers/providers/docker/container-event-update.ts +++ b/app/watchers/providers/docker/container-event-update.ts @@ -34,6 +34,10 @@ export async function processDockerEvent( if (containerFound) { dependencies.updateContainerFromInspect(containerFound, containerInspect); + } else if (action === 'rename') { + // Rename can race with create and happen before the new container is in store. + // Schedule a full refresh so the final human-readable name is captured. + await dependencies.watchCronDebounced(); } } catch (e: any) { dependencies.debug( diff --git a/app/watchers/providers/docker/docker-image-details-orchestration.test.ts b/app/watchers/providers/docker/docker-image-details-orchestration.test.ts index 00ff9063..c0622abf 100644 --- a/app/watchers/providers/docker/docker-image-details-orchestration.test.ts +++ b/app/watchers/providers/docker/docker-image-details-orchestration.test.ts @@ -242,6 +242,86 @@ describe('docker image details orchestration module', () => { expect(containerInStore.status).toBe('running'); }); + test('refreshes cached container name from Docker summary when it changes', async () => { + const containerInStore = { + id: 'container-1', + name: 'temp-name-123', + displayName: 'temp-name-123', + status: 'running', + error: undefined, + details: { + ports: [], + volumes: [], + env: [], + }, + image: { + name: 'acme/service', + id: 'image-old', + digest: { + repo: 'sha256:old', + value: 'sha256:old', + }, + created: '2025-01-01T00:00:00.000Z', + }, + }; + vi.spyOn(storeContainer, 'getContainer').mockReturnValue(containerInStore as any); + + const { watcher } = createWatcher({ + configuration: { watchevents: true }, + }); + + const result = await addImageDetailsToContainerOrchestration( + watcher as any, + createDockerSummaryContainer({ Names: ['/service'] }), + {}, + createHelpers() as any, + ); + + expect(result).toBe(containerInStore); + expect(containerInStore.name).toBe('service'); + expect(containerInStore.displayName).toBe('service'); + }); + + test('keeps custom displayName when container name changes', async () => { + const containerInStore = { + id: 'container-1', + name: 'temp-name-123', + displayName: 'Friendly Service', + status: 'running', + error: undefined, + details: { + ports: [], + volumes: [], + env: [], + }, + image: { + name: 'acme/service', + id: 'image-old', + digest: { + repo: 'sha256:old', + value: 'sha256:old', + }, + created: '2025-01-01T00:00:00.000Z', + }, + }; + vi.spyOn(storeContainer, 'getContainer').mockReturnValue(containerInStore as any); + + const { watcher } = createWatcher({ + configuration: { watchevents: true }, + }); + + const result = await addImageDetailsToContainerOrchestration( + watcher as any, + createDockerSummaryContainer({ Names: ['/service'] }), + {}, + createHelpers() as any, + ); + + expect(result).toBe(containerInStore); + expect(containerInStore.name).toBe('service'); + expect(containerInStore.displayName).toBe('Friendly Service'); + }); + test('throws a clear error when image inspection fails for a new container', async () => { vi.spyOn(storeContainer, 'getContainer').mockReturnValue(undefined); @@ -429,4 +509,34 @@ describe('docker image details orchestration module', () => { }); expect(watcher.log.warn).not.toHaveBeenCalled(); }); + + test('removes stale same-name container entries when a new container id is discovered', async () => { + vi.spyOn(storeContainer, 'getContainer').mockReturnValue(undefined); + const getContainersSpy = vi.spyOn(storeContainer, 'getContainers').mockReturnValue([ + { + id: 'old-container-id', + watcher: 'docker-test', + name: 'service', + } as any, + ]); + const deleteContainerSpy = vi + .spyOn(storeContainer, 'deleteContainer') + .mockImplementation(() => {}); + + const { watcher } = createWatcher(); + + const result = await addImageDetailsToContainerOrchestration( + watcher as any, + createDockerSummaryContainer({ + Id: 'new-container-id', + Names: ['/service'], + }), + {}, + createHelpers() as any, + ); + + expect(result?.id).toBe('new-container-id'); + expect(getContainersSpy).toHaveBeenCalledWith({ watcher: 'docker-test', name: 'service' }); + expect(deleteContainerSpy).toHaveBeenCalledWith('old-container-id'); + }); }); diff --git a/app/watchers/providers/docker/docker-image-details-orchestration.ts b/app/watchers/providers/docker/docker-image-details-orchestration.ts index 33412ada..58076599 100644 --- a/app/watchers/providers/docker/docker-image-details-orchestration.ts +++ b/app/watchers/providers/docker/docker-image-details-orchestration.ts @@ -7,6 +7,7 @@ import { getRepoDigest, isDigestToWatch, type ResolvedImgset, + shouldUpdateDisplayNameFromContainerName, } from './docker-helpers.js'; import { areRuntimeDetailsEqual, @@ -153,6 +154,7 @@ export async function addImageDetailsToContainerOrchestration( ): Promise { const containerId = container.Id; const containerLabels: Record = container.Labels || {}; + const dockerContainerName = getContainerName(container); const runtimeDetailsFromSummary = getRuntimeDetailsFromContainerSummary(container); // Is container already in store? Refresh volatile image fields, then return it @@ -160,6 +162,21 @@ export async function addImageDetailsToContainerOrchestration( if (containerInStore !== undefined && containerInStore.error === undefined) { watcher.ensureLogger(); watcher.log.debug(`Container ${containerInStore.id} already in store`); + const existingName = containerInStore.name || ''; + if (dockerContainerName !== '' && existingName !== dockerContainerName) { + const shouldUpdateDisplayName = shouldUpdateDisplayNameFromContainerName( + dockerContainerName, + existingName, + containerInStore.displayName, + ); + containerInStore.name = dockerContainerName; + if (shouldUpdateDisplayName) { + containerInStore.displayName = getContainerDisplayName( + dockerContainerName, + containerInStore.image?.name || '', + ); + } + } const cachedRuntimeDetails = normalizeRuntimeDetails(containerInStore.details); let runtimeDetailsToApply = mergeRuntimeDetails( runtimeDetailsFromSummary, @@ -273,16 +290,15 @@ export async function addImageDetailsToContainerOrchestration( } catch { // Degrade gracefully to summary details. } - const containerName = getContainerName(container); if (!isSemver && !watchDigest) { watcher.ensureLogger(); watcher.log.warn( - `Image is not a semver and digest watching is disabled so drydock won't report any update for container "${containerName}". Please review the configuration to enable digest watching for this container or exclude this container from being watched`, + `Image is not a semver and digest watching is disabled so drydock won't report any update for container "${dockerContainerName}". Please review the configuration to enable digest watching for this container or exclude this container from being watched`, ); } - return helpers.normalizeContainer({ + const containerToReturn = helpers.normalizeContainer({ id: containerId, - name: containerName, + name: dockerContainerName, status: container.State, watcher: watcher.name, includeTags: resolvedConfig.includeTags, @@ -291,7 +307,7 @@ export async function addImageDetailsToContainerOrchestration( tagFamily: resolvedConfig.tagFamily, linkTemplate: resolvedConfig.linkTemplate, displayName: getContainerDisplayName( - containerName, + dockerContainerName, parsedImage.path, resolvedConfig.displayName, ), @@ -328,4 +344,12 @@ export async function addImageDetailsToContainerOrchestration( updateAvailable: false, updateKind: { kind: 'unknown' }, } as Container); + const containersWithSameName = storeContainer.getContainers({ + watcher: watcher.name, + name: containerToReturn.name, + }); + containersWithSameName + .filter((staleContainer) => staleContainer.id !== containerToReturn.id) + .forEach((staleContainer) => storeContainer.deleteContainer(staleContainer.id)); + return containerToReturn; } diff --git a/biome.json b/biome.json index df30fa21..02fdbae8 100644 --- a/biome.json +++ b/biome.json @@ -7,7 +7,7 @@ }, "files": { "ignoreUnknown": true, - "includes": ["**", "!ui/public/index.html", "!website", "!apps/web/app/globals.css"] + "includes": ["**", "!ui/public/index.html", "!apps/web/app/globals.css"] }, "formatter": { "enabled": true, diff --git a/content/docs/current/configuration/actions/index.mdx b/content/docs/current/configuration/actions/index.mdx index ffb29aad..d6dd7000 100644 --- a/content/docs/current/configuration/actions/index.mdx +++ b/content/docs/current/configuration/actions/index.mdx @@ -33,7 +33,7 @@ Container actions can be individually toggled with environment variables: ## Update action -The update action triggers the Docker trigger for the selected container. It will: +The update action triggers the Docker or Docker Compose trigger for the selected container. It will: 1. Pull the latest image 2. Backup the current image diff --git a/content/docs/current/configuration/authentications/oidc/index.mdx b/content/docs/current/configuration/authentications/oidc/index.mdx index 24858dea..e8d0d11c 100644 --- a/content/docs/current/configuration/authentications/oidc/index.mdx +++ b/content/docs/current/configuration/authentications/oidc/index.mdx @@ -17,19 +17,25 @@ The `oidc` authentication lets you protect drydock access using the [Openid Conn | `DD_AUTH_OIDC_{auth_name}_CLIENTID` | 🔴 | Client ID | | | | `DD_AUTH_OIDC_{auth_name}_CLIENTSECRET` | 🔴 | Client Secret | | | | `DD_AUTH_OIDC_{auth_name}_DISCOVERY` | 🔴 | Oidc discovery URL | | | +| `DD_AUTH_OIDC_{auth_name}_CAFILE` | ⚪ | Path to a PEM CA certificate (or chain) used for OIDC HTTPS requests | File path | empty | +| `DD_AUTH_OIDC_{auth_name}_INSECURE` | ⚪ | Disable TLS certificate verification for OIDC HTTPS requests (development only) | `true`, `false` | `false` | | `DD_AUTH_OIDC_{auth_name}_REDIRECT` | ⚪ | Skip internal login page & automatically redirect to the OIDC provider | `true`, `false` | `false` | | `DD_AUTH_OIDC_{auth_name}_LOGOUTURL` | ⚪ | Explicit IdP logout URL fallback when discovery metadata has no `end_session_endpoint` | URL | empty | | `DD_AUTH_OIDC_{auth_name}_TIMEOUT` | ⚪ | Timeout (in ms) when calling the OIDC provider | Minimum is 500 | `5000` | +| `DD_PUBLIC_URL` | 🔴 | Public URL of your drydock instance (used to build the OIDC callback URL) | URL | | -The callback URL (to configure in the IDP is built as `${drydock_public_url}/auth/oidc/${auth_name}/cb` +The callback URL (to configure in the IDP) is built as `${DD_PUBLIC_URL}/auth/oidc/${auth_name}/cb` -drydock tries its best to determine the public address to forge redirections on its own. -If it fails (irregular reverse proxy configuration...), you can enforce the value using the env var `DD_PUBLIC_URL` +`DD_PUBLIC_URL` is **required** when OIDC is configured. Without it, the OIDC provider will fail to register at startup and the login page will show "No authentication methods configured". Check startup logs for the exact error. If your IdP is on a different domain and callback fails with `OIDC session is missing or expired`, check `DD_SERVER_COOKIE_SAMESITE` (server config). Default is `lax` for OIDC compatibility; `strict` can break cross-site callbacks. Use `none` only for explicit cross-site/embed cases over HTTPS. `http://` OIDC discovery URLs are deprecated and will be removed in v1.6.0. When the discovery URL uses `http:`, drydock passes `allowInsecureRequests` to the OIDC client. Migrate your IdP to HTTPS before upgrading. +If your provider uses a private/self-signed CA, the quickest container-level workaround is `NODE_EXTRA_CA_CERTS=/path/to/ca.pem`. Drydock now also supports OIDC-scoped TLS settings with `DD_AUTH_OIDC_{auth_name}_CAFILE` and `DD_AUTH_OIDC_{auth_name}_INSECURE`. + +`DD_AUTH_OIDC_{auth_name}_INSECURE=true` disables TLS certificate verification for OIDC HTTPS calls (discovery, token, userinfo, JWKS). Use only for local/dev troubleshooting. + **Redirect URL validation** — Drydock validates all OIDC authorization redirect URLs against an allowlist derived from the configured callback URL origin. This prevents open redirect attacks through crafted callback parameters. No configuration is needed — validation is enforced automatically. **OIDC logout URL resolution** — drydock first tries the provider discovery `end_session_endpoint`. If it is missing, you can set `DD_AUTH_OIDC_{auth_name}_LOGOUTURL` when your provider (or proxy) exposes a logout URL outside discovery metadata. If neither is available, `POST /auth/logout` only destroys the local drydock session. @@ -88,6 +94,7 @@ services: image: codeswhat/drydock ... environment: + - DD_PUBLIC_URL=https:// - DD_AUTH_OIDC_AUTHELIA_CLIENTID=my-drydock-client-id - DD_AUTH_OIDC_AUTHELIA_CLIENTSECRET=this-is-a-very-secure-secret - DD_AUTH_OIDC_AUTHELIA_DISCOVERY=https:///.well-known/openid-configuration @@ -97,6 +104,7 @@ services: ```bash docker run \ + -e DD_PUBLIC_URL="https://" \ -e DD_AUTH_OIDC_AUTHELIA_CLIENTID="my-drydock-client-id" \ -e DD_AUTH_OIDC_AUTHELIA_CLIENTSECRET="this-is-a-very-secure-secret" \ -e DD_AUTH_OIDC_AUTHELIA_DISCOVERY="https:///.well-known/openid-configuration" \ @@ -129,6 +137,7 @@ services: image: codeswhat/drydock ... environment: + - DD_PUBLIC_URL=https:// - DD_AUTH_OIDC_AUTH0_CLIENTID= - DD_AUTH_OIDC_AUTH0_CLIENTSECRET= - DD_AUTH_OIDC_AUTH0_DISCOVERY=https:///.well-known/openid-configuration @@ -138,6 +147,7 @@ services: ```bash docker run \ + -e DD_PUBLIC_URL="https://" \ -e DD_AUTH_OIDC_AUTH0_CLIENTID="" \ -e DD_AUTH_OIDC_AUTH0_CLIENTSECRET="" \ -e DD_AUTH_OIDC_AUTH0_DISCOVERY="https:///.well-known/openid-configuration" \ @@ -182,6 +192,7 @@ services: image: codeswhat/drydock ... environment: + - DD_PUBLIC_URL=https:// - DD_AUTH_OIDC_AUTHENTIK_CLIENTID= - DD_AUTH_OIDC_AUTHENTIK_CLIENTSECRET= - DD_AUTH_OIDC_AUTHENTIK_DISCOVERY=/application/o//.well-known/openid-configuration @@ -192,6 +203,7 @@ services: ```bash docker run \ + -e DD_PUBLIC_URL="https://" \ -e DD_AUTH_OIDC_AUTHENTIK_CLIENTID="" \ -e DD_AUTH_OIDC_AUTHENTIK_CLIENTSECRET="" \ -e DD_AUTH_OIDC_AUTHENTIK_DISCOVERY="/application/o//.well-known/openid-configuration" \ @@ -217,6 +229,7 @@ services: image: codeswhat/drydock ... environment: + - DD_PUBLIC_URL=https:// - DD_AUTH_OIDC_DEX_CLIENTID= - DD_AUTH_OIDC_DEX_CLIENTSECRET= - DD_AUTH_OIDC_DEX_DISCOVERY=https:///dex/.well-known/openid-configuration @@ -228,6 +241,7 @@ services: ```bash docker run \ + -e DD_PUBLIC_URL="https://" \ -e DD_AUTH_OIDC_DEX_CLIENTID="" \ -e DD_AUTH_OIDC_DEX_CLIENTSECRET="" \ -e DD_AUTH_OIDC_DEX_DISCOVERY="https:///dex/.well-known/openid-configuration" \ diff --git a/content/docs/current/configuration/triggers/docker-compose/index.mdx b/content/docs/current/configuration/triggers/docker-compose/index.mdx index d942db95..34e65e11 100644 --- a/content/docs/current/configuration/triggers/docker-compose/index.mdx +++ b/content/docs/current/configuration/triggers/docker-compose/index.mdx @@ -56,9 +56,11 @@ Most Docker Compose users do not need to configure the file path at all. As long Do not forget to mount the docker-compose.yml file in the drydock container. +The Docker Compose trigger uses the Docker Engine API directly for all runtime operations (pull, stop, recreate). The `docker compose` / `docker-compose` CLI binary is **not required** inside the drydock container. + ## Tag vs digest updates -For **tag-based updates**, the trigger patches the compose file with the new tag and then recreates the container. For **digest-only updates** (same tag, new digest), the compose file is left unchanged and only the container runtime is refreshed via `docker compose pull` + `docker compose up -d`. +For **tag-based updates**, the trigger patches the compose file with the new tag and then recreates the container. For **digest-only updates** (same tag, new digest), the compose file is left unchanged and only the container runtime is refreshed (pull new image, stop/remove old container, recreate with new image). When `DIGESTPINNING=true`, compose mutations use digest references (`image@sha256:...`) when a digest is available. @@ -72,7 +74,7 @@ When a tag-based update requires a compose file change, Drydock patches Compose If a service uses flow-style mapping without an `image` key (for example `nginx: { restart: always }`), Drydock will not auto-insert `image`. Convert that service to block-style YAML first. -Before persisting a compose mutation, Drydock validates the candidate file with `docker compose config --quiet` (and falls back to `docker-compose config --quiet` when needed). Invalid candidate configs are not written. +Before persisting a compose mutation, Drydock validates the candidate file by parsing the full compose chain in-process. Invalid candidate configs are not written. ## Reconciliation behavior @@ -84,7 +86,7 @@ Before lifecycle execution, Drydock checks the running container image against t ## Compose-file-once mode -When `COMPOSEFILEONCE=true` and multiple services in the same compose stack/file require runtime refresh, Drydock executes one grouped compose pull/up pass for that stack and skips repeated per-service compose pull/up calls in that batch. +When `COMPOSEFILEONCE=true` and multiple services in the same compose stack/file require runtime refresh, Drydock performs one runtime refresh per unique service and skips repeated refreshes for subsequent containers sharing the same service in that batch. ## Multi-file compose chains @@ -94,7 +96,7 @@ During an update: 1. Drydock searches every file in the chain to find which one declares the target service's `image` key. 2. The last file in the chain that contains the service definition and is writable is selected as the update target. This matches Docker Compose override semantics where later files take precedence. -3. All files in the chain are passed to `docker compose -f file1 -f file2 config --quiet` for validation before and after any mutation. +3. All files in the chain are parsed in-process for validation before and after any mutation. The compose file chain for each container is visible in the container detail view of the UI. diff --git a/content/docs/current/configuration/triggers/mqtt/index.mdx b/content/docs/current/configuration/triggers/mqtt/index.mdx index 18492480..af37f6c3 100644 --- a/content/docs/current/configuration/triggers/mqtt/index.mdx +++ b/content/docs/current/configuration/triggers/mqtt/index.mdx @@ -23,7 +23,9 @@ The `mqtt` trigger lets you send container update notifications to an MQTT broke | `DD_TRIGGER_MQTT_{trigger_name}_HASS_DISCOVERY` | ⚪ | Enable [Home-assistant](https://www.home-assistant.io/) integration including discovery | `true`, `false` | `false` | | `DD_TRIGGER_MQTT_{trigger_name}_HASS_PREFIX` | ⚪ | Base topic for hass entity discovery | | `homeassistant` | | `DD_TRIGGER_MQTT_{trigger_name}_HASS_ATTRIBUTES` | ⚪ | Attribute preset controlling which container fields are included in MQTT payloads. `full` sends everything; `short` excludes large fields like SBOM documents, scan vulnerabilities, details, and labels. | `full`, `short` | `full` | -| `DD_TRIGGER_MQTT_{trigger_name}_EXCLUDE` | ⚪ | Comma-separated dot-paths to exclude from the MQTT payload. Overrides the `HASS_ATTRIBUTES` preset when set. For example `security.sbom.documents,labels` removes those nested fields before publishing. | Comma-separated dot-paths | | +| `DD_TRIGGER_MQTT_{trigger_name}_HASS_FILTER_INCLUDE` | ⚪ | Comma-separated list of flattened MQTT attribute keys to keep (include-mode). When set, only these keys are published. | Comma-separated flattened keys | | +| `DD_TRIGGER_MQTT_{trigger_name}_HASS_FILTER_EXCLUDE` | ⚪ | Comma-separated list of flattened MQTT attribute keys to remove (exclude-mode). Used when `HASS_FILTER_INCLUDE` is empty. | Comma-separated flattened keys | | +| `DD_TRIGGER_MQTT_{trigger_name}_EXCLUDE` | ⚪ | Legacy comma-separated dot-paths to exclude from the nested container object before flattening. Used only when both `HASS_FILTER_INCLUDE` and `HASS_FILTER_EXCLUDE` are empty. | Comma-separated dot-paths | | | `DD_TRIGGER_MQTT_{trigger_name}_TLS_CACHAIN` | ⚪ | The path to the file containing the server CA chain (when TLS with a private Certificate Authority) | Any valid file path | | | `DD_TRIGGER_MQTT_{trigger_name}_TLS_CLIENTCERT` | ⚪ | The path to the file containing the client public certificate (when TLS mutual authentication) | Any valid file path | | | `DD_TRIGGER_MQTT_{trigger_name}_TLS_CLIENTKEY` | ⚪ | The path to the file containing the client private key (when TLS mutual authentication) | Any valid file path | | @@ -215,22 +217,42 @@ Entities expose all the details of the container as attributes: Container MQTT payloads can include large nested fields (SBOM documents, scan vulnerability arrays, environment details, labels) that exceed Home Assistant's `json_attributes_topic` size limits or clutter entity attributes. -Two layers of filtering are available: +Filtering is applied in this precedence order: -### Layer 1: Attribute preset (`HASS_ATTRIBUTES`) +1. `HASS_FILTER_INCLUDE` (flattened key include list) +2. `HASS_FILTER_EXCLUDE` (flattened key exclude list) +3. `EXCLUDE` (legacy nested dot-path exclude list) +4. `HASS_ATTRIBUTES` preset fallback -| Preset | Behavior | -| --- | --- | -| `full` (default) | Sends the entire container object — no filtering | -| `short` | Excludes `security.sbom.documents`, `security.updateSbom.documents`, `security.scan.vulnerabilities`, `security.updateScan.vulnerabilities`, `details`, and `labels` | +### Include mode (`HASS_FILTER_INCLUDE`) + +Set `DD_TRIGGER_MQTT_{trigger_name}_HASS_FILTER_INCLUDE` to keep only selected flattened keys. + +```yaml +environment: + - DD_TRIGGER_MQTT_MOSQUITTO_HASS_FILTER_INCLUDE=name,image_name,update_available,result_tag +``` -### Layer 2: Explicit exclude paths (`EXCLUDE`) +### Exclude mode (`HASS_FILTER_EXCLUDE`) -Set `DD_TRIGGER_MQTT_{trigger_name}_EXCLUDE` to a comma-separated list of dot-paths to remove from the payload. When `EXCLUDE` is set, it overrides the preset entirely. +Set `DD_TRIGGER_MQTT_{trigger_name}_HASS_FILTER_EXCLUDE` to remove selected flattened keys. ```yaml environment: - - DD_TRIGGER_MQTT_MOSQUITTO_EXCLUDE=security.sbom.documents,security.scan.vulnerabilities,labels + - DD_TRIGGER_MQTT_MOSQUITTO_HASS_FILTER_EXCLUDE=security_sbom_documents_0_spdx_version,security_scan_vulnerabilities_0_id ``` -Filtering clones only the path ancestors needed for deletion — unrelated fields keep their original references with no deep-clone overhead. When no filtering is configured, the original container object is passed through with zero overhead. +### Legacy nested exclude (`EXCLUDE`) + +`DD_TRIGGER_MQTT_{trigger_name}_EXCLUDE` still supports comma-separated nested dot-paths (for example `security.sbom.documents,labels`). This legacy filter is only used when `HASS_FILTER_INCLUDE` and `HASS_FILTER_EXCLUDE` are both empty. + +### Attribute preset fallback (`HASS_ATTRIBUTES`) + +When no explicit include/exclude list is configured, `HASS_ATTRIBUTES` is used: + +| Preset | Behavior | +| --- | --- | +| `full` (default) | Sends the entire container object — no filtering | +| `short` | Excludes `security.sbom.documents`, `security.updateSbom.documents`, `security.scan.vulnerabilities`, `security.updateScan.vulnerabilities`, `details`, and `labels` | + +`HASS_FILTER_INCLUDE` and `HASS_FILTER_EXCLUDE` match flattened MQTT keys (snake_case + underscore delimiter), because filtering is applied after flattening to the payload shape Home Assistant receives. diff --git a/lefthook.yml b/lefthook.yml index 773ad551..57ac61c4 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -2,9 +2,12 @@ # Lefthook configuration — mirrors CI pipeline for local/CI parity. # # Pre-push pipeline (piped = sequential, fail-fast): +# 0. Clean tree gate — rejects untracked/uncommitted/stashed files # 1. Lint gate (~20s) — catches formatting/lint before burning CPU # 2. Build + test (~26s parallel) — independent workspaces run concurrently -# 3. E2E + security (advisory) +# 3. E2E + advisory scans (zizmor) +# +# Snyk scans are CI-only (release workflow) to preserve the 200/month quota. # # Biome runs directly (not via qlty) because qlty's biome integration # does not reliably apply fixes. Qlty handles all other linters. @@ -24,6 +27,32 @@ pre-commit: pre-push: piped: true commands: + # ── Clean tree gate: fail if working tree is dirty ───────────────── + # Catches untracked files (which hang qlty on interactive prompts), + # uncommitted changes (which pass locally but fail CI), and stashed + # changes (which hide work that should be committed or discarded). + clean-tree: + run: | + dirty=$(git status --porcelain 2>/dev/null) + stash=$(git stash list 2>/dev/null) + fail=0 + if [ -n "$dirty" ]; then + echo "❌ Working tree is not clean:" + echo "$dirty" + echo "" + echo "Commit, stash, or remove these files before pushing." + fail=1 + fi + if [ -n "$stash" ]; then + echo "⚠️ Stash is not empty:" + echo "$stash" + echo "" + echo "Apply or drop stashed changes — they won't be in CI." + fail=1 + fi + exit $fail + priority: 0 + # ── Lint gate: fast checks that catch formatting/lint issues ────── ts-nocheck: run: node scripts/check-ts-nocheck-allowlist.mjs @@ -52,19 +81,3 @@ pre-push: skip: - run: '! command -v zizmor >/dev/null 2>&1' priority: 6 - - # ── Local security gates (skip if tool missing or feature branch) ─ - # Snyk has a 200 scan/month limit — only run on release/main pushes. - snyk-deps: - root: app/ - run: ../scripts/snyk-deps-gate.sh --org=codeswhat - skip: - - run: '! command -v snyk >/dev/null 2>&1' - - run: 'git rev-parse --abbrev-ref HEAD | grep -qvE "^(main|release/)"' - priority: 7 - snyk-code: - run: scripts/snyk-code-gate.sh --org=codeswhat - skip: - - run: '! command -v snyk >/dev/null 2>&1' - - run: 'git rev-parse --abbrev-ref HEAD | grep -qvE "^(main|release/)"' - priority: 8 diff --git a/test/mosquitto.conf b/test/mosquitto.conf new file mode 100644 index 00000000..c8348ac4 --- /dev/null +++ b/test/mosquitto.conf @@ -0,0 +1,2 @@ +listener 1883 +allow_anonymous true diff --git a/test/qa-compose.yml b/test/qa-compose.yml index 597d2c80..4666af9c 100644 --- a/test/qa-compose.yml +++ b/test/qa-compose.yml @@ -7,6 +7,7 @@ services: - "3333:3000" volumes: - /var/run/docker.sock:/var/run/docker.sock + - ./qa-compose.yml:/drydock/qa-compose.yml:ro environment: - DD_RUN_AS_ROOT=true - DD_ALLOW_INSECURE_ROOT=true @@ -38,6 +39,16 @@ services: - DD_TRIGGER_SMTP_MYEMAIL_PASS=fakesmtppassword - DD_TRIGGER_SMTP_MYEMAIL_FROM=alerts@example.com - DD_TRIGGER_SMTP_MYEMAIL_TO=admin@example.com + # --- MQTT trigger (Home Assistant mode with filtering) --- + - DD_TRIGGER_MQTT_QA_URL=mqtt://mosquitto:1883 + - DD_TRIGGER_MQTT_QA_HASS_ENABLED=true + - DD_TRIGGER_MQTT_QA_HASS_PREFIX=homeassistant + - DD_TRIGGER_MQTT_QA_HASS_DISCOVERY=true + - DD_TRIGGER_MQTT_QA_HASS_ATTRIBUTES=short + - DD_TRIGGER_MQTT_QA_HASS_FILTER_INCLUDE=name,image_name,result_tag,update_kind_kind + # --- Docker Compose trigger (manual only) --- + - DD_TRIGGER_DOCKERCOMPOSE_QA_FILE=/drydock/qa-compose.yml + - DD_TRIGGER_DOCKERCOMPOSE_QA_AUTO=false # --- GHCR registry (authenticated) --- - DD_REGISTRY_GHCR_PRIVATE_USERNAME=myghuser - DD_REGISTRY_GHCR_PRIVATE_TOKEN=ghp_fakeGitHubTokenForTesting123 @@ -52,7 +63,7 @@ services: - DD_SECURITY_BLOCK_SEVERITY=CRITICAL - DD_SECURITY_SBOM_ENABLED=true - DD_SECURITY_SBOM_FORMATS=spdx-json,cyclonedx-json - - DD_SECURITY_VERIFY_SIGNATURES=true + - DD_SECURITY_VERIFY_SIGNATURES=false # --- Maintenance window (QA) --- # Uncomment to test; adjust cron to match/not-match current time # - DD_WATCHER_LOCAL_MAINTENANCE_WINDOW=0 2-6 * * * @@ -66,6 +77,8 @@ services: depends_on: docker-remote: condition: service_healthy + mosquitto: + condition: service_healthy # ── OIDC Identity Provider (Dex) ───────────────────── dex: @@ -77,6 +90,21 @@ services: - ./dex-config.yaml:/etc/dex/config.yaml:ro command: ["dex", "serve", "/etc/dex/config.yaml"] + # ── MQTT broker (Mosquitto) ────────────────────────── + mosquitto: + image: eclipse-mosquitto:2 + container_name: mosquitto + ports: + - "1883:1883" + volumes: + - ./mosquitto.conf:/mosquitto/config/mosquitto.conf:ro + healthcheck: + test: ["CMD-SHELL", "mosquitto_sub -t '$$SYS/#' -C 1 -W 3 || exit 1"] + interval: 5s + timeout: 5s + retries: 5 + start_period: 5s + # ── Trivy vulnerability scanner (client-server mode) ── trivy-server: image: aquasec/trivy:latest @@ -119,6 +147,7 @@ services: # ── Containers with updates available ────────────────── # Nginx with lifecycle hooks (tests hooks UI + group view) + # Icon uses colon separator with nested prefix (tests Fix 03 + Fix 06) nginx-hooked: image: nginx:1.25.5 pull_policy: never @@ -126,12 +155,14 @@ services: labels: - dd.watch=true - dd.display.name=Nginx (Hooked) + - dd.display.icon=si:si:nginx - dd.group=web-stack - dd.hook.pre=echo "pre-update check" - dd.hook.post=echo "post-update cleanup" - dd.hook.timeout=30000 # Redis in same group (tests group view) + # Icon uses colon separator (tests Fix 06) redis-cache: image: redis:7.2.0 pull_policy: never @@ -139,9 +170,11 @@ services: labels: - dd.watch=true - dd.display.name=Redis Cache + - dd.display.icon=si:redis - dd.group=web-stack # Traefik with auto-rollback (tests rollback UI) + # Icon uses dash separator (standard format — baseline comparison) traefik-proxy: image: traefik:v3.0.0 pull_policy: never @@ -149,6 +182,7 @@ services: labels: - dd.watch=true - dd.display.name=Traefik Proxy + - dd.display.icon=si-traefikproxy - dd.group=infra - dd.rollback.auto=true - dd.rollback.window=120000 diff --git a/ui/babel.config.js b/ui/babel.config.js deleted file mode 100644 index eeca9b42..00000000 --- a/ui/babel.config.js +++ /dev/null @@ -1,8 +0,0 @@ -module.exports = { - presets: ['@babel/preset-env'], - env: { - test: { - presets: [['@babel/preset-env', { targets: { node: 'current' } }]], - }, - }, -}; diff --git a/ui/src/components/ConfirmDialog.vue b/ui/src/components/ConfirmDialog.vue index 417806d0..2a3d8eb4 100644 --- a/ui/src/components/ConfirmDialog.vue +++ b/ui/src/components/ConfirmDialog.vue @@ -38,7 +38,7 @@ function handleKeydown(e: KeyboardEvent) { !isTextEntryTarget(e.target) ) { e.preventDefault(); - accept(); + void accept(); } } @@ -76,7 +76,7 @@ onUnmounted(() => globalThis.removeEventListener('keydown', handleKeydown));
+
@@ -152,6 +184,17 @@ const { +
+ + {{ error }} + +
+ diff --git a/ui/src/components/containers/ContainerFullPageTabContent.vue b/ui/src/components/containers/ContainerFullPageTabContent.vue index c5ddf4b9..2cc8e501 100644 --- a/ui/src/components/containers/ContainerFullPageTabContent.vue +++ b/ui/src/components/containers/ContainerFullPageTabContent.vue @@ -127,6 +127,7 @@ const { updateOperationsError, scanContainer, confirmUpdate, + confirmForceUpdate, registryColorBg, registryColorText, registryLabel, @@ -679,7 +680,15 @@ const { @click="runContainerPreview"> {{ previewLoading ? 'Previewing...' : 'Preview Update' }} - + + - + - @@ -343,15 +353,19 @@ const {
@@ -359,16 +373,22 @@ const {
- @@ -556,25 +576,35 @@ const {