[change] Migrated OpenWISP modules to the 1.3 pre-release line#625
[change] Migrated OpenWISP modules to the 1.3 pre-release line#625CodingWithSaksham wants to merge 1 commit into
Conversation
Repointed all OpenWISP app sources in images/openwisp_base/Dockerfile to the shared 1.3 branch (monitoring, firmware, topology, radius, controller, notifications, users) so the interdependent core moves as one unit (utils/ipam resolve transitively). Temporary until the 1.3 line is released. Includes the two dashboard fixes required to run on 1.3: - urls.py: register the firmware-upgrader "upgrader" URL namespace; the 1.3 device-admin inline reverses upgrader:..., else the device change page 500s. - load_init_data.py: dedup the "SSH Keys" template by name. controller 1.3 made config a JSONB field, so config__contains no longer substring-matches and the old guard caused a duplicate-insert crash loop.
📝 WalkthroughWalkthroughThis PR updates the OpenWISP Docker image build process and dashboard configuration. The Dockerfile now splits system package installation into separate layers and migrates core OpenWISP module sources from PyPI version constraints to GitHub tarball URLs (v1.3), with custom modules (controller, notifications, users) now defaulting to GitHub sources instead of requiring manual ARG overrides. The dashboard load_init_data.py changes SSH key template detection to use name and default flag matching instead of config content inspection. Finally, the dashboard URLs are updated to include firmware upgrader routes alongside private-storage-backed firmware routes. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ 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 |
| apt-get install --yes --no-install-recommends \ | ||
| libcairo2 apt-utils libpangocairo-1.0-0 \ | ||
| gdal-bin gettext fping openssh-client && \ | ||
| rm -rf /var/lib/apt/lists/* /root/.cache/pip/* /tmp/* |
There was a problem hiding this comment.
Changes done by hadolint, Will have to keep. Same with R15-17, 20, 22, , 52-54, 57-59, 62-64, 72-73, 102-178
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@images/openwisp_base/Dockerfile`:
- Around line 33-44: The Dockerfile currently installs OpenWISP packages using
moving branch tarball URLs via ARGs (OPENWISP_MONITORING_SOURCE,
OPENWISP_FIRMWARE_SOURCE, OPENWISP_TOPOLOGY_SOURCE, OPENWISP_RADIUS_SOURCE);
update each ARG value to point to a fixed release tag or commit SHA tarball URL
(or a stable release archive URL) instead of refs/heads/1.3 to make builds
reproducible, and update any corresponding build docs or CI variables that
override these ARGs so they supply the pinned tag/SHA.
In `@images/openwisp_dashboard/load_init_data.py`:
- Around line 170-172: The current check only returns an existing Template when
name="SSH Keys" AND default=True, which allows duplicates; change the logic in
load_init_data.py where you search Templates so you first look up
Template.objects.filter(name="SSH Keys").first(), and if found set its
default=True (save it) and return it instead of creating a new one; only proceed
to create a new Template if no Template with name="SSH Keys" exists. Update
references to the Template model access in that function 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: 3ab72fcf-f390-451c-8fa0-e958559a010b
📒 Files selected for processing (3)
images/openwisp_base/Dockerfileimages/openwisp_dashboard/load_init_data.pyimages/openwisp_dashboard/urls.py
📜 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: CI Build
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{py,html}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
images/openwisp_dashboard/load_init_data.pyimages/openwisp_dashboard/urls.py
🪛 Checkov (3.2.529)
images/openwisp_base/Dockerfile
[low] 1-178: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
🪛 Trivy (0.69.3)
images/openwisp_base/Dockerfile
[error] 101-178: Secrets passed via build-args or envs or copied secret files
Possible exposure of secret env "DJANGO_SECRET_KEY" in ENV
Rule: DS-0031
(IaC/Dockerfile)
[error] 101-178: Secrets passed via build-args or envs or copied secret files
Possible exposure of secret env "EMAIL_HOST_PASSWORD" in ENV
Rule: DS-0031
(IaC/Dockerfile)
[error] 101-178: Secrets passed via build-args or envs or copied secret files
Possible exposure of secret env "SSH_PRIVATE_KEY_PATH" in ENV
Rule: DS-0031
(IaC/Dockerfile)
🔇 Additional comments (4)
images/openwisp_base/Dockerfile (3)
19-24: LGTM!
54-68: LGTM!
101-178: LGTM!images/openwisp_dashboard/urls.py (1)
40-44: Check potential URL resolution conflicts for firmware includes
images/openwisp_dashboard/urls.pymounts bothopenwisp_firmware_upgrader.urlsandopenwisp_firmware_upgrader.private_storage.urlsatpath("", ...); if they define overlapping root-level patterns (e.g.,serve_private_file), the first include will win and the private-storage routes could be shadowed.- This repository does not include the
openwisp_firmware_upgraderURL modules (imports fail here), so the actual pattern overlap can’t be assessed locally; inspect the upstreamopenwisp_firmware_upgrader.urlsvsopenwisp_firmware_upgrader.private_storage.urlsto ensure they don’t collide (or that ordering is intentionally safe).
| ARG OPENWISP_MONITORING_SOURCE="https://github.com/openwisp/openwisp-monitoring/archive/refs/heads/1.3.tar.gz" | ||
| # hadolint ignore=DL3013 | ||
| RUN pip install --no-cache-dir --user --upgrade ${OPENWISP_MONITORING_SOURCE} | ||
| ARG OPENWISP_FIRMWARE_SOURCE="openwisp-firmware-upgrader~=1.2.0" | ||
| ARG OPENWISP_FIRMWARE_SOURCE="https://github.com/openwisp/openwisp-firmware-upgrader/archive/refs/heads/1.3.tar.gz" | ||
| # hadolint ignore=DL3013 | ||
| RUN pip install --no-cache-dir --user --upgrade ${OPENWISP_FIRMWARE_SOURCE} | ||
| ARG OPENWISP_TOPOLOGY_SOURCE="openwisp-network-topology~=1.2.0" | ||
| ARG OPENWISP_TOPOLOGY_SOURCE="https://github.com/openwisp/openwisp-network-topology/archive/refs/heads/1.3.tar.gz" | ||
| # hadolint ignore=DL3013 | ||
| RUN pip install --no-cache-dir --user --upgrade ${OPENWISP_TOPOLOGY_SOURCE} | ||
| ARG OPENWISP_RADIUS_SOURCE="openwisp-radius~=1.2.0" | ||
| ARG OPENWISP_RADIUS_SOURCE="https://github.com/openwisp/openwisp-radius/archive/refs/heads/1.3.tar.gz" | ||
| # hadolint ignore=DL3013 | ||
| RUN pip install --no-cache-dir --user --upgrade ${OPENWISP_RADIUS_SOURCE} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify GitHub tarball URLs for OpenWISP 1.3 branches are accessible
urls=(
"https://github.com/openwisp/openwisp-monitoring/archive/refs/heads/1.3.tar.gz"
"https://github.com/openwisp/openwisp-firmware-upgrader/archive/refs/heads/1.3.tar.gz"
"https://github.com/openwisp/openwisp-network-topology/archive/refs/heads/1.3.tar.gz"
"https://github.com/openwisp/openwisp-radius/archive/refs/heads/1.3.tar.gz"
"https://github.com/openwisp/openwisp-controller/archive/refs/heads/1.3.tar.gz"
"https://github.com/openwisp/openwisp-notifications/archive/refs/heads/1.3.tar.gz"
"https://github.com/openwisp/openwisp-users/archive/refs/heads/1.3.tar.gz"
)
for url in "${urls[@]}"; do
echo "Checking: $url"
if curl --head --silent --fail "$url" > /dev/null 2>&1; then
echo "✓ Accessible"
else
echo "✗ Failed"
fi
doneRepository: openwisp/docker-openwisp
Length of output: 782
images/openwisp_base/Dockerfile: GitHub refs/heads/1.3 tarball URLs are reachable; consider pinning for reproducible builds
The refs/heads/1.3.tar.gz tarball URLs for the listed OpenWISP modules (including monitoring, firmware-upgrader, network-topology, radius, controller, notifications, users) are accessible, so the migration doesn’t appear to break due to missing branches. Still, using a moving heads/1.3 reference makes builds non-reproducible—pin to a specific tag or commit SHA for stability.
🤖 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 `@images/openwisp_base/Dockerfile` around lines 33 - 44, The Dockerfile
currently installs OpenWISP packages using moving branch tarball URLs via ARGs
(OPENWISP_MONITORING_SOURCE, OPENWISP_FIRMWARE_SOURCE, OPENWISP_TOPOLOGY_SOURCE,
OPENWISP_RADIUS_SOURCE); update each ARG value to point to a fixed release tag
or commit SHA tarball URL (or a stable release archive URL) instead of
refs/heads/1.3 to make builds reproducible, and update any corresponding build
docs or CI variables that override these ARGs so they supply the pinned tag/SHA.
| existing = Template.objects.filter(name="SSH Keys", default=True).first() | ||
| if existing: | ||
| return existing |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for migration documentation or tests covering template lookup changes
# Check for migration guides or upgrade documentation
fd -e md -e rst -e txt | xargs rg -l "SSH.{0,10}Keys.{0,20}template" 2>/dev/null
# Check for related test coverage
fd -g '*test*.py' | xargs rg -A 5 'create_ssh_key_template' 2>/dev/null
# Look for duplicate template handling in the codebase
rg -n "Template\.objects\.filter.*name.*SSH" --type pyRepository: openwisp/docker-openwisp
Length of output: 194
Prevent duplicate “SSH Keys” templates when upgrading existing deployments.
- The template reuse only matches
Template.objects.filter(name="SSH Keys", default=True).first(), so an existing template withname="SSH Keys"butdefault=False(or with a renamed template) can cause the code to take the creation path and end up with multiple “SSH Keys” templates. - No tests/docs addressing this edge case were found in the current repo scope.
🤖 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 `@images/openwisp_dashboard/load_init_data.py` around lines 170 - 172, The
current check only returns an existing Template when name="SSH Keys" AND
default=True, which allows duplicates; change the logic in load_init_data.py
where you search Templates so you first look up
Template.objects.filter(name="SSH Keys").first(), and if found set its
default=True (save it) and return it instead of creating a new one; only proceed
to create a new Template if no Template with name="SSH Keys" exists. Update
references to the Template model access in that function accordingly.
1 participant
Repointed all OpenWISP app sources in images/openwisp_base/Dockerfile to the shared 1.3 branch (monitoring, firmware, topology, radius, controller, notifications, users) so the interdependent core moves as one unit (utils/ipam resolve transitively). Temporary until the 1.3 line is released.
Checklist
Reference to Existing Issue
Related to #587
Description of Changes
load_init_data.py: dedup the "SSH Keys" template by name. controller 1.3 made config a JSONB field, soconfig__containsno longer substring-matches and the old guard caused a duplicate-insert crash loop.