Skip to content

Commit fc50f25

Browse files
wenyingdcursoragent
andcommitted
fix(dns): wrap hostname-mismatch error as DNSZoneValidationError
ValidateEndpointsByZone returned a plain fmt.Errorf when a hostname did not match any allowed DNS zone. The controller only updates the DNSRecordReady condition on *DNSZoneValidationError, so the condition was left stale (True) while the reconciler looped with exponential backoff. Also fix both early-exit paths in the endpoint loop to return the already-computed allowedZones instead of nil, so the controller can correctly scope the stale-record cleanup to out-of-zone records. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent c952c11 commit fc50f25

2 files changed

Lines changed: 41 additions & 18 deletions

File tree

pkg/nsx/services/dns/recordservice_test.go

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -227,15 +227,17 @@ func TestValidateEndpointsByZone_table(t *testing.T) {
227227
ncWithoutDNSZones.Spec.DNSZones = nil
228228

229229
tests := []struct {
230-
name string
231-
nc *v1alpha1.VPCNetworkConfiguration
232-
vpcErr error
233-
buildOwner func(ns string) *ResourceRef // nil → tableOwner(ns)
234-
ns string
235-
eps []*extdns.Endpoint
236-
wantZone string
237-
errSub string
238-
wantN *int // when non-nil, assert len(rows)==*wantN instead of len(eps)
230+
name string
231+
nc *v1alpha1.VPCNetworkConfiguration
232+
vpcErr error
233+
buildOwner func(ns string) *ResourceRef // nil → tableOwner(ns)
234+
ns string
235+
eps []*extdns.Endpoint
236+
wantZone string
237+
errSub string
238+
wantZoneValErr bool // error must be *DNSZoneValidationError
239+
wantAllowedOnErr map[string]string // non-nil: allowedZones must equal this on error
240+
wantN *int // when non-nil, assert len(rows)==*wantN instead of len(eps)
239241
}{
240242
{
241243
name: "happy_path",
@@ -244,9 +246,10 @@ func TestValidateEndpointsByZone_table(t *testing.T) {
244246
wantN: intPtr(1),
245247
},
246248
{
247-
name: "no_DNS_zones_in_VPC_spec",
248-
nc: ncWithoutDNSZones,
249-
ns: "n", eps: []*extdns.Endpoint{ep}, errSub: "no DNS zones are permitted for the namespace",
249+
name: "no_DNS_zones_in_VPC_spec",
250+
nc: ncWithoutDNSZones,
251+
ns: "n", eps: []*extdns.Endpoint{ep}, errSub: "no DNS zones are permitted for the namespace",
252+
wantZoneValErr: true,
250253
},
251254
{
252255
name: "GetVPCNetworkConfigByNamespace_error",
@@ -278,10 +281,23 @@ func TestValidateEndpointsByZone_table(t *testing.T) {
278281
buildOwner: func(ns string) *ResourceRef {
279282
return &ResourceRef{Kind: "UnknownKind", Object: &metav1.ObjectMeta{Namespace: ns, Name: "x"}}
280283
},
281-
ns: "tenant",
282-
eps: []*extdns.Endpoint{ep},
283-
errSub: "unsupported resource kind",
284-
wantN: intPtr(0),
284+
ns: "tenant",
285+
eps: []*extdns.Endpoint{ep},
286+
errSub: "unsupported resource kind",
287+
wantZoneValErr: true,
288+
wantAllowedOnErr: map[string]string{testDNSZonePathT: "example.com"},
289+
wantN: intPtr(0),
290+
},
291+
{
292+
// hostname does not lie under any allowed zone → must be *DNSZoneValidationError
293+
// and allowedZones must still be returned so the controller can clean up stale records.
294+
name: "hostname_not_in_zone_is_DNSZoneValidationError_with_allowedZones",
295+
nc: testVPCNetworkConfiguration(), // zone = example.com
296+
ns: "tenant",
297+
eps: []*extdns.Endpoint{extdns.NewEndpoint("svc.other.example", extdns.RecordTypeA, "10.0.0.1")},
298+
errSub: "does not match any allowed DNS domain",
299+
wantZoneValErr: true,
300+
wantAllowedOnErr: map[string]string{testDNSZonePathT: "example.com"},
285301
},
286302
}
287303
for _, tt := range tests {
@@ -295,6 +311,13 @@ func TestValidateEndpointsByZone_table(t *testing.T) {
295311
if tt.errSub != "" {
296312
require.Error(t, err)
297313
require.Contains(t, err.Error(), tt.errSub)
314+
if tt.wantZoneValErr {
315+
var zve *DNSZoneValidationError
316+
require.ErrorAs(t, err, &zve, "expected *DNSZoneValidationError")
317+
}
318+
if tt.wantAllowedOnErr != nil {
319+
require.Equal(t, tt.wantAllowedOnErr, allowed, "allowedZones must be returned even on error")
320+
}
298321
return
299322
}
300323
require.NoError(t, err)

pkg/nsx/services/dns/zones.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,12 @@ func (s *DNSRecordService) ValidateEndpointsByZone(namespace string, owner *Reso
101101
}
102102
recName, zonePath, parseErr := s.getZonePathForHostname(z, ep.DNSName)
103103
if parseErr != nil {
104-
return nil, nil, parseErr
104+
return nil, allowedZones, &DNSZoneValidationError{Msg: parseErr.Error()}
105105
}
106106
log.Debug("Mapped DNS endpoint to zone", "dnsName", ep.DNSName, "zonePath", zonePath, "recordName", recName)
107107
row, validErr := s.validateEndpointRowConflict(zonePath, ep, recName, owner)
108108
if validErr != nil {
109-
return nil, nil, &DNSZoneValidationError{Msg: "DNS endpoint validation failed for DNS zone policy", Cause: validErr}
109+
return nil, allowedZones, &DNSZoneValidationError{Msg: "DNS endpoint validation failed for DNS zone policy", Cause: validErr}
110110
}
111111
rows = append(rows, *row)
112112
}

0 commit comments

Comments
 (0)