From e4be5dfe41800d88c852ab3dc192a8ff90f0e11f Mon Sep 17 00:00:00 2001 From: reoring Date: Sun, 1 Nov 2026 22:03:08 +0900 Subject: [PATCH] endpoint: a provably never-published endpoint has no withdrawal obligation to capture DEP-0003 (#17) holds retirement capture whenever no publication Zone resolves for a retired hostname/address pair. Any ERS that changes hostname or address before a matching Zone exists therefore never captures: the endpoint controller returns early on pending retirements, observedGeneration freezes, active hostnames keep stale diagnostics, and the gateway endpoint controller never releases the HTTPRoute finalizer. On a platform without a DNS Provider this blocks every HTTPPublish from ever reaching its terminal state and every owning Project from deleting. Capture is now vacuous for a pair only after an explicit ZoneNotResolved result and authoritative proof that the endpoint was never published: no retained hostname Zone binding or generated-record reference/count, no sticky WithdrawalProtected=True, an uncached read finding no suffix-matching Zone, and an uncached source-label-bound read finding no generated RecordSet. Everything else keeps capture pending and retains finalizers: an unusable matching Zone, conversion/capability rejection, any existing or deleting child, incomplete lists, a missing reader, or a read error. A Zone that disappears after publication stays fail-closed. --- deps/dep-0003-record-withdrawal.md | 13 + docs/design/endpoint-api.md | 15 ++ .../endpointrecordset/controller.go | 5 + .../endpointrecordset/withdrawal.go | 22 +- .../endpointrecordset/withdrawal_no_zone.go | 53 +++++ .../withdrawal_no_zone_test.go | 222 ++++++++++++++++++ 6 files changed, 326 insertions(+), 4 deletions(-) create mode 100644 internal/go/apps/endpoint/controllers/endpointrecordset/withdrawal_no_zone.go create mode 100644 internal/go/apps/endpoint/controllers/endpointrecordset/withdrawal_no_zone_test.go diff --git a/deps/dep-0003-record-withdrawal.md b/deps/dep-0003-record-withdrawal.md index f90479e..a667a40 100644 --- a/deps/dep-0003-record-withdrawal.md +++ b/deps/dep-0003-record-withdrawal.md @@ -143,6 +143,19 @@ Both collections use `listType=map` keyed by entry ID. Limits are 64 target-reti 2. Source writes never prune. The endpoint controller resolves the Zone/provider for each retired pair and creates external Core claims by deterministic create-or-get, checking complete immutable shape/provenance on collision. 3. Only after every associated claim is durably created/read back does endpoint prune exact captured entry IDs using an optimistic-lock patch. Current-generation status confirms captured entries. A create failure leaves retirement intent; a crash between create and prune is harmless. Source retries merge current state, never restore an old whole spec over concurrent entries. 4. Non-external pairs finish existing child cleanup and prune without an external DNS wait. Generated RecordSets/ERSs can finish ordinary deletion once independent claims exist; they do not wait for customer DNS changes. +5. A never-published endpoint has no managed withdrawal obligation. The endpoint + controller may acknowledge it without creating a claim only after explicit + `ZoneNotResolved`, no retained hostname Zone binding or generated-record + reference/count, no sticky `WithdrawalProtected=True`, and authoritative reads + proving no suffix-matching Zone and no source-label-bound generated RecordSet. + These reads bypass informer caches so a successful child Create followed by a + failed status write cannot look unpublished. Existing/deleting children, + missing prior authority, an unusable matching Zone, incomplete lists, or read + failures retain intent and finalizers. This is not proof of arbitrary DNS + absence, and does not weaken withdrawal for any previously published endpoint. + Normal pruning lets active hostname diagnostics reach the current generation; + normal retire-only capture lets source cleanup finish. No environment-specific + exception or synthetic Provider is introduced. Missing/deleting Gateway, rejected admission, and every cleanup path, **including `cleanupForRoute`**, move remaining pairs to retired state rather than immediately delete the ERS. Source deletion waits for current-generation capture confirmation. diff --git a/docs/design/endpoint-api.md b/docs/design/endpoint-api.md index aa868d4..428c085 100644 --- a/docs/design/endpoint-api.md +++ b/docs/design/endpoint-api.md @@ -99,6 +99,21 @@ Sources install `endpoint.dns.appthrust.io/generated-recordsets` at ERS creation Before pruning retirement entries or deleting generated children, the endpoint controller resolves each retired hostname/IP pair to its retained publication Zone binding, or the ordinary single-pair publication resolution/conversion/fallback path. For a Provider version supporting withdrawal it durably creates a deterministic independent `RecordWithdrawal`; successful Create is persisted evidence, and AlreadyExists requires Get plus exact immutable shape, lifecycle, address label, and provenance checks. Duplicate pairs at the same retirement generation share a claim. Missing resolution, collisions, write failures, and collection overflow hold capture rather than discard intent. A known non-supporting Provider requires no withdrawal claim. +The sole no-authority exception is a provably never-published endpoint. After an +explicit `Resolved=False/ZoneNotResolved` result, capture is vacuous only when +the ERS has no retained hostname Zone binding or generated-record reference/count, +no sticky `WithdrawalProtected=True`, an uncached Zone read finds no +suffix-matching authority, and an uncached label-bound RecordSet read proves no +generated child exists. An unusable matching Zone, conversion/capability rejection, +any existing or deleting child, incomplete list, missing reader, or read error +holds capture. The authoritative read closes the child-create/status-write crash +and cache-lag window; a missing previously bound Zone never authorizes cleanup. +This exception creates no withdrawal claim and makes no assertion about arbitrary +customer DNS. It uses the ordinary optimistic-lock prune/finalizer paths. Active +intent then resumes current-generation reconciliation and still reports +`ZoneNotResolved`, not fabricated DNS readiness. There is no Kind, provider-name, +hostname-suffix, or environment-specific bypass. + Active ERSs clear captured retirement collections with an optimistic-lock patch; a concurrent append conflicts instead of being lost. Retire-only ERSs retain entries until DELETE and report current-generation `WithdrawalCaptured=True/Captured`, message `Every retired hostname/address pair has been durably captured.` Their `Resolved`, `Accepted`, `Programmed`, and `Ready` conditions are `False/Retiring`, message `EndpointRecordSet is retiring; no active records are published.` They issue no empty conversion request or RecordSet. Capture failure reports `WithdrawalCapturePending` and `Ready=False`; source overflow leaves the old spec unchanged and is never truncated. `WithdrawalProtected` defaults closed: only current-generation `False` after all hostname providers resolve without withdrawal support permits the old unprotected cleanup path. Once True, protection is sticky for that ERS incarnation. `WithdrawalCaptured=True` is accepted only at the current ERS generation. Capture acknowledgment proves independent obligation persistence, not DNS withdrawal. diff --git a/internal/go/apps/endpoint/controllers/endpointrecordset/controller.go b/internal/go/apps/endpoint/controllers/endpointrecordset/controller.go index f4f88a0..77a5058 100644 --- a/internal/go/apps/endpoint/controllers/endpointrecordset/controller.go +++ b/internal/go/apps/endpoint/controllers/endpointrecordset/controller.go @@ -46,6 +46,8 @@ const ( type Reconciler struct { client.Client + // APIReader is the authoritative absence boundary for never-published withdrawals. + APIReader client.Reader Scheme *runtime.Scheme RESTConfig *rest.Config RecordSetNamespace string @@ -119,6 +121,9 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { if r.Scheme == nil { r.Scheme = mgr.GetScheme() } + if r.APIReader == nil { + r.APIReader = mgr.GetAPIReader() + } return ctrl.NewControllerManagedBy(mgr). Named("endpoint-recordset"). For(&endpointv1alpha1.EndpointRecordSet{}). diff --git a/internal/go/apps/endpoint/controllers/endpointrecordset/withdrawal.go b/internal/go/apps/endpoint/controllers/endpointrecordset/withdrawal.go index e0eb2e8..479c36d 100644 --- a/internal/go/apps/endpoint/controllers/endpointrecordset/withdrawal.go +++ b/internal/go/apps/endpoint/controllers/endpointrecordset/withdrawal.go @@ -72,11 +72,13 @@ func (r *Reconciler) captureWithdrawals(ctx context.Context, ers *endpointv1alph if err != nil { return err } - supported, err := r.withdrawalSupported(ctx, zone.Spec.Provider) - if err != nil { - return err + selected.zone = zone + if zone != nil { + selected.protected, err = r.withdrawalSupported(ctx, zone.Spec.Provider) + if err != nil { + return err + } } - selected = binding{zone: zone, protected: supported} bindings[key] = selected } if !selected.protected { @@ -127,6 +129,8 @@ func (r *Reconciler) captureWithdrawals(ctx context.Context, ers *endpointv1alph // withdrawalZone prefers the retained publication binding. Without one, run the // same publication resolver with a nonempty single-pair snapshot, including // conversion rejection and Zone fallback. This never applies its RecordSets. +// A nil Zone with no error means authoritative evidence proves this endpoint +// was never published and therefore has no withdrawal obligation to capture. func (r *Reconciler) withdrawalZone(ctx context.Context, ers *endpointv1alpha1.EndpointRecordSet, namespace, hostname string, address netip.Addr) (*dnsv1alpha1.Zone, error) { for _, published := range ers.Status.Hostnames { if retirement.CanonicalHostname(published.Hostname) != hostname || published.Zone == nil { @@ -148,6 +152,16 @@ func (r *Reconciler) withdrawalZone(ctx context.Context, ers *endpointv1alpha1.E return nil, err } if resolved.Zone == nil { + condition := meta.FindStatusCondition(resolved.Conditions, "Resolved") + if condition != nil && condition.Status == metav1.ConditionFalse && condition.Reason == "ZoneNotResolved" { + unpublished, err := r.unpublishedWithoutZone(ctx, ers, namespace, hostname) + if err != nil { + return nil, err + } + if unpublished { + return nil, nil + } + } return nil, fmt.Errorf("%w: no publication Zone binding resolved for %s", retirement.ErrCapturePending, hostname) } return &dnsv1alpha1.Zone{ diff --git a/internal/go/apps/endpoint/controllers/endpointrecordset/withdrawal_no_zone.go b/internal/go/apps/endpoint/controllers/endpointrecordset/withdrawal_no_zone.go new file mode 100644 index 0000000..dcd5465 --- /dev/null +++ b/internal/go/apps/endpoint/controllers/endpointrecordset/withdrawal_no_zone.go @@ -0,0 +1,53 @@ +package endpointrecordset + +import ( + "context" + "fmt" + "strings" + + "github.com/appthrust/dns-api/internal/go/apps/endpoint/retirement" + dnsv1alpha1 "github.com/appthrust/dns-api/pkg/go/api/dns/v1alpha1" + endpointv1alpha1 "github.com/appthrust/dns-api/pkg/go/api/endpoint/v1alpha1" + "k8s.io/apimachinery/pkg/api/meta" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// unpublishedWithoutZone proves absence of a managed publication obligation, not +// absence of arbitrary customer DNS. Missing authority after publication remains +// blocked. The caller has already received an explicit ZoneNotResolved result. +func (r *Reconciler) unpublishedWithoutZone(ctx context.Context, ers *endpointv1alpha1.EndpointRecordSet, namespace, hostname string) (bool, error) { + if meta.IsStatusConditionTrue(ers.Status.Conditions, retirement.ProtectedCondition) || ers.Status.GeneratedRecordSetCount != 0 { + return false, nil + } + for _, published := range ers.Status.Hostnames { + if published.Zone != nil || len(published.RecordSets) != 0 { + return false, nil + } + } + if r.APIReader == nil { + return false, fmt.Errorf("%w: authoritative reader is required to prove an unpublished endpoint", retirement.ErrCapturePending) + } + // ZoneNotResolved also covers conversion/capability rejection. A matching + // Zone, even an unusable one, is not evidence that no authority ever existed. + var zones dnsv1alpha1.ZoneList + if err := r.APIReader.List(ctx, &zones); err != nil { + return false, fmt.Errorf("read publication Zones: %w", err) + } + for _, zone := range zones.Items { + if hostname == zone.Spec.DomainName || strings.HasSuffix(hostname, "."+zone.Spec.DomainName) { + return false, nil + } + } + // A cached empty list can lag a successful Create followed by a failed + // status write. This live, label-bound read closes that crash/cache window. + // Even a deleting child or foreign manager on the same source holds capture. + var records dnsv1alpha1.RecordSetList + if err := r.APIReader.List(ctx, &records, client.InNamespace(namespace), client.MatchingLabels{ + generatedLabelEndpointRecordSetNamespace: ers.Namespace, + generatedLabelEndpointRecordSetName: ers.Name, + }, client.Limit(1)); err != nil { + return false, fmt.Errorf("read generated RecordSets for unpublished endpoint: %w", err) + } + return len(records.Items) == 0 && records.Continue == "" && + (records.RemainingItemCount == nil || *records.RemainingItemCount == 0), nil +} diff --git a/internal/go/apps/endpoint/controllers/endpointrecordset/withdrawal_no_zone_test.go b/internal/go/apps/endpoint/controllers/endpointrecordset/withdrawal_no_zone_test.go new file mode 100644 index 0000000..444dc67 --- /dev/null +++ b/internal/go/apps/endpoint/controllers/endpointrecordset/withdrawal_no_zone_test.go @@ -0,0 +1,222 @@ +package endpointrecordset + +import ( + "context" + "errors" + "reflect" + "slices" + "testing" + + "github.com/appthrust/dns-api/internal/go/apps/endpoint/retirement" + dnsv1alpha1 "github.com/appthrust/dns-api/pkg/go/api/dns/v1alpha1" + endpointv1alpha1 "github.com/appthrust/dns-api/pkg/go/api/endpoint/v1alpha1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" +) + +func TestWithdrawalNoZoneRefreshesNeverPublishedEndpoint(t *testing.T) { + ctx := context.Background() + r, ers, zone := withdrawalFixture(t, true, interceptor.Funcs{}) + r.APIReader = r.Client + if err := r.Delete(ctx, zone); err != nil { + t.Fatal(err) + } + ers = getWithdrawalERS(t, r.Client, client.ObjectKeyFromObject(ers)) + // Identity lane 41305702992 retained generation 12 with observedGeneration 1: + // no Zone/RecordSet ever existed, but route churn created retirement entries. + ers.Status = endpointv1alpha1.EndpointRecordSetStatus{ + ObservedGeneration: 1, + Conditions: []metav1.Condition{{Type: retirement.ProtectedCondition, Status: metav1.ConditionUnknown, ObservedGeneration: 1}}, + Hostnames: []endpointv1alpha1.EndpointRecordSetHostnameStatus{{ + Hostname: "h2.example.com", + Conditions: []metav1.Condition{{Type: "Resolved", Status: metav1.ConditionFalse, Reason: "ZoneNotResolved"}}, + }}, + } + if err := r.Status().Update(ctx, ers); err != nil { + t.Fatal(err) + } + if _, err := r.Reconcile(ctx, ctrlRequest(ers)); err != nil { + t.Fatalf("never-published retirement must complete: %v", err) + } + current := getWithdrawalERS(t, r.Client, client.ObjectKeyFromObject(ers)) + if hasRetirements(current) || len(listWithdrawalClaims(t, r.Client)) != 0 { + t.Fatal("never-published retirement retained intent or manufactured a withdrawal claim") + } + if _, err := r.Reconcile(ctx, ctrlRequest(ers)); err != nil { + t.Fatal(err) + } + current = getWithdrawalERS(t, r.Client, client.ObjectKeyFromObject(ers)) + if current.Status.ObservedGeneration != current.Generation || len(current.Status.Hostnames) != 1 { + t.Fatalf("active hostname observation did not converge: %#v", current.Status) + } + resolved := meta.FindStatusCondition(current.Status.Hostnames[0].Conditions, "Resolved") + if resolved == nil || resolved.Status != metav1.ConditionFalse || resolved.Reason != "ZoneNotResolved" || meta.IsStatusConditionTrue(current.Status.Conditions, "Ready") { + t.Fatalf("missing Zone must remain explicit, not fabricate DNS readiness: %#v", current.Status) + } +} + +func TestWithdrawalNoZoneCompletesUnpublishedDeletion(t *testing.T) { + for _, mode := range []string{"retire-only", "direct-delete"} { + t.Run(mode, func(t *testing.T) { + ctx := context.Background() + r, ers, zone := withdrawalFixture(t, true, interceptor.Funcs{}) + r.APIReader = r.Client + if err := r.Delete(ctx, zone); err != nil { + t.Fatal(err) + } + // An unrelated endpoint must not turn the exact source-label absence + // query into a namespace-wide publication prohibition. + unrelated := &dnsv1alpha1.RecordSet{ObjectMeta: metav1.ObjectMeta{ + Namespace: ers.Namespace, Name: "other-endpoint", + Labels: map[string]string{ + generatedLabelEndpointRecordSetNamespace: ers.Namespace, + generatedLabelEndpointRecordSetName: "other", + }, + }} + if err := r.Create(ctx, unrelated); err != nil { + t.Fatal(err) + } + if mode == "retire-only" { + ers = getWithdrawalERS(t, r.Client, client.ObjectKeyFromObject(ers)) + ers.Spec.Hostnames, ers.Spec.Targets = nil, nil + if err := r.Update(ctx, ers); err != nil { + t.Fatal(err) + } + if _, err := r.Reconcile(ctx, ctrlRequest(ers)); err != nil { + t.Fatal(err) + } + ers = getWithdrawalERS(t, r.Client, client.ObjectKeyFromObject(ers)) + if !retirement.Captured(ers) || !hasRetirements(ers) { + t.Fatal("retire-only capture must acknowledge this generation without pruning its final intent") + } + if err := retirement.RetireOrDelete(ctx, r.Client, ers); err != nil { + t.Fatal(err) + } + } else if err := r.Delete(ctx, ers); err != nil { + t.Fatal(err) + } + if _, err := r.Reconcile(ctx, ctrlRequest(ers)); err != nil { + t.Fatal(err) + } + if err := r.Get(ctx, client.ObjectKeyFromObject(ers), &endpointv1alpha1.EndpointRecordSet{}); !apierrors.IsNotFound(err) { + t.Fatalf("never-published ERS finalizer was not released: %v", err) + } + if len(listWithdrawalClaims(t, r.Client)) != 0 { + t.Fatal("never-published deletion manufactured a withdrawal claim") + } + if err := r.Get(ctx, client.ObjectKeyFromObject(unrelated), &dnsv1alpha1.RecordSet{}); err != nil { + t.Fatalf("unrelated endpoint record was affected: %v", err) + } + }) + } +} + +func TestWithdrawalNoZoneRetainsPublicationEvidenceAndReadFailures(t *testing.T) { + for _, scenario := range []string{ + "sticky-protection", "lost-bound-zone", "other-host-binding", "retained-record-reference", + "generated-record", "cached-empty-generated-record", "record-list-error", "zone-list-error", + "unresolved-capability", "missing-reader", "partial-record-list", + } { + for _, mode := range []string{"active", "deleting"} { + t.Run(scenario+"/"+mode, func(t *testing.T) { + deleting := mode == "deleting" + ctx := context.Background() + r, ers, zone := withdrawalFixture(t, true, interceptor.Funcs{}) + base := r.Client.(client.WithWatch) + r.APIReader = base + if scenario != "unresolved-capability" { + if err := r.Delete(ctx, zone); err != nil { + t.Fatal(err) + } + } else { + r.conversionFunc = func(context.Context, *endpointv1alpha1.EndpointProviderCapability, endpointv1alpha1.EndpointRecordSetConversionInput) ([]endpointv1alpha1.RecordSetSpecFragment, string, error) { + return nil, "conversion refused", nil + } + } + ers = getWithdrawalERS(t, base, client.ObjectKeyFromObject(ers)) + switch scenario { + case "sticky-protection": + ers.Status.Conditions = []metav1.Condition{{Type: retirement.ProtectedCondition, Status: metav1.ConditionTrue, ObservedGeneration: 1}} + case "lost-bound-zone", "other-host-binding": + host := "h1.example.com" + if scenario == "other-host-binding" { + host = "unrelated.example.com" + } + ers.Status.Hostnames = []endpointv1alpha1.EndpointRecordSetHostnameStatus{{ + Hostname: host, Zone: &endpointv1alpha1.EndpointRecordSetZoneStatus{ + Ref: dnsv1alpha1.ObjectReference{Namespace: zone.Namespace, Name: zone.Name}, + DomainName: zone.Spec.DomainName, Provider: zone.Spec.Provider, + }, + }} + case "retained-record-reference": + ers.Status.Hostnames = []endpointv1alpha1.EndpointRecordSetHostnameStatus{{ + Hostname: "h1.example.com", RecordSets: []endpointv1alpha1.EndpointRecordSetGeneratedRecordSetStatus{{ + Ref: dnsv1alpha1.ObjectReference{Namespace: ers.Namespace, Name: "published"}, + }}, + }} + case "generated-record", "cached-empty-generated-record": + record := &dnsv1alpha1.RecordSet{ObjectMeta: metav1.ObjectMeta{ + Namespace: ers.Namespace, Name: "published", + Labels: map[string]string{ + generatedLabelEndpointRecordSetNamespace: ers.Namespace, + generatedLabelEndpointRecordSetName: ers.Name, + }, + }} + if err := base.Create(ctx, record); err != nil { + t.Fatal(err) + } + if scenario == "cached-empty-generated-record" { + r.Client = interceptor.NewClient(base, interceptor.Funcs{List: func(ctx context.Context, c client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + if records, ok := list.(*dnsv1alpha1.RecordSetList); ok { + records.Items = nil + return nil + } + return c.List(ctx, list, opts...) + }}) + } + case "record-list-error", "zone-list-error", "partial-record-list": + r.APIReader = interceptor.NewClient(base, interceptor.Funcs{List: func(ctx context.Context, c client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + switch records := list.(type) { + case *dnsv1alpha1.RecordSetList: + if scenario == "record-list-error" { + return errors.New("generated record observation unavailable") + } + if scenario == "partial-record-list" { + records.Continue = "unread-page" + return nil + } + case *dnsv1alpha1.ZoneList: + if scenario == "zone-list-error" { + return errors.New("authoritative Zone observation unavailable") + } + } + return c.List(ctx, list, opts...) + }}) + case "missing-reader": + r.APIReader = nil + } + if err := base.Status().Update(ctx, ers); err != nil { + t.Fatal(err) + } + if deleting { + if err := base.Delete(ctx, ers); err != nil { + t.Fatal(err) + } + } + if _, err := r.Reconcile(ctx, ctrlRequest(ers)); err == nil { + t.Fatal("publication evidence or an unavailable observation authorized capture") + } + current := getWithdrawalERS(t, base, client.ObjectKeyFromObject(ers)) + ready := meta.FindStatusCondition(current.Status.Conditions, "Ready") + if !reflect.DeepEqual(current.Spec, ers.Spec) || !slices.Contains(current.Finalizers, generatedRecordSetsFinalizer) || + ready == nil || ready.Status != metav1.ConditionFalse || ready.Reason != retirement.PendingCondition || + len(listWithdrawalClaims(t, base)) != 0 { + t.Fatalf("capture failure discarded intent/finalizer or manufactured success: %#v", current) + } + }) + } + } +}