docs: add plan for porting house conventions

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-08 20:29:48 +02:00
2 changed files with 215 additions and 0 deletions
@@ -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
@@ -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<Func>_<scenario>`, `t.Parallel()`, `-short` guard, `-race` | skill, uh-cli, egress | no tests at all |
+| Plans in `docs/plans/YYYY-MM-DD-HHMM-<slug>.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/<slug>` 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 <target> *)` 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 `:<tag>` 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/<timestamp>-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.