Repository navigation
Conversation
3ebfa8a to
2770683
Compare
c96fd61 to
a32a731
Compare
1a095af to
61920e4
Compare
61920e4 to
7fdf439
Compare
7fdf439 to
38041c1
Compare
38041c1 to
cd43bb8
Compare
ecv
left a comment
There was a problem hiding this comment.
Approving at bb43d81. An object with resolved status but no reference ConfigMap is now marked Unverified and stays detached, so it cannot re-attach to a replacement. Ack status patches use an optimistic lock, so concurrent ack workers cannot overwrite a newer observation. Tests fail when either fix is undone. Malformed reference ConfigMaps are skipped, and a scope with no project UID now errors.
One rollout cost for the body: Unverified is never cleared. Every object staging resolved since infra#7473 stays not-current until it is recreated. Say so, so staging's rollout plans the recreate.
ecv
left a comment
There was a problem hiding this comment.
Re-approving at 63d53b7. The new commits carry the #236 PROXY change and make prepareDNSDistRouter rewrite the process settings on startup while keeping the accepted tenant rules. A retained file without a complete rules section is an error, not a silent rewrite. My note on recreating staging's Unverified objects still applies.
ecv
left a comment
There was a problem hiding this comment.
Re-approving at 6f66e14. restoreActive now refreshes the dnsdist process header on retained files before comparing, so an upgrade keeps the accepted tenant rules without a forced reload. Ack projection retries on conflict against a direct client, and an older ack becomes a no-op rather than an empty patch. My note on recreating staging's Unverified objects still applies.
Bring in the dns-operator-served /validate-internal-dns webhook, the reference immutability rules, and the API bounds. Keep this branch's name-only references: zone and resolver context references on associations, registrations, and naming policies are immutable by name, and no rule compares a UID. The removed Managed field is no longer set on managed zone associations, and the webhook and API tests now use name references. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…name-references Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> # Conflicts: # config/components/admission-webhooks/README.md
…name-references Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ecv
left a comment
There was a problem hiding this comment.
Approve at 798ca73. My earlier approvals were dismissed when #243 was merged in again. The new #243 webhook merge (7f454ab) keeps references name-only. Zone and context names on associations, registrations and naming policies are immutable through CEL, and no rule compares a UID. go build, go vet and the touched packages' tests pass locally with envtest assets. make manifests generate leaves the tree clean.
Prior findings at this head
| Finding | Status | Evidence |
|---|---|---|
| Existing objects re-attach by name | Fixed | reference_state.go:69, :87: a source with resolved status but no reference state is held Unverified and gets no new pin. Covered by TestMissingReferenceStateDoesNotReattachObservedResource |
4 concurrent consumeAcks workers |
Fixed | acks.go:50: retry on conflict, optimistic-lock status patches, and an older ack is now a no-op (acks.go:140, :189). Covered by the concurrent health projection test |
One bad references-* ConfigMap fails the reconcile |
Fixed | reference_state.go:143: malformed, unknown-kind and identity-mismatch objects are logged and skipped |
Scopes without ProjectUID skipped silently |
Fixed | acks.go:209 now errors. ProjectNamespaces is gone, and discovery sets ProjectUID (project_discovery.go:52) |
| Registration labels as an authorization boundary | Documented | README "Reference-state recovery" and the PR body name registration RBAC as the boundary. Selectors and references stay in one namespace (DNSNameReference has no namespace), and reference state lives in the platform namespace, so a tenant cannot select across projects or forge pins |
DNSRecordSet watch not mentioned in the body |
Fixed | The body now says "Reconcile static record changes" |
Blocking
None.
Non-blocking
- Staging rollout cost still stands. Objects staging resolved before this change have no reference state, so they stay
Unverifieduntil someone recreates them. The README says "Restore a verified reference-state backup" (config/internal-dns/README.md:260), but no backup exists for pins that only lived in the olduidfields. For those objects, recreating them is the only way back. Say so for staging. ResolveNaming(naming.go:46) builds aReconcilerwithoutPlatformNamespace, so it always reads reference state frominternal-dns-system. Only tests call it today. If it gets wired to a deployment with another platform namespace, every policy will be skipped without an error. Pass the namespace in.- Reference-state ConfigMaps are cleaned up only by a reconcile of the same project (
collectReferenceStates). If a whole project is removed, itsreferences-*objects stay in the platform namespace. Consider adding them to the project cleanup path. - The metrics listener (
run.go:193) serves/metricswithout authentication. The README says to restrict it to the monitoring network, which is fine for now.
| An observed association, naming policy, or registration requires its private | ||
| reference state to retain the target object's identity. If that state is | ||
| missing, DNS rejects the source instead of attaching it to a same-name target. | ||
| The rejection persists across retries. Restore a verified reference-state |
There was a problem hiding this comment.
Objects that predate reference state (staging since infra#7473) have no backup to restore. For them, recreating the object is the only recovery. Worth saying that here.
| return nil, err | ||
| } | ||
| out := []ResolvedName{{DNSZoneRef: dnsv1alpha1.DNSObjectReference{Name: canonical.Name, UID: canonical.UID}, Name: label, Canonical: true}} | ||
| r := &Reconciler{Client: c, Options: Options{ProjectUID: projectUID}} |
There was a problem hiding this comment.
This always reads reference state from the default internal-dns-system namespace. With any other PlatformNamespace, every policy is skipped without an error. Only tests call it today, so this is non-blocking. Consider passing the namespace in.
The merge-base changed after approval.
Bring in the merged private DNS stack and keep this branch's name-only references. - Project API: keep DNSNameReference for naming policy zone references and adopt main's 253-character limits on scopePrefix and namePrefix. - Admission: keep the name-based resolver context check; main's UID pinning does not apply to name references. Keep main's note that associations do not require a Ready context. - Naming: keep the lookup by context name and adopt main's stricter readiness, requiring a Ready condition for the current generation and a managed zone. - Tests from main now use name references, and the resolver context test checks a missing context instead of a replaced UID. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Summary
Configure private zones, registrations, and naming policies using resource names and registration labels. Keep reference identities in private service state, and reject observed resources whose identity state is missing instead of attaching them to replacement targets. Reconcile static record changes and process fleet acknowledgements concurrently while preserving newer health observations. Expose optional operation metrics and preserve readiness timestamps when serving health is unchanged. Preserve accepted routing across restarts and support consumer UDP and TCP queries over trusted backend TCP connections.
User experience
Zone associations and naming policies select a resolver context by name; registrations select a zone by name. Naming rules match registration labels, and an empty selector matches all registrations in the context. Registration write access is the RBAC boundary: selectors organize names and do not grant separate publisher authorization. Recover missing identity state from a verified backup or recreate the source resource to select its targets explicitly.
Important
A resource whose identity state is missing is marked
Unverified, and that mark is never cleared. Resources resolved before this change have no identity state to restore, because their target identities lived only in the removeduidfields. This includes every association, naming policy, and registration that staging resolved since infra#7473. They stay detached until they are recreated, so the staging rollout must plan to recreate them.Test plan
Stacked on #243. Architecture: #229.