kubebuilder's generic scaffold defensively wires up webhook TLS-cert machinery and an unconditional cert-manager install in the e2e suite, in case a project grows admission webhooks later. This one never will -- the spec's non-goals explicitly rule out admission webhooks and cert-manager wiring -- so none of it does anything. Verified before removing: no config/webhook/, no +kubebuilder:webhook markers anywhere, and config/*/kustomization.yaml's [CERTMANAGER] blocks are all inert (never uncommented). cmd/main.go: drops the webhook import, the three webhook-cert-* flags, and the WebhookServer wiring on ctrl.Options -- the manager now runs with no webhook server, correctly, since nothing registers one. Left the metrics-cert flags alone; those are unrelated to webhooks. test/e2e/e2e_suite_test.go: drops the unconditional cert-manager install/uninstall around the suite. test/utils/utils.go: drops the now-dead InstallCertManager/ UninstallCertManager/IsCertManagerCRDsInstalled and their warnError helper, plus UncommentCode -- unrelated to cert-manager, but found to have zero callers even before this cleanup. Left the inert commented-out [WEBHOOK]/[CERTMANAGER] kustomize blocks and kubebuilder's scaffold marker comments alone: pure comments, no runtime behavior, unlike the cert-manager install this actually removed. Verified clean with both build tags (go build/vet, and -tags=e2e for test/e2e). make test unchanged and green. Co-Authored-By: Claude <noreply@anthropic.com>
480 lines
23 KiB
Markdown
480 lines
23 KiB
Markdown
# Execution log: proxy-operator
|
|
|
|
Pairs with [docs/plans/2026-08-07-1747-proxy-operator.md](../plans/2026-08-07-1747-proxy-operator.md).
|
|
|
|
## Status
|
|
|
|
- [x] Step 0 — Branch and scaffold
|
|
- [x] Step 1 — API types (`api/v1alpha1/proxy_types.go`)
|
|
- [x] Step 2 — Provider contract (`internal/provider/`)
|
|
- [x] Step 3 — Mock provider (`internal/provider/mock/`)
|
|
- [ ] Step 4 — Reconciler (`internal/controller/`)
|
|
- [ ] Step 5 — Health engine (`internal/health/`)
|
|
- [ ] Step 6 — Lease store (`internal/lease/`)
|
|
- [ ] Step 7 — Discovery API (`internal/discovery/`)
|
|
- [ ] Step 8 — GCP provider (`internal/provider/gcp/`)
|
|
- [ ] Step 9 — Orphan GC + metrics
|
|
- [ ] Step 10 — Wiring, config, docs
|
|
- [ ] Step 11 — Tests
|
|
- [ ] Verification (vet/test/kind e2e) + commit, push, open MR
|
|
|
|
## Step 0 — Branch and scaffold
|
|
|
|
Branched off the unborn `main`:
|
|
|
|
```bash
|
|
git checkout -b feat/proxy-operator
|
|
```
|
|
|
|
Installed kubebuilder v4.15.0 into a scratch `GOBIN` rather than the default
|
|
`$(go env GOPATH)/bin`, since the module's package layout changed and
|
|
`go install .../cmd/kubebuilder@v4.15.0` (the path from the plan) 404s — the binary is
|
|
now the module root itself:
|
|
|
|
```bash
|
|
GOBIN=<scratch>/bin go install sigs.k8s.io/kubebuilder/v4@v4.15.0
|
|
```
|
|
|
|
Scaffolded in place, with `kubebuilder` on `PATH`:
|
|
|
|
```bash
|
|
kubebuilder version
|
|
# KubeBuilder: v4.15.0, Kubernetes: 1.36.0
|
|
|
|
kubebuilder init --domain example.com \
|
|
--repo gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator --plugins go/v4
|
|
# WARN: target directory not empty (expected — CLAUDE.md, docs/, .claude/ already existed)
|
|
|
|
kubebuilder create api --group crawl --version v1alpha1 --kind Proxy \
|
|
--resource --controller
|
|
```
|
|
|
|
`create api` auto-ran `make manifests` at the end, which pulled and ran
|
|
`controller-gen` itself:
|
|
|
|
```bash
|
|
sigs.k8s.io/controller-tools/cmd/controller-gen@v0.21.0
|
|
"$(bin)/controller-gen" object:headerFile="hack/boilerplate.go.txt",year=2026 paths="./..."
|
|
```
|
|
|
|
Confirmed the CRD group landed correctly (no doubling — `--domain example.com
|
|
--group crawl` was used specifically to avoid the `crawl.crawl.example.com` trap
|
|
called out in the plan):
|
|
|
|
```bash
|
|
grep -A2 "GroupVersion =" api/v1alpha1/groupversion_info.go
|
|
# SchemeGroupVersion = schema.GroupVersion{Group: "crawl.example.com", Version: "v1alpha1"}
|
|
```
|
|
|
|
Dropped the scaffolded GitHub Actions workflows (remote is Gitea, not GitHub):
|
|
|
|
```bash
|
|
git rm -r --cached .github 2>/dev/null; rm -rf .github
|
|
```
|
|
|
|
Ran the full manifest/codegen pass once more to confirm the toolchain is reproducible
|
|
end to end:
|
|
|
|
```bash
|
|
make manifests generate
|
|
# controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
|
|
# controller-gen object:headerFile="hack/boilerplate.go.txt",year=2026 paths="./..."
|
|
```
|
|
|
|
then `go build ./...` and `go vet ./...`, both clean with no output.
|
|
|
|
Worth noting: `CONTROLLER_TOOLS_VERSION` in the generated `Makefile` came out at
|
|
`v0.21.0` by default in this kubebuilder release, so the Makefile edit the plan
|
|
anticipated wasn't needed. Pre-existing `CLAUDE.md`/`CHANGELOG.md` content survived
|
|
untouched; kubebuilder added its own `README.md`, `AGENTS.md`, `.golangci.yml`,
|
|
`.devcontainer/`, `Dockerfile` on top of them — those get edited or left as-is in
|
|
later steps. Committed as `076bc66`.
|
|
|
|
## Step 1 — API types (`api/v1alpha1/proxy_types.go`)
|
|
|
|
Wrote the full `ProxySpec`/`ProxyStatus`/`Proxy` types per the plan, including the
|
|
four corrections called out there (`MaxLeases *int32`, `HealthCheck` with
|
|
`+kubebuilder:default={}`, `MinLength=1` on `Provider`/`CloudInit.Inline`,
|
|
`Conditions` with `+listType=map`), the 7 CEL `XValidation` rules (6 on `ProxySpec`,
|
|
1 on `CloudInitSpec`), and pure helpers in `helpers.go`
|
|
(`EffectivePort`/`EffectiveHost`/`HealthCheckOrDefault`/`MaxLeasesOrDefault`) with
|
|
table-driven tests in `helpers_test.go`.
|
|
|
|
Regenerated deepcopy and the CRD:
|
|
|
|
```bash
|
|
make manifests generate
|
|
# controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
|
|
# controller-gen object:headerFile="hack/boilerplate.go.txt",year=2026 paths="./..."
|
|
```
|
|
|
|
Confirmed all 7 CEL rules and the `healthCheck` `default: {}` block landed in the
|
|
generated CRD as expected:
|
|
|
|
```bash
|
|
grep -B1 "rule:" config/crd/bases/crawl.example.com_proxies.yaml
|
|
# 7 matches, one per XValidation marker written
|
|
```
|
|
|
|
Ran the full suite, not just `go build`/`go vet`, since this was a good opportunity to
|
|
confirm envtest itself works end to end for the first time:
|
|
|
|
```bash
|
|
make test
|
|
# Setting up envtest binaries for Kubernetes version 1.36...
|
|
# .../bin/k8s/1.36.2-darwin-arm64 (confirms the plan's envtest version note)
|
|
```
|
|
|
|
This failed on the first run — not because of anything in the new types, but because
|
|
kubebuilder's scaffolded placeholder test in `proxy_controller_test.go` creates a bare
|
|
`Proxy{}` with no `spec.mode`, which our new required/enum field correctly rejects:
|
|
|
|
```text
|
|
Proxy.crawl.example.com "test-resource" is invalid: [spec.mode: Unsupported value: "":
|
|
supported values: "Managed", "External", ...]
|
|
```
|
|
|
|
That's a real envtest apiserver enforcing our schema for the first time, which is
|
|
useful confirmation on its own. Patched just the resource literal in that scaffold
|
|
test to a minimal valid spec (`Mode: External` + `Endpoint.Host`) rather than
|
|
rewriting the file — that whole test gets replaced in Step 4 alongside the real
|
|
reconciler, so a deeper fix now would be thrown away. `make test` then passed clean:
|
|
`api/v1alpha1` at 20.5% coverage (helpers only — CEL itself isn't unit-testable, it's
|
|
exercised by the real apiserver as shown above), `internal/controller` at 66.7%.
|
|
|
|
Worth noting: the `go test ./...` command from the plan's own verification section
|
|
does *not* work directly for the envtest suite — it needs `KUBEBUILDER_ASSETS` set,
|
|
which only `make test` does via `setup-envtest`. Plain `go test ./...` fails the
|
|
`internal/controller` package with a `/usr/local/kubebuilder/bin/etcd: no such file`
|
|
error that has nothing to do with the code. Use `make test`, not `go test ./...`,
|
|
whenever the controller package is in scope.
|
|
|
|
## Step 2 — Provider contract (`internal/provider/`)
|
|
|
|
Wrote the `Provider` interface (`Create`/`Get`/`Delete`/`ListByTag`), `Instance`,
|
|
`Placement`, `CreateRequest`, and the GC-contract label constants
|
|
(`provider.go`); the error taxonomy with multi-error `Unwrap() []error` so
|
|
`errors.Is` and `errors.As` both work off the same wrapped value
|
|
(`errors.go`); deterministic instance naming via SHA-256 → base32 → 16
|
|
chars (`name.go`); and `--providers-config` YAML parsing with fail-fast
|
|
validation (`config.go`).
|
|
|
|
One deliberate deviation from the plan's file layout: the plan listed
|
|
`internal/provider/metrics.go` as part of this step, but the
|
|
`provider.WithMetrics` decorator it describes is Step 9's concern (it needs
|
|
the Prometheus vectors that don't exist until the metrics package is
|
|
built) and nothing in this step depends on it existing yet. Deferred to
|
|
Step 9 rather than writing a decorator with nowhere to register its
|
|
metrics.
|
|
|
|
The registry package (`internal/provider/registry/registry.go`) came out
|
|
slightly different from the plan's sketch, and better for it: instead of a
|
|
package-level `var builtin = map[string]Constructor{"mock": mock.New, "gcp":
|
|
gcp.New}` living inside the registry package, `Build` takes the
|
|
`map[string]Constructor` as a parameter. This means `registry` has zero
|
|
import on `internal/provider/mock` or `internal/provider/gcp` — neither of
|
|
which exists yet at this point in the plan (mock is Step 3, gcp is Step 8)
|
|
— so the package compiles today instead of only once both are done, and the
|
|
explicit wiring lives at the composition root (`cmd/main.go`, Step 10)
|
|
rather than being smeared into the registry package itself. Still fully
|
|
avoids the import-cycle trap the plan called out.
|
|
|
|
Ran the full suite:
|
|
|
|
```bash
|
|
go mod tidy # sigs.k8s.io/yaml (already an indirect dep of the k8s.io toolchain) promoted to direct
|
|
go build ./... && go vet ./...
|
|
go test -race -v ./internal/provider/...
|
|
make test
|
|
```
|
|
|
|
`internal/provider` landed at 96.2% coverage, `internal/provider/registry` at
|
|
100%. `make test` also ran `go fmt ./...`, which reformatted `errors.go`'s
|
|
struct-field comment alignment before its first commit — no logic change,
|
|
just gofmt on a brand-new file.
|
|
|
|
Worth noting: `sigs.k8s.io/yaml` (not `gopkg.in/yaml.v3`) was picked for
|
|
`--providers-config` parsing specifically because it has `UnmarshalStrict`
|
|
built in (rejects unknown fields, which is what "fail fast on unknown type"
|
|
in the plan actually needs) and was already pulled in transitively by the
|
|
k8s.io toolchain, so no new dependency was added — `go mod tidy` just
|
|
promoted it from indirect to direct.
|
|
|
|
## Step 3 — Mock provider (`internal/provider/mock/`)
|
|
|
|
Started from the plan's design (state as a pure function of an injectable
|
|
clock, no background timers) but had to redesign the "real proxy" part
|
|
before writing any code, once a load-bearing assumption turned out false.
|
|
|
|
The plan (and the earlier decision to make the mock run a real proxy)
|
|
assumed each instance could get its own loopback address —
|
|
`127.0.0.1:<port>` per instance, "a fake IP from a private range." Verified
|
|
that assumption directly before committing to it:
|
|
|
|
```bash
|
|
cat <<'EOF' > /tmp/loopbacktest.go
|
|
package main
|
|
import ("fmt"; "net")
|
|
func main() {
|
|
for _, addr := range []string{"127.0.0.2:0", "127.0.0.55:0", "127.1.2.3:0"} {
|
|
l, err := net.Listen("tcp", addr)
|
|
if err != nil { fmt.Printf("%s: FAIL: %v\n", addr, err); continue }
|
|
fmt.Printf("%s: OK\n", addr)
|
|
l.Close()
|
|
}
|
|
}
|
|
EOF
|
|
go run /tmp/loopbacktest.go
|
|
# 127.0.0.2:0: FAIL: listen tcp 127.0.0.2:0: bind: can't assign requested address
|
|
# 127.0.0.55:0: FAIL: listen tcp 127.0.0.55:0: bind: can't assign requested address
|
|
# 127.1.2.3:0: FAIL: listen tcp 127.1.2.3:0: bind: can't assign requested address
|
|
```
|
|
|
|
Only `127.0.0.1` binds on macOS without `sudo ifconfig lo0 alias ... up` —
|
|
Linux routes the whole `127.0.0.0/8` block to loopback by default, macOS
|
|
doesn't. That's not something the operator can or should do at runtime, so
|
|
per-instance loopback IPs were out. There's also a second problem the
|
|
per-instance-IP design didn't solve anyway: `EffectivePort()`
|
|
(`api/v1alpha1/helpers.go`, Step 1) is computed purely from `spec.port`
|
|
with no channel for a provider to report back a different *port* — so
|
|
whatever a mock instance actually listens on has to be the literal port the
|
|
reconciler will pass through `CreateRequest.Port`, not an OS-assigned
|
|
ephemeral one.
|
|
|
|
Redesigned around one real listener **per port**, shared and
|
|
reference-counted across every instance that uses it, rather than one
|
|
listener per instance (`proxy.go`, `sharedProxies`). This fixes both
|
|
problems at once: every instance binds the same `127.0.0.1` (no OS issue),
|
|
and any number of instances can share a port without conflict since it's
|
|
the exact same underlying listener. Deliberately made the refcounting
|
|
**package-level**, not a field on `mock.Provider`, because a bound TCP port
|
|
is a genuinely process-global OS resource — two separately configured
|
|
mock-typed provider entries (e.g. two named `"mock"` instances in
|
|
`providers-config.yaml`) would otherwise both try to bind the same default
|
|
port and the second one would just fail. This is a case where a package
|
|
global is the correct model, not a shortcut: it mirrors an OS-level
|
|
singleton, not application state.
|
|
|
|
Instance lifecycle otherwise follows the plan exactly: `Get`/`ListByTag`
|
|
derive `Provisioning → Running → Terminated → purged (ErrNotFound)` from
|
|
`createdAt`/`deletedAt` compared against an injectable clock, with the real
|
|
proxy listener acquired lazily on the first observed `Running` and released
|
|
on `Delete` (or lazily on purge, so orphaned records can't leak a
|
|
reference). `Create` is idempotent by name. Fault injection is wired both
|
|
ways per the plan: `MockConfig.FailNextCreates`/`FailWith` for the demo
|
|
config, `InjectCreateFailures(n, class)` for tests.
|
|
|
|
Tests initially had a real flake, caught by running with `-race -count=3`
|
|
rather than trusting one green run:
|
|
|
|
```bash
|
|
go test -race -v ./internal/provider/mock/... 2>&1 | tail -5
|
|
# --- FAIL: TestAcquireProxy_sharedAcrossAcquires
|
|
# proxy_test.go:37: port not released after last reference: bind: address already in use
|
|
```
|
|
|
|
Root cause wasn't the refcounting logic — it was the test helper. `freePort`
|
|
asked the OS for a free port by binding to `:0` and immediately closing it,
|
|
which is a classic TOCTOU race under `t.Parallel()`: two tests can be handed
|
|
the same "free" port before either actually claims it, since nothing holds
|
|
it open in between. Fixed by replacing the OS-asks approach with a
|
|
monotonic counter (`20000 + atomic.Int32`) — these tests only need a port
|
|
unique *within this test run*, not one verified free by the OS at an
|
|
instant in time, so guaranteeing uniqueness outright is both simpler and
|
|
correct where the "ask and hope" approach wasn't. Reran `-race -count=3`
|
|
clean afterward.
|
|
|
|
Added tests beyond the plan's list to close real coverage gaps rather than
|
|
stopping at "green": config-override branches in `New`, all four
|
|
`failClassFromString` branches, `ListByTag`'s purge-on-list and
|
|
Running/IP-inclusion paths, and a plain-`http://` forwarding test
|
|
(`handleForward`) alongside the CONNECT one, since a `probeURL` override
|
|
could use either scheme. Landed at 91.1% coverage; the remainder is
|
|
OS-failure branches (bind errors, hijack failures) not worth simulating for
|
|
a prototype.
|
|
|
|
The `TestProvider_realProxyTunnelsConnect` test is the one that actually
|
|
matters most here: it opens real sockets end to end — mock `Create` →
|
|
`Get` past `provisionDelay` → real `http.Client` with
|
|
`Transport.Proxy` dialing through the mock's CONNECT tunnel to a real
|
|
`httptest.NewTLSServer` — and gets a real `204` back. That's the concrete
|
|
proof the "mock runs a real proxy" decision actually delivers a genuine
|
|
end-to-end healthcheck, not a simulated one.
|
|
|
|
`internal/provider/mock` at 91.1% coverage. `make test` green across the
|
|
whole repo.
|
|
|
|
## Step 3 (revised) — Kubernetes pod provider replaces the mock
|
|
|
|
After the mock provider above was built and working, the user pushed back:
|
|
it felt too far from the real system to build confidence in, and they'd
|
|
rather have simpler, more "real" code than a fast-but-simulated test
|
|
double — a `kind`-based verification pass "once in a while" is an
|
|
acceptable trade. They proposed replacing it outright with a provider that
|
|
creates real Pods in the operator's own cluster, rather than keeping mock
|
|
around as a fallback.
|
|
|
|
**Talked through the trade-off before agreeing to it, since it wasn't free
|
|
of downsides.** Pods sharing a cluster's egress IPs don't solve what this
|
|
operator actually exists for (routing around IP-based rate limiting needs
|
|
genuinely distinct egress paths — that's still only `gcp`), and `envtest`
|
|
has no kubelet, so a Pod-based provider can never be exercised by the fast
|
|
test suite either. Net effect: this is a replacement for the mock's role
|
|
in local dev/CI confidence-building, not a new alternative to GCP, and the
|
|
reconciler's own state-machine tests (Step 4) still need a minimal
|
|
in-test stub `Provider` — a handful of lines in the test file, not a
|
|
package with its own config format or fault-injection surface, which is
|
|
the specific kind of complexity the user was pushing back on.
|
|
|
|
**Picked the proxy software by actually checking what's out there,** not
|
|
by guessing a Docker Hub path:
|
|
|
|
```bash
|
|
curl -s "https://hub.docker.com/v2/repositories/vimagick/tinyproxy/" | head -c 400
|
|
# last_updated 2021-07-22 — stale
|
|
curl -s "https://hub.docker.com/v2/repositories/ubuntu/squid/" | head -c 400
|
|
# last_updated 2026-08-07T04:36:14Z — updated the same day, Canonical-published, 50M+ pulls
|
|
```
|
|
|
|
`ubuntu/squid` won clearly: actively maintained (rebuilt the same day this
|
|
check ran), official publisher, and — since it's a public image — `kind`
|
|
nodes pull it directly, no build/load step needed for the quickstart.
|
|
Pinned to `6.6-24.04_edge` (Ubuntu 24.04 LTS base) rather than floating
|
|
`latest`, for reproducibility.
|
|
|
|
**Design, in `internal/provider/kubernetes/`:**
|
|
|
|
- **`pod.go`** — `buildPod` is a pure function (mirrors the GCP provider's
|
|
planned `buildInsertRequest`): builds a `corev1.Pod` with one Squid
|
|
container. Config is generated in Go and passed via a `SQUID_CONF` env
|
|
var that the container's command writes to `/etc/squid/squid.conf`
|
|
before `exec squid` — deliberately not a separate `ConfigMap`, so
|
|
there's still exactly one Kubernetes object per proxy instance to
|
|
create, track, and clean up. The config itself is intentionally
|
|
permissive (`http_access allow all`, `via off`, `forwarded_for off`) —
|
|
documented in-code as a prototype-for-a-private-cluster choice, the same
|
|
posture the spec already takes toward cloud-init on the GCP provider
|
|
(installing/configuring proxy software is explicitly out of scope
|
|
there; this provider doesn't try to do more).
|
|
- **`kubernetes.go`** — `Create`/`Get`/`Delete`/`ListByTag` against a
|
|
`client.Client`. **`providerID` is `<namespace>/<podName>`**, parsed
|
|
with `k8s.io/client-go/tools/cache.SplitMetaNamespaceKey` — the exact
|
|
same reasoning as the GCP provider's planned zone-qualified providerID
|
|
(Step 8): `Get`/`Delete` need to be self-contained without re-deriving
|
|
where the resource lives. State mapping collapses `Succeeded`/`Failed`/
|
|
`Unknown` all into `Terminated`, since the reconciler already treats
|
|
Stopped and Terminated identically (delete + recreate) — no finer
|
|
distinction would change any behavior. `Running` with no `PodIP` yet
|
|
maps to `Provisioning`, not `Running`, so an empty IP is never
|
|
published — the same rule the plan already called out for GCP's
|
|
`RUNNING`-without-`NatIP` case.
|
|
- **Client construction is the one genuinely new pattern this provider
|
|
needed** that GCP/mock didn't: it builds its own `client.Client` via
|
|
`ctrl.GetConfig()`, which auto-detects in-cluster config when running as
|
|
a Pod and falls back to the local kubeconfig otherwise. That's what
|
|
makes `make run` against a local `kind` cluster and running in-cluster
|
|
use the exact same code path with zero provider-specific wiring in
|
|
`cmd/main.go`. The corresponding risk: `New()` must never run under
|
|
`go test` — it would happily connect to whatever real cluster the
|
|
developer's kubeconfig points at. Solved the same way `mock.Provider`
|
|
solved clock injection: an unexported `newWithClient(c, cfg)` constructor
|
|
that tests call directly, bypassing `ctrl.GetConfig()` entirely.
|
|
`New()` sits at 0% test coverage, deliberately — it's the one function
|
|
that must stay untested by design.
|
|
- **RBAC implication worth flagging now** (implemented in Step 10):
|
|
`ListByTag` lists Pods across every namespace, not just the namespace(s)
|
|
Proxies live in, because orphan GC needs to find every Pod this operator
|
|
tagged regardless of where it landed. That means the operator's Pod
|
|
permissions have to be a `ClusterRole`, not scoped to a single
|
|
namespace — a real trade-off against the "fleet lives in one namespace,
|
|
keeps RBAC simple" principle the spec states for the CRD itself.
|
|
Documented rather than special-cased away.
|
|
- **One known simplification, documented in a code comment rather than
|
|
solved:** Kubernetes surfaces both RBAC-denied and quota-exceeded as the
|
|
same 403 Forbidden, and `apierrors` has no helper to tell them apart.
|
|
Both classify as `ErrPermanent` — the safer of the two defaults (stop
|
|
retrying rather than hammering an API server that will never allow the
|
|
request), but a real ResourceQuota failure that would clear once other
|
|
proxies are deleted won't get the 5-minute-backoff-and-retry treatment
|
|
`ErrQuotaExceeded` gives on the GCP path.
|
|
|
|
**Testing:** unlike the mock, this package's tests use
|
|
`sigs.k8s.io/controller-runtime/pkg/client/fake` — real `corev1.Pod`
|
|
objects, the real `client.Client` interface, not a hand-rolled in-memory
|
|
map. That's a strictly more realistic test double than what mock.Provider
|
|
was, while still being fast (no cluster, no kubelet) — it just can't prove
|
|
a container actually starts and serves traffic, which is exactly the gap
|
|
the `kind`-based verification pass is for. 77.6% coverage; the only
|
|
meaningfully uncovered function is `New()` itself, deliberately.
|
|
|
|
`make test` green across the whole repo (`go build`/`go vet` clean,
|
|
`internal/provider` 96.0%, `internal/provider/kubernetes` 77.6%,
|
|
`internal/provider/registry` 100%, unchanged).
|
|
|
|
## Cleanup — removed unused cert-manager/webhook scaffolding
|
|
|
|
Not a plan step; the user asked for this directly after I explained what
|
|
`make test-e2e` currently does, and didn't want unused scaffold machinery
|
|
carried forward into later work.
|
|
|
|
kubebuilder's generic scaffold assumes a project *might* grow admission
|
|
webhooks later, so it wires up webhook TLS-cert machinery and an
|
|
unconditional cert-manager install in the e2e suite defensively. Checked
|
|
whether any of it was actually load-bearing before touching anything:
|
|
|
|
```bash
|
|
grep -rn "cert-manager\|certmanager\|CertManager" config/
|
|
# only inside commented-out [CERTMANAGER] blocks in kustomization.yaml —
|
|
# the whole block is inert, never uncommented
|
|
grep -n "webhook" PROJECT
|
|
# no output — kubebuilder create webhook was never run
|
|
```
|
|
|
|
Confirmed nothing here does anything for this project — no `config/webhook/`
|
|
exists, no `+kubebuilder:webhook` markers exist anywhere, and the spec's
|
|
own non-goals explicitly rule out admission webhooks and cert-manager
|
|
wiring forever, not just "not yet."
|
|
|
|
Removed:
|
|
|
|
- **`cmd/main.go`**: the `webhook` import, the three `webhook-cert-*`
|
|
flags, the `webhookServerOptions`/`webhookServer` construction, and the
|
|
`WebhookServer:` field on `ctrl.Options` — the manager runs with no
|
|
webhook server at all now, which is correct since nothing registers one.
|
|
Left the metrics-cert flags alone (`--metrics-cert-path` etc.) — those
|
|
let real certs be mounted for the metrics endpoint without cert-manager,
|
|
which is unrelated to webhooks and still useful. Reworded a comment that
|
|
said "TODO(user): If you enable certManager..." since that will never
|
|
happen here.
|
|
- **`test/e2e/e2e_suite_test.go`**: the unconditional cert-manager install
|
|
in `BeforeSuite` and matching uninstall in `AfterSuite`
|
|
(`setupCertManager`/`teardownCertManager`/`shouldCleanupCertManager`),
|
|
and the doc comment claiming the suite "requires Kind and CertManager"
|
|
(it only requires Kind now).
|
|
- **`test/utils/utils.go`**: `InstallCertManager`, `UninstallCertManager`,
|
|
`IsCertManagerCRDsInstalled`, and their now-unused `warnError` helper and
|
|
`certmanagerVersion`/`certmanagerURLTmpl` constants. Also removed
|
|
`UncommentCode` — grepped first and confirmed it had zero callers even
|
|
before this cleanup; it was dead scaffold code from the start, unrelated
|
|
to cert-manager, just found while in there.
|
|
|
|
Left the inert commented-out `[WEBHOOK]`/`[CERTMANAGER]` blocks in the
|
|
`config/*/kustomization.yaml` files and the
|
|
`+kubebuilder:scaffold:e2e-webhooks-checks`-style marker comments in
|
|
`test/e2e/e2e_test.go` alone — those are standard kubebuilder codegen
|
|
anchors and pure comments with no runtime behavior, unlike the cert-manager
|
|
install this cleanup actually removed. Stripping every trace of "webhook"
|
|
from every scaffold comment across the tree would be a much bigger, purely
|
|
cosmetic diff for no behavioral benefit; this cleanup targeted the things
|
|
that were actually *doing* something.
|
|
|
|
Verified with both build tags, since `test/e2e` only compiles under `-tags=e2e`:
|
|
|
|
```bash
|
|
go build ./... && go vet ./...
|
|
go build -tags=e2e ./... && go vet -tags=e2e ./...
|
|
```
|
|
|
|
Both clean. `make test` green across the whole repo, unchanged from before
|
|
the cleanup.
|