Skip to content

feat: add github and linear webhook events#34

Merged
knechtionscoding merged 5 commits intoprodfrom
feat/github-linear-webhook-events
Mar 30, 2026
Merged

feat: add github and linear webhook events#34
knechtionscoding merged 5 commits intoprodfrom
feat/github-linear-webhook-events

Conversation

@knechtionscoding
Copy link
Copy Markdown
Collaborator

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
Copy link
Copy Markdown

greptile-apps bot commented Mar 30, 2026

Greptile Summary

This PR adds GitHub and Linear webhook event sources to Kelos, introducing a new kelos-webhook-server binary, two new CRD fields (githubWebhook and linearWebhook on TaskSpawnerSpec.When), per-source Helm Deployments/Services, HMAC-SHA256 signature validation, and a flexible filter system for event type, action, labels, state, and draft status.

The overall architecture is well-structured — per-source servers provide fault isolation, idempotency is tracked via a delivery cache, and filter semantics (OR within type, AND across filter fields) are correctly documented and implemented. The signature validation and Linear filter logic in particular are clean and well-tested.

Issues found:

  • P1ServeHTTP does not type-assert MaxConcurrencyError from processWebhook, so it always returns HTTP 500 instead of 503 with Retry-After when concurrency limits are exceeded. GitHub/Linear retry logic won't be triggered correctly.
  • P1spawner.APIVersion and spawner.Kind will be empty strings for objects returned by client.List() in controller-runtime. The owner references set on created tasks are therefore invalid, breaking garbage collection. Controller and BlockOwnerDeletion fields are also missing.
  • P1 — TOCTOU race between DeliveryCache.IsProcessed and MarkProcessed allows two concurrent requests with the same delivery ID to both pass the idempotency check, potentially creating duplicate tasks.
  • P2 — Task name truncation at 63 characters can produce names ending with -, which are rejected by the Kubernetes API.
  • P2 — The BodyContains filter is silently ignored for IssuesEvent (only applied to IssueCommentEvent), so issue body filtering never takes effect.

Confidence Score: 4/5

Three P1 bugs in handler.go should be resolved before merging — concurrency response code, broken owner references, and a delivery-ID race condition.

