diff --git a/.github/workflows/deploy-dev.yaml b/.github/workflows/deploy-dev.yaml index aa25d525..cbe7d49a 100644 --- a/.github/workflows/deploy-dev.yaml +++ b/.github/workflows/deploy-dev.yaml @@ -53,14 +53,15 @@ jobs: run: | bin/kelos install --version main --image-pull-policy Always \ --spawner-resource-requests cpu=100m,memory=128Mi \ + --ghproxy-resource-requests cpu=10m,memory=64Mi \ --token-refresher-resource-requests cpu=50m,memory=64Mi \ --controller-resource-requests cpu=10m,memory=64Mi \ --controller-resource-limits cpu=500m,memory=128Mi kubectl rollout restart deployment/kelos-controller-manager -n kelos-system kubectl rollout status deployment/kelos-controller-manager -n kelos-system --timeout=120s - kubectl rollout restart deployment/ghproxy -n kelos-system - kubectl rollout status deployment/ghproxy -n kelos-system --timeout=120s + kubectl rollout restart deployment -l kelos.dev/component=ghproxy -n "${KELOS_NAMESPACE}" kubectl rollout restart deployment -l kelos.dev/component=spawner -n "${KELOS_NAMESPACE}" + kubectl rollout status deployment -l kelos.dev/component=ghproxy -n "${KELOS_NAMESPACE}" --timeout=120s kubectl rollout status deployment -l kelos.dev/component=spawner -n "${KELOS_NAMESPACE}" --timeout=120s - name: Apply PodMonitoring @@ -107,13 +108,15 @@ jobs: kind: PodMonitoring metadata: name: ghproxy - namespace: kelos-system + namespace: ${KELOS_NAMESPACE} labels: - app.kubernetes.io/name: ghproxy + kelos.dev/name: kelos + kelos.dev/component: ghproxy spec: selector: matchLabels: - app.kubernetes.io/name: ghproxy + kelos.dev/name: kelos + kelos.dev/component: ghproxy endpoints: - port: metrics interval: 30s diff --git a/.github/workflows/reusable-e2e.yaml b/.github/workflows/reusable-e2e.yaml index 5e90be91..10700824 100644 --- a/.github/workflows/reusable-e2e.yaml +++ b/.github/workflows/reusable-e2e.yaml @@ -44,6 +44,7 @@ jobs: with: ref: ${{ inputs.checkout-ref }} persist-credentials: ${{ inputs.persist-credentials }} + fetch-depth: 2 - name: Validate checked-out commit matches approved SHA if: inputs.expected-sha != '' diff --git a/Makefile b/Makefile index 6da54c58..63488e6d 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # Image configuration REGISTRY ?= ghcr.io/kelos-dev VERSION ?= latest -IMAGE_DIRS ?= cmd/kelos-controller cmd/kelos-spawner cmd/kelos-token-refresher cmd/ghproxy claude-code codex gemini opencode cursor +IMAGE_DIRS ?= cmd/kelos-controller cmd/kelos-spawner cmd/kelos-token-refresher cmd/kelos-webhook-server cmd/ghproxy claude-code codex gemini opencode cursor # Version injection for the kelos CLI – only set ldflags when an explicit # version is given so that dev builds fall through to runtime/debug info. diff --git a/README.md b/README.md index fa17058a..8a6be801 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,7 @@ spec: priorityLabels: - priority/critical-urgent - priority/important-soon + pollInterval: 1m maxConcurrency: 3 taskTemplate: model: opus @@ -101,7 +102,6 @@ spec: - update an existing PR to fix the issue - comment on the issue or the PR if you cannot fix it ... - pollInterval: 1m ``` The key pattern is `excludeLabels: [kelos/needs-input]` — this creates a feedback loop where the agent works autonomously until it needs human input, then pauses. Removing the label re-queues the issue on the next poll. @@ -343,6 +343,7 @@ spec: githubIssues: labels: [bug] state: open + pollInterval: 5m taskTemplate: type: claude-code workspaceRef: @@ -352,7 +353,6 @@ spec: secretRef: name: claude-oauth-token promptTemplate: "Fix: {{.Title}}\n{{.Body}}" - pollInterval: 5m ``` ```bash diff --git a/api/v1alpha1/taskspawner_types.go b/api/v1alpha1/taskspawner_types.go index 7dc07719..2ba72b93 100644 --- a/api/v1alpha1/taskspawner_types.go +++ b/api/v1alpha1/taskspawner_types.go @@ -36,6 +36,10 @@ type When struct { // Jira discovers issues from a Jira project. // +optional Jira *Jira `json:"jira,omitempty"` + + // GitHubWebhook triggers task spawning on GitHub webhook events. + // +optional + GitHubWebhook *GitHubWebhook `json:"githubWebhook,omitempty"` } // Cron triggers task spawning on a cron schedule. @@ -295,6 +299,65 @@ type Jira struct { PollInterval string `json:"pollInterval,omitempty"` } +// GitHubWebhook configures webhook-driven task spawning from GitHub events. +type GitHubWebhook struct { + // Events is the list of GitHub event types to listen for. + // e.g., "issue_comment", "pull_request_review", "push", "issues" + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinItems=1 + Events []string `json:"events"` + + // Repository restricts webhooks to a specific repository (owner/repo format). + // If empty, webhooks from any repository are accepted. + // +optional + Repository string `json:"repository,omitempty"` + + // Filters refine which events trigger tasks. If multiple filters match + // the same event type, any match triggers a task (OR semantics). + // If empty, all events in the Events list trigger tasks. + // +optional + Filters []GitHubWebhookFilter `json:"filters,omitempty"` +} + +// GitHubWebhookFilter defines filtering criteria for GitHub webhook events. +type GitHubWebhookFilter struct { + // Event is the GitHub event type this filter applies to. + // +kubebuilder:validation:Required + Event string `json:"event"` + + // Action filters by webhook action (e.g., "created", "opened", "submitted"). + // +optional + Action string `json:"action,omitempty"` + + // BodyContains filters by substring match on the comment/review body. + // +optional + BodyContains string `json:"bodyContains,omitempty"` + + // Labels requires the issue/PR to have all of these labels. + // +optional + Labels []string `json:"labels,omitempty"` + + // ExcludeLabels excludes issues/PRs with any of these labels. + // +optional + ExcludeLabels []string `json:"excludeLabels,omitempty"` + + // State filters by issue/PR state ("open", "closed"). + // +optional + State string `json:"state,omitempty"` + + // Branch filters push events by branch name (exact match or glob). + // +optional + Branch string `json:"branch,omitempty"` + + // Draft filters PRs by draft status. nil = don't filter. + // +optional + Draft *bool `json:"draft,omitempty"` + + // Author filters by the event sender's username. + // +optional + Author string `json:"author,omitempty"` +} + // TaskTemplateMetadata holds optional labels and annotations for spawned Tasks. type TaskTemplateMetadata struct { // Labels are merged into the spawned Task's labels. Values support Go @@ -355,6 +418,7 @@ type TaskTemplate struct { // Available variables (all sources): {{.ID}}, {{.Title}}, {{.Kind}} // GitHub issue/Jira sources: {{.Number}}, {{.Body}}, {{.URL}}, {{.Labels}}, {{.Comments}} // GitHub pull request sources additionally expose: {{.Branch}}, {{.ReviewState}}, {{.ReviewComments}} + // GitHub webhook sources: {{.Event}}, {{.Action}}, {{.Sender}}, {{.Ref}}, {{.Payload}} (full payload access) // Cron sources: {{.Time}}, {{.Schedule}} // +optional Branch string `json:"branch,omitempty"` @@ -363,6 +427,7 @@ type TaskTemplate struct { // Available variables (all sources): {{.ID}}, {{.Title}}, {{.Kind}} // GitHub issue/Jira sources: {{.Number}}, {{.Body}}, {{.URL}}, {{.Labels}}, {{.Comments}} // GitHub pull request sources additionally expose: {{.Branch}}, {{.ReviewState}}, {{.ReviewComments}} + // GitHub webhook sources: {{.Event}}, {{.Action}}, {{.Sender}}, {{.Ref}}, {{.Payload}} (full payload access) // Cron sources: {{.Time}}, {{.Schedule}} // +optional PromptTemplate string `json:"promptTemplate,omitempty"` @@ -396,7 +461,7 @@ type TaskTemplate struct { } // TaskSpawnerSpec defines the desired state of TaskSpawner. -// +kubebuilder:validation:XValidation:rule="!(has(self.when.githubIssues) || has(self.when.githubPullRequests)) || has(self.taskTemplate.workspaceRef)",message="taskTemplate.workspaceRef is required when using githubIssues or githubPullRequests source" +// +kubebuilder:validation:XValidation:rule="!(has(self.when.githubIssues) || has(self.when.githubPullRequests) || has(self.when.githubWebhook)) || has(self.taskTemplate.workspaceRef)",message="taskTemplate.workspaceRef is required when using githubIssues, githubPullRequests, or githubWebhook source" type TaskSpawnerSpec struct { // When defines the conditions that trigger task spawning. // +kubebuilder:validation:Required diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 536ad3b2..6853a7cd 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -328,6 +328,63 @@ func (in *GitHubReporting) DeepCopy() *GitHubReporting { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GitHubWebhook) DeepCopyInto(out *GitHubWebhook) { + *out = *in + if in.Events != nil { + in, out := &in.Events, &out.Events + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Filters != nil { + in, out := &in.Filters, &out.Filters + *out = make([]GitHubWebhookFilter, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitHubWebhook. +func (in *GitHubWebhook) DeepCopy() *GitHubWebhook { + if in == nil { + return nil + } + out := new(GitHubWebhook) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GitHubWebhookFilter) DeepCopyInto(out *GitHubWebhookFilter) { + *out = *in + if in.Labels != nil { + in, out := &in.Labels, &out.Labels + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.ExcludeLabels != nil { + in, out := &in.ExcludeLabels, &out.ExcludeLabels + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Draft != nil { + in, out := &in.Draft, &out.Draft + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitHubWebhookFilter. +func (in *GitHubWebhookFilter) DeepCopy() *GitHubWebhookFilter { + if in == nil { + return nil + } + out := new(GitHubWebhookFilter) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GitRemote) DeepCopyInto(out *GitRemote) { *out = *in @@ -878,6 +935,11 @@ func (in *When) DeepCopyInto(out *When) { *out = new(Jira) **out = **in } + if in.GitHubWebhook != nil { + in, out := &in.GitHubWebhook, &out.GitHubWebhook + *out = new(GitHubWebhook) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new When. diff --git a/claude-code/Dockerfile b/claude-code/Dockerfile index 6b37a98d..f006958d 100644 --- a/claude-code/Dockerfile +++ b/claude-code/Dockerfile @@ -27,7 +27,7 @@ RUN ARCH=$(dpkg --print-architecture) \ ENV PATH="/usr/local/go/bin:${PATH}" -ARG CLAUDE_CODE_VERSION=2.1.87 +ARG CLAUDE_CODE_VERSION=2.1.88 RUN npm install -g @anthropic-ai/claude-code@${CLAUDE_CODE_VERSION} COPY claude-code/kelos_entrypoint.sh /kelos_entrypoint.sh diff --git a/cmd/ghproxy/main.go b/cmd/ghproxy/main.go index ba546f9b..c8e3a3df 100644 --- a/cmd/ghproxy/main.go +++ b/cmd/ghproxy/main.go @@ -12,6 +12,7 @@ import ( "sync" "time" + "github.com/go-logr/logr" "github.com/prometheus/client_golang/prometheus/promhttp" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -21,7 +22,7 @@ import ( ) const ( - // defaultUpstream is used when no header is provided. + // defaultUpstream is used when no upstream is explicitly configured. defaultUpstream = "https://api.github.com" // defaultCacheTTL is the default freshness window for cached GET responses. @@ -41,37 +42,39 @@ type cacheEntry struct { } type proxy struct { - mu sync.RWMutex - cache map[string]*cacheEntry - upstream *http.Client - allowedUpstreams map[string]bool - cacheTTL time.Duration - now func() time.Time + mu sync.RWMutex + cache map[string]*cacheEntry + upstream *http.Client + upstreamBaseURL string + cacheTTL time.Duration + now func() time.Time + tokenResolver func() string } -func newProxy(allowed []string, cacheTTL time.Duration) *proxy { - m := make(map[string]bool, len(allowed)) - for _, u := range allowed { - m[strings.TrimSuffix(strings.TrimSpace(u), "/")] = true +func newProxy(upstreamBaseURL string, cacheTTL time.Duration, tokenResolver func() string) *proxy { + if upstreamBaseURL == "" { + upstreamBaseURL = defaultUpstream + } + if tokenResolver == nil { + tokenResolver = func() string { return "" } } return &proxy{ cache: make(map[string]*cacheEntry), upstream: &http.Client{ Timeout: 30 * time.Second, }, - allowedUpstreams: m, - cacheTTL: cacheTTL, - now: time.Now, + upstreamBaseURL: strings.TrimSuffix(strings.TrimSpace(upstreamBaseURL), "/"), + cacheTTL: cacheTTL, + now: time.Now, + tokenResolver: tokenResolver, } } -// cacheKey returns a key that includes the upstream, request path+query, and -// Accept header so that the same path on different upstreams or with different -// content types is cached separately. Authorization is intentionally excluded -// so that spawners with different tokens share cached responses for the same -// repo, enabling cross-pod deduplication. -func cacheKey(upstream, pathAndQuery, accept string) string { - return upstream + "|" + pathAndQuery + "|" + accept +// cacheKey returns a key that includes the request path+query and Accept +// header so that the same path with different content types is cached +// separately. The upstream is fixed per proxy instance. +func cacheKey(pathAndQuery, accept string) string { + return pathAndQuery + "|" + accept } // rewriteLinkHeader rewrites absolute URLs in a Link header, replacing the @@ -102,112 +105,89 @@ func writeCachedResponse(w http.ResponseWriter, proxyBase, upstream string, entr w.Write(entry.body) } -func (p *proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { - log := ctrl.Log.WithName("ghproxy") - resource := source.ClassifyResource(r.URL.Path) - statusCode := http.StatusBadGateway - cacheResult := "skip" - defer func() { - githubAPIRequestsTotal.WithLabelValues(r.Method, strconv.Itoa(statusCode), resource, cacheResult).Inc() - }() - - upstream := r.Header.Get(source.UpstreamBaseURLHeader) - if upstream == "" { - upstream = defaultUpstream - } - upstream = strings.TrimSuffix(upstream, "/") - - if !p.allowedUpstreams[upstream] { - statusCode = http.StatusForbidden - http.Error(w, "Upstream not allowed", http.StatusForbidden) - log.Info("Rejected request for disallowed upstream", "upstream", upstream) - return - } +type responsePayload struct { + statusCode int + cacheResult string + contentType string + etag string + link string + body []byte + headers map[string]string +} - scheme := "http" - if proto := r.Header.Get("X-Forwarded-Proto"); proto != "" { - scheme = strings.Split(proto, ",")[0] - } else if r.TLS != nil { - scheme = "https" - } - proxyBase := scheme + "://" + r.Host - key := cacheKey(upstream, r.URL.RequestURI(), r.Header.Get("Accept")) +func (p *proxy) fetchResponse(log logr.Logger, upstream string, key string, r *http.Request) (*responsePayload, error) { var entry *cacheEntry if r.Method == http.MethodGet { p.mu.RLock() entry = p.cache[key] p.mu.RUnlock() if p.isFresh(entry) { - statusCode = entry.status - cacheResult = "fresh_hit" - writeCachedResponse(w, proxyBase, upstream, entry) - return + return &responsePayload{ + statusCode: entry.status, + cacheResult: "fresh_hit", + contentType: entry.contentType, + etag: entry.etag, + link: entry.link, + body: entry.body, + headers: map[string]string{}, + }, nil } } - // Build the upstream URL by combining the upstream base with the - // incoming request path and query string. target, err := url.Parse(upstream + r.URL.RequestURI()) if err != nil { - http.Error(w, "Bad upstream URL", http.StatusBadGateway) - log.Error(err, "Failed to parse upstream URL", "upstream", upstream, "path", r.URL.RequestURI()) - return + return nil, fmt.Errorf("parsing upstream URL: %w", err) } outReq, err := http.NewRequestWithContext(r.Context(), r.Method, target.String(), r.Body) if err != nil { - http.Error(w, "Failed to create request", http.StatusInternalServerError) - log.Error(err, "Failed to create upstream request") - return + return nil, fmt.Errorf("creating upstream request: %w", err) } - // Copy relevant headers from the original request. - for _, h := range []string{"Accept", "Authorization", "User-Agent"} { + for _, h := range []string{"Accept", "User-Agent"} { if v := r.Header.Get(h); v != "" { outReq.Header.Set(h, v) } } - - // For GET requests, attach a cached ETag as If-None-Match so the - // upstream can return 304 when nothing changed. + if token := p.tokenResolver(); token != "" { + outReq.Header.Set("Authorization", "token "+token) + } if r.Method == http.MethodGet && entry != nil { outReq.Header.Set("If-None-Match", entry.etag) } resp, err := p.upstream.Do(outReq) if err != nil { - http.Error(w, "Upstream request failed", http.StatusBadGateway) - log.Error(err, "Upstream request failed", "url", target.String()) - return + return nil, fmt.Errorf("upstream request failed: %w", err) } defer resp.Body.Close() - // On 304, serve the cached body directly. - if resp.StatusCode == http.StatusNotModified { - if entry != nil { - refreshed := *entry - refreshed.freshUntil = p.nextFreshUntil() - p.mu.Lock() - p.cache[key] = &refreshed - p.mu.Unlock() - - statusCode = entry.status - cacheResult = "revalidated_hit" - writeCachedResponse(w, proxyBase, upstream, &refreshed) - return - } - // Cache miss after 304 — should not happen, but fall through - // and treat it as an error by returning the 304 as-is. + if resp.StatusCode == http.StatusNotModified && entry != nil { + refreshed := *entry + refreshed.freshUntil = p.nextFreshUntil() + p.mu.Lock() + p.cache[key] = &refreshed + p.mu.Unlock() + return &responsePayload{ + statusCode: entry.status, + cacheResult: "revalidated_hit", + contentType: refreshed.contentType, + etag: refreshed.etag, + link: refreshed.link, + body: refreshed.body, + headers: map[string]string{ + "X-RateLimit-Limit": resp.Header.Get("X-RateLimit-Limit"), + "X-RateLimit-Remaining": resp.Header.Get("X-RateLimit-Remaining"), + "X-RateLimit-Reset": resp.Header.Get("X-RateLimit-Reset"), + }, + }, nil } body, err := io.ReadAll(resp.Body) if err != nil { - http.Error(w, "Failed to read upstream response", http.StatusBadGateway) - log.Error(err, "Failed to read upstream response body") - return + return nil, fmt.Errorf("reading upstream response body: %w", err) } - // Cache successful GET responses that include an ETag. if r.Method == http.MethodGet && resp.StatusCode >= 200 && resp.StatusCode < 300 { if etag := resp.Header.Get("ETag"); etag != "" { p.mu.Lock() @@ -223,31 +203,111 @@ func (p *proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } - // Copy response headers. - for _, h := range []string{"Content-Type", "ETag", "X-RateLimit-Limit", "X-RateLimit-Remaining", "X-RateLimit-Reset"} { - if v := resp.Header.Get(h); v != "" { + cacheResult := "skip" + if r.Method == http.MethodGet { + cacheResult = "miss" + log.Info("Cache miss", "key", key, "status", resp.StatusCode, "resource", source.ClassifyResource(r.URL.Path)) + } + return &responsePayload{ + statusCode: resp.StatusCode, + cacheResult: cacheResult, + contentType: resp.Header.Get("Content-Type"), + etag: resp.Header.Get("ETag"), + link: resp.Header.Get("Link"), + body: body, + headers: map[string]string{ + "X-RateLimit-Limit": resp.Header.Get("X-RateLimit-Limit"), + "X-RateLimit-Remaining": resp.Header.Get("X-RateLimit-Remaining"), + "X-RateLimit-Reset": resp.Header.Get("X-RateLimit-Reset"), + }, + }, nil +} + +func (p *proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { + log := ctrl.Log.WithName("ghproxy") + resource := source.ClassifyResource(r.URL.Path) + statusCode := http.StatusBadGateway + cacheResult := "skip" + defer func() { + githubAPIRequestsTotal.WithLabelValues(r.Method, strconv.Itoa(statusCode), resource, cacheResult).Inc() + }() + + upstream := p.upstreamBaseURL + scheme := "http" + if proto := r.Header.Get("X-Forwarded-Proto"); proto != "" { + scheme = strings.Split(proto, ",")[0] + } else if r.TLS != nil { + scheme = "https" + } + proxyBase := scheme + "://" + r.Host + key := cacheKey(r.URL.RequestURI(), r.Header.Get("Accept")) + + payload, err := p.fetchResponse(log, upstream, key, r) + if err != nil { + http.Error(w, "Upstream request failed", http.StatusBadGateway) + log.Error(err, "Upstream request failed", "upstream", upstream, "path", r.URL.RequestURI()) + return + } + + for _, h := range []string{"X-RateLimit-Limit", "X-RateLimit-Remaining", "X-RateLimit-Reset"} { + if v := payload.headers[h]; v != "" { w.Header().Set(h, v) } } - if link := resp.Header.Get("Link"); link != "" { - w.Header().Set("Link", rewriteLinkHeader(link, upstream, proxyBase)) + if payload.contentType != "" { + w.Header().Set("Content-Type", payload.contentType) } - statusCode = resp.StatusCode - if r.Method == http.MethodGet { - cacheResult = "miss" + if payload.etag != "" { + w.Header().Set("ETag", payload.etag) + } + if payload.link != "" { + w.Header().Set("Link", rewriteLinkHeader(payload.link, upstream, proxyBase)) + } + + statusCode = payload.statusCode + cacheResult = payload.cacheResult + w.WriteHeader(payload.statusCode) + w.Write(payload.body) +} + +func newTokenResolver(staticToken, tokenFile string) func() string { + var ( + mu sync.Mutex + cached string + cachedAt time.Time + refreshRate = 30 * time.Second + ) + return func() string { + if tokenFile == "" { + return strings.TrimSpace(staticToken) + } + mu.Lock() + defer mu.Unlock() + if cached != "" && time.Since(cachedAt) < refreshRate { + return cached + } + data, err := os.ReadFile(tokenFile) + if err == nil { + if token := strings.TrimSpace(string(data)); token != "" { + cached = token + cachedAt = time.Now() + return token + } + } + return strings.TrimSpace(staticToken) } - w.WriteHeader(resp.StatusCode) - w.Write(body) } func main() { var listenAddr string var metricsAddr string - var allowedUpstreamsFlag string + var upstreamBaseURL string + var githubTokenFile string var cacheTTL time.Duration flag.StringVar(&listenAddr, "listen-address", ":8888", "Address to listen on") flag.StringVar(&metricsAddr, "metrics-address", ":9090", "Address to serve Prometheus metrics on") - flag.StringVar(&allowedUpstreamsFlag, "allowed-upstreams", defaultUpstream, "Comma-separated list of allowed upstream base URLs") + flag.StringVar(&upstreamBaseURL, "upstream-base-url", defaultUpstream, "GitHub API base URL to proxy") + flag.StringVar(&githubTokenFile, "github-token-file", "", "Path to file containing GitHub token") flag.DurationVar(&cacheTTL, "cache-ttl", defaultCacheTTL, "Duration to serve cached GET responses without upstream revalidation") opts, applyVerbosity := logging.SetupZapOptions(flag.CommandLine) @@ -261,8 +321,7 @@ func main() { ctrl.SetLogger(zap.New(zap.UseFlagOptions(opts))) log := ctrl.Log.WithName("ghproxy") - allowed := strings.Split(allowedUpstreamsFlag, ",") - p := newProxy(allowed, cacheTTL) + p := newProxy(upstreamBaseURL, cacheTTL, newTokenResolver(os.Getenv("GITHUB_TOKEN"), githubTokenFile)) srv := &http.Server{ Addr: listenAddr, @@ -284,7 +343,7 @@ func main() { } }() - log.Info("Starting ghproxy", "address", listenAddr, "metricsAddress", metricsAddr, "allowedUpstreams", allowed, "cacheTTL", cacheTTL) + log.Info("Starting ghproxy", "address", listenAddr, "metricsAddress", metricsAddr, "upstreamBaseURL", upstreamBaseURL, "cacheTTL", cacheTTL) if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed { log.Error(err, "Server failed") os.Exit(1) diff --git a/cmd/ghproxy/main_test.go b/cmd/ghproxy/main_test.go index 25f84d5d..d5adce7c 100644 --- a/cmd/ghproxy/main_test.go +++ b/cmd/ghproxy/main_test.go @@ -1,14 +1,21 @@ package main import ( + "bytes" "fmt" "io" "net/http" "net/http/httptest" + "os" + "path/filepath" + "strings" "sync/atomic" "testing" "time" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + "github.com/kelos-dev/kelos/internal/source" ) @@ -27,7 +34,7 @@ func TestProxy_ServesFreshCacheWithoutRevalidating(t *testing.T) { })) defer upstream.Close() - p := newProxy([]string{upstream.URL}, time.Minute) + p := newProxy(upstream.URL, time.Minute, nil) p.now = func() time.Time { return now } proxyServer := httptest.NewServer(p) defer proxyServer.Close() @@ -82,7 +89,7 @@ func TestProxy_RevalidatesStaleGETWithETag(t *testing.T) { })) defer upstream.Close() - p := newProxy([]string{upstream.URL}, time.Second) + p := newProxy(upstream.URL, time.Second, nil) p.now = func() time.Time { return now } proxyServer := httptest.NewServer(p) defer proxyServer.Close() @@ -124,25 +131,20 @@ func TestProxy_RevalidatesStaleGETWithETag(t *testing.T) { func TestProxy_SeparatesCacheByUpstream(t *testing.T) { var hits atomic.Int32 upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Header.Get("If-None-Match") != "" { - w.WriteHeader(http.StatusNotModified) - return - } hits.Add(1) w.Header().Set("ETag", `"e1"`) w.Header().Set("Content-Type", "application/json") - // Differentiate by the upstream header the proxy forwards. - w.Write([]byte(`{"hit":` + fmt.Sprintf("%d", hits.Load()) + `}`)) + w.Write([]byte(`{"hit":1}`)) })) defer upstream.Close() - p := newProxy([]string{upstream.URL + "/public", upstream.URL + "/enterprise"}, time.Minute) + p := newProxy(upstream.URL, time.Minute, func() string { return "" }) proxyServer := httptest.NewServer(p) defer proxyServer.Close() - doGET := func(upstreamURL string) { + doGET := func() { req, _ := http.NewRequest("GET", proxyServer.URL+"/repos/owner/repo", nil) - req.Header.Set(source.UpstreamBaseURLHeader, upstreamURL) + req.Header.Set(source.UpstreamBaseURLHeader, "https://ignored.example.com") resp, err := http.DefaultClient.Do(req) if err != nil { t.Fatal(err) @@ -150,12 +152,10 @@ func TestProxy_SeparatesCacheByUpstream(t *testing.T) { resp.Body.Close() } - // Two different upstream headers for the same path should produce - // two separate cache entries (2 upstream hits). - doGET(upstream.URL + "/public") - doGET(upstream.URL + "/enterprise") - if hits.Load() != 2 { - t.Fatalf("expected 2 upstream hits for different upstreams, got %d", hits.Load()) + doGET() + doGET() + if hits.Load() != 1 { + t.Fatalf("expected 1 upstream hit with fixed configured upstream, got %d", hits.Load()) } } @@ -168,7 +168,7 @@ func TestProxy_PassesThroughNonGET(t *testing.T) { })) defer upstream.Close() - p := newProxy([]string{upstream.URL}, time.Minute) + p := newProxy(upstream.URL, time.Minute, nil) proxyServer := httptest.NewServer(p) defer proxyServer.Close() @@ -188,10 +188,9 @@ func TestProxy_PassesThroughNonGET(t *testing.T) { } } -func TestProxy_DefaultUpstream(t *testing.T) { - // Verify that the cache key includes the default upstream. - key := cacheKey(defaultUpstream, "/repos/owner/repo", "") - if key != "https://api.github.com|/repos/owner/repo|" { +func TestProxy_CacheKeyFormat(t *testing.T) { + key := cacheKey("/repos/owner/repo", "") + if key != "/repos/owner/repo|" { t.Fatalf("unexpected cache key: %s", key) } } @@ -207,7 +206,7 @@ func TestProxy_RewritesLinkHeader(t *testing.T) { })) defer upstream.Close() - p := newProxy([]string{upstream.URL}, time.Minute) + p := newProxy(upstream.URL, time.Minute, nil) proxyServer := httptest.NewServer(p) defer proxyServer.Close() @@ -242,7 +241,7 @@ func TestProxy_CachedResponsePreservesLinkHeader(t *testing.T) { })) defer upstream.Close() - p := newProxy([]string{upstream.URL}, 0) + p := newProxy(upstream.URL, 0, nil) proxyServer := httptest.NewServer(p) defer proxyServer.Close() @@ -277,37 +276,54 @@ func TestProxy_CachedResponsePreservesLinkHeader(t *testing.T) { } } -func TestProxy_RejectsDisallowedUpstream(t *testing.T) { - p := newProxy([]string{"https://api.github.com"}, time.Minute) +func TestProxy_UsesConfiguredStaticToken(t *testing.T) { + var authHeader string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + authHeader = r.Header.Get("Authorization") + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"ok":true}`)) + })) + defer upstream.Close() + + p := newProxy(upstream.URL, time.Minute, func() string { return "static-token" }) proxyServer := httptest.NewServer(p) defer proxyServer.Close() req, _ := http.NewRequest("GET", proxyServer.URL+"/repos/owner/repo", nil) - req.Header.Set(source.UpstreamBaseURLHeader, "https://evil.example.com") resp, err := http.DefaultClient.Do(req) if err != nil { t.Fatal(err) } defer resp.Body.Close() - if resp.StatusCode != http.StatusForbidden { - t.Fatalf("expected 403 for disallowed upstream, got %d", resp.StatusCode) + if resp.StatusCode != http.StatusOK { + t.Fatalf("expected 200, got %d", resp.StatusCode) + } + if authHeader != "token static-token" { + t.Fatalf("expected static token auth header, got %q", authHeader) } } -func TestProxy_AllowsConfiguredUpstream(t *testing.T) { +func TestProxy_UsesTokenFile(t *testing.T) { + var authHeader string upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + authHeader = r.Header.Get("Authorization") w.WriteHeader(http.StatusOK) w.Write([]byte(`{"ok":true}`)) })) defer upstream.Close() - p := newProxy([]string{"https://api.github.com", upstream.URL}, time.Minute) + tmpDir := t.TempDir() + tokenFile := filepath.Join(tmpDir, "token") + if err := os.WriteFile(tokenFile, []byte("file-token\n"), 0o600); err != nil { + t.Fatalf("writing token file: %v", err) + } + + p := newProxy(upstream.URL, time.Minute, newTokenResolver("", tokenFile)) proxyServer := httptest.NewServer(p) defer proxyServer.Close() req, _ := http.NewRequest("GET", proxyServer.URL+"/repos/owner/repo", nil) - req.Header.Set(source.UpstreamBaseURLHeader, upstream.URL) resp, err := http.DefaultClient.Do(req) if err != nil { t.Fatal(err) @@ -315,22 +331,71 @@ func TestProxy_AllowsConfiguredUpstream(t *testing.T) { defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - t.Fatalf("expected 200 for allowed upstream, got %d", resp.StatusCode) + t.Fatalf("expected 200, got %d", resp.StatusCode) + } + if authHeader != "token file-token" { + t.Fatalf("expected token file auth header, got %q", authHeader) } } -func TestCacheKeyVariesByAcceptNotAuthorization(t *testing.T) { - key1 := cacheKey(defaultUpstream, "/repos/o/r/issues", "application/json") - key2 := cacheKey(defaultUpstream, "/repos/o/r/issues", "application/vnd.github.raw+json") +func TestCacheKeyVariesByAccept(t *testing.T) { + key1 := cacheKey("/repos/o/r/issues", "application/json") + key2 := cacheKey("/repos/o/r/issues", "application/vnd.github.raw+json") if key1 == key2 { t.Fatal("expected cache key to vary by Accept header") } - if key1 != cacheKey(defaultUpstream, "/repos/o/r/issues", "application/json") { + if key1 != cacheKey("/repos/o/r/issues", "application/json") { t.Fatal("expected cache key to be stable for identical inputs") } } +func TestProxy_LogsCacheMiss(t *testing.T) { + var buf bytes.Buffer + ctrl.SetLogger(zap.New(zap.WriteTo(&buf), zap.UseDevMode(true))) + + now := time.Unix(1700000000, 0) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("ETag", `"v1"`) + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"ok":true}`)) + })) + defer upstream.Close() + + p := newProxy(upstream.URL, time.Minute, nil) + p.now = func() time.Time { return now } + proxyServer := httptest.NewServer(p) + defer proxyServer.Close() + + doGET := func() { + req, _ := http.NewRequest("GET", proxyServer.URL+"/repos/owner/repo/issues", nil) + req.Header.Set(source.UpstreamBaseURLHeader, upstream.URL) + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + } + + // First request is a cache miss — should log. + doGET() + logOutput := buf.String() + if !strings.Contains(logOutput, "Cache miss") { + t.Errorf("expected cache miss log on first request, got: %s", logOutput) + } + if !strings.Contains(logOutput, "/repos/owner/repo/issues") { + t.Errorf("expected log to contain request path, got: %s", logOutput) + } + + // Second request within TTL is a fresh hit — no additional miss log. + buf.Reset() + now = now.Add(10 * time.Second) + doGET() + if strings.Contains(buf.String(), "Cache miss") { + t.Error("unexpected cache miss log for fresh cache hit") + } +} + func TestRewriteLinkHeader(t *testing.T) { tests := []struct { name string diff --git a/cmd/ghproxy/metrics.go b/cmd/ghproxy/metrics.go index a0fb0374..b7f824a5 100644 --- a/cmd/ghproxy/metrics.go +++ b/cmd/ghproxy/metrics.go @@ -7,7 +7,7 @@ import ( var ( githubAPIRequestsTotal = prometheus.NewCounterVec( prometheus.CounterOpts{ - Name: "ghproxy_github_api_requests_total", + Name: "kelos_ghproxy_github_api_requests_total", Help: "Total number of GitHub API requests proxied by ghproxy", }, []string{"method", "status_code", "resource", "cache"}, diff --git a/cmd/ghproxy/metrics_test.go b/cmd/ghproxy/metrics_test.go index e7de9b7f..b862adfc 100644 --- a/cmd/ghproxy/metrics_test.go +++ b/cmd/ghproxy/metrics_test.go @@ -20,7 +20,7 @@ func TestProxy_RecordsMetrics(t *testing.T) { })) defer upstream.Close() - p := newProxy([]string{upstream.URL}, time.Minute) + p := newProxy(upstream.URL, time.Minute, nil) proxyServer := httptest.NewServer(p) defer proxyServer.Close() @@ -53,7 +53,7 @@ func TestProxy_RecordsFreshCacheHitMetric(t *testing.T) { })) defer upstream.Close() - p := newProxy([]string{upstream.URL}, time.Minute) + p := newProxy(upstream.URL, time.Minute, nil) proxyServer := httptest.NewServer(p) defer proxyServer.Close() @@ -87,6 +87,7 @@ func TestProxy_RecordsFreshCacheHitMetric(t *testing.T) { func TestProxy_RecordsRevalidatedCacheHitMetric(t *testing.T) { var reqCount int + now := time.Unix(1700000000, 0) upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { reqCount++ if r.Header.Get("If-None-Match") == `"v1"` { @@ -99,7 +100,8 @@ func TestProxy_RecordsRevalidatedCacheHitMetric(t *testing.T) { })) defer upstream.Close() - p := newProxy([]string{upstream.URL}, 0) + p := newProxy(upstream.URL, time.Second, nil) + p.now = func() time.Time { return now } proxyServer := httptest.NewServer(p) defer proxyServer.Close() @@ -120,6 +122,7 @@ func TestProxy_RecordsRevalidatedCacheHitMetric(t *testing.T) { // First request: cache miss. doGET() // Second request: stale entry is revalidated and served from cache. + now = now.Add(2 * time.Second) doGET() hitAfter := testutil.ToFloat64(githubAPIRequestsTotal.WithLabelValues("GET", "200", "pulls", "revalidated_hit")) @@ -136,23 +139,28 @@ func TestProxy_RecordsRevalidatedCacheHitMetric(t *testing.T) { } } -func TestProxy_RecordsRejectedUpstreamMetric(t *testing.T) { - p := newProxy([]string{"https://api.github.com"}, time.Minute) +func TestProxy_RecordsNonGETSkipMetric(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"id":1}`)) + })) + defer upstream.Close() + + p := newProxy(upstream.URL, time.Minute, nil) proxyServer := httptest.NewServer(p) defer proxyServer.Close() - before := testutil.ToFloat64(githubAPIRequestsTotal.WithLabelValues("GET", "403", "issues", "skip")) + before := testutil.ToFloat64(githubAPIRequestsTotal.WithLabelValues("POST", "201", "issues", "skip")) - req, _ := http.NewRequest("GET", proxyServer.URL+"/repos/owner/repo/issues", nil) - req.Header.Set(source.UpstreamBaseURLHeader, "https://evil.example.com") + req, _ := http.NewRequest("POST", proxyServer.URL+"/repos/owner/repo/issues", nil) resp, err := http.DefaultClient.Do(req) if err != nil { t.Fatal(err) } resp.Body.Close() - after := testutil.ToFloat64(githubAPIRequestsTotal.WithLabelValues("GET", "403", "issues", "skip")) + after := testutil.ToFloat64(githubAPIRequestsTotal.WithLabelValues("POST", "201", "issues", "skip")) if after != before+1 { - t.Errorf("expected rejected counter to increment by 1, got delta %f", after-before) + t.Errorf("expected non-GET skip counter to increment by 1, got delta %f", after-before) } } diff --git a/cmd/kelos-controller/main.go b/cmd/kelos-controller/main.go index a572cd28..6bdc6301 100644 --- a/cmd/kelos-controller/main.go +++ b/cmd/kelos-controller/main.go @@ -53,6 +53,10 @@ func main() { var spawnerImagePullPolicy string var spawnerResourceRequests string var spawnerResourceLimits string + var ghProxyImage string + var ghProxyImagePullPolicy string + var ghProxyResourceRequests string + var ghProxyResourceLimits string var tokenRefresherImage string var tokenRefresherImagePullPolicy string var tokenRefresherResourceRequests string @@ -80,6 +84,10 @@ func main() { flag.StringVar(&spawnerImagePullPolicy, "spawner-image-pull-policy", "", "The image pull policy for spawner Deployments (e.g., Always, Never, IfNotPresent).") flag.StringVar(&spawnerResourceRequests, "spawner-resource-requests", "", "Resource requests for spawner containers as comma-separated name=value pairs (e.g., cpu=250m,memory=512Mi).") flag.StringVar(&spawnerResourceLimits, "spawner-resource-limits", "", "Resource limits for spawner containers as comma-separated name=value pairs (e.g., cpu=1,memory=1Gi).") + flag.StringVar(&ghProxyImage, "ghproxy-image", controller.DefaultGHProxyImage, "The image to use for workspace ghproxy Deployments.") + flag.StringVar(&ghProxyImagePullPolicy, "ghproxy-image-pull-policy", "", "The image pull policy for workspace ghproxy Deployments (e.g., Always, Never, IfNotPresent).") + flag.StringVar(&ghProxyResourceRequests, "ghproxy-resource-requests", "", "Resource requests for workspace ghproxy containers as comma-separated name=value pairs (e.g., cpu=50m,memory=64Mi).") + flag.StringVar(&ghProxyResourceLimits, "ghproxy-resource-limits", "", "Resource limits for workspace ghproxy containers as comma-separated name=value pairs (e.g., cpu=200m,memory=128Mi).") flag.StringVar(&tokenRefresherImage, "token-refresher-image", controller.DefaultTokenRefresherImage, "The image to use for the token refresher sidecar.") flag.StringVar(&tokenRefresherImagePullPolicy, "token-refresher-image-pull-policy", "", "The image pull policy for the token refresher sidecar (e.g., Always, Never, IfNotPresent).") flag.StringVar(&tokenRefresherResourceRequests, "token-refresher-resource-requests", "", "Resource requests for token refresher sidecars as comma-separated name=value pairs (e.g., cpu=100m,memory=128Mi).") @@ -116,6 +124,23 @@ func main() { Limits: limits, } } + var ghProxyResources *corev1.ResourceRequirements + ghProxyRequests, err := controller.ParseResourceList(ghProxyResourceRequests) + if err != nil { + fmt.Fprintf(os.Stderr, "Error parsing --ghproxy-resource-requests: %v\n", err) + os.Exit(1) + } + ghProxyLimits, err := controller.ParseResourceList(ghProxyResourceLimits) + if err != nil { + fmt.Fprintf(os.Stderr, "Error parsing --ghproxy-resource-limits: %v\n", err) + os.Exit(1) + } + if ghProxyRequests != nil || ghProxyLimits != nil { + ghProxyResources = &corev1.ResourceRequirements{ + Requests: ghProxyRequests, + Limits: ghProxyLimits, + } + } var tokenRefresherResources *corev1.ResourceRequirements tokenRefresherRequests, err := controller.ParseResourceList(tokenRefresherResourceRequests) if err != nil { @@ -214,6 +239,22 @@ func main() { deploymentBuilder.TokenRefresherImage = tokenRefresherImage deploymentBuilder.TokenRefresherImagePullPolicy = corev1.PullPolicy(tokenRefresherImagePullPolicy) deploymentBuilder.TokenRefresherResources = tokenRefresherResources + workspaceProxyBuilder := controller.NewWorkspaceGHProxyBuilder() + workspaceProxyBuilder.GHProxyImage = ghProxyImage + workspaceProxyBuilder.GHProxyImagePullPolicy = corev1.PullPolicy(ghProxyImagePullPolicy) + workspaceProxyBuilder.GHProxyResources = ghProxyResources + workspaceProxyBuilder.TokenRefresherImage = tokenRefresherImage + workspaceProxyBuilder.TokenRefresherImagePullPolicy = corev1.PullPolicy(tokenRefresherImagePullPolicy) + workspaceProxyBuilder.TokenRefresherResources = tokenRefresherResources + if err = (&controller.WorkspaceReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + ProxyBuilder: workspaceProxyBuilder, + Recorder: mgr.GetEventRecorderFor("kelos-controller"), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "Workspace") + os.Exit(1) + } if err = (&controller.TaskSpawnerReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), diff --git a/cmd/kelos-spawner/main.go b/cmd/kelos-spawner/main.go index 59df50e8..3ef7ddcb 100644 --- a/cmd/kelos-spawner/main.go +++ b/cmd/kelos-spawner/main.go @@ -27,10 +27,9 @@ import ( "github.com/kelos-dev/kelos/internal/logging" "github.com/kelos-dev/kelos/internal/reporting" "github.com/kelos-dev/kelos/internal/source" + "github.com/kelos-dev/kelos/internal/taskbuilder" ) -const ghProxyURL = "http://ghproxy.kelos-system:8888" - var scheme = runtime.NewScheme() func init() { @@ -43,6 +42,7 @@ func main() { var namespace string var githubOwner string var githubRepo string + var ghProxyURL string var githubAPIBaseURL string var githubTokenFile string var jiraBaseURL string @@ -54,6 +54,7 @@ func main() { flag.StringVar(&namespace, "taskspawner-namespace", "", "Namespace of the TaskSpawner") flag.StringVar(&githubOwner, "github-owner", "", "GitHub repository owner") flag.StringVar(&githubRepo, "github-repo", "", "GitHub repository name") + flag.StringVar(&ghProxyURL, "gh-proxy-url", "", "Workspace ghproxy base URL for GitHub read requests") flag.StringVar(&githubAPIBaseURL, "github-api-base-url", "", "GitHub API base URL for enterprise servers (e.g. https://github.example.com/api/v3)") flag.StringVar(&githubTokenFile, "github-token-file", "", "Path to file containing GitHub token (refreshed by sidecar)") flag.StringVar(&jiraBaseURL, "jira-base-url", "", "Jira instance base URL (e.g. https://mycompany.atlassian.net)") @@ -95,15 +96,7 @@ func main() { log.Info("Starting spawner", "taskspawner", key, "oneShot", oneShot) - upstreamURL := githubAPIBaseURL - if upstreamURL == "" { - upstreamURL = "https://api.github.com" - } - transport := source.NewUpstreamHeaderTransport( - source.NewMetricsTransport(http.DefaultTransport), - upstreamURL, - ) - httpClient := &http.Client{Transport: transport} + httpClient := &http.Client{Transport: source.NewMetricsTransport(http.DefaultTransport)} cfgArgs := spawnerRuntimeConfig{ GitHubOwner: githubOwner, @@ -178,8 +171,12 @@ func runReportingCycle(ctx context.Context, cl client.Client, key types.Namespac } func runCycle(ctx context.Context, cl client.Client, key types.NamespacedName, githubOwner, githubRepo, githubAPIBaseURL, githubTokenFile, jiraBaseURL, jiraProject, jiraJQL string, httpClient *http.Client) error { + return runCycleWithProxy(ctx, cl, key, githubOwner, githubRepo, "", githubAPIBaseURL, githubTokenFile, jiraBaseURL, jiraProject, jiraJQL, httpClient) +} + +func runCycleWithProxy(ctx context.Context, cl client.Client, key types.NamespacedName, githubOwner, githubRepo, ghProxyURL, githubAPIBaseURL, githubTokenFile, jiraBaseURL, jiraProject, jiraJQL string, httpClient *http.Client) error { start := time.Now() - err := runCycleCore(ctx, cl, key, githubOwner, githubRepo, githubAPIBaseURL, githubTokenFile, jiraBaseURL, jiraProject, jiraJQL, httpClient) + err := runCycleCore(ctx, cl, key, githubOwner, githubRepo, ghProxyURL, githubAPIBaseURL, githubTokenFile, jiraBaseURL, jiraProject, jiraJQL, httpClient) discoveryDurationSeconds.Observe(time.Since(start).Seconds()) if err != nil { discoveryErrorsTotal.Inc() @@ -187,13 +184,13 @@ func runCycle(ctx context.Context, cl client.Client, key types.NamespacedName, g return err } -func runCycleCore(ctx context.Context, cl client.Client, key types.NamespacedName, githubOwner, githubRepo, githubAPIBaseURL, githubTokenFile, jiraBaseURL, jiraProject, jiraJQL string, httpClient *http.Client) error { +func runCycleCore(ctx context.Context, cl client.Client, key types.NamespacedName, githubOwner, githubRepo, ghProxyURL, githubAPIBaseURL, githubTokenFile, jiraBaseURL, jiraProject, jiraJQL string, httpClient *http.Client) error { var ts kelosv1alpha1.TaskSpawner if err := cl.Get(ctx, key, &ts); err != nil { return fmt.Errorf("fetching TaskSpawner: %w", err) } - src, err := buildSource(&ts, githubOwner, githubRepo, githubAPIBaseURL, githubTokenFile, jiraBaseURL, jiraProject, jiraJQL, httpClient) + src, err := buildSourceWithProxy(&ts, githubOwner, githubRepo, ghProxyURL, githubAPIBaseURL, githubTokenFile, jiraBaseURL, jiraProject, jiraJQL, httpClient) if err != nil { return fmt.Errorf("building source: %w", err) } @@ -338,70 +335,49 @@ func runCycleWithSourceCore(ctx context.Context, cl client.Client, key types.Nam taskName := fmt.Sprintf("%s-%s", ts.Name, item.ID) - prompt, err := source.RenderPrompt(ts.Spec.TaskTemplate.PromptTemplate, item) - if err != nil { - log.Error(err, "rendering prompt", "item", item.ID) - continue - } + templateVars := source.WorkItemToTemplateVars(item) - renderedLabels, renderedAnnotations, err := renderTaskTemplateMetadata(&ts, item) + tb, err := taskbuilder.NewTaskBuilder(cl) if err != nil { - log.Error(err, "Rendering task template metadata", "item", item.ID) + log.Error(err, "creating task builder", "item", item.ID) continue } - labels := make(map[string]string) - for k, v := range renderedLabels { - labels[k] = v - } - labels["kelos.dev/taskspawner"] = ts.Name - - annotations := mergeStringMaps(renderedAnnotations, sourceAnnotations(&ts, item)) - - task := &kelosv1alpha1.Task{ - ObjectMeta: metav1.ObjectMeta{ - Name: taskName, - Namespace: ts.Namespace, - Labels: labels, - Annotations: annotations, - }, - Spec: kelosv1alpha1.TaskSpec{ - Type: ts.Spec.TaskTemplate.Type, - Prompt: prompt, - Credentials: ts.Spec.TaskTemplate.Credentials, - Model: ts.Spec.TaskTemplate.Model, - Image: ts.Spec.TaskTemplate.Image, - TTLSecondsAfterFinished: ts.Spec.TaskTemplate.TTLSecondsAfterFinished, - PodOverrides: ts.Spec.TaskTemplate.PodOverrides, + task, err := tb.BuildTask( + taskName, + ts.Namespace, + &ts.Spec.TaskTemplate, + templateVars, + &taskbuilder.SpawnerRef{ + Name: ts.Name, + UID: string(ts.UID), + APIVersion: kelosv1alpha1.GroupVersion.String(), + Kind: "TaskSpawner", }, + ) + if err != nil { + log.Error(err, "building task", "item", item.ID) + continue } - if ts.Spec.TaskTemplate.WorkspaceRef != nil { - task.Spec.WorkspaceRef = ts.Spec.TaskTemplate.WorkspaceRef - } - if ts.Spec.TaskTemplate.AgentConfigRef != nil { - task.Spec.AgentConfigRef = ts.Spec.TaskTemplate.AgentConfigRef - } - - if len(ts.Spec.TaskTemplate.DependsOn) > 0 { - task.Spec.DependsOn = ts.Spec.TaskTemplate.DependsOn - } - if ts.Spec.TaskTemplate.Branch != "" { - branch, err := source.RenderTemplate(ts.Spec.TaskTemplate.Branch, item) - if err != nil { - log.Error(err, "rendering branch template", "item", item.ID) - continue + // Apply source-specific annotations (GitHub reporting metadata) + srcAnnotations := sourceAnnotations(&ts, item) + if len(srcAnnotations) > 0 { + if task.Annotations == nil { + task.Annotations = make(map[string]string) + } + for k, v := range srcAnnotations { + task.Annotations[k] = v } - task.Spec.Branch = branch } // Propagate upstream repo for fork workflows. Explicit template // value takes precedence; otherwise derive from the source repo // override (githubIssues.repo or githubPullRequests.repo). - if ts.Spec.TaskTemplate.UpstreamRepo != "" { - task.Spec.UpstreamRepo = ts.Spec.TaskTemplate.UpstreamRepo - } else if upstreamRepo := deriveUpstreamRepo(&ts); upstreamRepo != "" { - task.Spec.UpstreamRepo = upstreamRepo + if task.Spec.UpstreamRepo == "" { + if upstreamRepo := deriveUpstreamRepo(&ts); upstreamRepo != "" { + task.Spec.UpstreamRepo = upstreamRepo + } } if err := cl.Create(ctx, task); err != nil { @@ -471,52 +447,6 @@ func runCycleWithSourceCore(ctx context.Context, cl client.Client, key types.Nam return nil } -// mergeStringMaps returns a new map with keys from base, then keys from overlay -// overwriting on duplicate keys. -func mergeStringMaps(base, overlay map[string]string) map[string]string { - if len(base) == 0 && len(overlay) == 0 { - return nil - } - out := make(map[string]string) - for k, v := range base { - out[k] = v - } - for k, v := range overlay { - out[k] = v - } - return out -} - -// renderTaskTemplateMetadata renders taskTemplate.metadata label and annotation -// values using source.RenderTemplate. -func renderTaskTemplateMetadata(ts *kelosv1alpha1.TaskSpawner, item source.WorkItem) (labels map[string]string, annotations map[string]string, err error) { - meta := ts.Spec.TaskTemplate.Metadata - if meta == nil { - return nil, nil, nil - } - if len(meta.Labels) > 0 { - labels = make(map[string]string) - for k, v := range meta.Labels { - rendered, err := source.RenderTemplate(v, item) - if err != nil { - return nil, nil, fmt.Errorf("label %q: %w", k, err) - } - labels[k] = rendered - } - } - if len(meta.Annotations) > 0 { - annotations = make(map[string]string) - for k, v := range meta.Annotations { - rendered, err := source.RenderTemplate(v, item) - if err != nil { - return nil, nil, fmt.Errorf("annotation %q: %w", k, err) - } - annotations[k] = rendered - } - } - return labels, annotations, nil -} - // sourceAnnotations returns annotations that stamp GitHub source metadata // onto a spawned Task. These annotations enable downstream consumers (such // as the reporting watcher) to identify the originating issue or PR. @@ -597,16 +527,26 @@ func resolveGitHubCommentPolicy(policy *kelosv1alpha1.GitHubCommentPolicy, legac } func buildSource(ts *kelosv1alpha1.TaskSpawner, owner, repo, apiBaseURL, tokenFile, jiraBaseURL, jiraProject, jiraJQL string, httpClient *http.Client) (source.Source, error) { + return buildSourceWithProxy(ts, owner, repo, "", apiBaseURL, tokenFile, jiraBaseURL, jiraProject, jiraJQL, httpClient) +} + +func buildSourceWithProxy(ts *kelosv1alpha1.TaskSpawner, owner, repo, ghProxyURL, apiBaseURL, tokenFile, jiraBaseURL, jiraProject, jiraJQL string, httpClient *http.Client) (source.Source, error) { if ts.Spec.When.GitHubIssues != nil { gh := ts.Spec.When.GitHubIssues - token, err := readGitHubToken(tokenFile) - if err != nil { - return nil, err - } commentPolicy, err := resolveGitHubCommentPolicy(gh.CommentPolicy, gh.TriggerComment, gh.ExcludeComments) if err != nil { return nil, err } + baseURL := apiBaseURL + token := "" + if ghProxyURL != "" { + baseURL = ghProxyURL + } else { + token, err = readGitHubToken(tokenFile) + if err != nil { + return nil, err + } + } return &source.GitHubSource{ Owner: owner, Repo: repo, @@ -617,7 +557,7 @@ func buildSource(ts *kelosv1alpha1.TaskSpawner, owner, repo, apiBaseURL, tokenFi Assignee: gh.Assignee, Author: gh.Author, Token: token, - BaseURL: apiBaseURL, + BaseURL: baseURL, Client: httpClient, TriggerComment: commentPolicy.TriggerComment, ExcludeComments: commentPolicy.ExcludeComments, @@ -630,14 +570,20 @@ func buildSource(ts *kelosv1alpha1.TaskSpawner, owner, repo, apiBaseURL, tokenFi if ts.Spec.When.GitHubPullRequests != nil { gh := ts.Spec.When.GitHubPullRequests - token, err := readGitHubToken(tokenFile) - if err != nil { - return nil, err - } commentPolicy, err := resolveGitHubCommentPolicy(gh.CommentPolicy, gh.TriggerComment, gh.ExcludeComments) if err != nil { return nil, err } + baseURL := apiBaseURL + token := "" + if ghProxyURL != "" { + baseURL = ghProxyURL + } else { + token, err = readGitHubToken(tokenFile) + if err != nil { + return nil, err + } + } return &source.GitHubPullRequestSource{ Owner: owner, @@ -647,7 +593,7 @@ func buildSource(ts *kelosv1alpha1.TaskSpawner, owner, repo, apiBaseURL, tokenFi State: gh.State, Author: gh.Author, Token: token, - BaseURL: apiBaseURL, + BaseURL: baseURL, Client: httpClient, ReviewState: gh.ReviewState, TriggerComment: commentPolicy.TriggerComment, diff --git a/cmd/kelos-spawner/main_test.go b/cmd/kelos-spawner/main_test.go index 1638249c..f67ce0ff 100644 --- a/cmd/kelos-spawner/main_test.go +++ b/cmd/kelos-spawner/main_test.go @@ -94,6 +94,7 @@ func newTaskSpawner(name, namespace string, maxConcurrency *int32) *kelosv1alpha ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, + UID: types.UID(name + "-uid"), }, Spec: kelosv1alpha1.TaskSpawnerSpec{ When: kelosv1alpha1.When{ @@ -299,6 +300,53 @@ func TestRunCycleWithSource_NoMaxConcurrency(t *testing.T) { } } +func TestRunCycleWithSource_OwnerReferenceSet(t *testing.T) { + ts := newTaskSpawner("spawner", "default", nil) + cl, key := setupTest(t, ts) + + src := &fakeSource{ + items: []source.WorkItem{ + {ID: "1", Title: "Item 1"}, + }, + } + + if err := runCycleWithSource(context.Background(), cl, key, src); err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + var taskList kelosv1alpha1.TaskList + if err := cl.List(context.Background(), &taskList, client.InNamespace("default")); err != nil { + t.Fatalf("Listing tasks: %v", err) + } + if len(taskList.Items) != 1 { + t.Fatalf("Expected 1 task, got %d", len(taskList.Items)) + } + + task := taskList.Items[0] + if len(task.OwnerReferences) != 1 { + t.Fatalf("Expected 1 owner reference, got %d", len(task.OwnerReferences)) + } + ref := task.OwnerReferences[0] + if ref.APIVersion != kelosv1alpha1.GroupVersion.String() { + t.Errorf("Expected APIVersion %s, got %s", kelosv1alpha1.GroupVersion.String(), ref.APIVersion) + } + if ref.Kind != "TaskSpawner" { + t.Errorf("Expected Kind TaskSpawner, got %s", ref.Kind) + } + if ref.Name != "spawner" { + t.Errorf("Expected Name spawner, got %s", ref.Name) + } + if ref.UID != ts.UID { + t.Errorf("Expected UID %s, got %s", ts.UID, ref.UID) + } + if ref.Controller == nil || !*ref.Controller { + t.Error("Expected Controller=true") + } + if ref.BlockOwnerDeletion != nil { + t.Errorf("Expected BlockOwnerDeletion to be nil, got %v", *ref.BlockOwnerDeletion) + } +} + func TestRunCycleWithSource_MaxConcurrencyLimitsCreation(t *testing.T) { ts := newTaskSpawner("spawner", "default", int32Ptr(2)) cl, key := setupTest(t, ts) diff --git a/cmd/kelos-spawner/reconciler.go b/cmd/kelos-spawner/reconciler.go index 8c2cab09..b3acdd12 100644 --- a/cmd/kelos-spawner/reconciler.go +++ b/cmd/kelos-spawner/reconciler.go @@ -69,7 +69,7 @@ func (r *spawnerReconciler) SetupWithManager(mgr ctrl.Manager) error { } func runOnce(ctx context.Context, cl client.Client, key types.NamespacedName, cfg spawnerRuntimeConfig) (time.Duration, error) { - if err := runCycle(ctx, cl, key, cfg.GitHubOwner, cfg.GitHubRepo, cfg.GHProxyURL, cfg.GitHubTokenFile, cfg.JiraBaseURL, cfg.JiraProject, cfg.JiraJQL, cfg.HTTPClient); err != nil { + if err := runCycleWithProxy(ctx, cl, key, cfg.GitHubOwner, cfg.GitHubRepo, cfg.GHProxyURL, cfg.GitHubAPIBaseURL, cfg.GitHubTokenFile, cfg.JiraBaseURL, cfg.JiraProject, cfg.JiraJQL, cfg.HTTPClient); err != nil { return 0, err } diff --git a/cmd/kelos-webhook-server/Dockerfile b/cmd/kelos-webhook-server/Dockerfile new file mode 100644 index 00000000..f0a36f11 --- /dev/null +++ b/cmd/kelos-webhook-server/Dockerfile @@ -0,0 +1,5 @@ +FROM gcr.io/distroless/static:nonroot +WORKDIR / +COPY bin/kelos-webhook-server . +USER 65532:65532 +ENTRYPOINT ["/kelos-webhook-server"] diff --git a/cmd/kelos-webhook-server/main.go b/cmd/kelos-webhook-server/main.go new file mode 100644 index 00000000..8a493e80 --- /dev/null +++ b/cmd/kelos-webhook-server/main.go @@ -0,0 +1,146 @@ +package main + +import ( + "context" + "flag" + "fmt" + "net/http" + "os" + "strings" + "time" + + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/healthz" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + + kelosv1alpha1 "github.com/kelos-dev/kelos/api/v1alpha1" + "github.com/kelos-dev/kelos/internal/logging" + "github.com/kelos-dev/kelos/internal/webhook" +) + +var ( + scheme = runtime.NewScheme() + setupLog = ctrl.Log.WithName("setup") +) + +func init() { + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(kelosv1alpha1.AddToScheme(scheme)) +} + +func main() { + var ( + source string + metricsAddr string + probeAddr string + webhookAddr string + enableLeaderElection bool + ) + + flag.StringVar(&source, "source", "", "Webhook source type (github)") + flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") + flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") + flag.StringVar(&webhookAddr, "webhook-bind-address", ":8443", "The address the webhook endpoint binds to.") + flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager.") + + opts, applyVerbosity := logging.SetupZapOptions(flag.CommandLine) + flag.Parse() + + if err := applyVerbosity(); err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + os.Exit(1) + } + + ctrl.SetLogger(zap.New(zap.UseFlagOptions(opts))) + + // Validate source parameter + source = strings.ToLower(strings.TrimSpace(source)) + var webhookSource webhook.WebhookSource + switch source { + case "github": + webhookSource = webhook.GitHubSource + default: + setupLog.Error(fmt.Errorf("invalid source: %s", source), + "Source must be 'github'") + os.Exit(1) + } + + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + Scheme: scheme, + Metrics: metricsserver.Options{BindAddress: metricsAddr}, + HealthProbeBindAddress: probeAddr, + LeaderElection: enableLeaderElection, + LeaderElectionID: fmt.Sprintf("kelos-webhook-%s", source), + }) + if err != nil { + setupLog.Error(err, "Unable to start manager") + os.Exit(1) + } + + // Set up signal handling context + ctx := ctrl.SetupSignalHandler() + + // Create webhook handler + handler, err := webhook.NewWebhookHandler( + ctx, + mgr.GetClient(), + webhookSource, + ctrl.Log.WithName("webhook").WithValues("source", source), + ) + if err != nil { + setupLog.Error(err, "Unable to create webhook handler") + os.Exit(1) + } + + // Set up HTTP server for webhooks + mux := http.NewServeMux() + mux.Handle("/", handler) + + webhookServer := &http.Server{ + Addr: webhookAddr, + Handler: mux, + ReadTimeout: 30 * time.Second, // Maximum time to read request including body + WriteTimeout: 30 * time.Second, // Maximum time to write response + ReadHeaderTimeout: 10 * time.Second, // Maximum time to read request headers + IdleTimeout: 120 * time.Second, // Maximum time for keep-alive connections + } + + // Start webhook server in goroutine + go func() { + setupLog.Info("Starting webhook server", "addr", webhookAddr, "source", source) + if err := webhookServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { + setupLog.Error(err, "Webhook server failed") + os.Exit(1) + } + }() + + // Add health checks + if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { + setupLog.Error(err, "Unable to set up health check") + os.Exit(1) + } + if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { + setupLog.Error(err, "Unable to set up ready check") + os.Exit(1) + } + + setupLog.Info("Starting manager") + + // Shutdown webhook server gracefully when context is cancelled + go func() { + <-ctx.Done() + setupLog.Info("Shutting down webhook server") + if err := webhookServer.Shutdown(context.Background()); err != nil { + setupLog.Error(err, "Error shutting down webhook server") + } + }() + + if err := mgr.Start(ctx); err != nil { + setupLog.Error(err, "Problem running manager") + os.Exit(1) + } +} diff --git a/cursor/Dockerfile b/cursor/Dockerfile index 76e0d42f..76d70cb5 100644 --- a/cursor/Dockerfile +++ b/cursor/Dockerfile @@ -37,7 +37,7 @@ RUN mkdir -p /home/agent/.cursor && chown -R agent:agent /home/agent USER agent -ARG CURSOR_CLI_VERSION=2026.03.25-933d5a6 +ARG CURSOR_CLI_VERSION=2026.03.30-a5d3e17 RUN ARCH=$(uname -m) \ && case "$ARCH" in x86_64) ARCH="x64" ;; aarch64) ARCH="arm64" ;; esac \ && INSTALL_DIR="$HOME/.local/share/cursor-agent/versions/${CURSOR_CLI_VERSION}" \ diff --git a/examples/10-taskspawner-github-webhook/README.md b/examples/10-taskspawner-github-webhook/README.md new file mode 100644 index 00000000..ff052d21 --- /dev/null +++ b/examples/10-taskspawner-github-webhook/README.md @@ -0,0 +1,143 @@ +# GitHub Webhook TaskSpawner Example + +This example demonstrates how to configure a TaskSpawner to respond to GitHub webhook events. + +## Overview + +The GitHub webhook TaskSpawner triggers task creation based on GitHub repository events like: +- Issues being opened, closed, or commented on +- Pull requests being opened, reviewed, or merged +- Code being pushed to specific branches +- And many other GitHub events + +## Prerequisites + +1. **Webhook Server**: Deploy the kelos-webhook-server with GitHub source configuration +2. **GitHub Webhook**: Configure your GitHub repository to send webhooks to your Kelos instance +3. **Secret**: Create a Kubernetes secret containing the webhook signing secret + +## Setup + +### 1. Create Webhook Secret + +```bash +kubectl create secret generic github-webhook-secret \ + --from-literal=WEBHOOK_SECRET=your-github-webhook-secret +``` + +### 2. Configure GitHub Repository Webhook + +In your GitHub repository settings: +1. Go to Settings → Webhooks → Add webhook +2. Set Payload URL to: `https://your-kelos-instance.com/webhook/github` +3. Set Content type to: `application/json` +4. Set Secret to the same value as in your Kubernetes secret +5. Select events you want to receive (or "Send me everything") + +### 3. Deploy TaskSpawner + +Apply the TaskSpawner configuration: + +```bash +kubectl apply -f taskspawner.yaml +``` + +## Configuration Details + +The example TaskSpawner demonstrates several filtering patterns: + +- **Event Types**: Only responds to `issues`, `pull_request`, and `issue_comment` events +- **Action Filtering**: Responds to specific actions like "opened", "created", etc. +- **Label Requirements**: Can require specific labels to be present +- **Author Filtering**: Can filter by the user who triggered the event +- **Draft Filtering**: Can exclude or include draft pull requests + +## Template Variables + +GitHub webhook events provide rich template variables for task creation: + +### Standard Variables +- `{{.Event}}` - GitHub event type (e.g., "issues", "pull_request") +- `{{.Action}}` - Webhook action (e.g., "opened", "created", "submitted") +- `{{.Sender}}` - Username of person who triggered the event +- `{{.ID}}` - Issue/PR number as string +- `{{.Title}}` - Issue/PR title +- `{{.Number}}` - Issue/PR number as integer +- `{{.Body}}` - Issue/PR body text +- `{{.URL}}` - Issue/PR HTML URL +- `{{.Branch}}` - PR source branch or push branch +- `{{.Ref}}` - Git ref for push events + +### Raw Payload Access +- `{{.Payload.*}}` - Access any field from the GitHub webhook payload + +Example template usage: +```yaml +promptTemplate: | + A new {{.Event}} event occurred in the repository. + + Event: {{.Event}} + Action: {{.Action}} + Triggered by: {{.Sender}} + + {{if .Title}}Title: {{.Title}}{{end}} + {{if .URL}}URL: {{.URL}}{{end}} + + Please investigate and take appropriate action. + +branch: "webhook-{{.Event}}-{{.ID}}" +``` + +## Webhook Security + +The webhook server validates GitHub signatures using HMAC-SHA256: +- GitHub sends signatures in `X-Hub-Signature-256` header with `sha256=` prefix +- The server validates against the secret stored in `WEBHOOK_SECRET` env var +- Invalid signatures result in HTTP 401 responses + +## Scaling and Reliability + +### Concurrency Control +- Set `maxConcurrency` to limit parallel tasks from webhook events +- When exceeded, returns HTTP 503 with `Retry-After` header +- GitHub will automatically retry failed webhook deliveries + +### Idempotency +- Webhook deliveries are tracked by `X-GitHub-Delivery` header +- Duplicate deliveries (e.g., retries) are ignored +- Delivery cache entries expire after 24 hours + +### Fault Isolation +- Per-source webhook servers provide fault isolation +- GitHub webhook failures don't affect Linear or other sources +- Each source can be scaled independently + +## Troubleshooting + +### Common Issues + +1. **Tasks not being created** + - Check webhook server logs for signature validation errors + - Verify GitHub webhook is configured with correct URL and secret + - Check TaskSpawner event type and filter configuration + +2. **Signature validation failures** + - Ensure WEBHOOK_SECRET matches GitHub webhook secret exactly + - Check for trailing newlines or encoding issues in secret + +3. **Max concurrency errors** + - Increase maxConcurrency limit or reduce webhook frequency + - Check for stuck tasks that aren't completing + +### Debugging + +Enable verbose logging: +```yaml +env: + - name: LOG_LEVEL + value: "debug" +``` + +Check webhook deliveries in GitHub: +- Repository Settings → Webhooks → Recent Deliveries +- Shows request/response details and retry attempts diff --git a/examples/10-taskspawner-github-webhook/taskspawner.yaml b/examples/10-taskspawner-github-webhook/taskspawner.yaml new file mode 100644 index 00000000..892e7546 --- /dev/null +++ b/examples/10-taskspawner-github-webhook/taskspawner.yaml @@ -0,0 +1,135 @@ +apiVersion: kelos.dev/v1alpha1 +kind: TaskSpawner +metadata: + name: github-webhook-responder + namespace: default +spec: + # Respond to GitHub webhook events + when: + githubWebhook: + # Listen for these GitHub event types + events: + - "issues" + - "pull_request" + - "issue_comment" + - "pull_request_review" + + # Optional filters to refine which events trigger tasks + # Multiple filters use OR semantics within the same event type + filters: + # Respond to newly opened issues + - event: "issues" + action: "opened" + + # Respond to pull requests opened or ready for review + - event: "pull_request" + action: "opened" + draft: false # Only non-draft PRs + + # Respond to pull requests that get the "review-requested" label + - event: "pull_request" + action: "labeled" + labels: ["review-requested"] + + # Respond to issues with "bug" label but exclude "wontfix" or "duplicate" + - event: "issues" + action: "opened" + labels: ["bug"] + excludeLabels: ["wontfix", "duplicate"] + + # Respond to issue comments containing "/kelos" + - event: "issue_comment" + action: "created" + bodyContains: "/kelos" + + # Respond to PR review requests from specific author + - event: "pull_request_review" + action: "submitted" + author: "senior-reviewer" + + # Task template configuration + taskTemplate: + # Agent type and credentials + type: claude-code + credentials: + type: api-key + secretRef: + name: claude-credentials + key: api-key + + # Reference the repository workspace + workspaceRef: + name: my-repo-workspace + + # Template for the git branch (optional) + branch: "kelos-webhook-{{.Event}}-{{.ID}}" + + # Template for the task prompt + promptTemplate: | + # GitHub {{.Event | title}} Event: {{.Action}} + + A GitHub webhook event has been triggered that requires attention. + + ## Event Details + - **Event Type**: {{.Event}} + - **Action**: {{.Action}} + - **Triggered by**: @{{.Sender}} + {{- if .Title}} + - **Title**: {{.Title}} + {{- end}} + {{- if .URL}} + - **URL**: {{.URL}} + {{- end}} + {{- if .Branch}} + - **Branch**: {{.Branch}} + {{- end}} + + {{- if eq .Event "issues"}} + ## Issue Description + {{.Body}} + + ## Task + Please review this issue and provide an initial analysis or triage. + {{- else if eq .Event "pull_request"}} + ## Pull Request Description + {{.Body}} + + ## Task + Please review this pull request and provide feedback on the code changes. + {{- else if eq .Event "issue_comment"}} + ## Task + A comment was made that mentioned our bot. Please review the comment and respond appropriately. + {{- else if eq .Event "pull_request_review"}} + ## Task + A pull request review was submitted. Please check if any follow-up actions are needed. + {{- end}} + + You have full access to the repository through the workspace. Use git commands and code analysis tools as needed. + + # Task metadata (optional) + metadata: + labels: + kelos.dev/trigger: "webhook" + kelos.dev/event: "{{.Event}}" + kelos.dev/action: "{{.Action}}" + annotations: + kelos.dev/github-url: "{{.URL}}" + kelos.dev/github-sender: "{{.Sender}}" + + # Auto-cleanup completed tasks after 1 hour + ttlSecondsAfterFinished: 3600 + +--- +# Example workspace configuration (referenced above) +apiVersion: kelos.dev/v1alpha1 +kind: Workspace +metadata: + name: my-repo-workspace + namespace: default +spec: + repo: + url: "https://github.com/myorg/myrepo.git" + # Optional: reference secret for private repos + # secretRef: + # name: github-token + # key: token diff --git a/examples/gateway-api-webhook.md b/examples/gateway-api-webhook.md new file mode 100644 index 00000000..c4ec4dba --- /dev/null +++ b/examples/gateway-api-webhook.md @@ -0,0 +1,169 @@ +# Using Gateway API with Kelos Webhooks + +This document explains how to configure Kelos webhook servers using the Gateway API instead of traditional Ingress resources. + +## Prerequisites + +1. **Gateway API CRDs installed** in your cluster: + ```bash + kubectl apply -f https://github.com/kubernetes-sigs/gateway-api/releases/download/v1.0.0/standard-install.yaml + ``` + +2. **Gateway Controller** deployed (choose one): + - **Istio**: Follow [Istio Gateway API setup](https://istio.io/latest/docs/tasks/traffic-management/ingress/gateway-api/) + - **Envoy Gateway**: Follow [Envoy Gateway installation](https://gateway.envoyproxy.io/latest/user/quickstart/) + - **Kong**: Follow [Kong Gateway API setup](https://docs.konghq.com/kubernetes-ingress-controller/latest/guides/using-gateway-api/) + - **Nginx Gateway Fabric**: Follow [Nginx Gateway setup](https://docs.nginx.com/nginx-gateway-fabric/) + +## Configuration + +### Basic Gateway Setup + +```yaml +# values.yaml +webhookServer: + sources: + github: + enabled: true + secretName: github-webhook-secret + + # Use Gateway API instead of Ingress + gateway: + enabled: true + gatewayClassName: "istio" # Or your gateway class + gatewayName: "kelos-webhook-gateway" + host: "webhooks.your-domain.com" + + tls: + enabled: true + certificateRefs: + - name: "webhook-tls-cert" + kind: "Secret" +``` + +### Gateway Classes by Provider + +| Provider | Gateway Class | Notes | +|----------|---------------|-------| +| Istio | `istio` | Built-in with Istio installation | +| Envoy Gateway | `eg` | Default class name | +| Kong | `kong` | Configured during Kong installation | +| Nginx Gateway Fabric | `nginx` | Default class name | + +### TLS Configuration + +#### Option 1: Pre-existing Certificate +```yaml +gateway: + tls: + enabled: true + certificateRefs: + - name: "my-tls-secret" + kind: "Secret" +``` + +#### Option 2: cert-manager Integration +```yaml +gateway: + enabled: true + host: "webhooks.example.com" + annotations: + cert-manager.io/cluster-issuer: "letsencrypt-prod" + tls: + enabled: true + certificateRefs: + - name: "webhook-tls-cert" # cert-manager will create this + kind: "Secret" +``` + +Then create a Certificate resource: +```yaml +apiVersion: cert-manager.io/v1 +kind: Certificate +metadata: + name: webhook-tls-cert + namespace: kelos-system +spec: + secretName: webhook-tls-cert + issuerRef: + name: letsencrypt-prod + kind: ClusterIssuer + dnsNames: + - webhooks.example.com +``` + +## Webhook Endpoints + +When Gateway API is enabled, webhooks are available at: + +- **GitHub**: `https://webhooks.your-domain.com/webhook/github` + +## Deployment + +```bash +# Deploy with Gateway API enabled +helm upgrade --install kelos ./internal/manifests/charts/kelos \ + --namespace kelos-system \ + --create-namespace \ + --values webhook-gateway-values.yaml +``` + +## Verification + +1. **Check Gateway status**: + ```bash + kubectl get gateway kelos-webhook-gateway -n kelos-system + kubectl describe gateway kelos-webhook-gateway -n kelos-system + ``` + +2. **Check HTTPRoute status**: + ```bash + kubectl get httproute kelos-webhook-routes -n kelos-system + kubectl describe httproute kelos-webhook-routes -n kelos-system + ``` + +3. **Test webhook endpoint**: + ```bash + curl -I https://webhooks.your-domain.com/webhook/github + ``` + +## Gateway API vs Ingress + +| Feature | Ingress | Gateway API | +|---------|---------|-------------| +| **Maturity** | Stable, widely supported | Newer, growing adoption | +| **Flexibility** | Basic HTTP routing | Advanced routing, protocol support | +| **Multi-tenancy** | Limited | Built-in namespace isolation | +| **Traffic splitting** | Extension-specific | Native support | +| **Protocol support** | HTTP/HTTPS mainly | HTTP, HTTPS, TCP, UDP, gRPC | +| **Vendor lock-in** | Controller-specific annotations | Standardized API | + +## Troubleshooting + +### Gateway Not Ready +```bash +# Check gateway controller logs +kubectl logs -n istio-system deployment/istio-gateway + +# Check gateway events +kubectl get events --field-selector involvedObject.name=kelos-webhook-gateway -n kelos-system +``` + +### Routes Not Working +```bash +# Check HTTPRoute status +kubectl describe httproute kelos-webhook-routes -n kelos-system + +# Verify backend services exist +kubectl get svc -l app.kubernetes.io/component=webhook-github -n kelos-system +``` + +### TLS Issues +```bash +# Check certificate status (if using cert-manager) +kubectl get certificate webhook-tls-cert -n kelos-system +kubectl describe certificate webhook-tls-cert -n kelos-system + +# Check secret exists +kubectl get secret webhook-tls-cert -n kelos-system +``` diff --git a/examples/helm-values-webhook.yaml b/examples/helm-values-webhook.yaml new file mode 100644 index 00000000..9bb6eef1 --- /dev/null +++ b/examples/helm-values-webhook.yaml @@ -0,0 +1,57 @@ +# Example Helm values to enable webhook servers + +# Enable webhook server for GitHub +webhookServer: + sources: + # Enable GitHub webhook server + github: + enabled: true + replicas: 2 # For high availability + secretName: github-webhook-secret # Must contain WEBHOOK_SECRET key + + # Enable ingress for external webhook access (traditional Ingress API) + ingress: + enabled: true + className: "nginx" # Or your ingress class + host: "webhooks.your-domain.com" + + # Enable TLS for secure HTTPS connections + tls: + enabled: true + # secretName: "" # Leave empty for cert-manager auto-generation + # secretName: "webhook-tls-secret" # Or specify existing secret name + + annotations: + cert-manager.io/cluster-issuer: "letsencrypt-prod" + nginx.ingress.kubernetes.io/ssl-redirect: "true" + nginx.ingress.kubernetes.io/force-ssl-redirect: "true" + + # Alternative: Enable Gateway API for external webhook access (next-gen routing) + # Note: Only enable one of ingress OR gateway, not both + gateway: + enabled: false # Set to true to use Gateway API instead of Ingress + gatewayClassName: "istio" # Or your gateway class (e.g., "istio", "envoy-gateway", "kong") + gatewayName: "kelos-webhook-gateway" + host: "webhooks.your-domain.com" + + # Enable TLS for secure HTTPS connections + tls: + enabled: true + # Reference to TLS certificates (Gateway API style) + certificateRefs: + - name: "webhook-tls-cert" + kind: "Secret" + # group: "" # Optional, defaults to core API group + + annotations: + # Gateway-specific annotations + cert-manager.io/cluster-issuer: "letsencrypt-prod" + +# Use versioned images for production +image: + tag: "v1.0.0" # Replace with your desired version + pullPolicy: "IfNotPresent" + +# Enable telemetry for monitoring +telemetry: + enabled: true diff --git a/examples/webhook-concurrency.md b/examples/webhook-concurrency.md new file mode 100644 index 00000000..81a20300 --- /dev/null +++ b/examples/webhook-concurrency.md @@ -0,0 +1,174 @@ +# Webhook TaskSpawner Concurrency Control + +This document explains how `maxConcurrency` works for webhook-driven TaskSpawners and the improved behavior. + +## Overview + +Webhook TaskSpawners support concurrency limits via the `maxConcurrency` field, but the behavior differs from polling-based TaskSpawners due to their event-driven nature. + +## How It Works + +### activeTasks Counting +- **Polling TaskSpawners**: The spawner process continuously counts active tasks +- **Webhook TaskSpawners**: The `kelos-controller` updates `activeTasks` when Tasks change status +- **Eventually Consistent**: There may be brief periods where the count is slightly stale + +### Concurrency Enforcement +```yaml +apiVersion: kelos.dev/v1alpha1 +kind: TaskSpawner +metadata: + name: github-webhook-limited +spec: + maxConcurrency: 3 # Limit to 3 concurrent tasks + when: + githubWebhook: + events: ["issues", "issue_comment"] + filters: + - event: "issues" + action: "opened" +``` + +### Behavior When Limit Exceeded + +**✅ Improved Behavior (Current)** +- Webhook is **accepted** with HTTP 200 OK +- Event processing continues for other TaskSpawners +- Task creation is **skipped** with clear logging +- GitHub sees successful delivery (no retries) + +**❌ Previous Behavior** +- Webhook returned HTTP 503 Service Unavailable +- Could cause unwanted retry storms +- Didn't work reliably with GitHub's retry logic + +## Example Scenarios + +### Scenario 1: Normal Operation +``` +Current activeTasks: 2 +maxConcurrency: 3 +Incoming webhook: ✅ Creates new task (activeTasks becomes 3) +``` + +### Scenario 2: At Limit +``` +Current activeTasks: 3 +maxConcurrency: 3 +Incoming webhook: ⚠️ Logs "Max concurrency reached, dropping webhook event" +Response: HTTP 200 OK (webhook accepted but task skipped) +``` + +### Scenario 3: Eventually Consistent Update +``` +1. Task completes → activeTasks should become 2 +2. Brief delay while kelos-controller reconciles +3. New webhook arrives → May see stale activeTasks=3 temporarily +4. Controller updates → activeTasks becomes 2 +5. Next webhook → Will see correct count +``` + +## Monitoring Concurrency + +### Logs +Look for these log messages: +``` +# Normal task creation +level=info msg="Successfully created task from webhook" spawner=my-spawner + +# Concurrency limit hit +level=info msg="Max concurrency reached, dropping webhook event" + activeTasks=3 maxConcurrency=3 + reason="Webhook accepted but task creation skipped due to concurrency limits" +``` + +### TaskSpawner Status +Check the status for current counts: +```bash +kubectl get taskspawner github-webhook-limited -o yaml +``` + +```yaml +status: + phase: Running + activeTasks: 3 + totalTasksCreated: 15 + message: "Webhook-driven TaskSpawner ready" +``` + +### Task Labels +All webhook tasks are labeled for easy filtering: +```bash +# Count active tasks for a specific TaskSpawner +kubectl get tasks -l kelos.dev/taskspawner=github-webhook-limited \ + --field-selector=status.phase!=Succeeded,status.phase!=Failed + +# View recent webhook tasks +kubectl get tasks -l kelos.dev/source-kind=webhook --sort-by=.metadata.creationTimestamp +``` + +## Best Practices + +### 1. **Choose Appropriate Limits** +```yaml +# For high-traffic repositories +maxConcurrency: 10 + +# For resource-constrained environments +maxConcurrency: 2 + +# For development/testing +maxConcurrency: 1 +``` + +### 2. **Monitor Both Sides** +- **Kubernetes**: TaskSpawner status and Task resources +- **GitHub**: Webhook delivery logs for any failures + +### 3. **Handle Bursts Gracefully** +```yaml +# Use reasonable concurrency limits +maxConcurrency: 5 + +# Set task cleanup timeouts +taskTemplate: + ttlSecondsAfterFinished: 3600 # Clean up after 1 hour +``` + +### 4. **Filter Judiciously** +```yaml +# Reduce webhook volume with good filtering +githubWebhook: + events: ["issues"] # Don't listen to every event type + filters: + - event: "issues" + action: "opened" # Only new issues + excludeLabels: ["wontfix", "duplicate"] # Skip certain labels +``` + +## Troubleshooting + +### Issue: Tasks Not Being Created +**Check**: TaskSpawner may be at concurrency limit +```bash +kubectl describe taskspawner +kubectl logs -l app.kubernetes.io/name=kelos,app.kubernetes.io/component=webhook-github +``` + +### Issue: activeTasks Count Seems Wrong +**Cause**: Eventually consistent updates +**Solution**: Wait a few seconds for controller reconciliation or trigger manually: +```bash +kubectl patch taskspawner -p '{"spec":{"suspend":false}}' +``` + +### Issue: Webhook Retries +**Check**: Should not happen with current implementation +**If it does**: Verify webhook server is returning HTTP 200, not 503 + +## Migration from Old Behavior + +If upgrading from a version that returned HTTP 503: +- **No action required** - behavior automatically improves +- **Monitor webhook delivery logs** - should see fewer retries +- **Adjust concurrency limits** if needed - can be more aggressive now diff --git a/examples/webhook-gateway-values.yaml b/examples/webhook-gateway-values.yaml new file mode 100644 index 00000000..70979507 --- /dev/null +++ b/examples/webhook-gateway-values.yaml @@ -0,0 +1,51 @@ +# Example Helm values for Kelos webhooks using Gateway API +# This is an alternative to the traditional Ingress configuration + +# Enable webhook servers +webhookServer: + sources: + # Enable GitHub webhook server + github: + enabled: true + replicas: 2 + secretName: github-webhook-secret + + # Configure resources for webhook pods + resources: + requests: + cpu: 50m + memory: 128Mi + limits: + cpu: 500m + memory: 256Mi + + # Disable traditional Ingress (using Gateway API instead) + ingress: + enabled: false + + # Enable Gateway API for advanced routing + gateway: + enabled: true + gatewayClassName: "istio" # Change to your gateway class + gatewayName: "kelos-webhook-gateway" + host: "webhooks.your-domain.com" + + # Enable TLS termination + tls: + enabled: true + certificateRefs: + - name: "webhook-tls-cert" + kind: "Secret" + + # Optional annotations (e.g., for cert-manager) + annotations: + cert-manager.io/cluster-issuer: "letsencrypt-prod" + +# Use specific image version for production +image: + tag: "v1.0.0" + pullPolicy: "IfNotPresent" + +# Enable telemetry for monitoring +telemetry: + enabled: true diff --git a/go.mod b/go.mod index ffce2050..22d7c3b8 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.25.0 require ( github.com/go-logr/logr v1.4.3 + github.com/google/go-github/v66 v66.0.0 github.com/google/uuid v1.6.0 github.com/google/yamlfmt v0.21.0 github.com/onsi/ginkgo/v2 v2.27.2 @@ -13,6 +14,7 @@ require ( github.com/prometheus/client_model v0.6.2 github.com/robfig/cron/v3 v3.0.1 github.com/spf13/cobra v1.10.2 + github.com/stretchr/testify v1.11.1 go.uber.org/zap v1.27.0 golang.org/x/mod v0.31.0 helm.sh/helm/v3 v3.20.1 @@ -54,6 +56,7 @@ require ( github.com/google/btree v1.1.3 // indirect github.com/google/gnostic-models v0.7.0 // indirect github.com/google/go-cmp v0.7.0 // indirect + github.com/google/go-querystring v1.1.0 // indirect github.com/google/pprof v0.0.0-20250403155104-27863c87afa6 // indirect github.com/google/renameio/v2 v2.0.0 // indirect github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect diff --git a/go.sum b/go.sum index e03035e3..7bd5416c 100644 --- a/go.sum +++ b/go.sum @@ -88,8 +88,13 @@ github.com/google/cel-go v0.26.0 h1:DPGjXackMpJWH680oGY4lZhYjIameYmR+/6RBdDGmaI= github.com/google/cel-go v0.26.0/go.mod h1:A9O8OU9rdvrK5MQyrqfIxo1a0u4g3sF8KB6PUIaryMM= github.com/google/gnostic-models v0.7.0 h1:qwTtogB15McXDaNqTZdzPJRHvaVJlAl+HVQnLmJEJxo= github.com/google/gnostic-models v0.7.0/go.mod h1:whL5G0m6dmc5cPxKc5bdKdEN3UjI7OUGxBlw57miDrQ= +github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/go-github/v66 v66.0.0 h1:ADJsaXj9UotwdgK8/iFZtv7MLc8E8WBl62WLd/D/9+M= +github.com/google/go-github/v66 v66.0.0/go.mod h1:+4SO9Zkuyf8ytMj0csN1NR/5OTR+MfqPp8P8dVlcvY4= +github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= +github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0= github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= @@ -274,6 +279,7 @@ golang.org/x/tools/go/expect v0.1.1-deprecated h1:jpBZDwmgPhXsKZC6WhL20P4b/wmnps golang.org/x/tools/go/expect v0.1.1-deprecated/go.mod h1:eihoPOH+FgIqa3FpoTwguz/bVUSGBlGQU67vpBeOrBY= golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated h1:1h2MnaIAIXISqTFKdENegdpAgUXz6NrPEsbIeWaBRvM= golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated/go.mod h1:RVAQXBGNv1ib0J382/DPCRS/BPnsGebyM1Gj5VSDpG8= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gomodules.xyz/jsonpatch/v2 v2.4.0 h1:Ci3iUJyx9UeRx7CeFN8ARgGbkESwJK+KB9lLcWxY/Zw= gomodules.xyz/jsonpatch/v2 v2.4.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY= google.golang.org/genproto v0.0.0-20241118233622-e639e219e697 h1:ToEetK57OidYuqD4Q5w+vfEnPvPpuTwedCNVohYJfNk= diff --git a/internal/cli/install.go b/internal/cli/install.go index 83e6c4a1..ebb6edb8 100644 --- a/internal/cli/install.go +++ b/internal/cli/install.go @@ -38,6 +38,8 @@ func newInstallCommand(cfg *ClientConfig) *cobra.Command { var disableHeartbeat bool var spawnerResourceRequests string var spawnerResourceLimits string + var ghproxyResourceRequests string + var ghproxyResourceLimits string var tokenRefresherResourceRequests string var tokenRefresherResourceLimits string var controllerResourceRequests string @@ -53,12 +55,14 @@ func newInstallCommand(cfg *ClientConfig) *cobra.Command { version.Version = flagVersion } - vals := disableChartCRDs(buildHelmValues( + vals := disableChartCRDs(buildHelmValuesWithGHProxyResources( version.Version, imagePullPolicy, disableHeartbeat, spawnerResourceRequests, spawnerResourceLimits, + ghproxyResourceRequests, + ghproxyResourceLimits, tokenRefresherResourceRequests, tokenRefresherResourceLimits, controllerResourceRequests, @@ -116,6 +120,8 @@ func newInstallCommand(cfg *ClientConfig) *cobra.Command { cmd.Flags().BoolVar(&disableHeartbeat, "disable-heartbeat", false, "do not install the telemetry heartbeat CronJob") cmd.Flags().StringVar(&spawnerResourceRequests, "spawner-resource-requests", "", "resource requests for spawner containers (e.g., cpu=250m,memory=512Mi)") cmd.Flags().StringVar(&spawnerResourceLimits, "spawner-resource-limits", "", "resource limits for spawner containers (e.g., cpu=1,memory=1Gi)") + cmd.Flags().StringVar(&ghproxyResourceRequests, "ghproxy-resource-requests", "", "resource requests for workspace ghproxy containers (e.g., cpu=50m,memory=64Mi)") + cmd.Flags().StringVar(&ghproxyResourceLimits, "ghproxy-resource-limits", "", "resource limits for workspace ghproxy containers (e.g., cpu=200m,memory=128Mi)") cmd.Flags().StringVar(&tokenRefresherResourceRequests, "token-refresher-resource-requests", "", "resource requests for token refresher sidecars (e.g., cpu=100m,memory=128Mi)") cmd.Flags().StringVar(&tokenRefresherResourceLimits, "token-refresher-resource-limits", "", "resource limits for token refresher sidecars (e.g., cpu=200m,memory=256Mi)") cmd.Flags().StringVar(&controllerResourceRequests, "controller-resource-requests", "", "resource requests for the controller container (e.g., cpu=10m,memory=64Mi)") @@ -127,6 +133,10 @@ func newInstallCommand(cfg *ClientConfig) *cobra.Command { // buildHelmValues constructs the values map for Helm chart rendering from CLI flags. func buildHelmValues(ver string, pullPolicy string, disableHeartbeat bool, spawnerResourceRequests string, spawnerResourceLimits string, tokenRefresherResourceRequests string, tokenRefresherResourceLimits string, controllerResourceRequests string, controllerResourceLimits string, ghproxyAllowedUpstreams string) map[string]interface{} { + return buildHelmValuesWithGHProxyResources(ver, pullPolicy, disableHeartbeat, spawnerResourceRequests, spawnerResourceLimits, "", "", tokenRefresherResourceRequests, tokenRefresherResourceLimits, controllerResourceRequests, controllerResourceLimits, ghproxyAllowedUpstreams) +} + +func buildHelmValuesWithGHProxyResources(ver string, pullPolicy string, disableHeartbeat bool, spawnerResourceRequests string, spawnerResourceLimits string, ghproxyResourceRequests string, ghproxyResourceLimits string, tokenRefresherResourceRequests string, tokenRefresherResourceLimits string, controllerResourceRequests string, controllerResourceLimits string, ghproxyAllowedUpstreams string) map[string]interface{} { imageVals := map[string]interface{}{ "tag": ver, } @@ -153,6 +163,21 @@ func buildHelmValues(ver string, pullPolicy string, disableHeartbeat bool, spawn "resources": spawnerResources, } } + ghproxyResources := map[string]interface{}{} + if ghproxyResourceRequests != "" { + ghproxyResources["requests"] = ghproxyResourceRequests + } + if ghproxyResourceLimits != "" { + ghproxyResources["limits"] = ghproxyResourceLimits + } + if len(ghproxyResources) > 0 { + ghproxyVals, _ := vals["ghproxy"].(map[string]interface{}) + if ghproxyVals == nil { + ghproxyVals = map[string]interface{}{} + } + ghproxyVals["resources"] = ghproxyResources + vals["ghproxy"] = ghproxyVals + } tokenRefresherResources := map[string]interface{}{} if tokenRefresherResourceRequests != "" { tokenRefresherResources["requests"] = tokenRefresherResourceRequests @@ -178,9 +203,12 @@ func buildHelmValues(ver string, pullPolicy string, disableHeartbeat bool, spawn } } if ghproxyAllowedUpstreams != "" { - vals["ghproxy"] = map[string]interface{}{ - "allowedUpstreams": ghproxyAllowedUpstreams, + ghproxyVals, _ := vals["ghproxy"].(map[string]interface{}) + if ghproxyVals == nil { + ghproxyVals = map[string]interface{}{} } + ghproxyVals["allowedUpstreams"] = ghproxyAllowedUpstreams + vals["ghproxy"] = ghproxyVals } return vals } diff --git a/internal/cli/webhook_test.go b/internal/cli/webhook_test.go new file mode 100644 index 00000000..ad856ff7 --- /dev/null +++ b/internal/cli/webhook_test.go @@ -0,0 +1,142 @@ +package cli + +import ( + "strings" + "testing" + + "github.com/kelos-dev/kelos/internal/helmchart" + "github.com/kelos-dev/kelos/internal/manifests" +) + +func TestRenderChart_WebhookServers(t *testing.T) { + vals := map[string]interface{}{ + "webhookServer": map[string]interface{}{ + "image": "ghcr.io/kelos-dev/kelos-webhook-server", + "sources": map[string]interface{}{ + "github": map[string]interface{}{ + "enabled": true, + "replicas": 1, + "secretName": "github-webhook-secret", + }, + }, + "ingress": map[string]interface{}{ + "enabled": true, + "className": "nginx", + "host": "webhooks.example.com", + }, + }, + "image": map[string]interface{}{ + "tag": "latest", + }, + } + + data, err := helmchart.Render(manifests.ChartFS, vals) + if err != nil { + t.Fatalf("rendering chart: %v", err) + } + + content := string(data) + + // Check for webhook components + expectedComponents := []string{ + "name: kelos-webhook-github", + "kind: Ingress", + "name: kelos-webhook-role", + "name: kelos-webhook", + "app.kubernetes.io/component: webhook-github", + "--source=github", + "github-webhook-secret", + } + + for _, component := range expectedComponents { + if !strings.Contains(content, component) { + t.Errorf("expected rendered chart to contain %q", component) + } + } +} + +func TestRenderChart_WebhookGateway(t *testing.T) { + vals := map[string]interface{}{ + "webhookServer": map[string]interface{}{ + "image": "ghcr.io/kelos-dev/kelos-webhook-server", + "sources": map[string]interface{}{ + "github": map[string]interface{}{ + "enabled": true, + "replicas": 1, + "secretName": "github-webhook-secret", + }, + }, + "gateway": map[string]interface{}{ + "enabled": true, + "gatewayClassName": "istio", + "gatewayName": "kelos-webhook-gateway", + "host": "webhooks.example.com", + "tls": map[string]interface{}{ + "enabled": true, + }, + }, + }, + "image": map[string]interface{}{ + "tag": "latest", + }, + } + + data, err := helmchart.Render(manifests.ChartFS, vals) + if err != nil { + t.Fatalf("rendering chart: %v", err) + } + + content := string(data) + + // Check for Gateway API components + expectedComponents := []string{ + "kind: Gateway", + "kind: HTTPRoute", + "name: kelos-webhook-gateway", + "name: kelos-webhook-routes", + "gatewayClassName: istio", + "/webhook/github", + "kelos-webhook-github", + } + + for _, component := range expectedComponents { + if !strings.Contains(content, component) { + t.Errorf("expected rendered chart to contain %q", component) + } + } +} + +func TestRenderChart_WebhookServersDisabled(t *testing.T) { + vals := map[string]interface{}{ + "webhookServer": map[string]interface{}{ + "sources": map[string]interface{}{ + "github": map[string]interface{}{ + "enabled": false, + }, + }, + }, + "image": map[string]interface{}{ + "tag": "latest", + }, + } + + data, err := helmchart.Render(manifests.ChartFS, vals) + if err != nil { + t.Fatalf("rendering chart: %v", err) + } + + content := string(data) + + // Should not contain webhook components when disabled + unexpectedComponents := []string{ + "name: kelos-webhook-github", + "--source=github", + "name: kelos-webhook-role", + } + + for _, component := range unexpectedComponents { + if strings.Contains(content, component) { + t.Errorf("did not expect rendered chart to contain %q when webhooks disabled", component) + } + } +} diff --git a/internal/controller/taskspawner_controller.go b/internal/controller/taskspawner_controller.go index 970c2bb6..abe9b050 100644 --- a/internal/controller/taskspawner_controller.go +++ b/internal/controller/taskspawner_controller.go @@ -2,6 +2,7 @@ package controller import ( "context" + "fmt" "reflect" "time" @@ -16,10 +17,12 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/client-go/util/retry" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" kelosv1alpha1 "github.com/kelos-dev/kelos/api/v1alpha1" @@ -54,6 +57,11 @@ func isCronBased(ts *kelosv1alpha1.TaskSpawner) bool { return ts.Spec.When.Cron != nil } +// isWebhookBased returns true if the TaskSpawner is webhook-driven. +func isWebhookBased(ts *kelosv1alpha1.TaskSpawner) bool { + return ts.Spec.When.GitHubWebhook != nil +} + // Reconcile handles TaskSpawner reconciliation. func (r *TaskSpawnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { logger := log.FromContext(ctx) @@ -96,9 +104,87 @@ func (r *TaskSpawnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) return r.reconcileCronJob(ctx, req, &ts, isSuspended) } + // Webhook-based TaskSpawners don't need deployments or cronjobs. + if isWebhookBased(&ts) { + return r.reconcileWebhook(ctx, req, &ts, isSuspended) + } + return r.reconcileDeployment(ctx, req, &ts, isSuspended) } +// reconcileWebhook handles webhook-based TaskSpawners by cleaning up any stale resources. +func (r *TaskSpawnerReconciler) reconcileWebhook(ctx context.Context, req ctrl.Request, ts *kelosv1alpha1.TaskSpawner, isSuspended bool) (ctrl.Result, error) { + logger := log.FromContext(ctx) + + // Clean up any stale Deployment or CronJob from previous configurations + if err := r.deleteStaleResource(ctx, req.NamespacedName, &appsv1.Deployment{}, "Deployment"); err != nil { + return ctrl.Result{}, err + } + if err := r.deleteStaleResource(ctx, req.NamespacedName, &batchv1.CronJob{}, "CronJob"); err != nil { + return ctrl.Result{}, err + } + + // Determine the desired phase for webhook TaskSpawners + desiredPhase := kelosv1alpha1.TaskSpawnerPhaseRunning + desiredMessage := "Webhook-driven TaskSpawner ready" + if isSuspended { + desiredPhase = kelosv1alpha1.TaskSpawnerPhaseSuspended + desiredMessage = "Suspended by user" + } + + // Count active tasks for this webhook TaskSpawner + activeTasks, err := r.countActiveTasks(ctx, req.NamespacedName) + if err != nil { + logger.Error(err, "Failed to count active tasks") + return ctrl.Result{}, err + } + + // Update status to clear any deployment/cronjob names and set appropriate phase + needsStatusUpdate := ts.Status.DeploymentName != "" || ts.Status.CronJobName != "" || + ts.Status.Phase != desiredPhase || ts.Status.ActiveTasks != activeTasks + if needsStatusUpdate { + if err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + if getErr := r.Get(ctx, req.NamespacedName, ts); getErr != nil { + return getErr + } + ts.Status.DeploymentName = "" + ts.Status.CronJobName = "" + ts.Status.Phase = desiredPhase + ts.Status.Message = desiredMessage + ts.Status.ActiveTasks = activeTasks + return r.Status().Update(ctx, ts) + }); err != nil { + logger.Error(err, "Unable to update TaskSpawner status") + return ctrl.Result{}, err + } + logger.Info("Updated webhook TaskSpawner status", "phase", desiredPhase, "activeTasks", activeTasks) + } + + return ctrl.Result{}, nil +} + +// countActiveTasks counts the number of active (non-terminal) Tasks for a TaskSpawner. +func (r *TaskSpawnerReconciler) countActiveTasks(ctx context.Context, taskSpawnerName types.NamespacedName) (int, error) { + var taskList kelosv1alpha1.TaskList + if err := r.List(ctx, &taskList, + client.InNamespace(taskSpawnerName.Namespace), + client.MatchingLabels{"kelos.dev/taskspawner": taskSpawnerName.Name}, + ); err != nil { + return 0, fmt.Errorf("listing tasks for TaskSpawner %s: %w", taskSpawnerName.Name, err) + } + + activeTasks := 0 + for i := range taskList.Items { + task := &taskList.Items[i] + // Count tasks that are not in terminal phases + if task.Status.Phase != kelosv1alpha1.TaskPhaseSucceeded && task.Status.Phase != kelosv1alpha1.TaskPhaseFailed { + activeTasks++ + } + } + + return activeTasks, nil +} + // reconcileDeployment handles the Deployment lifecycle for polling-based TaskSpawners. func (r *TaskSpawnerReconciler) reconcileDeployment(ctx context.Context, req ctrl.Request, ts *kelosv1alpha1.TaskSpawner, isSuspended bool) (ctrl.Result, error) { logger := log.FromContext(ctx) @@ -175,6 +261,30 @@ func (r *TaskSpawnerReconciler) reconcileDeployment(ctx context.Context, req ctr } } } + if err := validateWorkspaceGHProxyRepoOverride(ts, workspace); err != nil { + if deployExists { + if deleteErr := r.Delete(ctx, &deploy); deleteErr != nil && !apierrors.IsNotFound(deleteErr) { + logger.Error(deleteErr, "Unable to delete Deployment for invalid repo override", "deployment", deploy.Name) + return ctrl.Result{}, deleteErr + } + deployExists = false + } + r.recordEvent(ts, corev1.EventTypeWarning, "InvalidGitHubRepoOverride", "%s", err.Error()) + if statusErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { + if getErr := r.Get(ctx, req.NamespacedName, ts); getErr != nil { + return getErr + } + ts.Status.Phase = kelosv1alpha1.TaskSpawnerPhaseFailed + ts.Status.Message = err.Error() + ts.Status.DeploymentName = "" + ts.Status.CronJobName = "" + return r.Status().Update(ctx, ts) + }); statusErr != nil { + logger.Error(statusErr, "Unable to update TaskSpawner status for invalid repo override") + return ctrl.Result{}, statusErr + } + return ctrl.Result{}, nil + } // Determine desired replica count based on suspend state desiredReplicas := int32(1) @@ -288,6 +398,30 @@ func (r *TaskSpawnerReconciler) reconcileCronJob(ctx context.Context, req ctrl.R } } } + if err := validateWorkspaceGHProxyRepoOverride(ts, workspace); err != nil { + if cronJobExists { + if deleteErr := r.Delete(ctx, &cronJob); deleteErr != nil && !apierrors.IsNotFound(deleteErr) { + logger.Error(deleteErr, "Unable to delete CronJob for invalid repo override", "cronJob", cronJob.Name) + return ctrl.Result{}, deleteErr + } + cronJobExists = false + } + r.recordEvent(ts, corev1.EventTypeWarning, "InvalidGitHubRepoOverride", "%s", err.Error()) + if statusErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { + if getErr := r.Get(ctx, req.NamespacedName, ts); getErr != nil { + return getErr + } + ts.Status.Phase = kelosv1alpha1.TaskSpawnerPhaseFailed + ts.Status.Message = err.Error() + ts.Status.DeploymentName = "" + ts.Status.CronJobName = "" + return r.Status().Update(ctx, ts) + }); statusErr != nil { + logger.Error(statusErr, "Unable to update TaskSpawner status for invalid repo override") + return ctrl.Result{}, statusErr + } + return ctrl.Result{}, nil + } if !cronJobExists { return r.createCronJob(ctx, ts, workspace, isGitHubApp, isSuspended) @@ -565,9 +699,9 @@ func (r *TaskSpawnerReconciler) updateCronJob(ctx context.Context, ts *kelosv1al return nil } -// deleteStaleResource deletes a resource by NamespacedName if it exists. +// deleteStaleResource deletes a resource by NamespacedName if it exists and is owned by a TaskSpawner. // This is used to clean up the old resource type when switching between -// Deployment-based and CronJob-based TaskSpawners. +// Deployment-based, CronJob-based, and webhook-based TaskSpawners. func (r *TaskSpawnerReconciler) deleteStaleResource(ctx context.Context, key types.NamespacedName, obj client.Object, kind string) error { logger := log.FromContext(ctx) @@ -578,6 +712,21 @@ func (r *TaskSpawnerReconciler) deleteStaleResource(ctx context.Context, key typ return err } + // Only delete if this resource is owned by a TaskSpawner to avoid deleting unrelated resources + ownerRefs := obj.GetOwnerReferences() + isOwnedByTaskSpawner := false + for _, ref := range ownerRefs { + if ref.APIVersion == "kelos.dev/v1alpha1" && ref.Kind == "TaskSpawner" && ref.Controller != nil && *ref.Controller { + isOwnedByTaskSpawner = true + break + } + } + + if !isOwnedByTaskSpawner { + logger.Info("Skipping deletion of "+kind+" not owned by TaskSpawner", "name", key.Name) + return nil + } + if err := r.Delete(ctx, obj); err != nil { if apierrors.IsNotFound(err) { return nil @@ -708,6 +857,11 @@ func (r *TaskSpawnerReconciler) SetupWithManager(mgr ctrl.Manager) error { Owns(&batchv1.CronJob{}). Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.findTaskSpawnersForSecret)). Watches(&kelosv1alpha1.Workspace{}, handler.EnqueueRequestsFromMapFunc(r.findTaskSpawnersForWorkspace)). + Watches(&kelosv1alpha1.Task{}, handler.EnqueueRequestsFromMapFunc(r.findTaskSpawnersForTask), + builder.WithPredicates(predicate.NewPredicateFuncs(func(obj client.Object) bool { + _, hasLabel := obj.GetLabels()["kelos.dev/taskspawner"] + return hasLabel + }))). Complete(r) } @@ -787,6 +941,28 @@ func (r *TaskSpawnerReconciler) findTaskSpawnersForWorkspace(ctx context.Context return requests } +// findTaskSpawnersForTask maps a Task change to the TaskSpawner that owns it. +// This allows webhook TaskSpawners to update their activeTasks count when Tasks complete/fail. +func (r *TaskSpawnerReconciler) findTaskSpawnersForTask(ctx context.Context, obj client.Object) []reconcile.Request { + task, ok := obj.(*kelosv1alpha1.Task) + if !ok { + return nil + } + + // Tasks have a label that identifies their TaskSpawner + taskSpawnerName, ok := task.Labels["kelos.dev/taskspawner"] + if !ok { + return nil + } + + return []reconcile.Request{{ + NamespacedName: types.NamespacedName{ + Namespace: task.Namespace, + Name: taskSpawnerName, + }, + }} +} + // resourceRequirementsEqual compares two ResourceRequirements using semantic // equality for quantities instead of reflect.DeepEqual, which can report false // negatives when the internal representation of equal quantities differs. diff --git a/internal/controller/taskspawner_controller_test.go b/internal/controller/taskspawner_controller_test.go new file mode 100644 index 00000000..22588beb --- /dev/null +++ b/internal/controller/taskspawner_controller_test.go @@ -0,0 +1,261 @@ +package controller + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + kelosv1alpha1 "github.com/kelos-dev/kelos/api/v1alpha1" +) + +func TestIsWebhookBased(t *testing.T) { + tests := []struct { + name string + ts *kelosv1alpha1.TaskSpawner + want bool + }{ + { + name: "GitHub webhook TaskSpawner", + ts: &kelosv1alpha1.TaskSpawner{ + Spec: kelosv1alpha1.TaskSpawnerSpec{ + When: kelosv1alpha1.When{ + GitHubWebhook: &kelosv1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + }, + }, + }, + }, + want: true, + }, + { + name: "polling TaskSpawner", + ts: &kelosv1alpha1.TaskSpawner{ + Spec: kelosv1alpha1.TaskSpawnerSpec{ + When: kelosv1alpha1.When{ + GitHubIssues: &kelosv1alpha1.GitHubIssues{}, + }, + }, + }, + want: false, + }, + { + name: "cron TaskSpawner", + ts: &kelosv1alpha1.TaskSpawner{ + Spec: kelosv1alpha1.TaskSpawnerSpec{ + When: kelosv1alpha1.When{ + Cron: &kelosv1alpha1.Cron{ + Schedule: "0 9 * * 1", + }, + }, + }, + }, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isWebhookBased(tt.ts) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestReconcileWebhook(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, kelosv1alpha1.AddToScheme(scheme)) + require.NoError(t, appsv1.AddToScheme(scheme)) + require.NoError(t, batchv1.AddToScheme(scheme)) + + tests := []struct { + name string + ts *kelosv1alpha1.TaskSpawner + existingObjs []client.Object + isSuspended bool + wantPhase kelosv1alpha1.TaskSpawnerPhase + wantMessage string + wantDeployment bool + wantCronJob bool + }{ + { + name: "active webhook TaskSpawner", + ts: &kelosv1alpha1.TaskSpawner{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-webhook", + Namespace: "default", + }, + Spec: kelosv1alpha1.TaskSpawnerSpec{ + When: kelosv1alpha1.When{ + GitHubWebhook: &kelosv1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + }, + }, + }, + }, + isSuspended: false, + wantPhase: kelosv1alpha1.TaskSpawnerPhaseRunning, + wantMessage: "Webhook-driven TaskSpawner ready", + }, + { + name: "suspended webhook TaskSpawner", + ts: &kelosv1alpha1.TaskSpawner{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-webhook", + Namespace: "default", + }, + Spec: kelosv1alpha1.TaskSpawnerSpec{ + When: kelosv1alpha1.When{ + GitHubWebhook: &kelosv1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + }, + }, + }, + }, + isSuspended: true, + wantPhase: kelosv1alpha1.TaskSpawnerPhaseSuspended, + wantMessage: "Suspended by user", + }, + { + name: "webhook TaskSpawner with stale deployment", + ts: &kelosv1alpha1.TaskSpawner{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-webhook", + Namespace: "default", + }, + Spec: kelosv1alpha1.TaskSpawnerSpec{ + When: kelosv1alpha1.When{ + GitHubWebhook: &kelosv1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + }, + }, + }, + Status: kelosv1alpha1.TaskSpawnerStatus{ + DeploymentName: "test-webhook", + }, + }, + existingObjs: []client.Object{ + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-webhook", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "kelos.dev/v1alpha1", + Kind: "TaskSpawner", + Name: "test-webhook", + Controller: func() *bool { b := true; return &b }(), + }, + }, + }, + }, + }, + isSuspended: false, + wantPhase: kelosv1alpha1.TaskSpawnerPhaseRunning, + wantMessage: "Webhook-driven TaskSpawner ready", + wantDeployment: false, // Should be deleted + }, + { + name: "webhook TaskSpawner with stale cronjob", + ts: &kelosv1alpha1.TaskSpawner{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-webhook", + Namespace: "default", + }, + Spec: kelosv1alpha1.TaskSpawnerSpec{ + When: kelosv1alpha1.When{ + GitHubWebhook: &kelosv1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + }, + }, + }, + Status: kelosv1alpha1.TaskSpawnerStatus{ + CronJobName: "test-webhook", + }, + }, + existingObjs: []client.Object{ + &batchv1.CronJob{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-webhook", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "kelos.dev/v1alpha1", + Kind: "TaskSpawner", + Name: "test-webhook", + Controller: func() *bool { b := true; return &b }(), + }, + }, + }, + }, + }, + isSuspended: false, + wantPhase: kelosv1alpha1.TaskSpawnerPhaseRunning, + wantMessage: "Webhook-driven TaskSpawner ready", + wantCronJob: false, // Should be deleted + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + objs := append([]client.Object{tt.ts}, tt.existingObjs...) + client := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objs...). + WithStatusSubresource(&kelosv1alpha1.TaskSpawner{}). + Build() + + reconciler := &TaskSpawnerReconciler{ + Client: client, + Scheme: scheme, + } + + req := ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: tt.ts.Name, + Namespace: tt.ts.Namespace, + }, + } + + _, err := reconciler.reconcileWebhook(context.Background(), req, tt.ts, tt.isSuspended) + require.NoError(t, err) + + // Check final TaskSpawner status + var finalTs kelosv1alpha1.TaskSpawner + err = client.Get(context.Background(), req.NamespacedName, &finalTs) + require.NoError(t, err) + + assert.Equal(t, tt.wantPhase, finalTs.Status.Phase) + assert.Equal(t, tt.wantMessage, finalTs.Status.Message) + assert.Empty(t, finalTs.Status.DeploymentName, "DeploymentName should be cleared") + assert.Empty(t, finalTs.Status.CronJobName, "CronJobName should be cleared") + + // Check that stale resources are deleted + var deployment appsv1.Deployment + err = client.Get(context.Background(), req.NamespacedName, &deployment) + if tt.wantDeployment { + assert.NoError(t, err, "Deployment should exist") + } else { + assert.True(t, apierrors.IsNotFound(err), "Deployment should not exist") + } + + var cronJob batchv1.CronJob + err = client.Get(context.Background(), req.NamespacedName, &cronJob) + if tt.wantCronJob { + assert.NoError(t, err, "CronJob should exist") + } else { + assert.True(t, apierrors.IsNotFound(err), "CronJob should not exist") + } + }) + } +} diff --git a/internal/controller/taskspawner_deployment_builder.go b/internal/controller/taskspawner_deployment_builder.go index 04d9273c..2797c9a7 100644 --- a/internal/controller/taskspawner_deployment_builder.go +++ b/internal/controller/taskspawner_deployment_builder.go @@ -90,10 +90,13 @@ func (b *DeploymentBuilder) buildPodParts(ts *kelosv1alpha1.TaskSpawner, workspa "--github-owner="+owner, "--github-repo="+repo, ) + if ts.Spec.TaskTemplate.WorkspaceRef != nil { + args = append(args, "--gh-proxy-url="+WorkspaceGHProxyServiceURL(ts.Namespace, ts.Spec.TaskTemplate.WorkspaceRef.Name)) + } if apiBaseURL := gitHubAPIBaseURL(host); apiBaseURL != "" { args = append(args, "--github-api-base-url="+apiBaseURL) } - if workspace.SecretRef != nil { + if workspace.SecretRef != nil && taskSpawnerNeedsGitHubToken(ts) { if isGitHubApp { // GitHub App: add token refresher as a native sidecar args = append(args, "--github-token-file=/shared/token/GITHUB_TOKEN") @@ -416,6 +419,34 @@ func githubSourceRepoOverride(ts *kelosv1alpha1.TaskSpawner) string { return "" } +func taskSpawnerNeedsGitHubToken(ts *kelosv1alpha1.TaskSpawner) bool { + if ts.Spec.When.GitHubIssues != nil && ts.Spec.When.GitHubIssues.Reporting != nil && ts.Spec.When.GitHubIssues.Reporting.Enabled { + return true + } + if ts.Spec.When.GitHubPullRequests != nil && ts.Spec.When.GitHubPullRequests.Reporting != nil && ts.Spec.When.GitHubPullRequests.Reporting.Enabled { + return true + } + return false +} + +func validateWorkspaceGHProxyRepoOverride(ts *kelosv1alpha1.TaskSpawner, workspace *kelosv1alpha1.WorkspaceSpec) error { + if workspace == nil { + return nil + } + repoOverride := githubSourceRepoOverride(ts) + if repoOverride == "" { + return nil + } + + workspaceHost, _, _ := parseGitHubRepo(workspace.Repo) + overrideHost, _, _ := parseGitHubRepo(repoOverride) + if overrideHost == "" || workspaceHost == "" || overrideHost == workspaceHost { + return nil + } + + return fmt.Errorf("github source repo override host %q does not match Workspace host %q", overrideHost, workspaceHost) +} + // ParseResourceList parses a comma-separated "name=value" string into a // corev1.ResourceList. An empty string returns nil. Each value must pass // Kubernetes quantity parsing. diff --git a/internal/controller/taskspawner_deployment_builder_test.go b/internal/controller/taskspawner_deployment_builder_test.go index 00c7f545..71da56e0 100644 --- a/internal/controller/taskspawner_deployment_builder_test.go +++ b/internal/controller/taskspawner_deployment_builder_test.go @@ -197,6 +197,62 @@ func TestGitHubAPIBaseURL(t *testing.T) { } } +func TestValidateWorkspaceGHProxyRepoOverride(t *testing.T) { + tests := []struct { + name string + repo string + wantError bool + }{ + { + name: "same host full URL allowed", + repo: "https://github.com/other/repo.git", + wantError: false, + }, + { + name: "owner repo shorthand allowed", + repo: "other/repo", + wantError: false, + }, + { + name: "cross host rejected", + repo: "https://github.example.com/other/repo.git", + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := &kelosv1alpha1.TaskSpawner{ + Spec: kelosv1alpha1.TaskSpawnerSpec{ + When: kelosv1alpha1.When{ + GitHubIssues: &kelosv1alpha1.GitHubIssues{Repo: tt.repo}, + }, + }, + } + workspace := &kelosv1alpha1.WorkspaceSpec{ + Repo: "https://github.com/kelos-dev/kelos.git", + } + + err := validateWorkspaceGHProxyRepoOverride(ts, workspace) + if tt.wantError && err == nil { + t.Fatal("expected error, got nil") + } + if !tt.wantError && err != nil { + t.Fatalf("expected no error, got %v", err) + } + }) + } +} + +func enableGitHubReporting(ts *kelosv1alpha1.TaskSpawner) { + if ts.Spec.When.GitHubIssues != nil { + ts.Spec.When.GitHubIssues.Reporting = &kelosv1alpha1.GitHubReporting{Enabled: true} + } + if ts.Spec.When.GitHubPullRequests != nil { + ts.Spec.When.GitHubPullRequests.Reporting = &kelosv1alpha1.GitHubReporting{Enabled: true} + } +} + func TestBuildDeploymentWithEnterpriseURL(t *testing.T) { builder := NewDeploymentBuilder() @@ -274,6 +330,7 @@ func TestDeploymentBuilder_GitHubApp(t *testing.T) { }, }, } + enableGitHubReporting(ts) workspace := &kelosv1alpha1.WorkspaceSpec{ Repo: "https://github.com/kelos-dev/kelos.git", SecretRef: &kelosv1alpha1.SecretReference{ @@ -354,6 +411,7 @@ func TestDeploymentBuilder_GitHubAppEnterprise(t *testing.T) { }, }, } + enableGitHubReporting(ts) workspace := &kelosv1alpha1.WorkspaceSpec{ Repo: "https://github.example.com/my-org/my-repo.git", SecretRef: &kelosv1alpha1.SecretReference{ @@ -396,6 +454,7 @@ func TestDeploymentBuilder_GitHubAppGitHubCom(t *testing.T) { }, }, } + enableGitHubReporting(ts) workspace := &kelosv1alpha1.WorkspaceSpec{ Repo: "https://github.com/kelos-dev/kelos.git", SecretRef: &kelosv1alpha1.SecretReference{ @@ -446,6 +505,7 @@ func TestDeploymentBuilder_TokenRefresherResources(t *testing.T) { }, }, } + enableGitHubReporting(ts) workspace := &kelosv1alpha1.WorkspaceSpec{ Repo: "https://github.com/kelos-dev/kelos.git", SecretRef: &kelosv1alpha1.SecretReference{ @@ -485,6 +545,7 @@ func TestDeploymentBuilder_PAT(t *testing.T) { }, }, } + enableGitHubReporting(ts) workspace := &kelosv1alpha1.WorkspaceSpec{ Repo: "https://github.com/kelos-dev/kelos.git", SecretRef: &kelosv1alpha1.SecretReference{ @@ -1084,6 +1145,7 @@ func TestUpdateDeployment_PATToGitHubApp(t *testing.T) { }, }, } + enableGitHubReporting(ts) workspace := &kelosv1alpha1.WorkspaceSpec{ Repo: "https://github.com/kelos-dev/kelos.git", SecretRef: &kelosv1alpha1.SecretReference{ @@ -1181,6 +1243,7 @@ func TestUpdateDeployment_GitHubAppToPAT(t *testing.T) { }, }, } + enableGitHubReporting(ts) workspace := &kelosv1alpha1.WorkspaceSpec{ Repo: "https://github.com/kelos-dev/kelos.git", SecretRef: &kelosv1alpha1.SecretReference{ @@ -1845,6 +1908,14 @@ func TestReconcileCronJob_DeletesStaleDeployment(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "my-spawner", Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "kelos.dev/v1alpha1", + Kind: "TaskSpawner", + Name: "my-spawner", + Controller: func() *bool { b := true; return &b }(), + }, + }, }, Spec: appsv1.DeploymentSpec{ Selector: &metav1.LabelSelector{ @@ -1916,6 +1987,14 @@ func TestReconcileDeployment_DeletesStaleCronJob(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "my-spawner", Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "kelos.dev/v1alpha1", + Kind: "TaskSpawner", + Name: "my-spawner", + Controller: func() *bool { b := true; return &b }(), + }, + }, }, Spec: batchv1.CronJobSpec{ Schedule: "0 9 * * 1", @@ -2022,18 +2101,11 @@ func TestBuildCronJob_WithWorkspacePAT(t *testing.T) { t.Errorf("expected --one-shot in args, got: %v", spawner.Args) } - // Verify GITHUB_TOKEN env var is injected from PAT secret - foundTokenEnv := false for _, env := range spawner.Env { - if env.Name == "GITHUB_TOKEN" && env.ValueFrom != nil && - env.ValueFrom.SecretKeyRef != nil && - env.ValueFrom.SecretKeyRef.Name == "gh-pat-secret" { - foundTokenEnv = true + if env.Name == "GITHUB_TOKEN" { + t.Fatalf("expected no GITHUB_TOKEN env for cron workspace discovery-only spawner, got: %v", spawner.Env) } } - if !foundTokenEnv { - t.Errorf("expected GITHUB_TOKEN env from secret gh-pat-secret, got: %v", spawner.Env) - } } func TestBuildCronJob_WithWorkspaceGitHubApp(t *testing.T) { @@ -2068,53 +2140,24 @@ func TestBuildCronJob_WithWorkspaceGitHubApp(t *testing.T) { cronJob := builder.BuildCronJob(ts, workspace, true) podSpec := cronJob.Spec.JobTemplate.Spec.Template.Spec - // Verify token-refresher init container is present - if len(podSpec.InitContainers) != 1 { - t.Fatalf("expected 1 init container (token-refresher), got %d", len(podSpec.InitContainers)) - } - if podSpec.InitContainers[0].Name != "token-refresher" { - t.Errorf("expected init container name %q, got %q", "token-refresher", podSpec.InitContainers[0].Name) - } - - // Verify volumes for GitHub App are present - foundTokenVol := false - foundSecretVol := false - for _, vol := range podSpec.Volumes { - if vol.Name == "github-token" { - foundTokenVol = true - } - if vol.Name == "github-app-secret" { - foundSecretVol = true - } - } - if !foundTokenVol { - t.Error("expected github-token volume") + if len(podSpec.InitContainers) != 0 { + t.Fatalf("expected no init containers for cron workspace discovery-only spawner, got %d", len(podSpec.InitContainers)) } - if !foundSecretVol { - t.Error("expected github-app-secret volume") + if len(podSpec.Volumes) != 0 { + t.Fatalf("expected no volumes for cron workspace discovery-only spawner, got %d", len(podSpec.Volumes)) } - // Verify spawner container has token file arg and volume mount spawner := podSpec.Containers[0] - foundTokenFileArg := false for _, arg := range spawner.Args { if arg == "--github-token-file=/shared/token/GITHUB_TOKEN" { - foundTokenFileArg = true + t.Fatalf("expected no github token file arg for cron workspace discovery-only spawner, got: %v", spawner.Args) } } - if !foundTokenFileArg { - t.Errorf("expected --github-token-file arg, got: %v", spawner.Args) - } - - foundTokenMount := false for _, vm := range spawner.VolumeMounts { - if vm.Name == "github-token" && vm.MountPath == "/shared/token" { - foundTokenMount = true + if vm.Name == "github-token" { + t.Fatalf("expected no github-token volume mount for cron workspace discovery-only spawner, got: %v", spawner.VolumeMounts) } } - if !foundTokenMount { - t.Errorf("expected github-token volume mount, got: %v", spawner.VolumeMounts) - } } func TestReconcileCronJob_ClearsStaleDeploymentName(t *testing.T) { @@ -2472,14 +2515,10 @@ func TestDeploymentBuilder_CronJob_TokenRefresherResources(t *testing.T) { } cronJob := builder.BuildCronJob(ts, workspace, true) - refresher := cronJob.Spec.JobTemplate.Spec.Template.Spec.InitContainers[0] spawner := cronJob.Spec.JobTemplate.Spec.Template.Spec.Containers[0] - if refresher.Resources.Requests.Cpu().String() != "100m" { - t.Errorf("expected token-refresher cpu request 100m on CronJob, got %s", refresher.Resources.Requests.Cpu().String()) - } - if refresher.Resources.Limits.Memory().String() != "256Mi" { - t.Errorf("expected token-refresher memory limit 256Mi on CronJob, got %s", refresher.Resources.Limits.Memory().String()) + if len(cronJob.Spec.JobTemplate.Spec.Template.Spec.InitContainers) != 0 { + t.Fatalf("expected no init containers for cron workspace discovery-only spawner, got %d", len(cronJob.Spec.JobTemplate.Spec.Template.Spec.InitContainers)) } if len(spawner.Resources.Requests) != 0 || len(spawner.Resources.Limits) != 0 { t.Errorf("expected spawner resources to remain empty on CronJob, got requests=%v limits=%v", spawner.Resources.Requests, spawner.Resources.Limits) @@ -2503,8 +2542,12 @@ func TestDeploymentBuilder_SpawnerResources_TokenRefresherUnaffected(t *testing. When: kelosv1alpha1.When{ GitHubIssues: &kelosv1alpha1.GitHubIssues{}, }, + TaskTemplate: kelosv1alpha1.TaskTemplate{ + WorkspaceRef: &kelosv1alpha1.WorkspaceReference{Name: "ws"}, + }, }, } + enableGitHubReporting(ts) workspace := &kelosv1alpha1.WorkspaceSpec{ Repo: "https://github.com/kelos-dev/kelos.git", SecretRef: &kelosv1alpha1.SecretReference{ @@ -2656,6 +2699,7 @@ func TestUpdateDeployment_TokenRefresherResourcesDrift(t *testing.T) { }, }, } + enableGitHubReporting(ts) workspace := &kelosv1alpha1.WorkspaceSpec{ Repo: "https://github.com/kelos-dev/kelos.git", SecretRef: &kelosv1alpha1.SecretReference{ @@ -2773,13 +2817,8 @@ func TestUpdateCronJob_TokenRefresherResourcesDrift(t *testing.T) { if err := cl.Get(ctx, client.ObjectKeyFromObject(cronJob), &updated); err != nil { t.Fatalf("getting CronJob: %v", err) } - - refresher := updated.Spec.JobTemplate.Spec.Template.Spec.InitContainers[0] - if refresher.Resources.Requests.Cpu().String() != "100m" { - t.Errorf("expected token-refresher cpu request 100m after drift update, got %s", refresher.Resources.Requests.Cpu().String()) - } - if refresher.Resources.Limits.Memory().String() != "256Mi" { - t.Errorf("expected token-refresher memory limit 256Mi after drift update, got %s", refresher.Resources.Limits.Memory().String()) + if len(updated.Spec.JobTemplate.Spec.Template.Spec.InitContainers) != 0 { + t.Fatalf("expected no init containers for cron workspace discovery-only spawner, got %d", len(updated.Spec.JobTemplate.Spec.Template.Spec.InitContainers)) } } diff --git a/internal/controller/workspace_controller.go b/internal/controller/workspace_controller.go new file mode 100644 index 00000000..c0c8547f --- /dev/null +++ b/internal/controller/workspace_controller.go @@ -0,0 +1,232 @@ +package controller + +import ( + "context" + "reflect" + "time" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + kelosv1alpha1 "github.com/kelos-dev/kelos/api/v1alpha1" + "github.com/kelos-dev/kelos/internal/githubapp" +) + +// WorkspaceReconciler reconciles workspace-scoped ghproxy resources. +type WorkspaceReconciler struct { + client.Client + Scheme *runtime.Scheme + ProxyBuilder *WorkspaceGHProxyBuilder + Recorder record.EventRecorder +} + +// +kubebuilder:rbac:groups=kelos.dev,resources=workspaces,verbs=get;list;watch +// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch + +// Reconcile ensures each Workspace has a ghproxy Deployment and Service. +func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + logger := log.FromContext(ctx) + + var workspace kelosv1alpha1.Workspace + if err := r.Get(ctx, req.NamespacedName, &workspace); err != nil { + if apierrors.IsNotFound(err) { + return ctrl.Result{}, nil + } + logger.Error(err, "Unable to fetch Workspace") + reconcileErrorsTotal.WithLabelValues("workspace").Inc() + return ctrl.Result{}, err + } + + isGitHubApp := false + if workspace.Spec.SecretRef != nil { + var secret corev1.Secret + if err := r.Get(ctx, client.ObjectKey{Namespace: workspace.Namespace, Name: workspace.Spec.SecretRef.Name}, &secret); err != nil { + if apierrors.IsNotFound(err) { + logger.Info("Workspace secret not found yet, requeuing", "workspace", workspace.Name, "secret", workspace.Spec.SecretRef.Name) + r.recordEvent(&workspace, corev1.EventTypeNormal, "WorkspaceSecretNotFound", "Workspace secret %s not found, requeuing", workspace.Spec.SecretRef.Name) + return ctrl.Result{RequeueAfter: 2 * time.Second}, nil + } + logger.Error(err, "Unable to fetch Workspace secret", "workspace", workspace.Name, "secret", workspace.Spec.SecretRef.Name) + return ctrl.Result{}, err + } + isGitHubApp = githubapp.IsGitHubApp(secret.Data) + } + + if err := r.reconcileService(ctx, &workspace); err != nil { + logger.Error(err, "Unable to reconcile workspace proxy Service", "workspace", workspace.Name) + return ctrl.Result{}, err + } + if err := r.reconcileDeployment(ctx, &workspace, isGitHubApp); err != nil { + logger.Error(err, "Unable to reconcile workspace proxy Deployment", "workspace", workspace.Name) + return ctrl.Result{}, err + } + + return ctrl.Result{}, nil +} + +func (r *WorkspaceReconciler) reconcileService(ctx context.Context, workspace *kelosv1alpha1.Workspace) error { + desired := r.ProxyBuilder.BuildService(workspace) + if err := controllerutil.SetControllerReference(workspace, desired, r.Scheme); err != nil { + return err + } + + var current corev1.Service + err := r.Get(ctx, client.ObjectKeyFromObject(desired), ¤t) + if apierrors.IsNotFound(err) { + if err := r.Create(ctx, desired); err != nil { + return err + } + r.recordEvent(workspace, corev1.EventTypeNormal, "WorkspaceProxyServiceCreated", "Created workspace ghproxy Service %s", desired.Name) + return nil + } + if err != nil { + return err + } + + if reflect.DeepEqual(current.Spec.Ports, desired.Spec.Ports) && + reflect.DeepEqual(current.Spec.Selector, desired.Spec.Selector) && + reflect.DeepEqual(current.Labels, desired.Labels) { + return nil + } + + current.Labels = desired.Labels + current.Spec.Ports = desired.Spec.Ports + current.Spec.Selector = desired.Spec.Selector + if err := r.Update(ctx, ¤t); err != nil { + return err + } + r.recordEvent(workspace, corev1.EventTypeNormal, "WorkspaceProxyServiceUpdated", "Updated workspace ghproxy Service %s", desired.Name) + return nil +} + +func (r *WorkspaceReconciler) reconcileDeployment(ctx context.Context, workspace *kelosv1alpha1.Workspace, isGitHubApp bool) error { + desired := r.ProxyBuilder.BuildDeployment(workspace, isGitHubApp) + if err := controllerutil.SetControllerReference(workspace, desired, r.Scheme); err != nil { + return err + } + + var current appsv1.Deployment + err := r.Get(ctx, client.ObjectKeyFromObject(desired), ¤t) + if apierrors.IsNotFound(err) { + if err := r.Create(ctx, desired); err != nil { + return err + } + r.recordEvent(workspace, corev1.EventTypeNormal, "WorkspaceProxyDeploymentCreated", "Created workspace ghproxy Deployment %s", desired.Name) + return nil + } + if err != nil { + return err + } + + needsUpdate := false + if !reflect.DeepEqual(current.Labels, desired.Labels) { + current.Labels = desired.Labels + needsUpdate = true + } + if current.Spec.Replicas == nil || desired.Spec.Replicas == nil || *current.Spec.Replicas != *desired.Spec.Replicas { + current.Spec.Replicas = desired.Spec.Replicas + needsUpdate = true + } + if !reflect.DeepEqual(current.Spec.Template.Labels, desired.Spec.Template.Labels) { + current.Spec.Template.Labels = desired.Spec.Template.Labels + needsUpdate = true + } + if !containersEqual(current.Spec.Template.Spec.InitContainers, desired.Spec.Template.Spec.InitContainers) { + current.Spec.Template.Spec.InitContainers = desired.Spec.Template.Spec.InitContainers + needsUpdate = true + } + if !reflect.DeepEqual(current.Spec.Template.Spec.Volumes, desired.Spec.Template.Spec.Volumes) { + current.Spec.Template.Spec.Volumes = desired.Spec.Template.Spec.Volumes + needsUpdate = true + } + if len(current.Spec.Template.Spec.Containers) != len(desired.Spec.Template.Spec.Containers) || + !containersEqual(current.Spec.Template.Spec.Containers, desired.Spec.Template.Spec.Containers) { + current.Spec.Template.Spec.Containers = desired.Spec.Template.Spec.Containers + needsUpdate = true + } + + if !needsUpdate { + return nil + } + + if err := r.Update(ctx, ¤t); err != nil { + return err + } + r.recordEvent(workspace, corev1.EventTypeNormal, "WorkspaceProxyDeploymentUpdated", "Updated workspace ghproxy Deployment %s", desired.Name) + return nil +} + +func (r *WorkspaceReconciler) recordEvent(obj runtime.Object, eventType, reason, messageFmt string, args ...interface{}) { + if r.Recorder != nil { + r.Recorder.Eventf(obj, eventType, reason, messageFmt, args...) + } +} + +// SetupWithManager sets up the controller with the Manager. +func (r *WorkspaceReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&kelosv1alpha1.Workspace{}). + Owns(&appsv1.Deployment{}). + Owns(&corev1.Service{}). + Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.findWorkspacesForSecret)). + Complete(r) +} + +func (r *WorkspaceReconciler) findWorkspacesForSecret(ctx context.Context, obj client.Object) []reconcile.Request { + secret, ok := obj.(*corev1.Secret) + if !ok { + return nil + } + + var workspaceList kelosv1alpha1.WorkspaceList + if err := r.List(ctx, &workspaceList, client.InNamespace(secret.Namespace)); err != nil { + return nil + } + + var requests []reconcile.Request + for _, workspace := range workspaceList.Items { + if workspace.Spec.SecretRef != nil && workspace.Spec.SecretRef.Name == secret.Name { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: workspace.Namespace, + Name: workspace.Name, + }, + }) + } + } + return requests +} + +func containersEqual(a, b []corev1.Container) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if !containerEqual(a[i], b[i]) { + return false + } + } + return true +} + +func containerEqual(a, b corev1.Container) bool { + if !resourceRequirementsEqual(a.Resources, b.Resources) { + return false + } + a.Resources = corev1.ResourceRequirements{} + b.Resources = corev1.ResourceRequirements{} + return reflect.DeepEqual(a, b) +} diff --git a/internal/controller/workspace_controller_test.go b/internal/controller/workspace_controller_test.go new file mode 100644 index 00000000..cde80be3 --- /dev/null +++ b/internal/controller/workspace_controller_test.go @@ -0,0 +1,151 @@ +package controller + +import ( + "context" + "strings" + "testing" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + kelosv1alpha1 "github.com/kelos-dev/kelos/api/v1alpha1" +) + +func newWorkspaceControllerTestScheme() *runtime.Scheme { + scheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(appsv1.AddToScheme(scheme)) + utilruntime.Must(kelosv1alpha1.AddToScheme(scheme)) + return scheme +} + +func TestWorkspaceReconciler_CreatesGitHubAppProxyResources(t *testing.T) { + scheme := newWorkspaceControllerTestScheme() + + workspace := &kelosv1alpha1.Workspace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-workspace", + Namespace: "default", + }, + Spec: kelosv1alpha1.WorkspaceSpec{ + Repo: "https://github.example.com/my-org/my-repo.git", + SecretRef: &kelosv1alpha1.SecretReference{ + Name: "github-app-creds", + }, + }, + } + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "github-app-creds", + Namespace: "default", + }, + Data: map[string][]byte{ + "appID": []byte("123"), + "installationID": []byte("456"), + "privateKey": []byte("pem"), + }, + } + + cl := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(workspace, secret). + Build() + + r := &WorkspaceReconciler{ + Client: cl, + Scheme: scheme, + ProxyBuilder: NewWorkspaceGHProxyBuilder(), + } + + _, err := r.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{Namespace: "default", Name: "example-workspace"}, + }) + if err != nil { + t.Fatalf("Reconcile error: %v", err) + } + + var deploy appsv1.Deployment + if err := cl.Get(context.Background(), client.ObjectKey{Namespace: "default", Name: WorkspaceGHProxyName("example-workspace")}, &deploy); err != nil { + t.Fatalf("getting Deployment: %v", err) + } + if len(deploy.OwnerReferences) != 1 || deploy.OwnerReferences[0].Name != "example-workspace" { + t.Fatalf("expected Deployment ownerReference to Workspace, got %v", deploy.OwnerReferences) + } + if len(deploy.Spec.Template.Spec.InitContainers) != 1 { + t.Fatalf("expected 1 init container, got %d", len(deploy.Spec.Template.Spec.InitContainers)) + } + args := deploy.Spec.Template.Spec.Containers[0].Args + if !containsArg(args, "--upstream-base-url=https://github.example.com/api/v3") { + t.Fatalf("expected enterprise upstream arg, got %v", args) + } + if !containsArg(args, "--github-token-file=/shared/token/GITHUB_TOKEN") { + t.Fatalf("expected github token file arg, got %v", args) + } + + var svc corev1.Service + if err := cl.Get(context.Background(), client.ObjectKey{Namespace: "default", Name: WorkspaceGHProxyName("example-workspace")}, &svc); err != nil { + t.Fatalf("getting Service: %v", err) + } + if len(svc.OwnerReferences) != 1 || svc.OwnerReferences[0].Name != "example-workspace" { + t.Fatalf("expected Service ownerReference to Workspace, got %v", svc.OwnerReferences) + } +} + +func TestWorkspaceGHProxyName_TruncatesLongWorkspaceNames(t *testing.T) { + name := WorkspaceGHProxyName(strings.Repeat("a", 70)) + if len(name) > 63 { + t.Fatalf("expected truncated name length <= 63, got %d", len(name)) + } + if !strings.HasPrefix(name, "ghproxy-") { + t.Fatalf("expected ghproxy prefix, got %q", name) + } +} + +func TestContainersEqual_UsesSemanticResourceComparison(t *testing.T) { + a := []corev1.Container{{ + Name: "ghproxy", + Image: "ghproxy:latest", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1000m"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + }, + }} + b := []corev1.Container{{ + Name: "ghproxy", + Image: "ghproxy:latest", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + }, + }} + + if !containersEqual(a, b) { + t.Fatal("expected semantically equal resource quantities to compare equal") + } +} + +func containsArg(args []string, want string) bool { + for _, arg := range args { + if arg == want { + return true + } + } + return false +} diff --git a/internal/controller/workspace_ghproxy_builder.go b/internal/controller/workspace_ghproxy_builder.go new file mode 100644 index 00000000..5338545e --- /dev/null +++ b/internal/controller/workspace_ghproxy_builder.go @@ -0,0 +1,272 @@ +package controller + +import ( + "crypto/sha1" + "encoding/hex" + "fmt" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + kelosv1alpha1 "github.com/kelos-dev/kelos/api/v1alpha1" +) + +const ( + // DefaultGHProxyImage is the default image for workspace ghproxy Deployments. + DefaultGHProxyImage = "ghcr.io/kelos-dev/ghproxy:latest" + + workspaceProxyPort = 8888 + workspaceProxyNamePrefix = "ghproxy-" +) + +// WorkspaceGHProxyBuilder constructs Services and Deployments for workspace-scoped ghproxy instances. +type WorkspaceGHProxyBuilder struct { + GHProxyImage string + GHProxyImagePullPolicy corev1.PullPolicy + GHProxyResources *corev1.ResourceRequirements + TokenRefresherImage string + TokenRefresherImagePullPolicy corev1.PullPolicy + TokenRefresherResources *corev1.ResourceRequirements +} + +// NewWorkspaceGHProxyBuilder creates a new WorkspaceGHProxyBuilder. +func NewWorkspaceGHProxyBuilder() *WorkspaceGHProxyBuilder { + return &WorkspaceGHProxyBuilder{ + GHProxyImage: DefaultGHProxyImage, + TokenRefresherImage: DefaultTokenRefresherImage, + } +} + +func workspaceProxyLabels(workspaceName string) map[string]string { + return map[string]string{ + "kelos.dev/name": "kelos", + "kelos.dev/component": "ghproxy", + "kelos.dev/managed-by": "kelos-controller", + "kelos.dev/workspace": workspaceName, + } +} + +// WorkspaceGHProxyName returns the deterministic resource name for a workspace-scoped proxy. +func WorkspaceGHProxyName(workspaceName string) string { + name := workspaceProxyNamePrefix + workspaceName + if len(name) <= 63 { + return name + } + + sum := sha1.Sum([]byte(name)) + suffix := hex.EncodeToString(sum[:])[:8] + maxPrefixLen := 63 - len(suffix) - 1 + return name[:maxPrefixLen] + "-" + suffix +} + +// WorkspaceGHProxyServiceURL returns the in-cluster Service URL for a workspace-scoped proxy. +func WorkspaceGHProxyServiceURL(namespace, workspaceName string) string { + return fmt.Sprintf("http://%s.%s:%d", WorkspaceGHProxyName(workspaceName), namespace, workspaceProxyPort) +} + +func workspaceProxyUpstreamBaseURL(workspace *kelosv1alpha1.Workspace) string { + host, _, _ := parseGitHubRepo(workspace.Spec.Repo) + if apiBaseURL := gitHubAPIBaseURL(host); apiBaseURL != "" { + return apiBaseURL + } + return "https://api.github.com" +} + +// BuildService creates a Service for the workspace ghproxy. +func (b *WorkspaceGHProxyBuilder) BuildService(workspace *kelosv1alpha1.Workspace) *corev1.Service { + labels := workspaceProxyLabels(workspace.Name) + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: WorkspaceGHProxyName(workspace.Name), + Namespace: workspace.Namespace, + Labels: labels, + }, + Spec: corev1.ServiceSpec{ + Selector: labels, + Ports: []corev1.ServicePort{ + { + Name: "http", + Port: workspaceProxyPort, + TargetPort: intstrFromInt(workspaceProxyPort), + Protocol: corev1.ProtocolTCP, + }, + { + Name: "metrics", + Port: 9090, + TargetPort: intstrFromInt(9090), + Protocol: corev1.ProtocolTCP, + }, + }, + }, + } +} + +// BuildDeployment creates a Deployment for the workspace ghproxy. +func (b *WorkspaceGHProxyBuilder) BuildDeployment(workspace *kelosv1alpha1.Workspace, isGitHubApp bool) *appsv1.Deployment { + labels := workspaceProxyLabels(workspace.Name) + args := []string{ + "--upstream-base-url=" + workspaceProxyUpstreamBaseURL(workspace), + } + + var env []corev1.EnvVar + var volumes []corev1.Volume + var volumeMounts []corev1.VolumeMount + var initContainers []corev1.Container + + if workspace.Spec.SecretRef != nil { + if isGitHubApp { + args = append(args, "--github-token-file=/shared/token/GITHUB_TOKEN") + volumes = append(volumes, + corev1.Volume{ + Name: "github-token", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + corev1.Volume{ + Name: "github-app-secret", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: workspace.Spec.SecretRef.Name, + }, + }, + }, + ) + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: "github-token", + MountPath: "/shared/token", + ReadOnly: true, + }) + + refresherEnv := []corev1.EnvVar{ + { + Name: "APP_ID", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: workspace.Spec.SecretRef.Name}, + Key: "appID", + }, + }, + }, + { + Name: "INSTALLATION_ID", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: workspace.Spec.SecretRef.Name}, + Key: "installationID", + }, + }, + }, + } + host, _, _ := parseGitHubRepo(workspace.Spec.Repo) + if apiBaseURL := gitHubAPIBaseURL(host); apiBaseURL != "" { + refresherEnv = append(refresherEnv, corev1.EnvVar{ + Name: "GITHUB_API_BASE_URL", + Value: apiBaseURL, + }) + } + + restartPolicyAlways := corev1.ContainerRestartPolicyAlways + refresher := corev1.Container{ + Name: "token-refresher", + Image: b.TokenRefresherImage, + ImagePullPolicy: b.TokenRefresherImagePullPolicy, + RestartPolicy: &restartPolicyAlways, + Env: refresherEnv, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "github-token", + MountPath: "/shared/token", + }, + { + Name: "github-app-secret", + MountPath: "/etc/github-app", + ReadOnly: true, + }, + }, + } + if b.TokenRefresherResources != nil { + refresher.Resources = *b.TokenRefresherResources + } + initContainers = append(initContainers, refresher) + } else { + env = append(env, corev1.EnvVar{ + Name: "GITHUB_TOKEN", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: workspace.Spec.SecretRef.Name}, + Key: "GITHUB_TOKEN", + }, + }, + }) + } + } + + container := corev1.Container{ + Name: "ghproxy", + Image: b.GHProxyImage, + ImagePullPolicy: b.GHProxyImagePullPolicy, + Args: args, + Env: env, + VolumeMounts: volumeMounts, + SecurityContext: &corev1.SecurityContext{ + AllowPrivilegeEscalation: ptrTo(false), + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + }, + }, + Ports: []corev1.ContainerPort{ + { + Name: "http", + ContainerPort: workspaceProxyPort, + Protocol: corev1.ProtocolTCP, + }, + { + Name: "metrics", + ContainerPort: 9090, + Protocol: corev1.ProtocolTCP, + }, + }, + } + if b.GHProxyResources != nil { + container.Resources = *b.GHProxyResources + } + + replicas := int32(1) + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: WorkspaceGHProxyName(workspace.Name), + Namespace: workspace.Namespace, + Labels: labels, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: &replicas, + Selector: &metav1.LabelSelector{ + MatchLabels: labels, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: labels, + }, + Spec: corev1.PodSpec{ + SecurityContext: &corev1.PodSecurityContext{ + RunAsNonRoot: ptrTo(true), + }, + Volumes: volumes, + InitContainers: initContainers, + Containers: []corev1.Container{container}, + }, + }, + }, + } +} + +func intstrFromInt(v int32) intstr.IntOrString { + return intstr.FromInt32(v) +} + +func ptrTo[T any](v T) *T { + return &v +} diff --git a/internal/manifests/charts/kelos/README.md b/internal/manifests/charts/kelos/README.md index 3b8b99a0..52cb1a0a 100644 --- a/internal/manifests/charts/kelos/README.md +++ b/internal/manifests/charts/kelos/README.md @@ -100,3 +100,105 @@ helm uninstall kelos -n kelos-system ``` Because `crds.keep=true` by default, uninstalling the chart does not delete the Kelos CRDs. + +## Webhook Server Configuration + +The chart includes an optional webhook server for GitHub integration. It is disabled by default and must be explicitly enabled. + +### Prerequisites + +1. Create secrets containing webhook signing secrets: + +```bash +# GitHub webhook secret +kubectl create secret generic github-webhook-secret \ + --from-literal=WEBHOOK_SECRET=your-github-webhook-secret \ + -n kelos-system +``` + +2. Configure webhooks in your GitHub repositories to send events to your webhook endpoints. + +### Enable Webhook Servers + +```bash +helm upgrade --install kelos oci://ghcr.io/kelos-dev/charts/kelos \ + -n kelos-system \ + --create-namespace \ + --set webhookServer.sources.github.enabled=true \ + --set webhookServer.sources.github.secretName=github-webhook-secret \ + --set webhookServer.ingress.enabled=true \ + --set webhookServer.ingress.host=webhooks.your-domain.com \ + --set webhookServer.ingress.className=nginx \ + --set webhookServer.ingress.tls.enabled=true \ + --set-json 'webhookServer.ingress.annotations={"cert-manager.io/cluster-issuer":"letsencrypt-prod"}' +``` + +### Webhook Configuration Options + +```yaml +webhookServer: + image: ghcr.io/kelos-dev/kelos-webhook-server + sources: + github: + enabled: false # Enable GitHub webhook server + replicas: 1 # Number of replicas + secretName: "" # Secret containing WEBHOOK_SECRET + ingress: + enabled: false # Enable ingress for external access + className: "" # Ingress class name (e.g., nginx) + host: "" # Hostname for webhook endpoints + annotations: {} # Additional ingress annotations + tls: + enabled: false # Enable TLS for the ingress + secretName: "" # Secret name containing TLS certificate +``` + +### TLS Configuration + +The webhook ingress supports TLS termination for secure HTTPS connections. TLS is strongly recommended for production deployments. + +#### Option 1: Use cert-manager for automatic certificate management + +```bash +# Install cert-manager if not already present +kubectl apply -f https://github.com/cert-manager/cert-manager/releases/download/v1.13.0/cert-manager.yaml + +# Configure with cert-manager annotations +helm upgrade --install kelos oci://ghcr.io/kelos-dev/charts/kelos \ + -n kelos-system \ + --set webhookServer.sources.github.enabled=true \ + --set webhookServer.sources.github.secretName=github-webhook-secret \ + --set webhookServer.ingress.enabled=true \ + --set webhookServer.ingress.host=webhooks.your-domain.com \ + --set webhookServer.ingress.className=nginx \ + --set webhookServer.ingress.tls.enabled=true \ + --set-json 'webhookServer.ingress.annotations={"cert-manager.io/cluster-issuer":"letsencrypt-prod"}' +``` + +#### Option 2: Use existing TLS certificate + +```bash +# Create TLS secret manually +kubectl create secret tls webhook-tls-secret \ + --cert=path/to/tls.crt \ + --key=path/to/tls.key \ + -n kelos-system + +# Configure ingress to use the secret +helm upgrade --install kelos oci://ghcr.io/kelos-dev/charts/kelos \ + -n kelos-system \ + --set webhookServer.ingress.enabled=true \ + --set webhookServer.ingress.host=webhooks.your-domain.com \ + --set webhookServer.ingress.tls.enabled=true \ + --set webhookServer.ingress.tls.secretName=webhook-tls-secret +``` + +### Webhook Endpoints + +When enabled, the webhook servers expose the following endpoints: + +- **GitHub**: `https://your-host/webhook/github` + +### Example Values File + +See `examples/helm-values-webhook.yaml` for a complete example configuration. diff --git a/internal/manifests/charts/kelos/templates/crds/taskspawner-crd.yaml b/internal/manifests/charts/kelos/templates/crds/taskspawner-crd.yaml index 25d9875a..54a08ff1 100644 --- a/internal/manifests/charts/kelos/templates/crds/taskspawner-crd.yaml +++ b/internal/manifests/charts/kelos/templates/crds/taskspawner-crd.yaml @@ -117,6 +117,7 @@ spec: Available variables (all sources): {{ "{{.ID}}" }}, {{ "{{.Title}}" }}, {{ "{{.Kind}}" }} GitHub issue/Jira sources: {{ "{{.Number}}" }}, {{ "{{.Body}}" }}, {{ "{{.URL}}" }}, {{ "{{.Labels}}" }}, {{ "{{.Comments}}" }} GitHub pull request sources additionally expose: {{ "{{.Branch}}" }}, {{ "{{.ReviewState}}" }}, {{ "{{.ReviewComments}}" }} + GitHub webhook sources: {{ "{{.Event}}" }}, {{ "{{.Action}}" }}, {{ "{{.Sender}}" }}, {{ "{{.Ref}}" }}, {{ "{{.Payload}}" }} (full payload access) Cron sources: {{ "{{.Time}}" }}, {{ "{{.Schedule}}" }} type: string credentials: @@ -438,6 +439,7 @@ spec: Available variables (all sources): {{ "{{.ID}}" }}, {{ "{{.Title}}" }}, {{ "{{.Kind}}" }} GitHub issue/Jira sources: {{ "{{.Number}}" }}, {{ "{{.Body}}" }}, {{ "{{.URL}}" }}, {{ "{{.Labels}}" }}, {{ "{{.Comments}}" }} GitHub pull request sources additionally expose: {{ "{{.Branch}}" }}, {{ "{{.ReviewState}}" }}, {{ "{{.ReviewComments}}" }} + GitHub webhook sources: {{ "{{.Event}}" }}, {{ "{{.Action}}" }}, {{ "{{.Sender}}" }}, {{ "{{.Ref}}" }}, {{ "{{.Payload}}" }} (full payload access) Cron sources: {{ "{{.Time}}" }}, {{ "{{.Schedule}}" }} type: string ttlSecondsAfterFinished: @@ -792,6 +794,78 @@ spec: rule: '!(has(self.commentPolicy) && ((has(self.triggerComment) && size(self.triggerComment) > 0) || (has(self.excludeComments) && size(self.excludeComments) > 0)))' + githubWebhook: + description: GitHubWebhook triggers task spawning on GitHub webhook + events. + properties: + events: + description: |- + Events is the list of GitHub event types to listen for. + e.g., "issue_comment", "pull_request_review", "push", "issues" + items: + type: string + minItems: 1 + type: array + filters: + description: |- + Filters refine which events trigger tasks. If multiple filters match + the same event type, any match triggers a task (OR semantics). + If empty, all events in the Events list trigger tasks. + items: + description: GitHubWebhookFilter defines filtering criteria + for GitHub webhook events. + properties: + action: + description: Action filters by webhook action (e.g., + "created", "opened", "submitted"). + type: string + author: + description: Author filters by the event sender's username. + type: string + bodyContains: + description: BodyContains filters by substring match + on the comment/review body. + type: string + branch: + description: Branch filters push events by branch name + (exact match or glob). + type: string + draft: + description: Draft filters PRs by draft status. nil + = don't filter. + type: boolean + event: + description: Event is the GitHub event type this filter + applies to. + type: string + excludeLabels: + description: ExcludeLabels excludes issues/PRs with + any of these labels. + items: + type: string + type: array + labels: + description: Labels requires the issue/PR to have all + of these labels. + items: + type: string + type: array + state: + description: State filters by issue/PR state ("open", + "closed"). + type: string + required: + - event + type: object + type: array + repository: + description: |- + Repository restricts webhooks to a specific repository (owner/repo format). + If empty, webhooks from any repository are accepted. + type: string + required: + - events + type: object jira: description: Jira discovers issues from a Jira project. properties: @@ -837,10 +911,10 @@ spec: - when type: object x-kubernetes-validations: - - message: taskTemplate.workspaceRef is required when using githubIssues - or githubPullRequests source - rule: '!(has(self.when.githubIssues) || has(self.when.githubPullRequests)) - || has(self.taskTemplate.workspaceRef)' + - message: taskTemplate.workspaceRef is required when using githubIssues, + githubPullRequests, or githubWebhook source + rule: '!(has(self.when.githubIssues) || has(self.when.githubPullRequests) + || has(self.when.githubWebhook)) || has(self.taskTemplate.workspaceRef)' status: description: TaskSpawnerStatus defines the observed state of TaskSpawner. properties: diff --git a/internal/manifests/charts/kelos/templates/deployment.yaml b/internal/manifests/charts/kelos/templates/deployment.yaml index 8e15ce98..5c6abf0d 100644 --- a/internal/manifests/charts/kelos/templates/deployment.yaml +++ b/internal/manifests/charts/kelos/templates/deployment.yaml @@ -60,6 +60,16 @@ spec: {{- if .Values.spawner.resources.limits }} - --spawner-resource-limits={{ .Values.spawner.resources.limits }} {{- end }} + - --ghproxy-image={{ .Values.ghproxy.image }}{{- if .Values.image.tag }}:{{ .Values.image.tag }}{{- end }} + {{- if .Values.image.pullPolicy }} + - --ghproxy-image-pull-policy={{ .Values.image.pullPolicy }} + {{- end }} + {{- if .Values.ghproxy.resources.requests }} + - --ghproxy-resource-requests={{ .Values.ghproxy.resources.requests }} + {{- end }} + {{- if .Values.ghproxy.resources.limits }} + - --ghproxy-resource-limits={{ .Values.ghproxy.resources.limits }} + {{- end }} - --token-refresher-image={{ .Values.tokenRefresher.image }}{{- if .Values.image.tag }}:{{ .Values.image.tag }}{{- end }} {{- if .Values.image.pullPolicy }} - --token-refresher-image-pull-policy={{ .Values.image.pullPolicy }} diff --git a/internal/manifests/charts/kelos/templates/ghproxy.yaml b/internal/manifests/charts/kelos/templates/ghproxy.yaml deleted file mode 100644 index 25977e5c..00000000 --- a/internal/manifests/charts/kelos/templates/ghproxy.yaml +++ /dev/null @@ -1,67 +0,0 @@ ---- -apiVersion: apps/v1 -kind: Deployment -metadata: - name: ghproxy - namespace: kelos-system - labels: - app.kubernetes.io/name: ghproxy -spec: - replicas: 1 - selector: - matchLabels: - app.kubernetes.io/name: ghproxy - template: - metadata: - labels: - app.kubernetes.io/name: ghproxy - spec: - securityContext: - runAsNonRoot: true - containers: - - name: ghproxy - image: {{ .Values.ghproxy.image }}{{- if .Values.image.tag }}:{{ .Values.image.tag }}{{- end }} - {{- if .Values.image.pullPolicy }} - imagePullPolicy: {{ .Values.image.pullPolicy }} - {{- end }} - args: - - --allowed-upstreams={{ .Values.ghproxy.allowedUpstreams }} - securityContext: - allowPrivilegeEscalation: false - capabilities: - drop: - - ALL - ports: - - name: http - containerPort: 8888 - protocol: TCP - - name: metrics - containerPort: 9090 - protocol: TCP - resources: - requests: - cpu: 10m - memory: 32Mi - limits: - cpu: 200m - memory: 128Mi ---- -apiVersion: v1 -kind: Service -metadata: - name: ghproxy - namespace: kelos-system - labels: - app.kubernetes.io/name: ghproxy -spec: - selector: - app.kubernetes.io/name: ghproxy - ports: - - name: http - port: 8888 - targetPort: http - protocol: TCP - - name: metrics - port: 9090 - targetPort: metrics - protocol: TCP diff --git a/internal/manifests/charts/kelos/templates/rbac.yaml b/internal/manifests/charts/kelos/templates/rbac.yaml index 2d1a6e1d..c4a2f49f 100644 --- a/internal/manifests/charts/kelos/templates/rbac.yaml +++ b/internal/manifests/charts/kelos/templates/rbac.yaml @@ -45,6 +45,18 @@ rules: - get - list - watch +- apiGroups: + - "" + resources: + - services + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - apps resources: @@ -217,3 +229,49 @@ subjects: - kind: ServiceAccount name: kelos-controller namespace: kelos-system +{{- if .Values.webhookServer.sources.github.enabled }} +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: kelos-webhook-role +rules: + - apiGroups: + - kelos.dev + resources: + - taskspawners + verbs: + - get + - list + - watch + - apiGroups: + - kelos.dev + resources: + - tasks + verbs: + - create + - get + - list + - apiGroups: + - kelos.dev + resources: + - agentconfigs + - workspaces + verbs: + - get + - list + - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: kelos-webhook-rolebinding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: kelos-webhook-role +subjects: + - kind: ServiceAccount + name: kelos-webhook + namespace: {{ .Release.Namespace }} +{{- end }} diff --git a/internal/manifests/charts/kelos/templates/serviceaccount.yaml b/internal/manifests/charts/kelos/templates/serviceaccount.yaml index 915109a9..377d73ba 100644 --- a/internal/manifests/charts/kelos/templates/serviceaccount.yaml +++ b/internal/manifests/charts/kelos/templates/serviceaccount.yaml @@ -4,3 +4,11 @@ kind: ServiceAccount metadata: name: kelos-controller namespace: kelos-system +{{- if .Values.webhookServer.sources.github.enabled }} +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: kelos-webhook + namespace: {{ .Release.Namespace }} +{{- end }} diff --git a/internal/manifests/charts/kelos/templates/webhook-gateway.yaml b/internal/manifests/charts/kelos/templates/webhook-gateway.yaml new file mode 100644 index 00000000..05fd89b4 --- /dev/null +++ b/internal/manifests/charts/kelos/templates/webhook-gateway.yaml @@ -0,0 +1,73 @@ +{{- if .Values.webhookServer.gateway.enabled }} +{{- if .Values.webhookServer.sources.github.enabled }} +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: {{ .Values.webhookServer.gateway.gatewayName }} + namespace: {{ .Release.Namespace }} + labels: + app.kubernetes.io/name: kelos + app.kubernetes.io/component: webhook-gateway + {{- with .Values.webhookServer.gateway.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +spec: + {{- if .Values.webhookServer.gateway.gatewayClassName }} + gatewayClassName: {{ .Values.webhookServer.gateway.gatewayClassName }} + {{- end }} + listeners: + - name: http + port: 80 + protocol: HTTP + hostname: {{ .Values.webhookServer.gateway.host | quote }} + {{- if not .Values.webhookServer.gateway.tls.enabled }} + allowedRoutes: + namespaces: + from: Same + {{- end }} + {{- if .Values.webhookServer.gateway.tls.enabled }} + - name: https + port: 443 + protocol: HTTPS + hostname: {{ .Values.webhookServer.gateway.host | quote }} + tls: + mode: Terminate + {{- if .Values.webhookServer.gateway.tls.certificateRefs }} + certificateRefs: + {{- toYaml .Values.webhookServer.gateway.tls.certificateRefs | nindent 10 }} + {{- end }} + allowedRoutes: + namespaces: + from: Same + {{- end }} +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: kelos-webhook-routes + namespace: {{ .Release.Namespace }} + labels: + app.kubernetes.io/name: kelos + app.kubernetes.io/component: webhook-routes +spec: + parentRefs: + - name: {{ .Values.webhookServer.gateway.gatewayName }} + {{- if .Values.webhookServer.gateway.tls.enabled }} + sectionName: https + {{- else }} + sectionName: http + {{- end }} + hostnames: + - {{ .Values.webhookServer.gateway.host | quote }} + rules: + - matches: + - path: + type: PathPrefix + value: /webhook/github + backendRefs: + - name: kelos-webhook-github + port: 8443 +{{- end }} +{{- end }} diff --git a/internal/manifests/charts/kelos/templates/webhook-ingress.yaml b/internal/manifests/charts/kelos/templates/webhook-ingress.yaml new file mode 100644 index 00000000..55f0c407 --- /dev/null +++ b/internal/manifests/charts/kelos/templates/webhook-ingress.yaml @@ -0,0 +1,41 @@ +{{- if .Values.webhookServer.ingress.enabled }} +{{- $githubEnabled := .Values.webhookServer.sources.github.enabled }} +{{- if $githubEnabled }} +--- +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: kelos-webhook + namespace: {{ .Release.Namespace }} + labels: + app.kubernetes.io/name: kelos + app.kubernetes.io/component: webhook-ingress + {{- with .Values.webhookServer.ingress.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +spec: + {{- if .Values.webhookServer.ingress.className }} + ingressClassName: {{ .Values.webhookServer.ingress.className }} + {{- end }} + {{- if .Values.webhookServer.ingress.tls.enabled }} + tls: + - hosts: + - {{ .Values.webhookServer.ingress.host }} + {{- if .Values.webhookServer.ingress.tls.secretName }} + secretName: {{ .Values.webhookServer.ingress.tls.secretName }} + {{- end }} + {{- end }} + rules: + - host: {{ .Values.webhookServer.ingress.host }} + http: + paths: + - path: /webhook/github + pathType: Prefix + backend: + service: + name: kelos-webhook-github + port: + number: 8443 +{{- end }} +{{- end }} diff --git a/internal/manifests/charts/kelos/templates/webhook-server.yaml b/internal/manifests/charts/kelos/templates/webhook-server.yaml new file mode 100644 index 00000000..2d82db3a --- /dev/null +++ b/internal/manifests/charts/kelos/templates/webhook-server.yaml @@ -0,0 +1,101 @@ +{{- if .Values.webhookServer.sources.github.enabled }} +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: kelos-webhook-github + namespace: {{ .Release.Namespace }} + labels: + app.kubernetes.io/name: kelos + app.kubernetes.io/component: webhook-github +spec: + replicas: {{ .Values.webhookServer.sources.github.replicas }} + selector: + matchLabels: + app.kubernetes.io/name: kelos + app.kubernetes.io/component: webhook-github + template: + metadata: + labels: + app.kubernetes.io/name: kelos + app.kubernetes.io/component: webhook-github + spec: + serviceAccountName: kelos-webhook + securityContext: + runAsNonRoot: true + containers: + - name: webhook-server + image: {{ .Values.webhookServer.image }}{{- if .Values.image.tag }}:{{ .Values.image.tag }}{{- end }} + {{- if .Values.image.pullPolicy }} + imagePullPolicy: {{ .Values.image.pullPolicy }} + {{- end }} + args: + - --source=github + - --webhook-bind-address=:8443 + - --metrics-bind-address=:8080 + - --health-probe-bind-address=:8081 + env: + - name: WEBHOOK_SECRET + valueFrom: + secretKeyRef: + name: {{ .Values.webhookServer.sources.github.secretName }} + key: WEBHOOK_SECRET + ports: + - name: webhook + containerPort: 8443 + protocol: TCP + - name: metrics + containerPort: 8080 + protocol: TCP + - name: health + containerPort: 8081 + protocol: TCP + livenessProbe: + httpGet: + path: /healthz + port: health + initialDelaySeconds: 15 + periodSeconds: 20 + readinessProbe: + httpGet: + path: /readyz + port: health + initialDelaySeconds: 5 + periodSeconds: 10 + {{- if .Values.webhookServer.resources }} + resources: + {{- toYaml .Values.webhookServer.resources | nindent 12 }} + {{- end }} + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + seccompProfile: + type: RuntimeDefault + capabilities: + drop: + - "ALL" +--- +apiVersion: v1 +kind: Service +metadata: + name: kelos-webhook-github + namespace: {{ .Release.Namespace }} + labels: + app.kubernetes.io/name: kelos + app.kubernetes.io/component: webhook-github +spec: + type: ClusterIP + ports: + - name: webhook + port: 8443 + targetPort: webhook + protocol: TCP + - name: metrics + port: 8080 + targetPort: metrics + protocol: TCP + selector: + app.kubernetes.io/name: kelos + app.kubernetes.io/component: webhook-github +{{- end }} + diff --git a/internal/manifests/charts/kelos/values.yaml b/internal/manifests/charts/kelos/values.yaml index f50003b0..3cec592d 100644 --- a/internal/manifests/charts/kelos/values.yaml +++ b/internal/manifests/charts/kelos/values.yaml @@ -13,6 +13,9 @@ controllerImage: ghcr.io/kelos-dev/kelos-controller ghproxy: image: ghcr.io/kelos-dev/ghproxy allowedUpstreams: "https://api.github.com" + resources: + requests: "" + limits: "" claudeCodeImage: ghcr.io/kelos-dev/claude-code codexImage: ghcr.io/kelos-dev/codex geminiImage: ghcr.io/kelos-dev/gemini @@ -32,3 +35,34 @@ controller: resources: requests: {} limits: {} +webhookServer: + image: ghcr.io/kelos-dev/kelos-webhook-server + sources: + github: + enabled: false + replicas: 1 + secretName: "" + resources: + limits: + cpu: 500m + memory: 128Mi + requests: + cpu: 10m + memory: 64Mi + ingress: + enabled: false + className: "" + host: "" + annotations: {} + tls: + enabled: false + secretName: "" + gateway: + enabled: false + gatewayClassName: "" + gatewayName: "kelos-webhook-gateway" + host: "" + annotations: {} + tls: + enabled: false + certificateRefs: [] diff --git a/internal/manifests/install-crd.yaml b/internal/manifests/install-crd.yaml index 96f24feb..72d03443 100644 --- a/internal/manifests/install-crd.yaml +++ b/internal/manifests/install-crd.yaml @@ -796,6 +796,7 @@ spec: Available variables (all sources): {{.ID}}, {{.Title}}, {{.Kind}} GitHub issue/Jira sources: {{.Number}}, {{.Body}}, {{.URL}}, {{.Labels}}, {{.Comments}} GitHub pull request sources additionally expose: {{.Branch}}, {{.ReviewState}}, {{.ReviewComments}} + GitHub webhook sources: {{.Event}}, {{.Action}}, {{.Sender}}, {{.Ref}}, {{.Payload}} (full payload access) Cron sources: {{.Time}}, {{.Schedule}} type: string credentials: @@ -1117,6 +1118,7 @@ spec: Available variables (all sources): {{.ID}}, {{.Title}}, {{.Kind}} GitHub issue/Jira sources: {{.Number}}, {{.Body}}, {{.URL}}, {{.Labels}}, {{.Comments}} GitHub pull request sources additionally expose: {{.Branch}}, {{.ReviewState}}, {{.ReviewComments}} + GitHub webhook sources: {{.Event}}, {{.Action}}, {{.Sender}}, {{.Ref}}, {{.Payload}} (full payload access) Cron sources: {{.Time}}, {{.Schedule}} type: string ttlSecondsAfterFinished: @@ -1471,6 +1473,78 @@ spec: rule: '!(has(self.commentPolicy) && ((has(self.triggerComment) && size(self.triggerComment) > 0) || (has(self.excludeComments) && size(self.excludeComments) > 0)))' + githubWebhook: + description: GitHubWebhook triggers task spawning on GitHub webhook + events. + properties: + events: + description: |- + Events is the list of GitHub event types to listen for. + e.g., "issue_comment", "pull_request_review", "push", "issues" + items: + type: string + minItems: 1 + type: array + filters: + description: |- + Filters refine which events trigger tasks. If multiple filters match + the same event type, any match triggers a task (OR semantics). + If empty, all events in the Events list trigger tasks. + items: + description: GitHubWebhookFilter defines filtering criteria + for GitHub webhook events. + properties: + action: + description: Action filters by webhook action (e.g., + "created", "opened", "submitted"). + type: string + author: + description: Author filters by the event sender's username. + type: string + bodyContains: + description: BodyContains filters by substring match + on the comment/review body. + type: string + branch: + description: Branch filters push events by branch name + (exact match or glob). + type: string + draft: + description: Draft filters PRs by draft status. nil + = don't filter. + type: boolean + event: + description: Event is the GitHub event type this filter + applies to. + type: string + excludeLabels: + description: ExcludeLabels excludes issues/PRs with + any of these labels. + items: + type: string + type: array + labels: + description: Labels requires the issue/PR to have all + of these labels. + items: + type: string + type: array + state: + description: State filters by issue/PR state ("open", + "closed"). + type: string + required: + - event + type: object + type: array + repository: + description: |- + Repository restricts webhooks to a specific repository (owner/repo format). + If empty, webhooks from any repository are accepted. + type: string + required: + - events + type: object jira: description: Jira discovers issues from a Jira project. properties: @@ -1516,10 +1590,10 @@ spec: - when type: object x-kubernetes-validations: - - message: taskTemplate.workspaceRef is required when using githubIssues - or githubPullRequests source - rule: '!(has(self.when.githubIssues) || has(self.when.githubPullRequests)) - || has(self.taskTemplate.workspaceRef)' + - message: taskTemplate.workspaceRef is required when using githubIssues, + githubPullRequests, or githubWebhook source + rule: '!(has(self.when.githubIssues) || has(self.when.githubPullRequests) + || has(self.when.githubWebhook)) || has(self.taskTemplate.workspaceRef)' status: description: TaskSpawnerStatus defines the observed state of TaskSpawner. properties: diff --git a/internal/source/prompt.go b/internal/source/prompt.go index 4978dcc1..38cb842d 100644 --- a/internal/source/prompt.go +++ b/internal/source/prompt.go @@ -26,7 +26,36 @@ func RenderPrompt(promptTemplate string, item WorkItem) (string, error) { return RenderTemplate(tmplStr, item) } +// WorkItemToTemplateVars converts a WorkItem into a map suitable for use with +// taskbuilder.BuildTask. The keys match the fields exposed by RenderTemplate +// so that the same promptTemplate / branch / metadata templates work for both +// polling-based and webhook-based TaskSpawners. +func WorkItemToTemplateVars(item WorkItem) map[string]interface{} { + kind := item.Kind + if kind == "" { + kind = "Issue" + } + return map[string]interface{}{ + "ID": item.ID, + "Number": item.Number, + "Title": item.Title, + "Body": item.Body, + "URL": item.URL, + "Labels": strings.Join(item.Labels, ", "), + "Comments": item.Comments, + "Kind": kind, + "Branch": item.Branch, + "ReviewState": item.ReviewState, + "ReviewComments": item.ReviewComments, + "Time": item.Time, + "Schedule": item.Schedule, + } +} + // RenderTemplate renders a Go text/template string with the given work item's fields. +// This function is used by polling-based TaskSpawners (githubIssues, githubPullRequests, jira, cron). +// Webhook-based TaskSpawners use a different template rendering path with additional variables. +// // Available variables (all sources): {{.ID}}, {{.Title}}, {{.Kind}} // GitHub issue/Jira sources: {{.Number}}, {{.Body}}, {{.URL}}, {{.Labels}}, {{.Comments}} // GitHub pull request sources additionally expose: {{.Branch}}, {{.ReviewState}}, {{.ReviewComments}} diff --git a/internal/taskbuilder/builder.go b/internal/taskbuilder/builder.go new file mode 100644 index 00000000..58f98d88 --- /dev/null +++ b/internal/taskbuilder/builder.go @@ -0,0 +1,171 @@ +package taskbuilder + +import ( + "bytes" + "fmt" + "text/template" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kelos-dev/kelos/api/v1alpha1" +) + +// TaskBuilder creates Tasks from templates and work item data. +type TaskBuilder struct { + client client.Client +} + +// SpawnerRef identifies the TaskSpawner that owns a created Task. +// When set, BuildTask adds the kelos.dev/taskspawner label and an owner reference. +type SpawnerRef struct { + Name string + UID string + APIVersion string + Kind string +} + +// NewTaskBuilder creates a new task builder. +func NewTaskBuilder(client client.Client) (*TaskBuilder, error) { + return &TaskBuilder{ + client: client, + }, nil +} + +// BuildTask creates a Task from a template and template variables. +// If spawnerRef is non-nil the kelos.dev/taskspawner label and a controller +// owner reference are set on the resulting Task. +func (tb *TaskBuilder) BuildTask( + name, namespace string, + taskTemplate *v1alpha1.TaskTemplate, + templateVars map[string]interface{}, + spawnerRef *SpawnerRef, +) (*v1alpha1.Task, error) { + // Render the prompt template + promptTemplate := taskTemplate.PromptTemplate + if promptTemplate == "" { + promptTemplate = "{{.Title}}" // Default template + } + + prompt, err := renderTemplate("prompt", promptTemplate, templateVars) + if err != nil { + return nil, fmt.Errorf("failed to render prompt template: %w", err) + } + + // Render the branch template + branch := taskTemplate.Branch + if branch != "" { + branch, err = renderTemplate("branch", branch, templateVars) + if err != nil { + return nil, fmt.Errorf("failed to render branch template: %w", err) + } + } + + // Create the Task + task := &v1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.TaskSpec{ + Type: taskTemplate.Type, + Credentials: taskTemplate.Credentials, + Prompt: prompt, + }, + } + + // Set optional fields + if taskTemplate.Model != "" { + task.Spec.Model = taskTemplate.Model + } + if taskTemplate.Image != "" { + task.Spec.Image = taskTemplate.Image + } + if taskTemplate.WorkspaceRef != nil { + task.Spec.WorkspaceRef = taskTemplate.WorkspaceRef + } + if taskTemplate.AgentConfigRef != nil { + task.Spec.AgentConfigRef = taskTemplate.AgentConfigRef + } + if len(taskTemplate.DependsOn) > 0 { + task.Spec.DependsOn = taskTemplate.DependsOn + } + if branch != "" { + task.Spec.Branch = branch + } + if taskTemplate.TTLSecondsAfterFinished != nil { + task.Spec.TTLSecondsAfterFinished = taskTemplate.TTLSecondsAfterFinished + } + if taskTemplate.PodOverrides != nil { + task.Spec.PodOverrides = taskTemplate.PodOverrides + } + if taskTemplate.UpstreamRepo != "" { + task.Spec.UpstreamRepo = taskTemplate.UpstreamRepo + } + + // Apply template metadata + if taskTemplate.Metadata != nil { + // Render labels + if len(taskTemplate.Metadata.Labels) > 0 { + if task.Labels == nil { + task.Labels = make(map[string]string) + } + for key, valueTemplate := range taskTemplate.Metadata.Labels { + value, err := renderTemplate(fmt.Sprintf("label[%s]", key), valueTemplate, templateVars) + if err != nil { + return nil, fmt.Errorf("failed to render label %s: %w", key, err) + } + task.Labels[key] = value + } + } + + // Render annotations + if len(taskTemplate.Metadata.Annotations) > 0 { + if task.Annotations == nil { + task.Annotations = make(map[string]string) + } + for key, valueTemplate := range taskTemplate.Metadata.Annotations { + value, err := renderTemplate(fmt.Sprintf("annotation[%s]", key), valueTemplate, templateVars) + if err != nil { + return nil, fmt.Errorf("failed to render annotation %s: %w", key, err) + } + task.Annotations[key] = value + } + } + } + + // Set spawner label and owner reference when a SpawnerRef is provided. + if spawnerRef != nil { + if task.Labels == nil { + task.Labels = make(map[string]string) + } + task.Labels["kelos.dev/taskspawner"] = spawnerRef.Name + + isController := true + task.OwnerReferences = append(task.OwnerReferences, metav1.OwnerReference{ + APIVersion: spawnerRef.APIVersion, + Kind: spawnerRef.Kind, + Name: spawnerRef.Name, + UID: types.UID(spawnerRef.UID), + Controller: &isController, + }) + } + + return task, nil +} + +// renderTemplate renders a Go text template with the given variables. +func renderTemplate(name, templateStr string, vars map[string]interface{}) (string, error) { + tmpl, err := template.New(name).Option("missingkey=error").Parse(templateStr) + if err != nil { + return "", fmt.Errorf("failed to parse template: %w", err) + } + + var buf bytes.Buffer + if err := tmpl.Execute(&buf, vars); err != nil { + return "", fmt.Errorf("failed to execute template: %w", err) + } + + return buf.String(), nil +} diff --git a/internal/webhook/github_filter.go b/internal/webhook/github_filter.go new file mode 100644 index 00000000..4710d273 --- /dev/null +++ b/internal/webhook/github_filter.go @@ -0,0 +1,412 @@ +package webhook + +import ( + "encoding/json" + "fmt" + "path/filepath" + "strings" + + "github.com/google/go-github/v66/github" + + "github.com/kelos-dev/kelos/api/v1alpha1" +) + +// GitHubEventData holds parsed GitHub event information for template rendering. +type GitHubEventData struct { + // Event type (e.g., "issues", "pull_request", "push") + Event string + // Action (e.g., "opened", "created", "submitted") + Action string + // Sender username + Sender string + // Git ref for push events + Ref string + // Repository information + Repository string // Full repository name (owner/repo) + RepositoryOwner string // Repository owner + RepositoryName string // Repository name only + // Raw parsed event payload for template access + RawEvent interface{} + // Standard template variables for compatibility + ID string + Title string + Number int + Body string + URL string + Branch string +} + +// ParseGitHubWebhook parses a GitHub webhook payload using the go-github SDK. +func ParseGitHubWebhook(eventType string, payload []byte) (*GitHubEventData, error) { + event, err := github.ParseWebHook(eventType, payload) + if err != nil { + return nil, fmt.Errorf("failed to parse GitHub webhook: %w", err) + } + + data := &GitHubEventData{ + Event: eventType, + RawEvent: event, + } + + // Extract repository information from any event that has it + switch e := event.(type) { + case *github.IssuesEvent: + if repo := e.GetRepo(); repo != nil { + data.Repository = repo.GetFullName() + data.RepositoryOwner = repo.GetOwner().GetLogin() + data.RepositoryName = repo.GetName() + } + case *github.PullRequestEvent: + if repo := e.GetRepo(); repo != nil { + data.Repository = repo.GetFullName() + data.RepositoryOwner = repo.GetOwner().GetLogin() + data.RepositoryName = repo.GetName() + } + case *github.IssueCommentEvent: + if repo := e.GetRepo(); repo != nil { + data.Repository = repo.GetFullName() + data.RepositoryOwner = repo.GetOwner().GetLogin() + data.RepositoryName = repo.GetName() + } + case *github.PullRequestReviewEvent: + if repo := e.GetRepo(); repo != nil { + data.Repository = repo.GetFullName() + data.RepositoryOwner = repo.GetOwner().GetLogin() + data.RepositoryName = repo.GetName() + } + case *github.PullRequestReviewCommentEvent: + if repo := e.GetRepo(); repo != nil { + data.Repository = repo.GetFullName() + data.RepositoryOwner = repo.GetOwner().GetLogin() + data.RepositoryName = repo.GetName() + } + case *github.PushEvent: + if pushRepo := e.GetRepo(); pushRepo != nil { + data.Repository = pushRepo.GetFullName() + if owner := pushRepo.GetOwner(); owner != nil { + data.RepositoryOwner = owner.GetLogin() + } + data.RepositoryName = pushRepo.GetName() + } + } + + // Extract common fields based on event type + switch e := event.(type) { + case *github.IssuesEvent: + data.Action = e.GetAction() + data.Sender = e.GetSender().GetLogin() + if issue := e.GetIssue(); issue != nil { + data.ID = fmt.Sprintf("%d", issue.GetNumber()) + data.Title = issue.GetTitle() + data.Number = issue.GetNumber() + data.Body = issue.GetBody() + data.URL = issue.GetHTMLURL() + } + + case *github.PullRequestEvent: + data.Action = e.GetAction() + data.Sender = e.GetSender().GetLogin() + if pr := e.GetPullRequest(); pr != nil { + data.ID = fmt.Sprintf("%d", pr.GetNumber()) + data.Title = pr.GetTitle() + data.Number = pr.GetNumber() + data.Body = pr.GetBody() + data.URL = pr.GetHTMLURL() + if head := pr.GetHead(); head != nil { + data.Branch = head.GetRef() + } + } + + case *github.IssueCommentEvent: + data.Action = e.GetAction() + data.Sender = e.GetSender().GetLogin() + if issue := e.GetIssue(); issue != nil { + data.ID = fmt.Sprintf("%d", issue.GetNumber()) + data.Title = issue.GetTitle() + data.Number = issue.GetNumber() + data.Body = issue.GetBody() + data.URL = issue.GetHTMLURL() + } + + case *github.PullRequestReviewEvent: + data.Action = e.GetAction() + data.Sender = e.GetSender().GetLogin() + if pr := e.GetPullRequest(); pr != nil { + data.ID = fmt.Sprintf("%d", pr.GetNumber()) + data.Title = pr.GetTitle() + data.Number = pr.GetNumber() + data.Body = pr.GetBody() + data.URL = pr.GetHTMLURL() + if head := pr.GetHead(); head != nil { + data.Branch = head.GetRef() + } + } + + case *github.PullRequestReviewCommentEvent: + data.Action = e.GetAction() + data.Sender = e.GetSender().GetLogin() + if pr := e.GetPullRequest(); pr != nil { + data.ID = fmt.Sprintf("%d", pr.GetNumber()) + data.Title = pr.GetTitle() + data.Number = pr.GetNumber() + data.Body = pr.GetBody() + data.URL = pr.GetHTMLURL() + if head := pr.GetHead(); head != nil { + data.Branch = head.GetRef() + } + } + + case *github.PushEvent: + data.Sender = e.GetSender().GetLogin() + data.Ref = e.GetRef() + // Extract branch name from refs/heads/branch-name + if strings.HasPrefix(data.Ref, "refs/heads/") { + data.Branch = strings.TrimPrefix(data.Ref, "refs/heads/") + } + if hc := e.GetHeadCommit(); hc != nil { + data.ID = hc.GetID() + } + data.Title = fmt.Sprintf("Push to %s", data.Branch) + + default: + // For other event types, try to extract sender from raw JSON + var raw map[string]interface{} + if err := json.Unmarshal(payload, &raw); err == nil { + if sender, ok := raw["sender"].(map[string]interface{}); ok { + if login, ok := sender["login"].(string); ok { + data.Sender = login + } + } + if action, ok := raw["action"].(string); ok { + data.Action = action + } + } + } + + return data, nil +} + +// MatchesGitHubEvent evaluates whether a GitHub webhook event matches the spawner's filters. +// It accepts pre-parsed event data to avoid redundant parsing. +func MatchesGitHubEvent(spawner *v1alpha1.GitHubWebhook, eventType string, eventData *GitHubEventData) (bool, error) { + // Check if event type is in the allowed list + eventAllowed := false + for _, allowedEvent := range spawner.Events { + if allowedEvent == eventType { + eventAllowed = true + break + } + } + if !eventAllowed { + return false, nil + } + + // If no filters, all events of the allowed types match + if len(spawner.Filters) == 0 { + return true, nil + } + + // Apply filters with OR semantics for the same event type + for _, filter := range spawner.Filters { + if filter.Event != eventType { + continue + } + + if matchesFilter(filter, eventData) { + return true, nil + } + } + + return false, nil +} + +// matchesFilter checks if event data matches a specific filter. +func matchesFilter(filter v1alpha1.GitHubWebhookFilter, eventData *GitHubEventData) bool { + // Action filter + if filter.Action != "" && filter.Action != eventData.Action { + return false + } + + // Author filter + if filter.Author != "" && filter.Author != eventData.Sender { + return false + } + + // Branch filter (for push events) + if filter.Branch != "" { + if eventData.Branch == "" { + return false + } + matched, _ := filepath.Match(filter.Branch, eventData.Branch) + if !matched { + return false + } + } + + // Event-specific filters + switch e := eventData.RawEvent.(type) { + case *github.IssuesEvent, *github.IssueCommentEvent: + var issue *github.Issue + if issueEvent, ok := e.(*github.IssuesEvent); ok { + issue = issueEvent.GetIssue() + } else if commentEvent, ok := e.(*github.IssueCommentEvent); ok { + issue = commentEvent.GetIssue() + } + + if issue != nil { + // State filter + if filter.State != "" && filter.State != issue.GetState() { + return false + } + + // Labels filter (all required labels must be present) + if len(filter.Labels) > 0 { + issueLabels := make(map[string]bool) + for _, label := range issue.Labels { + issueLabels[label.GetName()] = true + } + for _, requiredLabel := range filter.Labels { + if !issueLabels[requiredLabel] { + return false + } + } + } + + // ExcludeLabels filter (issue must NOT have any of these labels) + if len(filter.ExcludeLabels) > 0 { + issueLabels := make(map[string]bool) + for _, label := range issue.Labels { + issueLabels[label.GetName()] = true + } + for _, excludeLabel := range filter.ExcludeLabels { + if issueLabels[excludeLabel] { + return false + } + } + } + } + + // BodyContains filter + if filter.BodyContains != "" { + if issueEvent, ok := e.(*github.IssuesEvent); ok { + if issue := issueEvent.GetIssue(); issue != nil { + if !strings.Contains(issue.GetBody(), filter.BodyContains) { + return false + } + } + } else if commentEvent, ok := e.(*github.IssueCommentEvent); ok { + if comment := commentEvent.GetComment(); comment != nil { + if !strings.Contains(comment.GetBody(), filter.BodyContains) { + return false + } + } + } + } + + case *github.PullRequestEvent, *github.PullRequestReviewEvent, *github.PullRequestReviewCommentEvent: + var pr *github.PullRequest + switch event := e.(type) { + case *github.PullRequestEvent: + pr = event.GetPullRequest() + case *github.PullRequestReviewEvent: + pr = event.GetPullRequest() + case *github.PullRequestReviewCommentEvent: + pr = event.GetPullRequest() + } + + if pr != nil { + // State filter + if filter.State != "" && filter.State != pr.GetState() { + return false + } + + // Draft filter + if filter.Draft != nil && *filter.Draft != pr.GetDraft() { + return false + } + + // Labels filter (all required labels must be present) + if len(filter.Labels) > 0 { + prLabels := make(map[string]bool) + for _, label := range pr.Labels { + prLabels[label.GetName()] = true + } + for _, requiredLabel := range filter.Labels { + if !prLabels[requiredLabel] { + return false + } + } + } + + // ExcludeLabels filter (PR must NOT have any of these labels) + if len(filter.ExcludeLabels) > 0 { + prLabels := make(map[string]bool) + for _, label := range pr.Labels { + prLabels[label.GetName()] = true + } + for _, excludeLabel := range filter.ExcludeLabels { + if prLabels[excludeLabel] { + return false + } + } + } + + // BodyContains filter for PRs and reviews + if filter.BodyContains != "" { + if _, ok := e.(*github.PullRequestEvent); ok { + if !strings.Contains(pr.GetBody(), filter.BodyContains) { + return false + } + } else if reviewEvent, ok := e.(*github.PullRequestReviewEvent); ok { + if review := reviewEvent.GetReview(); review != nil { + if !strings.Contains(review.GetBody(), filter.BodyContains) { + return false + } + } + } else if commentEvent, ok := e.(*github.PullRequestReviewCommentEvent); ok { + if comment := commentEvent.GetComment(); comment != nil { + if !strings.Contains(comment.GetBody(), filter.BodyContains) { + return false + } + } + } + } + } + } + + return true +} + +// ExtractGitHubWorkItem extracts template variables from GitHub webhook events for task creation. +func ExtractGitHubWorkItem(eventData *GitHubEventData) map[string]interface{} { + vars := map[string]interface{}{ + "Event": eventData.Event, + "Action": eventData.Action, + "Sender": eventData.Sender, + "Ref": eventData.Ref, + "Repository": eventData.Repository, + "RepositoryOwner": eventData.RepositoryOwner, + "RepositoryName": eventData.RepositoryName, + "Payload": eventData.RawEvent, + // Standard variables for compatibility + "ID": eventData.ID, + "Title": eventData.Title, + "Kind": "webhook", + } + + // Add number, body, URL if available + if eventData.Number > 0 { + vars["Number"] = eventData.Number + } + if eventData.Body != "" { + vars["Body"] = eventData.Body + } + if eventData.URL != "" { + vars["URL"] = eventData.URL + } + if eventData.Branch != "" { + vars["Branch"] = eventData.Branch + } + + return vars +} diff --git a/internal/webhook/github_filter_test.go b/internal/webhook/github_filter_test.go new file mode 100644 index 00000000..cae21608 --- /dev/null +++ b/internal/webhook/github_filter_test.go @@ -0,0 +1,922 @@ +package webhook + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "strings" + "testing" + + "github.com/kelos-dev/kelos/api/v1alpha1" +) + +// parseAndMatch is a test helper that parses a payload and calls MatchesGitHubEvent. +func parseAndMatch(t *testing.T, spawner *v1alpha1.GitHubWebhook, eventType string, payload []byte) (bool, error) { + t.Helper() + eventData, err := ParseGitHubWebhook(eventType, payload) + if err != nil { + return false, err + } + return MatchesGitHubEvent(spawner, eventType, eventData) +} + +func TestMatchesGitHubEvent_EventTypeFilter(t *testing.T) { + spawner := &v1alpha1.GitHubWebhook{ + Events: []string{"issues", "pull_request"}, + } + + tests := []struct { + name string + eventType string + want bool + wantErr bool + }{ + { + name: "allowed event type", + eventType: "issues", + want: true, + }, + { + name: "another allowed event type", + eventType: "pull_request", + want: true, + }, + { + name: "disallowed event type", + eventType: "push", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + payload := []byte(`{"action":"opened","sender":{"login":"user"}}`) + got, err := parseAndMatch(t, spawner, tt.eventType, payload) + if (err != nil) != tt.wantErr { + t.Errorf("MatchesGitHubEvent() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("MatchesGitHubEvent() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestMatchesGitHubEvent_ActionFilter(t *testing.T) { + spawner := &v1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + Filters: []v1alpha1.GitHubWebhookFilter{ + { + Event: "issues", + Action: "opened", + }, + }, + } + + tests := []struct { + name string + payload string + want bool + }{ + { + name: "matching action", + payload: `{"action":"opened","sender":{"login":"user"}}`, + want: true, + }, + { + name: "non-matching action", + payload: `{"action":"closed","sender":{"login":"user"}}`, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseAndMatch(t, spawner, "issues", []byte(tt.payload)) + if err != nil { + t.Errorf("MatchesGitHubEvent() error = %v", err) + return + } + if got != tt.want { + t.Errorf("MatchesGitHubEvent() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestMatchesGitHubEvent_AuthorFilter(t *testing.T) { + spawner := &v1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + Filters: []v1alpha1.GitHubWebhookFilter{ + { + Event: "issues", + Author: "specific-user", + }, + }, + } + + tests := []struct { + name string + payload string + want bool + }{ + { + name: "matching author", + payload: `{"action":"opened","sender":{"login":"specific-user"}}`, + want: true, + }, + { + name: "non-matching author", + payload: `{"action":"opened","sender":{"login":"other-user"}}`, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseAndMatch(t, spawner, "issues", []byte(tt.payload)) + if err != nil { + t.Errorf("MatchesGitHubEvent() error = %v", err) + return + } + if got != tt.want { + t.Errorf("MatchesGitHubEvent() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestMatchesGitHubEvent_LabelsFilter(t *testing.T) { + spawner := &v1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + Filters: []v1alpha1.GitHubWebhookFilter{ + { + Event: "issues", + Labels: []string{"bug", "priority:high"}, + }, + }, + } + + tests := []struct { + name string + payload string + want bool + }{ + { + name: "has all required labels", + payload: `{ + "action":"opened", + "sender":{"login":"user"}, + "issue":{ + "number":1, + "title":"Test issue", + "body":"Test body", + "html_url":"https://github.com/owner/repo/issues/1", + "state":"open", + "labels":[ + {"name":"bug"}, + {"name":"priority:high"}, + {"name":"frontend"} + ] + } + }`, + want: true, + }, + { + name: "missing required label", + payload: `{ + "action":"opened", + "sender":{"login":"user"}, + "issue":{ + "number":1, + "title":"Test issue", + "body":"Test body", + "html_url":"https://github.com/owner/repo/issues/1", + "state":"open", + "labels":[ + {"name":"bug"}, + {"name":"frontend"} + ] + } + }`, + want: false, + }, + { + name: "no labels", + payload: `{ + "action":"opened", + "sender":{"login":"user"}, + "issue":{ + "number":1, + "title":"Test issue", + "body":"Test body", + "html_url":"https://github.com/owner/repo/issues/1", + "state":"open", + "labels":[] + } + }`, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseAndMatch(t, spawner, "issues", []byte(tt.payload)) + if err != nil { + t.Errorf("MatchesGitHubEvent() error = %v", err) + return + } + if got != tt.want { + t.Errorf("MatchesGitHubEvent() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestMatchesGitHubEvent_ExcludeLabelsFilter(t *testing.T) { + tests := []struct { + name string + eventType string + spawner *v1alpha1.GitHubWebhook + payload string + want bool + }{ + { + name: "issue - no excluded labels", + eventType: "issues", + spawner: &v1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + Filters: []v1alpha1.GitHubWebhookFilter{ + { + Event: "issues", + ExcludeLabels: []string{"wontfix", "duplicate"}, + }, + }, + }, + payload: `{ + "action": "opened", + "issue": { + "number": 1, + "title": "Test issue", + "labels": [ + {"name": "bug"}, + {"name": "frontend"} + ] + } + }`, + want: true, + }, + { + name: "issue - has excluded label", + eventType: "issues", + spawner: &v1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + Filters: []v1alpha1.GitHubWebhookFilter{ + { + Event: "issues", + ExcludeLabels: []string{"wontfix", "duplicate"}, + }, + }, + }, + payload: `{ + "action": "opened", + "issue": { + "number": 1, + "title": "Test issue", + "labels": [ + {"name": "bug"}, + {"name": "wontfix"} + ] + } + }`, + want: false, + }, + { + name: "PR - no excluded labels", + eventType: "pull_request", + spawner: &v1alpha1.GitHubWebhook{ + Events: []string{"pull_request"}, + Filters: []v1alpha1.GitHubWebhookFilter{ + { + Event: "pull_request", + ExcludeLabels: []string{"do-not-merge", "draft"}, + }, + }, + }, + payload: `{ + "action": "opened", + "pull_request": { + "number": 1, + "title": "Test PR", + "labels": [ + {"name": "feature"}, + {"name": "ready-for-review"} + ] + } + }`, + want: true, + }, + { + name: "PR - has excluded label", + eventType: "pull_request", + spawner: &v1alpha1.GitHubWebhook{ + Events: []string{"pull_request"}, + Filters: []v1alpha1.GitHubWebhookFilter{ + { + Event: "pull_request", + ExcludeLabels: []string{"do-not-merge", "draft"}, + }, + }, + }, + payload: `{ + "action": "opened", + "pull_request": { + "number": 1, + "title": "Test PR", + "labels": [ + {"name": "feature"}, + {"name": "do-not-merge"} + ] + } + }`, + want: false, + }, + { + name: "empty labels - should match", + eventType: "issues", + spawner: &v1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + Filters: []v1alpha1.GitHubWebhookFilter{ + { + Event: "issues", + ExcludeLabels: []string{"wontfix"}, + }, + }, + }, + payload: `{ + "action": "opened", + "issue": { + "number": 1, + "title": "Test issue", + "labels": [] + } + }`, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseAndMatch(t, tt.spawner, tt.eventType, []byte(tt.payload)) + if err != nil { + t.Errorf("MatchesGitHubEvent() error = %v", err) + return + } + if got != tt.want { + t.Errorf("MatchesGitHubEvent() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestMatchesGitHubEvent_PullRequestDraftFilter(t *testing.T) { + spawner := &v1alpha1.GitHubWebhook{ + Events: []string{"pull_request"}, + Filters: []v1alpha1.GitHubWebhookFilter{ + { + Event: "pull_request", + Draft: func() *bool { b := false; return &b }(), // Only ready PRs + }, + }, + } + + tests := []struct { + name string + payload string + want bool + }{ + { + name: "ready PR", + payload: `{ + "action":"opened", + "sender":{"login":"user"}, + "pull_request":{ + "number":1, + "title":"Test PR", + "body":"Test body", + "html_url":"https://github.com/owner/repo/pull/1", + "state":"open", + "draft":false, + "head":{"ref":"feature-branch"} + } + }`, + want: true, + }, + { + name: "draft PR", + payload: `{ + "action":"opened", + "sender":{"login":"user"}, + "pull_request":{ + "number":1, + "title":"Test PR", + "body":"Test body", + "html_url":"https://github.com/owner/repo/pull/1", + "state":"open", + "draft":true, + "head":{"ref":"feature-branch"} + } + }`, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseAndMatch(t, spawner, "pull_request", []byte(tt.payload)) + if err != nil { + t.Errorf("MatchesGitHubEvent() error = %v", err) + return + } + if got != tt.want { + t.Errorf("MatchesGitHubEvent() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestMatchesGitHubEvent_BodyContainsPullRequest(t *testing.T) { + spawner := &v1alpha1.GitHubWebhook{ + Events: []string{"pull_request"}, + Filters: []v1alpha1.GitHubWebhookFilter{ + { + Event: "pull_request", + BodyContains: "/deploy", + }, + }, + } + + tests := []struct { + name string + payload string + want bool + }{ + { + name: "PR body contains keyword", + payload: `{ + "action":"opened", + "sender":{"login":"user"}, + "pull_request":{ + "number":1, + "title":"Test PR", + "body":"Please /deploy this to staging", + "html_url":"https://github.com/owner/repo/pull/1", + "state":"open", + "head":{"ref":"feature-branch"} + } + }`, + want: true, + }, + { + name: "PR body does not contain keyword", + payload: `{ + "action":"opened", + "sender":{"login":"user"}, + "pull_request":{ + "number":1, + "title":"Test PR", + "body":"Just a regular PR", + "html_url":"https://github.com/owner/repo/pull/1", + "state":"open", + "head":{"ref":"feature-branch"} + } + }`, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseAndMatch(t, spawner, "pull_request", []byte(tt.payload)) + if err != nil { + t.Errorf("MatchesGitHubEvent() error = %v", err) + return + } + if got != tt.want { + t.Errorf("MatchesGitHubEvent() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestMatchesGitHubEvent_BranchFilter(t *testing.T) { + spawner := &v1alpha1.GitHubWebhook{ + Events: []string{"push"}, + Filters: []v1alpha1.GitHubWebhookFilter{ + { + Event: "push", + Branch: "main", + }, + }, + } + + tests := []struct { + name string + payload string + want bool + }{ + { + name: "matching branch", + payload: `{ + "ref":"refs/heads/main", + "sender":{"login":"user"}, + "head_commit":{"id":"abc123"} + }`, + want: true, + }, + { + name: "non-matching branch", + payload: `{ + "ref":"refs/heads/feature", + "sender":{"login":"user"}, + "head_commit":{"id":"abc123"} + }`, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseAndMatch(t, spawner, "push", []byte(tt.payload)) + if err != nil { + t.Errorf("MatchesGitHubEvent() error = %v", err) + return + } + if got != tt.want { + t.Errorf("MatchesGitHubEvent() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestMatchesGitHubEvent_ORSemantics(t *testing.T) { + // Multiple filters for the same event type should use OR semantics + spawner := &v1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + Filters: []v1alpha1.GitHubWebhookFilter{ + { + Event: "issues", + Action: "opened", + }, + { + Event: "issues", + Action: "closed", + }, + }, + } + + tests := []struct { + name string + payload string + want bool + }{ + { + name: "matches first filter", + payload: `{"action":"opened","sender":{"login":"user"}}`, + want: true, + }, + { + name: "matches second filter", + payload: `{"action":"closed","sender":{"login":"user"}}`, + want: true, + }, + { + name: "matches neither filter", + payload: `{"action":"edited","sender":{"login":"user"}}`, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseAndMatch(t, spawner, "issues", []byte(tt.payload)) + if err != nil { + t.Errorf("MatchesGitHubEvent() error = %v", err) + return + } + if got != tt.want { + t.Errorf("MatchesGitHubEvent() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestParseGitHubWebhook(t *testing.T) { + tests := []struct { + name string + eventType string + payload string + wantEvent string + wantTitle string + wantErr bool + }{ + { + name: "issues event", + eventType: "issues", + payload: `{ + "action":"opened", + "sender":{"login":"testuser"}, + "issue":{ + "number":42, + "title":"Test Issue", + "body":"This is a test issue", + "html_url":"https://github.com/owner/repo/issues/42", + "state":"open" + } + }`, + wantEvent: "issues", + wantTitle: "Test Issue", + wantErr: false, + }, + { + name: "pull request event", + eventType: "pull_request", + payload: `{ + "action":"opened", + "sender":{"login":"testuser"}, + "pull_request":{ + "number":123, + "title":"Test PR", + "body":"This is a test PR", + "html_url":"https://github.com/owner/repo/pull/123", + "state":"open", + "head":{"ref":"feature-branch"} + } + }`, + wantEvent: "pull_request", + wantTitle: "Test PR", + wantErr: false, + }, + { + name: "invalid JSON", + eventType: "issues", + payload: `{invalid json}`, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseGitHubWebhook(tt.eventType, []byte(tt.payload)) + if (err != nil) != tt.wantErr { + t.Errorf("ParseGitHubWebhook() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !tt.wantErr { + if got.Event != tt.wantEvent { + t.Errorf("ParseGitHubWebhook() Event = %v, want %v", got.Event, tt.wantEvent) + } + if got.Title != tt.wantTitle { + t.Errorf("ParseGitHubWebhook() Title = %v, want %v", got.Title, tt.wantTitle) + } + } + }) + } +} + +// buildTaskName mirrors the task name generation logic from handler.go. +func buildTaskName(spawnerName, eventType, deliveryID string) string { + sanitizedEventType := strings.ReplaceAll(eventType, "_", "-") + sum := sha256.Sum256([]byte(deliveryID)) + shortHash := hex.EncodeToString(sum[:])[:12] + taskName := fmt.Sprintf("%s-%s-%s", spawnerName, sanitizedEventType, shortHash) + if len(taskName) > 63 { + taskName = strings.TrimRight(taskName[:63], "-.") + } + return taskName +} + +func TestTaskNameSanitization(t *testing.T) { + tests := []struct { + name string + spawnerName string + eventType string + deliveryID string + }{ + { + name: "pull_request event with delivery ID", + spawnerName: "dep-review", + eventType: "pull_request", + deliveryID: "a1b2c3d4-e5f6-7890-abcd-ef1234567890", + }, + { + name: "issue_comment event with delivery ID", + spawnerName: "comment-handler", + eventType: "issue_comment", + deliveryID: "deadbeef-1234-5678-9abc-def012345678", + }, + { + name: "push event with short delivery ID", + spawnerName: "push-handler", + eventType: "push", + deliveryID: "abc123", + }, + { + name: "long task name truncated correctly", + spawnerName: "very-long-spawner-name-that-exceeds-kubernetes-limits", + eventType: "pull_request_review_comment", + deliveryID: "a1b2c3d4-e5f6-7890-abcd-ef1234567890", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + taskName := buildTaskName(tt.spawnerName, tt.eventType, tt.deliveryID) + + // Verify the task name is valid for Kubernetes + if strings.Contains(taskName, "_") { + t.Errorf("Task name contains underscores which are invalid for Kubernetes: %v", taskName) + } + if len(taskName) > 63 { + t.Errorf("Task name exceeds 63 character limit: %v (length: %d)", taskName, len(taskName)) + } + if strings.HasSuffix(taskName, "-") || strings.HasSuffix(taskName, ".") { + t.Errorf("Task name ends with invalid character: %v", taskName) + } + }) + } +} + +func TestTaskNameUniqueness(t *testing.T) { + // Different delivery IDs must produce different task names + nameA := buildTaskName("spawner", "issues", "delivery-a") + nameB := buildTaskName("spawner", "issues", "delivery-b") + if nameA == nameB { + t.Errorf("Different delivery IDs produced identical task names: %s", nameA) + } + + // Same delivery ID must produce the same task name (deterministic) + name1 := buildTaskName("spawner", "issues", "same-delivery") + name2 := buildTaskName("spawner", "issues", "same-delivery") + if name1 != name2 { + t.Errorf("Same delivery ID produced different task names: %s vs %s", name1, name2) + } +} + +func TestParseGitHubWebhook_RepositoryExtraction(t *testing.T) { + tests := []struct { + name string + eventType string + payload string + wantRepo string + wantRepoOwner string + wantRepoName string + }{ + { + name: "issues event with repository", + eventType: "issues", + payload: `{ + "action":"opened", + "sender":{"login":"testuser"}, + "repository":{"full_name":"myorg/myrepo","name":"myrepo","owner":{"login":"myorg"}}, + "issue":{"number":42,"title":"Test","state":"open"} + }`, + wantRepo: "myorg/myrepo", + wantRepoOwner: "myorg", + wantRepoName: "myrepo", + }, + { + name: "pull_request event with repository", + eventType: "pull_request", + payload: `{ + "action":"opened", + "sender":{"login":"testuser"}, + "repository":{"full_name":"owner/repo","name":"repo","owner":{"login":"owner"}}, + "pull_request":{"number":10,"title":"PR","state":"open","head":{"ref":"main"}} + }`, + wantRepo: "owner/repo", + wantRepoOwner: "owner", + wantRepoName: "repo", + }, + { + name: "issue_comment event with repository", + eventType: "issue_comment", + payload: `{ + "action":"created", + "sender":{"login":"testuser"}, + "repository":{"full_name":"org/project","name":"project","owner":{"login":"org"}}, + "issue":{"number":5,"title":"Issue","state":"open"}, + "comment":{"body":"hello"} + }`, + wantRepo: "org/project", + wantRepoOwner: "org", + wantRepoName: "project", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseGitHubWebhook(tt.eventType, []byte(tt.payload)) + if err != nil { + t.Fatalf("ParseGitHubWebhook() error = %v", err) + } + if got.Repository != tt.wantRepo { + t.Errorf("Repository = %v, want %v", got.Repository, tt.wantRepo) + } + if got.RepositoryOwner != tt.wantRepoOwner { + t.Errorf("RepositoryOwner = %v, want %v", got.RepositoryOwner, tt.wantRepoOwner) + } + if got.RepositoryName != tt.wantRepoName { + t.Errorf("RepositoryName = %v, want %v", got.RepositoryName, tt.wantRepoName) + } + }) + } +} + +func TestMatchesGitHubEvent_RepositoryFiltering(t *testing.T) { + payloadRepoA := `{ + "action":"opened", + "sender":{"login":"testuser"}, + "repository":{"full_name":"org/repo-a","name":"repo-a","owner":{"login":"org"}}, + "issue":{"number":1,"title":"Issue in A","state":"open"} + }` + + payloadRepoB := `{ + "action":"opened", + "sender":{"login":"testuser"}, + "repository":{"full_name":"org/repo-b","name":"repo-b","owner":{"login":"org"}}, + "issue":{"number":1,"title":"Issue in B","state":"open"} + }` + + spawnerRepoA := &v1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + Repository: "org/repo-a", + } + + spawnerNoRepo := &v1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + } + + tests := []struct { + name string + spawner *v1alpha1.GitHubWebhook + payload string + want bool + }{ + { + name: "spawner with repo filter matches correct repo", + spawner: spawnerRepoA, + payload: payloadRepoA, + want: true, + }, + { + name: "spawner with repo filter rejects wrong repo", + spawner: spawnerRepoA, + payload: payloadRepoB, + want: false, + }, + { + name: "spawner without repo filter accepts any repo", + spawner: spawnerNoRepo, + payload: payloadRepoA, + want: true, + }, + { + name: "spawner without repo filter accepts other repo", + spawner: spawnerNoRepo, + payload: payloadRepoB, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + eventData, err := ParseGitHubWebhook("issues", []byte(tt.payload)) + if err != nil { + t.Fatalf("ParseGitHubWebhook() error = %v", err) + } + + // First check repository filter (simulating matchesSpawner logic) + got := true + if tt.spawner.Repository != "" { + if eventData.Repository != tt.spawner.Repository { + got = false + } + } + + // Then check event/action filters + if got { + matched, err := MatchesGitHubEvent(tt.spawner, "issues", eventData) + if err != nil { + t.Fatalf("MatchesGitHubEvent() error = %v", err) + } + got = matched + } + + if got != tt.want { + t.Errorf("Repository filtering = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/webhook/handler.go b/internal/webhook/handler.go new file mode 100644 index 00000000..fd81190f --- /dev/null +++ b/internal/webhook/handler.go @@ -0,0 +1,383 @@ +package webhook + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "net/http" + "os" + "strings" + "sync" + "time" + + "github.com/go-logr/logr" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kelos-dev/kelos/api/v1alpha1" + "github.com/kelos-dev/kelos/internal/taskbuilder" +) + +// WebhookSource represents the type of webhook source. +type WebhookSource string + +const ( + GitHubSource WebhookSource = "github" + + // GitHub webhook headers + GitHubEventHeader = "X-GitHub-Event" + GitHubSignatureHeader = "X-Hub-Signature-256" + GitHubDeliveryHeader = "X-GitHub-Delivery" +) + +// WebhookHandler handles webhook requests for a specific source type. +type WebhookHandler struct { + client client.Client + source WebhookSource + log logr.Logger + taskBuilder *taskbuilder.TaskBuilder + secret []byte + deliveryCache *DeliveryCache +} + +// DeliveryCache tracks processed webhook deliveries for idempotency. +type DeliveryCache struct { + mu sync.RWMutex + cache map[string]time.Time +} + +// NewDeliveryCache creates a new delivery cache with cleanup. +func NewDeliveryCache(ctx context.Context) *DeliveryCache { + cache := &DeliveryCache{ + cache: make(map[string]time.Time), + } + + // Clean up expired entries every hour + go func() { + ticker := time.NewTicker(time.Hour) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + cache.cleanup() + } + } + }() + + return cache +} + +// CheckAndMark atomically checks if a delivery ID was already processed and marks it if not. +// Returns true if already processed, false if this is the first time. +func (d *DeliveryCache) CheckAndMark(deliveryID string) (alreadyProcessed bool) { + d.mu.Lock() + defer d.mu.Unlock() + if _, exists := d.cache[deliveryID]; exists { + return true + } + d.cache[deliveryID] = time.Now() + return false +} + +// cleanup removes entries older than 24 hours. +func (d *DeliveryCache) cleanup() { + d.mu.Lock() + defer d.mu.Unlock() + + cutoff := time.Now().Add(-24 * time.Hour) + for id, timestamp := range d.cache { + if timestamp.Before(cutoff) { + delete(d.cache, id) + } + } +} + +// NewWebhookHandler creates a new webhook handler for the specified source. +func NewWebhookHandler(ctx context.Context, client client.Client, source WebhookSource, log logr.Logger) (*WebhookHandler, error) { + secret := []byte(os.Getenv("WEBHOOK_SECRET")) + if len(secret) == 0 { + return nil, fmt.Errorf("WEBHOOK_SECRET environment variable is required") + } + + taskBuilder, err := taskbuilder.NewTaskBuilder(client) + if err != nil { + return nil, fmt.Errorf("failed to create task builder: %w", err) + } + + return &WebhookHandler{ + client: client, + source: source, + log: log, + taskBuilder: taskBuilder, + secret: secret, + deliveryCache: NewDeliveryCache(ctx), + }, nil +} + +// ServeHTTP handles webhook HTTP requests. +func (h *WebhookHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + log := h.log.WithValues("method", r.Method, "path", r.URL.Path, "source", h.source, "remoteAddr", r.RemoteAddr) + + // Log incoming webhook request + log.Info("Received webhook request") + + // Only accept POST requests + if r.Method != http.MethodPost { + log.Info("Rejected non-POST request", "method", r.Method) + http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) + return + } + + // Read the payload with a size limit to prevent resource exhaustion. + // GitHub caps webhook payloads at 25 MB; we use a 10 MB limit. + const maxPayloadSize = 10 * 1024 * 1024 // 10 MB + body, err := io.ReadAll(io.LimitReader(r.Body, maxPayloadSize+1)) + if err != nil { + log.Error(err, "Failed to read request body") + http.Error(w, "Bad request", http.StatusBadRequest) + return + } + if len(body) > maxPayloadSize { + log.Info("Rejected oversized webhook payload", "size", len(body)) + http.Error(w, "Payload too large", http.StatusRequestEntityTooLarge) + return + } + + // Extract headers and validate signature + var eventType, signature, deliveryID string + + switch h.source { + case GitHubSource: + eventType = r.Header.Get(GitHubEventHeader) + signature = r.Header.Get(GitHubSignatureHeader) + deliveryID = r.Header.Get(GitHubDeliveryHeader) + + log.Info("Processing GitHub webhook", "eventType", eventType, "deliveryID", deliveryID, "payloadSize", len(body)) + + if err := ValidateGitHubSignature(body, signature, h.secret); err != nil { + log.Error(err, "GitHub signature validation failed", "eventType", eventType, "deliveryID", deliveryID) + http.Error(w, "Unauthorized", http.StatusUnauthorized) + return + } + + default: + log.Error(fmt.Errorf("unsupported source: %s", h.source), "Unsupported webhook source") + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + + // Check for duplicate delivery + if deliveryID != "" && h.deliveryCache.CheckAndMark(deliveryID) { + log.Info("Duplicate webhook delivery, returning cached response", "eventType", eventType, "deliveryID", deliveryID) + w.WriteHeader(http.StatusOK) + return + } + + // Process the webhook + _, err = h.processWebhook(ctx, eventType, body, deliveryID) + if err != nil { + log.Error(err, "Failed to process webhook", "eventType", eventType, "deliveryID", deliveryID) + http.Error(w, "Internal server error", http.StatusInternalServerError) + return + } + + log.Info("Webhook processed successfully", "eventType", eventType, "deliveryID", deliveryID) + w.WriteHeader(http.StatusOK) +} + +// processWebhook processes a validated webhook payload. +func (h *WebhookHandler) processWebhook(ctx context.Context, eventType string, payload []byte, deliveryID string) (bool, error) { + log := h.log.WithValues("eventType", eventType, "deliveryID", deliveryID) + + // Parse the webhook payload once up front and reuse across matching and task creation. + var eventData *GitHubEventData + if h.source == GitHubSource { + var err error + eventData, err = ParseGitHubWebhook(eventType, payload) + if err != nil { + return false, fmt.Errorf("failed to parse %s webhook: %w", h.source, err) + } + if eventData.ID != "" { + log = log.WithValues("githubID", eventData.ID) + if eventData.Title != "" { + log = log.WithValues("githubTitle", eventData.Title) + } + } + } + + log.Info("Processing webhook event", "issueID", eventData.ID, "title", eventData.Title) + + // Get all TaskSpawners that match this source type + spawners, err := h.getMatchingSpawners(ctx) + if err != nil { + return false, fmt.Errorf("failed to get matching spawners: %w", err) + } + + if len(spawners) == 0 { + log.Info("No matching TaskSpawners found for webhook") + return true, nil // Not an error, just nothing to do + } + + log.Info("Found matching TaskSpawners", "count", len(spawners)) + + tasksCreated := 0 + + for _, spawner := range spawners { + spawnerLog := log.WithValues("spawner", spawner.Name, "namespace", spawner.Namespace) + + // Check if spawner is suspended + if spawner.Spec.Suspend != nil && *spawner.Spec.Suspend { + spawnerLog.V(1).Info("Skipping suspended spawner") + continue + } + + // Check max concurrency + // Note: For webhook TaskSpawners, activeTasks is updated by the kelos-controller + // when Tasks change status. This provides eventually consistent rate limiting. + if spawner.Spec.MaxConcurrency != nil && *spawner.Spec.MaxConcurrency > 0 { + activeTasks := spawner.Status.ActiveTasks + if int32(activeTasks) >= *spawner.Spec.MaxConcurrency { + spawnerLog.Info("Max concurrency reached, dropping webhook event", + "activeTasks", activeTasks, + "maxConcurrency", *spawner.Spec.MaxConcurrency, + "reason", "Webhook accepted but task creation skipped due to concurrency limits") + continue // Skip this spawner, continue with others + } + } + + // Check if this webhook matches the spawner's filters + matches, err := h.matchesSpawner(spawner, eventType, eventData) + if err != nil { + spawnerLog.Error(err, "Failed to check spawner match") + continue + } + + if !matches { + spawnerLog.Info("Webhook does not match spawner filters") + continue + } + + spawnerLog.Info("Webhook matches spawner filters - creating task") + + // Create task for this spawner + err = h.createTask(ctx, spawner, eventType, eventData, deliveryID) + if err != nil { + spawnerLog.Error(err, "Failed to create task") + continue + } + + tasksCreated++ + spawnerLog.Info("Successfully created task from webhook") + } + + log.Info("Webhook processing completed", "totalSpawners", len(spawners), "tasksCreated", tasksCreated) + return tasksCreated > 0, nil +} + +// getMatchingSpawners returns TaskSpawners that match the webhook source. +func (h *WebhookHandler) getMatchingSpawners(ctx context.Context) ([]*v1alpha1.TaskSpawner, error) { + var spawnerList v1alpha1.TaskSpawnerList + if err := h.client.List(ctx, &spawnerList, &client.ListOptions{}); err != nil { + return nil, err + } + + var matching []*v1alpha1.TaskSpawner + for i := range spawnerList.Items { + spawner := &spawnerList.Items[i] + + switch h.source { + case GitHubSource: + if spawner.Spec.When.GitHubWebhook != nil { + matching = append(matching, spawner) + } + } + } + + return matching, nil +} + +// matchesSpawner checks if the webhook matches the spawner's configuration. +func (h *WebhookHandler) matchesSpawner(spawner *v1alpha1.TaskSpawner, eventType string, eventData *GitHubEventData) (bool, error) { + switch h.source { + case GitHubSource: + if spawner.Spec.When.GitHubWebhook == nil { + return false, nil + } + + // Check repository filter first + if spawner.Spec.When.GitHubWebhook.Repository != "" { + if eventData.Repository != spawner.Spec.When.GitHubWebhook.Repository { + return false, nil + } + } + + return MatchesGitHubEvent(spawner.Spec.When.GitHubWebhook, eventType, eventData) + + default: + return false, fmt.Errorf("unsupported source: %s", h.source) + } +} + +// createTask creates a new Task from the webhook event. +func (h *WebhookHandler) createTask(ctx context.Context, spawner *v1alpha1.TaskSpawner, eventType string, eventData *GitHubEventData, deliveryID string) error { + log := h.log.WithValues("spawner", spawner.Name, "namespace", spawner.Namespace, "eventType", eventType, "deliveryID", deliveryID) + + // Extract template variables based on source + var templateVars map[string]interface{} + + switch h.source { + case GitHubSource: + templateVars = ExtractGitHubWorkItem(eventData) + + default: + return fmt.Errorf("unsupported source: %s", h.source) + } + + log.Info("Extracted template variables", "ID", templateVars["ID"], "Title", templateVars["Title"], "Action", templateVars["Action"]) + + // Build unique task name using a hash of the delivery ID to avoid collisions. + // Hashing gives uniform 12-hex-char suffix regardless of input length, + // avoiding the collision risk of simple prefix truncation. + sanitizedEventType := strings.ReplaceAll(eventType, "_", "-") + sum := sha256.Sum256([]byte(deliveryID)) + shortHash := hex.EncodeToString(sum[:])[:12] + taskName := fmt.Sprintf("%s-%s-%s", spawner.Name, sanitizedEventType, shortHash) + if len(taskName) > 63 { + taskName = strings.TrimRight(taskName[:63], "-.") + } + + // Resolve GVK for the spawner owner reference + gvks, _, err := h.client.Scheme().ObjectKinds(spawner) + if err != nil || len(gvks) == 0 { + return fmt.Errorf("failed to get GVK for TaskSpawner: %w", err) + } + gvk := gvks[0] + + // Create the task — BuildTask sets kelos.dev/taskspawner label and owner reference + task, err := h.taskBuilder.BuildTask( + taskName, + spawner.Namespace, + &spawner.Spec.TaskTemplate, + templateVars, + &taskbuilder.SpawnerRef{ + Name: spawner.Name, + UID: string(spawner.UID), + APIVersion: gvk.GroupVersion().String(), + Kind: gvk.Kind, + }, + ) + if err != nil { + return fmt.Errorf("failed to build task: %w", err) + } + + if err := h.client.Create(ctx, task); err != nil { + return fmt.Errorf("failed to create task: %w", err) + } + + return nil +} diff --git a/internal/webhook/handler_test.go b/internal/webhook/handler_test.go new file mode 100644 index 00000000..ff718799 --- /dev/null +++ b/internal/webhook/handler_test.go @@ -0,0 +1,455 @@ +package webhook + +import ( + "bytes" + "context" + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-logr/logr" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + kelosv1alpha1 "github.com/kelos-dev/kelos/api/v1alpha1" + "github.com/kelos-dev/kelos/internal/taskbuilder" +) + +const testSecret = "test-webhook-secret" + +// signPayload computes the GitHub-style HMAC-SHA256 signature for a payload. +func signPayload(payload, secret []byte) string { + mac := hmac.New(sha256.New, secret) + mac.Write(payload) + return "sha256=" + hex.EncodeToString(mac.Sum(nil)) +} + +// newTestHandler creates a WebhookHandler backed by a fake client with the given objects. +func newTestHandler(t *testing.T, objs ...client.Object) *WebhookHandler { + t.Helper() + scheme := runtime.NewScheme() + if err := kelosv1alpha1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objs...). + WithStatusSubresource(&kelosv1alpha1.TaskSpawner{}). + Build() + + tb, err := taskbuilder.NewTaskBuilder(fakeClient) + if err != nil { + t.Fatal(err) + } + + return &WebhookHandler{ + client: fakeClient, + source: GitHubSource, + log: logr.Discard(), + taskBuilder: tb, + secret: []byte(testSecret), + deliveryCache: NewDeliveryCache(context.Background()), + } +} + +// issuesPayload is a minimal valid GitHub issues webhook payload. +const issuesPayload = `{ + "action": "opened", + "sender": {"login": "testuser"}, + "repository": {"full_name": "org/repo", "name": "repo", "owner": {"login": "org"}}, + "issue": { + "number": 42, + "title": "Test Issue", + "body": "Test body", + "html_url": "https://github.com/org/repo/issues/42", + "state": "open", + "labels": [] + } +}` + +func TestServeHTTP_RejectsNonPOST(t *testing.T) { + handler := newTestHandler(t) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + rr := httptest.NewRecorder() + + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusMethodNotAllowed { + t.Errorf("Expected %d, got %d", http.StatusMethodNotAllowed, rr.Code) + } +} + +func TestServeHTTP_RejectsInvalidSignature(t *testing.T) { + handler := newTestHandler(t) + + payload := []byte(issuesPayload) + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(payload)) + req.Header.Set(GitHubEventHeader, "issues") + req.Header.Set(GitHubSignatureHeader, "sha256=invalid") + req.Header.Set(GitHubDeliveryHeader, "test-delivery-1") + rr := httptest.NewRecorder() + + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusUnauthorized { + t.Errorf("Expected %d, got %d", http.StatusUnauthorized, rr.Code) + } +} + +func TestServeHTTP_AcceptsValidSignature(t *testing.T) { + handler := newTestHandler(t) + + payload := []byte(issuesPayload) + sig := signPayload(payload, []byte(testSecret)) + + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(payload)) + req.Header.Set(GitHubEventHeader, "issues") + req.Header.Set(GitHubSignatureHeader, sig) + req.Header.Set(GitHubDeliveryHeader, "test-delivery-2") + rr := httptest.NewRecorder() + + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Errorf("Expected %d, got %d", http.StatusOK, rr.Code) + } +} + +func TestServeHTTP_DuplicateDeliveryIsIdempotent(t *testing.T) { + handler := newTestHandler(t) + + payload := []byte(issuesPayload) + sig := signPayload(payload, []byte(testSecret)) + deliveryID := "duplicate-delivery-id" + + // First request + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(payload)) + req.Header.Set(GitHubEventHeader, "issues") + req.Header.Set(GitHubSignatureHeader, sig) + req.Header.Set(GitHubDeliveryHeader, deliveryID) + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Fatalf("First request: expected %d, got %d", http.StatusOK, rr.Code) + } + + // Second request with same delivery ID + req = httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(payload)) + req.Header.Set(GitHubEventHeader, "issues") + req.Header.Set(GitHubSignatureHeader, sig) + req.Header.Set(GitHubDeliveryHeader, deliveryID) + rr = httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Errorf("Duplicate request: expected %d, got %d", http.StatusOK, rr.Code) + } +} + +func TestServeHTTP_CreatesTaskForMatchingSpawner(t *testing.T) { + spawner := &kelosv1alpha1.TaskSpawner{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-spawner", + Namespace: "default", + UID: "test-uid-123", + }, + Spec: kelosv1alpha1.TaskSpawnerSpec{ + When: kelosv1alpha1.When{ + GitHubWebhook: &kelosv1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + Filters: []kelosv1alpha1.GitHubWebhookFilter{ + { + Event: "issues", + Action: "opened", + }, + }, + }, + }, + TaskTemplate: kelosv1alpha1.TaskTemplate{ + Type: "claude-code", + Credentials: kelosv1alpha1.Credentials{ + Type: "api-key", + }, + WorkspaceRef: &kelosv1alpha1.WorkspaceReference{ + Name: "test-workspace", + }, + PromptTemplate: "{{.Title}}", + }, + }, + } + + handler := newTestHandler(t, spawner) + + payload := []byte(issuesPayload) + sig := signPayload(payload, []byte(testSecret)) + + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(payload)) + req.Header.Set(GitHubEventHeader, "issues") + req.Header.Set(GitHubSignatureHeader, sig) + req.Header.Set(GitHubDeliveryHeader, "create-task-delivery") + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Fatalf("Expected %d, got %d", http.StatusOK, rr.Code) + } + + // Verify the task was created + var taskList kelosv1alpha1.TaskList + if err := handler.client.List(context.Background(), &taskList); err != nil { + t.Fatal(err) + } + + if len(taskList.Items) != 1 { + t.Fatalf("Expected 1 task, got %d", len(taskList.Items)) + } + + task := taskList.Items[0] + if task.Namespace != "default" { + t.Errorf("Expected task namespace 'default', got %q", task.Namespace) + } + if task.Labels["kelos.dev/taskspawner"] != "test-spawner" { + t.Errorf("Expected taskspawner label 'test-spawner', got %q", task.Labels["kelos.dev/taskspawner"]) + } + if task.Spec.Prompt != "Test Issue" { + t.Errorf("Expected prompt 'Test Issue', got %q", task.Spec.Prompt) + } + // Verify owner reference was set by BuildTask + if len(task.OwnerReferences) != 1 { + t.Fatalf("Expected 1 owner reference, got %d", len(task.OwnerReferences)) + } + if task.OwnerReferences[0].Name != "test-spawner" { + t.Errorf("Expected owner ref name 'test-spawner', got %q", task.OwnerReferences[0].Name) + } + if task.OwnerReferences[0].Kind != "TaskSpawner" { + t.Errorf("Expected owner ref kind 'TaskSpawner', got %q", task.OwnerReferences[0].Kind) + } +} + +func TestServeHTTP_SkipsNonMatchingSpawner(t *testing.T) { + // Spawner only listens for pull_request events, not issues + spawner := &kelosv1alpha1.TaskSpawner{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pr-only-spawner", + Namespace: "default", + UID: "test-uid-456", + }, + Spec: kelosv1alpha1.TaskSpawnerSpec{ + When: kelosv1alpha1.When{ + GitHubWebhook: &kelosv1alpha1.GitHubWebhook{ + Events: []string{"pull_request"}, + }, + }, + TaskTemplate: kelosv1alpha1.TaskTemplate{ + Type: "claude-code", + Credentials: kelosv1alpha1.Credentials{ + Type: "api-key", + }, + WorkspaceRef: &kelosv1alpha1.WorkspaceReference{ + Name: "test-workspace", + }, + }, + }, + } + + handler := newTestHandler(t, spawner) + + payload := []byte(issuesPayload) + sig := signPayload(payload, []byte(testSecret)) + + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(payload)) + req.Header.Set(GitHubEventHeader, "issues") + req.Header.Set(GitHubSignatureHeader, sig) + req.Header.Set(GitHubDeliveryHeader, "no-match-delivery") + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Fatalf("Expected %d, got %d", http.StatusOK, rr.Code) + } + + // Verify no task was created + var taskList kelosv1alpha1.TaskList + if err := handler.client.List(context.Background(), &taskList); err != nil { + t.Fatal(err) + } + + if len(taskList.Items) != 0 { + t.Errorf("Expected 0 tasks, got %d", len(taskList.Items)) + } +} + +func TestServeHTTP_SkipsSuspendedSpawner(t *testing.T) { + suspended := true + spawner := &kelosv1alpha1.TaskSpawner{ + ObjectMeta: metav1.ObjectMeta{ + Name: "suspended-spawner", + Namespace: "default", + UID: "test-uid-789", + }, + Spec: kelosv1alpha1.TaskSpawnerSpec{ + Suspend: &suspended, + When: kelosv1alpha1.When{ + GitHubWebhook: &kelosv1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + }, + }, + TaskTemplate: kelosv1alpha1.TaskTemplate{ + Type: "claude-code", + Credentials: kelosv1alpha1.Credentials{ + Type: "api-key", + }, + WorkspaceRef: &kelosv1alpha1.WorkspaceReference{ + Name: "test-workspace", + }, + }, + }, + } + + handler := newTestHandler(t, spawner) + + payload := []byte(issuesPayload) + sig := signPayload(payload, []byte(testSecret)) + + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(payload)) + req.Header.Set(GitHubEventHeader, "issues") + req.Header.Set(GitHubSignatureHeader, sig) + req.Header.Set(GitHubDeliveryHeader, "suspended-delivery") + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Fatalf("Expected %d, got %d", http.StatusOK, rr.Code) + } + + // Verify no task was created + var taskList kelosv1alpha1.TaskList + if err := handler.client.List(context.Background(), &taskList); err != nil { + t.Fatal(err) + } + + if len(taskList.Items) != 0 { + t.Errorf("Expected 0 tasks for suspended spawner, got %d", len(taskList.Items)) + } +} + +func TestServeHTTP_MaxConcurrencyDropsEvent(t *testing.T) { + maxConcurrency := int32(1) + spawner := &kelosv1alpha1.TaskSpawner{ + ObjectMeta: metav1.ObjectMeta{ + Name: "limited-spawner", + Namespace: "default", + UID: "test-uid-max", + }, + Spec: kelosv1alpha1.TaskSpawnerSpec{ + MaxConcurrency: &maxConcurrency, + When: kelosv1alpha1.When{ + GitHubWebhook: &kelosv1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + }, + }, + TaskTemplate: kelosv1alpha1.TaskTemplate{ + Type: "claude-code", + Credentials: kelosv1alpha1.Credentials{ + Type: "api-key", + }, + WorkspaceRef: &kelosv1alpha1.WorkspaceReference{ + Name: "test-workspace", + }, + PromptTemplate: "{{.Title}}", + }, + }, + Status: kelosv1alpha1.TaskSpawnerStatus{ + ActiveTasks: 1, // Already at max + }, + } + + handler := newTestHandler(t, spawner) + + payload := []byte(issuesPayload) + sig := signPayload(payload, []byte(testSecret)) + + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(payload)) + req.Header.Set(GitHubEventHeader, "issues") + req.Header.Set(GitHubSignatureHeader, sig) + req.Header.Set(GitHubDeliveryHeader, "max-concurrency-delivery") + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Fatalf("Expected %d, got %d", http.StatusOK, rr.Code) + } + + // Verify no task was created + var taskList kelosv1alpha1.TaskList + if err := handler.client.List(context.Background(), &taskList); err != nil { + t.Fatal(err) + } + + if len(taskList.Items) != 0 { + t.Errorf("Expected 0 tasks when at max concurrency, got %d", len(taskList.Items)) + } +} + +func TestServeHTTP_RepositoryFilterRejectsWrongRepo(t *testing.T) { + spawner := &kelosv1alpha1.TaskSpawner{ + ObjectMeta: metav1.ObjectMeta{ + Name: "repo-filtered-spawner", + Namespace: "default", + UID: "test-uid-repo", + }, + Spec: kelosv1alpha1.TaskSpawnerSpec{ + When: kelosv1alpha1.When{ + GitHubWebhook: &kelosv1alpha1.GitHubWebhook{ + Events: []string{"issues"}, + Repository: "other-org/other-repo", + }, + }, + TaskTemplate: kelosv1alpha1.TaskTemplate{ + Type: "claude-code", + Credentials: kelosv1alpha1.Credentials{ + Type: "api-key", + }, + WorkspaceRef: &kelosv1alpha1.WorkspaceReference{ + Name: "test-workspace", + }, + PromptTemplate: "{{.Title}}", + }, + }, + } + + handler := newTestHandler(t, spawner) + + // issuesPayload has repository "org/repo", spawner expects "other-org/other-repo" + payload := []byte(issuesPayload) + sig := signPayload(payload, []byte(testSecret)) + + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(payload)) + req.Header.Set(GitHubEventHeader, "issues") + req.Header.Set(GitHubSignatureHeader, sig) + req.Header.Set(GitHubDeliveryHeader, "repo-filter-delivery") + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Fatalf("Expected %d, got %d", http.StatusOK, rr.Code) + } + + // Verify no task was created + var taskList kelosv1alpha1.TaskList + if err := handler.client.List(context.Background(), &taskList); err != nil { + t.Fatal(err) + } + + if len(taskList.Items) != 0 { + t.Errorf("Expected 0 tasks for wrong repo, got %d", len(taskList.Items)) + } +} diff --git a/internal/webhook/signature.go b/internal/webhook/signature.go new file mode 100644 index 00000000..b27d3f8e --- /dev/null +++ b/internal/webhook/signature.go @@ -0,0 +1,38 @@ +package webhook + +import ( + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "fmt" + "strings" +) + +// ValidateGitHubSignature validates a GitHub webhook signature. +// GitHub sends signatures in the format "sha256=". +func ValidateGitHubSignature(payload []byte, signature string, secret []byte) error { + if signature == "" { + return fmt.Errorf("missing signature") + } + + // GitHub signature format: "sha256=" + if !strings.HasPrefix(signature, "sha256=") { + return fmt.Errorf("invalid signature format: expected sha256= prefix") + } + + expectedSig := signature[7:] // Remove "sha256=" prefix + return validateHMACSignature(payload, expectedSig, secret) +} + +// validateHMACSignature performs HMAC-SHA256 validation against the expected hex digest. +func validateHMACSignature(payload []byte, expectedSig string, secret []byte) error { + mac := hmac.New(sha256.New, secret) + mac.Write(payload) + actualSig := hex.EncodeToString(mac.Sum(nil)) + + if !hmac.Equal([]byte(actualSig), []byte(expectedSig)) { + return fmt.Errorf("signature verification failed") + } + + return nil +} diff --git a/internal/webhook/signature_test.go b/internal/webhook/signature_test.go new file mode 100644 index 00000000..86651338 --- /dev/null +++ b/internal/webhook/signature_test.go @@ -0,0 +1,51 @@ +package webhook + +import ( + "testing" +) + +func TestValidateGitHubSignature(t *testing.T) { + secret := []byte("my-secret-key") + payload := []byte(`{"action":"opened","number":1}`) + + tests := []struct { + name string + signature string + wantErr bool + }{ + { + name: "valid signature", + signature: "sha256=fb463367c1f8d533acc23e11f8d09ff396c4e2ed73668fcf782f221f779e0424", + wantErr: false, + }, + { + name: "invalid signature", + signature: "sha256=invalid", + wantErr: true, + }, + { + name: "missing prefix", + signature: "fb463367c1f8d533acc23e11f8d09ff396c4e2ed73668fcf782f221f779e0424", + wantErr: true, + }, + { + name: "empty signature", + signature: "", + wantErr: true, + }, + { + name: "wrong prefix", + signature: "sha1=fb463367c1f8d533acc23e11f8d09ff396c4e2ed73668fcf782f221f779e0424", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateGitHubSignature(payload, tt.signature, secret) + if (err != nil) != tt.wantErr { + t.Errorf("ValidateGitHubSignature() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/opencode/Dockerfile b/opencode/Dockerfile index 43eaad23..9c8e79d3 100644 --- a/opencode/Dockerfile +++ b/opencode/Dockerfile @@ -27,7 +27,7 @@ RUN ARCH=$(dpkg --print-architecture) \ ENV PATH="/usr/local/go/bin:${PATH}" -ARG OPENCODE_VERSION=1.3.4 +ARG OPENCODE_VERSION=1.3.9 RUN npm install -g opencode-ai@${OPENCODE_VERSION} COPY opencode/kelos_entrypoint.sh /kelos_entrypoint.sh diff --git a/test/integration/taskspawner_test.go b/test/integration/taskspawner_test.go index 84eed323..5d1d2d7b 100644 --- a/test/integration/taskspawner_test.go +++ b/test/integration/taskspawner_test.go @@ -118,6 +118,7 @@ var _ = Describe("TaskSpawner Controller", func() { "--taskspawner-namespace="+ns.Name, "--github-owner=kelos-dev", "--github-repo=kelos", + "--gh-proxy-url="+controller.WorkspaceGHProxyServiceURL(ns.Name, "test-workspace"), )) By("Verifying the ServiceAccount") @@ -193,7 +194,9 @@ var _ = Describe("TaskSpawner Controller", func() { }, Spec: kelosv1alpha1.TaskSpawnerSpec{ When: kelosv1alpha1.When{ - GitHubIssues: &kelosv1alpha1.GitHubIssues{}, + GitHubIssues: &kelosv1alpha1.GitHubIssues{ + Reporting: &kelosv1alpha1.GitHubReporting{Enabled: true}, + }, }, TaskTemplate: kelosv1alpha1.TaskTemplate{ Type: "claude-code", @@ -794,7 +797,8 @@ var _ = Describe("TaskSpawner Controller", func() { Spec: kelosv1alpha1.TaskSpawnerSpec{ When: kelosv1alpha1.When{ GitHubIssues: &kelosv1alpha1.GitHubIssues{ - State: "open", + State: "open", + Reporting: &kelosv1alpha1.GitHubReporting{Enabled: true}, }, }, TaskTemplate: kelosv1alpha1.TaskTemplate{ @@ -1541,7 +1545,9 @@ var _ = Describe("TaskSpawner Controller", func() { }, Spec: kelosv1alpha1.TaskSpawnerSpec{ When: kelosv1alpha1.When{ - GitHubIssues: &kelosv1alpha1.GitHubIssues{}, + GitHubIssues: &kelosv1alpha1.GitHubIssues{ + Reporting: &kelosv1alpha1.GitHubReporting{Enabled: true}, + }, }, TaskTemplate: kelosv1alpha1.TaskTemplate{ Type: "claude-code", @@ -1679,7 +1685,9 @@ var _ = Describe("TaskSpawner Controller", func() { }, Spec: kelosv1alpha1.TaskSpawnerSpec{ When: kelosv1alpha1.When{ - GitHubIssues: &kelosv1alpha1.GitHubIssues{}, + GitHubIssues: &kelosv1alpha1.GitHubIssues{ + Reporting: &kelosv1alpha1.GitHubReporting{Enabled: true}, + }, }, TaskTemplate: kelosv1alpha1.TaskTemplate{ Type: "claude-code", @@ -1825,7 +1833,9 @@ var _ = Describe("TaskSpawner Controller", func() { }, Spec: kelosv1alpha1.TaskSpawnerSpec{ When: kelosv1alpha1.When{ - GitHubIssues: &kelosv1alpha1.GitHubIssues{}, + GitHubIssues: &kelosv1alpha1.GitHubIssues{ + Reporting: &kelosv1alpha1.GitHubReporting{Enabled: true}, + }, }, TaskTemplate: kelosv1alpha1.TaskTemplate{ Type: "claude-code",