Merge Main to Prod

[knechtionscoding]

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR is related to:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

[greptile-apps]

Greptile Summary

This PR merges a substantial set of features from main into prod, including: GitHub webhook-driven task spawning, per-workspace ghproxy Deployments, a shared taskbuilder package, and a refactored ghproxy that uses a fixed single upstream with its own token resolver instead of a client-supplied allowlist.

Key changes:

• New GitHubWebhook source type in the TaskSpawner API — a new kelos-webhook-server binary listens for GitHub webhooks, filters events against all TaskSpawner resources with when.githubWebhook, and creates Task objects. Signature validation (HMAC-SHA256), idempotency via an in-memory delivery cache, and comprehensive event-type filters are all present.
• Per-workspace ghproxy — the WorkspaceReconciler now provisions a dedicated ghproxy Deployment + Service per Workspace.
• ghproxy refactor — simplified to a single fixed upstream with hot-reloading token file support.
• taskbuilder package — task construction extracted into a shared package used by both the spawner and webhook handler.
• Owner references on spawned Tasks — enabling accurate activeTasks counting for webhook-based concurrency control.

One P1 issue found: the README documents HTTP 503 when maxConcurrency is exceeded, but the code silently drops the event and returns 200, causing permanent event loss under load.

Confidence Score: 4/5

Safe to merge after resolving the max-concurrency 200-vs-503 discrepancy.

One P1 finding: when max concurrency is exceeded the handler returns 200 OK instead of 503, so GitHub considers the delivery successful and will not retry — causing silent event loss. All other findings are P2 style/cleanup. The rest of the PR is well-structured with good test coverage.

internal/webhook/handler.go — max concurrency path returns 200 instead of 503.

Important Files Changed

Filename	Overview
internal/webhook/handler.go	New webhook handler: max concurrency check silently drops events with 200 instead of 503 as documented; idempotency, signature validation, and task creation paths are otherwise correct.
internal/webhook/github_filter.go	New GitHub webhook event parser and filter evaluator; comprehensive coverage of issue/PR/push event types, branch glob matching, label filtering.
internal/webhook/signature.go	New HMAC-SHA256 GitHub signature validation; uses constant-time comparison via hmac.Equal — correct and secure.
cmd/kelos-webhook-server/main.go	New webhook server binary; proper timeouts, graceful shutdown, health/readiness probes, and leader-election wired up correctly.
cmd/ghproxy/main.go	Refactored from allowlist-based multi-upstream to single fixed upstream with per-instance token resolver; security surface reduced, cache key simplified, token file hot-reloading added.
internal/controller/workspace_ghproxy_builder.go	New workspace-scoped ghproxy builder; uses SHA1 for name truncation (minor style issue); otherwise correctly wires token refresher sidecar and deployment/service resources.
internal/controller/workspace_controller.go	New Workspace reconciler that manages per-workspace ghproxy Deployment and Service; controller references, ownership, and requeue logic are correct.
internal/controller/taskspawner_controller.go	Added webhook-based TaskSpawner reconciliation path, Task watch for active-count updates, and safety guard in deleteStaleResource to check ownership before deletion.
internal/taskbuilder/builder.go	New shared task construction helper; consolidates template rendering, label/annotation rendering, and owner reference into one place.
api/v1alpha1/taskspawner_types.go	Added GitHubWebhook and GitHubWebhookFilter API types, updated CEL validation rule.
cmd/ghproxy/main_test.go	Test suite updated for new single-upstream proxy; one test still sets the now-ignored UpstreamBaseURLHeader.
cmd/kelos-spawner/main.go	Spawner now accepts --gh-proxy-url flag; task construction delegated to shared taskbuilder; owner references added to spawned Tasks.

Sequence Diagram

sequenceDiagram
    participant GH as GitHub
    participant WS as kelos-webhook-server
    participant K8s as Kubernetes API
    participant Ctrl as kelos-controller
    participant Proxy as workspace-ghproxy
    participant Spawner as kelos-spawner

    GH->>WS: POST /webhook (X-Hub-Signature-256)
    WS->>WS: ValidateGitHubSignature (HMAC-SHA256)
    WS->>WS: CheckAndMark deliveryID (idempotency)
    WS->>K8s: List TaskSpawners with githubWebhook
    K8s-->>WS: matching spawners
    WS->>WS: MatchesGitHubEvent (filter evaluation)
    WS->>K8s: Create Task (owner ref to TaskSpawner)
    K8s-->>WS: 201 Created
    WS-->>GH: 200 OK

    K8s-->>Ctrl: Task status change
    Ctrl->>K8s: Update TaskSpawner.status.activeTasks

    Spawner->>Proxy: GET /repos/.../issues (no token)
    Proxy->>Proxy: tokenResolver() reads token file
    Proxy->>GH: GET api.github.com (Authorization: token)
    GH-->>Proxy: 200
    Proxy-->>Spawner: 200 (cached response)

Loading

_{Reviews (1): Last reviewed commit: "Merge pull request #854 from kelos-dev/u..." | Re-trigger Greptile}

[greptile-apps]

Max concurrency silently drops events instead of returning 503

The README.md for the webhook example explicitly states: "When exceeded, returns HTTP 503 with Retry-After header — GitHub will automatically retry failed webhook deliveries."

However, the code silently continues and returns HTTP 200 (success) when all spawners are over the concurrency limit (processWebhook returns (false, nil)). GitHub will interpret a 200 as successful delivery and will not retry the event, causing permanent, silent event loss.

At minimum, update the README to accurately reflect the current 200 behaviour, or update the code to match the documented 503 behaviour.

[greptile-apps]

Leftover UpstreamBaseURLHeader has no effect after proxy refactor

The proxy was refactored to use a single fixed upstreamBaseURL configured at construction time; the X-Upstream-Base-URL header is now completely ignored in ServeHTTP. This header set here is dead code and can mislead future readers into thinking it still controls routing.

Suggested change

	req.Header.Set(source.UpstreamBaseURLHeader, upstream.URL)
	req, _ := http.NewRequest("GET", proxyServer.URL+"/repos/owner/repo/issues", nil)

[greptile-apps]

SHA1 used for name truncation hashing

SHA1 is cryptographically broken. While the usage here is purely for generating deterministic short suffixes (not security-sensitive), the rest of the codebase uses SHA256 (e.g., delivery-ID hashing in handler.go). Prefer sha256 for consistency:

sum := sha256.Sum256([]byte(name))
suffix := hex.EncodeToString(sum[:])[:8]