From 36030f40111de0cd5c980e76b80ef8c0851ad0ce Mon Sep 17 00:00:00 2001 From: Jan Novak Date: Thu, 8 Oct 2026 20:29:48 +0200 Subject: [PATCH] docs: add plan for porting house conventions Co-Authored-By: Claude Opus 5.5 --- .../2026-10-08-2028-house-conventions.md | 12 ++ .../2026-10-08-2028-house-conventions.md | 203 ++++++++++++++++++ 2 files changed, 215 insertions(+) create mode 100644 docs/plans-executions/2026-10-08-2028-house-conventions.md create mode 100644 docs/plans/2026-10-08-2028-house-conventions.md diff --git a/docs/plans-executions/2026-10-08-2028-house-conventions.md b/docs/plans-executions/2026-10-08-2028-house-conventions.md new file mode 100644 index 0000000..39c2f7c --- /dev/null +++ b/docs/plans-executions/2026-10-08-2028-house-conventions.md @@ -0,0 +1,12 @@ +# Execution: Bring gateway-helper-operator up to house conventions + +Plan: `docs/plans/2026-10-08-2028-house-conventions.md` + +- [x] Step 0 — Save and commit the plan +- [ ] 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 diff --git a/docs/plans/2026-10-08-2028-house-conventions.md b/docs/plans/2026-10-08-2028-house-conventions.md new file mode 100644 index 0000000..8c27c7a --- /dev/null +++ b/docs/plans/2026-10-08-2028-house-conventions.md @@ -0,0 +1,203 @@ +# Plan: Bring gateway-helper-operator up to house conventions + +**Created:** 2026-10-08 20:28 CEST + +## Context + +This repo dates from March 2026: 381 lines of Go, no tests, no Makefile, no lint +config, a `/init`-generated CLAUDE.md (uncommitted, with factual errors) and a +one-line `/ship`. The newer repos share a consistent set of practices, distilled in +the `go-project-bootstrap` skill (which names this repo as its operator example) and +extended in `egress-proxies-operator`, the closest analog. This plan ports those +practices here so the following bug-fix and feature work runs on the same rails: +tests, a quality gate, plans with execution logs, a changelog, and branch + MR flow. + +No reconcile behaviour changes in this plan. Bugs found during the survey are listed +at the end as the backlog for the next phase. + +### What the newer projects have in common + +| Practice | Source | Here today | +|---|---|---| +| CLAUDE.md in five sections (What this is, Commands, Architecture, Conventions, Gotchas), under ~150 lines | skill | `/init` output, three factual errors | +| House Go rules (ctx first, `%w` / `errors.Join`, no `init()`, stdlib first, fail fast) | skill, uh-cli, egress | absent | +| Testing rules: stdlib `testing`, table-driven, `Test_`, `t.Parallel()`, `-short` guard, `-race` | skill, uh-cli, egress | no tests at all | +| Plans in `docs/plans/YYYY-MM-DD-HHMM-.md`, committed alone before work | uh-cli, egress, capi-lab | absent | +| Paired `docs/plans-executions/` log with a resumable step checklist | egress, capi-lab | absent | +| Timestamped `CHANGELOG.md`, newest first, written after you confirm it works | all three | absent | +| `feat/` branch, `tea pr create`, never merge or delete from the CLI | all three | everything on `main` | +| `/ship` = commit + push with branch guard; `/release` = tag + confirm + push | egress, capi-lab | `/ship` tags every commit `0.NN` | +| Reviewer agents, findings saved to `docs/reviews/` | egress | absent | +| Committed `.claude/settings.json` allowlist, `settings.local.json` ignored | uh-cli, egress, capi-lab | only an untracked, un-ignored local file | +| Makefile with `help` and a one-shot `check` gate | uh-cli | absent | +| `.golangci.yml` v2 | uh-cli, egress | absent | +| Commit baked into the binary (`internal/version`, `--version`, startup log, OCI label, `sha-<12>` image tag) | egress | CI passes build args the Dockerfile discards | +| CI `check` job gating the image build; `:latest` moves only on tag pushes | egress | build only, `:latest` on every run | + +### Decisions + +- Docs workflow: egress-style (plans, execution logs, changelog, reviews). No + decisions/sessions/future-tasks files. +- `/ship` and `/release` split; new tags are semver `vMAJOR.MINOR.PATCH`, starting + at `v0.1.0`. Old `0.NN` tags stay as they are. +- `kubernetes-deploy.yaml` is not touched here (deferred to the bug-fix phase). +- Deviations from the house text, written into CLAUDE.md as such: + - Logging stays on logr/zap through controller-runtime, not `slog` (same as egress + does in practice). + - Integration tests mean envtest behind `testing.Short()`, not testcontainers. + - Commit subjects keep this repo's existing prefixes (`feat:`, `fix:`, `deploy:`, + `ci:`, `docs:`). +- `.agents/`, `.codex/`, `AGENTS.md` are not ported: in egress they are untracked + auto-migrations or kubebuilder scaffold, not conventions. + +## Steps (one commit each, execution-log entry per step) + +Branch: `feat/house-conventions` off `main`. + +### Step 0 — Save and commit the plan + +Copy this file to `docs/plans/2026-10-08-2028-house-conventions.md`, create +`docs/plans-executions/2026-10-08-2028-house-conventions.md` with the step +checklist, commit alone. + +### Step 1 — CLAUDE.md + +Rewrite `CLAUDE.md` in the skill's five-section shape +(`~/.claude/skills/go-project-bootstrap/references/claude-md-structure.md`). + +- **What this is / Commands / Architecture**: keep the annotation table and + reconcile-loop write-up; point to `docs/flows.md` for detail. Fix the errors: + leader election is not enabled; cert deletion does not clean up; `listener-port` + must not be listed as supported. +- **Conventions**: paste verbatim from the skill references + (`go-conventions.md`, `docs-conventions.md`, `git-workflow.md` Gitea variant, + `containers-k8s.md`) plus egress's "Plan execution summaries" and plan "Ordering + matters" paragraphs from + `/Users/jan.novak/srv/go/egress-proxies-operator/CLAUDE.md`. Remote: + `gitea.home.hrajfrisbee.cz/kacerr/gatewayapi-certificates-operator`. Add the + deviations above, the semver tag rule, and the push rule (confirm before + `git push`, except inside `/ship`). +- **Gotchas**: three project names (directory, remote/image, module/manifests); + port annotation declared but unused; stale listeners on cert delete, annotation + removal and retarget; `--metrics-bind-address` parsed but ignored (metrics still + serve on `:8080`); `ListenersEqual` is partial; `auto-` prefix is the only + ownership marker; deployment uses mutable `:latest`; Dockerfile base images are + not digest-pinned; `kubernetes-deploy.yaml` runs on every branch push. + +Also in this step: commit `docs/flows.md` after correcting its three wrong claims +(metrics "never bound"; cleanup via resync; name collisions being "last write +wins" — the API server rejects the patch), and move `docs/README.md` to the repo +root with `kubectl apply -f deploy/manifests.yaml` corrected to +`kubectl apply -k deploy/`. + +### Step 2 — `.claude/` and docs scaffolding + +- `.claude/settings.json`: the skill's `assets/settings.json` starter allowlist + plus `Bash(tea pr create:*)`, `Bash(make *)` for the Step 3 targets and + `Bash(docker build:*)`. No `git commit`, `git push` or `git tag`. +- `.claude/commands/ship.md` and `release.md`: copy from + `egress-proxies-operator/.claude/commands/`; in `release.md` replace the "no + tagging scheme" paragraph with the semver rule. +- `.claude/agents/operator-reviewer.md`, `go-operator-reviewer.md`: copy from + `egress-proxies-operator/.claude/agents/` unchanged. +- `CHANGELOG.md` (`# Changelog` header only), `docs/reviews/.gitkeep`. +- `.gitignore`: add `.claude/settings.local.json`, `bin/`, `cover.out`; fix the + missing trailing newline. + +### Step 3 — Makefile, lint, formatting + +- `Makefile` modelled on `/Users/jan.novak/srv/go/uh-cli/Makefile`: `help` + (self-documenting), `build` (to `bin/manager`), `run`, `test` (`-race -cover`), + `test-short`, `vet`, `fmt`, `lint`, `tidy`, `check` (lint + vet + test-short), + `docker-build` (passes `GIT_COMMIT`, using egress's dirty-aware definition), + `clean`. +- `.golangci.yml`: v2, default linters, as in uh-cli. Egress's config is unmodified + kubebuilder scaffold and currently does not pass, so it is not the model. +- `.dockerignore`: egress's allowlist form. +- `gofmt` `main.go` and `internal/gateway/patch.go`; fix whatever `make lint` + reports (the dead `gatewayGroup` / `gatewayKind` vars are expected). +- Add the quality-gate sentence for `make check` to CLAUDE.md. + +### Step 4 — Module path and version identity + +- Rename the module from `github.com/example/gateway-cert-operator` to + `gitea.home.hrajfrisbee.cz/kacerr/gatewayapi-certificates-operator` (`go.mod`, + the import in `main.go`). +- `internal/version/version.go` + test, ported from + `egress-proxies-operator/internal/version/` (`Commit` via ldflags, fallback to + `debug.ReadBuildInfo`). +- `main.go`: `--version` flag; first log line carries commit and Go version; zap + options bound to flags (`opts.BindFlags`, `Development: true` default, so output + is unchanged unless flags are passed); pass `metricsAddr` to + `metricsserver.Options` so the existing flag takes effect (default stays `:8080`). +- `Dockerfile`: `ARG GIT_COMMIT=unknown`, `-ldflags -X …/internal/version.Commit`, + `GOARCH=${TARGETARCH:-amd64}`, `org.opencontainers.image.revision` label, drop the + unused `GIT_TAG` / `BUILD_DATE` expectations. + +### Step 5 — Baseline tests + +`internal/gateway/patch_test.go`: table-driven tests for +`BuildListenersFromCertificates`, `MergeListeners`, `ListenersEqual` and +`listenerNameFor`, covering current correct behaviour only (single and multiple +DNS names, sort order, manual listeners preserved, stale `auto-` listeners +dropped, equality ignoring order). Known-buggy inputs (wildcards, collisions, +truncation) are left for the bug-fix phase so the tests do not pin wrong +behaviour. Reconciler tests with the fake client come with the first reconcile fix. + +### Step 6 — CI build workflow + +`.gitea/workflows/build.yaml`, following +`egress-proxies-operator/.gitea/workflows/build.yaml`: `check` job (`go vet`, +`go build`, `go test -short`) gating `build`; `GIT_COMMIT` build arg and OCI +labels; push `:` and `:sha-<12>`; `:latest` only on real tag pushes; +concurrency group. Update the CI/CD paragraph in CLAUDE.md to match. + +Not verifiable locally. If `actions/setup-go@v5` fails on the runner (uh-cli hit +that), fall back to uh-cli's curl install from the `go.mod` version. + +### Step 7 — Close out + +Run both reviewer agents on the branch, save findings to +`docs/reviews/-house-conventions.md`, fix what is in scope. Push with +`/ship`, open the MR with `tea pr create`. The CHANGELOG entry is added after you +confirm it works. + +## Verification + +```bash +make check # lint + vet + test-short, all green +go test -race -cover ./... # new tests pass under -race +go build ./... && go vet ./... +make build && ./bin/manager --version # prints the 12-char commit +make docker-build IMG=gateway-cert-operator:dev +docker run --rm gateway-cert-operator:dev --version +kubectl kustomize deploy/ >/dev/null # manifests still render +``` + +Live check after merge: push tag `v0.1.0`, confirm the `check` job runs before +`build`, and that the registry has `:v0.1.0`, `:sha-<12>` and `:latest`. + +## Out of scope — backlog for the bug-fix phase + +Found during the survey, with file references; each is high confidence unless noted. + +1. Stale listeners after cert deletion, annotation removal or retarget + (`certificate_controller.go:71-91`). Root cause: the reconcile key is the + Certificate but the unit of desired state is the Gateway. +2. Wildcard DNS names produce an invalid listener name, and the rejected patch + blocks every cert on that Gateway (`patch.go:145`). +3. Listener name collisions: across namespaces, across cert/DNS splits, and after + truncation at 253 chars (`patch.go:143-151`). +4. `listener-port` annotation never read (`certificate_controller.go:25`, + `patch.go:44`). +5. Whole-array merge patch from a cached read, no optimistic lock + (`certificate_controller.go:118`). +6. `ListenersEqual` ignores protocol, allowed routes, TLS mode and extra refs + (`patch.go:93-139`). +7. Missing Gateway retried forever as an error (`certificate_controller.go:105-108`). +8. Cross-namespace secret refs without a ReferenceGrant; any Certificate author + can add `AllowedRoutes: All` listeners to any Gateway. +9. No events, status or metrics; no watch predicates; no leader election. +10. `kubernetes-deploy.yaml`: Vault, Kanidm and OIDC tokens echoed to the log, + TLS verification off, deploys on every branch push. +11. Deployment pinned to `:latest`, so new builds do not roll out.