Skip to content

[ci] Speed up CI by splitting tests into multiple groups and pre-baking a Docker image #1317#1381

Draft
CodingWithSaksham wants to merge 6 commits into
openwisp:masterfrom
CodingWithSaksham:issues/1317-speedup-ci
Draft

[ci] Speed up CI by splitting tests into multiple groups and pre-baking a Docker image #1317#1381
CodingWithSaksham wants to merge 6 commits into
openwisp:masterfrom
CodingWithSaksham:issues/1317-speedup-ci

Conversation

@CodingWithSaksham
Copy link
Copy Markdown

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

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Reference to Existing Issue

Closes #1317

Description of Changes

  1. Dockerfile.ci
  • Introduces a pre-built Docker image based on node:lts-bookworm-slim with all CI dependencies baked in: GDAL/SpatiaLite, Prettier, Firefox-ESR, Chromium, geckodriver, and Redis
  • Eliminates the per-run apt-get and npm install overhead that added several minutes to every job
  • Includes build-time version checks to catch missing binaries before the image is ever pushed
  1. .github/workflows/ci.yml
  • Adds a resolve-image job that picks the right image for each run: a branch-specific build when Dockerfile.ci or requirements files changed, the cached :stable image otherwise, or a community fallback image for fork PRs
  • The image is only rebuilt when something actually changed; subsequent pushes on the same branch reuse
    the existing cached image
  • On master merges, a promote-stable job updates the :stable tag automatically
  • Tests are now split into three parallel groups (standard, selenium, sample_app) instead of running sequentially, which cuts the overall wall-clock time
  • QA checks run in their own job inside the same pre-built container
  • Geckodriver logs are uploaded as artifacts on both failure and cancellation for easier debugging
  • Doc-only pushes skip the test matrix but still run QA
  • A 20-minute job timeout prevents hung browser tests from blocking the queue
  • Use setup-uv instead of setup-python due to dependency issues and overall better performance of uv

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
@CodingWithSaksham CodingWithSaksham changed the title Speed up CI by splitting tests into multiple groups and pre-baking a Docker image #1317 [ci] Speed up CI by splitting tests into multiple groups and pre-baking a Docker image #1317 May 28, 2026
@openwisp-companion
Copy link
Copy Markdown

The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (1/3).

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4fe65114-33d3-42df-b4d5-1590aacb4666

📥 Commits

Reviewing files that changed from the base of the PR and between bbe4a1a and 92c9822.

📒 Files selected for processing (1)
  • Dockerfile.ci
📜 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)
  • GitHub Check: Build CI base image (branch)
🧰 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)
Dockerfile.ci (1)

4-27: LGTM!

Also applies to: 30-47, 49-56


📝 Walkthrough

Walkthrough

Refactors 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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows the required [ci] prefix format and clearly describes the main changes: speeding up CI through test splitting and Docker image pre-baking, with the issue reference #1317.
Description check ✅ Passed The description covers all major sections: problem statement, objectives of the changes, checklist items, reference to issue #1317, and detailed change breakdown following the template structure.
Linked Issues check ✅ Passed The PR comprehensively addresses issue #1317's objectives: eliminates per-run install overhead via pre-baked Docker image, caches dependencies, implements test parallelism across three matrix groups, rebuilds only on relevant changes, and improves workflow reliability.
Out of Scope Changes check ✅ Passed All changes directly support the stated goal of speeding up CI: Dockerfile.ci introduces the pre-built image, ci.yml implements the resolve/promote workflow and test matrix splitting, with no unrelated modifications.
Bug Fixes ✅ Passed PR is CI infrastructure (GitHub Actions + Docker), not user-facing. Falls under explicit exception for GitHub Actions workflow fixes where regression tests are impractical.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openwisp-companion
Copy link
Copy Markdown

The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (2/3).

@openwisp-companion
Copy link
Copy Markdown

The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (3/3).

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between bcc688f and 3009d0e.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • Dockerfile.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

Learn more

(IaC/Dockerfile)


[info] 1-1: No HEALTHCHECK defined

Add HEALTHCHECK instruction in your Dockerfile

Rule: DS-0026

Learn more

(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)

Comment thread .github/workflows/ci.yml
Comment on lines +19 to +22
permissions:
contents: read
packages: read
pull-requests: write
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 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)
PY

Repository: 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" || true

Repository: 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:


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.

