Skip to content

Commit 56579eb

Browse files
committed
πŸ› fix(watcher): re-derive label-driven fields on docker update events
updateContainerFromInspect previously updated containerFound.labels on docker start/update/die events but did NOT recompute the derived fields that depend on labels (tagFamily, includeTags, excludeTags, transformTags, linkTemplate, triggerInclude, triggerExclude). After a `docker compose up -d` recreate, the stored Container.tagFamily stayed undefined even though containerFound.labels['dd.tag.family'] = 'loose', so getTagFamilyPolicy() defaulted to 'strict' and the strict-family filter fired the misleading "Strict tag-family policy filtered out..." warning. The label→field derivation already had a canonical implementation (resolveLabelsFromContainer in container-init.ts) consumed by the full getContainers() path; this commit exposes it via a new applyDerivedLabelFieldsToContainer helper and wires it into updateContainerFromInspect so event-path updates stay consistent. displayName, displayIcon, and lookupImage are intentionally excluded: displayName has its own merge path via getCustomDisplayNameFromLabels, and displayIcon / lookupImage require imgset/image-reference context that's only available during the full watch cycle. Fixes: GH-342
1 parent ffd1b57 commit 56579eb

5 files changed

Lines changed: 274 additions & 0 deletions

File tree

β€Žapp/watchers/providers/docker/Docker.tsβ€Ž

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import Watcher from '../../Watcher.js';
3737
import { updateContainerFromInspect as updateContainerFromInspectState } from './container-event-update.js';
3838
import {
3939
type AliasFilterDecision,
40+
applyDerivedLabelFieldsToContainer,
4041
filterRecreatedContainerAliases,
4142
getDockerWatcherRegistryId,
4243
getDockerWatcherSourceKey,
@@ -990,6 +991,7 @@ class Docker extends Watcher<DockerWatcherConfiguration> {
990991
getCustomDisplayNameFromLabels: (labels) => getLabel(labels, ddDisplayName, wudDisplayName),
991992
updateContainer: (container) => storeContainer.updateContainer(container),
992993
logInfo: (message) => logContainer.info(message),
994+
applyDerivedLabelFieldsToContainer,
993995
});
994996
}
995997

β€Žapp/watchers/providers/docker/container-event-update.test.tsβ€Ž

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -619,4 +619,134 @@ describe('container event update helpers', () => {
619619
expect(container.labels).toEqual({ alpha: '1', beta: 'changed' });
620620
expect(updateContainer).toHaveBeenCalledWith(container);
621621
});
622+
623+
test('updateContainerFromInspect calls applyDerivedLabelFieldsToContainer when labels change', () => {
624+
const container = createMockContainer({
625+
name: 'my-app',
626+
displayName: 'my-app',
627+
status: 'running',
628+
labels: {},
629+
tagFamily: undefined,
630+
});
631+
const updateContainer = vi.fn();
632+
const applyDerivedLabelFieldsToContainer = vi.fn();
633+
634+
updateContainerFromInspect(
635+
container as any,
636+
{
637+
Name: '/my-app',
638+
State: { Status: 'running' },
639+
Config: { Labels: { 'dd.tag.family': 'loose' } },
640+
},
641+
{
642+
getCustomDisplayNameFromLabels: () => undefined,
643+
updateContainer,
644+
applyDerivedLabelFieldsToContainer,
645+
},
646+
);
647+
648+
expect(applyDerivedLabelFieldsToContainer).toHaveBeenCalledWith(container, {
649+
'dd.tag.family': 'loose',
650+
});
651+
expect(updateContainer).toHaveBeenCalledWith(container);
652+
});
653+
654+
test('updateContainerFromInspect does not call applyDerivedLabelFieldsToContainer when labels are unchanged', () => {
655+
const container = createMockContainer({
656+
name: 'my-app',
657+
displayName: 'my-app',
658+
status: 'running',
659+
labels: { 'dd.tag.family': 'loose' },
660+
});
661+
const updateContainer = vi.fn();
662+
const applyDerivedLabelFieldsToContainer = vi.fn();
663+
664+
updateContainerFromInspect(
665+
container as any,
666+
{
667+
Name: '/my-app',
668+
State: { Status: 'running' },
669+
Config: { Labels: { 'dd.tag.family': 'loose' } },
670+
},
671+
{
672+
getCustomDisplayNameFromLabels: () => undefined,
673+
updateContainer,
674+
applyDerivedLabelFieldsToContainer,
675+
},
676+
);
677+
678+
expect(applyDerivedLabelFieldsToContainer).not.toHaveBeenCalled();
679+
expect(updateContainer).not.toHaveBeenCalled();
680+
});
681+
682+
test('updateContainerFromInspect works without applyDerivedLabelFieldsToContainer dependency (backward compat)', () => {
683+
const container = createMockContainer({
684+
name: 'my-app',
685+
displayName: 'my-app',
686+
status: 'running',
687+
labels: {},
688+
});
689+
const updateContainer = vi.fn();
690+
691+
expect(() =>
692+
updateContainerFromInspect(
693+
container as any,
694+
{
695+
Name: '/my-app',
696+
State: { Status: 'running' },
697+
Config: { Labels: { 'dd.tag.family': 'loose' } },
698+
},
699+
{
700+
getCustomDisplayNameFromLabels: () => undefined,
701+
updateContainer,
702+
// applyDerivedLabelFieldsToContainer intentionally omitted
703+
},
704+
),
705+
).not.toThrow();
706+
707+
expect(updateContainer).toHaveBeenCalledWith(container);
708+
});
709+
710+
test('updateContainerFromInspect re-derives multiple label fields when labels change', () => {
711+
const container = createMockContainer({
712+
name: 'my-app',
713+
status: 'running',
714+
labels: { 'dd.tag.include': '^1\\.' },
715+
tagFamily: undefined,
716+
includeTags: '^1\\.',
717+
excludeTags: undefined,
718+
});
719+
const updateContainer = vi.fn();
720+
const applyDerivedLabelFieldsToContainer = vi.fn((c, labels) => {
721+
// Simulate what the real function does
722+
c.includeTags = labels['dd.tag.include'];
723+
c.excludeTags = labels['dd.tag.exclude'];
724+
c.tagFamily = labels['dd.tag.family'];
725+
});
726+
727+
updateContainerFromInspect(
728+
container as any,
729+
{
730+
Name: '/my-app',
731+
State: { Status: 'running' },
732+
Config: {
733+
Labels: {
734+
'dd.tag.include': '^2\\.',
735+
'dd.tag.exclude': '^alpha',
736+
'dd.tag.family': 'loose',
737+
},
738+
},
739+
},
740+
{
741+
getCustomDisplayNameFromLabels: () => undefined,
742+
updateContainer,
743+
applyDerivedLabelFieldsToContainer,
744+
},
745+
);
746+
747+
expect(container.includeTags).toBe('^2\\.');
748+
expect(container.excludeTags).toBe('^alpha');
749+
expect(container.tagFamily).toBe('loose');
750+
expect(updateContainer).toHaveBeenCalledWith(container);
751+
});
622752
});

