mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
Caddy logs a site only when its container labels contain the log directives. Deployments reuse the stored custom_labels, so applications whose labels were stored before analytics was enabled never got the log directives, and a redeploy did not help. Toggling traffic analytics on a Caddy server now regenerates the stored labels of applications with read-only labels. The destination uses the current server instance, because the cached server can still have the old analytics setting. Fixes #12117 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
12 KiB
12 KiB
Lessons
Prove regressions before changing code
- Reproduce the reported failure on the unchanged baseline before adding a fix.
- When a symptom matches an earlier fix, inspect that fix and prove why it no longer works before adding another workaround.
- Test old reports against the current branch because later changes can make the report obsolete.
- Use the same regression test before and after the production change so the result shows the behavior difference.
- Several dev instances can run from one checkout (see
./scripts/dev urls). After you add a migration, run it on each running instance of that checkout, or those instances fail on the new code. - The checkout is shared with other sessions. When the full suite fails, rerun each failing file alone and compare with a clean
git archive HEADcopy before you connect a failure to your change. - Run seeders and other artisan commands that write SSH keys in a dev container as
www-data(docker exec -u www-data). Root-owned files instorage/app/sshmake every SSH call from the web process fail. sshdkeeps the login shell of an open multiplexed SSH connection. CallSshMultiplexingHelper::removeMuxFile()before a live test of SSH or login-shell behavior.- SSH retries replay the full command batch. Only exit code 255 is an SSH transport failure; other exit codes come from the remote command, so do not retry them. Keep batches safe to run twice (for example
docker rm -f <name> || truebeforedocker run --name <name>). - Redirect browser test output to a file (
> /tmp/x.log 2>&1); piping it (| tail) hangs because the Playwright server keeps the pipe open. - Browser
click('Text')can match a hidden element first (for example inactive modal steps) and wait forever; usebutton:visible:has-text("Text"). - Never run
php artisan testinside a dev container (./scripts/dev execordocker exec coolify-dev-*). The container setsDB_CONNECTION=pgsqlas a real environment variable, which wins overphpunit.xml, soRefreshDatabaserunsmigrate:freshon the dev database and wipes it. Run tests on the host only. - Call
visit()directly in eachtests/v4/Browsertest body; Pest does not mark a test that only uses helper-wrappedvisit()as a browser test, so it fails withsendText() on null.
Verify the complete user flow
- Do not use a passing unit test, a successful build, or a healthy process as proof for a reported UI failure.
- Verify the exact live flow, persisted state, relevant logs, and queue state when they affect the result.
- When the request covers more than one interface or resource type, inventory and verify each supported path.
Preserve product scope
- Do not replace required SPA navigation with a full-page redirect to hide a lifecycle or ordering defect.
- Do not add billing restrictions, live reconciliation, or fallback behavior unless the request includes them.
- Treat implementation constraints as details. Do not expand a requested team-level control into a more complex policy model.
Test behavior, not source markup
- Do not write tests that read Blade or CSS files as text to assert layout, classes, or copy; they break on every redesign and miss real failures.
- Render the component (
Livewire::test(), an HTTP request) and assert the result, including authorization; use a fewtests/v4/Browsertests with screenshots for visual flows. - A source-text check is acceptable only for a correctness or security rule that rendering cannot prove easily, such as stable
wire:keyvalues or no raw user output. - Call the app code under test. Do not copy app logic into a test, assert only literals, constants,
class_exists/method_exists, or PHP built-ins such asescapeshellarg. - Pest test files share one global function scope. Give file-level helper functions a unique name.
- Call
Server::flushIdentityMap()inbeforeEachof tests that useServermodels; the identity map leaks servers and settings between test files in one run.
Keep dynamic Livewire identities stable
- In dynamic lists, key components and actions with immutable record identities, not counts, indexes, or array positions.
- Use targeted refresh events. Do not refresh a parent and a child that the parent can remove or hide during the same operation.
- Prove lifecycle and redirect causes directly; an effects assertion alone is not sufficient.
Keep modal structure consistent
- Identify the parent page that owns a modal trigger and move the complete requested workflow into that modal.
- Use a flat form layout when the modal already supplies its title and description.
- Put destructive actions on the footer's left and primary actions last on the right.
- Use shared section, helper, tooltip, and icon-button components instead of local variants.
- Keep validation, preview, and save controls in a fixed footer when the body is large.
x-modal-confirmationparsessubmitActionwithresources/js/modal-confirmation.js: quoted arguments lose their quotes, unquoted ones arrive as text. Quote text that can contain commas or parentheses.
Verify layered UI behavior visually
- Inspect the real layout with all conditional elements visible, especially compound status badges.
- For animation flicker, inspect state timing, loading indicators, keyframe fill mode, and focus restoration.
- Add
fill-mode-forwardsto Alpine leave transitions that use tw-animate-cssanimate-outso the element does not flash before Alpine hides it.
Preserve inherited values and clear API semantics
- An unchanged displayed default must remain inherited; store an override only when the user selects a different value.
- Expose named API values for special modes. Keep existing numeric sentinels only as compatibility aliases unless a breaking change is requested.
Trace infrastructure changes end to end
- For container image changes, inspect Compose services and every relevant Dockerfile build stage.
- Pin a stable release tag instead of using a floating
latesttag. - Never give a test build an official image tag (for example
coollabsio/sentinel:<version>) in the host Docker. Dev instances that usetesting-hostshare the host Docker socket and run that tag without a pull. Load such a tag only inside a dev VM, and give the build the same version string, or Coolify's Sentinel check restarts it every minute. - A successful image pull does not prove that the complete application build no longer uses the old image.
- Do not use
docker compose up --waitfor a stack with one-shot services; wait for the required long-running service's health instead.
Make distributed schedules durable
- Use the database as the correctness source for dynamic cron occurrences shared by multiple scheduler and Horizon nodes; Redis locks are load controls, not a durable execution ledger.
- Give each schedule occurrence a unique database identity and make queue consumers claim it atomically before external work.
- Run a per-minute dispatcher from the scheduler process, not as a queued job, so queue backlog cannot delay it.
- A Redis instance that holds queues must use
maxmemory-policy noeviction; an evicting policy deletes queued jobs without an error. Before changing scheduler code for missed runs, checkINFO statsevicted_keys. - Keep pending occurrences recoverable across publisher interruptions, and define an explicit bounded policy for late or offline schedules.
- Horizon workers are long-lived: flush every static or
once()cache (for exampleServer::flushIdentityMap()) inQueue::before, or later jobs decide with stale state. - A job of a killed worker comes back after
retry_after(one day) as attempt 2. Withtries > 1it runs again a day late; queued jobs that must not run late needtries = 1or an attempt guard, andfailed()must not guess its execution row from "the latest" record.
Fail closed at public webhook boundaries
- Reject missing or blank secrets before signature verification, and return generic errors without logging secrets, signatures, or payloads.
Pass identities to Livewire actions
- Pass record IDs to Livewire actions instead of display values, and resolve team-scoped records on the server. When JavaScript needs text, use
@js()orJs::from(). - Mark Livewire properties that select records or feed server-side lookups as
#[Locked]; clients can change every other public property.
Keep host test runs away from the dev app cache
- The repository is bind-mounted into the dev
coolifycontainer. Tests that callapp:initrunoptimizeand write a testing config/route cache intobootstrap/cache, so the dev app returns 500. For broad host test runs, setAPP_CONFIG_CACHE,APP_ROUTES_CACHE,APP_EVENTS_CACHE,APP_SERVICES_CACHE, andAPP_PACKAGES_CACHEto a temporary directory.
Test the real runtime image
- Deployment shell commands run in the Alpine/BusyBox helper image and pass through the non-root sudo parser. Verify new flags and shell syntax in that image and with
parseCommandsByLineForSudo(); faked command output hides both failures. - Put multi-step remote shell logic in one
sh -c '<script>' sh <args>line. The non-root parser then only puts sudo in front of it; it rewritesx=$(...),&&,|and shell keywords in any other line. Serverhas an identity map. Tests that create servers in several dataset cases withRefreshDatabasemust callServer::flushIdentityMap()inbeforeEach/afterEach, or a case reads the cached server of the previous case.- Host test runs share
storage/with the dev container. Fake thessh-keys/ssh-muxdisks andProcessin tests that run seeders or write SSH files, or the test deletes/chowns the dev instance's SSH keys and SSH breaks forwww-data. - Dev QEMU servers from
dev:qemuare seeded, not validated: they have nocoolifyDocker network, and Alpine has no bash untilInstallPrerequisitesruns.
Format only your own files
pint --dirtyalso rewrites uncommitted files that belong to other work in the tree. When the tree has unrelated changes, pass your changed paths to Pint.
Match containers by UUID, never by numeric id
- Container ownership labels are
coolify.applicationUuid,coolify.serviceUuid,coolify.service.subUuid, andcoolify.databaseUuid. Numeric ids change when a server moves to another instance. - Use
resolveContainerOwner(),resolveServiceContainerOwner(), andcontainersOwnedBy()/dockerPsByOwnerCommands()frombootstrap/helpers/docker.php. Order: UUID label, thencom.docker.compose.project(the UUID only since July 2024), then the local numeric id label of older containers. - Read owners from the flat label list.
Arr::undot()breakscom.docker.compose.projectbecausecom.docker.compose.project.config_filesis nested under it.
Non-root SSH users: keep file access behind sudo
parseCommandsByLineForSudo()does not prefixcdorecho, and the SSH user's shell opens redirects (>,<) and expands globs. On the Coolify host,/data/coolifyis9999:root 0700, so these fail for a non-root user.- In remote commands, use absolute paths (
docker compose -f <dir>/docker-compose.yml,--project-directory <dir>),echo ... | tee <file> > /dev/null, andfindinstead of globs. scp also runs as the SSH user; stage files outside/data/coolify. - The parsers add sudo only to line starts (and after
&&,||,|,$(). Putif/elsebranches on their own lines. When a redirect orcdmust stay, make the whole line onesh -c '...'script; it runs assudo sh -cwithout inner sudo or bash. - Tests that replace
DatabaseStartCommandExecutoror use root servers miss non-root bugs. Test the parsed commands of a non-root server.
Cached and stored state
StandaloneDocker/SwarmDockerserverusesServer::findCached(), a per-process identity map that is flushed only when theserversrow updates. AServerSettingsave does not flush it, so pass the currentServerinstance (setRelation('server', $server)) when generated output depends on just-saved settings.- Application deployments use the stored
custom_labelswhen they exist. When a server-level setting changes the generated labels, regenerate the stored labels of read-only (is_container_label_readonly_enabled) applications, or a redeploy keeps the old labels.