Comment thread .github/workflows/ci.yml
image: ${{ steps.resolve.outputs.image }}
docs_only: ${{ steps.paths.outputs.docs_only }}
steps:
- uses: actions/checkout@v6
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 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 200

Repository: 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" || true

Repository: 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.

Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/ci.yml Outdated
Comment on lines +69 to +75
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"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

Comment thread .github/workflows/ci.yml
Comment thread Dockerfile.ci
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.
@openwisp-companion
Copy link
Copy Markdown

The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (1/3).

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 3009d0e and bbe4a1a.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • Dockerfile.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: write permission appears unnecessary.

This permission was flagged in a prior review. The workflow builds/pushes GHCR images and runs tests; coverallsapp/github-action@v2 does 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@v6 and docker/login-action@v4 was 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 changing Dockerfile.ci, a stale image may be reused. Consider keying the tag off github.sha or a digest of build inputs.


49-65: LGTM!

The doc-only detection now correctly uses BEFORE_SHA and AFTER_SHA for push events, addressing the prior concern about origin/master...HEAD being empty on pushes to master.


241-251: LGTM!

The df_check step now uses BEFORE_SHA from github.event.before to compute the diff range, addressing the prior concern about multi-commit pushes missing Dockerfile.ci changes.


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!

Comment thread .github/workflows/ci.yml
Comment on lines 371 to +374
- uses: actions/checkout@v6
with:
ref: ${{ github.event.pull_request.head.sha }}
ref: ${{ github.event.pull_request.head.sha}}
persist-credentials: false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

@openwisp-companion
Copy link
Copy Markdown

The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (2/3).

@openwisp-companion
Copy link
Copy Markdown

The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (3/3).

@openwisp-companion
Copy link
Copy Markdown

Docker Build Failure

Hello @CodingWithSaksham,
(Analysis for commit bbe4a1a)

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 geckodriver binary, and it exited with a non-zero status code (1), signaling an error.

Error:
buildx failed with: ERROR: failed to build: failed to solve: process "/bin/sh -c ARCH=\"$(dpkg --print-architecture)\" && case \"$ARCH\" in amd64) GECKO_ARCH=linux64 ;; arm64) GECKO_ARCH=linux-aarch64 ;; *) echo \"Unsupported arch $ARCH\" >&2; exit 1 ;; esac && BASE_URL=\"https://github.com/mozilla/geckodriver/releases/download/v${GECKODRIVER_VERSION}\" && curl -fsSL \"${BASE_URL}/geckodriver-v${GECKODRIVER_VERSION}-${GECKO_ARCH}.tar.gz\" -o /tmp/geckodriver.tar.gz && curl -fsSL \"${BASE_URL}/geckodriver-v${GECKODRIVER_VERSION}-${GECKO_ARCH}.tar.gz.sha256\" -o /tmp/geckodriver.sha256 && EXPECTED=\"$(awk '{print $1}' /tmp/geckodriver.sha256)\" && ACTUAL=\"$(sha256sum /tmp/geckodriver.tar.gz | awk '{print $1}')\" && [ \"$EXPECTED\" = \"$ACTUAL\" ] || { echo \"geckodriver SHA256 mismatch\" >&2; exit 1; } && tar -xz -C /usr/local/bin geckodriver -f /tmp/geckodriver.tar.gz && rm /tmp/geckodriver.tar.gz /tmp/geckodriver.sha256 && chmod +x /usr/local/bin/geckodriver" did not complete successfully: exit code: 1

Reason:
The most likely cause is a network issue preventing the curl commands from downloading the geckodriver files, or a problem with the GitHub release URL or the integrity check (SHA256 mismatch).

Fix:
This is likely a transient issue. Please re-run the CI pipeline. If the issue persists, it might indicate a problem with the GECKODRIVER_VERSION variable or the availability of the specified geckodriver release on GitHub. You may need to check the Dockerfile for any outdated versions or network-related configurations.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

@openwisp-companion
Copy link
Copy Markdown

The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (1/3).

@openwisp-companion
Copy link
Copy Markdown

The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (2/3).

@openwisp-companion
Copy link
Copy Markdown

The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (3/3).

Copy link
Copy Markdown
Member

@pandafy pandafy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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:

Image

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

  1. This should be generalized such that we can use it in all OpenWISP repositories
  2. We will need to create a separate repository
  3. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[chores] Investigate and brainstorm ways to speed up CI

2 participants