β€Žapp/watchers/providers/docker/container-event-update.tsβ€Ž

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,14 @@ interface UpdateContainerFromInspectDependencies {
173173
getCustomDisplayNameFromLabels: (labels: Record<string, string>) => string | undefined;
174174
updateContainer: (container: Container) => void;
175175
logInfo?: (message: string) => void;
176+
/**
177+
* Re-derive label-driven fields (tagFamily, includeTags, etc.) from a fresh
178+
* label set and write them onto the container. Called when labels change.
179+
*/
180+
applyDerivedLabelFieldsToContainer?: (
181+
container: Container,
182+
labels: Record<string, string>,
183+
) => void;
176184
}
177185

178186
function areLabelsEqual(labelsA: Record<string, string>, labelsB: Record<string, string>): boolean {
@@ -238,6 +246,7 @@ export function updateContainerFromInspect(
238246

239247
if (labelsChanged) {
240248
containerFound.labels = labelsToApply;
249+
dependencies.applyDerivedLabelFieldsToContainer?.(containerFound, labelsToApply);
241250
changed = true;
242251
}
243252

β€Žapp/watchers/providers/docker/container-init-coverage.test.tsβ€Ž

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, expect, test, vi } from 'vitest';
22
import {
3+
applyDerivedLabelFieldsToContainer,
34
filterRecreatedContainerAliases,
45
getLabel,
56
getMatchingImgsetConfiguration,
@@ -330,6 +331,99 @@ describe('container-init coverage', () => {
330331
]);
331332
});
332333

