fix(ci): repair the Coolify preview deployment and make previews opt-in - #1348
Conversation
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe preview Compose stack adopts Coolify service naming, isolates database and runtime volumes per deployment, initializes volume permissions, updates appserver/webapp dependencies, and revises shared webhook-server environment handling and documentation. ChangesCoolify preview deployment
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docker/preview/compose.app.yaml (1)
359-362: 🔒 Security & Privacy | 🔵 TrivialDocker-socket RW mount grants host-level control from the preview container.
Mounting
/var/run/docker.sockread-write intoappservergives that container the ability to spawn arbitrary privileged containers on the host — effectively root-equivalent access. The comment notes "prod does the same," so this isn't a new pattern introduced here, but previews are the environment most likely to run less-trusted/experimental code (PR branches), so it's worth confirming this tradeoff is intentional for preview too, not just inherited from prod without re-evaluation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/preview/compose.app.yaml` around lines 359 - 362, Review the appserver volume configuration and explicitly confirm whether the read-write /var/run/docker.sock mount is intentional for preview deployments. If Docker host control is not required, remove the mount or replace it with a least-privilege alternative; otherwise document and preserve the deliberate security tradeoff rather than inheriting it implicitly from production.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker/preview/compose.app.yaml`:
- Around line 351-358: Update WEBHOOK_EXTERNAL_URL in the preview compose
configuration to default to the shared-infra webhook-server host rather than the
undefined PREVIEW_DOMAIN-based preview origin, while preserving explicit
WEBHOOK_EXTERNAL_URL overrides.
---
Nitpick comments:
In `@docker/preview/compose.app.yaml`:
- Around line 359-362: Review the appserver volume configuration and explicitly
confirm whether the read-write /var/run/docker.sock mount is intentional for
preview deployments. If Docker host control is not required, remove the mount or
replace it with a least-privilege alternative; otherwise document and preserve
the deliberate security tradeoff rather than inheriting it implicitly from
production.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27932fb4-91ff-4d51-9f5b-4836c654c4ad
📒 Files selected for processing (2)
docker/preview/compose.app.yamldocker/preview/compose.shared-infra.yaml
| # Webhook auto-registration. Points at the shared-infra receiver's own host, not this | ||
| # deploy's origin: webhook-server lives in the shared-infra stack and is routed there. | ||
| # Bare origin — GitLabWebhookService appends /webhooks/gitlab itself. | ||
| WEBHOOK_SECRET: ${WEBHOOK_SECRET:-} | ||
| WEBHOOK_EXTERNAL_URL: https://${PREVIEW_DOMAIN:?Required} | ||
| WEBHOOK_EXTERNAL_URL: ${WEBHOOK_EXTERNAL_URL:-https://${PREVIEW_DOMAIN}} | ||
| # thc healthcheck target: actuator liveness on the management port. | ||
| THC_PORT: "8080" | ||
| THC_PATH: /actuator/health/liveness |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant compose files and search for the variable names involved.
git ls-files 'docker/preview/*.yaml' 'docker/preview/*.yml' | sort
printf '\n--- SEARCH ---\n'
rg -n '\b(PREVIEW_DOMAIN|WEBHOOK_EXTERNAL_URL|SERVICE_FQDN_|SERVICE_NAME_)' docker/preview -SRepository: ls1intum/Hephaestus
Length of output: 2937
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- PREVIEW_DOMAIN SEARCH ---'
rg -n '\bPREVIEW_DOMAIN\b' docker/preview -S
printf '\n%s\n' '--- SHARED-INFRA COMPOSE OUTLINE ---'
ast-grep outline docker/preview/compose.shared-infra.yaml --view expanded || true
printf '\n%s\n' '--- APP COMPOSE AROUND WEBHOOK LINES ---'
sed -n '330,365p' docker/preview/compose.app.yaml
printf '\n%s\n' '--- SHARED-INFRA AROUND WEBHOOK/FQDN LINES ---'
sed -n '1,220p' docker/preview/compose.shared-infra.yamlRepository: ls1intum/Hephaestus
Length of output: 10174
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '\bWEBHOOK_(EXTERNAL_URL|SERVER|HOST|URL)|webhooks\.' docker/preview README.md -SRepository: ls1intum/Hephaestus
Length of output: 553
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- PREVIEW_* / DOMAIN SEARCH ---'
rg -n '\bPREVIEW_[A-Z0-9_]+\b|\bBASE_DOMAIN\b|\bWEBHOOK_EXTERNAL_URL\b|\bSERVICE_FQDN_[A-Z0-9_]+\b' . -S
printf '\n%s\n' '--- COMPOSE FILE LOCATIONS ---'
git ls-files 'docker/preview/*.yaml' 'docker/preview/*.yml' 'docker/preview/*.md' | sortRepository: ls1intum/Hephaestus
Length of output: 3620
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- docker/compose.core.yaml around WEBHOOK_EXTERNAL_URL ---'
sed -n '1,80p' docker/compose.core.yaml
printf '\n%s\n' '--- docker/compose.app.yaml around WEBHOOK_EXTERNAL_URL ---'
sed -n '140,175p' docker/compose.app.yaml
printf '\n%s\n' '--- server application.yml around webhook external-url ---'
sed -n '476,492p' server/src/main/resources/application.yml
printf '\n%s\n' '--- preview compose.app.yaml around APP_FQDN and webhook block ---'
sed -n '88,100p;348,358p' docker/preview/compose.app.yamlRepository: ls1intum/Hephaestus
Length of output: 10606
WEBHOOK_EXTERNAL_URL falls back to an undefined variable here
PREVIEW_DOMAIN isn’t defined anywhere in the preview compose files, so previews without an explicit override resolve this to an invalid URL and webhook auto-registration fails. Point this at the shared-infra webhook-server host instead of the preview origin.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker/preview/compose.app.yaml` around lines 351 - 358, Update
WEBHOOK_EXTERNAL_URL in the preview compose configuration to default to the
shared-infra webhook-server host rather than the undefined PREVIEW_DOMAIN-based
preview origin, while preserving explicit WEBHOOK_EXTERNAL_URL overrides.
b15e392 to
1f74782
Compare
FelixTJDietrich
changed the title
docker/preview/* was never updated for the native-auth cutover (#1317) or the Java webhook receiver (#1300, #1306), and had drifted far enough that no stack could boot. Previews are now opt-in: Coolify's all-or-nothing preview switch stays off, and the Preview / Coolify job refreshes only the previews that already exist. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1f74782 to
e061f99
Compare
📚 Documentation Preview
|
1 participant
Description
The Coolify preview deployment has been broken for months.
docker/preview/*was never updated for the native-auth cutover (#1317) or the Java webhook receiver (#1300, #1306), and had drifted far enough that no stack could boot — the app resource sat atexited:unhealthywhile shared infra still ran six-month-old Keycloak,webhook-ingestand postfix containers.This restores it end to end, and makes previews opt-in rather than automatic. Every item below is one independently fatal bug, found by deploying to
hephaestus-test.aet.cit.tum.deand reading the failures.Why nothing could boot
${VAR:?Required}became the literal stringRequired. Coolify seeds its own env store from these placeholders and keeps the error message as the value. The stack therefore ran withHEPHAESTUS_AUTH_STATE_COOKIE_KEY=Required, which base64-decodes to 6 bytes;AuthSecurityConfigrequires 32 and aborts the boot. All:?defaults are gone.HEPHAESTUS_SECURITY_ENCRYPTION_KEYwas never set.JwtSigningKeySealerfail-fasts without it in prod.webhook-serverhad no datasource.application-prod.ymlbindsspring.datasource.urltojdbc:${DATABASE_URL}with no default andapplication-webhook.ymlkeeps JPA active, so an unset value failed placeholder resolution before the context started.build: context: ./serverhas never been valid — the image comes from Paketo buildpacks (spring-boot:build-image), which is also what supplies the/workspace/health-checkbinary the healthchecks invoke. Every build died withfailed to read dockerfile. It is now pulled from GHCR.Why login could not have worked
application-prod.ymlpinshephaestus.auth.api-base-path=/api, which is correct for production, where Traefik strips that prefix. Nothing strips it here, so every OAuth callback was built ashttps://<api-host>/api/login/oauth2/code/<provider>and 404'd. The prefix is now cleared for this stack.Separately, the trusted-proxies regex reached the container double-escaped (Coolify re-escapes backslashes when it writes the stack
.env), so\.arrived as\\.— which as a Java regex matches a literal backslash and therefore never matches an IP. Tomcat silently distrusted Traefik, ignoredX-Forwarded-Proto, and built everyredirect_uriashttp://, which the IdP rejects. The value must be written without backslashes, and there is an inline comment saying so.Preview isolation
hostname: postgreslet previews reach each other's databases. Coolify attaches every stack to the sharedcoolifynetwork, where a fixed hostname made every preview's Postgres answer to the samepostgresname. Services are now addressed through the per-deploySERVICE_NAME_*aliases.sync.nats.serverandagent.nats.serverboth default to${NATS_SERVER}. They are now pinned separately, so pointing the integration consumer at another environment's NATS cannot also submit this stack's agent jobs into that environment'sAGENTqueue for its workers to execute.Volume ownership
Named volumes are created
root:rootbut the buildpack image runs ascnb(1002:1001), soGitRepositoryManagercould notmkdirunder/data/git-repos(every checkout failed) and Tomcat'sAccessLogValvecould not open/var/log/hephaestus/access— which was never mounted here at all. Avolume-initstep now hands both mounts to the runtime user. It has to live in the compose file: previews get fresh root-owned volumes on every deploy, so a one-offchownwould silently regress.Routing
Coolify generates the Traefik routers itself from the per-service domains, so the stack is served as:
https://<domain>https://api.<domain>https://webhooks.<domain>/webhooks/{github,gitlab,slack}Two constraints are worth recording, because both silently produce a working-looking but wrong deployment:
${VARS}insidelabels:, so the rule reaches Traefik with a literal${APP_HOSTNAME}host and never matches./webhooks/{kind}path, so a stripped/webhooks404s every delivery; a path-less domain is never stripped.The API service is named
appserver, notapplication-server, and the name is load-bearing — see the inline comment.Preview lifecycle: opt in, then stay current
Coolify's preview switch is all-or-nothing: enabled, it deploys every opened PR. We don't want a stack per PR, but we do want the previews that exist to stay current. So the switch is now off, a preview is created from the Coolify UI when a PR actually needs one, and the
Preview / Coolifyjob refreshes it on each push. Coolify's deploy API only accepts a pull request that already has a preview, so a PR without one is a no-op:The job is configured entirely through repository variables (
COOLIFY_URL,COOLIFY_APP_UUID,COOLIFY_PROJECT_UUID,COOLIFY_ENVIRONMENT_UUID) plus aCOOLIFY_API_TOKENsecret scoped to Coolify'sdeployability. No instance identifiers are hardcoded, so a fork — or anyone running their own Coolify — skips the job cleanly instead of posting links into someone else's instance. Fork PRs are skipped by design: a preview runs with the instance's real credentials.How to test
Deployed and verified on
hephaestus-test.aet.cit.tum.de(Coolify): 5/5 containers healthy, 0ERRORlines in the app log.GitHub and GitLab (
gitlab.lrz.de, scoperead_user) both resolve to the correct authorize URL, and Liquibase migrates cleanly against the existing database. ThePreview / Coolifyjob on this PR exercises the no-preview path shown above and passes.Notes for reviewers
fix(ci), notfix(docker): nothing in the shipped application changed — only preview deployment config and the preview job — anddockeris a release-triggering scope in.releaserc, so it would cut a patch release and publish new production images for an unchanged app. (Separately: the PR template's header comment listsdockerunder "NO RELEASE" while.releasercdoes not. The template is wrong; follow-up.)AgentConfigis seeded, but exercising the mentor needs a real workspace and PR.IMAGE_TAG=pr-<id>to exercise its server too (that tag exists only when the PR touchedserver/).1772284124265-7doesRESTART WITH (MAX(id) + 1), which Postgres rejects when the max id is negative. GitLab-synced milestones carry hashed negative ids, so this fails on any database that has them (it did on the test box).GREATEST(…, 1)would make it safe, but the changeset is very likely already applied on staging and prod, and editing it would break their Liquibase checksums — so it needs its own follow-up.