thwap/thwap-pagesd

Watch

Fork 0

feat(kubernetes): implement Kubernetes client wrapper #137

Merged

fuzzy merged 2 commits from feat/kubernetes-wrapper into main

2026-08-02 18:20:55 +00:00

automation commented

2026-08-02 17:54:25 +00:00

Owner

What

Implement the KubernetesService interface from task #32 on Client in internal/kubernetes using client-go networking/v1 Ingress resources:

CreateSiteIngress (#47, #48) — creates or updates the production Ingress (upsert). Host = the site's CustomDomain when set, otherwise <name>.<base_domain>; path / → backend service <name>; sets ingressClassName.
CreatePreviewIngress (#49) — creates an Ingress routing /preview/<branch> to the <name>-<branch> service on the site host (path-based routing).
DeletePreview (#50) — removes a preview Ingress, idempotent when missing.
DeleteSite — removes the production Ingress, idempotent when missing.
PublishSite — returns a not-implemented error; the deployment/service publish lands with the resource templates task (#55).
New(clientset, namespace, ingressClass, baseDomain) takes an injected clientset for testability; var _ KubernetesService = (*Client)(nil) assertion.

Config: added base_domain and ingress_class to KubernetesConfig (defaults pages.example.com / nginx).

Dependencies: promoted k8s.io/api and k8s.io/apimachinery to direct deps; vendored the client-go fake package (needed for tests).

Tests: client-go fake.NewSimpleClientset() (no cluster needed) — default vs custom host, ingress class, backend service, preview path/backend, upsert behavior, delete idempotency, and PublishSite not-implemented.

Why

Phase 4 task #51 — the Kubernetes layer used by site registration (#67), the webhook receiver (#73), and preview deployments (Phase 6).

Testing

go build ./...
go vet ./...
go test -race ./internal/kubernetes/ — 9 tests pass
gofmt -l ./cmd ./internal ./pkg clean
golangci-lint run — 0 issues
pre-commit hooks all pass

Breaking Changes

None.

Notes

PublishSite intentionally returns not-implemented until the Kubernetes resource templates task (#55) defines the Deployment/Service templates.
Issues #47–#51 were closed via the Forgejo API as part of this task per the workflow.

Closes #47
Closes #48
Closes #49
Closes #50
Closes #51

## What Implement the `KubernetesService` interface from task #32 on `Client` in `internal/kubernetes` using client-go `networking/v1` Ingress resources: - **CreateSiteIngress (#47, #48)** — creates or updates the production Ingress (upsert). Host = the site's `CustomDomain` when set, otherwise `<name>.<base_domain>`; path `/` → backend service `<name>`; sets `ingressClassName`. - **CreatePreviewIngress (#49)** — creates an Ingress routing `/preview/<branch>` to the `<name>-<branch>` service on the site host (path-based routing). - **DeletePreview (#50)** — removes a preview Ingress, idempotent when missing. - **DeleteSite** — removes the production Ingress, idempotent when missing. - **PublishSite** — returns a not-implemented error; the deployment/service publish lands with the resource templates task (#55). - `New(clientset, namespace, ingressClass, baseDomain)` takes an injected clientset for testability; `var _ KubernetesService = (*Client)(nil)` assertion. **Config**: added `base_domain` and `ingress_class` to `KubernetesConfig` (defaults `pages.example.com` / `nginx`). **Dependencies**: promoted `k8s.io/api` and `k8s.io/apimachinery` to direct deps; vendored the client-go fake package (needed for tests). **Tests**: client-go `fake.NewSimpleClientset()` (no cluster needed) — default vs custom host, ingress class, backend service, preview path/backend, upsert behavior, delete idempotency, and `PublishSite` not-implemented. ## Why Phase 4 task #51 — the Kubernetes layer used by site registration (#67), the webhook receiver (#73), and preview deployments (Phase 6). ## Testing - [x] `go build ./...` - [x] `go vet ./...` - [x] `go test -race ./internal/kubernetes/` — 9 tests pass - [x] `gofmt -l ./cmd ./internal ./pkg` clean - [x] `golangci-lint run` — 0 issues - [x] pre-commit hooks all pass ## Breaking Changes None. ## Notes - `PublishSite` intentionally returns not-implemented until the Kubernetes resource templates task (#55) defines the Deployment/Service templates. - Issues #47–#51 were closed via the Forgejo API as part of this task per the workflow. Closes #47 Closes #48 Closes #49 Closes #50 Closes #51

automation added 1 commit

2026-08-02 17:54:25 +00:00

feat(kubernetes): implement Kubernetes client wrapper

Test and Release / test (pull_request) Successful in 27s

Details

Test and Release / lint (pull_request) Successful in 27s

Details

afea3b2cc6

Implement the KubernetesService interface on Client using client-go
networking/v1 Ingress resources:

- CreateSiteIngress creates or updates the production Ingress, using
  the site's custom domain when set, otherwise <name>.<base_domain>,
  routing path / to the <name> service
- CreatePreviewIngress creates an Ingress routing /preview/<branch>
  to the <name>-<branch> service on the site host
- DeletePreview and DeleteSite remove Ingresses, idempotent on missing
- PublishSite returns not-implemented until the resource templates task
- New takes an injected clientset for testability; fake clientset
  tests cover host selection, preview path/backend, deletes, and
  upsert behavior

Add base_domain and ingress_class to KubernetesConfig. Vendor the
client-go fake package and promote k8s.io/api and apimachinery to
direct dependencies. Mark task done in ROADMAP.md.

closes #47
closes #48
closes #49
closes #50
closes #51

automation added the

Kind/Feature

label

2026-08-02 17:54:30 +00:00

the.auditor requested changes

2026-08-02 17:58:19 +00:00

Dismissed

the.auditor left a comment

Review Summary

Verified locally on the PR head (afea3b2): go build, go vet, go mod verify, gofmt (incl. ./pkg), golangci-lint run all clean; all 9 kubernetes tests pass with -race. The injected-clientset design and upsert/idempotent-delete patterns are correct, and config wiring (base_domain, ingress_class) is consistent with defaults and tests.

Blocking Issue

internal/kubernetes/kubernetes.go — preview resource names embed the raw branch name, producing invalid Kubernetes object names for the test/* branch pattern this feature targets (roadmap task #49). For branch test/foo:
- Ingress name becomes blog-preview-test/foo (previewIngressName, used by both CreatePreviewIngress and DeletePreview).
- Backend Service name becomes blog-test/foo (previewIngress → serviceBackend).
  Both fail validation.IsDNS1123Subdomain (verified empirically — the / is invalid). The fake clientset performs no name validation, so the tests pass while a real API server rejects the create.
Fix: sanitize the branch when deriving resource names — e.g. replace / (and any other non-[a-z0-9-] characters) with - in previewIngressName and the preview service name. The ingress path (/preview/test/foo) can keep the raw branch and is fine. Add a test using a slashed branch like test/foo.

Non-blocking Suggestions

stringPtr/pathTypePtr reinvent ptr.To (stdlib, Go 1.18+) — minor cleanup.
CreateSiteIngress/CreatePreviewIngress set no labels or owner references on the Ingress — fine now, but labels would help future cleanup/selection.
Site-derived host names (<name>.<base_domain>) and ingress names assume site.Name is already DNS-safe — worth a validation note at registration time (Phase 5).

The blocking item is a small fix in the name-derivation helpers.

## Review Summary Verified locally on the PR head (`afea3b2`): `go build`, `go vet`, `go mod verify`, `gofmt` (incl. `./pkg`), `golangci-lint run` all clean; all 9 kubernetes tests pass with `-race`. The injected-clientset design and upsert/idempotent-delete patterns are correct, and config wiring (`base_domain`, `ingress_class`) is consistent with defaults and tests. ## Blocking Issue 1. `internal/kubernetes/kubernetes.go` — preview resource names embed the raw branch name, producing invalid Kubernetes object names for the `test/*` branch pattern this feature targets (roadmap task #49). For branch `test/foo`: - Ingress name becomes `blog-preview-test/foo` (`previewIngressName`, used by both `CreatePreviewIngress` and `DeletePreview`). - Backend Service name becomes `blog-test/foo` (`previewIngress` → `serviceBackend`). Both fail `validation.IsDNS1123Subdomain` (verified empirically — the `/` is invalid). The fake clientset performs no name validation, so the tests pass while a real API server rejects the create. **Fix:** sanitize the branch when deriving resource names — e.g. replace `/` (and any other non-`[a-z0-9-]` characters) with `-` in `previewIngressName` and the preview service name. The ingress *path* (`/preview/test/foo`) can keep the raw branch and is fine. Add a test using a slashed branch like `test/foo`. ## Non-blocking Suggestions 1. `stringPtr`/`pathTypePtr` reinvent `ptr.To` (stdlib, Go 1.18+) — minor cleanup. 2. `CreateSiteIngress`/`CreatePreviewIngress` set no labels or owner references on the Ingress — fine now, but labels would help future cleanup/selection. 3. Site-derived host names (`<name>.<base_domain>`) and ingress names assume `site.Name` is already DNS-safe — worth a validation note at registration time (Phase 5). The blocking item is a small fix in the name-derivation helpers.

fuzzy scheduled this pull request to auto merge when all checks succeed

2026-08-02 18:09:04 +00:00

fuzzy added 1 commit

2026-08-02 18:13:43 +00:00

fix(kubernetes): sanitize branch names in resource names

Test and Release / lint (pull_request) Successful in 7m2s

Details

Test and Release / test (pull_request) Successful in 7m8s

Details

3d2323e268

Preview resource names derived from branches embedded the raw branch
name, producing invalid Kubernetes object names for the test/* branch
pattern (e.g. test/foo -> blog-preview-test/foo). Sanitize the branch
when building the Ingress name and backend Service name, replacing
characters outside [a-z0-9-] with '-', while preserving the raw branch
in the routing path (/preview/test/foo).

Replace the stringPtr/pathTypePtr helpers with ptr.To. Add regression
tests for slashed branches covering create, backend naming, delete,
and the sanitizer itself.

automation commented

2026-08-02 18:13:49 +00:00

Author

Owner

Review changes addressed in 3d2323e:

Blocking issue — added sanitizeName, which replaces characters outside [a-z0-9-] with - (and lowercases), applied when deriving the preview Ingress name and backend Service name. The routing path still preserves the raw branch (/preview/test/foo). Regression tests added: TestCreatePreviewIngressSlashedBranch, TestDeletePreviewSlashedBranch, and TestSanitizeName.

Non-blocking #1 — replaced stringPtr/pathTypePtr helpers with k8s.io/utils/ptr.To.

Verified: go build, go vet, gofmt, golangci-lint run clean; all 11 kubernetes tests pass with -race. k8s.io/utils promoted to a direct dependency.

Review changes addressed in `3d2323e`: **Blocking issue** — added `sanitizeName`, which replaces characters outside `[a-z0-9-]` with `-` (and lowercases), applied when deriving the preview Ingress name and backend Service name. The routing path still preserves the raw branch (`/preview/test/foo`). Regression tests added: `TestCreatePreviewIngressSlashedBranch`, `TestDeletePreviewSlashedBranch`, and `TestSanitizeName`. **Non-blocking #1** — replaced `stringPtr`/`pathTypePtr` helpers with `k8s.io/utils/ptr.To`. Verified: `go build`, `go vet`, `gofmt`, `golangci-lint run` clean; all 11 kubernetes tests pass with `-race`. `k8s.io/utils` promoted to a direct dependency.

the.auditor approved these changes

2026-08-02 18:15:37 +00:00

the.auditor left a comment

Review Summary

The blocking issue from the previous review is resolved.

Blocker fixed — sanitizeName now lowercases and replaces any character outside [a-z0-9-] with -, applied to both previewIngressName and the preview backend Service name. Verified: branch test/foo now yields valid k8s object names blog-preview-test-foo and blog-test-foo (pass IsDNS1123Subdomain), while the ingress path /preview/test/foo keeps the raw branch for routing.
Regression covered — new TestCreatePreviewIngressSlashedBranch, TestDeletePreviewSlashedBranch, and table-driven TestSanitizeName exercise the previously-broken path.
ptr.To cleanup — stringPtr/pathTypePtr replaced with stdlib-style ptr.To (k8s.io/utils promoted to a direct dep), as suggested.

Verified locally on the PR head (3d2323e): go build, go vet, go mod verify, gofmt, golangci-lint run all clean; all 10 kubernetes tests pass with -race.

No blocking issues. Approving.

## Review Summary The blocking issue from the previous review is resolved. 1. **Blocker fixed** — `sanitizeName` now lowercases and replaces any character outside `[a-z0-9-]` with `-`, applied to both `previewIngressName` and the preview backend Service name. Verified: branch `test/foo` now yields valid k8s object names `blog-preview-test-foo` and `blog-test-foo` (pass `IsDNS1123Subdomain`), while the ingress path `/preview/test/foo` keeps the raw branch for routing. 2. **Regression covered** — new `TestCreatePreviewIngressSlashedBranch`, `TestDeletePreviewSlashedBranch`, and table-driven `TestSanitizeName` exercise the previously-broken path. 3. **`ptr.To` cleanup** — `stringPtr`/`pathTypePtr` replaced with stdlib-style `ptr.To` (k8s.io/utils promoted to a direct dep), as suggested. Verified locally on the PR head (`3d2323e`): `go build`, `go vet`, `go mod verify`, `gofmt`, `golangci-lint run` all clean; all 10 kubernetes tests pass with `-race`. **No blocking issues.** Approving.

fuzzy merged commit 3d2323e268 into main

2026-08-02 18:20:55 +00:00

fuzzy deleted branch feat/kubernetes-wrapper

2026-08-02 18:20:56 +00:00