334+
describe('applyDerivedLabelFieldsToContainer', () => {
335+
function makeContainer(overrides: Record<string, any> = {}) {
336+
return {
337+
id: 'ctr1',
338+
name: 'my-app',
339+
displayName: 'my-app',
340+
status: 'running',
341+
watcher: 'local',
342+
image: { name: 'library/nginx', tag: { value: '1.0', semver: true } },
343+
labels: {},
344+
updateAvailable: false,
345+
updateKind: { kind: 'unknown' },
346+
...overrides,
347+
} as any;
348+
}
349+
350+
test('derives tagFamily from dd.tag.family label', () => {
351+
const container = makeContainer();
352+
applyDerivedLabelFieldsToContainer(container, { 'dd.tag.family': 'loose' });
353+
expect(container.tagFamily).toBe('loose');
354+
});
355+
356+
test('derives includeTags from dd.tag.include label', () => {
357+
const container = makeContainer();
358+
applyDerivedLabelFieldsToContainer(container, { 'dd.tag.include': '^1\\..*' });
359+
expect(container.includeTags).toBe('^1\\..*');
360+
});
361+
362+
test('derives excludeTags from dd.tag.exclude label', () => {
363+
const container = makeContainer();
364+
applyDerivedLabelFieldsToContainer(container, { 'dd.tag.exclude': '^alpha' });
365+
expect(container.excludeTags).toBe('^alpha');
366+
});
367+
368+
test('derives transformTags from dd.tag.transform label', () => {
369+
const container = makeContainer();
370+
applyDerivedLabelFieldsToContainer(container, { 'dd.tag.transform': 's/v//' });
371+
expect(container.transformTags).toBe('s/v//');
372+
});
373+
374+
test('derives linkTemplate from dd.link.template label', () => {
375+
const container = makeContainer();
376+
applyDerivedLabelFieldsToContainer(container, {
377+
'dd.link.template': 'https://example.com/${major}',
378+
});
379+
expect(container.linkTemplate).toBe('https://example.com/${major}');
380+
});
381+
382+
test('derives triggerInclude from dd.action.include label', () => {
383+
const container = makeContainer();
384+
applyDerivedLabelFieldsToContainer(container, { 'dd.action.include': 'my-action' });
385+
expect(container.triggerInclude).toBe('my-action');
386+
});
387+
388+
test('derives triggerExclude from dd.notification.exclude label', () => {
389+
const container = makeContainer();
390+
applyDerivedLabelFieldsToContainer(container, { 'dd.notification.exclude': 'slack' });
391+
expect(container.triggerExclude).toBe('slack');
392+
});
393+
394+
test('falls back to wud.* label when dd.* label is absent', () => {
395+
const container = makeContainer();
396+
applyDerivedLabelFieldsToContainer(container, { 'wud.tag.include': '^v' });
397+
expect(container.includeTags).toBe('^v');
398+
});
399+
400+
test('clears derived fields when labels are removed', () => {
401+
const container = makeContainer({
402+
tagFamily: 'loose',
403+
includeTags: '^1\\..*',
404+
excludeTags: '^alpha',
405+
});
406+
applyDerivedLabelFieldsToContainer(container, {});
407+
expect(container.tagFamily).toBeUndefined();
408+
expect(container.includeTags).toBeUndefined();
409+
expect(container.excludeTags).toBeUndefined();
410+
});
411+
412+
test('handles undefined labels gracefully by treating as empty object', () => {
413+
const container = makeContainer({ tagFamily: 'loose' });
414+
// Pass empty record (undefined labels are normalized upstream before this point)
415+
applyDerivedLabelFieldsToContainer(container, {});
416+
expect(container.tagFamily).toBeUndefined();
417+
});
418+
419+
test('does not modify displayName (managed separately by event handler)', () => {
420+
const container = makeContainer({ displayName: 'My Custom App' });
421+
applyDerivedLabelFieldsToContainer(container, { 'dd.display.name': 'New Display Name' });
422+
// displayName is intentionally NOT updated by applyDerivedLabelFieldsToContainer
423+
expect(container.displayName).toBe('My Custom App');
424+
});
425+
});
426+
333427
test('filterRecreatedContainerAliases skips aliases that are still fresh', () => {
334428
const aliasName = '/7ea6b8a42686_termix';
335429
const container = {

β€Žapp/watchers/providers/docker/container-init.tsβ€Ž

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,45 @@ export function resolveLabelsFromContainer(
602602
return resolvedOverrides;
603603
}
604604

605+
/**
606+
* Re-derive label-driven container fields from a fresh label set and write
607+
* them back onto the stored container record.
608+
*
609+
* Used on the Docker-event update path (start/die/update events) where the
610+
* container already exists in the store but its labels may have changed since
611+
* it was first registered β€” e.g. after `docker compose up -d` recreates a
612+
* service with a new `dd.tag.family` label.
613+
*
614+
* Note: imgset configuration is intentionally NOT re-applied here because
615+
* imgset matching requires a parsed image reference that is not available on
616+
* the event path. The imgset defaults established during the last full watch
617+
* cycle remain in effect; only direct label values are refreshed.
618+
*/
619+
export function applyDerivedLabelFieldsToContainer(
620+
container: Container,
621+
labels: Record<string, string>,
622+
): void {
623+
const resolved = resolveLabelsFromContainer(labels);
624+
container.includeTags = resolved.includeTags;
625+
container.excludeTags = resolved.excludeTags;
626+
container.transformTags = resolved.transformTags;
627+
container.tagFamily = resolved.tagFamily;
628+
container.linkTemplate = resolved.linkTemplate;
629+
container.triggerInclude = resolved.triggerInclude;
630+
container.triggerExclude = resolved.triggerExclude;
631+
// displayName is managed separately by updateContainerFromInspect via
632+
// getCustomDisplayNameFromLabels, which handles the "no custom name β†’
633+
// fall back to container name" logic. We do not overwrite it here.
634+
//
635+
// displayIcon is stored but not re-derived on the event path because
636+
// Docker events do not carry image metadata needed to validate icon refs.
637+
// It will be refreshed on the next full watch cycle.
638+
//
639+
// lookupImage / registryLookupUrl flow into image.registry.lookupImage
640+
// which is part of the image reference block β€” only re-derived during a
641+
// full addImageDetailsToContainer pass, not on lightweight event updates.
642+
}
643+
605644
function resolveLookupImageFromContainerLabels(
606645
containerLabels: Record<string, string>,
607646
overrides: ContainerLabelOverrides,

0 commit comments

Comments
Β (0)