Updating workflow to integrate with GH action. #1427
Conversation
WalkthroughMultiple CI workflow tweaks and new automation scripts: a minor reorder in GitHub Actions branches, CircleCI now optionally dispatches a GitHub repository event, new nginx host-normalization conf added and wired into the nginx Docker image, entrypoint now delegates to a Quant provisioning script, and two new Quant-related scripts (download-db and provision-quant) were introduced. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub
participant CCI as CircleCI
participant GHA as GitHub Actions
Dev->>GH: Push / PR / Manual
GH->>CCI: Trigger CircleCI job
alt CIRCLECI has GITHUB_TOKEN
CCI->>GH: POST /repos/:owner/:repo/dispatches (event_type: "circleci_success", payload: {branch, sha, build_url})
else Token absent
CCI-->>CCI: Skip dispatch
end
GH-->>GHA: repository_dispatch (circleci_success) or manual workflow_dispatch
GHA->>GHA: Evaluate workflow conditions and run jobs as applicable
sequenceDiagram
autonumber
participant Shell as Shell script
participant Py as Python helper
participant Node as Node helper (qc API)
participant QC as Quant CLI/API
participant Remote as Remote storage
Shell->>Py: resolve `qc` path
Shell->>Node: request create backup (org/app/env)
Node->>QC: call QC API to create snapshot
QC-->>Node: ack backup_id
Node->>Shell: backup_id returned
loop poll
Shell->>Node: query backup status
Node->>QC: get status
QC-->>Node: status (pending/completed/failed)
Node-->>Shell: status
end
alt completed
Node->>QC: request download URL
QC-->>Node: download URL
Node-->>Shell: URL
Shell->>Remote: download archive
Shell->>Shell: decompress or copy fallback
else failed / timeout
Shell-->>Shell: exit with error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
richardgaunt
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/workflows/build-deploy.yml (4)
7-11: Avoid duplicate/racy runs from both check_suite and status.Both events will fire for the same commit; the job can run twice and rely on concurrency to cancel one. Prefer a single source of truth (Checks API) or keep
statusonly as a fallback with a stricter guard (actor/context).Option A (preferred): drop
statustrigger.- # Alternative: Use status event for the specific CircleCI workflow check - status: + # (status removed; rely on check_suite)Option B: keep
statusbut add the stricterifguard as in my other comment.
13-15: Make concurrency key robust for events lacking github.ref.
check_suite/statusmay not populategithub.refreliably. Fall back to the head branch or run_id.- group: build-and-push-${{ github.ref }} + group: build-and-push-${{ github.event.check_suite.head_branch || github.ref || github.run_id }}
3-11: Harden default token permissions.This workflow only needs read access to contents. Reduce blast radius by setting explicit permissions.
Add near the top:
name: Build and Push civictheme-monorepo-drupal to Quant Cloud 'on': +permissions: + contents: read
19-27: Verified: 'circleci-checks' app slug present; update/remove legacy status check.
'circleci-checks' appears in the check-suite app slugs for the head SHA, so the slug check is correct. No legacy status context 'ci/circleci: commit' was found for this SHA — the status branch won't match; remove or update that condition to an actual context (e.g., one of the ci/circleci: build-* contexts) or rely on the check_suite clause.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build-deploy.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: commit
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/build-deploy.yml (1)
3-9: Add least‑privilege permissions at workflow level.Lock down the default GITHUB_TOKEN; this workflow only needs read access to fetch code.
'on': # Manual trigger for testing and emergency deployments workflow_dispatch: # Trigger when CircleCI check suite completes successfully check_suite: types: [completed] + +permissions: + contents: read
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build-deploy.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: commit
🔇 Additional comments (3)
.github/workflows/build-deploy.yml (3)
24-27: Nice fix: prefix‑match CircleCI contexts.Switching from equality to
startsWith('ci/circleci:')avoids brittle context matching and unblocks runs across different CircleCI job names.
21-23: Confirm CircleCI app slug.Validate that the app slug is actually
circleci-checksin this repo. Some orgs seecirclecior different slugs depending on the CircleCI GitHub App generation.Would you like a quick script to dump the incoming
check_suite/check_runpayload for one run and confirm the slug and fields?
10-12:statusis not a valid GitHub Actions trigger; usecheck_run(or drop it).Workflows can’t subscribe to the
statuswebhook event. This branch will never fire. Replace it withcheck_run(completed) or remove it.Apply:
# Trigger when CircleCI check suite completes successfully check_suite: types: [completed] - # Alternative: Use status event for the specific CircleCI workflow check - status: + # Also react to individual check runs (CircleCI jobs) completing + check_run: + types: [completed]Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.circleci/config.yml(2 hunks).github/workflows/build-deploy.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: commit
.github/workflows/build-deploy.yml
Outdated
| # Manual trigger for testing and emergency deployments | ||
| workflow_dispatch: | ||
|
|
||
| # Trigger when CircleCI notifies via repository dispatch | ||
| repository_dispatch: | ||
| types: [circleci_success] |
There was a problem hiding this comment.
repository_dispatch doesn’t populate github.ref; you’re building the default branch instead of the CircleCI branch/sha.
On repository_dispatch, actions/checkout defaults to the default branch. Use the payload’s sha/branch, and make the “quant-cloud-migration” override key off that branch too.
Apply:
on:
# Manual trigger for testing and emergency deployments
workflow_dispatch:
# Trigger when CircleCI notifies via repository dispatch
repository_dispatch:
types: [circleci_success]
+permissions:
+ contents: read
+
jobs:
build-and-push:
@@
- - name: Checkout code
- uses: actions/checkout@v4
+ - name: Checkout code
+ uses: actions/checkout@v4
+ with:
+ # Prefer exact commit when dispatched by CircleCI; fall back to ref for manual runs.
+ ref: ${{ github.event.client_payload.sha || github.ref }}
+ fetch-depth: 0
@@
- - name: Override outputs for quant-cloud-migration branch
+ - name: Override outputs for quant-cloud-migration branch
id: override-outputs
run: |-
- # Override outputs for quant-cloud-migration branch to treat it as production
- if [[ "${{ github.ref }}" == "refs/heads/quant-cloud-migration" ]]; then
+ # Determine branch from dispatch payload or github.ref_name
+ BRANCH="${{ github.event.client_payload.branch || github.ref_name }}"
+ # Override outputs for quant-cloud-migration branch to treat it as production
+ if [[ "$BRANCH" == "quant-cloud-migration" ]]; then
echo "image_suffix=-latest" >> $GITHUB_OUTPUT
echo "image_suffix_clean=latest" >> $GITHUB_OUTPUT
echo "is_production=true" >> $GITHUB_OUTPUT
echo "environment_name=production" >> $GITHUB_OUTPUT
echo "environment_exists=true" >> $GITHUB_OUTPUT
echo "Overriding outputs for quant-cloud-migration branch: using -latest suffix and production environment"
else📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Manual trigger for testing and emergency deployments | |
| workflow_dispatch: | |
| # Trigger when CircleCI notifies via repository dispatch | |
| repository_dispatch: | |
| types: [circleci_success] | |
| on: | |
| # Manual trigger for testing and emergency deployments | |
| workflow_dispatch: | |
| # Trigger when CircleCI notifies via repository dispatch | |
| repository_dispatch: | |
| types: [circleci_success] | |
| permissions: | |
| contents: read |
🤖 Prompt for AI Agents
In .github/workflows/build-deploy.yml around lines 3 to 8, repository_dispatch
doesn’t populate github.ref so the workflow is checking out the default branch;
update the checkout step to use the SHA/branch from the repository_dispatch
payload (e.g. github.event.client_payload.sha or
github.event.client_payload.ref/branch) with a fallback to github.ref, and
compute the "quant-cloud-migration" override key from that same payload-derived
branch/sha instead of the default ref so the job builds the CircleCI
commit/branch that triggered the dispatch.
|
Need to investigate further tomorrow. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Add quant.type labels to docker-compose services - Add GitHub Actions build-deploy workflow - Update Drupal settings for Quant Cloud environment variables - Make Elasticsearch compatible with ECS Fargate - Convert post-rollout tasks to entrypoint scripts - Add nginx configuration for CDN header handling - Add Quant Cloud scripts (provision and database backup)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
scripts/quant/download-db.sh (3)
36-45: Remove hard dependency on Python for resolving qc path (use coreutils fallback).Avoid requiring python3 in minimal environments; prefer realpath/readlink with python as last resort.
Apply this diff:
-QC_BIN_REALPATH="$(python3 - <<'PY' -import os -import shutil -path = shutil.which('qc') -if not path: - raise SystemExit('qc binary not found') -print(os.path.realpath(path)) -PY -)" +QC_BIN_PATH="$(command -v qc)" +if command -v realpath >/dev/null 2>&1; then + QC_BIN_REALPATH="$(realpath "${QC_BIN_PATH}")" +elif command -v readlink >/dev/null 2>&1; then + QC_BIN_REALPATH="$(readlink -f "${QC_BIN_PATH}" || echo "${QC_BIN_PATH}")" +elif command -v python3 >/dev/null 2>&1; then + QC_BIN_REALPATH="$(python3 - <<'PY' +import os, shutil +path = shutil.which('qc') +print(os.path.realpath(path) if path else '') +PY +)" + [ -n "${QC_BIN_REALPATH}" ] || { echo "qc binary not found" >&2; exit 1; } +else + QC_BIN_REALPATH="${QC_BIN_PATH}" +fi
93-95: Harden API module resolution and fail fast with a clear error if the path is wrong.The path assumes utils/api.js is adjacent to the qc binary; verify and guard.
Apply this diff:
-const { ApiClient } = await import(pathToFileURL(path.join(moduleDir, 'utils/api.js')).href); +const apiModulePath = path.join(moduleDir, 'utils/api.js'); +if (!fs.existsSync(apiModulePath)) { + console.error(`[quant] Could not locate Quant CLI API module at ${apiModulePath}. Verify your qc installation or adjust QUANT_CLI_MODULE_DIR.`); + process.exit(1); +} +const { ApiClient } = await import(pathToFileURL(apiModulePath).href);Also please confirm the actual location of utils/api.js in your qc installation.
170-178: Add a download timeout to avoid hanging indefinitely on stalled HTTP connections.Use AbortController with a sane timeout.
Apply this diff:
-console.error(`[quant] Downloading backup to ${downloadPath}...`); -const response = await fetch(downloadUrl); +console.error(`[quant] Downloading backup to ${downloadPath}...`); +const controller = new AbortController(); +const dlTimeoutMs = 10 * 60 * 1000; // 10 minutes +const dlTimer = setTimeout(() => controller.abort(), dlTimeoutMs); +let response; +try { + response = await fetch(downloadUrl, { signal: controller.signal }); +} finally { + clearTimeout(dlTimer); +} if (!response.ok || !response.body) { console.error(`[quant] Failed to download backup: HTTP ${response.status}`); process.exit(1); }.docker/nginx-drupal.dockerfile (1)
20-23: Verify that the snippet is actually included by the base image.Ensure uselagoon/nginx-drupal includes /etc/nginx/conf.d/drupal/*.conf in the right context so fastcgi_param applies to PHP requests.
Optionally, you can squash the chmod into COPY to reduce layers:
-COPY .docker/config/nginx/location_drupal_prepend_host.conf /etc/nginx/conf.d/drupal/ -RUN chmod 0644 /etc/nginx/conf.d/drupal/location_drupal_prepend_host.conf +COPY --chmod=0644 .docker/config/nginx/location_drupal_prepend_host.conf /etc/nginx/conf.d/drupal/.docker/config/nginx/location_drupal_prepend_host.conf (1)
1-24: Confirm correct include context; consider using map for header precedence (optional).
Confirm this is included in server or location context where if/set are valid and before fastcgi_pass evaluation.
Optional: replace if/set chain with map in http context for clarity/performance.
Example (outside this snippet, in http context):
map $http_host $final_host { default $http_host; } map $http_x_forwarded_host $final_host { "~^(.+)$" $http_x_forwarded_host; default $final_host; } map $http_quant_orig_host $final_host { "~^(.+)$" $http_quant_orig_host; default $final_host; } map $final_host $final_host { "~^([^,\s]+)" $1; default $final_host; }Then keep: fastcgi_param HTTP_HOST $final_host;
scripts/quant/provision-quant.sh (4)
9-12: Fix ShellCheck directive and make .env loading robust; avoid temp file leak on missing .env.SC1090 is the relevant rule; ensure cleanup even if .env is absent.
Apply this diff:
-# shellcheck disable=SC1091 +# shellcheck disable=SC1090,SC1091 @@ -# Load project environment variables while preserving current exports. -t=$(mktemp) && export -p >"${t}" && set -a && . ./.env && if [ -f ./.env.local ]; then . ./.env.local; fi && set +a && . "${t}" && rm "${t}" && unset t +# Load project environment variables while preserving current exports. +t="$(mktemp)" +export -p >"${t}" +set -a +[ -f ./.env ] && . ./.env +[ -f ./.env.local ] && . ./.env.local +set +a +. "${t}" +rm -f "${t}" +unset t
31-39: Use portable checks instead of bash-specific [ -v ] to detect Quant environment.This improves compatibility across bash versions.
Apply this diff:
-has_quant_environment=0 -if [ -v QUANT_ENV_TYPE ] || [ -v QUANT_ENV_NAME ] || [ -v QUANT_APP_NAME ]; then - has_quant_environment=1 -fi +has_quant_environment=0 +if [ -n "${QUANT_ENV_TYPE:-}" ] || [ -n "${QUANT_ENV_NAME:-}" ] || [ -n "${QUANT_APP_NAME:-}" ]; then + has_quant_environment=1 +fiAlso update QUANT_ROUTE checks below similarly.
45-59: Make QUANT_ROUTE checks portable.Apply this diff:
-if [ -v QUANT_ROUTE ] && [ -n "${QUANT_ROUTE}" ]; then +if [ -n "${QUANT_ROUTE:-}" ]; then @@ -elif [ -v QUANT_ROUTE ]; then +elif [ -n "${QUANT_ROUTE+x}" ]; then
61-67: Fail fast if Drush is missing.Avoid late failures on first drush invocation.
Apply this diff:
drush_quant() { if [ -n "${drush_uri}" ]; then ./vendor/bin/drush -y --uri="${drush_uri}" "$@" else ./vendor/bin/drush -y "$@" fi } + +if [ ! -x "./vendor/bin/drush" ]; then + fail "Drush not found at ./vendor/bin/drush. Did you run composer install?" + exit 1 +fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.circleci/config.yml(2 hunks).docker/config/nginx/location_drupal_prepend_host.conf(1 hunks).docker/entrypoints/cli/03-provision-site.sh(1 hunks).docker/nginx-drupal.dockerfile(1 hunks).github/workflows/build-deploy.yml(1 hunks)scripts/quant/download-db.sh(1 hunks)scripts/quant/provision-quant.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .circleci/config.yml
- .github/workflows/build-deploy.yml
🧰 Additional context used
🪛 Shellcheck (0.11.0)
scripts/quant/provision-quant.sh
[warning] 12-12: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-and-push
- GitHub Check: build-and-push
- GitHub Check: commit
| # Delegate Drupal provisioning to the Quant-aware script. The standard | ||
| # DrevOps provision script is not compatible with Quant Cloud because it relies | ||
| # on Lagoon-specific tooling (e.g., rsync to Lagoon SSH). | ||
| if [ -x "./scripts/quant/provision-quant.sh" ]; then | ||
| ./scripts/quant/provision-quant.sh | ||
| else | ||
| echo "Quant provisioning script missing or not executable." >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Do not fail when the Quant script is absent; degrade gracefully to avoid breaking non‑Quant environments.
Current behavior exits 1, which can regress existing Lagoon/non‑Quant flows.
Apply this diff:
-if [ -x "./scripts/quant/provision-quant.sh" ]; then
- ./scripts/quant/provision-quant.sh
-else
- echo "Quant provisioning script missing or not executable." >&2
- exit 1
-fi
+if [ -x "./scripts/quant/provision-quant.sh" ]; then
+ ./scripts/quant/provision-quant.sh
+else
+ echo "Quant provisioning script missing or not executable; skipping." >&2
+fiAlternatively, gate on a Quant env var (e.g., QUANT_ENV_TYPE) before delegating.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Delegate Drupal provisioning to the Quant-aware script. The standard | |
| # DrevOps provision script is not compatible with Quant Cloud because it relies | |
| # on Lagoon-specific tooling (e.g., rsync to Lagoon SSH). | |
| if [ -x "./scripts/quant/provision-quant.sh" ]; then | |
| ./scripts/quant/provision-quant.sh | |
| else | |
| echo "Quant provisioning script missing or not executable." >&2 | |
| exit 1 | |
| fi | |
| # Delegate Drupal provisioning to the Quant-aware script. The standard | |
| # DrevOps provision script is not compatible with Quant Cloud because it relies | |
| # on Lagoon-specific tooling (e.g., rsync to Lagoon SSH). | |
| if [ -x "./scripts/quant/provision-quant.sh" ]; then | |
| ./scripts/quant/provision-quant.sh | |
| else | |
| echo "Quant provisioning script missing or not executable; skipping." >&2 | |
| fi |
🤖 Prompt for AI Agents
In .docker/entrypoints/cli/03-provision-site.sh around lines 32–40, the script
currently exits with status 1 if the Quant provisioning script is missing, which
breaks non‑Quant flows; change the logic to degrade gracefully by either (A)
gating execution on a Quant-specific env var (e.g., only attempt to run
./scripts/quant/provision-quant.sh when QUANT_ENV_TYPE or a similar flag
indicates a Quant environment), or (B) if you prefer automatic detection, keep
the executable check but remove the exit: log a warning that the Quant script is
missing/not executable and continue without failing so existing Lagoon/non‑Quant
provisioning proceeds; ensure any downstream provisioning still runs when
skipping the Quant script.
| if ! command -v qc >/dev/null 2>&1; then | ||
| echo "Quant CLI (qc) is required but was not found. Install it first." >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| QUANT_ORG_NAME="${QUANT_ORG_NAME:-salsa-digital}" |
There was a problem hiding this comment.
Guard for Node.js >= 18 (ESM + global fetch) to avoid runtime failure.
The Node block relies on ESM top‑level await and global fetch. Add a preflight check.
Apply this diff:
if ! command -v qc >/dev/null 2>&1; then
echo "Quant CLI (qc) is required but was not found. Install it first." >&2
exit 1
fi
+if ! command -v node >/dev/null 2>&1; then
+ echo "Node.js >= 18 is required but was not found. Please install it." >&2
+ exit 1
+fi
+NODE_MAJOR="$(node -e "console.log(parseInt(process.versions.node.split('.')[0], 10))")"
+if [ "${NODE_MAJOR}" -lt 18 ]; then
+ echo "Node.js >= 18 is required (for ESM and global fetch). Current: $(node -v)" >&2
+ exit 1
+fi
+
QUANT_ORG_NAME="${QUANT_ORG_NAME:-salsa-digital}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ! command -v qc >/dev/null 2>&1; then | |
| echo "Quant CLI (qc) is required but was not found. Install it first." >&2 | |
| exit 1 | |
| fi | |
| QUANT_ORG_NAME="${QUANT_ORG_NAME:-salsa-digital}" | |
| if ! command -v qc >/dev/null 2>&1; then | |
| echo "Quant CLI (qc) is required but was not found. Install it first." >&2 | |
| exit 1 | |
| fi | |
| if ! command -v node >/dev/null 2>&1; then | |
| echo "Node.js ≥ 18 is required but was not found. Please install it." >&2 | |
| exit 1 | |
| fi | |
| NODE_MAJOR="$(node -e "console.log(parseInt(process.versions.node.split('.')[0], 10))")" | |
| if [ "${NODE_MAJOR}" -lt 18 ]; then | |
| echo "Node.js ≥ 18 is required (for ESM and global fetch). Current: $(node -v)" >&2 | |
| exit 1 | |
| fi | |
| QUANT_ORG_NAME="${QUANT_ORG_NAME:-salsa-digital}" |
🤖 Prompt for AI Agents
In scripts/quant/download-db.sh around lines 5 to 10, the script may later run a
Node.js ESM script that requires Node >= 18 for top-level await and global
fetch, but there is no preflight check; add a guard that verifies the node
executable exists and that its major version is >= 18 (parse `node --version`),
print a clear error to stderr instructing to install/upgrade Node.js if the
check fails, and exit non‑zero; keep the existing qc check and ensure the new
Node check runs before any Node-based operations.
2 participants
Checklist before requesting a review
Changed
Screenshots
Summary by CodeRabbit
New Features
Chores
Bug Fixes