The new feature is architecturally sound and well-tested, but handler.go has three distinct P1 defects that affect observable production behavior: wrong HTTP status for concurrency limits, broken task ownership (garbage collection won't work), and a race condition that can produce duplicate tasks under concurrent retries. These are straightforward to fix but real enough to block merge.

internal/webhook/handler.go requires the most attention for all three P1 issues; internal/webhook/github_filter.go has a P2 silent-filter gap for IssuesEvent BodyContains.

Important Files Changed

Filename Overview
internal/webhook/handler.go Core webhook dispatcher — three P1 bugs: MaxConcurrencyError returns 500 instead of 503, owner reference APIVersion/Kind are empty from List(), and a TOCTOU race in idempotency check
internal/webhook/github_filter.go GitHub event parsing and filter logic; BodyContains filter is silently ignored for IssuesEvent (P2); all other filter paths look correct
internal/webhook/linear_filter.go Linear event parsing and filter logic; state, label, and excludeLabel filters are well-implemented with correct null handling
internal/webhook/signature.go HMAC-SHA256 signature validation for GitHub and Linear using constant-time comparison; implementation is correct
api/v1alpha1/taskspawner_types.go Adds GitHubWebhook and LinearWebhook CRD types with appropriate kubebuilder validation markers; XValidation rule updated correctly
cmd/kelos-webhook-server/main.go Webhook server entrypoint with graceful shutdown; correctly uses ctrl.SetupSignalHandler and context-based cancellation

Sequence Diagram

sequenceDiagram
    participant GH as GitHub/Linear
    participant WH as WebhookHandler (ServeHTTP)
    participant SIG as signature.go (ValidateSignature)
    participant DC as DeliveryCache
    participant PW as processWebhook
    participant K8S as Kubernetes API (client.List / client.Create)
    participant TB as TaskBuilder

    GH->>WH: POST /webhook (event headers + body)
    WH->>SIG: ValidateGitHubSignature / ValidateLinearSignature
    SIG-->>WH: ok / err
    WH->>DC: IsProcessed(deliveryID)
    DC-->>WH: false (not yet seen)
    WH->>PW: processWebhook(eventType, body)
    PW->>K8S: client.List(TaskSpawners)
    K8S-->>PW: []TaskSpawner
    loop For each matching TaskSpawner
        PW->>PW: matchesSpawner (MatchesGitHubEvent / MatchesLinearEvent)
        PW->>TB: BuildTask(templateVars)
        TB-->>PW: *Task
        PW->>K8S: client.Create(Task)
    end
    PW-->>WH: (tasksCreated, nil)
    WH->>DC: MarkProcessed(deliveryID)
    WH-->>GH: HTTP 200 OK
Loading

Reviews (1): Last reviewed commit: "feat: add github and linear webhook even..." | Re-trigger Greptile

1. P1 - MaxConcurrencyError HTTP Response ✅
Fixed ServeHTTP to properly handle MaxConcurrencyError with HTTP 503 and Retry-After header
Previously returned generic HTTP 500, now correctly returns 503 for rate limiting
2. P1 - Empty APIVersion/Kind in Owner References ✅
Fixed owner reference creation to use client.Scheme().ObjectKinds() to get proper GVK
Added missing Controller and BlockOwnerDeletion fields for proper garbage collection
Previously had empty strings, now has correct API version and kind
3. P1 - TOCTOU Race Condition in Idempotency ✅
Replaced separate IsProcessed()/MarkProcessed() with atomic CheckAndMark()
Eliminated race window where two requests with same delivery ID could both pass the check
Now uses single lock operation for check-and-set
4. P2 - Invalid Kubernetes Names from Truncation ✅
Fixed task name generation to handle nil/empty ID values safely
Added strings.TrimRight(taskName[:63], "-.") to ensure names don't end with invalid characters
Prevents server-side validation failures
5. P2 - BodyContains Filter Ignored for IssuesEvent ✅
Fixed GitHub filter to check issue body for BodyContains on IssuesEvent
Previously only checked comment body on IssueCommentEvent
Now properly filters by issue body content
All tests pass and the implementation builds successfully. The webhook system now properly handles rate limiting, ensures atomic idempotency checks, creates valid owner references for garbage collection, generates valid Kubernetes resource names, and correctly applies all filter conditions.
@knechtionscoding knechtionscoding merged commit ff4fd01 into prod Mar 30, 2026
7 of 8 checks passed
@knechtionscoding knechtionscoding deleted the feat/github-linear-webhook-events branch March 30, 2026 10:34
knechtionscoding added a commit that referenced this pull request Mar 30, 2026
* feat: add github and linear webhook events

* fix: build images

* Fixed Issues
1. P1 - MaxConcurrencyError HTTP Response ✅
Fixed ServeHTTP to properly handle MaxConcurrencyError with HTTP 503 and Retry-After header
Previously returned generic HTTP 500, now correctly returns 503 for rate limiting
2. P1 - Empty APIVersion/Kind in Owner References ✅
Fixed owner reference creation to use client.Scheme().ObjectKinds() to get proper GVK
Added missing Controller and BlockOwnerDeletion fields for proper garbage collection
Previously had empty strings, now has correct API version and kind
3. P1 - TOCTOU Race Condition in Idempotency ✅
Replaced separate IsProcessed()/MarkProcessed() with atomic CheckAndMark()
Eliminated race window where two requests with same delivery ID could both pass the check
Now uses single lock operation for check-and-set
4. P2 - Invalid Kubernetes Names from Truncation ✅
Fixed task name generation to handle nil/empty ID values safely
Added strings.TrimRight(taskName[:63], "-.") to ensure names don't end with invalid characters
Prevents server-side validation failures
5. P2 - BodyContains Filter Ignored for IssuesEvent ✅
Fixed GitHub filter to check issue body for BodyContains on IssuesEvent
Previously only checked comment body on IssueCommentEvent
Now properly filters by issue body content
All tests pass and the implementation builds successfully. The webhook system now properly handles rate limiting, ensures atomic idempotency checks, creates valid owner references for garbage collection, generates valid Kubernetes resource names, and correctly applies all filter conditions.

* chore: generate
knechtionscoding added a commit that referenced this pull request Mar 30, 2026
* feat: add github and linear webhook events

* fix: build images

* Fixed Issues
1. P1 - MaxConcurrencyError HTTP Response ✅
Fixed ServeHTTP to properly handle MaxConcurrencyError with HTTP 503 and Retry-After header
Previously returned generic HTTP 500, now correctly returns 503 for rate limiting
2. P1 - Empty APIVersion/Kind in Owner References ✅
Fixed owner reference creation to use client.Scheme().ObjectKinds() to get proper GVK
Added missing Controller and BlockOwnerDeletion fields for proper garbage collection
Previously had empty strings, now has correct API version and kind
3. P1 - TOCTOU Race Condition in Idempotency ✅
Replaced separate IsProcessed()/MarkProcessed() with atomic CheckAndMark()
Eliminated race window where two requests with same delivery ID could both pass the check
Now uses single lock operation for check-and-set
4. P2 - Invalid Kubernetes Names from Truncation ✅
Fixed task name generation to handle nil/empty ID values safely
Added strings.TrimRight(taskName[:63], "-.") to ensure names don't end with invalid characters
Prevents server-side validation failures
5. P2 - BodyContains Filter Ignored for IssuesEvent ✅
Fixed GitHub filter to check issue body for BodyContains on IssuesEvent
Previously only checked comment body on IssueCommentEvent
Now properly filters by issue body content
All tests pass and the implementation builds successfully. The webhook system now properly handles rate limiting, ensures atomic idempotency checks, creates valid owner references for garbage collection, generates valid Kubernetes resource names, and correctly applies all filter conditions.

* chore: generate
knechtionscoding added a commit that referenced this pull request Mar 30, 2026
* feat: add github and linear webhook events

* fix: build images

* Fixed Issues
1. P1 - MaxConcurrencyError HTTP Response ✅
Fixed ServeHTTP to properly handle MaxConcurrencyError with HTTP 503 and Retry-After header
Previously returned generic HTTP 500, now correctly returns 503 for rate limiting
2. P1 - Empty APIVersion/Kind in Owner References ✅
Fixed owner reference creation to use client.Scheme().ObjectKinds() to get proper GVK
Added missing Controller and BlockOwnerDeletion fields for proper garbage collection
Previously had empty strings, now has correct API version and kind
3. P1 - TOCTOU Race Condition in Idempotency ✅
Replaced separate IsProcessed()/MarkProcessed() with atomic CheckAndMark()
Eliminated race window where two requests with same delivery ID could both pass the check
Now uses single lock operation for check-and-set
4. P2 - Invalid Kubernetes Names from Truncation ✅
Fixed task name generation to handle nil/empty ID values safely
Added strings.TrimRight(taskName[:63], "-.") to ensure names don't end with invalid characters
Prevents server-side validation failures
5. P2 - BodyContains Filter Ignored for IssuesEvent ✅
Fixed GitHub filter to check issue body for BodyContains on IssuesEvent
Previously only checked comment body on IssueCommentEvent
Now properly filters by issue body content
All tests pass and the implementation builds successfully. The webhook system now properly handles rate limiting, ensures atomic idempotency checks, creates valid owner references for garbage collection, generates valid Kubernetes resource names, and correctly applies all filter conditions.

* chore: generate
knechtionscoding added a commit that referenced this pull request Mar 30, 2026
* feat: add github and linear webhook events

* fix: build images

* Fixed Issues
1. P1 - MaxConcurrencyError HTTP Response ✅
Fixed ServeHTTP to properly handle MaxConcurrencyError with HTTP 503 and Retry-After header
Previously returned generic HTTP 500, now correctly returns 503 for rate limiting
2. P1 - Empty APIVersion/Kind in Owner References ✅
Fixed owner reference creation to use client.Scheme().ObjectKinds() to get proper GVK
Added missing Controller and BlockOwnerDeletion fields for proper garbage collection
Previously had empty strings, now has correct API version and kind
3. P1 - TOCTOU Race Condition in Idempotency ✅
Replaced separate IsProcessed()/MarkProcessed() with atomic CheckAndMark()
Eliminated race window where two requests with same delivery ID could both pass the check
Now uses single lock operation for check-and-set
4. P2 - Invalid Kubernetes Names from Truncation ✅
Fixed task name generation to handle nil/empty ID values safely
Added strings.TrimRight(taskName[:63], "-.") to ensure names don't end with invalid characters
Prevents server-side validation failures
5. P2 - BodyContains Filter Ignored for IssuesEvent ✅
Fixed GitHub filter to check issue body for BodyContains on IssuesEvent
Previously only checked comment body on IssueCommentEvent
Now properly filters by issue body content
All tests pass and the implementation builds successfully. The webhook system now properly handles rate limiting, ensures atomic idempotency checks, creates valid owner references for garbage collection, generates valid Kubernetes resource names, and correctly applies all filter conditions.

* chore: generate
knechtionscoding added a commit that referenced this pull request Mar 30, 2026
* feat: add github and linear webhook events

* fix: build images

* Fixed Issues
1. P1 - MaxConcurrencyError HTTP Response ✅
Fixed ServeHTTP to properly handle MaxConcurrencyError with HTTP 503 and Retry-After header
Previously returned generic HTTP 500, now correctly returns 503 for rate limiting
2. P1 - Empty APIVersion/Kind in Owner References ✅
Fixed owner reference creation to use client.Scheme().ObjectKinds() to get proper GVK
Added missing Controller and BlockOwnerDeletion fields for proper garbage collection
Previously had empty strings, now has correct API version and kind
3. P1 - TOCTOU Race Condition in Idempotency ✅
Replaced separate IsProcessed()/MarkProcessed() with atomic CheckAndMark()
Eliminated race window where two requests with same delivery ID could both pass the check
Now uses single lock operation for check-and-set
4. P2 - Invalid Kubernetes Names from Truncation ✅
Fixed task name generation to handle nil/empty ID values safely
Added strings.TrimRight(taskName[:63], "-.") to ensure names don't end with invalid characters
Prevents server-side validation failures
5. P2 - BodyContains Filter Ignored for IssuesEvent ✅
Fixed GitHub filter to check issue body for BodyContains on IssuesEvent
Previously only checked comment body on IssueCommentEvent
Now properly filters by issue body content
All tests pass and the implementation builds successfully. The webhook system now properly handles rate limiting, ensures atomic idempotency checks, creates valid owner references for garbage collection, generates valid Kubernetes resource names, and correctly applies all filter conditions.

* chore: generate
* fix: normalize the tasknames
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant