From 4123793e68a43cc60b8e9b3a4665d1ae9be58886 Mon Sep 17 00:00:00 2001 From: Anso Date: Tue, 7 Jul 2026 14:41:18 -0400 Subject: [PATCH] fix(drift): resolve explicit network names that equal compose keys (#1588) Compare runtimeResourceName against the project-prefixed default instead of the compose key so networks like tailscale: { name: tailscale } are not mis-resolved as network_tailscale in Drift, Fleet summary, and preflight. Fixes #1581 --- .../compose-network-inspector.test.ts | 8 +++- backend/src/__tests__/drift-detection.test.ts | 38 +++++++++++++++++++ backend/src/__tests__/preflight-rules.test.ts | 30 ++++++++++++++- backend/src/services/network/normalize.ts | 5 ++- backend/src/services/preflight/rules.ts | 7 +--- 5 files changed, 77 insertions(+), 11 deletions(-) diff --git a/backend/src/__tests__/compose-network-inspector.test.ts b/backend/src/__tests__/compose-network-inspector.test.ts index 5cf33345..d8d0d062 100644 --- a/backend/src/__tests__/compose-network-inspector.test.ts +++ b/backend/src/__tests__/compose-network-inspector.test.ts @@ -127,10 +127,16 @@ describe('assembleStackNetworkFacts', () => { describe('runtimeResourceName', () => { it('uses a name override, else the project prefix', () => { expect(runtimeResourceName('myapp', 'backend', undefined)).toBe('myapp_backend'); - expect(runtimeResourceName('myapp', 'backend', 'backend')).toBe('myapp_backend'); // name == key is not an override + expect(runtimeResourceName('myapp', 'backend', 'myapp_backend')).toBe('myapp_backend'); + expect(runtimeResourceName('myapp', 'backend', 'backend')).toBe('backend'); expect(runtimeResourceName('myapp', 'shared', 'shared_net')).toBe('shared_net'); }); + it('resolves explicit name equal to the compose key (regression: #1581)', () => { + expect(runtimeResourceName('network', 'tailscale', 'tailscale')).toBe('tailscale'); + expect(runtimeResourceName('network', 'proxy', 'proxy')).toBe('proxy'); + }); + it('never project-prefixes an external resource (runtime name is the key, or a name override)', () => { // Compose references an external network/volume by its real name and never // prefixes the project, so an external resource with no name override keeps diff --git a/backend/src/__tests__/drift-detection.test.ts b/backend/src/__tests__/drift-detection.test.ts index 203117df..e927c303 100644 --- a/backend/src/__tests__/drift-detection.test.ts +++ b/backend/src/__tests__/drift-detection.test.ts @@ -396,6 +396,30 @@ describe('assembleStackDrift - network drift', () => { expect(report.findings.filter(f => f.kind.startsWith('network-'))).toEqual([]); }); + it('does not false-flag explicit network names equal to compose keys (regression: #1581)', () => { + const report = assembleStackDrift({ + stack: 'network', + declared: { + services: [service({ name: 'tsbridge', networks: ['tailscale'] })], + networks: { + tailscale: { external: false, name: 'tailscale' }, + }, + volumes: {}, + projectName: 'network', + }, + containers: [container({ + id: 'c1', + service: 'tsbridge', + stack: 'network', + networks: [{ name: 'tailscale', id: 't', ip: '' }, { name: 'network_default', id: 'd', ip: '' }], + })], + networks: [depNet('tailscale'), depNet('network_default')], + }); + expect(report.findings.filter(f => f.kind === 'network-undeclared')).toEqual([]); + expect(report.findings.filter(f => f.kind === 'network-missing')).toEqual([]); + expect(report.status).toBe('in-sync'); + }); + it('does not flag an attachment to a declared external network (regression: shared arr-net)', () => { // arr-net is declared `external: true` with no name override, so Docker attaches // the container to the pre-existing network named "arr-net" (no project prefix). @@ -444,6 +468,20 @@ describe('declaredFromEffectiveModel', () => { }; expect(fromDeclaredCompose(declaredFromEffectiveModel(model), 'myapp')).toEqual(fromEffectiveModel(model)); }); + + it('round-trips explicit network names equal to compose keys (regression: #1581)', () => { + const model: EffectiveModel = { + projectName: 'network', + services: [effSvc({ name: 'tsbridge', networks: [{ key: 'tailscale', aliases: [] }] })], + networks: { + proxy: { name: 'proxy', external: false, internal: false }, + tailscale: { name: 'tailscale', external: false, internal: false }, + vlan: { name: 'vlan', external: false, internal: false }, + }, + volumes: {}, + }; + expect(fromDeclaredCompose(declaredFromEffectiveModel(model), 'network')).toEqual(fromEffectiveModel(model)); + }); }); // ── normalizeImageRef ───────────────────────────────────────────────────── diff --git a/backend/src/__tests__/preflight-rules.test.ts b/backend/src/__tests__/preflight-rules.test.ts index 2a86fdc5..d55cf204 100644 --- a/backend/src/__tests__/preflight-rules.test.ts +++ b/backend/src/__tests__/preflight-rules.test.ts @@ -234,12 +234,38 @@ describe('network / volume rules', () => { const f = runRules(ctx({ model: m, existingNetworkNames: new Set(['shared']) })); expect(ids(f, 'external-network-missing')).toHaveLength(0); }); - it('reports a new network/volume as info when absent on the node', () => { - const m = model([svc()], { networks: { backend: { name: 'backend', external: false, internal: false } }, volumes: { data: { name: 'data', external: false, internal: false } } }); + it('reports a new network/volume as info when absent on the node (implicit default names)', () => { + const m = model([svc()], { + networks: { backend: { name: 'proj_backend', external: false, internal: false } }, + volumes: { data: { name: 'proj_data', external: false, internal: false } }, + }); const f = runRules(ctx({ model: m })); expect(ids(f, 'new-network')[0].severity).toBe('info'); + expect(ids(f, 'new-network')[0].message).toContain('proj_backend'); expect(ids(f, 'new-volume')[0].message).toContain('proj_data'); }); + it('uses explicit name equal to the compose key for new-network/new-volume (regression: #1581)', () => { + const m = model([svc()], { + projectName: 'proj', + networks: { backend: { name: 'backend', external: false, internal: false } }, + volumes: { data: { name: 'data', external: false, internal: false } }, + }); + const f = runRules(ctx({ model: m })); + expect(ids(f, 'new-network')[0].message).toContain('"backend"'); + expect(ids(f, 'new-network')[0].message).not.toContain('proj_backend'); + expect(ids(f, 'new-volume')[0].message).toContain('"data"'); + expect(ids(f, 'new-volume')[0].message).not.toContain('proj_data'); + }); + it('reports new-network with the true explicit name when project and key collide (regression: #1581)', () => { + const m = model([svc()], { + projectName: 'network', + networks: { tailscale: { name: 'tailscale', external: false, internal: false } }, + }); + const f = runRules(ctx({ model: m })); + const finding = ids(f, 'new-network')[0]; + expect(finding.message).toContain('tailscale'); + expect(finding.message).not.toContain('network_tailscale'); + }); it('flags an anonymous volume as info and stays silent without one', () => { const anon = model([svc({ storageMounts: [{ type: 'anonymous', target: '/data', readOnly: false }] })]); const f = runRules(ctx({ model: anon })); diff --git a/backend/src/services/network/normalize.ts b/backend/src/services/network/normalize.ts index b7a1daf6..c2694cb1 100644 --- a/backend/src/services/network/normalize.ts +++ b/backend/src/services/network/normalize.ts @@ -36,8 +36,9 @@ export function isHostNetwork(mode: string | undefined): boolean { * by its real name (the key, or a `name:` override), so prefixing it would invent * a `_` that no runtime resource matches and read as foreign drift. */ export function runtimeResourceName(projectName: string, key: string, declaredName: string | undefined, external = false): string { - if (declaredName && declaredName !== key) return declaredName; - return external ? key : `${projectName}_${key}`; + const defaultName = external ? key : `${projectName}_${key}`; + if (declaredName && declaredName !== defaultName) return declaredName; + return defaultName; } /** Extract host port numbers referenced by free-text access URLs, for the diff --git a/backend/src/services/preflight/rules.ts b/backend/src/services/preflight/rules.ts index 29a34099..f7b6cd49 100644 --- a/backend/src/services/preflight/rules.ts +++ b/backend/src/services/preflight/rules.ts @@ -1,7 +1,7 @@ import type { PreflightContext, PreflightFinding, PreflightSeverity, NodePortBinding } from './types'; import type { EffService, EffPortSpec } from './effectiveModel'; import type { ExposureIntent } from '../network/types'; -import { isLoopback } from '../network/normalize'; +import { isLoopback, runtimeResourceName } from '../network/normalize'; /** Higher number = more severe. Used to derive a run's overall status. */ export const SEVERITY_RANK: Record = { info: 0, warning: 1, high: 2, blocker: 3 }; @@ -52,11 +52,6 @@ function hasUidGidSignal(svc: EffService): boolean { return svc.user !== undefined || svc.envKeys.some(k => UID_GID_KEYS.has(k)); } -/** Resolved runtime name of a top-level network/volume (compose prefixes the project). */ -function runtimeResourceName(projectName: string, key: string, declaredName: string): string { - return declaredName !== key ? declaredName : `${projectName}_${key}`; -} - // ----- rules ---------------------------------------------------------------- const renderFailed: PreflightRule = {