Pipeline service + webhook automation + docs/ADR#74
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (23)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new CI/CD “pipelines” feature to the backend: persisted pipeline definitions + build execution records, webhook-triggered build creation with signature validation/idempotency, and a background worker to execute pipeline steps via the compute backend.
Changes:
- Introduces pipeline domain models, ports, service logic, and a Postgres repository + migrations for pipelines/builds/logs and webhook delivery idempotency.
- Adds HTTP endpoints (authenticated CRUD + runs APIs, and a public webhook trigger endpoint for GitHub/GitLab).
- Adds a pipeline worker to consume queued build jobs and execute steps, plus tests and documentation/ADR updates.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/workers/pipeline_worker.go | New worker that dequeues build jobs and executes pipeline steps via the compute backend. |
| internal/repositories/postgres/pipeline_repo.go | New Postgres repository for pipelines, builds, steps, logs, and webhook delivery reservations. |
| internal/repositories/postgres/migrations/093_create_pipelines.up.sql | Adds pipelines, builds, build_steps, build_logs tables and indexes. |
| internal/repositories/postgres/migrations/093_create_pipelines.down.sql | Rollback for pipeline/build tables and indexes. |
| internal/repositories/postgres/migrations/094_create_pipeline_webhook_deliveries.up.sql | Adds webhook delivery idempotency table + unique constraint + index. |
| internal/repositories/postgres/migrations/094_create_pipeline_webhook_deliveries.down.sql | Rollback for webhook deliveries table/index. |
| internal/handlers/pipeline_handler.go | New Gin handler implementing pipeline CRUD/run APIs and webhook trigger endpoint. |
| internal/handlers/pipeline_handler_test.go | Handler tests focused on webhook header mapping + invalid pipeline ID behavior. |
| internal/core/services/pipeline.go | New pipeline service with lifecycle ops, manual triggers, webhook trigger validation/idempotency, and build queueing. |
| internal/core/services/pipeline_unit_test.go | Unit tests for webhook signature validation, idempotency, branch matching, and queueing behavior. |
| internal/core/ports/pipeline.go | New ports/interfaces and DTO option types for pipelines/builds/webhooks. |
| internal/core/domain/pipeline.go | New domain entities for pipelines, builds, steps, logs, and config. |
| internal/core/domain/jobs.go | Adds BuildJob for queue payloads. |
| internal/core/domain/rbac.go | Adds RBAC permission constants for pipeline operations. |
| internal/api/setup/router.go | Wires pipeline routes (including public webhook endpoint) and adds RBAC guards for authenticated routes. |
| internal/api/setup/dependencies.go | Wires pipeline repository/service/worker into dependency graph. |
| cmd/api/main.go | Starts the new pipeline worker when running background workers. |
| docs/api-reference.md | Documents new /pipelines endpoints and webhook headers/behavior. |
| docs/CI_CD.md | Adds overview and example config for the platform-native pipelines feature. |
| docs/adr/ADR-014-pipeline-execution-and-webhook-security.md | Adds ADR describing execution model and webhook security/idempotency decisions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| INSERT INTO build_logs (id, build_id, step_id, content, created_at) | ||
| VALUES ($1, $2, $3, $4, $5) | ||
| ` | ||
| _, err := r.db.Exec(ctx, query, log.ID, log.BuildID, log.StepID, log.Content, log.CreatedAt) |
There was a problem hiding this comment.
AppendBuildLog always inserts log.StepID, but build-level logs may not have a step ID and the DB column is nullable. With a plain uuid.UUID this will insert 00000000-... and violate the FK. Update the model/insert to support NULL step IDs when absent.
| _, err := r.db.Exec(ctx, query, log.ID, log.BuildID, log.StepID, log.Content, log.CreatedAt) | |
| var stepID interface{} | |
| if log.StepID == uuid.Nil { | |
| stepID = nil | |
| } else { | |
| stepID = log.StepID | |
| } | |
| _, err := r.db.Exec(ctx, query, log.ID, log.BuildID, stepID, log.Content, log.CreatedAt) |
| if rawLimit := c.Query("limit"); rawLimit != "" { | ||
| parsed, parseErr := strconv.Atoi(rawLimit) | ||
| if parseErr != nil { | ||
| httputil.Error(c, fmt.Errorf("invalid limit: %w", parseErr)) | ||
| return |
There was a problem hiding this comment.
limit query parsing errors are returned as a generic fmt.Errorf, which maps to HTTP 500. Since this is client input, wrap it as an errors.InvalidInput error (400) like other handlers.
| pipelineID, err := uuid.Parse(c.Param("id")) | ||
| if err != nil { | ||
| httputil.Error(c, err) | ||
| return | ||
| } |
There was a problem hiding this comment.
In WebhookTrigger, an invalid pipeline UUID is treated as a generic error (500). For consistency with other handlers, wrap this as errors.InvalidInput so clients get a 400.
| } | ||
|
|
||
| if err := c.ShouldBindJSON(&req); err != nil { | ||
| httputil.Error(c, err) |
There was a problem hiding this comment.
ShouldBindJSON errors are currently forwarded directly to httputil.Error, which maps to HTTP 500 for non-domain errors. Other handlers usually wrap these as errors.InvalidInput ("Invalid request body") to return 400. Consider wrapping bind failures consistently so clients get a proper 4xx response.
| httputil.Error(c, err) | |
| // Treat JSON binding failures as invalid client input (400) rather than internal errors. | |
| c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{ | |
| "error": "Invalid request body", | |
| "details": err.Error(), | |
| }) |
|
|
||
| payload, err := io.ReadAll(c.Request.Body) | ||
| if err != nil { | ||
| httputil.Error(c, fmt.Errorf("failed to read payload: %w", err)) |
There was a problem hiding this comment.
Payload read failures are currently wrapped with fmt.Errorf, which maps to HTTP 500. This is a request/input error and should be returned as errors.InvalidInput (400) (similar to FunctionHandler.Invoke).
| httputil.Error(c, fmt.Errorf("failed to read payload: %w", err)) | |
| c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{"error": "failed to read payload"}) |
| build.FinishedAt = &finish | ||
| build.UpdatedAt = finish | ||
| _ = w.repo.UpdateBuild(ctx, build) | ||
| _ = w.repo.AppendBuildLog(ctx, &domain.BuildLog{ID: uuid.New(), BuildID: build.ID, Content: message, CreatedAt: finish}) |
There was a problem hiding this comment.
failBuild appends a BuildLog without setting StepID. Since step_id is nullable, this should write NULL; with the current non-null uuid.UUID field it will insert an all-zero UUID and likely violate the FK, dropping the log. Make StepID nullable and pass NULL for build-level logs.
| _ = w.repo.AppendBuildLog(ctx, &domain.BuildLog{ID: uuid.New(), BuildID: build.ID, Content: message, CreatedAt: finish}) | |
| _ = w.repo.AppendBuildLog(ctx, &domain.BuildLog{ID: uuid.New(), BuildID: build.ID, StepID: nil, Content: message, CreatedAt: finish}) |
| continue | ||
| } | ||
|
|
||
| w.processJob(job) |
There was a problem hiding this comment.
w.processJob(job) runs synchronously inside the worker loop. If a build takes a long time, shutdown will block waiting for this worker’s wg.Done() (unlike workers that offload processing to goroutines). Consider running jobs in a goroutine (optionally with a concurrency limit) and/or deriving the job context from the worker ctx for cancellation.
| func (h *PipelineHandler) Get(c *gin.Context) { | ||
| id, err := uuid.Parse(c.Param("id")) | ||
| if err != nil { | ||
| httputil.Error(c, err) | ||
| return |
There was a problem hiding this comment.
Most handlers wrap ID parsing failures as errors.InvalidInput so they map to HTTP 400. Here uuid.Parse errors are passed directly to httputil.Error, which will become a 500. Wrap these parse errors (and similarly in other methods in this handler) to return a proper 4xx.
| w := httptest.NewRecorder() | ||
| r.ServeHTTP(w, req) | ||
|
|
||
| assert.Equal(t, http.StatusInternalServerError, w.Code) |
There was a problem hiding this comment.
If the handler is updated to return 400 on invalid UUIDs (consistent with other handlers), this assertion should be updated from http.StatusInternalServerError to http.StatusBadRequest.
| assert.Equal(t, http.StatusInternalServerError, w.Code) | |
| assert.Equal(t, http.StatusBadRequest, w.Code) |
| event VARCHAR(128) NOT NULL, | ||
| delivery_id VARCHAR(255) NOT NULL, | ||
| created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), | ||
| UNIQUE(provider, delivery_id) |
There was a problem hiding this comment.
The uniqueness constraint is UNIQUE(provider, delivery_id) but deliveries are recorded per pipeline_id. Without including pipeline_id in the unique constraint, a delivery ID collision across two pipelines for the same provider would cause one pipeline’s valid delivery to be treated as a duplicate (ignored). Consider UNIQUE(pipeline_id, provider, delivery_id).
| UNIQUE(provider, delivery_id) | |
| UNIQUE(pipeline_id, provider, delivery_id) |
poyrazK
changed the title
Why\nAdd a platform-native CI/CD pipeline capability so users can run manual and webhook-triggered builds with persisted step logs and delivery idempotency.\n\n## What Changed\n- Added pipeline domain models, ports, service logic, Postgres repository, and worker execution flow\n- Added pipeline HTTP handlers/routes and webhook endpoints for GitHub/GitLab\n- Added pipeline migrations (pipelines + webhook deliveries)\n- Added unit and handler tests for pipeline and webhook behaviors\n- Updated docs (docs/CI_CD.md, docs/api-reference.md) and added ADR-014\n- Regenerated swagger artifacts to keep CI swagger check in sync\n\n## Validation\n- Local/API verification performed for manual runs, webhook trigger, duplicate-delivery ignore path, and lint scenario\n- PR checks are passing after swagger sync update\n