feat: GitLab preview deployments via webhook#4089
feat: GitLab preview deployments via webhook#4089edouard-mangel wants to merge 6 commits intoDokploy:canaryfrom
Conversation
dosubot
bot
added
the
size:XXL
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const previewLimit = app.previewLimit ?? 0; | ||
| if (app.previewDeployments.length > previewLimit) { | ||
| continue; |
There was a problem hiding this comment.
With previewLimit = 3 and exactly 3 existing deployments, the check 3 > 3 evaluates to false, so the guard is bypassed and a 4th deployment is created — one beyond the configured limit. The condition should use >= so the limit is enforced as an inclusive maximum.
| const previewLimit = app.previewLimit ?? 0; | |
| if (app.previewDeployments.length > previewLimit) { | |
| continue; | |
| // Preview limit | |
| const previewLimit = app.previewLimit ?? 0; | |
| if (app.previewDeployments.length >= previewLimit) { |
There was a problem hiding this comment.
Fixed in commit 6dcaafb — changed > to >= so exactly N deployments at the configured limit correctly blocks the next one. Test updated to use the exact boundary case (3 existing == limit of 3) to verify the off-by-one is resolved.
|
|
||
| const totalApps = apps.length + composeApps.length; | ||
| res.status(200).json({ | ||
| message: |
There was a problem hiding this comment.
totalApps count includes apps skipped by watchPaths filtering
totalApps is computed as apps.length + composeApps.length, but the loop skips apps where shouldDeploy(app.watchPaths, modifiedFiles) returns false. Those skipped apps are still counted, so the success message can report "Deployed N apps" when fewer (or zero) were actually queued for deployment.
Consider tracking the count of actually-deployed apps separately if response accuracy matters.
There was a problem hiding this comment.
Fixed in commit 6dcaafb — the Push Hook now tracks a \ variable incremented only when a job is actually dispatched (after the \ / \ filter passes). Note: this comment's line shifted to the Tag Push Hook block, which has no \ filter, so \ there remains accurate.
There was a problem hiding this comment.
Pull request overview
Adds GitLab webhook-driven deployments (push/tag + merge-request preview deploy/teardown) to mirror the existing GitHub preview deployment flow, including permission-gated MR previews and UI support for configuring webhook secrets.
Changes:
- Introduces a new GitLab webhook handler for Push/Tag/Merge Request events with preview deployment lifecycle management.
- Adds GitLab provider utilities for member permission checks and MR note posting/deduplication for security blocks.
- Persists and surfaces a per-provider
webhook_secret(DB schema + migration + dashboard UI), plus adds Vitest coverage for the new flow.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/server/src/utils/providers/gitlab.ts | Adds GitLab member permission check + MR note helpers for security block messaging. |
| packages/server/src/services/gitlab.ts | Adds lookup by webhook secret for authenticating incoming GitLab webhooks. |
| packages/server/src/db/schema/gitlab.ts | Adds webhookSecret column with application-side default generation. |
| apps/dokploy/server/api/routers/gitlab.ts | Tightens gitlab.one access by enforcing active organization ownership. |
| apps/dokploy/pages/api/deploy/gitlab.ts | New GitLab webhook endpoint implementing push/tag deploys and MR preview deploy/teardown. |
| apps/dokploy/drizzle/0152_gitlab_preview_webhook.sql | Migration to add/backfill webhook_secret and enforce NOT NULL. |
| apps/dokploy/components/dashboard/settings/git/gitlab/edit-gitlab-provider.tsx | Exposes webhook URL + secret in the GitLab provider UI with copy actions. |
| apps/dokploy/components/dashboard/application/preview-deployments/show-preview-settings.tsx | Updates preview permission text for GitLab vs GitHub sources. |
| apps/dokploy/test/utils/gitlab-preview-utils.test.ts | Unit tests for new GitLab provider utility functions. |
| apps/dokploy/test/deploy/gitlab.test.ts | End-to-end-ish tests for the new GitLab webhook handler behaviors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const response = await fetch( | ||
| `${baseUrl}/api/v4/projects/${projectId}/merge_requests/${mrIid}/notes`, | ||
| { headers: { Authorization: `Bearer ${gitlabProvider.accessToken}` } }, | ||
| ); | ||
| if (!response.ok) { | ||
| return false; | ||
| } | ||
| const notes: { id: number; body: string }[] = await response.json(); | ||
| return notes.some((note) => note.body.includes(SECURITY_SENTINEL)); |
There was a problem hiding this comment.
hasExistingSecurityMRNote only fetches the first page of MR notes. GitLab paginates this endpoint (default page size is small), so the sentinel note may not be seen and duplicates can be posted. Consider iterating pages (e.g., via page/per_page or x-next-page headers) until exhausted / found, similar to the pagination used elsewhere in this file (branches/projects).
| const response = await fetch( | |
| `${baseUrl}/api/v4/projects/${projectId}/merge_requests/${mrIid}/notes`, | |
| { headers: { Authorization: `Bearer ${gitlabProvider.accessToken}` } }, | |
| ); | |
| if (!response.ok) { | |
| return false; | |
| } | |
| const notes: { id: number; body: string }[] = await response.json(); | |
| return notes.some((note) => note.body.includes(SECURITY_SENTINEL)); | |
| let page = 1; | |
| const perPage = 100; | |
| // Iterate through all pages of MR notes until the sentinel is found or no more pages remain | |
| while (true) { | |
| const response = await fetch( | |
| `${baseUrl}/api/v4/projects/${projectId}/merge_requests/${mrIid}/notes?per_page=${perPage}&page=${page}`, | |
| { headers: { Authorization: `Bearer ${gitlabProvider.accessToken}` } }, | |
| ); | |
| if (!response.ok) { | |
| return false; | |
| } | |
| const notes: { id: number; body: string }[] = await response.json(); | |
| if (notes.some((note) => note.body.includes(SECURITY_SENTINEL))) { | |
| return true; | |
| } | |
| const nextPageHeader = response.headers.get("x-next-page"); | |
| if (!nextPageHeader) { | |
| break; | |
| } | |
| const nextPage = Number(nextPageHeader); | |
| if (!Number.isFinite(nextPage) || nextPage <= page) { | |
| break; | |
| } | |
| page = nextPage; | |
| } | |
| return false; |
There was a problem hiding this comment.
Fixed in commit 6dcaafb — replaced the single \ with a paginated loop that follows the \ header, requesting 100 notes per page until the sentinel is found or the header is empty. Tests updated to provide \ on mock responses.
| await createSecurityBlockedMRNote( | ||
| gitlabProvider.gitlabId, | ||
| projectId, | ||
| mrIid, | ||
| mrAuthor, | ||
| pathNamespace, | ||
| blockedAccessLevel, | ||
| ); |
There was a problem hiding this comment.
createSecurityBlockedMRNote can throw (e.g., GitLab notes API error). Since this call is not wrapped in a try/catch, a transient GitLab API failure will cause the entire webhook to fail and can prevent otherwise-authorized preview deployments from being queued. Treat the note as best-effort (catch/log errors) or handle it inside createSecurityBlockedMRNote similar to the GitHub security comment flow.
| await createSecurityBlockedMRNote( | |
| gitlabProvider.gitlabId, | |
| projectId, | |
| mrIid, | |
| mrAuthor, | |
| pathNamespace, | |
| blockedAccessLevel, | |
| ); | |
| try { | |
| await createSecurityBlockedMRNote( | |
| gitlabProvider.gitlabId, | |
| projectId, | |
| mrIid, | |
| mrAuthor, | |
| pathNamespace, | |
| blockedAccessLevel, | |
| ); | |
| } catch (error) { | |
| console.error("Error creating security blocked MR note:", error); | |
| } |
There was a problem hiding this comment.
Fixed in commit 6dcaafb — \ is now wrapped in a . A transient GitLab API error is logged but no longer throws, so authorized preview deployments are not blocked by a failed note post.
| vi.mocked(checkGitlabMemberPermissions) | ||
| .mockResolvedValueOnce({ hasWriteAccess: false, accessLevel: 20 }) | ||
| .mockResolvedValueOnce({ hasWriteAccess: true, accessLevel: 30 }); | ||
|
|
There was a problem hiding this comment.
This test assumes checkGitlabMemberPermissions is called once per app (it sets two mockResolvedValueOnce values). The handler hoists the permission check and calls it at most once per webhook event, so the second mocked return value is never used and the assertions about one blocked and one allowed app will fail. Update the test to reflect the single permission check behavior (or change the handler if per-app checks are intended).
There was a problem hiding this comment.
Fixed in commit 6dcaafb — the test was updated to reflect the hoisted permission check. It now mocks \ with a single \ (not two \ calls), expects exactly 1 API call covering all apps, and asserts that all apps are blocked when access is denied (semantically correct: access is per-MR-author, not per-app).
Replicates GitHub preview deployment functionality for GitLab: - New /api/deploy/gitlab webhook endpoint with X-Gitlab-Token auth - checkGitlabMemberPermissions (access_level >= 30 = Developer+) - mrNoteExists / hasExistingSecurityMRNote / createMergeRequestNote / updateMergeRequestNote / createSecurityBlockedMRNote - findGitlabByWebhookSecret service lookup - webhookSecret column added to gitlab table (migration 0152) - Teardown on close/merge runs before mrAuthor null-guard (security fix) - Each blocked app captures its own access level for the security note - Tests written TDD-first: 24/24 pass (10 unit + 14 handler) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…abel - edit-gitlab-provider.tsx: add Webhook Configuration section with read-only webhook URL and secret token fields (each with copy button) - show-preview-settings.tsx: previewRequireCollaboratorPermissions label switches to 'Require Member Access' with GitLab roles (Owner/Maintainer/ Developer) when sourceType is 'gitlab' Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- checkGitlabMemberPermissions: guard against userId=undefined when GitLab username lookup returns no results (prevents malformed API URL) - gitlab.one tRPC endpoint: add ownership check (organizationId + userId) so webhookSecret cannot be read by other authenticated users - Push Hook: extract added/modified/removed files from commits and pass to shouldDeploy so watchPaths filtering works correctly - Push Hook: add shouldDeploy filtering to compose apps loop (was missing) - Add tests: userId=undefined guard, watchPaths filtering on push Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- checkGitlabMemberPermissions: throw on non-ok userResponse (previously silently swallowed API failures for the username lookup) - gitlab.one tRPC: relax ownership check to org-scoped only (remove userId constraint so all org members can access shared providers, consistent with how gitlabProviders filters results) - Add test: userResponse 503 throws with clear error message Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Guard `hasExistingSecurityMRNote` against non-ok HTTP responses before
parsing JSON (was a silent failure path)
- Separate "Tag Push Hook" from "Push Hook" into distinct handlers; tag
deploys now parse `refs/tags/`, filter by `triggerType = "tag"`, and
omit the branch constraint — matching the GitHub handler pattern
- Add `triggerType = "push"` filter to push-hook app and compose queries
so tag-triggered repos are not accidentally deployed on push events
- Remove `unlabeled` from the MR action allowlist; the event previously
triggered security checks and the full app loop while doing nothing
(shouldCreateDeployment already excluded it)
- Hoist `checkGitlabMemberPermissions` before the per-app loop — the
check is per-MR-author so N apps no longer make N API calls
- Drop explicit `{ gitProvider: any }` return type on
`findGitlabByWebhookSecret`; TypeScript now infers the Drizzle type
- Use `encode(gen_random_bytes(21), 'base64')` in migration instead of
`gen_random_uuid()::text` for consistency with the rest of the schema
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
edouard-mangel
force-pushed
the
feat/gitlab-preview-deployments
branch
from
2035278 to
cf2f177
Compare
- Wrap createSecurityBlockedMRNote in try/catch — a transient GitLab API error no longer blocks authorized preview deployments from being queued - Fix off-by-one in preview limit guard: use >= so exactly N deployments at the configured limit correctly prevents a further deployment - Track deployed count in Push Hook handler so the response message reports apps actually queued, not apps matched before watchPaths filter - Paginate hasExistingSecurityMRNote with x-next-page header to avoid missing the security sentinel on MRs with more than 100 notes - Update tests: permission-hoist test now expects one checkGitlabMemberPermissions call covering all apps; preview-limit test uses exact boundary (3 == limit); fetch mocks include headers.get for the new pagination code path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Changes
Webhook handler (
pages/api/deploy/gitlab.ts)Push Hook(branch deploys withtriggerType = push) andTag Push Hook(tag deploys withtriggerType = tag) as separate code paths — mirrors the GitHub handler patternMerge Request Hookactions:open,update,reopen,labeled→ create/redeploy;close/merge→ teardown preview deploymentsServer utilities (
packages/server/src/utils/providers/gitlab.ts)checkGitlabMemberPermissions: verifies MR author has Developer+ access via GitLab Members APIhasExistingSecurityMRNote: deduplicates security block notes (guardsresponse.okbefore JSON parse)createSecurityBlockedMRNote/createMergeRequestNote: posts/updates MR notesServices (
packages/server/src/services/gitlab.ts)findGitlabByWebhookSecret: looks up the GitLab provider by webhook token (return type inferred, noany)Migration (
drizzle/0152_gitlab_preview_webhook.sql)webhook_secretcolumn, backfills withencode(gen_random_bytes(21), 'base64'), then setsNOT NULLTest plan
/api/deploy/gitlabtriggerType = pushand matching branch are deployedtriggerType = tagare deployed🤖 Generated with Claude Code
Greptile Summary
This PR implements GitLab preview deployment support via webhooks, mirroring the existing GitHub flow. It handles
Push Hook,Tag Push Hook, andMerge Request Hookevents, adds permission-gated deployments for MR authors, and exposes the webhook secret in the GitLab provider UI.The implementation is generally well-structured and tests cover the key paths. Two issues were found:
apps/dokploy/pages/api/deploy/gitlab.ts): The guardif (app.previewDeployments.length > previewLimit)allows one extra deployment to be created when the count equals the limit exactly (e.g. 3 deployments withpreviewLimit = 3passes the check). The fix is to change>to>=.totalAppscount in Push Hook response: The response message is derived fromapps.length + composeApps.lengthbeforewatchPaths-based filtering, so it can report "Deployed N apps" when fewer were actually queued.edit-gitlab-provider.tsx:navigator.clipboard.writeTextreturns aPromisethat can reject in non-HTTPS or permission-denied contexts; the rejection is currently silently ignored and the success toast fires unconditionally.Confidence Score: 4/5
Safe to merge after fixing the preview-limit off-by-one bug, which allows one more preview deployment than configured.
One P1 bug (preview limit off-by-one) means users can exceed their configured deployment limit by one. The rest of the implementation — permission checking, teardown logic, migration, and MR note deduplication — is correct and well-tested.
apps/dokploy/pages/api/deploy/gitlab.ts — preview-limit guard needs
>=instead of>Important Files Changed
>should be>=), and a P2 inaccuracy in thetotalAppsresponse counter for push events whenwatchPathsfiltering skips some apps.response.okbefore JSON parse, deduplicates security notes via sentinel text, and correctly identifies Developer+ access (level >= 30).findGitlabByWebhookSecretservice function with clean return type inference (returnsnullwhen not found). Straightforward addition.webhook_secretcolumn: adds as nullable, backfills usingencode(gen_random_bytes(21), 'base64'), then sets NOT NULL.onequery. Safe:findGitlabByIdalready throws aNOT_FOUNDTRPCError for unknown IDs.copyToClipboardhelper doesn't handle Promise rejection fromnavigator.clipboard.writeText.webhookSecretcolumn withnotNull()constraint andnanoid()default. Matches the migration strategy for new rows.Comments Outside Diff (1)
apps/dokploy/components/dashboard/settings/git/gitlab/edit-gitlab-provider.tsx, line 826-834 (link)navigator.clipboard.writeTextnavigator.clipboard.writeTextreturns aPromisethat rejects when clipboard access is unavailable (non-HTTPS context, permission denied, etc.). The rejection is silently swallowed here —toast.successfires and the copied state is set even before the write resolves. Consider awaiting the call inside anasynchandler with acatchthat shows a toast error:Reviews (1): Last reviewed commit: "fix: address PR review issues in GitLab ..." | Re-trigger Greptile
(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!