diff --git a/admin/app/utils/service_catalog_merge.ts b/admin/app/utils/service_catalog_merge.ts index 65503791..9a625213 100644 --- a/admin/app/utils/service_catalog_merge.ts +++ b/admin/app/utils/service_catalog_merge.ts @@ -108,14 +108,19 @@ export function mergeContainerConfigPreservingHostPorts * scheme is always taken, so a catalog change like Vaultwarden moving to `https:` * still reaches an install running on an alternate port. * - * The live port is only honoured when it matches a host port the install actually - * publishes. That keeps a stale or hand-edited `ui_location` from pinning the link - * to a port nothing is listening on. + * `mergedConfig` must be the container_config this sync is about to WRITE, meaning + * the output of `mergeContainerConfigPreservingHostPorts`, not the live row. The + * live port is only honoured when that config actually publishes it, which does + * two things: it keeps a stale or hand-edited `ui_location` from pinning the link + * to a port nothing is listening on, and it keeps the link in step with the config + * in the one case the catalog wins outright. When the catalog moves the + * container-side port, no live host port is preserved, so the link takes the + * catalog port rather than pointing at a port the recreate will not bind. */ export function mergeUiLocationPreservingHostPort( catalogUiLocation: string | null, liveUiLocation: string | null | undefined, - liveConfig: SerializedConfig + mergedConfig: SerializedConfig ): string | null { if (!catalogUiLocation || !liveUiLocation) return catalogUiLocation @@ -126,7 +131,7 @@ export function mergeUiLocationPreservingHostPort( const livePort = liveMatch[2] if (livePort === catalogMatch[2]) return catalogUiLocation - const bindings = readPortBindings(liveConfig) + const bindings = readPortBindings(mergedConfig) const publishedPorts = new Set( bindings ? Object.values(bindings) diff --git a/admin/database/seeders/service_seeder.ts b/admin/database/seeders/service_seeder.ts index 437a3c90..f4931033 100644 --- a/admin/database/seeders/service_seeder.ts +++ b/admin/database/seeders/service_seeder.ts @@ -584,11 +584,15 @@ export default class ServiceSeeder extends BaseSeeder { existing.container_config ) : service.container_config + // Derive the link from the config we are about to WRITE, not the live one. When a + // catalog change to the container-side port means the live host port was not + // preserved above, the link has to follow the catalog too, or the row ships a + // container bound to one port and an Open button pointing at another. const uiLocation = existing.installed ? mergeUiLocationPreservingHostPort( service.ui_location, existing.ui_location, - existing.container_config + containerConfig ) : service.ui_location diff --git a/admin/tests/unit/service_catalog_merge.spec.ts b/admin/tests/unit/service_catalog_merge.spec.ts index 97c98817..ee87f6ff 100644 --- a/admin/tests/unit/service_catalog_merge.spec.ts +++ b/admin/tests/unit/service_catalog_merge.spec.ts @@ -113,6 +113,27 @@ test('a path ui_location is never rewritten', () => { assert.equal(mergeUiLocationPreservingHostPort('/chat', '9999', config({ '80/tcp': '9999' })), '/chat') }) +// The third argument is the config the sync is about to WRITE, not the live row. +// When the catalog moves the container-side port, the port merge keeps nothing, so +// the link has to take the catalog port too. Feeding the live config here instead +// writes a row whose container binds 8090 while its Open button points at 9999. +test('follows the catalog when a container-side port change wins', () => { + const catalogConfig = config({ '3000/tcp': '8090' }) + const liveConfig = config({ '8080/tcp': '9999' }) + const mergedConfig = mergeContainerConfigPreservingHostPorts(catalogConfig, liveConfig) + + assert.equal(firstPublishedHostPort(mergedConfig), '8090') + assert.equal(mergeUiLocationPreservingHostPort('8090', '9999', mergedConfig), '8090') +}) + +test('a preserved host port still reaches ui_location through the merged config', () => { + const mergedConfig = mergeContainerConfigPreservingHostPorts( + config({ '8080/tcp': '8090' }), + config({ '8080/tcp': '9999' }) + ) + assert.equal(mergeUiLocationPreservingHostPort('8090', '9999', mergedConfig), '9999') +}) + test('ignores a live port the install does not actually publish', () => { // Stale or hand-edited ui_location: pinning the link to it would point the // Open button at a port with nothing behind it.