diff --git a/.gitea/workflows/build.yaml b/.gitea/workflows/build.yaml index 66fccac..534f053 100644 --- a/.gitea/workflows/build.yaml +++ b/.gitea/workflows/build.yaml @@ -4,15 +4,14 @@ on: workflow_dispatch: inputs: tag: - description: 'Image tag' + description: 'Image tag (not "latest")' required: true - default: 'latest' push: tags: - '*' concurrency: - group: build-${{ github.ref }} + group: build-${{ github.ref }}-${{ inputs.tag }} cancel-in-progress: true jobs: @@ -45,6 +44,11 @@ jobs: run: | if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then TAG="${{ inputs.tag }}" + # The Deployment runs :latest; only real tag pushes may move it. + if [ "$TAG" = "latest" ]; then + echo "refusing to dispatch with tag 'latest'" >&2 + exit 1 + fi else TAG="${{ github.ref_name }}" fi diff --git a/CLAUDE.md b/CLAUDE.md index 6a1d374..3147d61 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -52,7 +52,8 @@ Reconcile loop (keyed by Certificate): 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. +same index. Manual edits to `auto-*` listeners are reverted only in the fields +`ListenersEqual` compares, and only while at least one Certificate targets the Gateway. Listeners named `auto-{certName}-{dnsName with dots as dashes}` belong to the operator. Any other listener is never modified or removed. @@ -210,9 +211,10 @@ 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 with a `tag` input. A `check` job (`go vet`, `go build`, - `go test -short`) gates the build. Every build pushes `:` and an immutable - `:sha-<12-char-commit>`; `:latest` moves only on real tag pushes. Needs the - `REGISTRY_TOKEN` repo secret. + `go test -short`) gates the build; it does not run lint or `-race`, so run + `make check` before tagging. Every build pushes `:` and an immutable + `:sha-<12-char-commit>`; `:latest` moves only on real tag pushes, and a dispatch + with `tag: latest` is rejected. Needs the `REGISTRY_TOKEN` repo secret. - `kubernetes-deploy.yaml` applies `deploy/` to the home-lab cluster through a Vault → Kanidm → OIDC auth chain. @@ -255,6 +257,8 @@ Gitea Actions, workflows in `.gitea/workflows/`: - **`make lint` pins `GOTOOLCHAIN` to the go.mod version.** golangci-lint cannot type-check a standard library newer than the Go it was built with, so running `golangci-lint run` directly fails when the host Go is ahead of it. +- **`make docker-build` builds `linux/amd64`** regardless of the host, because that + is what the cluster runs. Override with `PLATFORM=`. - **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. diff --git a/Dockerfile b/Dockerfile index b17fd67..4de424f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,6 @@ FROM golang:1.25 AS builder ARG TARGETOS ARG TARGETARCH -ARG GIT_COMMIT=unknown WORKDIR /workspace COPY go.mod go.sum ./ @@ -11,7 +10,9 @@ RUN go mod download COPY . . # The build context has no .git, so Go's automatic VCS stamp is absent here; -# the commit comes in through GIT_COMMIT instead. +# the commit comes in through GIT_COMMIT instead. Declared this late so a new +# commit does not invalidate the module-download layer. +ARG GIT_COMMIT=unknown RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH:-amd64} go build \ -trimpath \ -ldflags "-s -w -X gitea.home.hrajfrisbee.cz/kacerr/gatewayapi-certificates-operator/internal/version.Commit=${GIT_COMMIT}" \ diff --git a/Makefile b/Makefile index c13e53c..a4cb63a 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,8 @@ BINARY := manager BIN_DIR := bin IMG ?= gateway-cert-operator:dev +# The cluster is amd64; without this an Apple Silicon host builds arm64. +PLATFORM ?= linux/amd64 # GIT_COMMIT is baked into the image (-ldflags in the Dockerfile); -dirty # covers staged and untracked changes too, which `git diff --quiet` misses. @@ -8,7 +10,7 @@ GIT_COMMIT ?= $(shell git rev-parse --short=12 HEAD 2>/dev/null || echo unknown) # golangci-lint type-checks the standard library of whatever toolchain is active, # and fails on one newer than the Go it was built with. Linting with the go.mod -# version keeps it working when the host Go is ahead, and matches CI. +# version keeps it working when the host Go is ahead. GO_MOD_VERSION := $(shell awk '$$1 == "go" {print $$2}' go.mod) .PHONY: all build run test test-short lint vet fmt tidy check docker-build clean help @@ -43,7 +45,7 @@ tidy: ## Tidy go.mod and go.sum check: lint vet test-short ## Run all quality checks (lint + vet + test-short) docker-build: ## Build the container image (IMG=name:tag) - docker build --build-arg GIT_COMMIT=$(GIT_COMMIT) -t $(IMG) . + docker build --platform $(PLATFORM) --build-arg GIT_COMMIT=$(GIT_COMMIT) -t $(IMG) . clean: ## Remove build artifacts rm -rf $(BIN_DIR) diff --git a/docs/flows.md b/docs/flows.md index 08d5030..48447ef 100644 --- a/docs/flows.md +++ b/docs/flows.md @@ -35,27 +35,35 @@ 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] + └─ main() [main.go:24] │ - ├─ flag.StringVar --metrics-bind-address default ":8080" [main.go:32] - ├─ flag.StringVar --health-probe-bind-address default ":8081" [main.go:33] + ├─ flag.StringVar --metrics-bind-address default ":8080" [main.go:29] + ├─ flag.StringVar --health-probe-bind-address default ":8081" [main.go:30] + ├─ flag.BoolVar --version [main.go:31] + ├─ zap.Options{Development: true, AddCaller}.BindFlags [main.go:32-33] + │ adds --zap-devel, --zap-encoder, --zap-log-level, … ├─ flag.Parse() [main.go:34] │ - ├─ ctrl.SetLogger(zap, UseDevMode=true, AddCaller) [main.go:36-41] - │ dev-mode console logger with file:line call site + ├─ --version ──► print version.Resolve(), return [main.go:36-39] + │ before any logger or cluster access + │ + ├─ scheme := runtime.NewScheme() [main.go:41-44] + │ ├─ clientgoscheme.AddToScheme(scheme) # core/v1: Secrets, Events, … + │ ├─ cmv1.AddToScheme(scheme) # cert-manager.io/v1 Certificate + │ └─ gwapiv1.Install(scheme) # gateway.networking.k8s.io/v1 Gateway + │ + ├─ ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) [main.go:46] + │ dev-mode console logger with file:line call site unless + │ --zap-devel=false is passed + ├─ log "starting gateway-cert-operator" commit=… goVersion=… [main.go:48] │ ├─ ctrl.GetConfigOrDie() # KUBECONFIG or in-cluster config │ on err ──► os.Exit(1) │ ├─ ctrl.NewManager(cfg, Options{Scheme, Metrics, HealthProbeBindAddress}) - │ on err ──► log + os.Exit(1) [main.go:44-47] + │ on err ──► log + os.Exit(1) [main.go:50-54] │ - ├─ controller.SetupCertificateReconciler(mgr) [main.go:53] + ├─ controller.SetupCertificateReconciler(mgr) [main.go:60] │ registers field index + watches (see §3) │ on err ──► log + os.Exit(1) │ @@ -63,7 +71,7 @@ process start ├─ mgr.AddReadyzCheck("readyz", healthz.Ping) → GET /readyz:8081 │ on err ──► log + os.Exit(1) │ - └─ mgr.Start(ctrl.SetupSignalHandler()) [main.go:68] + └─ mgr.Start(ctrl.SetupSignalHandler()) [main.go:75] SetupSignalHandler installs SIGINT / SIGTERM cancel mgr.Start blocks; on shutdown → graceful stop on err ──► log + os.Exit(1) @@ -479,7 +487,7 @@ Described in §6c. Drift in `Protocol`, `AllowedRoutes`, `TLS.Options`, or any ` 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. +`ctrl.NewManager` at `main.go:50` 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. 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 92f65a3..cdac887 100644 --- a/docs/plans-executions/2026-10-08-2028-house-conventions.md +++ b/docs/plans-executions/2026-10-08-2028-house-conventions.md @@ -141,3 +141,15 @@ things could differ from egress on this repo's runner: `actions/setup-go@v5` (uh had to install Go with curl instead), and lint is not part of the `check` job, same as egress. The unused `GIT_TAG` and `BUILD_DATE` build args are gone; `kubernetes-deploy.yaml` is untouched. + +## Step 7 — Close out (review) + +Ran both reviewer agents on `main...HEAD`; findings and their status are in +`docs/reviews/2026-10-08-2040-house-conventions.md`. Ten items fixed on the branch, four new +bugs deferred to the bug-fix phase, one test simplification declined. + +Worth noting: the reviewer agents were not registered as agent types when the review +started (they were added in this session), so they were run as general-purpose +agents told to follow the agent files. The `:latest` dispatch hole came in with the +workflow copied from egress and is still open there. Step 7 stays unchecked until +the branch is pushed and the MR is open. diff --git a/docs/reviews/2026-10-08-2040-house-conventions.md b/docs/reviews/2026-10-08-2040-house-conventions.md new file mode 100644 index 0000000..4c6fdfd --- /dev/null +++ b/docs/reviews/2026-10-08-2040-house-conventions.md @@ -0,0 +1,56 @@ +# Review: house conventions port + +**Created:** 2026-10-08 20:40 CEST +**Scope:** `main...feat/house-conventions` at `8c07428` (7 commits) +**Reviewers:** `operator-reviewer`, `go-operator-reviewer` + +Both reviewers ran `make check` and `go test -race ./...` (green) and confirmed the +branch changes no reconcile behaviour. No merge-blockers. + +## Worth fixing + +- [x] `build.yaml`: a manual dispatch with the default `tag: latest` overwrote + `:latest`, the tag the Deployment runs. Default removed; `latest` is rejected. + (Inherited from the egress workflow, which still has it.) +- [x] `Dockerfile`: `ARG GIT_COMMIT` sat above `go mod download`, so every commit + re-downloaded all modules. Moved to just above the build. +- [x] `patch_test.go`: `TestMergeListeners_ownership` compared names only; a + mutation that kept stale listener content passed. Added + `TestMergeListeners_replacesManagedContent`. +- [x] `make docker-build` followed the host architecture (arm64 on this Mac, where + the Dockerfile used to hard-code amd64). Now builds `linux/amd64` by default. +- [x] `docs/flows.md` §2 described the old `main.go` (line refs, hardcoded dev + logger, no `--version`). Rewritten against the current file. +- [x] `CLAUDE.md` overstated the reverse watch. Reworded. +- [x] `build.yaml`: two dispatches from the same ref shared a concurrency group and + cancelled each other. Group now includes the tag input. + +## Simplifications + +- [x] `patch.go`: package-level `tlsModeTerminate` / `namespacesFromAll` were shared + by address across every listener. Replaced with `ptrTo(...)`; `defaultPort` is a + const. +- [x] `main.go`: `init()` removed, scheme built in `main()` (CLAUDE.md says no + `init()`). +- [x] `Makefile` comment claimed the lint toolchain pin "matches CI"; CI does not + lint. Claim dropped. +- [ ] `TestListenerNameFor_sanitizesDots` tests an unexported helper and overlaps the + names table. Kept as the home for truncation and collision cases in the bug-fix + phase. + +## Deferred to the bug-fix phase (new, not in the plan's backlog) + +- [ ] Empty desired set: when a Gateway has only `auto-*` listeners and no cert + yields any, `MergeListeners` returns nil and the patch sends `listeners: null`, + which the API server rejects (`MinItems=1`). Permanent error requeue. Any fix for + "last cert deleted" runs straight into this. `patch.go` `MergeListeners`, + `certificate_controller.go:119-120`. +- [ ] Listeners are added before the Certificate is Ready, so they reference a + Secret that may not exist yet. +- [ ] ClusterRole grants `events` create/patch; nothing records events. +- [ ] CI `check` job has no golangci-lint and no `-race`. Needs a linter install + step on the runner (uh-cli does it from a pinned tarball). + +## Nice-to-have + +- [x] Assert the secret ref `Group` in `TestBuildListenersFromCertificates_listenerFields`. diff --git a/internal/gateway/patch.go b/internal/gateway/patch.go index ed62409..54c4b39 100644 --- a/internal/gateway/patch.go +++ b/internal/gateway/patch.go @@ -9,11 +9,7 @@ import ( gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" ) -var ( - defaultPort gwapiv1.PortNumber = 443 - tlsModeTerminate = gwapiv1.TLSModeTerminate - namespacesFromAll gwapiv1.FromNamespaces = gwapiv1.NamespacesFromAll -) +const defaultPort gwapiv1.PortNumber = 443 // BuildListenersFromCertificates creates the desired set of managed listeners // from all annotated Certificate resources targeting a single Gateway. @@ -43,11 +39,11 @@ func BuildListenersFromCertificates(certs []cmv1.Certificate, prefix string) []g Protocol: gwapiv1.HTTPSProtocolType, AllowedRoutes: &gwapiv1.AllowedRoutes{ Namespaces: &gwapiv1.RouteNamespaces{ - From: &namespacesFromAll, + From: ptrTo(gwapiv1.NamespacesFromAll), }, }, TLS: &gwapiv1.ListenerTLSConfig{ - Mode: &tlsModeTerminate, + Mode: ptrTo(gwapiv1.TLSModeTerminate), CertificateRefs: []gwapiv1.SecretObjectReference{ { Group: ptrTo(gwapiv1.Group("")), diff --git a/internal/gateway/patch_test.go b/internal/gateway/patch_test.go index d5da493..5da7542 100644 --- a/internal/gateway/patch_test.go +++ b/internal/gateway/patch_test.go @@ -143,6 +143,9 @@ func TestBuildListenersFromCertificates_listenerFields(t *testing.T) { if ref.Kind == nil || *ref.Kind != "Secret" { t.Errorf("ref kind = %v, want Secret", ref.Kind) } + if ref.Group == nil || *ref.Group != "" { + t.Errorf("ref group = %v, want the core group", ref.Group) + } if l.AllowedRoutes == nil || l.AllowedRoutes.Namespaces == nil || l.AllowedRoutes.Namespaces.From == nil || *l.AllowedRoutes.Namespaces.From != gwapiv1.NamespacesFromAll { @@ -202,6 +205,21 @@ func TestMergeListeners_ownership(t *testing.T) { } } +func TestMergeListeners_replacesManagedContent(t *testing.T) { + t.Parallel() + + old := listener("auto-web-a-example-com", "a.example.com", "default", "old-tls", 443) + desired := listener("auto-web-a-example-com", "a.example.com", "default", "web-tls", 443) + + got := MergeListeners([]gwapiv1.Listener{old}, []gwapiv1.Listener{desired}, testPrefix) + if len(got) != 1 { + t.Fatalf("got %d listeners, want 1", len(got)) + } + if name := got[0].TLS.CertificateRefs[0].Name; name != "web-tls" { + t.Errorf("secret name = %q, want the desired web-tls", name) + } +} + func TestListenersEqual_managedFields(t *testing.T) { t.Parallel() diff --git a/main.go b/main.go index ff65c84..ed6ba62 100644 --- a/main.go +++ b/main.go @@ -21,14 +21,6 @@ import ( "gitea.home.hrajfrisbee.cz/kacerr/gatewayapi-certificates-operator/internal/version" ) -var scheme = runtime.NewScheme() - -func init() { - utilruntime.Must(clientgoscheme.AddToScheme(scheme)) - utilruntime.Must(cmv1.AddToScheme(scheme)) - utilruntime.Must(gwapiv1.Install(scheme)) -} - func main() { var metricsAddr string var probeAddr string @@ -46,6 +38,11 @@ func main() { return } + scheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(cmv1.AddToScheme(scheme)) + utilruntime.Must(gwapiv1.Install(scheme)) + ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) log := ctrl.Log.WithName("setup") log.Info("starting gateway-cert-operator", "commit", version.Resolve(), "goVersion", goruntime.Version())