diff --git a/CLAUDE.md b/CLAUDE.md index 63516e1..3eb53d0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,5 +1,246 @@ -# Claude Instructions +# CLAUDE.md -## Git +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. -- Always ask for confirmation before running `git push`. \ No newline at end of file +## What this is + +A Kubernetes operator (plain controller-runtime, no CRDs) that bridges cert-manager +`Certificate` resources and Gateway API `Gateway` resources. When a `Certificate` +carries the right annotations, the operator manages HTTPS listeners on the target +Gateway: one listener per entry in `spec.dnsNames`, terminating TLS with the +Certificate's secret. + +## Commands + +```bash +go build ./... # build +go vet ./... # vet +go test ./... # all tests +go test -race -v -run TestName ./path/to/package # single test +golangci-lint run # lint +go run . # run against the current kubeconfig context +docker build -t gateway-cert-operator . +kubectl kustomize deploy/ # render the manifests +``` + +## Architecture + +| Path | Role | +|---|---| +| `main.go` | Composition root: scheme, flags, manager, health probes. No leader election. | +| `internal/controller/certificate_controller.go` | `CertificateReconciler`: watches `Certificate` and `Gateway`, runs the reconcile loop | +| `internal/gateway/patch.go` | Pure functions: `BuildListenersFromCertificates`, `MergeListeners`, `ListenersEqual` | +| `deploy/` | Kustomize manifests; namespace `gateway-cert-operator-system`, single replica | + +Annotations on `cert-manager.io/v1` `Certificate`: + +| Annotation | Required | Default | +|---|---|---| +| `gateway-cert-operator.io/gateway-name` | yes | — | +| `gateway-cert-operator.io/gateway-namespace` | no | Certificate's namespace | + +Reconcile loop (keyed by Certificate): + +1. Fetch the `Certificate`; on not-found return without doing anything. +2. Resolve the target Gateway from the annotations; no annotation, no action. +3. List all Certificates targeting that Gateway through the `IndexFieldGatewayTarget` + field index (key `"namespace/name"`). +4. `BuildListenersFromCertificates` computes the desired `auto-*` listeners. +5. `MergeListeners` keeps every non-`auto-` listener and replaces the rest. +6. Patch the Gateway only when `ListenersEqual` reports a difference. + +A reverse watch maps Gateway events back to the targeting Certificates through the +same index, so manual Gateway edits are reverted for `auto-*` listeners. + +Listeners named `auto-{certName}-{dnsName with dots as dashes}` belong to the +operator. Any other listener is never modified or removed. + +`docs/flows.md` has the full control-path reference (startup, watch wiring, decision +tree, scenarios). + +## Conventions + +### Go Conventions + +- Use the latest stable Go version (check https://go.dev/dl/ before starting a new feature). +- Logging: `logr` through controller-runtime (`log.FromContext(ctx)`), not `slog` — + a deliberate deviation from the house rule, because the manager, client and + controller all log through logr. Keys are lowerCamel. +- `context.Context` is the first parameter of every function that does I/O or can be cancelled. +- No `init()` unless there is no other option. +- Error wrapping: `fmt.Errorf("...: %w", err)` for single errors; `errors.Join` for multiple. +- Prefer stdlib over third-party dependencies where reasonable. +- No vendoring unless explicitly requested. +- Fail fast, return early. +- No obvious comments. + +### Testing + +**All new Go code must have corresponding tests.** + +- Use stdlib `testing` — no testify unless already present in the module. +- Table-driven tests are the default pattern. +- Test files live next to source: `foo.go` → `foo_test.go`. +- Name tests `Test_` (e.g. `TestMergeListeners_keepsManual`). +- Use `t.Parallel()` where safe. +- Use subtests for table entries: `t.Run(tc.name, func(t *testing.T) { ... })`. +- Test behaviour, not implementation. +- Guard slow tests with `testing.Short()`: `if testing.Short() { t.Skip() }`. +- Reconciler tests use controller-runtime's fake client (`WithIndex` for the field + index). Reach for envtest only for what the fake cannot do (API-server validation + of listeners), and guard it with `testing.Short()`. This replaces the house + `testcontainers-go` rule, which does not fit an operator. +- A bug fix lands with a regression test. + +### Plans + +When Claude Code's plan mode is used, save the plan file inside the repo at +`docs/plans/YYYY-MM-DD-HHMM-.md` instead of the default `~/.claude/plans/` +location. Get the timestamp with `date "+%Y-%m-%d-%H%M"`. The `` is a short +kebab-case summary of the plan's topic. + +Include the same timestamp in the plan's header, e.g.: + + # Plan: + **Created:** 2026-08-07 15:30 + +Plan files are committed to the repo so other contributors can review historical +decisions. + +**Ordering matters:** plan mode itself restricts writes to the default +`~/.claude/plans/` location — that's a hard tool restriction during plan mode, not a +choice. So the copy into `docs/plans/` cannot happen until *after* `ExitPlanMode`. +When it happens, it must be the very first action taken post-exit, committed on its +own, *before* any implementation work starts. Do not fold the plan-file commit into +the first implementation commit. + +### Plan execution summaries + +After finishing each step of an approved plan, append a short, human-readable summary +to `docs/plans-executions/YYYY-MM-DD-HHMM-<slug>.md` — same timestamp and slug as the +originating plan, so the two files pair up 1:1. Sections go in chronological order +(bottom of the file), not newest-first like `CHANGELOG.md`. + +- **Status checklist.** The top of the file is a markdown checklist with one line per + plan step (`- [ ] Step 0 — <title>`). Check a box in the same commit that adds that + step's entry. This checklist is the authoritative status record: a new session + should be able to read this file alone and know where to pick up. +- **Keep the prose light** — what was done, plus anything non-obvious (a judgment + call, a spec gap, something that did not work as expected). Skip steps that went + exactly as planned. +- **Include exact commands** for anything that is not a plain file edit, as fenced + blocks, each with one line on why it was run that way. Call out deviations from + the plan explicitly; close with a short "worth noting" paragraph. +- Commit the summary update together with that step's implementation commit. + +`docs/plans-executions/2026-10-08-2028-house-conventions.md` is the reference example. + +### Changelog + +Maintain a running changelog in `CHANGELOG.md` at the repo root. After every +significant change, fix, or update — once the user confirms it works — append a new +entry **at the top** of the file in this format: + + ## YYYY-MM-DD HH:MM TZ — short title + + - One-line summary of what changed and why. + - Key files touched (optional, only if useful for traceability). + +Get the timestamp with `date "+%Y-%m-%d %H:%M %Z"`. Never write a literal placeholder +like `HH:MM` — always run `date` first to get the real current time. Skip trivial edits +(typos, formatting, comment tweaks); only log changes a future reader would care about. + +### Reviews + +Before opening an MR for a feature, run the `operator-reviewer` and +`go-operator-reviewer` agents on the branch and save the findings to +`docs/reviews/YYYY-MM-DD-HHMM-<slug>.md`, most severe first, as a `- [ ]` list that is +ticked as items are processed. + +### Branching & Merge Requests + +The remote is Gitea (`gitea.home.hrajfrisbee.cz/kacerr/gatewayapi-certificates-operator`). +For **features**, do not commit to `main` directly. Use a branch + merge request flow: + +1. **Create a branch off `main`** before starting work: `feat/<slug>` for features, + `fix/<slug>` for bug-fix branches the user explicitly asks for; `<slug>` is short + kebab-case. +2. **Commit on the branch** following the commit conventions below. +3. **Push the branch** to `origin` with `-u` so it tracks. +4. **Open the MR with `tea`** rather than printing a compare URL: + + ```bash + tea pr create --title "<short title>" --description "<body>" --base main --head <branch> + ``` + + Print the resulting PR URL. If `tea` is unavailable, fall back to printing the + compare URL + (`gitea.home.hrajfrisbee.cz/kacerr/gatewayapi-certificates-operator/compare/main...<branch>`). +5. **Do not merge or delete the branch** from the CLI — neither via `tea`, `gh`, nor + `git push --delete`. The user does that in Gitea. + +**Exceptions — committing straight to `main` is fine for:** small bug fixes or +hotfixes the user describes as such; typo, comment and formatting tweaks; edits the +user explicitly says to push to `main`. When uncertain whether something is a feature +or a small fix, ask before branching. + +### Git Commits, Pushes and Tags + +- Subjects use a lowercase type prefix, as the existing history does: `feat:`, `fix:`, + `deploy:`, `ci:`, `docs:`, `test:`, `chore:`. +- Always append a `Co-Authored-By:` trailer for Claude. +- Ask for confirmation before running `git push`. The exception is `/ship`, which + commits and pushes without asking. +- Tags are semver `vMAJOR.MINOR.PATCH` and mark releases, not commits. They are + created only through `/release`, which confirms before pushing. The old `0.01`–`0.07` + tags predate this rule and stay as they are. + +### CI/CD + +Gitea Actions, workflows in `.gitea/workflows/`: + +- `build.yaml` builds the image and pushes it to + `gitea.home.hrajfrisbee.cz/kacerr/gatewayapi-certificates-operator` on any tag push + or manual dispatch. Needs the `REGISTRY_TOKEN` repo secret. +- `kubernetes-deploy.yaml` applies `deploy/` to the home-lab cluster through a + Vault → Kanidm → OIDC auth chain. + +### Containers + +- Multi-stage builds; distroless/scratch final images. +- Non-root user, read-only rootfs for production builds. +- Pin base image digests in production Dockerfiles. + +### K8s Manifests + +- Always include resource requests/limits. +- Liveness/readiness probes with sane defaults. +- PodDisruptionBudgets for replicated workloads. + +## Gotchas + +- **Three names for one project.** Directory `gateway-helper-operator`; git remote and + image `gatewayapi-certificates-operator`; binary, manifests, namespace and annotation + prefix `gateway-cert-operator`. +- **`listener-port` annotation is not implemented.** `AnnotationListenerPort` is + declared, never read; every listener is on port 443. +- **Stale listeners.** Deleting a Certificate, removing its annotation or retargeting + it does not clean up the old Gateway. A sibling Certificate's next reconcile fixes + it; with no sibling the listener stays forever. +- **A single bad listener blocks the whole Gateway.** The patch replaces the full + listeners array, so a wildcard DNS name (`*` ends up in the listener name) or a name + collision makes the API server reject every update for that Gateway. +- **`ListenersEqual` is partial**: name, hostname, port and the first secret ref only. + Changing what `BuildListenersFromCertificates` generates in other fields will not + roll out to existing listeners. +- **`auto-` prefix is the only ownership marker.** A user listener named `auto-…` gets + deleted. +- **`--metrics-bind-address` is parsed but ignored**; metrics are still served on + controller-runtime's default `:8080`. Logging is hardcoded to zap dev mode. +- **No leader election**; safe only at `replicas: 1`. +- **Deployment uses the mutable `:latest` tag**, so a new image does not roll out on + its own. +- **Dockerfile base images are tags, not digests** (contradicts the Containers rule; + not yet fixed). +- **`kubernetes-deploy.yaml` runs on every push to any branch** and prints auth + tokens to the job log. Do not copy its patterns; fixing it is on the backlog. diff --git a/docs/README.md b/README.md similarity index 86% rename from docs/README.md rename to README.md index ae9edd4..1545963 100644 --- a/docs/README.md +++ b/README.md @@ -53,12 +53,14 @@ spec: 3. **Patching**: For each DNS name defined in `spec.dnsNames`, it mathematically appends a new HTTPS `Listener` on port `443` terminating TLS using the secret created by `cert-manager` (`my-app-tls`). 4. **Safe Coexistence**: To safely coexist with manually defined listeners, the operator uniquely prefixes its configured listeners with `auto-` (e.g., `auto-my-app-cert-app-example-com`). It solely manages listeners with this prefix. -When a certificate is deleted or updated (for instance if a DNS name is changed), the operator will instantly reconcile the modified states by cleanly removing older `auto-*` listeners and inserting the new ones while preserving the manually defined resources. +When a certificate is updated (for instance if a DNS name is changed), the operator removes the older `auto-*` listeners and inserts the new ones while preserving the manually defined resources. + +Known limitation: when a certificate is deleted, its listeners are removed only on the next reconcile of another certificate targeting the same Gateway. ## Deployment To deploy the operator, applying the provided manifest sets up the necessary RBAC permissions and the controller manager: ```bash -kubectl apply -f deploy/manifests.yaml +kubectl apply -k deploy/ ``` diff --git a/docs/flows.md b/docs/flows.md new file mode 100644 index 0000000..1bbda45 --- /dev/null +++ b/docs/flows.md @@ -0,0 +1,493 @@ +# Runtime Flows + +Deep-dive reference for every control path in the gateway-cert-operator. Complements the user-facing overview in `docs/README.md`. + +--- + +## 1. Overview + +The operator sits between two Kubernetes APIs and keeps them in sync: + +``` + +--------------------+ +------------------------+ + | cert-manager | | gateway-cert-operator | + | Certificate |----(watches)-------->| (this binary) | + | (source of | | | + | truth for TLS) | +----------+-------------+ + +--------------------+ | + | patches Spec.Listeners + v + +--------------------+ +------------------------+ + | Gateway API | | Gateway | + | controller |<----(watches)--------| (target object) | + | (routes traffic) | +------------------------+ + +--------------------+ +``` + +The operator makes no changes to cert-manager objects. Its only write action is a `PATCH` on `Gateway.Spec.Listeners`. All other interactions are reads. + +--- + +## 2. Startup Flow + +Executed once at process start (`main.go`): + +``` +process start + │ + ├─ init() [main.go:22-26] + │ ├─ clientgoscheme.AddToScheme(scheme) # core/v1: Secrets, Events, … + │ ├─ cmv1.AddToScheme(scheme) # cert-manager.io/v1 Certificate + │ └─ gwapiv1.Install(scheme) # gateway.networking.k8s.io/v1 Gateway + │ + └─ main() [main.go:28] + │ + ├─ flag.StringVar --metrics-bind-address default ":8080" [main.go:32] + ├─ flag.StringVar --health-probe-bind-address default ":8081" [main.go:33] + ├─ flag.Parse() [main.go:34] + │ + ├─ ctrl.SetLogger(zap, UseDevMode=true, AddCaller) [main.go:36-41] + │ dev-mode console logger with file:line call site + │ + ├─ ctrl.GetConfigOrDie() # KUBECONFIG or in-cluster config + │ on err ──► os.Exit(1) + │ + ├─ ctrl.NewManager(cfg, Options{Scheme, HealthProbeBindAddress: ":8081"}) + │ on err ──► log + os.Exit(1) [main.go:44-47] + │ NOTE: metricsAddr is parsed above but NOT passed here; + │ the manager falls back to its own default, which + │ is also ":8080" (see §10, item 2) + │ + ├─ controller.SetupCertificateReconciler(mgr) [main.go:53] + │ registers field index + watches (see §3) + │ on err ──► log + os.Exit(1) + │ + ├─ mgr.AddHealthzCheck("healthz", healthz.Ping) → GET /healthz:8081 + ├─ mgr.AddReadyzCheck("readyz", healthz.Ping) → GET /readyz:8081 + │ on err ──► log + os.Exit(1) + │ + └─ mgr.Start(ctrl.SetupSignalHandler()) [main.go:68] + SetupSignalHandler installs SIGINT / SIGTERM cancel + mgr.Start blocks; on shutdown → graceful stop + on err ──► log + os.Exit(1) +``` + +--- + +## 3. Watch Wiring and Event Sources + +`SetupCertificateReconciler` registers two watches before starting the controller +(`certificate_controller.go:59-63`): + +``` + controller-runtime manager + │ + ├─ For(&cmv1.Certificate{}) primary watch + │ every Certificate create/update/delete + │ ──► enqueue Reconcile(cert.Namespace/cert.Name) + │ + └─ Watches(&gwapiv1.Gateway{}, + EnqueueRequestsFromMapFunc(r.gatewayToCertificates)) + secondary watch + every Gateway create/update/delete + ──► gatewayToCertificates maps Gateway → []Certificates + ──► enqueue Reconcile for each matching Certificate +``` + +Full trigger table: + +| ID | Event | Enqueued object | Code path | +|----|-------|-----------------|-----------| +| E1 | Certificate **create** | that Certificate | primary watch | +| E2 | Certificate **update** (any field — no predicate filter) | that Certificate | primary watch | +| E3 | Certificate **delete** | that Certificate | primary watch | +| E4 | Gateway **create / update / delete** | every Certificate targeting it (via field index) | secondary watch + `gatewayToCertificates` | +| E5 | Reconcile returns non-nil error | same Certificate | controller-runtime rate-limited backoff re-queue | +| E6 | Controller-runtime cache resync (~10 h default) | every known Certificate | implicit periodic replay | + +--- + +## 4. Field Index + +The field index is the only mechanism that lets the operator find all Certificates pointing at a given Gateway without scanning the entire namespace. It is registered during setup and queried in two places. + +``` + IndexFieldGatewayTarget = ".metadata.annotations.gatewayTarget" + (identifier used by controller-runtime; no annotation by this name exists) + + Indexer function (certificate_controller.go:44-55): + ┌─────────────────────────────────────────────────────────┐ + │ for each Certificate obj: │ + │ gwName = obj.Annotations["…/gateway-name"] │ + │ if gwName == "" → return nil (not indexed) │ + │ gwNs = obj.Annotations["…/gateway-namespace"] │ + │ if gwNs == "" → gwNs = obj.Namespace (fallback) │ + │ return [ "<gwNs>/<gwName>" ] │ + └─────────────────────────────────────────────────────────┘ + + Key shape: "<targetGatewayNamespace>/<targetGatewayName>" + + Two query sites: + + ┌──────────────────────────────────────────────────────┐ + │ Reconcile [certificate_controller.go:95-99] │ + │ List(Certs, MatchingFields{index: "<gwNs>/<gwName>"})│ + │ returns all sibling Certs targeting the same GW │ + └──────────────────────────────────────────────────────┘ + + ┌──────────────────────────────────────────────────────┐ + │ gatewayToCertificates [certificate_controller.go:138]│ + │ List(Certs, MatchingFields{index: "<gw.Ns>/<gw.Name>"})│ + │ used by the secondary watch to fan-out GW events │ + │ to the relevant Certs │ + └──────────────────────────────────────────────────────┘ +``` + +The namespace-fallback rule (`gwNs = obj.Namespace` when the annotation is absent) is duplicated in two places: the indexer (`:51-53`) and `Reconcile` (`:89-91`). Both must apply the same rule or the index key used during write will differ from the key used during lookup. + +--- + +## 5. Reconcile Decision Tree + +Every enqueued item arrives here. Inputs: one `Reconcile.Request` containing the namespace/name of a Certificate. Side effects: 0 or 1 `PATCH` on a Gateway. + +``` +Reconcile(ctx, req) + │ + ├─(A) r.Get(ctx, req.NamespacedName, &cert) [controller:71] + │ │ + │ ├─ NotFound ──► client.IgnoreNotFound → nil err + │ │ return Result{}, nil ►► STOP — NO ACTION + │ │ + │ │ ┌─ IMPORTANT: the deleted cert's listener is NOT removed here. + │ │ │ The controller has no record of which Gateway was targeted. + │ │ │ Cleanup happens on the next E2/E4/E6 event for any sibling + │ │ │ cert that still references the same Gateway — at that point + │ │ │ BuildListenersFromCertificates omits the deleted cert and + │ │ │ MergeListeners replaces all auto-* listeners. Until that + │ │ │ event fires, the stale listener remains in the Gateway. + │ │ │ (See §10, item 3 for details.) + │ │ └─────────────────────────────────────────────────────────── + │ │ + │ └─ other error ──► return Result{}, err ►► REQUEUE (E5) + │ + ├─(B) gwName, ok := cert.Annotations["…/gateway-name"] [controller:82] + │ │ + │ └─ !ok ──► log V(1) "missing gateway annotation" + │ return Result{}, nil ►► STOP — NO ACTION + │ + ├─(C) gwNamespace := cert.Annotations["…/gateway-namespace"] [controller:88] + │ └─ if "" ──► gwNamespace = cert.Namespace (DEFAULT FALLBACK) + │ + ├─(D) r.List(ctx, &certList, [controller:95] + │ MatchingFields{IndexFieldGatewayTarget: "<gwNs>/<gwName>"}) + │ │ + │ └─ err ──► return Result{}, err ►► REQUEUE + │ + ├─(E) desiredListeners := + │ gw.BuildListenersFromCertificates(certList.Items, "auto-") + │ (see §6a) [controller:102] + │ + ├─(F) r.Get(ctx, {gwNs, gwName}, &gateway) [controller:106] + │ │ + │ └─ err (including NotFound) ──► return Result{}, err ►► REQUEUE + │ + │ NOTE: Gateway-not-found is NOT specially handled — the + │ reconciler will retry with backoff indefinitely. A typo'd + │ gateway-name annotation produces a perpetual retry loop. + │ (See §10, item 5.) + │ + ├─(G) updated := gw.MergeListeners( [controller:111] + │ gateway.Spec.Listeners, + │ desiredListeners, "auto-") + │ (see §6b) + │ + ├─(H) gw.ListenersEqual(gateway.Spec.Listeners, updated) [controller:113] + │ │ + │ └─ true ──► log V(1) "already up to date" + │ return Result{}, nil ►► SHORT-CIRCUIT — NO PATCH + │ + ├─(I) patch := client.MergeFrom(gateway.DeepCopy()) [controller:118] + │ gateway.Spec.Listeners = updated + │ r.Patch(ctx, &gateway, patch) [controller:120] + │ │ + │ └─ err ──► return Result{}, err ►► REQUEUE + │ + └─(J) log Info "patched gateway listeners" [controller:124] + return Result{}, nil ►► DONE + +Cluster I/O per reconcile: + Reads: 1 Certificate Get (step A) + 1 Certificate List via index (step D) + 1 Gateway Get (step F) + Writes: 0 or 1 Gateway Patch (step I, only when listeners changed) +``` + +--- + +## 6. Build / Merge / Equal Pipeline + +The three pure functions in `internal/gateway/patch.go` do all computation. `Reconcile` only calls them; it holds no business logic of its own. + +``` + certList.Items + │ + v + BuildListenersFromCertificates(certs, "auto-") [patch.go:22] + │ + └──► desired []gwapiv1.Listener + │ + │ gateway.Spec.Listeners (current state from API server) + │ │ + v v + MergeListeners(existing, desired, "auto-") [patch.go:76] + │ + └──► updated []gwapiv1.Listener + │ + v + ListenersEqual(gateway.Spec.Listeners, updated) [patch.go:93] + │ + ├─ true ──► short-circuit, no patch + └─ false ──► Patch(gateway) +``` + +### 6a. BuildListenersFromCertificates + +``` +for cert in certs: + │ + ├─ if cert.DeletionTimestamp != nil ──► skip [patch.go:26] + │ (cert is being deleted; omitting it removes its listeners + │ from the desired set and MergeListeners will drop them) + │ + ├─ if cert.Spec.SecretName == "" ──► skip [patch.go:30] + │ + └─ for dns in cert.Spec.DNSNames: [patch.go:34] + │ + └─ emit Listener{ + Name: "auto-<certName>-<sanitized-dns>" + Hostname: dns + Port: 443 (hardcoded, see §8) + Protocol: HTTPS + AllowedRoutes: { From: All } + TLS: { + Mode: Terminate + CertificateRefs: [{ + Group: "" (core) + Kind: "Secret" + Name: cert.Spec.SecretName + Namespace: cert.Namespace (same as cert) + }] + } + } + +sort by Name ascending [patch.go:67] +(deterministic order prevents spurious diffs on every reconcile) +``` + +### 6b. MergeListeners + +``` +existing listeners + │ + ├─ Name starts with "auto-" ──► DROP (managed, will be replaced) + └─ Name does NOT start with "auto-" ──► KEEP (user-managed) + +merged = [kept user listeners] + [all desired auto-* listeners] +``` + +The result: user-created listeners appear first in their original order; operator-managed listeners follow, sorted by name. + +**Edge case:** a user-created listener whose name happens to start with `auto-` will be treated as managed and will be deleted on the next reconcile. + +### 6c. ListenersEqual + +`ListenersEqual` is a **partial comparison** — only the fields the operator actively manages are checked: + +| Compared | Ignored | +|----------|---------| +| listener `Name` | `Protocol` | +| `Hostname` | `AllowedRoutes` | +| `Port` | `TLS.Options` | +| first `CertificateRef` (`<ns>/<name>`) | cert refs beyond index 0 | +| | `Group`, `Kind` of ref | + +Drift in an ignored field will not trigger a corrective patch. For example: if someone manually flips the Protocol of an auto-managed listener from HTTPS to HTTP, the operator will consider it "equal" and leave it alone. + +The comparison is done via a name-keyed map (`listenerMap`, `patch.go:118`), so listener order does not affect the result. + +--- + +## 7. Listener Naming and Sanitization + +`listenerNameFor` (`patch.go:143`): + +``` +input: prefix="auto-" certName="my-app-cert" dnsName="app.example.com" + +step 1: replace every "." with "-" + "app.example.com" ──► "app-example-com" + +step 2: concatenate + "auto-" + "my-app-cert" + "-" + "app-example-com" + ──► "auto-my-app-cert-app-example-com" + +step 3: if len > 253 bytes: name = name[:253] (hard byte slice) +``` + +Example outputs for a cert with two DNS names: + +| `dnsName` | listener `Name` | +|-----------|-----------------| +| `app.example.com` | `auto-my-app-cert-app-example-com` | +| `*.example.com` | `auto-my-app-cert-*-example-com` | + +The second entry is a risk: `*` is not valid in a Gateway API `SectionName` (which must match RFC 1123). The API server will reject the patch and the reconciler will requeue forever (see §10, item 7). + +The sanitization is also lossy: `a.b.com` and `a-b-com` produce the same result. If a certificate lists both, two listeners with the same name are appended; the API server rejects the patch (listener names must be unique), so no listener on that Gateway is updated. + +--- + +## 8. Defaults Baked In + +All of these are hardcoded in `internal/gateway/patch.go`. None are configurable via annotations (the `listener-port` annotation exists as a constant but is not read — see §10, item 1). + +| Property | Value | Source | +|----------|-------|--------| +| Listener port | `443` | `defaultPort` `patch.go:13` | +| TLS mode | `TLSModeTerminate` | `tlsModeTerminate` `patch.go:14` | +| Allowed routes | `NamespacesFromAll` | `namespacesFromAll` `patch.go:15` | +| Protocol | `HTTPSProtocolType` | inline `patch.go:45` | +| CertificateRef Group | `""` (core API group) | `patch.go:55` | +| CertificateRef Kind | `"Secret"` | `patch.go:56` | +| Secret namespace | cert's own namespace | `patch.go:39,58` | +| Listener name prefix | `"auto-"` | `ListenerPrefix` `controller:28` | +| SectionName max length | 253 bytes | `patch.go:147` | +| Gateway namespace fallback | cert's own namespace | `controller:51-53` and `controller:89-91` | + +--- + +## 9. End-to-End Scenarios + +### Scenario 1 — First Certificate created, Gateway has no auto-listeners + +``` +1. User creates Certificate "my-cert" (namespace: default) + Annotations: gateway-name=my-gateway + Spec: dnsNames=[app.example.com], secretName=my-cert-tls + +2. (E1) Cert create event fires → Reconcile(default/my-cert) + +3. (A) Get cert → found + (B) gateway-name="my-gateway" → ok + (C) gateway-namespace missing → fallback to "default" + (D) List certs by index "default/my-gateway" → [my-cert] + (E) BuildListeners → [Listener{Name:"auto-my-cert-app-example-com", Port:443, …}] + (F) Get gateway "default/my-gateway" → found, Listeners=[] + (G) MergeListeners([], desired) → [auto-my-cert-app-example-com] + (H) ListenersEqual([], updated) → false (len differs) + (I) Patch gateway → Spec.Listeners=[auto-my-cert-app-example-com] + (J) Done. Log: "patched gateway listeners" + +Cluster writes: 1 Gateway PATCH +``` + +### Scenario 2 — Second Certificate added for the same Gateway + +``` +1. User creates Certificate "other-cert" targeting the same Gateway + dnsNames=[api.example.com], secretName=other-cert-tls + +2. (E1) Reconcile(default/other-cert) + +3. (D) List certs by index "default/my-gateway" → [my-cert, other-cert] + (E) BuildListeners → [ + auto-my-cert-app-example-com, + auto-other-cert-api-example-com + ] (sorted alphabetically by name) + (F) Gateway still has [auto-my-cert-app-example-com] + (G) MergeListeners → [auto-my-cert-app-example-com, + auto-other-cert-api-example-com] + (H) ListenersEqual(old 1-item, new 2-item) → false + (I) Patch → Gateway now has both listeners + +Cluster writes: 1 Gateway PATCH +``` + +### Scenario 3 — Certificate dnsNames updated + +``` +1. User edits "my-cert": dnsNames=[app.example.com] → [app.example.com, www.example.com] + +2. (E2) Cert update event → Reconcile(default/my-cert) + +3. (E) BuildListeners produces TWO listeners for my-cert now: + auto-my-cert-app-example-com + auto-my-cert-www-example-com + (plus any from other-cert if still present) + (H) ListenersEqual: len changed → false + (I) Patch gateway with new full set + +Cluster writes: 1 Gateway PATCH +``` + +### Scenario 4 — Certificate deleted + +``` +1. User deletes "my-cert" + +2. (E3) Cert delete event → Reconcile(default/my-cert) + +3. (A) Get cert → NotFound + return Result{}, nil ──► STOP, NO ACTION + +4. Gateway still contains "auto-my-cert-app-example-com" — stale. + +5. Cleanup happens only when the next relevant event fires: + - If "other-cert" is updated or created → (E2) Reconcile(other-cert) + (D) List by index returns [other-cert] only (my-cert is gone) + (E) BuildListeners → only other-cert's listeners + (G) MergeListeners drops the stale auto-my-cert-* listener + (I) Patch removes it + - OR if the Gateway is manually edited → (E4) all targeting Certs + re-enqueued, same convergence path + - OR on the next cache resync (E6, ~10 h) + + Every path above needs at least one surviving Certificate that targets + the Gateway. If my-cert was the last one, nothing is ever enqueued for + it and the stale listener stays until someone removes it by hand. + +Cluster writes: 0 at deletion time; 1 Gateway PATCH at next sibling event +``` + +--- + +## 10. Known Limitations + +Recorded as-found. None are fixed by this document. + +**1. `AnnotationListenerPort` is dead code** +`certificate_controller.go:25` declares the constant; `BuildListenersFromCertificates` (`patch.go:44`) always uses `defaultPort = 443`. Setting `gateway-cert-operator.io/listener-port` on a Certificate has no effect. + +**2. `--metrics-bind-address` flag is parsed but not wired** +`main.go:32` parses the flag; `main.go:44-47` never passes `MetricsBindAddress` to `ctrl.Options`. controller-runtime defaults an empty bind address to `:8080`, so metrics are served and the Service in `deploy/service_gateway-cert-operator-metrics.yaml` works; only changing the flag has no effect. + +**3. No finalizer → cert deletion leaves stale listeners** +When a Certificate is deleted, Reconcile returns early at branch (A) with no cluster write. The stale `auto-*` listener persists in the Gateway until a sibling event triggers re-convergence (see Scenario 4 above), or permanently if no other Certificate targets that Gateway. Adding a finalizer to the Certificate would close this gap by allowing cleanup before the object is removed from the API server. + +**4. `ListenersEqual` is a partial comparison** +Described in §6c. Drift in `Protocol`, `AllowedRoutes`, `TLS.Options`, or any `CertificateRef` beyond the first will not be detected or corrected. + +**5. Gateway-not-found retries forever** +Branch (F) of Reconcile treats all Gateway `Get` errors the same (including `NotFound`) and returns them as re-queue-able errors. A Certificate with a typo'd `gateway-name` annotation will retry with rate-limited backoff indefinitely, generating log noise and API load. There is no terminal failure path. + +**6. No leader election** +`ctrl.NewManager` at `main.go:44` does not set `LeaderElection: true`. The `deploy/deployment_gateway-cert-operator.yaml` runs `replicas: 1` which makes this safe today. Scaling to multiple replicas without enabling leader election would result in concurrent Gateway patches. + +**7. `listenerNameFor` edge cases** +- **Wildcard hostnames**: `*.example.com` produces `SectionName` `auto-…-*-example-com`. The `*` character is not valid in an RFC 1123 label; the Gateway API server will reject the Patch, causing an error-retry loop. +- **Collisions after sanitization**: `a.b.com` and `a-b-com` produce identical sanitized names. The same happens across certificates (cert `a` with `b-c.com` and cert `a-b` with `c.com`) and across namespaces, since the name omits the cert namespace. Both listeners are appended and the API server rejects the patch for the whole Gateway. +- **Truncation at 253 bytes**: `patch.go:147` slices on bytes, not runes (safe for ASCII DNS names). Two DNS names sharing the first 253 characters after sanitization will map to the same listener name. + +**Dead symbols in `patch.go`** +`gatewayGroup` and `gatewayKind` are declared at `patch.go:16-17` but never referenced. They can be removed without effect. diff --git a/docs/plans-executions/2026-10-08-2028-house-conventions.md b/docs/plans-executions/2026-10-08-2028-house-conventions.md index 39c2f7c..d07411f 100644 --- a/docs/plans-executions/2026-10-08-2028-house-conventions.md +++ b/docs/plans-executions/2026-10-08-2028-house-conventions.md @@ -3,10 +3,32 @@ Plan: `docs/plans/2026-10-08-2028-house-conventions.md` - [x] Step 0 — Save and commit the plan -- [ ] Step 1 — CLAUDE.md +- [x] Step 1 — CLAUDE.md - [ ] Step 2 — `.claude/` and docs scaffolding - [ ] Step 3 — Makefile, lint, formatting - [ ] Step 4 — Module path and version identity - [ ] Step 5 — Baseline tests - [ ] Step 6 — CI build workflow - [ ] Step 7 — Close out + +## Step 1 — CLAUDE.md + +Rewrote `CLAUDE.md` in the five-section shape. The Go, Testing, Changelog and +Branching text comes from the `go-project-bootstrap` skill references; the plan +ordering and execution-summary rules come from `egress-proxies-operator/CLAUDE.md`, +condensed from prose into bullets. + +Deviations from the plan: + +- The house container rules about Ubuntu dev images and separate dev/prod + Dockerfiles were left out: this repo has one production Dockerfile and no dev + image, and listing rules nobody follows only adds noise. +- The `listener-port` row was removed from the annotation table and moved to + Gotchas, so the table lists only what works. +- Added a short Reviews subsection so the reviewer agents and `docs/reviews/` have a + documented trigger. +- `README.md` also claimed that deleting a certificate removes its listeners + "instantly". Reworded and added the known limitation. + +Worth noting: Commands and CI/CD describe the repo as it is at this commit; Steps 3, +4 and 6 update them. `docs/flows.md` cites `main.go` line numbers that Step 4 shifts.