test(api7): deploy test components by docker compose#429
Merged
Conversation
- Replace tarball download (run.sh) with Docker Compose (Dashboard + PostgreSQL only) - ADC only needs Dashboard for Admin API CRUD, no need for DP Manager/Gateway/etcd - Add dev version testing for all backends: - apisix: add dev to matrix with BACKEND_APISIX_IMAGE=dev - api7: add dev using GHCR images (ghcr.io/api7/api7-ee-3-integrated:dev) - api7 dev uses separate license from BACKEND_API7_DEV_LICENSE secret - Add proper teardown via globalSetup return function - Add health check wait loop for Dashboard readiness
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Runner
participant Env as GITHUB_ENV
participant DC as Docker Compose
participant PG as PostgreSQL
participant DB as Dashboard
participant TS as Test Suite
CI->>Env: write matrix-backed BACKEND_* variables
CI->>DC: (optionally docker/login for dev) run `docker compose up -d`
DC->>PG: start postgres container
PG-->>DC: healthcheck passes (pg_isready)
DC->>DB: start dashboard (mount config, use env tags)
DB->>PG: connect & initialize
CI->>DB: poll GET /api/status (waitForDashboard)
alt status ok
DB-->>CI: 200 OK
else fallback
CI->>DB: POST /api/login (accept any status)
DB-->>CI: login response
end
CI->>TS: run e2e tests against dashboard
TS-->>CI: tests complete
CI->>DC: invoke teardown (async returned) -> `docker compose down -v`
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/backend-api7/e2e/support/global-setup.ts (1)
174-202:⚠️ Potential issue | 🟠 MajorClean up the compose stack on setup/bootstrap failure too.
The teardown function is only returned after
setupAPI7(),initUser(), andgenerateToken()all succeed. Ifdocker compose up -dworks butwaitForDashboard()or the user bootstrap fails, Vitest never receives the teardown callback and the stack stays up for the next run.Suggested shape
export default async () => { - if (process.env['SKIP_API7_SETUP'] !== 'true') await setupAPI7(); + const shouldManageStack = process.env['SKIP_API7_SETUP'] !== 'true'; + const teardown = async () => { + if (!shouldManageStack) return; + console.log('Tearing down API7 via Docker Compose'); + runCompose('down', '-v'); + }; + try { + if (shouldManageStack) await setupAPI7(); await initUser(); await generateToken(); } catch (err) { - console.log(err); + await teardown(); throw err; } ... - return async () => { - if (process.env['SKIP_API7_SETUP'] === 'true') return; - console.log('Tearing down API7 via Docker Compose'); - spawnSync('sh', ['-c', 'docker compose down -v'], { - cwd: assetsDir, - stdio: 'inherit', - }); - }; + return teardown; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/backend-api7/e2e/support/global-setup.ts` around lines 174 - 202, The teardown callback must be returned/registered even if setupAPI7(), initUser(), or generateToken() fail so the compose stack is always cleaned; create the teardown function (the async function that runs spawnSync('sh', ['-c', 'docker compose down -v'], ...)) before calling setupAPI7()/initUser()/generateToken() and ensure it is returned in all code paths (e.g., define teardown early and use try { await setupAPI7(); await initUser(); await generateToken(); } catch (err) { console.log(err); throw err; } finally { if (process.env['SKIP_API7_SETUP'] !== 'true') return teardown; } or simply wrap the bootstrap in try/finally and always return the teardown), referencing the existing setupAPI7, initUser, generateToken functions and the spawnSync teardown logic so the compose stack is cleaned on any failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e.yaml:
- Around line 146-152: The workflow is missing the explicit permissions needed
for GHCR pulls; add "packages: read" to the workflow permissions so the
GITHUB_TOKEN can authenticate to ghcr.io for the "Login to GHCR" step that uses
docker/login-action@v3 (and subsequent pulls of
ghcr.io/api7/api7-ee-3-integrated:dev). Update the workflow-level permissions
block to include packages: read alongside contents: read, ensure the job using
GITHUB_TOKEN inherits those permissions, and keep the existing
docker/login-action@v3 step and secret usage unchanged.
- Around line 130-142: The step "Determine API7 image and license" currently
writes BACKEND_API7_LICENSE to $GITHUB_ENV via a single-line echo which breaks
on multiline secrets; replace those echo lines with GitHub Actions heredoc
syntax to append the license safely (e.g., use a <<EOF block redirected to
$GITHUB_ENV) for both the dev and non-dev branches so multiline license values
are preserved; keep the other env writes the same but ensure
BACKEND_API7_LICENSE uses the heredoc approach in both branches.
In `@libs/backend-api7/e2e/assets/docker-compose.yaml`:
- Around line 22-23: The compose file currently pins the Dashboard to host port
"7443:7443"; change the docker-compose.yaml ports entry to use an ephemeral host
port (e.g., replace "7443:7443" with "0:7443" or remove ports and use "expose: -
7443") so multiple concurrent runs can start, and update global-setup.ts (which
currently hardcodes https://localhost:7443) to discover the actual mapped host
port at runtime and set the SERVER value / axios baseURL accordingly (read the
container's mapped port via your docker/test harness and inject the resolved
https://localhost:<mappedPort> into SERVER or the axios client before tests
run).
In `@libs/backend-api7/e2e/support/global-setup.ts`:
- Around line 72-82: The spawnSync invocation that runs `docker compose
${pullPolicy} up -d` currently only checks result.status; update the setup block
around the spawnSync call (symbols: spawnSync, result, assetsDir, pullPolicy) to
explicitly handle launch errors (check result.error) and signal termination
(check result.signal) and include those details in the thrown Error or logged
message; likewise, in the teardown path that invokes docker compose down,
capture its spawnSync result, check result.error/result.signal/result.status,
and fail or log with detailed context instead of ignoring it so teardown
failures don't silently leave resources running.
---
Outside diff comments:
In `@libs/backend-api7/e2e/support/global-setup.ts`:
- Around line 174-202: The teardown callback must be returned/registered even if
setupAPI7(), initUser(), or generateToken() fail so the compose stack is always
cleaned; create the teardown function (the async function that runs
spawnSync('sh', ['-c', 'docker compose down -v'], ...)) before calling
setupAPI7()/initUser()/generateToken() and ensure it is returned in all code
paths (e.g., define teardown early and use try { await setupAPI7(); await
initUser(); await generateToken(); } catch (err) { console.log(err); throw err;
} finally { if (process.env['SKIP_API7_SETUP'] !== 'true') return teardown; } or
simply wrap the bootstrap in try/finally and always return the teardown),
referencing the existing setupAPI7, initUser, generateToken functions and the
spawnSync teardown logic so the compose stack is cleaned on any failure.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1795c012-fbd1-4020-8ed1-3d9ab187adec
📒 Files selected for processing (5)
.github/workflows/e2e.yamllibs/backend-api7/e2e/assets/dashboard-conf.yamllibs/backend-api7/e2e/assets/docker-compose.yamllibs/backend-api7/e2e/support/global-setup.tslibs/backend-api7/e2e/support/global-teardown.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/e2e.yaml (1)
154-160:⚠️ Potential issue | 🟠 MajorMissing
packages: readpermission for GHCR image pull.The GHCR login step uses
GITHUB_TOKENto authenticate, but the workflow-level permissions (lines 2-3) only declarecontents: read. Pulling the private imageghcr.io/api7/api7-ee-3-integrated:devrequires explicitpackages: readpermission.permissions: contents: read + packages: read,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e.yaml around lines 154 - 160, The workflow is missing the required workflow-level permission to pull GHCR images; update the workflow permissions to include packages: read so the "Login to GHCR" step (uses: docker/login-action@v3) can authenticate with GITHUB_TOKEN and pull ghcr.io images. Add or extend the permissions block to include packages: read alongside any existing permissions (e.g., contents: read).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/e2e.yaml:
- Around line 154-160: The workflow is missing the required workflow-level
permission to pull GHCR images; update the workflow permissions to include
packages: read so the "Login to GHCR" step (uses: docker/login-action@v3) can
authenticate with GITHUB_TOKEN and pull ghcr.io images. Add or extend the
permissions block to include packages: read alongside any existing permissions
(e.g., contents: read).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aec15adf-3da7-4e64-8ff0-d96855664db5
📒 Files selected for processing (1)
.github/workflows/e2e.yaml
bitnami/postgresql:15.4.0-debian-11-r45 no longer exists on Docker Hub. Switch to the official postgres:15-alpine image with the correct env vars.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/e2e.yaml (1)
131-151: Fail fast when license secrets are missing to avoid opaque downstream failures.Right now, if a secret is unset/empty, the workflow still writes
BACKEND_API7_LICENSEand fails much later. Add an explicit guard in each branch for clearer diagnostics.💡 Suggested hardening
- name: Determine API7 image and license run: | if [ "${{ matrix.version }}" = "dev" ]; then + if [ -z "${{ secrets.BACKEND_API7_DEV_LICENSE }}" ]; then + echo "::error::Missing required secret BACKEND_API7_DEV_LICENSE" + exit 1 + fi echo "BACKEND_API7_VERSION=999.999.999" >> $GITHUB_ENV echo "API7_DASHBOARD_IMAGE=ghcr.io/api7/api7-ee-3-integrated" >> $GITHUB_ENV echo "API7_IMAGE_TAG=dev" >> $GITHUB_ENV { echo "BACKEND_API7_LICENSE<<EOLICENSE" echo "${{ secrets.BACKEND_API7_DEV_LICENSE }}" echo "EOLICENSE" } >> $GITHUB_ENV else + if [ -z "${{ secrets.BACKEND_API7_LICENSE }}" ]; then + echo "::error::Missing required secret BACKEND_API7_LICENSE" + exit 1 + fi echo "BACKEND_API7_VERSION=${{ matrix.version }}" >> $GITHUB_ENV echo "API7_DASHBOARD_IMAGE=api7/api7-ee-3-integrated" >> $GITHUB_ENV echo "API7_IMAGE_TAG=v${{ matrix.version }}" >> $GITHUB_ENV { echo "BACKEND_API7_LICENSE<<EOLICENSE" echo "${{ secrets.BACKEND_API7_LICENSE }}" echo "EOLICENSE" } >> $GITHUB_ENV fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e.yaml around lines 131 - 151, The workflow currently writes BACKEND_API7_LICENSE even when secrets.BACKEND_API7_DEV_LICENSE or secrets.BACKEND_API7_LICENSE are empty, causing opaque failures later; add an explicit guard in both branches (the dev branch where matrix.version == "dev" and the else branch) to check the corresponding secret is non-empty (inspect matrix.version and the secret variables secrets.BACKEND_API7_DEV_LICENSE and secrets.BACKEND_API7_LICENSE) and if empty emit a clear error (e.g., echo "::error::... missing secret" or >&2) and exit 1 immediately to fail fast before writing BACKEND_API7_LICENSE to GITHUB_ENV.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/e2e.yaml:
- Around line 131-151: The workflow currently writes BACKEND_API7_LICENSE even
when secrets.BACKEND_API7_DEV_LICENSE or secrets.BACKEND_API7_LICENSE are empty,
causing opaque failures later; add an explicit guard in both branches (the dev
branch where matrix.version == "dev" and the else branch) to check the
corresponding secret is non-empty (inspect matrix.version and the secret
variables secrets.BACKEND_API7_DEV_LICENSE and secrets.BACKEND_API7_LICENSE) and
if empty emit a clear error (e.g., echo "::error::... missing secret" or >&2)
and exit 1 immediately to fail fast before writing BACKEND_API7_LICENSE to
GITHUB_ENV.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7cf69d29-506b-4330-b036-0390093a16ba
📒 Files selected for processing (3)
.github/workflows/e2e.yamllibs/backend-api7/e2e/assets/docker-compose.yamllibs/backend-api7/e2e/support/global-setup.ts
✅ Files skipped from review due to trivial changes (1)
- libs/backend-api7/e2e/assets/docker-compose.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/backend-api7/e2e/support/global-setup.ts
Prevents cancellation of healthy jobs when one version fails. The dev job has a known test mismatch (https_verify_certificate removed from health check response in latest CP).
nic-6443
reviewed
Apr 16, 2026
nic-6443
added
the
test/apisix-standalone
|
Test case failures caused by |
nic-6443
reviewed
Apr 16, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/e2e.yaml (1)
132-152: Fail early when required API7 license secret is missing.At Line 140 and Line 149, an unset secret becomes an empty
BACKEND_API7_LICENSE, and the job fails later with less actionable errors. Add an explicit guard in this step to fail fast with a clear message.Proposed hardening
- name: Determine API7 image and license run: | if [ "${{ matrix.version }}" = "dev" ]; then + if [ -z "${{ secrets.BACKEND_API7_DEV_LICENSE }}" ]; then + echo "::error::Missing required secret BACKEND_API7_DEV_LICENSE" + exit 1 + fi echo "BACKEND_API7_VERSION=999.999.999" >> $GITHUB_ENV echo "API7_DASHBOARD_IMAGE=ghcr.io/api7/api7-ee-3-integrated" >> $GITHUB_ENV echo "API7_IMAGE_TAG=dev" >> $GITHUB_ENV { echo "BACKEND_API7_LICENSE<<EOLICENSE" echo "${{ secrets.BACKEND_API7_DEV_LICENSE }}" echo "EOLICENSE" } >> $GITHUB_ENV else + if [ -z "${{ secrets.BACKEND_API7_LICENSE }}" ]; then + echo "::error::Missing required secret BACKEND_API7_LICENSE" + exit 1 + fi echo "BACKEND_API7_VERSION=${{ matrix.version }}" >> $GITHUB_ENV echo "API7_DASHBOARD_IMAGE=api7/api7-ee-3-integrated" >> $GITHUB_ENV echo "API7_IMAGE_TAG=v${{ matrix.version }}" >> $GITHUB_ENV { echo "BACKEND_API7_LICENSE<<EOLICENSE" echo "${{ secrets.BACKEND_API7_LICENSE }}" echo "EOLICENSE" } >> $GITHUB_ENV fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e.yaml around lines 132 - 152, Add a guard after reading the license secret into BACKEND_API7_LICENSE to fail fast if the secret is empty: after echoing the HERE-doc for BACKEND_API7_LICENSE (when using secrets.BACKEND_API7_DEV_LICENSE or secrets.BACKEND_API7_LICENSE) check that the resulting variable is non-empty and, if empty, emit a clear error message and exit 1 so the job stops immediately instead of failing later with obscure errors; update the "Determine API7 image and license" step to validate BACKEND_API7_LICENSE and exit on missing secret.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/e2e.yaml:
- Around line 132-152: Add a guard after reading the license secret into
BACKEND_API7_LICENSE to fail fast if the secret is empty: after echoing the
HERE-doc for BACKEND_API7_LICENSE (when using secrets.BACKEND_API7_DEV_LICENSE
or secrets.BACKEND_API7_LICENSE) check that the resulting variable is non-empty
and, if empty, emit a clear error message and exit 1 so the job stops
immediately instead of failing later with obscure errors; update the "Determine
API7 image and license" step to validate BACKEND_API7_LICENSE and exit on
missing secret.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1f8a9202-fa92-491c-abcd-714949fbf291
📒 Files selected for processing (1)
.github/workflows/e2e.yaml
- Use api7/postgresql:15.4.0-debian-11-r45 with bitnami env vars - Remove label-based condition from api7 job (run for all PRs) - Remove https_verify_certificate from sync-and-dump-2 test expectation (dev CP no longer returns this default field)
Contributor
Author
|
Fixed — removed |
nic-6443
reviewed
Apr 16, 2026
nic-6443
reviewed
Apr 16, 2026
Remove cron_spec, session_options_config, max_open_conns, access_log, status server, and other non-essential config items.
nic-6443
approved these changes
Apr 16, 2026
bzp2010
reviewed
Apr 16, 2026
- Restore apisix workflow to original (no dev, no packages:read) - Restore label condition on api7 job (needed for fork contributors) - Move packages:read to api7 job-level permissions - Update versions: 3.8.22→3.8.23, 3.9.2→3.9.9 - Switch to named setup/teardown exports per vitest docs - Delete unused global-teardown.ts
jarvis9443
changed the title
bzp2010
changed the title
bzp2010
approved these changes
Apr 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace the tarball-based API7 E2E test infrastructure with Docker Compose, deploying only Dashboard + PostgreSQL (the components ADC actually tests against). Also adds
devversion testing for the api7 backend.Changes
Docker Compose for API7 E2E
libs/backend-api7/e2e/assets/docker-compose.yaml— Dashboard + PostgreSQL serviceslibs/backend-api7/e2e/assets/dashboard-conf.yaml— Minimal dashboard config (TLS, log, DSN only)global-setup.ts— Usesdocker compose up/down, namedsetup/teardownexportsglobal-teardown.ts— Teardown handled by named export in global-setup.tsDev version testing (api7 only)
devto api7 CI matrixghcr.io/api7/api7-ee-3-integrated:dev) withBACKEND_API7_DEV_LICENSEfail-fast: falseon api7 matrix to prevent healthy jobs from being canceledpackages: readpermission at api7 job level for GHCR accessVersion updates
Test fix
https_verify_certificatefromsync-and-dump-2test expectation (dev CP no longer returns this default field)Why only Dashboard + PostgreSQL?
ADC E2E tests only interact with the Dashboard Admin API (
/apisix/admin/*proxied through Dashboard on port 7443). No data plane traffic is ever sent to the gateway. The previous tarball approach deployed DPM, Gateway, and etcd unnecessarily.