[ci] Speed up CI by splitting tests into multiple groups and pre-baking a Docker image #1317#1381
Conversation
CI currently installs all system dependencies from run ,GDAL, SpatiaLite, Prettier, browsers, and Redis via apt-get and npm, adding several minutes of pure setup overhead before a single test runs. This introduces a pre-built Docker image (Dockerfile.ci) cached in GHCR that ships all dependencies pre-installed, so jobs start immediately without any package installation. The image is rebuilt only when Dockerfile.ci or requirements files change; all other pushes reuse the cached image. Tests are also split into three parallel matrix groups (standard, selenium,sample_app) to reduce wall-clock time further, and QA checks now run in the same container so the entire pipeline benefits from the cache.
CI currently installs all system dependencies from run ,GDAL, SpatiaLite, Prettier, browsers, and Redis via apt-get and npm, adding several minutes of pure setup overhead before a single test runs. This introduces a pre-built Docker image (Dockerfile.ci) cached in GHCR that ships all dependencies pre-installed, so jobs start immediately without any package installation. The image is rebuilt only when Dockerfile.ci or requirements files change; all other pushes reuse the cached image. Tests are also split into three parallel matrix groups (standard, selenium,sample_app) to reduce wall-clock time further, and QA checks now run in the same container so the entire pipeline benefits from the cache. Closes openwisp#1317
…/openwisp-controller into issues/1317-speedup-ci
|
The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (1/3). |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ 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)
🧰 Additional context used🪛 Hadolint (2.14.0)Dockerfile.ci[warning] 34-34: Set the SHELL option -o pipefail before RUN with a pipe in it. If you are using /bin/sh in an alpine image or if your shell is symlinked to busybox then consider explicitly setting your SHELL to /bin/ash, or disable this check (DL4006) 🔇 Additional comments (1)
📝 WalkthroughWalkthroughRefactors CI to use a reusable Dockerfile.ci image and an image-driven workflow: adds workflow-level concurrency/permissions, a resolve-image job that chooses between prebuilt or branch images, conditional build/promote jobs, runs qa/tests inside resolved images with a non-prebuilt fallback, restructures tests into a test-group matrix (standard, selenium, sample_app) with per-group coverage artifacts, and combines/uploads coverage to Coveralls. Sequence Diagram(s)sequenceDiagram
participant Runner as GitHub Actions Runner
participant Resolve as resolve-image
participant Build as build-image
participant QA as qa Job
participant TestMatrix as test-group matrix
participant Coverage as coverage-combine
participant Coveralls as Coveralls Service
Runner->>Resolve: determine image (use_prebuilt, needs_build, image_tag)
Resolve-->>Runner: selected image
Resolve-->>Build: signal if needs_build
Build->>Runner: pushed branch image
Runner->>QA: run QA inside selected image
Runner->>TestMatrix: spawn test groups (standard/selenium/sample_app)
TestMatrix->>Runner: upload coverage-* artifacts
Runner->>Coverage: download and combine coverage-*
Coverage->>Coveralls: upload coverage.xml
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (2/3). |
|
The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (3/3). |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 69-75: BRANCH_TAG currently only depends on branch name which can
yield stale images after force-push; update the BRANCH_TAG generation (and any
image reuse checks that reference it) to include a stable build-specific
identifier such as GITHUB_SHA or a digest of build inputs (e.g., sha256 of
Dockerfile.ci plus requirements files) so the tag changes when inputs change;
modify the BRANCH_TAG assignment (and IMAGE usage that references it) to append
or replace the branch-hash with either the github.sha or the computed
inputs-digest and ensure the image existence/reuse check uses that new tag so
rebuilt images aren’t incorrectly reused after history rewrites.
- Around line 227-234: The df_check step currently diffs only HEAD^..HEAD so
multi-commit pushes can miss changes to Dockerfile.ci; modify the git diff
invocation in the df_check run block to compare the full push range using the
event before and after SHAs (replace the HEAD^ HEAD args with the push range:
github.event.before → github.sha or the equivalent workflow expressions) so the
check for Dockerfile.ci changes covers the entire push range and correctly sets
df_changed_master. Ensure you reference the df_check step and the Dockerfile.ci
path when updating the command.
- Around line 49-57: The doc-only detection step (name "Detect doc-only
changes", id "paths") always uses origin/master...HEAD which is empty on push
events; update the script to choose the diff range based on the event: if the
GitHub event is a push use the push range (github.event.before → github.sha) and
otherwise use origin/master...HEAD, then compute changed from that range and
write docs_only to GITHUB_OUTPUT; implement this by setting base/head at the top
of the run block (using the workflow expressions for github.event.before and
github.sha for pushes) and then using git diff "$base...$head" to populate the
changed variable.
- Line 38: Replace all mutable action tags in the workflow (e.g.,
"actions/checkout@v6", "docker/login-action@v4", "docker/build-push-action@v7",
"coverallsapp/github-action@v2" and every other "uses:" entry noted in the
comment) with their corresponding immutable commit SHAs: look up the specific
action repositories (actions/checkout, docker/login-action,
docker/build-push-action, coverallsapp/github-action, etc.), find the reviewed
commit SHA you want to pin, and change each "uses: <repo>@<tag>" to "uses:
<repo>@<commit-sha>" so CI references a fixed revision; update the workflow
entries at the same positions as the reported examples to ensure all occurrences
are pinned and include a short comment with the chosen SHA for future
maintenance.
- Around line 19-22: Remove the top-level workflow permission entry
"pull-requests: write" from the permissions block (the key shown as
pull-requests: write) in the CI workflow; keep only the minimal scopes required
(contents: read, packages: read) and rely on any job-level overrides such as
packages: write where needed so the workflow no longer grants unnecessary PR
write permissions at workflow-level.
In `@Dockerfile.ci`:
- Around line 29-40: The Dockerfile.ci RUN block that downloads and extracts
geckodriver (using ARG GECKODRIVER_VERSION and GECKO_ARCH) must verify the
artifact before extraction: modify the RUN steps that curl the geckodriver
tarball to also fetch the corresponding .asc or .sha256 (e.g.,
geckodriver-v${GECKODRIVER_VERSION}-${GECKO_ARCH}.tar.gz.asc or .sha256),
import/ensure the Mozilla GPG key (if using .asc) and run gpg --verify against
the tarball, or compute a SHA256 and compare it to a pinned value, and only then
tar -xz and install /usr/local/bin/geckodriver; fail the build if verification
fails. Ensure the verification steps reference the same GECKODRIVER_VERSION and
GECKO_ARCH variables so they remain deterministic.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 3634f69a-901d-483b-b6e8-15fd21d66fbe
📒 Files selected for processing (2)
.github/workflows/ci.ymlDockerfile.ci
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-02-24T16:24:55.443Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1233
File: .github/workflows/backport.yml:22-22
Timestamp: 2026-02-24T16:24:55.443Z
Learning: In repositories within the OpenWISP organization, it is acceptable to reference reusable workflows from other OpenWISP-controlled repos using mutable refs (e.g., master) in .github/workflows. This is permissible due to the shared trust boundary within the organization. If applying this pattern, ensure the target repos are under the same organization and maintain awareness of potential breakages from upstream mutable refs; consider pinning to a tagged version for longer-term stability when appropriate.
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2026-02-24T16:25:20.080Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1233
File: .github/workflows/backport.yml:35-35
Timestamp: 2026-02-24T16:25:20.080Z
Learning: In .github/workflows/backport.yml, enforce that backport-on-comment triggers only for users with author_association MEMBE R or OWNER (COLLABORATOR excluded), reflecting maintainer feedback. Update the trigger condition to check author_association and restrict to MEMBERS/OWNERS; document rationale and PR `#1233` reference in code comments.
Applied to files:
.github/workflows/ci.yml
🪛 Checkov (3.2.529)
Dockerfile.ci
[low] 1-51: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-51: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 Hadolint (2.14.0)
Dockerfile.ci
[warning] 9-9: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>
(DL3008)
🪛 Trivy (0.69.3)
Dockerfile.ci
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
[info] 1-1: No HEALTHCHECK defined
Add HEALTHCHECK instruction in your Dockerfile
Rule: DS-0026
(IaC/Dockerfile)
🪛 zizmor (1.25.2)
.github/workflows/ci.yml
[error] 22-22: overly broad permissions (excessive-permissions): pull-requests: write is overly broad at the workflow level
(excessive-permissions)
[error] 38-38: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 60-60: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 172-172: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 178-178: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 185-185: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 189-189: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 206-206: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 222-222: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 237-237: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 244-244: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 248-248: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 262-262: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 283-283: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 301-301: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[warning] 21-21: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[warning] 170-170: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[warning] 220-220: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[error] 278-278: unpinned image references (unpinned-images): container image may be unpinned
(unpinned-images)
[error] 326-326: unpinned image references (unpinned-images): container image may be unpinned
(unpinned-images)
[warning] 385-385: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 387-387: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 434-434: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 354-354: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 377-377: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 412-412: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 441-441: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 453-453: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 457-457: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 465-465: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 482-482: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
| permissions: | ||
| contents: read | ||
| packages: read | ||
| pull-requests: write |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
echo "== File: $FILE =="
ls -la "$FILE" || true
sed -n '1,120p' "$FILE" || true
echo
echo "== Search permissions usage =="
rg -n "permissions:|pull-requests:|pull_request|github-script|actions/github-script|gh pr|octokit|GITHUB_TOKEN" "$FILE" .github/workflows || true
echo
echo "== Extract effective permission blocks (top-level and jobs) =="
python3 - <<'PY'
import re, pathlib
path = pathlib.Path(".github/workflows/ci.yml")
txt = path.read_text()
# crude YAML scan for "permissions:" blocks and following indented lines
lines = txt.splitlines()
for i,l in enumerate(lines):
if re.match(r'^\s*permissions:\s*$', l):
# capture next 10 lines
block = "\n".join(lines[i:i+15])
print("\n--- permissions block at line", i+1, "---")
print(block)
PYRepository: openwisp/openwisp-controller
Length of output: 12510
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
echo "== Rerun targeted searches in $FILE =="
rg -n "actions/github-script|github-script|octokit|api\.github\.com|REST|gh pr|gh api|pulls\/|issues\/|comments|create.*comment|update.*comment|review|request.*review|create.*review|approve|label|milestone|merge" "$FILE" || true
echo
echo "== Show job-level permissions overrides (if any) =="
rg -n "^[[:space:]]+permissions:|^permissions:" "$FILE" || true
echo
echo "== Display sections around the two job-level permissions blocks =="
# line ranges around 160-200 and 210-260 from earlier scan
sed -n '140,210p' "$FILE"
echo "-----"
sed -n '210,280p' "$FILE"
echo "-----"
sed -n '280,380p' "$FILE"
echo "-----"
sed -n '380,520p' "$FILE"Repository: openwisp/openwisp-controller
Length of output: 13088
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
echo "== Search for GitHub PR-write related tooling =="
rg -n "actions/github-script|github-script|octokit|api\.github\.com|REST|/pulls/|/issues/|/comments/|create.*comment|update.*comment|request.*review|create.*review|approve|label|milestone|merge" "$FILE" || true
echo
echo "== Search for gh CLI PR usage =="
rg -n "(\bgh\b.*\bpr\b)|\bgh pr\b|\bgh issue\b|\bgh api\b" "$FILE" || true
echo
echo "== Search for direct GITHUB_TOKEN use in GitHub API contexts =="
rg -n "GITHUB_TOKEN|secrets\.GITHUB_TOKEN" "$FILE" || true
echo
echo "== Show any 'with:' blocks mentioning pull requests number or API =="
rg -n "pull_request|pull-requests|issue\.number|pr\.number|PR_NUMBER|number.*pull" "$FILE" || trueRepository: openwisp/openwisp-controller
Length of output: 1156
🌐 Web query:
coverallsapp/github-action v2 required GitHub token permissions pull-requests: write
💡 Result:
You generally do NOT need to grant the GitHub Actions default GITHUB_TOKEN the permission pull-requests: write just to use coverallsapp/github-action@v2. What the Coveralls GitHub Action requires is its github-token input (defaulting to github.token / secrets.GITHUB_TOKEN) to authenticate with Coveralls and post Coveralls status updates/comments depending on configuration [1][2]. The Coveralls action’s own docs do not specify additional workflow permissions like pull-requests: write for that token; it only requires providing the github-token input [1][2]. However, if your setup is also posting PR comments directly from GitHub (for example, if you use a different “cover-2” style marketplace action that explicitly documents pull-requests: write), then pull-requests: write may be required by that other action, not by coverallsapp/github-action@v2 itself [3].
Citations:
- 1: https://github.com/coverallsapp/github-action/blob/main/action.yml
- 2: https://github.com/coverallsapp/github-action
- 3: https://github.com/marketplace/actions/cover-2
Remove workflow-level pull-requests: write permission
Top-level pull-requests: write isn’t used by this workflow: it only builds/pushes GHCR images and runs tests, plus coverallsapp/github-action@v2 uses secrets.GITHUB_TOKEN without requiring PR-write scope for its normal status reporting. Limit permissions to the minimal scopes needed (or rely on the existing job-level packages: write overrides).
🧰 Tools
🪛 zizmor (1.25.2)
[error] 22-22: overly broad permissions (excessive-permissions): pull-requests: write is overly broad at the workflow level
(excessive-permissions)
[warning] 21-21: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 19 - 22, Remove the top-level workflow
permission entry "pull-requests: write" from the permissions block (the key
shown as pull-requests: write) in the CI workflow; keep only the minimal scopes
required (contents: read, packages: read) and rely on any job-level overrides
such as packages: write where needed so the workflow no longer grants
unnecessary PR write permissions at workflow-level.
| image: ${{ steps.resolve.outputs.image }} | ||
| docs_only: ${{ steps.paths.outputs.docs_only }} | ||
| steps: | ||
| - uses: actions/checkout@v6 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
# Ensure file exists
ls -la "$FILE"
# Show relevant uses lines with line numbers
rg -n "^\s*-\s*uses:" "$FILE" || true
# Also show any uses lines for docker/buildx or caching etc (broader)
rg -n "uses:" "$FILE" | head -n 200Repository: openwisp/openwisp-controller
Length of output: 1422
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
ls -la "$FILE"
echo "=== uses entries ==="
rg -n "^\s*-\s*uses:" "$FILE" || true
echo
echo "=== summary of checkout version tags ==="
rg -n "actions/checkout@" "$FILE" || trueRepository: openwisp/openwisp-controller
Length of output: 706
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
if [ ! -f "$FILE" ]; then
echo "Missing $FILE"
exit 1
fi
echo "=== All uses lines with line numbers (first 250 matches) ==="
rg -n "^\s*-\s*uses:" "$FILE" | head -n 250
echo
echo "=== Check whether any uses entries are pinned to SHAs ==="
# Heuristic: SHA is 40+ hex characters optionally prefixed by v?
rg -n "^\s*-\s*uses:\s*[^@]+@([0-9a-f]{40,})\s*$" "$FILE" || true
echo
echo "=== Check for checkout specifically ==="
rg -n "actions/checkout@" "$FILE" || true
echo
echo "=== Show surrounding context for line ~38 ==="
nl -ba "$FILE" | sed -n '30,50p'Repository: openwisp/openwisp-controller
Length of output: 838
Pin uses: action references to commit SHAs in .github/workflows/ci.yml.
This workflow uses mutable action tags (e.g., actions/checkout@v6, docker/login-action@v4, docker/build-push-action@v7, coverallsapp/github-action@v2) so an upstream retag/compromise could affect CI. Pin each uses: entry to a specific reviewed commit SHA and update intentionally. Also applies to: 60-60, 172-172, 178-178, 185-185, 189-189, 206-206, 222-222, 237-237, 244-244, 248-248, 262-262, 283-283, 301-301, 354-354, 377-377, 412-412, 441-441, 453-453, 457-457, 465-465, 482-482, 495-495
🧰 Tools
🪛 zizmor (1.25.2)
[error] 38-38: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml at line 38, Replace all mutable action tags in the
workflow (e.g., "actions/checkout@v6", "docker/login-action@v4",
"docker/build-push-action@v7", "coverallsapp/github-action@v2" and every other
"uses:" entry noted in the comment) with their corresponding immutable commit
SHAs: look up the specific action repositories (actions/checkout,
docker/login-action, docker/build-push-action, coverallsapp/github-action,
etc.), find the reviewed commit SHA you want to pin, and change each "uses:
<repo>@<tag>" to "uses: <repo>@<commit-sha>" so CI references a fixed revision;
update the workflow entries at the same positions as the reported examples to
ensure all occurrences are pinned and include a short comment with the chosen
SHA for future maintenance.
| BRANCH_REF: ${{ github.head_ref || github.ref_name }} | ||
| EVENT_NAME: ${{ github.event_name }} | ||
| HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} | ||
| run: | | ||
| BRANCH_TAG="branch-$(printf '%s' "$BRANCH_REF" | sha256sum | cut -c1-12)" | ||
| IMAGE="ghcr.io/${GITHUB_REPOSITORY,,}/ci-base" | ||
| CATTHEHACKER="ghcr.io/catthehacker/ubuntu:act-24.04" |
There was a problem hiding this comment.
Branch image reuse can serve stale contents after a force-push or rebase.
BRANCH_TAG only depends on the branch name, and the reuse check only looks at HEAD^..HEAD. If the branch history is rewritten or an earlier commit changes Dockerfile.ci or the requirements files, Line 131 can reuse an image built from old inputs. Key the tag off github.sha or a digest of the build inputs instead.
Also applies to: 128-137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 69 - 75, BRANCH_TAG currently only
depends on branch name which can yield stale images after force-push; update the
BRANCH_TAG generation (and any image reuse checks that reference it) to include
a stable build-specific identifier such as GITHUB_SHA or a digest of build
inputs (e.g., sha256 of Dockerfile.ci plus requirements files) so the tag
changes when inputs change; modify the BRANCH_TAG assignment (and IMAGE usage
that references it) to append or replace the branch-hash with either the
github.sha or the computed inputs-digest and ensure the image existence/reuse
check uses that new tag so rebuilt images aren’t incorrectly reused after
history rewrites.
When a PR is opened before any `:stable` image exists in upstream GHCR, `resolve-image` now falls back to building from scratch instead of using catthehacker. Fork PRs now check only `ghcr.io/openwisp/openwisp-controller/ci-base`. Also fixes doc-only detection on push events, df_check for multi-commit pushes, and adds geckodriver SHA256 verification in Dockerfile.ci.
|
The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (1/3). |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 371-374: The checkout step using actions/checkout@v6 sets ref to
"${{ github.event.pull_request.head.sha }}" with no fallback, which breaks for
push events; update the ref expression to include the fallback "${{
github.event.pull_request.head.sha || github.sha }}" (matching other checkout
steps) so push workflows use github.sha when pull_request.head.sha is empty;
modify the step that references actions/checkout@v6 and its ref property
accordingly.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: c62ec557-6375-4165-b51c-3b0b1ef541ab
📒 Files selected for processing (2)
.github/workflows/ci.ymlDockerfile.ci
📜 Review details
⏰ 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: Build CI base image (branch)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-02-24T16:24:55.443Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1233
File: .github/workflows/backport.yml:22-22
Timestamp: 2026-02-24T16:24:55.443Z
Learning: In repositories within the OpenWISP organization, it is acceptable to reference reusable workflows from other OpenWISP-controlled repos using mutable refs (e.g., master) in .github/workflows. This is permissible due to the shared trust boundary within the organization. If applying this pattern, ensure the target repos are under the same organization and maintain awareness of potential breakages from upstream mutable refs; consider pinning to a tagged version for longer-term stability when appropriate.
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2026-02-24T16:25:20.080Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1233
File: .github/workflows/backport.yml:35-35
Timestamp: 2026-02-24T16:25:20.080Z
Learning: In .github/workflows/backport.yml, enforce that backport-on-comment triggers only for users with author_association MEMBE R or OWNER (COLLABORATOR excluded), reflecting maintainer feedback. Update the trigger condition to check author_association and restrict to MEMBERS/OWNERS; document rationale and PR `#1233` reference in code comments.
Applied to files:
.github/workflows/ci.yml
🔇 Additional comments (12)
.github/workflows/ci.yml (8)
19-22:pull-requests: writepermission appears unnecessary.This permission was flagged in a prior review. The workflow builds/pushes GHCR images and runs tests;
coverallsapp/github-action@v2does not require PR-write scope for status reporting. Consider removing to follow least-privilege.
38-38: Pin action references to commit SHAs.Using mutable tags like
actions/checkout@v6anddocker/login-action@v4was flagged previously. Pinning to reviewed SHAs prevents supply-chain risks from upstream retags.Also applies to: 68-68
145-146: Branch image reuse can serve stale contents after force-push.This was flagged previously. The check
git diff --quiet HEAD^ HEAD -- ...only looks at the last commit. If the branch is rebased or force-pushed with earlier commits changingDockerfile.ci, a stale image may be reused. Consider keying the tag offgithub.shaor a digest of build inputs.
49-65: LGTM!The doc-only detection now correctly uses
BEFORE_SHAandAFTER_SHAfor push events, addressing the prior concern aboutorigin/master...HEADbeing empty on pushes to master.
241-251: LGTM!The
df_checkstep now usesBEFORE_SHAfromgithub.event.beforeto compute the diff range, addressing the prior concern about multi-commit pushes missingDockerfile.cichanges.
285-330: LGTM!
332-462: LGTM!The test job matrix with test groups, coverage handling, and geckodriver log upload on failure looks well structured.
464-515: LGTM!Coverage combine and Coveralls finalization jobs correctly handle artifact download, combination, and upload with appropriate error handling.
Dockerfile.ci (4)
1-5: LGTM!
7-27: LGTM!
29-46: LGTM!The geckodriver installation now correctly downloads and verifies the SHA256 checksum before extraction, addressing the prior integrity verification concern.
48-57: LGTM!
| - uses: actions/checkout@v6 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
| ref: ${{ github.event.pull_request.head.sha}} | ||
| persist-credentials: false |
There was a problem hiding this comment.
Missing || github.sha fallback for push events.
Line 373 uses ${{ github.event.pull_request.head.sha}} without a fallback. For push events (e.g., to master), this evaluates to an empty string, potentially causing checkout to use an unintended ref or fail. Other checkout steps in this workflow (e.g., Line 40) correctly include the fallback.
🔧 Proposed fix
- uses: actions/checkout@v6
with:
- ref: ${{ github.event.pull_request.head.sha}}
+ ref: ${{ github.event.pull_request.head.sha || github.sha }}
persist-credentials: false🧰 Tools
🪛 zizmor (1.25.2)
[error] 371-371: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 371 - 374, The checkout step using
actions/checkout@v6 sets ref to "${{ github.event.pull_request.head.sha }}" with
no fallback, which breaks for push events; update the ref expression to include
the fallback "${{ github.event.pull_request.head.sha || github.sha }}" (matching
other checkout steps) so push workflows use github.sha when
pull_request.head.sha is empty; modify the step that references
actions/checkout@v6 and its ref property accordingly.
|
The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (2/3). |
|
The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (3/3). |
Docker Build FailureHello @CodingWithSaksham, The CI build failed because the Docker image build process encountered an error. The specific error message indicates a failure during the execution of a shell command within the Dockerfile. The command attempts to download and verify a Error: Reason: Fix: |
|
Actionable comments posted: 0 |
|
The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (1/3). |
|
The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (2/3). |
|
The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (3/3). |
pandafy
left a comment
There was a problem hiding this comment.
@CodingWithSaksham thanks for picking up this issue but I am not a fan of this solution. If you look at previous CI runs for master branch, you will notice that the major chunk of time is not spent on setting up the CI environment, but it is spent on running tests:
Source https://github.com/openwisp/openwisp-controller/actions/runs/26530301877/job/78300909980
Moreover, if we want to create an image with pre-installed software, then
- This should be generalized such that we can use it in all OpenWISP repositories
- We will need to create a separate repository
- Maintenance of this image will be a major undertaking. If the dependencies break, then CI in all OpenWISP repo will break.
Before going ahead on this, I recommend opening a discussion for this at https://github.com/orgs/openwisp/discussions
At the current state, this PR adds more work than it solves.
CI currently installs all system dependencies from run ,GDAL, SpatiaLite, Prettier, browsers, and Redis via apt-get and npm, adding several minutes of pure setup overhead before a single test runs. This introduces a pre-built Docker image (Dockerfile.ci) cached in GHCR that ships all dependencies pre-installed, so jobs start immediately without any package installation. The image is rebuilt only when Dockerfile.ci or requirements files change; all other pushes reuse the cached image. Tests are also split into three parallel matrix groups (standard, selenium,sample_app) to reduce wall-clock time further, and QA checks now run in the same container so the entire pipeline benefits from the cache.
Checklist
Reference to Existing Issue
Closes #1317
Description of Changes
node:lts-bookworm-slimwith all CI dependencies baked in: GDAL/SpatiaLite, Prettier, Firefox-ESR, Chromium, geckodriver, and Redis.github/workflows/ci.yml:stableimage otherwise, or a community fallback image for fork PRsthe existing cached image
:stabletag automaticallysetup-uvinstead ofsetup-pythondue to dependency issues and overall better performance ofuv