diff --git a/.ddev/addon-metadata/redis/manifest.yaml b/.ddev/addon-metadata/redis/manifest.yaml new file mode 100644 index 00000000000..c0fcfe56158 --- /dev/null +++ b/.ddev/addon-metadata/redis/manifest.yaml @@ -0,0 +1,38 @@ +name: redis +repository: ddev/ddev-redis +version: v2.2.0 +install_date: "2026-09-08T02:37:13Z" +project_files: + - docker-compose.redis.yaml + - redis/scripts/settings.ddev.redis.php + - redis/scripts/setup-drupal-settings.sh + - redis/scripts/setup-redis-optimized-config.sh + - redis/redis.conf + - redis/advanced.conf + - redis/append.conf + - redis/general.conf + - redis/io.conf + - redis/memory.conf + - redis/network.conf + - redis/security.conf + - redis/snapshots.conf + - commands/host/redis-backend + - commands/redis/redis-cli + - commands/redis/redis-flush +global_files: [] +removal_actions: + - | + #ddev-description:Remove redis settings if applicable + files=( + "${DDEV_APPROOT}/${DDEV_DOCROOT}/sites/default/settings.ddev.redis.php" + "${DDEV_APPROOT}/.ddev/docker-compose.redis_extra.yaml" + ) + for file in "${files[@]}"; do + if [ -f "$file" ]; then + if grep -q '#ddev-generated' "$file"; then + rm -f "$file" + else + echo "Unwilling to remove '$file' because it does not have #ddev-generated in it; you can manually delete it if it is safe to delete." + fi + fi + done diff --git a/.ddev/commands/host/redis-backend b/.ddev/commands/host/redis-backend new file mode 100755 index 00000000000..fbcaba553b8 --- /dev/null +++ b/.ddev/commands/host/redis-backend @@ -0,0 +1,124 @@ +#!/usr/bin/env bash +#ddev-generated + +## Description: Use a different key-value store for Redis +## Usage: redis-backend [optimize] +## Example: ddev redis-backend redis-alpine optimize + +REDIS_DOCKER_IMAGE=${1:-} +REDIS_CONFIG=${2:-} +NAME=$REDIS_DOCKER_IMAGE + +function show_help() { + cat < [optimize] + +Choose from predefined aliases, or provide any Redis-compatible Docker image. +Note that not every Docker image can work right away, and you may need to override +the "command:" in the docker-compose.redis_extra.yaml file + +Available aliases: + redis redis:7 + redis-alpine redis:7-alpine + valkey valkey/valkey:8 + valkey-alpine valkey/valkey:8-alpine + +Custom backend: + You can specify any Docker image, e.g.: + ddev redis-backend redis:6 + +Optional: + optimize Apply additional Redis configuration with resource limits + optimized Same as optimize + +Examples: + ddev redis-backend redis-alpine optimize + ddev redis-backend valkey + ddev redis-backend redis:7.2-alpine +EOF + exit 0 +} + +function optimize_config() { + [[ "$REDIS_CONFIG" != "optimized" && "$REDIS_CONFIG" != "optimize" ]] && return + ddev dotenv set .ddev/.env.redis --redis-optimized=true +} + +function change_hostname() { + [[ "${REDIS_HOSTNAME:-}" == "" ]] && return + ddev dotenv set .ddev/.env.redis --redis-hostname="$REDIS_HOSTNAME" +} + +function cleanup() { + rm -f "$DDEV_APPROOT/.ddev/.env.redis" + rm -rf "$DDEV_APPROOT/.ddev/redis/" + rm -f "$DDEV_APPROOT/.ddev/docker-compose.redis.yaml" "$DDEV_APPROOT/.ddev/docker-compose.redis_extra.yaml" + + redis_volume="ddev-$(ddev status -j | docker run -i --rm ddev/ddev-utilities jq -r '.raw.name')_redis" + if docker volume ls -q | grep -qw "$redis_volume"; then + ddev stop + docker volume rm "$redis_volume" + fi +} + +function check_docker_image() { + echo "Pulling ${REDIS_DOCKER_IMAGE}..." + if ! docker pull "$REDIS_DOCKER_IMAGE"; then + echo >&2 "❌ Unable to pull ${REDIS_DOCKER_IMAGE}" + exit 2 + fi +} + +function use_docker_image() { + [[ "$REDIS_DOCKER_IMAGE" != "redis:7" ]] && ddev dotenv set .ddev/.env.redis --redis-docker-image="$REDIS_DOCKER_IMAGE" + REPO=$(ddev add-on list --installed -j 2>/dev/null | docker run -i --rm ddev/ddev-utilities jq -r '.raw[] | select(.Name=="redis") | .Repository // empty' 2>/dev/null) + ddev add-on get "${REPO:-ddev/ddev-redis}" +} + +case "$REDIS_DOCKER_IMAGE" in + redis) + NAME="Redis 7" + REDIS_DOCKER_IMAGE="redis:7" + ;; + redis-alpine) + NAME="Redis 7 Alpine" + REDIS_DOCKER_IMAGE="redis:7-alpine" + ;; + valkey) + NAME="Valkey 8" + REDIS_DOCKER_IMAGE="valkey/valkey:8" + REDIS_HOSTNAME="valkey" + ;; + valkey-alpine) + NAME="Valkey 8 Alpine" + REDIS_DOCKER_IMAGE="valkey/valkey:8-alpine" + REDIS_HOSTNAME="valkey" + ;; + ""|--help|-h) + show_help + ;; + *) + NAME="$REDIS_DOCKER_IMAGE" + # Allow unknown image, nothing to override + ;; +esac + +check_docker_image +cleanup +optimize_config +change_hostname +use_docker_image + +echo +echo "✅ Redis backend: $REDIS_DOCKER_IMAGE" +if [[ "$REDIS_CONFIG" == "optimized" || "$REDIS_CONFIG" == "optimize" ]]; then + echo "⚙️ Redis config: optimized" +else + echo "⚙️ Redis config: default" +fi + +echo +echo "📝 Commit the '.ddev' directory to version control" + +echo +echo "🔄 Redis config available after 'ddev restart'" diff --git a/.ddev/commands/redis/redis-cli b/.ddev/commands/redis/redis-cli new file mode 100755 index 00000000000..2800343ed53 --- /dev/null +++ b/.ddev/commands/redis/redis-cli @@ -0,0 +1,13 @@ +#!/usr/bin/env sh + +#ddev-generated +## Description: Run redis-cli inside the Redis container +## Usage: redis-cli [flags] [args] +## Example: "ddev redis-cli KEYS *" or "ddev redis-cli INFO" or "ddev redis-cli --version" +## Aliases: redis + +if [ -f /etc/redis/conf/security.conf ]; then + redis-cli -p 6379 -h "${REDIS_HOSTNAME:-redis}" -a redis --no-auth-warning $@ +else + redis-cli -p 6379 -h "${REDIS_HOSTNAME:-redis}" $@ +fi diff --git a/.ddev/commands/redis/redis-flush b/.ddev/commands/redis/redis-flush new file mode 100755 index 00000000000..db90558a7f2 --- /dev/null +++ b/.ddev/commands/redis/redis-flush @@ -0,0 +1,12 @@ +#!/usr/bin/env sh + +#ddev-generated +## Description: Flush all cache inside the Redis container +## Usage: redis-flush +## Example: "ddev redis-flush" + +if [ -f /etc/redis/conf/security.conf ]; then + redis-cli -p 6379 -h "${REDIS_HOSTNAME:-redis}" -a redis --no-auth-warning FLUSHALL ASYNC +else + redis-cli -p 6379 -h "${REDIS_HOSTNAME:-redis}" FLUSHALL ASYNC +fi diff --git a/.ddev/config.yaml b/.ddev/config.yaml new file mode 100644 index 00000000000..3b60bd2022e --- /dev/null +++ b/.ddev/config.yaml @@ -0,0 +1,69 @@ +name: convoy4x +type: laravel +docroot: public +php_version: "8.2" +webserver_type: nginx-fpm + +# 4.x ships MySQL (docker-compose.yml pins mysql:8.0), unlike 5.x's Postgres. +database: + type: mysql + version: "8.0" + +# Without this DDEV assigns a random ephemeral host port on every start, which +# breaks saved connections in GUI clients (TablePlus, DataGrip, mysql aliases). +host_db_port: "3306" + +# The release workflow builds on Node 20. +nodejs_version: "20" +corepack_enable: false + +# ext-gmp is required by composer.json. Without it `composer install` fails its +# platform check -- and `ddev composer` still exits 0, so vendor/ silently stays +# stale and every artisan call dies on the next platform_check.php. +webimage_extra_packages: + - php8.2-gmp + +# MySQL + redis + mail overrides. These are real container env vars, so they +# take precedence over .env (Laravel's Dotenv does not overwrite existing env). +# Note CACHE_DRIVER, not 5.x's CACHE_STORE: this branch is on Laravel 11 but +# still ships the pre-11 config/cache.php, which reads the old key. +web_environment: + - APP_URL=https://convoy4x.ddev.site + - DB_CONNECTION=mysql + - DB_HOST=db + - DB_PORT=3306 + - DB_DATABASE=db + - DB_USERNAME=db + - DB_PASSWORD=db + - REDIS_HOST=redis + - REDIS_PORT=6379 + - REDIS_PASSWORD= + - CACHE_DRIVER=redis + - QUEUE_CONNECTION=redis + - SESSION_DRIVER=redis + - MAIL_MAILER=smtp + - MAIL_HOST=localhost + - MAIL_PORT=1025 + +# Deliberately no `db_test` database, unlike 5.x. This branch's tests/Pest.php +# uses DatabaseTransactions rather than RefreshDatabase, so they run against +# whatever is in the dev database and roll their own writes back. That means +# seeded dev data is visible to the suite and will fail any test asserting on a +# fleet-wide aggregate (the overview endpoint's counts, most of all). Run +# `ddev artisan migrate:fresh` before a full run, or scope to one file. + +# Replaces the compose `workers` service and its scheduler. +web_extra_daemons: + - name: horizon + command: "php artisan horizon" + directory: /var/www/html + - name: scheduler + command: "php artisan schedule:work" + directory: /var/www/html + +# Expose the Vite dev server, which vite.config.js pins to 1234. +web_extra_exposed_ports: + - name: vite + container_port: 1234 + http_port: 1235 + https_port: 1234 diff --git a/.ddev/docker-compose.redis.yaml b/.ddev/docker-compose.redis.yaml new file mode 100644 index 00000000000..e9da4c6e48a --- /dev/null +++ b/.ddev/docker-compose.redis.yaml @@ -0,0 +1,27 @@ +#ddev-generated +services: + redis: + container_name: ddev-${DDEV_SITENAME}-redis + image: ${REDIS_DOCKER_IMAGE:-redis:7} + hostname: ${REDIS_HOSTNAME:-redis} + # These labels ensure this service is discoverable by ddev. + labels: + com.ddev.site-name: ${DDEV_SITENAME} + com.ddev.approot: ${DDEV_APPROOT} + restart: "no" + expose: + - 6379 + volumes: + - ".:/mnt/ddev_config" + - "ddev-global-cache:/mnt/ddev-global-cache" + - "./redis:/etc/redis/conf" + - "redis:/data" + command: /etc/redis/conf/redis.conf + x-ddev: + describe-url-port: | + Backend: ${REDIS_DOCKER_IMAGE:-redis:7} + describe-info: | + Pass: + +volumes: + redis: diff --git a/.ddev/redis/redis.conf b/.ddev/redis/redis.conf new file mode 100644 index 00000000000..937c4e5d34e --- /dev/null +++ b/.ddev/redis/redis.conf @@ -0,0 +1,13 @@ +# Redis configuration. +# #ddev-generated +# Example configuration files for reference: +# http://download.redis.io/redis-stable/redis.conf +# http://download.redis.io/redis-stable/sentinel.conf + +maxmemory 2048mb +maxmemory-policy allkeys-lfu + +# to disable Redis persistence, remove ddev-generated from this file, +# and uncomment the two lines below: +#appendonly no +#save "" diff --git a/.sbx/README.md b/.sbx/README.md new file mode 100644 index 00000000000..767a9d1323f --- /dev/null +++ b/.sbx/README.md @@ -0,0 +1,65 @@ +# sbx kits + +Provisioning for running a coding agent in a [Docker Sandbox](https://docs.docker.com/ai/sandboxes/) +(`sbx`) against this repo. sbx has no auto-detection for repo-local kits, so a kit +is just a committed directory you reference explicitly with `--kit`. + +## `dev/` — Convoy 4.x dev environment + +Installs ddev and starts the stack inside the sandbox (its Docker daemon, DB, and +volumes are isolated from your host ddev). It also sets up headless browsing: +Playwright is pinned and installed to `/opt/sbx-e2e` (never the repo), and +`dev/browser.mjs` is published to `/opt/sbx-e2e/browser.mjs` for scripts to import. +Because `/etc/hosts` is a read-only mount in a sandbox — which otherwise makes +`ddev start` fail outright — a startup step overmounts a writable copy so ddev can +register `*.ddev.site`. + +```sh +sbx run --kit .sbx/dev claude +``` + +The first run installs ddev and pulls its images (slow, once). To make later +starts instant, snapshot the provisioned sandbox into a template: + +```sh +sbx template save convoy4x-dev +sbx run -t convoy4x-dev --kit .sbx/dev claude # install is now a no-op; only `ddev start` runs +``` + +Then finish app provisioning inside the sandbox (see the kit's `agentContext`, or +`.sbx/dev/spec.yaml`): + +```sh +ddev composer install +npm install +ddev artisan key:generate +ddev artisan migrate +ddev artisan db:seed --class=ServerSeeder +``` + +## Differences from the 5.x kit + +The two branches run different stacks, so the kits are not interchangeable — the +project provisioning differs even though the sandbox plumbing is identical. + +- MySQL 8.0 and PHP 8.2 here, Postgres 17 and PHP 8.4 on 5.x. +- The ddev project is `convoy4x`, so the app is at `https://convoy4x.ddev.site`. + That one has to differ: a 4.x and a 5.x sandbox otherwise fight over the same + ddev project name. The kit itself is still `convoy-dev` on both branches — + kits are referenced by path (`--kit .sbx/dev`), so there is nothing to collide. + The suggested *template* name is scoped, though, since the saved image bakes in + this branch's PHP and database versions and would otherwise clobber 5.x's. +- No separate test database. `tests/Pest.php` uses `DatabaseTransactions` rather + than `RefreshDatabase`, so the suite runs against the dev database and rolls its + own writes back — seeded dev data is visible to it and will fail any test that + asserts on a fleet-wide aggregate. `ddev artisan migrate:fresh` before a full run. +- Seeding is `ServerSeeder` (a location, a node, ten servers), and it needs no + Proxmox credentials. + +## Boundaries + +- **Secrets** live in the gitignored `.env`, mounted into the sandbox — never in a + kit. +- **Notifications** and any personal network setup come from global kits in the + operator's own dotfiles, injected automatically by their `sbx` wrapper; `.sbx/dev` + is project provisioning only. Multiple `--kit` refs compose, so they layer cleanly. diff --git a/.sbx/dev/browser.mjs b/.sbx/dev/browser.mjs new file mode 100644 index 00000000000..77bc4130a61 --- /dev/null +++ b/.sbx/dev/browser.mjs @@ -0,0 +1,117 @@ +/* + * Playwright helpers for driving the sandbox's own ddev app. + * + * The dev kit copies this to /opt/sbx-e2e/browser.mjs on every start, next to a + * pinned `playwright` install. Import it by absolute path from a throwaway + * script anywhere (e.g. your scratchpad) — resolving `playwright` relative to + * /opt/sbx-e2e means the repo never needs a devDependency for a local probe: + * + * import { BASE, launch, newContext, login, capture } from '/opt/sbx-e2e/browser.mjs' + * + * const browser = await launch() + * const ctx = await newContext(browser) + * const page = await login(ctx, { email: '…', password: '…' }) + * await capture(ctx, { url: '/admin', width: 768, path: '/tmp/overview.png' }) + * await browser.close() + */ +import { chromium } from 'playwright' +import { execFileSync } from 'node:child_process' + +export const BASE = process.env.SBX_APP_URL ?? 'https://convoy4x.ddev.site' + +const PROXY = process.env.HTTPS_PROXY ?? 'http://gateway.docker.internal:3128' + +/* + * Chromium hands hostnames to the proxy rather than resolving them, and the + * sandbox proxy resolves them on the HOST — so an unbypassed request to + * convoy4x.ddev.site drives your real host app (leaked sessions, mutated data). + * Bypassing the proxy for the app keeps it on this sandbox's loopback; the + * preflight below is the backstop that refuses to run if it ever slips. + */ +const BYPASS = '.ddev.site,localhost,127.0.0.1' + +export function assertSandboxApp(base = BASE) { + const ip = execFileSync('curl', ['-sk', `${base}/up`, '-o', '/dev/null', '-w', '%{remote_ip}']) + .toString() + .trim() + + if (ip !== '127.0.0.1') { + throw new Error( + `refusing to drive ${base}: it answered from ${ip || '(unreachable)'}, not this ` + + `sandbox's ddev (127.0.0.1). Check 'ddev describe', that .ddev.site is in ` + + `NO_PROXY, and that /etc/hosts is the writable overmount the dev kit sets up.` + ) + } +} + +export async function launch(options = {}) { + assertSandboxApp() + + return chromium.launch({ proxy: { server: PROXY, bypass: BYPASS }, ...options }) +} + +// The ddev cert is mkcert-signed and that CA isn't in the sandbox trust store. +export async function newContext(browser, options = {}) { + return browser.newContext({ + ignoreHTTPSErrors: true, + viewport: { width: 1440, height: 1000 }, + deviceScaleFactor: 2, + ...options, + }) +} + +export async function login(context, { email, password } = {}) { + const page = await context.newPage() + + await page.goto(`${BASE}/auth/login`, { waitUntil: 'domcontentloaded' }) + await page.getByLabel(/email/i).fill(email ?? process.env.SBX_APP_EMAIL) + await page.getByLabel(/password/i).fill(password ?? process.env.SBX_APP_PASSWORD) + await page.getByRole('button', { name: /sign in|log in|login/i }).click() + // `waitUntil: 'commit'` matters here. The default is 'load', and this app + // leaves /auth/login by a client-side react-router redirect that fires no + // second load event -- so the default hangs for the full timeout on a login + // that actually succeeded. 'commit' resolves as soon as the URL changes, + // which is the only thing this predicate is asking about. + await page.waitForURL(u => !u.pathname.includes('/auth/login'), { + timeout: 30_000, + waitUntil: 'commit', + }) + + return page +} + +/* + * Screenshot one route at one viewport. Returns the horizontal overflow in px, + * which is the layout failure mode worth failing a visual check on. + */ +export async function capture(context, { url, path, width = 1440, height = 1000, settle = 1000 }) { + const page = await context.newPage() + + try { + await page.setViewportSize({ width, height }) + await page.goto(BASE + url, { waitUntil: 'networkidle' }) + await page.waitForTimeout(settle) + await page.screenshot({ path, fullPage: true }) + + // Awaited, not returned directly: `return ` inside a try lets + // the finally close the page while evaluate is still in flight, and the + // call dies with "Target page, context or browser has been closed". + const overflow = await page.evaluate( + () => document.documentElement.scrollWidth - document.documentElement.clientWidth + ) + + return overflow + } finally { + await page.close() + } +} + +// Attach before navigating; the returned array fills as the page misbehaves. +export function collectErrors(page) { + const errors = [] + + page.on('pageerror', e => errors.push(`pageerror: ${e.message}`)) + page.on('console', m => m.type() === 'error' && errors.push(`console: ${m.text()}`)) + + return errors +} diff --git a/.sbx/dev/spec.yaml b/.sbx/dev/spec.yaml new file mode 100644 index 00000000000..fe1b127b6ae --- /dev/null +++ b/.sbx/dev/spec.yaml @@ -0,0 +1,196 @@ +schemaVersion: "1" +kind: mixin +name: convoy-dev +displayName: Convoy 4.x dev sandbox +description: Provision ddev inside a Docker Sandbox for working on Convoy 4.x. + +# Appended to the agent's memory at sandbox creation — tells the agent how to +# finish provisioning and run common tasks. (ddev itself is installed/started by +# the commands below; these are the project-state steps that shouldn't run +# automatically.) +agentContext: | + # Convoy 4.x dev sandbox + + ddev is installed and started for you (Laravel + MySQL + redis). Its **database** + is this sandbox's own, and the install step below proxy-isolates `*.ddev.site` + (adds it to NO_PROXY) so `curl`/Playwright hitting https://convoy4x.ddev.site + stay on THIS sandbox's ddev instead of being resolved by the proxy on the host + and driving your real host app. Sanity check after a rebuild: + `curl -sk https://convoy4x.ddev.site/ -o /dev/null -w '%{remote_ip}\n'` must + print `127.0.0.1` (not a proxy address). Back it up host-side with + `sbx policy deny network '*.ddev.site'`. + + Playwright + Chromium are pre-installed for visual and e2e checks — but NOT in + the repo. They live in `/opt/sbx-e2e` (pinned version, browsers in + ~/.cache/ms-playwright), so **do not `npm install playwright` in the project**; + that would put sandbox-only tooling in package.json. Write throwaway scripts + wherever you like and import the helpers by absolute path: + + import { BASE, launch, newContext, login, capture } from '/opt/sbx-e2e/browser.mjs' + + const browser = await launch() // proxy-bypassed + preflighted + const ctx = await newContext(browser) // ignores the mkcert cert + const page = await login(ctx, { email: '…', password: '…' }) + await capture(ctx, { url: '/admin', width: 768, path: '/tmp/overview.png' }) + await browser.close() + + `launch()` refuses to run unless the app answers from 127.0.0.1, so a + misconfigured sandbox fails loudly instead of driving your host app. Source: + `.sbx/dev/browser.mjs` (copied into place on every start — edit it there). + + If ddev is unreachable, fix ddev; never tunnel to the host's instance. + + Finish provisioning once: + + ddev composer install + npm install + ddev artisan key:generate + ddev artisan migrate + ddev artisan db:seed --class=ServerSeeder # a location, a node, 10 servers + + Everyday: + + ddev exec ./vendor/bin/pest # test suite + ddev artisan # artisan + ddev describe # status / URLs + + The app serves **prebuilt** assets from `public/build`, and no Vite dev server + runs by default — so a frontend edit is invisible in the browser until you + `npx vite build`. A stale `public/hot` from a killed `npm run dev` makes the + page render blank; `rm -f public/hot` before a built-asset check. + + Two traps specific to this branch: + + - `composer.json` requires **ext-gmp**. `ddev composer install` exits 0 even + when the platform check fails, so a missing extension leaves vendor/ stale + and the next artisan call dies in `platform_check.php`. The committed + `.ddev/config.yaml` installs php8.2-gmp; don't drop it. + - `tests/Pest.php` uses **DatabaseTransactions**, not RefreshDatabase, so the + suite runs against the dev database and merely rolls its own writes back. + Seeded dev data is therefore visible to it and will fail anything asserting + on a fleet-wide aggregate (the overview endpoint's counts, most of all). Run + `ddev artisan migrate:fresh` before a full run, or scope to one file. + + Note: vendor/ and node_modules/ are shared with your host repo via the mounted + workspace, so composer/npm installs here also land in your host checkout. + +commands: + # Runs ONCE at creation (as root unless a user is set). Slow the first time + # (installs ddev + Chromium). Bake it into a template so it isn't repeated: + # sbx template save convoy4x-dev + install: + # ddev's installer refuses to run as root and uses sudo itself, so run it as + # the agent user (uid 1000, which is in the sudo group). + - command: "command -v ddev >/dev/null 2>&1 || curl -fsSL https://ddev.com/install.sh | bash" + user: "1000" + description: install ddev + + # Keep *.ddev.site resolving to THIS sandbox's ddev (127.0.0.1) instead of the + # host's. The sandbox proxy otherwise resolves the hostname on the host side, so + # curl/Playwright silently drive your real host app (leaking e2e sessions, + # mutating host data). Marker-guarded because CLAUDE_ENV_FILE is sourced before + # every command — a plain append would grow NO_PROXY without bound. + - command: | + cat >> "${CLAUDE_ENV_FILE:-/etc/sandbox-persistent.sh}" <<'SBXEOF' + if [ -z "${SBX_DDEV_NOPROXY_DONE:-}" ]; then + export NO_PROXY="${NO_PROXY:+$NO_PROXY,}.ddev.site,ddev.site" + export no_proxy="$NO_PROXY" + export SBX_DDEV_NOPROXY_DONE=1 + fi + SBXEOF + description: proxy-isolate *.ddev.site from the host dev server + + # Playwright lives OUTSIDE the repo so a visual check never adds a devDependency + # to package.json. Scripts import /opt/sbx-e2e/browser.mjs by absolute path, and + # Node resolves `playwright` by walking up from there to /opt/sbx-e2e/node_modules. + - command: "install -d -o 1000 -g 1000 /opt/sbx-e2e" + description: create the sandbox-local e2e prefix + + # Pinned, not @latest: the browser build id is tied to the playwright version, so + # a floating version installed later in a session no longer matches the browsers + # baked in here and dies with "Executable doesn't exist at .../chromium-". + # A real package.json (rather than --no-save) is what keeps it installed — with + # nothing declared, the next npm command in this prefix prunes node_modules. + - command: | + set -eu + cat > /opt/sbx-e2e/package.json <<'PKGEOF' + { + "name": "sbx-e2e", + "private": true, + "type": "module", + "dependencies": { "playwright": "1.62.0" } + } + PKGEOF + npm --prefix /opt/sbx-e2e install --loglevel=error + user: "1000" + description: install Playwright (pinned) + + # Chromium's shared libraries via apt (needs root — the default here). + - command: "/opt/sbx-e2e/node_modules/.bin/playwright install-deps chromium || echo 'convoy-dev: playwright install-deps failed; agent can rerun on demand'" + description: install Chromium OS dependencies + + # The browser itself, as the agent user so it lands in the home cache the agent + # actually uses. Non-fatal so a download hiccup can't block creation. + - command: "/opt/sbx-e2e/node_modules/.bin/playwright install chromium || echo 'convoy-dev: playwright chromium preinstall skipped'" + user: "1000" + description: pre-install Chromium for visual/e2e checks + + # Runs on EVERY start (as the agent user). Fast once ddev's images are baked + # into a template. + startup: + # /etc/hosts is a read-only bind mount from the host, so ddev's hostname step + # ("Failed to add hosts entry … read-only file system") aborts `ddev start` — + # and *.ddev.site has no public DNS answer in here to fall back on. Overmount a + # writable copy so ddev can register its own names; the marker line makes it + # idempotent across restarts. Sandbox-local by construction: nothing is written + # to the workspace, and the overmount dies with the sandbox. + - command: + - "bash" + - "-lc" + - | + set -euo pipefail + mark='# sbx: writable hosts overmount' + if grep -qxF "$mark" /etc/hosts 2>/dev/null; then + echo 'convoy-dev: /etc/hosts already writable' + exit 0 + fi + tmp=$(mktemp) + { cat /etc/hosts; echo "$mark"; } > "$tmp" + sudo install -m 0644 -o root -g root "$tmp" /var/lib/sbx-hosts + rm -f "$tmp" + sudo mount --bind /var/lib/sbx-hosts /etc/hosts + echo 'convoy-dev: overmounted /etc/hosts (writable copy at /var/lib/sbx-hosts)' + description: make /etc/hosts writable so ddev can register *.ddev.site + + # Copied rather than symlinked: Node resolves a symlinked module to its realpath, + # which would send the `playwright` lookup into the repo instead of /opt/sbx-e2e. + - command: ["bash", "-lc", "install -m 0644 \"${WORKSPACE_DIR:-.}/.sbx/dev/browser.mjs\" /opt/sbx-e2e/browser.mjs"] + description: publish the Playwright helpers to /opt/sbx-e2e + + # ddev signs the project cert with the mkcert CA of whichever machine runs + # `ddev start`, and drops it in .ddev/traefik/certs — which is part of the + # mounted workspace. Left alone, this sandbox's throwaway CA overwrites the + # host's cert in the checkout, and the next host `ddev start` copies it into + # the host router: every *.ddev.site project on the Mac then fails TLS in + # the browser, until a host-side `ddev restart` regenerates it. Overmount a + # sandbox-local dir (the same trick as /etc/hosts above) so ddev in here + # signs its own certs and the host checkout is never written to. + - command: + - "bash" + - "-lc" + - | + set -euo pipefail + certs="${WORKSPACE_DIR:-.}/.ddev/traefik/certs" + mkdir -p "$certs" + certs=$(cd "$certs" && pwd -P) + if awk -v d="$certs" '$2 == d { found = 1 } END { exit !found }' /proc/self/mounts; then + echo 'convoy-dev: .ddev/traefik/certs already overmounted' + exit 0 + fi + sudo install -d -m 0755 -o 1000 -g 1000 /var/lib/sbx-ddev-certs + sudo mount --bind /var/lib/sbx-ddev-certs "$certs" + echo 'convoy-dev: overmounted .ddev/traefik/certs (sandbox-local)' + description: keep sandbox-signed ddev certs out of the host checkout + + - command: ["bash", "-lc", "cd \"${WORKSPACE_DIR:-.}\" && ddev start -y"] + description: boot the ddev stack diff --git a/CHANGELOG.md b/CHANGELOG.md index 05e435096d7..cfd188b49a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,30 @@ This file is a running track of new features and fixes to each version of the pa The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project follows [Semantic Versioning](https://semver.org) guidelines. +## Unreleased + +### Changed + +- The admin overview's Attention card now names what needs attention instead of reporting a bare count, and every row + links to the record itself -- a failed server to its admin page, a failed backup to its server's backups tab. Groups + holding several records open a sheet listing them, capped at the 25 most recent with a note saying how many more + there are. Previously the card showed only a failed-server count while its caption mixed in failed backups and + servers mid-delete, so it could read "0" with backups broken, and its link went to the unfiltered server list either + way. Suspended and mid-delete servers are deliberately left off it: neither is a failure, and both are already + counted under Server State. + +### Fixed + +- Fixed modals taller than the browser window clipping their own header and submit row with no way to scroll to them. + The panel is now bounded by the viewport and its body scrolls within it, so Create Server and Create Node stay usable + at shorter window heights. +- Fixed opening a modal leaving keyboard focus behind it, which sent the first few Tab presses through the page + underneath instead of the dialog. +- Fixed the create-server form requesting addresses and template groups before a node is chosen, producing a handful of + 404s every time the modal opened. +- Fixed modals going out without an accessible name, so a screen reader announced only "dialog" instead of reading the + heading already on screen. + ## v4.6.1 ### Security diff --git a/app/Services/Admin/OverviewService.php b/app/Services/Admin/OverviewService.php index b5574ad48d0..d40f0d2e999 100644 --- a/app/Services/Admin/OverviewService.php +++ b/app/Services/Admin/OverviewService.php @@ -11,6 +11,7 @@ use Convoy\Models\Node; use Convoy\Models\Server; use Convoy\Models\User; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Cache; @@ -20,6 +21,14 @@ class OverviewService private const BYTES_PER_MEBIBYTE = 1048576; + /** + * Per-group cap on the records carried for the attention card. The card lists a + * handful and the group's count stays the authority for how many there really + * are, so a fleet where everything is broken costs a bounded query rather than a + * full table read on every dashboard load. + */ + private const ATTENTION_LIMIT = 25; + public function metrics(): array { return Cache::remember( @@ -43,6 +52,7 @@ private function build(): array 'addresses' => $this->addresses(), 'backups' => $this->backups(), 'isos' => $this->isos(), + 'attention' => $this->attention(), 'nodes' => $nodes ->map(fn (Node $node) => $this->node($node, $allocations)) ->all(), @@ -117,6 +127,89 @@ private function failedServers(Collection $statuses): int ); } + /** + * The records behind the attention card, each carrying the key its destination + * route takes -- a server's `uuid_short` throughout. Both the admin and client + * server routes are bound by it (RouteServiceProvider resolves an 8-character + * value as `uuid_short` and anything else as `uuid`, so the primary key never + * matches), and every other server link in the panel is built the same way. + * + * The card used to be a stat tile showing only the failed-server count while its + * caption mixed in failed backups and servers mid-delete, so it could read "0" + * with backups broken, and its link went to the unfiltered server list either + * way. Listing the records themselves is what lets a click land on the thing + * that is wrong. + * + * Two states are deliberately absent. Mid-delete is a transient state rather + * than a failure, and the server-state card already counts it. A suspension is + * an administrative decision someone made on purpose -- it is the panel working + * as asked, not something to be fixed, and listing it next to real failures + * dilutes the card into a status feed. + */ + private function attention(): array + { + return [ + 'failed_servers' => $this->serverSubjects( + Server::query()->whereIn('status', [ + Status::INSTALL_FAILED->value, + Status::DELETION_FAILED->value, + ]), + fn (Server $server) => sprintf( + '%s on %s', + $server->status === Status::DELETION_FAILED->value + ? 'Deletion failed' + : 'Installation failed', + $server->node?->name ?? 'an unknown node', + ), + ), + 'failed_backups' => $this->failedBackupSubjects(), + ]; + } + + /** + * @param callable(Server): ?string $detail + */ + private function serverSubjects(Builder $query, callable $detail): array + { + return $query + ->with('node:id,name') + ->orderByDesc('id') + ->limit(self::ATTENTION_LIMIT) + ->get(['id', 'uuid_short', 'name', 'node_id', 'status']) + ->map(fn (Server $server) => [ + 'id' => $server->uuid_short, + 'label' => $server->name, + 'detail' => $detail($server), + ]) + ->all(); + } + + /** + * Failed backups, keyed by the owning server's uuid_short -- the client server's + * backups tab is the only page that shows a backup, and ServerPolicy::before lets + * an admin open it for any server. A backup carries no failure message on this + * branch, so the detail names the server and when it gave up instead. + * + * The key is the same shape the server groups use; only the destination differs. + */ + private function failedBackupSubjects(): array + { + return Backup::query() + ->whereNotNull('completed_at') + ->where('is_successful', false) + ->whereHas('server') + ->with('server:id,uuid_short,name') + ->orderByDesc('completed_at') + ->limit(self::ATTENTION_LIMIT) + ->get(['id', 'server_id', 'name', 'completed_at']) + ->map(fn (Backup $backup) => [ + 'id' => $backup->server->uuid_short, + 'label' => $backup->name, + 'detail' => $backup->server->name.' · failed '.$backup->completed_at->diffForHumans(), + ]) + ->all(); + } + private function capacity(Collection $nodes, Collection $allocations): array { $memoryAllocated = $allocations->sum( diff --git a/app/Transformers/Admin/OverviewTransformer.php b/app/Transformers/Admin/OverviewTransformer.php index 694b081c02b..7cccb96a633 100644 --- a/app/Transformers/Admin/OverviewTransformer.php +++ b/app/Transformers/Admin/OverviewTransformer.php @@ -49,6 +49,10 @@ public function transform(array $overview): array 'successful' => $overview['isos']['successful'], 'pending' => $overview['isos']['pending'], ], + 'attention' => [ + 'failed_servers' => $this->subjects($overview['attention']['failed_servers']), + 'failed_backups' => $this->subjects($overview['attention']['failed_backups']), + ], 'nodes' => collect($overview['nodes']) ->map(fn (array $node) => [ 'id' => $node['id'], @@ -63,6 +67,22 @@ public function transform(array $overview): array ]; } + /** + * One attention group: the records behind a row, each with the route key its + * destination takes. Capped upstream, so a group can be shorter than the count + * reported beside it. + */ + private function subjects(array $subjects): array + { + return collect($subjects) + ->map(fn (array $subject) => [ + 'id' => $subject['id'], + 'label' => $subject['label'], + 'detail' => $subject['detail'], + ]) + ->all(); + } + private function metric(array $metric): array { return [ diff --git a/database/seeders/AttentionSeeder.php b/database/seeders/AttentionSeeder.php new file mode 100644 index 00000000000..f020cc0ce45 --- /dev/null +++ b/database/seeders/AttentionSeeder.php @@ -0,0 +1,165 @@ +update(['status' => 'deletion_failed']);" + * + * Healthy noise (successful and pending backups, servers mid-install, mid-delete + * and suspended) comes along so the surrounding cards are not all zeros. Those + * last two are also negative checks: mid-delete and suspended both belong in + * Server State and NOT on the attention card. + * + * Everything lands on a placeholder node, never the live one from + * DevNodeSeeder: a card link opens the server's admin page, and a phantom vmid + * on a real host is a worse failure than one on a node that was never real. + * + * Idempotent -- it drops its own servers (by name) and their backups first, so + * re-running does not multiply the fixtures. + * + * Run: ddev artisan db:seed --class=AttentionSeeder + */ +class AttentionSeeder extends Seeder +{ + private const NODE_NAME = 'ord-hv-01'; + + private const NODE_FQDN = 'ord-hv-01.fixtures.invalid'; + + /** Fixture server names, also the handle used to clean up a previous run. */ + private const FAILED = ['ord-web-04']; + + private const SUSPENDED = ['fra-mail-01']; + + private const BACKUP_OWNERS = ['ams-app-01', 'ams-app-02', 'ams-cache-01']; + + private const HEALTHY = ['syd-web-01', 'syd-web-02', 'syd-queue-01', 'syd-api-03']; + + public function run(ServerCreationService $service): void + { + $names = array_merge(self::FAILED, self::SUSPENDED, self::BACKUP_OWNERS, self::HEALTHY); + + $stale = Server::query()->whereIn('name', $names)->pluck('id'); + Backup::query()->whereIn('server_id', $stale)->forceDelete(); + Server::query()->whereIn('id', $stale)->delete(); + + $user = User::query()->orderBy('id')->first() ?? User::factory()->create(); + $node = $this->placeholderNode(); + + $make = function (string $name, ?string $status) use ($service, $user, $node): Server { + $uuid = $service->generateUniqueUuidCombo(); + + return Server::factory()->create([ + 'uuid' => $uuid, + 'uuid_short' => substr($uuid, 0, 8), + 'name' => $name, + 'hostname' => $name.'.example.com', + 'status' => $status, + 'user_id' => $user->id, + 'node_id' => $node->id, + 'cpu' => 2, + 'memory' => 2048 * 1024 * 1024, + 'disk' => 20 * 1024 * 1024 * 1024, + 'backup_limit' => 16, + 'snapshot_limit' => 16, + 'bandwidth_limit' => 100 * 1024 * 1024 * 1024, + ]); + }; + + // Exactly one, to hold the card's single-record path open. + $make(self::FAILED[0], Status::INSTALL_FAILED->value); + + // Not failures: suspended, mid-install and mid-delete are Server State's + // business and must stay off the card. + $make(self::SUSPENDED[0], Status::SUSPENDED->value); + $make(self::HEALTHY[0], Status::INSTALLING->value); + $make(self::HEALTHY[1], Status::INSTALLING->value); + $make(self::HEALTHY[2], Status::DELETING->value); + $make(self::HEALTHY[3], Status::RESTORING_BACKUP->value); + + $owners = collect(self::BACKUP_OWNERS)->map(fn (string $name) => $make($name, null)); + + // 28 failures against a cap of 25. A backup counts as failed only once it + // has given up -- completed_at set, is_successful false -- so a pending + // backup (completed_at null) is neither failed nor successful. + foreach (range(1, 28) as $i) { + $this->backup($owners[$i % $owners->count()], "daily-{$i}", false, now()->subHours($i)); + } + + foreach (range(1, 6) as $i) { + $this->backup($owners[$i % $owners->count()], "weekly-{$i}", true, now()->subDays($i)); + } + + foreach (range(1, 3) as $i) { + $this->backup($owners[$i % $owners->count()], "in-flight-{$i}", false, null); + } + + $this->command->info(sprintf( + 'AttentionSeeder: %d servers and %d backups on node #%d (%s).', + count($names), + 37, + $node->id, + $node->name, + )); + } + + private function backup(Server $server, string $name, bool $successful, ?object $completedAt): void + { + Backup::factory()->create([ + 'server_id' => $server->id, + 'name' => $name, + 'is_successful' => $successful, + 'is_locked' => false, + 'completed_at' => $completedAt, + ]); + } + + /** + * The node the fixtures hang off. Its own, rather than whichever node happens + * to be first: the card prints the node name in every detail line ("Deletion + * failed on ..."), so a faker word there makes the thing being tested harder + * to read. `.invalid` is reserved by RFC 2606 and can never resolve, which is + * the point -- nothing here should ever reach a host. + */ + private function placeholderNode(): Node + { + $existing = Node::query()->where('fqdn', self::NODE_FQDN)->first(); + + if ($existing) { + return $existing; + } + + return Node::factory() + ->for(Location::query()->firstOrCreate( + ['short_code' => 'ord'], + ['description' => 'Chicago (fixtures)'], + )) + ->create([ + 'name' => self::NODE_NAME, + 'cluster' => self::NODE_NAME, + 'fqdn' => self::NODE_FQDN, + ]); + } +} diff --git a/database/seeders/DevNodeSeeder.php b/database/seeders/DevNodeSeeder.php new file mode 100644 index 00000000000..a1ebc787299 --- /dev/null +++ b/database/seeders/DevNodeSeeder.php @@ -0,0 +1,132 @@ +' failed". + * + * Idempotent: an existing node with the same fqdn is left alone, so re-running + * after migrate:fresh never creates duplicates. + * + * Run: ddev artisan db:seed --class=DevNodeSeeder + */ +class DevNodeSeeder extends Seeder +{ + public function run(): void + { + $fqdn = env('PROXMOX_FQDN'); + $tokenId = env('PROXMOX_TOKEN_ID'); + $tokenSecret = env('PROXMOX_TOKEN_SECRET'); + + if (! $fqdn || ! $tokenId || ! $tokenSecret) { + $this->command->warn( + 'DevNodeSeeder skipped: set PROXMOX_FQDN, PROXMOX_TOKEN_ID and ' + .'PROXMOX_TOKEN_SECRET in .env first.' + ); + + return; + } + + if ($existing = Node::query()->where('fqdn', $fqdn)->first()) { + $this->command->info("DevNodeSeeder: node for {$fqdn} already exists (#{$existing->id})."); + + return; + } + + $cluster = env('PROXMOX_NODE_NAME') ?: explode('.', $fqdn)[0]; + + $location = Location::query()->firstOrCreate( + ['short_code' => 'dev'], + ['description' => 'Dev Proxmox'], + ); + + $verifyTls = filter_var(env('PROXMOX_VERIFY_TLS', false), FILTER_VALIDATE_BOOLEAN); + $port = (int) env('PROXMOX_PORT', 8006); + + // Advertised capacity drives the admin Capacity card and every + // overallocation check, so read it off the host rather than inventing it. + // Falls back to the factory's numbers when the node cannot be reached -- + // the seeder still has to work offline. + $capacity = $this->probeCapacity($fqdn, $port, $cluster, $tokenId, $tokenSecret, $verifyTls); + + // The factory supplies resource defaults; only the connection details and + // storage names come from the environment. + $node = Node::factory()->for($location)->create([ + 'name' => $cluster, + 'cluster' => $cluster, + 'fqdn' => $fqdn, + 'port' => $port, + 'verify_tls' => $verifyTls, + 'token_id' => $tokenId, + 'secret' => $tokenSecret, // encrypted by the model cast on save + 'vm_storage' => env('PROXMOX_VM_STORAGE', 'local'), + 'backup_storage' => env('PROXMOX_BACKUP_STORAGE', 'local'), + 'iso_storage' => env('PROXMOX_ISO_STORAGE', 'local'), + 'network' => env('PROXMOX_NETWORK', 'vmbr0'), + ] + $capacity); + + $this->command->info("DevNodeSeeder: created node #{$node->id} for {$fqdn}:{$node->port} (cluster {$cluster})."); + } + + /** + * Real memory and disk totals for the node, as a partial attribute array. + * + * Disk is the configured `vm_storage`'s total, which is the pool servers are + * actually built on -- summing every storage would double-count a host where + * one directory backs several entries. Returns an empty array when the node is + * unreachable, leaving the factory defaults in place. + */ + private function probeCapacity( + string $fqdn, + int $port, + string $cluster, + string $tokenId, + string $tokenSecret, + bool $verifyTls, + ): array { + $vmStorage = env('PROXMOX_VM_STORAGE', 'local'); + + try { + $client = Http::withOptions(['verify' => $verifyTls]) + ->withHeaders(['Authorization' => "PVEAPIToken={$tokenId}={$tokenSecret}"]) + ->timeout(15) + ->baseUrl("https://{$fqdn}:{$port}"); + + $memory = $client->get("/api2/json/nodes/{$cluster}/status")->json('data.memory.total'); + + $disk = collect($client->get("/api2/json/nodes/{$cluster}/storage")->json('data') ?? []) + ->firstWhere('storage', $vmStorage)['total'] ?? null; + } catch (Throwable $e) { + $this->command->warn("DevNodeSeeder: capacity probe failed ({$e->getMessage()}); using factory defaults."); + + return []; + } + + return array_filter([ + 'memory' => $memory ? (int) $memory : null, + 'disk' => $disk ? (int) $disk : null, + ]); + } +} diff --git a/lang/en_US/admin/overview.php b/lang/en_US/admin/overview.php index a6a5ce7067a..3d145bc3d25 100644 --- a/lang/en_US/admin/overview.php +++ b/lang/en_US/admin/overview.php @@ -7,7 +7,21 @@ 'nodes_locations_detail_one' => 'across :count location', 'nodes_locations_detail_other' => 'across :count locations', 'attention' => 'Attention', - 'attention_detail' => ':backups failed backups, :deleting deleting', + 'attention_description' => 'Everything currently in a state you have to do something about.', + 'attention_all_clear' => 'All clear', + 'attention_all_clear_detail' => 'No failed servers, no failed backups.', + 'attention_fix' => 'Fix', + 'attention_view' => 'View', + 'attention_failed_servers_one' => ':count server failed', + 'attention_failed_servers_other' => ':count servers failed', + 'attention_failed_backups_one' => ':count backup failed', + 'attention_failed_backups_other' => ':count backups failed', + // Passing `count` makes i18next look for the plural suffixes first, so + // spell them out rather than relying on the bare-key fallback. + 'attention_and_more_one' => 'and :count more', + 'attention_and_more_other' => 'and :count more', + 'attention_close' => 'Close', + 'attention_showing' => 'Showing the :shown most recent of :total.', 'capacity' => 'Capacity', 'capacity_description' => 'Convoy allocated resources across all nodes.', 'allocated_percent' => ':percent% allocated in Convoy', diff --git a/resources/scripts/api/admin/nodes/addresses/useAddressesSWR.ts b/resources/scripts/api/admin/nodes/addresses/useAddressesSWR.ts index 9a7748d6b90..4721c5b959a 100644 --- a/resources/scripts/api/admin/nodes/addresses/useAddressesSWR.ts +++ b/resources/scripts/api/admin/nodes/addresses/useAddressesSWR.ts @@ -4,6 +4,7 @@ import getAddresses, { AddressResponse, QueryParams, } from '@/api/admin/nodes/addresses/getAddresses' +import { isValidNodeId } from '@/api/admin/nodes/isValidNodeId' interface Params extends QueryParams { id?: string | number @@ -11,9 +12,13 @@ interface Params extends QueryParams { const useAddressesSWR = (nodeId: number, { page, id, ...params }: Params) => { return useSWR( - ['admin:node:addresses', nodeId, page, id], + // See useTemplateGroupsSWR: skip the fetch until a real node is selected, + // rather than requesting /api/admin/nodes/NaN/addresses. + isValidNodeId(nodeId) + ? ['admin:node:addresses', nodeId, page, id] + : null, () => getAddresses(nodeId, { page, ...params }) ) } -export default useAddressesSWR \ No newline at end of file +export default useAddressesSWR diff --git a/resources/scripts/api/admin/nodes/isValidNodeId.ts b/resources/scripts/api/admin/nodes/isValidNodeId.ts new file mode 100644 index 00000000000..f529c08e856 --- /dev/null +++ b/resources/scripts/api/admin/nodes/isValidNodeId.ts @@ -0,0 +1,10 @@ +/** + * Whether a node id is usable in a request path. + * + * Forms bind the node select to a string and coerce with `Number`, so before a + * node is chosen the id is `''` (which coerces to 0) or `NaN`, and callers that + * default to `-1` pass that straight through. None of those name a node, and a + * request built from one 404s. + */ +export const isValidNodeId = (nodeId: unknown): nodeId is number => + typeof nodeId === 'number' && Number.isInteger(nodeId) && nodeId > 0 diff --git a/resources/scripts/api/admin/nodes/templateGroups/useTemplateGroupsSWR.ts b/resources/scripts/api/admin/nodes/templateGroups/useTemplateGroupsSWR.ts index b91907af09f..580d7053c82 100644 --- a/resources/scripts/api/admin/nodes/templateGroups/useTemplateGroupsSWR.ts +++ b/resources/scripts/api/admin/nodes/templateGroups/useTemplateGroupsSWR.ts @@ -1,5 +1,6 @@ import useSWR from 'swr' +import { isValidNodeId } from '@/api/admin/nodes/isValidNodeId' import getTemplateGroups, { TemplateGroup, } from '@/api/admin/nodes/templateGroups/getTemplateGroups' @@ -9,7 +10,10 @@ const useTemplateGroupsSWR = ( fallbackData?: TemplateGroup[] ) => { return useSWR( - ['admin:node:template-groups', nodeId], + // A null key tells SWR not to fetch. The create-server form renders this + // before a node is picked, where nodeId is '' or NaN, and the request went + // out anyway as /api/admin/nodes//template-groups -> 404. + isValidNodeId(nodeId) ? ['admin:node:template-groups', nodeId] : null, () => getTemplateGroups(nodeId), { fallbackData, @@ -17,4 +21,4 @@ const useTemplateGroupsSWR = ( ) } -export default useTemplateGroupsSWR \ No newline at end of file +export default useTemplateGroupsSWR diff --git a/resources/scripts/api/admin/overview/getOverview.ts b/resources/scripts/api/admin/overview/getOverview.ts index 9f7f50dfcf6..365c34f3072 100644 --- a/resources/scripts/api/admin/overview/getOverview.ts +++ b/resources/scripts/api/admin/overview/getOverview.ts @@ -46,6 +46,22 @@ export interface DashboardIsos { pending: number } +/** + * One record behind an attention row. `id` is the route key its group's + * destination takes -- a server id for the server groups, a server's short uuid + * for a backup, since the backups tab is where a backup actually lives. + */ +export interface AttentionSubject { + id: string + label: string + detail: string | null +} + +export interface DashboardAttention { + failedServers: AttentionSubject[] + failedBackups: AttentionSubject[] +} + export interface DashboardNode { id: number name: string @@ -67,9 +83,17 @@ export interface DashboardOverview { addresses: DashboardAddresses backups: DashboardBackups isos: DashboardIsos + attention: DashboardAttention nodes: DashboardNode[] } +const rawSubjects = (data: any): AttentionSubject[] => + (data ?? []).map((subject: any) => ({ + id: String(subject.id), + label: subject.label, + detail: subject.detail ?? null, + })) + const rawMetric = (data: any): DashboardMetric => ({ allocated: data.allocated, total: data.total, @@ -93,6 +117,10 @@ export const rawDataToOverview = (data: any): DashboardOverview => ({ addresses: data.addresses, backups: data.backups, isos: data.isos, + attention: { + failedServers: rawSubjects(data.attention?.failed_servers), + failedBackups: rawSubjects(data.attention?.failed_backups), + }, nodes: data.nodes.map((node: any) => ({ id: node.id, name: node.name, diff --git a/resources/scripts/components/admin/overview/AttentionCard.tsx b/resources/scripts/components/admin/overview/AttentionCard.tsx new file mode 100644 index 00000000000..97a04fb282f --- /dev/null +++ b/resources/scripts/components/admin/overview/AttentionCard.tsx @@ -0,0 +1,263 @@ +import { + ArrowRightIcon, + CheckCircleIcon, + ExclamationTriangleIcon, +} from '@heroicons/react/24/outline' +import { ComponentType, useState } from 'react' +import { useTranslation } from 'react-i18next' +import { Link } from 'react-router-dom' + +import { + AttentionSubject, + DashboardOverview, +} from '@/api/admin/overview/getOverview' + +import Card from '@/components/elements/Card' +import Modal from '@/components/elements/Modal' + +interface IconProps { + className?: string +} + +interface AttentionGroup { + key: string + icon: ComponentType + /** Summary line used when the group holds more than one record. */ + title: string + actionLabel: string + /** The records themselves, capped by the endpoint. */ + subjects: AttentionSubject[] + /** Destination for one record. */ + to: (subject: AttentionSubject) => string + /** Real total, which can exceed what `subjects` carries. */ + total: number +} + +const iconClasses = 'text-error border-error-light bg-error-lighter' + +const SubjectRow = ({ + subject, + to, + onNavigate, +}: { + subject: AttentionSubject + to: string + onNavigate: () => void +}) => ( + +

