Skip to content

Commit c9e2337

Browse files
fix: Match DNS add to the owning record
Adding a record looked up the RecordSet by zone and type only, then took items[0]. Zones with more than one RecordSet of that type appended to the wrong object or created a duplicate, and the operator webhook rejected it as already claimed. Resolve by (zone, type, name). Cancel in-flight list fetches on delete so a stale refetch cannot put the row back. Fixes: #1511
1 parent c68528b commit c9e2337

10 files changed

Lines changed: 492 additions & 104 deletions

app/modules/watch/use-resource-watch.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export function useResourceWatch<T>({
4747
getItemKey,
4848
updateListCache,
4949
updateSingleCache,
50+
applyCacheUpdates = true,
5051
...watchOptions
5152
}: UseResourceWatchOptions<T>) {
5253
const queryClient = useQueryClient();
@@ -56,6 +57,7 @@ export function useResourceWatch<T>({
5657
const getItemKeyRef = useRef(getItemKey);
5758
const updateListCacheRef = useRef(updateListCache);
5859
const updateSingleCacheRef = useRef(updateSingleCache);
60+
const applyCacheUpdatesRef = useRef(applyCacheUpdates);
5961
const invalidateTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
6062
const subscriptionStartTimeRef = useRef<number>(0);
6163
const lastRefetchTimeRef = useRef<number>(0);
@@ -72,6 +74,7 @@ export function useResourceWatch<T>({
7274
getItemKeyRef.current = getItemKey;
7375
updateListCacheRef.current = updateListCache;
7476
updateSingleCacheRef.current = updateSingleCache;
77+
applyCacheUpdatesRef.current = applyCacheUpdates;
7578
throttleMsRef.current = throttleMs;
7679
debounceMsRef.current = debounceMs;
7780
skipInitialSyncRef.current = skipInitialSync;
@@ -137,6 +140,10 @@ export function useResourceWatch<T>({
137140
// Call custom event handler if provided
138141
onEventRef.current?.(transformedEvent);
139142

143+
if (!applyCacheUpdatesRef.current) {
144+
return;
145+
}
146+
140147
// Update React Query cache based on event type
141148
switch (event.type) {
142149
case 'ADDED':

app/modules/watch/watch.types.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,11 @@ export interface UseResourceWatchOptions<T> extends WatchOptions {
9494
* @example (oldData, newItem) => ({ ...newItem, preservedField: oldData.preservedField })
9595
*/
9696
updateSingleCache?: (oldData: T | undefined, newItem: T) => T;
97+
/**
98+
* When false, watch events are forwarded to `onEvent` only — the hook does
99+
* not write the query cache. Use when cache items are a different shape
100+
* than the watched object (e.g. flattened DNS rows vs DNSRecordSet).
101+
* @default true
102+
*/
103+
applyCacheUpdates?: boolean;
97104
}

app/resources/dns-records/dns-record.adapter.test.ts

Lines changed: 108 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
import {
2+
mergeRecordSetIntoListCache,
3+
ownerNameForResource,
4+
removeRecordSetFromListCache,
25
toCreateDnsRecordSetPayload,
36
toDnsRecordSet,
47
toDnsRecordSetList,
@@ -146,15 +149,116 @@ describe('toFlattenedDnsRecords', () => {
146149
});
147150
});
148151

152+
describe('mergeRecordSetIntoListCache', () => {
153+
it('replaces only the rows for that RecordSet after a record is removed', () => {
154+
const previous = toFlattenedDnsRecords([
155+
{
156+
uid: 'rs-txt',
157+
name: 'zone-txt-apex',
158+
recordType: 'TXT',
159+
dnsZoneId: 'z',
160+
records: [
161+
{ name: '@', txt: { content: 'keep' } },
162+
{ name: '@', txt: { content: 'drop' } },
163+
],
164+
},
165+
{
166+
uid: 'rs-www',
167+
name: 'zone-txt-www',
168+
recordType: 'TXT',
169+
dnsZoneId: 'z',
170+
records: [{ name: 'www', txt: { content: 'www-verify' } }],
171+
},
172+
] as never);
173+
174+
const patched = {
175+
uid: 'rs-txt',
176+
name: 'zone-txt-apex',
177+
recordType: 'TXT',
178+
dnsZoneId: 'z',
179+
records: [{ name: '@', txt: { content: 'keep' } }],
180+
} as DnsRecordSet;
181+
182+
const merged = mergeRecordSetIntoListCache(previous, patched);
183+
expect(merged.map((r) => `${r.recordSetName}:${r.value}`).sort()).toEqual([
184+
'zone-txt-apex:keep',
185+
'zone-txt-www:www-verify',
186+
]);
187+
});
188+
});
189+
190+
describe('removeRecordSetFromListCache', () => {
191+
it('drops every flattened row for a deleted RecordSet', () => {
192+
const previous = toFlattenedDnsRecords([
193+
{
194+
uid: 'rs-txt',
195+
name: 'zone-txt-apex',
196+
recordType: 'TXT',
197+
dnsZoneId: 'z',
198+
records: [{ name: '@', txt: { content: 'gone' } }],
199+
},
200+
{
201+
uid: 'rs-www',
202+
name: 'zone-txt-www',
203+
recordType: 'TXT',
204+
dnsZoneId: 'z',
205+
records: [{ name: 'www', txt: { content: 'stay' } }],
206+
},
207+
] as never);
208+
209+
const remaining = removeRecordSetFromListCache(previous, 'zone-txt-apex');
210+
expect(remaining?.map((r) => r.recordSetName)).toEqual(['zone-txt-www']);
211+
});
212+
});
213+
214+
describe('ownerNameForResource', () => {
215+
it('maps apex names to apex', () => {
216+
expect(ownerNameForResource('@')).toBe('apex');
217+
expect(ownerNameForResource('')).toBe('apex');
218+
expect(ownerNameForResource(undefined)).toBe('apex');
219+
});
220+
221+
it('sanitizes service and wildcard names for DNS-1123', () => {
222+
expect(ownerNameForResource('_dmarc')).toBe('dmarc');
223+
expect(ownerNameForResource('*')).toBe('wildcard');
224+
expect(ownerNameForResource('*.cdn')).toBe('wildcard-cdn');
225+
expect(ownerNameForResource('www')).toBe('www');
226+
});
227+
});
228+
149229
describe('toCreateDnsRecordSetPayload', () => {
150-
it('derives a lowercased name from zone id + record type', () => {
230+
it('includes a sanitized owner suffix so same-type RecordSets do not collide', () => {
151231
const payload = toCreateDnsRecordSetPayload(
152-
{ dnsZoneRef: { name: 'acme-zone' }, recordType: 'A', records: [] } as never,
232+
{
233+
dnsZoneRef: { name: 'acme-zone' },
234+
recordType: 'TXT',
235+
records: [{ name: '@', txt: { content: 'v=spf1 -all' } }],
236+
} as never,
153237
'Acme-Zone'
154238
);
155239
expect(payload.kind).toBe('DNSRecordSet');
156-
expect(payload.metadata?.name).toBe('acme-zone-a');
157-
expect(payload.spec?.recordType).toBe('A');
240+
expect(payload.metadata?.name).toBe('acme-zone-txt-apex');
241+
expect(payload.spec?.recordType).toBe('TXT');
242+
});
243+
244+
it('uses the owner name for subdomain RecordSets', () => {
245+
const payload = toCreateDnsRecordSetPayload(
246+
{
247+
dnsZoneRef: { name: 'acme-zone' },
248+
recordType: 'A',
249+
records: [{ name: 'www', a: { content: '1.2.3.4' } }],
250+
} as never,
251+
'acme-zone'
252+
);
253+
expect(payload.metadata?.name).toBe('acme-zone-a-www');
254+
});
255+
256+
it('defaults empty records to an apex suffix', () => {
257+
const payload = toCreateDnsRecordSetPayload(
258+
{ dnsZoneRef: { name: 'acme-zone' }, recordType: 'A', records: [] } as never,
259+
'Acme-Zone'
260+
);
261+
expect(payload.metadata?.name).toBe('acme-zone-a-apex');
158262
});
159263
});
160264

app/resources/dns-records/dns-record.adapter.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ import { ComMiloapisNetworkingDnsV1Alpha1DnsRecordSet } from '@/modules/control-
99
import { ControlPlaneStatus } from '@/resources/base';
1010
import { transformControlPlaneStatus } from '@/utils/helpers/control-plane.helper';
1111
import { extractValue } from '@/utils/helpers/dns/flatten.helper';
12+
import { normalizeRecordName } from '@/utils/helpers/dns/record-comparison.helper';
1213
import { getDnsRecordTypePriority } from '@/utils/helpers/dns/record-type.helper';
14+
import { sanitizeForK8s } from '@/utils/helpers/format.helper';
1315

1416
/** Labels set by the Gateway controller when a DNSRecordSet is created for Application Load Balancer (proxy) */
1517
const DNS_SOURCE_KIND_LABEL = 'dns.datumapis.com/source-kind';
@@ -132,6 +134,31 @@ export function toFlattenedDnsRecordsByPriority(recordSets: DnsRecordSet[]): Fla
132134
return toFlattenedDnsRecords(recordSets);
133135
}
134136

137+
/**
138+
* Replace flattened list rows for one RecordSet with rows from that object.
139+
* Used by create/update mutations and by the RecordSet watch (cache is flattened
140+
* rows, watch events are whole RecordSets).
141+
*/
142+
export function mergeRecordSetIntoListCache(
143+
old: FlattenedDnsRecord[] | undefined,
144+
recordSet: DnsRecordSet
145+
): FlattenedDnsRecord[] {
146+
const newRows = toFlattenedDnsRecords([recordSet]);
147+
if (!old) return newRows;
148+
return [...old.filter((record) => record.recordSetName !== recordSet.name), ...newRows];
149+
}
150+
151+
/**
152+
* Drop every flattened row that belonged to a deleted RecordSet.
153+
*/
154+
export function removeRecordSetFromListCache(
155+
old: FlattenedDnsRecord[] | undefined,
156+
recordSetName: string
157+
): FlattenedDnsRecord[] | undefined {
158+
if (!old) return old;
159+
return old.filter((record) => record.recordSetName !== recordSetName);
160+
}
161+
135162
/**
136163
* Extract TTL from record
137164
*/
@@ -142,18 +169,36 @@ function extractTTL(record: any): number | undefined {
142169
return undefined;
143170
}
144171

172+
/**
173+
* DNS-1123 owner suffix for a RecordSet resource name.
174+
* Apex (`@` / empty) becomes `apex` so same-type RecordSets at different
175+
* names do not collide on `{zone}-{type}`.
176+
*/
177+
export function ownerNameForResource(name: string | undefined | null): string {
178+
const normalized = normalizeRecordName(name);
179+
if (normalized === '@') return 'apex';
180+
if (normalized === '*') return 'wildcard';
181+
if (normalized.startsWith('*.')) {
182+
const rest = sanitizeForK8s(normalized.slice(2)).replace(/^\.+|\.+$/g, '');
183+
return rest ? `wildcard-${rest}` : 'wildcard';
184+
}
185+
const sanitized = sanitizeForK8s(normalized).replace(/^\.+|\.+$/g, '');
186+
return sanitized || 'apex';
187+
}
188+
145189
/**
146190
* Transform CreateDnsRecordSetInput to API payload
147191
*/
148192
export function toCreateDnsRecordSetPayload(
149193
input: CreateDnsRecordSetInput,
150194
dnsZoneId: string
151195
): ComMiloapisNetworkingDnsV1Alpha1DnsRecordSet {
196+
const ownerName = ownerNameForResource(input.records?.[0]?.name);
152197
return {
153198
kind: 'DNSRecordSet',
154199
apiVersion: 'dns.networking.miloapis.com/v1alpha1',
155200
metadata: {
156-
name: `${dnsZoneId}-${input.recordType}`.toLowerCase(),
201+
name: `${dnsZoneId}-${input.recordType}-${ownerName}`.toLowerCase(),
157202
},
158203
spec: {
159204
dnsZoneRef: input.dnsZoneRef,

0 commit comments

Comments
 (0)