Skip to content

Commit aeb027d

Browse files
committed
fix: nil check and valid apex domain check
1 parent 72524c9 commit aeb027d

3 files changed

Lines changed: 119 additions & 17 deletions

File tree

api/v1alpha/domain_types.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ const (
5757
// DNS or HTTP.
5858
DomainConditionVerified = "Verified"
5959

60+
// DomainConditionValidDomain indicates whether the provided domain name is
61+
// registrable (i.e., a valid eTLD+1) and suitable for verification/registration.
62+
DomainConditionValidDomain = "ValidDomain"
63+
6064
// This condition tracks verification attempts via DNS.
6165
DomainConditionVerifiedDNS = "VerifiedDNS"
6266

@@ -88,6 +92,13 @@ const (
8892
// DomainReasonVerified indicates domain ownership has been successfully
8993
// verified
9094
DomainReasonVerified = "Verified"
95+
96+
// DomainReasonInvalidDomain indicates the provided domain name is not
97+
// registrable (e.g., only a public suffix like "com"), and flows are paused.
98+
DomainReasonInvalidDomain = "InvalidApex"
99+
100+
// DomainReasonValid indicates the provided domain name is registrable.
101+
DomainReasonValid = "Valid"
91102
)
92103

93104
// DomainVerificationStatus represents the verification status of a domain

internal/controller/domain_controller.go

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,51 @@ func (r *DomainReconciler) Reconcile(ctx context.Context, req mcreconcile.Reques
7878

7979
origStatus := domain.Status.DeepCopy()
8080

81+
// Validate domain is registrable (eTLD+1 present and not a public suffix only)
82+
validCond := conditionutil.FindStatusConditionOrDefault(domain.Status.Conditions, &metav1.Condition{
83+
Type: networkingv1alpha.DomainConditionValidDomain,
84+
Status: metav1.ConditionUnknown,
85+
Reason: networkingv1alpha.DomainReasonValid,
86+
Message: "",
87+
LastTransitionTime: metav1.Now(),
88+
})
89+
validCond.ObservedGeneration = domain.Generation
90+
91+
apex, apexErr := registeredApex(domain.Spec.DomainName)
92+
registrable := (apexErr == nil && apex != "")
93+
if registrable {
94+
validCond.Status = metav1.ConditionTrue
95+
validCond.Reason = networkingv1alpha.DomainReasonValid
96+
validCond.Message = "Domain is registrable"
97+
} else {
98+
validCond.Status = metav1.ConditionFalse
99+
validCond.Reason = networkingv1alpha.DomainReasonInvalidDomain
100+
validCond.Message = "Domain is not registrable (public-suffix only or invalid)"
101+
// Clear timers to avoid auto-retries
102+
if domain.Status.Verification != nil {
103+
domain.Status.Verification.NextVerificationAttempt = metav1.Time{}
104+
}
105+
if domain.Status.Registration != nil {
106+
domain.Status.Registration.NextRegistrationAttempt = metav1.Time{}
107+
}
108+
}
109+
apimeta.SetStatusCondition(&domain.Status.Conditions, *validCond)
110+
111+
// Persist and short-circuit if invalid
112+
if validCond.Status == metav1.ConditionFalse {
113+
if !equality.Semantic.DeepEqual(*origStatus, domain.Status) {
114+
if err := cl.GetClient().Status().Update(ctx, domain); err != nil {
115+
return ctrl.Result{}, fmt.Errorf("failed updating domain status: %w", err)
116+
}
117+
}
118+
return ctrl.Result{}, nil
119+
}
120+
81121
// Delegate all verification work (including timers/backoff)
82122
nextVerification := r.reconcileVerification(ctx, domain)
83123

84124
// Delegate all registration work (including timers/backoff)
85-
nextRegistration := r.reconcileRegistration(ctx, domain)
125+
nextRegistration := r.reconcileRegistration(ctx, domain, apex)
86126

87127
// Persist status if changed
88128
if !equality.Semantic.DeepEqual(*origStatus, domain.Status) {
@@ -355,7 +395,7 @@ func defaultHTTPGet(ctx context.Context, url string) ([]byte, *http.Response, er
355395

356396
// reconcileRegistration keeps Status.Registration fresh and schedules independent timers.
357397
// Returns the next registration attempt time (zero if none).
358-
func (r *DomainReconciler) reconcileRegistration(ctx context.Context, d *networkingv1alpha.Domain) time.Time {
398+
func (r *DomainReconciler) reconcileRegistration(ctx context.Context, d *networkingv1alpha.Domain, apex string) time.Time {
359399
logger := log.FromContext(ctx)
360400

361401
st := &d.Status
@@ -370,12 +410,7 @@ func (r *DomainReconciler) reconcileRegistration(ctx context.Context, d *network
370410
return st.Registration.NextRegistrationAttempt.Time
371411
}
372412

373-
apex, err := registeredApex(d.Spec.DomainName)
374-
if err != nil || apex == "" {
375-
// if we cannot compute apex, short backoff
376-
st.Registration.NextRegistrationAttempt = metav1.NewTime(now.Add(r.Config.DomainRegistration.RetryBackoff.Duration))
377-
return st.Registration.NextRegistrationAttempt.Time
378-
}
413+
// apex is guaranteed valid by the ValidDomain gate
379414
st.Apex = strings.EqualFold(strings.TrimSuffix(d.Spec.DomainName, "."), apex)
380415

381416
ctxLookup, cancel := context.WithTimeout(ctx, r.Config.DomainRegistration.LookupTimeout.Duration)
@@ -400,7 +435,7 @@ func (r *DomainReconciler) reconcileRegistration(ctx context.Context, d *network
400435
st.Registration.NextRegistrationAttempt = metav1.NewTime(now.Add(r.Config.DomainRegistration.RetryBackoff.Duration))
401436
return st.Registration.NextRegistrationAttempt.Time
402437
}
403-
reg := r.mapRDAPDomainToRegistration(rdapDomain)
438+
reg := r.mapRDAPDomainToRegistration(*rdapDomain)
404439

405440
// Registry from bootstrap (authoritative)
406441
if resp.BootstrapAnswer != nil && len(resp.BootstrapAnswer.URLs) > 0 {
@@ -464,7 +499,7 @@ func (r *DomainReconciler) reconcileRegistration(ctx context.Context, d *network
464499
}
465500

466501
reg.Source = "rdap"
467-
st.Registration = reg
502+
st.Registration = &reg
468503

469504
// Schedule next refresh (with jitter)
470505
interval := r.Config.DomainRegistration.RefreshInterval.Duration
@@ -475,21 +510,18 @@ func (r *DomainReconciler) reconcileRegistration(ctx context.Context, d *network
475510
}
476511

477512
// mapRDAPDomainToRegistration maps a raw RDAP domain into our Registration model.
478-
func (r *DomainReconciler) mapRDAPDomainToRegistration(d *rdap.Domain) *networkingv1alpha.Registration {
479-
reg := &networkingv1alpha.Registration{}
480-
if d == nil {
481-
return reg
482-
}
513+
func (r *DomainReconciler) mapRDAPDomainToRegistration(d rdap.Domain) networkingv1alpha.Registration {
514+
reg := networkingv1alpha.Registration{}
483515

484516
reg.Domain = d.LDHName
485517
reg.Handle = d.Handle
486518

487519
// IDs & statuses
488520
reg.RegistryDomainID = pickRegistryDomainID(d.PublicIDs)
489-
copyStatuses(reg, d.Status)
521+
copyStatuses(&reg, d.Status)
490522

491523
// Lifecycle timestamps
492-
applyLifecycleFromEvents(reg, d.Events)
524+
applyLifecycleFromEvents(&reg, d.Events)
493525

494526
// DNSSEC
495527
if d.SecureDNS != nil {

internal/controller/domain_controller_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,65 @@ func TestDomainVerification(t *testing.T) {
421421
}
422422
}
423423

424+
func TestValidDomainGate_InvalidApex_SetsConditionAndSkipsFlows(t *testing.T) {
425+
testScheme := runtime.NewScheme()
426+
assert.NoError(t, scheme.AddToScheme(testScheme))
427+
assert.NoError(t, networkingv1alpha.AddToScheme(testScheme))
428+
429+
operatorConfig := config.NetworkServicesOperator{}
430+
config.SetObjectDefaults_NetworkServicesOperator(&operatorConfig)
431+
432+
ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test", UID: uuid.NewUUID()}}
433+
434+
// Domain with non-registrable name (public suffix only)
435+
dom := &networkingv1alpha.Domain{
436+
ObjectMeta: metav1.ObjectMeta{
437+
Namespace: ns.Name,
438+
Name: "invalid",
439+
},
440+
Spec: networkingv1alpha.DomainSpec{DomainName: "com"},
441+
}
442+
443+
fakeClient := fake.NewClientBuilder().
444+
WithScheme(testScheme).
445+
WithObjects(dom, ns).
446+
WithStatusSubresource(dom).
447+
Build()
448+
449+
reconciler := &DomainReconciler{
450+
mgr: &fakeMockManager{cl: fakeClient},
451+
Config: operatorConfig,
452+
timeNow: time.Now,
453+
httpGet: func(ctx context.Context, url string) ([]byte, *http.Response, error) { return nil, nil, nil },
454+
lookupTXT: func(ctx context.Context, name string) ([]string, error) { return nil, nil },
455+
lookupNS: func(ctx context.Context, name string) ([]*net.NS, error) { return nil, &net.DNSError{IsNotFound: true} },
456+
lookupIP: func(ctx context.Context, name string) ([]net.IPAddr, error) { return nil, nil },
457+
rdapDo: func(ctx context.Context, req *rdap.Request) (*rdap.Response, error) { return &rdap.Response{}, nil },
458+
rdapQueryIP: func(ctx context.Context, ip string) (*rdap.IPNetwork, error) { return nil, nil },
459+
}
460+
461+
req := mcreconcile.Request{Request: reconcile.Request{NamespacedName: client.ObjectKey{Namespace: ns.Name, Name: dom.Name}}}
462+
_, err := reconciler.Reconcile(context.Background(), req)
463+
assert.NoError(t, err)
464+
465+
// Fetch and assert conditions were set and flows skipped
466+
got := &networkingv1alpha.Domain{}
467+
assert.NoError(t, fakeClient.Get(context.Background(), client.ObjectKey{Namespace: ns.Name, Name: dom.Name}, got))
468+
469+
cond := apimeta.FindStatusCondition(got.Status.Conditions, networkingv1alpha.DomainConditionValidDomain)
470+
if assert.NotNil(t, cond, "ValidDomain condition missing") {
471+
assert.Equal(t, metav1.ConditionFalse, cond.Status)
472+
assert.Equal(t, networkingv1alpha.DomainReasonInvalidDomain, cond.Reason)
473+
}
474+
// Timers should be zeroed so no automatic retries
475+
if got.Status.Verification != nil {
476+
assert.True(t, got.Status.Verification.NextVerificationAttempt.IsZero())
477+
}
478+
if got.Status.Registration != nil {
479+
assert.True(t, got.Status.Registration.NextRegistrationAttempt.IsZero())
480+
}
481+
}
482+
424483
func newDomain(namespace, name string, opts ...func(*networkingv1alpha.Domain)) *networkingv1alpha.Domain {
425484
domain := &networkingv1alpha.Domain{
426485
ObjectMeta: metav1.ObjectMeta{

0 commit comments

Comments
 (0)