+ {subject.label} +

+ {subject.detail && ( +

{subject.detail}

+ )} + +) + +/** + * The names behind a collapsed group, so the summary line carries something the + * count does not. The names truncate and the overflow count does not: baked into + * one string, `truncate` cuts from the end, so "and 3 more" -- the only part the + * count does not already say -- was the first thing to go. They are returned + * separately so the row can let the names shrink around a pinned suffix. + */ +const preview = (group: AttentionGroup) => ({ + names: group.subjects.map(subject => subject.label).join(', '), + hidden: group.total - group.subjects.length, +}) + +/** + * One group. A single record is named outright and links straight at itself -- + * "1 server failed to install" plus a hunt through the server list is strictly + * less information than naming the server. Several open a sheet over direct + * links, because the count alone is what made the old card useless. + * + * The sheet is what keeps the dashboard still. Expanding 25 records inline grew + * the card by several hundred pixels and shoved every card below it down the + * page, so reading one row rearranged the rest -- and the group most worth + * opening moved the page most. Modal.Body is the panel's scrolling section and + * the list is bounded however broken the fleet is. + */ +const GroupRow = ({ group }: { group: AttentionGroup }) => { + const [open, setOpen] = useState(false) + const { t } = useTranslation('admin.overview') + const Icon = group.icon + const truncated = group.total > group.subjects.length + const { names, hidden } = preview(group) + + const icon = ( +
+ +
+ ) + + if (group.subjects.length === 1) { + const [subject] = group.subjects + + return ( + + {icon} +
+

+ {subject.label} +

+ {subject.detail && ( +

+ {subject.detail} +

+ )} +
+ + {group.actionLabel} + + + + ) + } + + return ( + <> + + setOpen(false)}> + + {group.title} + + +
+ {group.subjects.map((subject, index) => ( + setOpen(false)} + /> + ))} +
+ {truncated && ( +

+ {t('attention_showing', { + shown: group.subjects.length, + total: group.total, + })} +

+ )} + + + setOpen(false)}> + {t('attention_close')} + + + + + ) +} + +const AttentionCard = ({ data }: { data: DashboardOverview }) => { + const { t } = useTranslation('admin.overview') + const { attention, summary, backups } = data + + const groups: AttentionGroup[] = [] + + if (attention.failedServers.length > 0) { + groups.push({ + key: 'failed-servers', + icon: ExclamationTriangleIcon, + title: t('attention_failed_servers', { + count: summary.failedServers, + }), + actionLabel: t('attention_fix'), + subjects: attention.failedServers, + to: subject => `/admin/servers/${subject.id}`, + total: summary.failedServers, + }) + } + + if (attention.failedBackups.length > 0) { + groups.push({ + key: 'failed-backups', + icon: ExclamationTriangleIcon, + title: t('attention_failed_backups', { count: backups.failed }), + actionLabel: t('attention_view'), + // The backups tab is the only page that shows a backup, and it is + // keyed by the server's short uuid -- the same key the admin server + // routes bind on. + to: subject => `/servers/${subject.id}/backups`, + subjects: attention.failedBackups, + total: backups.failed, + }) + } + + return ( + +
+
+

{t('attention')}

+

+ {t('attention_description')} +

+
+ 0 ? 'text-error' : 'text-accent-400' + }`} + /> +
+ + {groups.length === 0 ? ( +
+ +

+ {t('attention_all_clear')} +

+

+ {t('attention_all_clear_detail')} +

+
+ ) : ( +
+ {groups.map(group => ( + + ))} +
+ )} + + ) +} + +export default AttentionCard diff --git a/resources/scripts/components/admin/overview/OverviewContainer.tsx b/resources/scripts/components/admin/overview/OverviewContainer.tsx index fc7a630666d..27cdff2ef39 100644 --- a/resources/scripts/components/admin/overview/OverviewContainer.tsx +++ b/resources/scripts/components/admin/overview/OverviewContainer.tsx @@ -2,7 +2,6 @@ import { bytesToString } from '@/util/helpers' import { CircleStackIcon, CpuChipIcon, - ExclamationTriangleIcon, ServerStackIcon, SignalIcon, UsersIcon, @@ -12,16 +11,18 @@ import { ComponentType } from 'react' import { useTranslation } from 'react-i18next' import { Link } from 'react-router-dom' -import useOverviewSWR from '@/api/admin/overview/useOverviewSWR' import { DashboardMetric, DashboardNode, } from '@/api/admin/overview/getOverview' +import useOverviewSWR from '@/api/admin/overview/useOverviewSWR' import Card from '@/components/elements/Card' import MessageBox from '@/components/elements/MessageBox' import PageContentBlock from '@/components/elements/PageContentBlock' +import AttentionCard from '@/components/admin/overview/AttentionCard' + interface IconProps { className?: string } @@ -32,23 +33,9 @@ interface StatCardProps { detail?: string icon: ComponentType to?: string - tone?: 'default' | 'warning' | 'error' -} - -const toneClasses = { - default: 'text-accent-600 border-accent-200 bg-accent-100', - warning: 'text-warning-dark border-warning bg-warning-lighter', - error: 'text-error border-error-light bg-error-lighter', } -const StatCard = ({ - title, - value, - detail, - icon: Icon, - to, - tone = 'default', -}: StatCardProps) => { +const StatCard = ({ title, value, detail, icon: Icon, to }: StatCardProps) => { const content = (
@@ -58,7 +45,7 @@ const StatCard = ({

{detail &&

{detail}

}
-
+
@@ -66,10 +53,7 @@ const StatCard = ({ if (to) { return ( - + {content} @@ -77,11 +61,7 @@ const StatCard = ({ ) } - return ( - - {content} - - ) + return {content} } const UsageBar = ({ @@ -153,15 +133,16 @@ const NodeRow = ({ node }: { node: DashboardNode }) => { const OverviewSkeleton = () => (
- {[1, 2, 3, 4].map(item => ( + {[1, 2, 3].map(item => ( ))} +
) @@ -177,9 +158,7 @@ const OverviewContainer = () => {

{tStrings('overview')}

-

- {t('description')} -

+

{t('description')}

@@ -222,21 +201,6 @@ const OverviewContainer = () => { icon={UsersIcon} to='/admin/users' /> - 0 - ? 'error' - : 'default' - } - />
@@ -268,8 +232,7 @@ const OverviewContainer = () => {

{t('addresses_detail', { - available: - data.addresses.available, + available: data.addresses.available, pools: data.addresses.pools, })}

@@ -294,8 +257,7 @@ const OverviewContainer = () => { {tStrings('iso', { count: 2 })}

- {data.isos.successful} /{' '} - {data.isos.total} + {data.isos.successful} / {data.isos.total}

{t('iso_status_detail', { @@ -306,7 +268,9 @@ const OverviewContainer = () => {

- + + +

{t('server_state')}

@@ -316,14 +280,11 @@ const OverviewContainer = () => {
-
+
{[ [t('ready'), data.servers.ready], [t('installing'), data.servers.installing], - [ - tStrings('suspended'), - data.servers.suspended, - ], + [tStrings('suspended'), data.servers.suspended], [t('restoring'), data.servers.restoring], [t('deleting'), data.servers.deleting], [t('failed'), data.servers.failed], @@ -342,9 +303,7 @@ const OverviewContainer = () => { -

- {tStrings('node', { count: 2 })} -

+

{tStrings('node', { count: 2 })}

{t('nodes_description')}

diff --git a/resources/scripts/components/elements/Drawer.tsx b/resources/scripts/components/elements/Drawer.tsx index 9cafee5b6c8..c5a53af7320 100644 --- a/resources/scripts/components/elements/Drawer.tsx +++ b/resources/scripts/components/elements/Drawer.tsx @@ -42,14 +42,50 @@ const Drawer = forwardRef( leaveFrom='opacity-100 translate-y-0' leaveTo='opacity-0 translate-y-[100vh] sm:-translate-y-[10vh]' > + {/* + * Bounded by the viewport, not by its content: the + * panel is the scroll container's only child and + * the container is `overflow-hidden`, so a panel + * taller than the screen has its head and foot + * clipped with no way to reach them. Capping it + * here and laying it out as a column lets whichever + * section opts into `overflow-y-auto` (Modal.Body) + * absorb the excess, while a modal that already + * fits is untouched. + * + * The `[&>form]` rules extend that column through a + * form. Most modals wrap Modal.Body and + * Modal.Actions in one so the footer can submit, and + * a plain block form is a flex item that refuses to + * shrink below its content -- which left Modal.Body + * with no bounded parent to size `flex-1` against, + * and pushed the submit row out through + * `overflow-hidden` with nothing to scroll it back. + * Making the form a column too puts Modal.Body back + * under the cap. (FormProvider/FormikProvider render + * no DOM, so the form really is a direct child.) + */} - {children} diff --git a/resources/scripts/components/elements/Modal.tsx b/resources/scripts/components/elements/Modal.tsx index 5da9e090282..3d1028f7720 100644 --- a/resources/scripts/components/elements/Modal.tsx +++ b/resources/scripts/components/elements/Modal.tsx @@ -45,15 +45,24 @@ const Modal: Modal = ({ open, onClose, children }) => { } Modal.Header = styled.div` - ${tw`p-8 sm:p-6 border-b border-accent-200`} + ${tw`shrink-0 p-8 sm:p-6 border-b border-accent-200`} ` -Modal.Title = styled.h3` +/* + * Dialog.Title rather than a bare h3: Headless UI points the dialog's + * `aria-labelledby` at whatever it renders, and without it the panel had no + * accessible name at all -- a screen reader announced "dialog" and nothing + * else, even though a visible heading was sitting right there. `as='h3'` + * keeps the heading level the markup already used. + */ +const StyledTitle = styled(Dialog.Title)` ${tw`text-xl font-medium text-foreground text-center`} ` +Modal.Title = ({ children }) => {children} + Modal.Body = styled.div` - ${tw`max-h-[60vh] overflow-y-auto p-6 bg-accent-100`} + ${tw`min-h-0 flex-1 overflow-y-auto p-6 bg-accent-100`} ` Modal.Description = ({ children, bottomMargin }) => { @@ -67,7 +76,7 @@ Modal.Description = ({ children, bottomMargin }) => { } Modal.Actions = styled.div` - ${tw`flex border-t border-accent-200`} + ${tw`shrink-0 flex border-t border-accent-200`} & > button:is(:first-of-type) { ${tw`rounded-bl`} diff --git a/tests/Feature/Controllers/Admin/OverviewControllerTest.php b/tests/Feature/Controllers/Admin/OverviewControllerTest.php index cfd91a7cbee..1ad44011915 100644 --- a/tests/Feature/Controllers/Admin/OverviewControllerTest.php +++ b/tests/Feature/Controllers/Admin/OverviewControllerTest.php @@ -9,6 +9,14 @@ use Convoy\Models\Node; use Convoy\Models\Server; use Convoy\Models\User; +use Illuminate\Support\Facades\Cache; + +beforeEach(function () { + // OverviewService caches its payload for 15s. Whether that survives between + // cases depends on the ambient cache driver -- an array store is per-process + // and hides the problem, a shared redis does not -- so pin it either way. + Cache::flush(); +}); it('returns overview metrics for admins', function () { $admin = User::factory()->create([ @@ -100,3 +108,108 @@ $this->actingAs($user)->getJson('/api/admin/overview') ->assertForbidden(); }); + +it('names the servers and backups behind each attention row', function () { + $admin = User::factory()->create(['root_admin' => true]); + $location = Location::factory()->create(); + $node = Node::factory()->for($location)->create(['name' => 'pve-1']); + + $failed = Server::factory()->for($node)->for($admin)->create([ + 'name' => 'broken-install', + 'status' => Status::INSTALL_FAILED->value, + ]); + $healthy = Server::factory()->for($node)->for($admin)->create([ + 'name' => 'nightly-host', + 'status' => null, + ]); + Backup::factory()->for($healthy)->create([ + 'name' => 'nightly', + 'is_successful' => false, + 'completed_at' => now(), + ]); + + $this->actingAs($admin)->getJson('/api/admin/overview') + ->assertOk() + // Each row carries the route key its own destination takes, so a click + // lands on the record rather than on the unfiltered server list. Every + // subject is keyed by the owning server's short uuid, which is what both + // the admin and client server routes bind on -- the primary key resolves + // to nothing. + ->assertJsonPath('data.attention.failed_servers.0.id', $failed->uuid_short) + ->assertJsonPath('data.attention.failed_servers.0.label', 'broken-install') + ->assertJsonPath('data.attention.failed_servers.0.detail', 'Installation failed on pve-1') + ->assertJsonPath('data.attention.failed_backups.0.id', $healthy->uuid_short) + ->assertJsonPath('data.attention.failed_backups.0.label', 'nightly'); +}); + +it('leaves a suspended server off the card entirely', function () { + $admin = User::factory()->create(['root_admin' => true]); + $node = Node::factory()->for(Location::factory())->create(); + Server::factory()->for($node)->for($admin)->create([ + 'status' => Status::SUSPENDED->value, + ]); + + // A suspension is deliberate, so it belongs in the server-state counts and + // nowhere near a list of things that need fixing. + $this->actingAs($admin)->getJson('/api/admin/overview') + ->assertOk() + ->assertJsonPath('data.servers.suspended', 1) + ->assertJsonPath('data.attention.failed_servers', []) + ->assertJsonMissingPath('data.attention.suspended_servers'); +}); + +it('keys attention subjects by something the server route can actually resolve', function () { + $admin = User::factory()->create(['root_admin' => true]); + $node = Node::factory()->for(Location::factory())->create(); + Server::factory()->for($node)->for($admin)->create([ + 'status' => Status::INSTALL_FAILED->value, + ]); + + $id = $this->actingAs($admin)->getJson('/api/admin/overview') + ->assertOk() + ->json('data.attention.failed_servers.0.id'); + + // The card links at /admin/servers/{id}, so whatever the endpoint hands back + // has to be the key that page's own request binds on. The primary key is not + // it: RouteServiceProvider reads a non-8-character value as a uuid, and the + // page dies with "No query results for model [Convoy\Models\Server]". + $this->actingAs($admin)->getJson("/api/admin/servers/{$id}")->assertOk(); +}); + +it('separates a deletion failure from an install failure in the detail', function () { + $admin = User::factory()->create(['root_admin' => true]); + $node = Node::factory()->for(Location::factory())->create(['name' => 'pve-2']); + + Server::factory()->for($node)->for($admin)->create([ + 'status' => Status::DELETION_FAILED->value, + ]); + + $this->actingAs($admin)->getJson('/api/admin/overview') + ->assertOk() + ->assertJsonPath('data.attention.failed_servers.0.detail', 'Deletion failed on pve-2'); +}); + +it('leaves the attention groups empty when nothing is wrong', function () { + $admin = User::factory()->create(['root_admin' => true]); + $node = Node::factory()->for(Location::factory())->create(); + Server::factory()->for($node)->for($admin)->create(['status' => null]); + + $this->actingAs($admin)->getJson('/api/admin/overview') + ->assertOk() + ->assertJsonPath('data.attention.failed_servers', []) + ->assertJsonPath('data.attention.failed_backups', []); +}); + +it('caps each attention group and leaves the count to say how many there really are', function () { + $admin = User::factory()->create(['root_admin' => true]); + $node = Node::factory()->for(Location::factory())->create(); + + Server::factory()->count(30)->for($node)->for($admin)->create([ + 'status' => Status::INSTALL_FAILED->value, + ]); + + $this->actingAs($admin)->getJson('/api/admin/overview') + ->assertOk() + ->assertJsonCount(25, 'data.attention.failed_servers') + ->assertJsonPath('data.summary.failed_servers', 30); +});