From 2640641262944b5dc1282bbddf0320d2c738fd90 Mon Sep 17 00:00:00 2001 From: Jarek Krochmalski Date: Sun, 6 Sep 2026 17:03:48 +0530 Subject: [PATCH] deploy a 1:1-mounted local stack by file so its compose path is preserved --- src/lib/server/git.ts | 20 ++++++--- src/lib/server/stacks.ts | 32 +++++++++++--- src/routes/api/stacks/+server.ts | 18 +++++--- tests/rewrite-compose-volume-paths.test.ts | 50 ++++++++++++++++++++++ 4 files changed, 102 insertions(+), 18 deletions(-) create mode 100644 tests/rewrite-compose-volume-paths.test.ts diff --git a/src/lib/server/git.ts b/src/lib/server/git.ts index bff2dac..e538c27 100644 --- a/src/lib/server/git.ts +++ b/src/lib/server/git.ts @@ -21,6 +21,7 @@ import { sendEventNotification } from './notifications'; import { buildBasicAuthHeader } from './git-auth'; import { assertSafeRepoUrl, assertSafeGitRef, repoFilePath, repoBaseEnvPath } from './git-url-safety'; import { assertSafeRepoTarget } from './git-branch-lookup'; +import { shouldDeployGitStack, shouldForceRecreateGitStack } from './git-deploy-policy'; import { resolveStackBranch } from '../git-stack-branch'; import { parseManifest, @@ -1348,10 +1349,14 @@ export async function deployGitStack(stackId: number, options?: { force?: boolea console.log(`${logPrefix} Env file vars (masked):`, JSON.stringify(redactEnvVarsForLog(syncResult.envFileVars), null, 2)); } - // Check if there are changes - skip redeploy if no changes and not forced - // Note: For new stacks (first deploy), syncResult.updated will be true - // forceRedeploy setting overrides the skip logic for webhooks/scheduled syncs - const shouldDeploy = force || gitStack.forceRedeploy || syncResult.updated; + // Check if there are changes - skip redeploy if no changes and not forced. + // For new stacks (first deploy), syncResult.updated is true. The forceRedeploy + // setting overrides the skip logic for webhooks/scheduled syncs. + const shouldDeploy = shouldDeployGitStack({ + force, + forceRedeploy: gitStack.forceRedeploy === true, + gitUpdated: syncResult.updated === true + }); if (!shouldDeploy) { console.log(`${logPrefix} No changes detected and force=false, forceRedeploy=false, skipping redeploy`); const skippedResult = { @@ -1363,8 +1368,11 @@ export async function deployGitStack(stackId: number, options?: { force?: boolea return skippedResult; } - const forceRecreate = syncResult.updated; - console.log(`${logPrefix} Will force recreate:`, forceRecreate, `(updated=${syncResult.updated})`); + const forceRecreate = shouldForceRecreateGitStack({ + forceRedeploy: gitStack.forceRedeploy === true, + gitUpdated: syncResult.updated === true + }); + console.log(`${logPrefix} Will force recreate:`, forceRecreate, `(updated=${syncResult.updated}, forceRedeploy=${gitStack.forceRedeploy})`); console.log(`${logPrefix} Build on deploy:`, gitStack.buildOnDeploy); console.log(`${logPrefix} Re-pull images:`, gitStack.repullImages); console.log(`${logPrefix} Force redeploy setting:`, gitStack.forceRedeploy); diff --git a/src/lib/server/stacks.ts b/src/lib/server/stacks.ts index e33b4a6..173d55d 100644 --- a/src/lib/server/stacks.ts +++ b/src/lib/server/stacks.ts @@ -47,6 +47,7 @@ import { setStackInjectedSecretKeys } from './db'; import { getProvider } from './secretproviders'; +import { stripSurroundingQuotes } from './secretproviders/shared'; import { resolveComposeDockerHost, buildComposeBaseArgs } from './compose-docker-args'; import { unregisterSchedule } from './scheduler'; import { sendEventNotification } from './notifications'; @@ -803,7 +804,7 @@ export async function saveStackComposeFile( oldEnvPath?: string; // Old env file path for renaming secretProviderId?: number | null; // secret provider binding (undefined = unchanged) } -): Promise<{ success: boolean; error?: string }> { +): Promise<{ success: boolean; error?: string; composePath?: string }> { // Validate stack name - Docker Compose requires lowercase alphanumeric, hyphens, underscores // Must also start with a letter or number if (!/^[a-z0-9][a-z0-9_-]*$/.test(name)) { @@ -1002,7 +1003,10 @@ export async function saveStackComposeFile( try { writeFileSync(composeFile, content); - return { success: true }; + // Return the path actually written so the caller can persist it even when it + // supplied no explicit composePath (else the stored path is null while the file + // exists at the default location - #1515). + return { success: true, composePath: composeFile }; } catch (err: any) { return { success: false, error: `Failed to ${create ? 'create' : 'save'} compose file: ${err.message}` }; } @@ -3055,9 +3059,19 @@ export async function deployStack(options: DeployStackOptions): Promise leave actualEnvPath undefined (no env file) console.log(`${logPrefix} Using custom path from DB:`, workingDir); } else { // Default: compose file should already exist (written by saveStackComposeFile) @@ -3545,20 +3559,24 @@ async function resolveProviderEnvVars( // Only providers that support inline references detect any here. const isRef = (value: unknown): value is string => provider?.supportsReferences ? provider.isReference(value) : false; + // The canonical reference string for lookup: surrounding quotes stripped so a value + // pasted straight from 1Password's "Copy Secret Reference" (which includes quotes) + // resolves the same as the bare op://... form (#1521). The STORED value is untouched. + const normalizeRef = (value: string): string => stripSurroundingQuotes(value); const envFileRefs = new Map(); for (const [key, value] of Object.entries(envFileVars)) { if (isRef(value)) { - envFileRefs.set(key, value.trim()); + envFileRefs.set(key, normalizeRef(value)); } } const refs = new Set(); for (const value of Object.values(dbNonSecretVars)) { - if (isRef(value)) refs.add(value.trim()); + if (isRef(value)) refs.add(normalizeRef(value)); } for (const value of Object.values(secretVars)) { - if (isRef(value)) refs.add(value.trim()); + if (isRef(value)) refs.add(normalizeRef(value)); } for (const ref of envFileRefs.values()) refs.add(ref); @@ -3584,7 +3602,7 @@ async function resolveProviderEnvVars( let promotedFromDb = 0; for (const [key, value] of Object.entries(dbNonSecretVars)) { if (isRef(value)) { - const resolved = refMap.get(value.trim()); + const resolved = refMap.get(normalizeRef(value)); if (resolved !== undefined) { delete dbNonSecretVars[key]; secretVars[key] = resolved; @@ -3594,7 +3612,7 @@ async function resolveProviderEnvVars( } for (const [key, value] of Object.entries(secretVars)) { if (isRef(value)) { - const resolved = refMap.get(value.trim()); + const resolved = refMap.get(normalizeRef(value)); if (resolved !== undefined) { secretVars[key] = resolved; } diff --git a/src/routes/api/stacks/+server.ts b/src/routes/api/stacks/+server.ts index b6ecc03..0add289 100644 --- a/src/routes/api/stacks/+server.ts +++ b/src/routes/api/stacks/+server.ts @@ -1,7 +1,7 @@ import { json } from '@sveltejs/kit'; import { listComposeStacks, deployStack, saveStackComposeFile, writeStackEnvFile, writeRawStackEnvFile, saveStackEnvVarsToDb } from '$lib/server/stacks'; import { EnvironmentNotFoundError, DockerConnectionError } from '$lib/server/docker'; -import { upsertStackSource, getStackSources } from '$lib/server/db'; +import { upsertStackSource, getStackSources, secretProviderExists } from '$lib/server/db'; import { authorize } from '$lib/server/authorize'; import { auditStack } from '$lib/server/audit'; import { createJobResponse } from '$lib/server/sse'; @@ -149,6 +149,12 @@ export const POST: RequestHandler = async (event) => { return json({ error: 'Permission denied: binding a secret provider requires the secrets permission' }, { status: 403 }); } + // A stale provider id (provider deleted/recreated while the editor held the old + // list) would otherwise hit a raw foreign-key error on save (#1522). + if (typeof secretProviderId === 'number' && !(await secretProviderExists(secretProviderId))) { + return json({ error: 'The selected secret provider no longer exists. Reopen the stack and pick a current provider.' }, { status: 400 }); + } + // If start is false, only create the compose file without deploying if (start === false) { const result = await saveStackComposeFile(name, compose, true, envIdNum, { @@ -176,12 +182,13 @@ export const POST: RequestHandler = async (event) => { } } - // Record the stack as internally created with custom paths if provided + // Persist the path the file was actually written to (the default location + // when the caller omitted composePath), not null (#1515). await upsertStackSource({ stackName: name, environmentId: envIdNum, sourceType: 'internal', - composePath: composePath || undefined, + composePath: composePath || result.composePath || undefined, envPath: envPath || undefined, secretProviderId, }); @@ -218,12 +225,13 @@ export const POST: RequestHandler = async (event) => { } } - // Record the stack in DB before deploying - ensures it exists even if deploy fails + // Record the stack in DB before deploying - ensures it exists even if deploy fails. + // Persist the actual written path (default location when composePath omitted), not null (#1515). await upsertStackSource({ stackName: name, environmentId: envIdNum, sourceType: 'internal', - composePath: composePath || undefined, + composePath: composePath || saveResult.composePath || undefined, envPath: envPath || undefined, secretProviderId }); diff --git a/tests/rewrite-compose-volume-paths.test.ts b/tests/rewrite-compose-volume-paths.test.ts new file mode 100644 index 0000000..1175b21 --- /dev/null +++ b/tests/rewrite-compose-volume-paths.test.ts @@ -0,0 +1,50 @@ +import { describe, test, expect, afterEach } from 'bun:test'; +import { rewriteComposeVolumePaths, __setCachedMountsForTest } from '../src/lib/server/host-path'; + +afterEach(() => __setCachedMountsForTest(null)); + +const compose = (bind: string) => `services:\n app:\n image: x\n volumes:\n - ${bind}\n`; + +describe('rewriteComposeVolumePaths', () => { + test('1:1 mount (host path == container path) leaves the relative bind untouched (#1514)', () => { + // TerraMaster/1:1 case: the compose dir is mounted at the identical path, so + // translating ./data_db to the same absolute path is pointless and would force + // a stdin deploy that loses com.docker.compose.project.config_files. + __setCachedMountsForTest([ + { source: '/Volume1/Development/Docker', destination: '/Volume1/Development/Docker' } + ]); + const r = rewriteComposeVolumePaths(compose('./data_db:/var/lib/postgresql/data'), '/Volume1/Development/Docker/authentik'); + expect(r.modified).toBe(false); + expect(r.content).toContain('- ./data_db:/var/lib/postgresql/data'); + expect(r.changes.length).toBe(0); + }); + + test('non-1:1 mount (container path differs from host) still translates (no regression)', () => { + // The Dockhand-in-Docker case: /app/data mounted from /docker/data on the host. + __setCachedMountsForTest([{ source: '/docker/data', destination: '/app/data' }]); + const r = rewriteComposeVolumePaths(compose('./sub:/x'), '/app/data/stacks/myapp'); + expect(r.modified).toBe(true); + expect(r.content).toContain('- /docker/data/stacks/myapp/sub:/x'); + }); + + test('untranslatable relative bind (no matching mount) is left unchanged', () => { + __setCachedMountsForTest([{ source: '/docker/data', destination: '/app/data' }]); + const r = rewriteComposeVolumePaths(compose('./x:/y'), '/somewhere/else'); + expect(r.modified).toBe(false); + expect(r.content).toContain('- ./x:/y'); + }); + + test('no mounts known: nothing is translated', () => { + __setCachedMountsForTest([]); + const r = rewriteComposeVolumePaths(compose('./x:/y'), '/app/data/s'); + expect(r.modified).toBe(false); + }); + + test('named volumes and absolute binds are never touched', () => { + __setCachedMountsForTest([{ source: '/docker/data', destination: '/app/data' }]); + const named = 'services:\n app:\n volumes:\n - dbdata:/var/lib\n - /abs/host:/x\n'; + const r = rewriteComposeVolumePaths(named, '/app/data/s'); + expect(r.modified).toBe(false); + expect(r.content).toBe(named); + }); +});