Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 145 additions & 0 deletions .claude/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,148 @@
## [2026-07-08] - Regression run + verify-crd-sync skill

**Author:** Erick Bourgeois

### Changed
- `deploy/operator/crds/bind9instances.crd.yaml`: regenerated — the merge of the
bug-fix batches left this CRD stale (missing `status.observedParentGeneration`);
a fresh `cargo run --bin crdgen` corrected it. All other CRDs were in sync.
- `.claude/skills/verify-crd-sync/SKILL.md`: new invocable skill (the reference in
`.claude/SKILL.md` was prose-only, so the Skill tool could not run it). Folds in
the offline drift check (`crdgen` + `git diff`) that caught the stale CRD above,
and corrects the CRD path (`deploy/operator/crds/`, not `deploy/crds/`).

### Regression results (compiled suite, `cargo test --all`)
- 1253 passed, 0 failed, 89 ignored (the 89 are cluster-required integration tests).
- `cargo fmt` clean, `cargo clippy --all-targets -D warnings` clean.
- CRD sync verified (see above). All 19 `examples/*.yaml` are well-formed.
- NOT run (require a rebuilt operator image + deploy, which is the user's step):
`make test-integration`, `test-integ-multi-tenancy`, `test-integ-cluster-provider`,
and the kind admission-policy phases.

### Impact
- [ ] Breaking change
- [ ] Requires cluster rollout
- [ ] Config change only
- [x] Tooling / CRD regeneration only

---

## [2026-07-07] - Bug-fix sweep: 24 bugs from codebase-wide audit (branch major-bug-fixes)

**Author:** Erick Bourgeois

Codebase-wide correctness audit (4 parallel reviewers + hand verification)
followed by six fix batches. All fixes TDD-first with ~130 new unit tests;
`cargo fmt`/`clippy -D warnings`/`cargo test` green throughout.

### Changed — DNS record operations (`src/bind9/records/`)
- TXT/MX/NS/SRV/CAA add paths now **replace** the RRset (delete-then-append via
shared `build_delete_rrset_record`), matching A/AAAA. Previously they only
appended, so spec changes served old + new rdata forever (broken SPF, stale MX).
- `delete_dns_record` now fails on REFUSED/NOTAUTH/NOTZONE/SERVFAIL; only
NoError/NXDomain/NXRRSet are idempotent success (`is_idempotent_delete_response_code`).
Previously TSIG/ACL failures returned Ok and orphaned records past finalizers.
- TTL-only spec changes now trigger updates (all 8 types compare TTL via
`effective_record_ttl`/`rrset_ttl_matches`).
- IPv6 RRset comparison parses both sides to `Ipv6Addr` (non-canonical spellings
like `2001:DB8::1` no longer cause endless delete/recreate churn).

### Changed — records reconciler (`src/reconcilers/records/`, `dnszone/discovery.rs`)
- `update_record_reconciled_timestamp` patched nonexistent `status.selectedRecords`
(schema-pruned no-op); now patches `records`. `lastReconciledAt` persists for
the first time.
- Records unselected from a zone are now deleted from BIND9 before untagging
(`cleanup_unselected_record_dns`; untag deferred on DNS failure for retry).
- **New CRD status field `publishedName`** on all 8 record types: tracks the
published FQDN so renames delete the old name; finalizer prefers it over spec.name.
- 7 duplicated per-type reconcilers collapsed into generic `reconcile_record`
(net −970 lines).
- `status.addresses` only published on successful, selected reconciles.
- Transient record-list failures no longer wipe `DNSZone.status.records`.

### Changed — bindcar HTTP/auth layer (`src/bind9/`)
- SA token no longer cached for process lifetime: `RwLock`-guarded cache with
freshness TTL re-reads the kubelet-rotated projected token (was: permanent 401s
~1h after operator start under bindcar 0.7 TokenReview).
- HTTP client now has connect (5s) / request (30s) timeouts; retry/backoff can
actually engage on hung connections. `create_zone_http` routed through the
shared retry path.
- `zone_exists` 404 detection fixed via `is_http_not_found()` downcast through
the anyhow context chain (`Ok(false)` was unreachable).
- `parse_rndc_key_file` skips comment lines (`#`, `//`, `/* */`).

### Changed — resource builders (`src/bind9_resources.rs`, `bind9instance/resources.rs`, `templates/`)
- Custom `configMapRefs` no longer produce broken Deployments: generated
ConfigMap always created with non-overridden files (rndc.conf always), `config`
volume always present, custom refs override per-file via subPath. Previously:
both refs → 422; one ref → FailedMount forever.
- `rndcKey.secretRef`/inline secret names now threaded into the Deployment
(env secretKeyRefs + volume). Previously hardcoded `{name}-rndc-key` (TODO).
- DNSSEC policy no longer emits invalid `nsec;` (NSEC = omitted `nsec3param` in
BIND 9.18 grammar; was CrashLoopBackOff on signing without nsec3).
`generate_dnssec_policies` fallback now matches `get_dnssec_signing_config`.
- Malformed RNDC Secrets are genuinely recreated after deletion
(`evaluate_existing_rndc_secret` pure decision fn; also removed a production unwrap).
- `Bind9Config.forwarders`/`listenOn`/`listenOnV6` are now rendered into
named.conf.options (instance + cluster paths; entries validated as IPs).
Previously silently ignored — forwarder-only setups did full recursion.

### Changed — reconcilers (`src/reconcilers/`)
- `observedGeneration` included in status-change checks at all 3 affected sites
(ClusterBind9Provider, Bind9Cluster, Bind9Instance) — statuses no longer lag
metadata.generation, ending permanent re-reconcile loops (bug-039 family).
- **New CRD status field `Bind9InstanceStatus.observedParentGeneration`**:
parent-config change detection no longer compares the parent's generation to
the instance's own observedGeneration (unrelated counters).
- ClusterBind9Provider drift detection now checks the same namespace set that
reconciliation creates (shared `expected_cluster_namespaces()`), fixing both
perpetual false drift and never-detected real drift.
- DNSZone Ready now compares instances-configured vs instances-expected
(`ZoneConfigOutcome`); an instance counts only if ALL its ready endpoints
accepted the zone. Partial pod failures no longer masked by endpoint counts.
- Zone conditions converge on every path (`set_final_zone_conditions`):
failure sets Ready=False (was: stale Ready=True during outages);
success resolves Progressing=False.
- DNSZone cleanup only treats kube 404 as "resource deleted"; transient API
errors propagate instead of triggering the self-healing path that deleted
live DNS records.
- Deletion cleanup skips (with loud warning) instances with no ready endpoints
and missing RNDC Secrets instead of blocking finalizers forever
(`EndpointFailurePolicy::SkipUnavailable`, deletion paths only).
- Finalizer add/remove uses guarded JSON Patches (test + add/remove, mirroring
kube-runtime) instead of whole-array merge patches that clobbered concurrent
writers. New dependency: `json-patch` 4.2 (already in tree via kube-core;
no new compiled crates).
- Pending pods without IPs are skipped during pod discovery instead of aborting
the listing.

### Changed — bootstrap (`src/bootstrap.rs`)
- `bindy bootstrap operator` can now authenticate to bindcar 0.7: projected
`audience: bindcar` token volume (3600s), `POD_NAMESPACE` downward-API env,
and TokenReview ClusterRole/Binding applied with the binding subject namespace
following `--namespace` (was hardcoded-implicitly to bindy-system).

### Why
See `.wolf/buglog.json` bug-039..044 and the 2026-07-07 audit. Highest-impact:
records serving stale rdata indefinitely, live DNS data deleted on transient
control-plane errors, operator auth expiring after 1h, and Ready=True masking
outages.

### Impact
- [ ] Breaking change
- [x] Requires cluster rollout (operator image + CRD update)
- [ ] Config change only
- [ ] Documentation only

CRD changes are additive only (`publishedName`, `observedParentGeneration`);
apply CRDs with `kubectl replace --force` per the standard procedure. Behavior
changes operators should know: record deletion failures now surface as errors
(previously silent), zone Ready is stricter (partial failures now show
Ready=False), and `forwarders`/`listenOn` fields now take effect — clusters
relying on them being ignored will see config changes on next reconcile.

---

## [2026-07-07] - Docs & examples: bindcar Mode B audience details

**Author:** Erick Bourgeois
Expand Down
85 changes: 85 additions & 0 deletions .claude/skills/verify-crd-sync/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
---
name: verify-crd-sync
description: Verify that the generated CRD YAMLs in deploy/operator/crds/ match the Rust source of truth in src/crd.rs. Use BEFORE investigating any reconciliation loop, infinite requeue, "field not appearing in kubectl output", or status patch that returns HTTP 200 but doesn't persist; and AFTER any edit to structs in src/crd.rs. Catches schema drift that causes silent field pruning.
---

# verify-crd-sync

CRD YAMLs in `deploy/operator/crds/` are **auto-generated** from `src/crd.rs` by
the `crdgen` binary. When they drift, the API server prunes struct fields that
exist in Rust but not in the deployed schema: patches succeed (HTTP 200) but the
data never persists, causing silent reconciliation loops and missing `kubectl`
output.

`src/crd.rs` is the single source of truth. Never hand-edit the YAMLs.

## When to use

- **After** any change to a struct in `src/crd.rs` (new field, rename, serde attr).
- **Before** investigating: reconciliation/requeue loops, a field not appearing in
`kubectl get -o yaml`, a status patch that returns 200 but doesn't stick, or any
"the controller ignores my change" report.
- As part of a regression run after merging branches that touched `src/crd.rs`
(large CRDs like `bind9instances.crd.yaml` exceed patch/merge tooling limits and
are the ones most likely to be left stale).

## Steps

### 1. Detect drift offline (no cluster required — preferred)

Regenerate from Rust and diff against what's committed on disk:

```bash
cargo run --bin crdgen
git diff --stat -- deploy/operator/crds/
```

- **No files listed** → CRDs are in sync. Done.
- **Files listed** → those YAMLs were stale; `crdgen` has just corrected them.
Inspect the change to confirm it's the expected field, then keep it:

```bash
git diff -- deploy/operator/crds/<name>.crd.yaml
```

The regenerated files ARE the fix — leave them in the working tree so they get
committed alongside the `src/crd.rs` change.

### 2. Confirm a specific field round-trips (optional, cluster required)

If a field still misbehaves after step 1, verify the deployed cluster schema:

```bash
# Deployed schema for the field
kubectl get crd <plural>.bindy.firestoned.io -o yaml | grep -A 20 "<fieldName>:"

# Rust definition it should match
rg -A 10 "pub struct <StructName>" src/crd.rs
```

### 3. Apply the corrected CRDs to the cluster (only when asked)

`Bind9Instance` CRD exceeds the 256 KB annotation limit, so `kubectl apply`
fails — use `replace --force`:

```bash
kubectl replace --force -f deploy/operator/crds/<name>.crd.yaml
```

Image build/push and cluster mutations are the user's to run — surface the
command, don't execute it unprompted.

## Verification

- `git diff --stat -- deploy/operator/crds/` is empty immediately after
`cargo run --bin crdgen` (regenerating twice is idempotent).
- The previously-missing field appears in `kubectl get -o yaml` after the patch.
- The reconciliation loop stops (observedGeneration catches up to
metadata.generation).

## Related

- `regen-crds` — the generation half of this workflow (edit `src/crd.rs` →
`crdgen` → update `examples/` → `regen-api-docs` LAST).
- CRD API reference docs are regenerated separately:
`cargo run --bin crddoc > docs/src/reference/api.md`.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ all-features = true
rustdoc-args = ["--cfg", "docsrs"]

[dependencies]
kube = { version = "3.1", features = ["runtime", "derive", "client"] }
kube = { version = "3.1", features = ["runtime", "derive", "client", "jsonpatch"] }
kube-lease-manager = "0.11.1"
# JSON Patch construction for atomic finalizer add/remove with `test` guards
# (mirrors kube-runtime's own finalizer helper; same version kube-core uses)
json-patch = "4.2"
# Only include needed tokio features for faster compilation
tokio = { version = "1", features = ["macros", "rt-multi-thread", "sync", "time", "fs", "signal"] }
serde = { version = "1", features = ["derive"] }
Expand Down
11 changes: 11 additions & 0 deletions deploy/operator/crds/aaaarecords.crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,17 @@ spec:
format: int64
nullable: true
type: integer
publishedName:
description: |-
DNS record name (from `spec.name`) most recently published to BIND9.

Set by the record reconciler after a successful dynamic DNS update.
When `spec.name` changes (a rename), the reconciler compares it against
this field, deletes the old FQDN from the zone, publishes the new name,
and then updates this field. Without it, renamed records would leave
their old FQDN orphaned in BIND9.
nullable: true
type: string
recordHash:
description: |-
SHA-256 hash of the record's spec data.
Expand Down
11 changes: 11 additions & 0 deletions deploy/operator/crds/arecords.crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,17 @@ spec:
format: int64
nullable: true
type: integer
publishedName:
description: |-
DNS record name (from `spec.name`) most recently published to BIND9.

Set by the record reconciler after a successful dynamic DNS update.
When `spec.name` changes (a rename), the reconciler compares it against
this field, deletes the old FQDN from the zone, publishes the new name,
and then updates this field. Without it, renamed records would leave
their old FQDN orphaned in BIND9.
nullable: true
type: string
recordHash:
description: |-
SHA-256 hash of the record's spec data.
Expand Down
16 changes: 16 additions & 0 deletions deploy/operator/crds/bind9instances.crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2805,6 +2805,22 @@ spec:
format: int64
nullable: true
type: integer
observedParentGeneration:
description: |-
Generation of the referenced parent cluster that was last reconciled.

Records the `metadata.generation` of the parent `Bind9Cluster` or
`ClusterBind9Provider` (whichever this instance references) as observed
during the last successful reconciliation. The instance reconciler
compares the parent's current generation against this value to detect
parent configuration changes (e.g., RNDC config added at the cluster
level) that must be propagated to the instance.

This is intentionally separate from `observed_generation`, which tracks
the instance's OWN spec generation - the two counters are unrelated.
format: int64
nullable: true
type: integer
rndcKeyRotation:
description: |-
RNDC key rotation status and tracking information.
Expand Down
11 changes: 11 additions & 0 deletions deploy/operator/crds/caarecords.crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,17 @@ spec:
format: int64
nullable: true
type: integer
publishedName:
description: |-
DNS record name (from `spec.name`) most recently published to BIND9.

Set by the record reconciler after a successful dynamic DNS update.
When `spec.name` changes (a rename), the reconciler compares it against
this field, deletes the old FQDN from the zone, publishes the new name,
and then updates this field. Without it, renamed records would leave
their old FQDN orphaned in BIND9.
nullable: true
type: string
recordHash:
description: |-
SHA-256 hash of the record's spec data.
Expand Down
11 changes: 11 additions & 0 deletions deploy/operator/crds/cnamerecords.crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,17 @@ spec:
format: int64
nullable: true
type: integer
publishedName:
description: |-
DNS record name (from `spec.name`) most recently published to BIND9.

Set by the record reconciler after a successful dynamic DNS update.
When `spec.name` changes (a rename), the reconciler compares it against
this field, deletes the old FQDN from the zone, publishes the new name,
and then updates this field. Without it, renamed records would leave
their old FQDN orphaned in BIND9.
nullable: true
type: string
recordHash:
description: |-
SHA-256 hash of the record's spec data.
Expand Down
11 changes: 11 additions & 0 deletions deploy/operator/crds/mxrecords.crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,17 @@ spec:
format: int64
nullable: true
type: integer
publishedName:
description: |-
DNS record name (from `spec.name`) most recently published to BIND9.

Set by the record reconciler after a successful dynamic DNS update.
When `spec.name` changes (a rename), the reconciler compares it against
this field, deletes the old FQDN from the zone, publishes the new name,
and then updates this field. Without it, renamed records would leave
their old FQDN orphaned in BIND9.
nullable: true
type: string
recordHash:
description: |-
SHA-256 hash of the record's spec data.
Expand Down
11 changes: 11 additions & 0 deletions deploy/operator/crds/nsrecords.crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,17 @@ spec:
format: int64
nullable: true
type: integer
publishedName:
description: |-
DNS record name (from `spec.name`) most recently published to BIND9.

Set by the record reconciler after a successful dynamic DNS update.
When `spec.name` changes (a rename), the reconciler compares it against
this field, deletes the old FQDN from the zone, publishes the new name,
and then updates this field. Without it, renamed records would leave
their old FQDN orphaned in BIND9.
nullable: true
type: string
recordHash:
description: |-
SHA-256 hash of the record's spec data.
Expand Down
Loading
Loading