[fix] add DOCKER_TAG variable for version pinning to match intended release version#555
[fix] add DOCKER_TAG variable for version pinning to match intended release version#555atif09 wants to merge 14 commits intoopenwisp:masterfrom
Conversation
…#554 this change introduces a DOCKER_TAG environment variable that allows users to pin specific image versions in .env file. Both 'docker compose pull' and 'make pull' now respect this variable, ensuring consistent version behavior across all deployment methods. Changes: - Add DOCKER_TAG=latest to .env file - Update all OpenWISP image tags in docker-compose.yml to use ${DOCKER_TAG:-latest} - Update Makefile to include .env and use DOCKER_TAG when retagging images Fixes openwisp#554
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughImage tag pinning was introduced and parameterized across the repository. Two environment variables were added: IMAGE_OWNER and OPENWISP_VERSION (default "edge") in .env. The Makefile now includes .env, exposes OPENWISP_VERSION, IMAGE_OWNER and a RELEASE_VERSION, and uses them for pull, build, publish, clean, and release flows. docker-compose.yml image references were changed to use ${OPENWISP_VERSION:-edge}. deploy/auto-install.sh now prompts "OpenWISP Version (leave blank for latest stable release)", writes OPENWISP_VERSION via set_env, and runs make start without passing TAG. Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Env as .env
participant Make as Makefile
participant Registry as Docker Registry
participant Compose as docker-compose
Dev->>Env: add/set IMAGE_OWNER & OPENWISP_VERSION
Dev->>Make: run make pull / build / publish / start
Make->>Env: include .env, read IMAGE_OWNER & OPENWISP_VERSION & RELEASE_VERSION
Make->>Registry: pull/build and tag IMAGE_OWNER/<image>:OPENWISP_VERSION
Make->>Registry: push IMAGE_OWNER/<image>:OPENWISP_VERSION
Dev->>Compose: docker compose pull / up
Compose->>Env: resolve ${OPENWISP_VERSION:-edge}
Compose->>Registry: pull IMAGE_OWNER/<image>:OPENWISP_VERSION
sequenceDiagram
participant User as Installer
participant Script as deploy/auto-install.sh
participant Env as .env
participant Make as Makefile
User->>Script: run installer (may leave version blank)
Script->>User: prompt "OpenWISP Version (leave blank for latest stable release)"
Script->>Env: set_env OPENWISP_VERSION = provided_or_latest
Script->>Make: run make start
Make->>Env: include .env and use OPENWISP_VERSION for start
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (1 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 |
![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
🤖 Fix all issues with AI agents
In @.env:
- Line 66: Move the DOCKER_TAG entry so it appears before EMAIL_DJANGO_DEFAULT
in the .env file to satisfy the dotenv-linter key ordering, and ensure the file
ends with a single trailing newline (add a blank line at EOF) so the linter no
longer reports a missing newline; update the DOCKER_TAG line (the literal key
"DOCKER_TAG") and verify EMAIL_DJANGO_DEFAULT remains unchanged.
In `@Makefile`:
- Line 26: The docker tag command can fail when DOCKER_TAG is empty; update the
recipe to use a fallback default for DOCKER_TAG (compose uses "latest") so the
tag is valid even if .env is missing. Locate the docker tag line in the Makefile
that references $${DOCKER_TAG} and replace it with a parameter expansion
fallback (e.g., use $${DOCKER_TAG:-latest}) so the command uses "latest" when
DOCKER_TAG is unset while keeping existing variables USER, image and TAG
unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.envMakefiledocker-compose.yml
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-06T11:56:48.600Z
Learnt from: pandafy
Repo: openwisp/docker-openwisp PR: 549
File: docker-compose.yml:85-85
Timestamp: 2026-01-06T11:56:48.600Z
Learning: In docker-openwisp projects, ensure CELERY_SERVICE_NETWORK_MODE is set to an empty string "" (which Docker Compose treats as unset/null). This allows containers to connect via the Compose default network with correct service name DNS resolution. Using "bridge" as the value disables service name resolution and breaks communication between celery, dashboard, postgres, and redis. Apply this guideline to docker-compose.yml files in the repository and any similar Compose files where CELERY services rely on service name DNS.
Applied to files:
docker-compose.yml
📚 Learning: 2026-01-06T11:55:13.672Z
Learnt from: pandafy
Repo: openwisp/docker-openwisp PR: 549
File: docker-compose.yml:85-85
Timestamp: 2026-01-06T11:55:13.672Z
Learning: In docker-compose.yml files, avoid leaving CELERY_SERVICE_NETWORK_MODE as an empty string; an empty value is ignored and Docker Compose falls back to default networking. Do not rely on setting CELERY_SERVICE_NETWORK_MODE to 'bridge' for celery services, as this will not affect their networking. If specific networking is required for celery services, define and attach explicit networks in the compose file and reference them on the celery services.
Applied to files:
docker-compose.yml
📚 Learning: 2026-01-06T11:28:41.789Z
Learnt from: pandafy
Repo: openwisp/docker-openwisp PR: 549
File: docker-compose.yml:171-171
Timestamp: 2026-01-06T11:28:41.789Z
Learning: In docker-openwisp, the OpenVPN container (images/common/init_command.sh) exits silently with exit code 0 when VPN_DOMAIN environment variable is not set. Combined with restart policy "on-failure" in docker-compose.yml, this prevents the container from restarting when VPN is disabled.
Applied to files:
docker-compose.yml
🪛 dotenv-linter (4.0.0)
.env
[warning] 66-66: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 66-66: [UnorderedKey] The DOCKER_TAG key should go before the EMAIL_DJANGO_DEFAULT key
(UnorderedKey)
⏰ 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 comments (2)
Makefile (1)
4-6: LGTM — .env inclusion/export enables tag propagation.docker-compose.yml (1)
15-15: LGTM — parameterized tags meet pinning objective and keep defaults.Also applies to: 38-38, 58-58, 73-73, 88-88, 102-102, 116-116, 143-143, 159-159, 170-170
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Move DOCKER_TAG variable placement in .env file to satisfy alphabetical ordering requirements and add missing EOF newline. Update Makefile to use fallback value for DOCKER_TAG variable. Related to openwisp#554
atif09
force-pushed
the
issues/554-fix-docker-tag
branch
from
f60f251 to
a4f27a0
Compare
nemesifier
changed the title
github-project-automation
bot
moved this from In progress
to Reviewer approved
in OpenWISP Priorities for next releases
github-project-automation
bot
moved this to In progress
in OpenWISP Priorities for next releases
github-project-automation
bot
moved this to To do (general)
in OpenWISP Contributor's Board
github-project-automation
bot
moved this from To do (general)
to In progress
in OpenWISP Contributor's Board
Keeping DOCKER_TAG=latest avoids manual updating with each release, users will have to change this to the specific version by explicitly setting it to the desired version Related to openwisp#554
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.env (1)
5-12:⚠️ Potential issue | 🟡 MinorFix dotenv-linter key ordering for
DOCKER_TAG.
dotenv-linterexpectsDOCKER_TAGto appear beforeSSH_PRIVATE_KEY_PATH. Please reorder to satisfy lint rules.♻️ Proposed fix
DASHBOARD_DOMAIN=dashboard.openwisp.org API_DOMAIN=api.openwisp.org +# Image tag pinning +DOCKER_TAG=latest # SSH Credentials Configurations SSH_PRIVATE_KEY_PATH=/home/openwisp/.ssh/id_ed25519 SSH_PUBLIC_KEY_PATH=/home/openwisp/.ssh/id_ed25519.pub VPN_DOMAIN=openvpn.openwisp.org -DOCKER_TAG=latest EMAIL_DJANGO_DEFAULT=example@example.org
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.env
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-06T11:28:41.789Z
Learnt from: pandafy
Repo: openwisp/docker-openwisp PR: 549
File: docker-compose.yml:171-171
Timestamp: 2026-01-06T11:28:41.789Z
Learning: In docker-openwisp, the OpenVPN container (images/common/init_command.sh) exits silently with exit code 0 when VPN_DOMAIN environment variable is not set. Combined with restart policy "on-failure" in docker-compose.yml, this prevents the container from restarting when VPN is disabled.
Applied to files:
.env
🪛 dotenv-linter (4.0.0)
.env
[warning] 11-11: [UnorderedKey] The DOCKER_TAG key should go before the SSH_PRIVATE_KEY_PATH key
(UnorderedKey)
⏰ 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
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
dotenv-linter expects DOCKER_TAG to appear before SSH_PRIVATE_KEY_PATH Related to openwisp#554
nemesifier
left a comment
nemesifier
left a comment
There was a problem hiding this comment.
BTW we have an OPENWISP_VERSION var in the makefile.
However, I ask @pandafy to review this.
The variable in the makefile cannot serve as single source of truth if the someones uses docker to manage the images (e.g. docker compose pull) which is a common flow. |
pandafy
left a comment
pandafy
left a comment
There was a problem hiding this comment.
This looks on the right track. But, we need to make sure that we have a single source of truth. IMO, we can use .env as single source of truth and make Makefile derive the version from there.
@nemesifier is there something I am missing here?
github-project-automation
bot
moved this from Reviewer approved
to In progress
in OpenWISP Priorities for next releases
pandafy
left a comment
pandafy
left a comment
There was a problem hiding this comment.
@nemesifier don't merge this PR yet. i need to organise my thoughts and think about all possible situations.
I will report back on Monday.
pandafy
left a comment
pandafy
left a comment
There was a problem hiding this comment.
The proposal is to introduce an OPENWISP_VERSION variable in the .env file, defaulting to edge, and update docker-compose.yml to reference images using openwisp/:${OPENWISP_VERSION}.
This approach allows the same configuration to support both local development builds and versioned, registry-based images, creating a clear distinction between development flexibility and production stability.
Effects on development flow
The development workflow remains essentially unchanged. Developers can continue using Makefile or docker compose commands directly. The default configuration will always use edge images. This makes working on new changes easy.
Effects on the release flow
edge images
edge images are published on every push to the master branch. This behavior is driven by the CI workflow and is unaffected by the proposed changes, so the edge release flow will remain exactly as it is today.
Versioned images (e.g. 25.10.2)
Currently, versioned images use the OPENWISP_VERSION value defined in the Makefile. With this proposal, OPENWISP_VERSION moves to .env and defaults to edge, where it becomes a runtime configuration rather than a release signal. As a result, it can no longer serve as the source of truth for release versioning.
To preserve a declarative and easy-to-maintain release process, we need a separate variable in the Makefile that explicitly defines the release version. This keeps the release version documented, version-controlled, and independent of runtime configuration.
While it would be possible to derive the version automatically from the latest Git tag, a declarative Makefile variable is preferred for clarity, predictability, and maintainability.
Effects on deployment
The auto-install script writes the user-selected version into the .env file during setup (defaulting to latest). As a result, deployments will use the latest stable images (e.g. 25.10). Alternatively, the user can enter the version number. This guarantees safer upgrades and predictable, reproducible installations without adding complexity to the development workflow.
If a user clones the docker-openwisp repository and spins up OpenWISP without using the auto-install script, then they will end up with the edge images (development version of OpenWISP).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deploy/auto-install.sh (1)
61-66:⚠️ Potential issue | 🔴 CriticalBlank input defaults to
latestgit branch, which doesn't exist and breaks installation.When users leave the version blank,
get_version_from_user()setsopenwisp_version="latest", which is then passed todownload_docker_openwisp(). This function setsGIT_BRANCH="latest"and attemptsgit clone --branch latest, which fails because the repository has nolatestbranch (only version branches like24.11,25.10, andmaster).The fix is to treat
"latest"the same as"edge"and map it to themasterbranch:Suggested fix
- if [[ "$openwisp_version" == "edge" ]]; then + if [[ "$openwisp_version" == "edge" || "$openwisp_version" == "latest" ]]; then GIT_BRANCH="master" else GIT_BRANCH="$openwisp_version" fi
🤖 Fix all issues with AI agents
In `@Makefile`:
- Around line 25-26: The Makefile currently pulls the image using $(TAG) then
re-tags it as openwisp/$${image}:$${OPENWISP_VERSION:-edge}, which bypasses
pinning; change the pull to use the desired release variable (OPENWISP_VERSION
if set, otherwise fall back to TAG) so you actually pull the pinned release
before tagging. Update the docker pull invocation to reference
$${OPENWISP_VERSION:-$${TAG}} (and keep the docker tag step tagging to
openwisp/$${image}:$${OPENWISP_VERSION:-edge} if you still want an edge
fallback) so the image fetched matches the release/version variables (variables:
USER, image, TAG, OPENWISP_VERSION).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.envMakefiledeploy/auto-install.shdocker-compose.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- .env
- docker-compose.yml
⏰ 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 comments (4)
Makefile (2)
121-122: Release tagging viaRELEASE_VERSIONlooks good.Clear separation between publish (latest) and release tag improves determinism.
4-7: The.envfile is committed to the repository and present in all workflows (fresh clones, CI, deployments). Hardinclude .envwill not fail and is appropriate. No changes needed.Likely an incorrect or invalid review comment.
deploy/auto-install.sh (2)
131-132: OPENWISP_VERSION persistence + start flow look consistent.Writing
OPENWISP_VERSIONto.envand relying onmake startwithout TAG aligns with the env-driven compose configuration.Also applies to: 182-182
195-205: Upgrade flow aligns with env-driven versioning.Updating
OPENWISP_VERSIONin the upgrade path and starting viamake startis consistent with the new workflow.
atif09
force-pushed
the
issues/554-fix-docker-tag
branch
from
b5f9fbb to
8d0e667
Compare
Introduces OPENWISP_VERSION environment variable to control image versions: - .env defaults to edge for development - auto-install.sh writes latest for production deployments - docker-compose.yml uses edge for all services - Makefile reads from .env and uses separate RELEASE_VERSION for releases Fixes openwisp#554
atif09
force-pushed
the
issues/554-fix-docker-tag
branch
from
8d0e667 to
b6b4e6f
Compare
Previously, the pull target always pulled the :edge image and retagged it as OPENWISP_VERSION, meaning production deployments would run dev images labelled as a release version. Now the pull target uses OPENWISP_VERSION directly for both pulling and tagging, ensuring the correct image version is used. Fixes openwisp#554
- Restored curl API call
- Removed latest from the edge condition
- Removed the redundant horizontal blank space
- Added /openwisp-base: and /openwisp-nfs: to the docker rmi list in clean target so locally-tagged images get removed too
- Replaced openwisp/391495{image}:latest with /391495{image}: as the source for docker tag, and removed the stale inline comments in publish target
Related to openwisp#554
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
Makefile (2)
44-47:base-buildbuilds asopenwisp/openwisp-base:latestthen retags toIMAGE_OWNER— verify intent.The build step produces
openwisp/openwisp-base:latest(hardcoded owner), then Line 47 retags it to$(IMAGE_OWNER)/openwisp-base:$(OPENWISP_VERSION). This works, but the intermediatelatesttag is left asopenwisp/...regardless ofIMAGE_OWNER. If the intent is to support custom image owners throughout, the initial build tag should also be parameterized — or at minimum, document thatopenwisp/openwisp-base:latestis a build-time artifact only.Same applies to
nfs-buildat Lines 50-52.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 44 - 47, The recipe currently builds images with a hardcoded owner then retags them, leaving a stale openwisp/...:latest artifact; update the Makefile targets base-build and nfs-build so the initial docker build uses the parameterized owner/tag (e.g., $(IMAGE_OWNER)/openwisp-base:$(OPENWISP_VERSION) or $(IMAGE_OWNER)/openwisp-base:latest) instead of openwisp/openwisp-base:latest, or explicitly document that openwisp/...:latest is only a transient build artifact; modify the docker build --tag arguments and corresponding docker tag commands accordingly to ensure the built image owner matches $(IMAGE_OWNER) (or add an extra docker tag step if you still need a separate latest tag).
121-123:RELEASE_VERSIONis hardcoded — consider sourcing from.envor a single location.
RELEASE_VERSION = 25.10.0on Line 6 must be manually bumped for each release. IfOPENWISP_VERSIONin.envis also25.10.0for a release deployment, there are now two places to update. Consider deriving one from the other, or at least adding a comment clarifying the relationship:
RELEASE_VERSION= the version being published to the registry.OPENWISP_VERSION(from.env) = the version being consumed by compose.This avoids a drift where someone updates
.envbut forgets the Makefile (or vice versa).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 121 - 123, The Makefile currently hardcodes RELEASE_VERSION (e.g., RELEASE_VERSION = 25.10.0) which can drift from OPENWISP_VERSION in .env; update the Makefile so RELEASE_VERSION is derived from a single source: either read OPENWISP_VERSION from the .env (parse .env and export OPENWISP_VERSION into the Makefile) or add a single canonical version file and have both the Makefile and any .env loader read from it; modify the Makefile's RELEASE_VERSION assignment and the release target (the release: recipe invoking make publish TAG=...) to use that derived value (referencing RELEASE_VERSION and OPENWISP_VERSION in the Makefile and release target) and add a one-line comment in the Makefile documenting that RELEASE_VERSION is the published/tagged release and OPENWISP_VERSION in .env is the consumed compose version to avoid future drift.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.envMakefiledeploy/auto-install.shdocker-compose.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- .env
- deploy/auto-install.sh
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-06T11:56:48.600Z
Learnt from: pandafy
Repo: openwisp/docker-openwisp PR: 549
File: docker-compose.yml:85-85
Timestamp: 2026-01-06T11:56:48.600Z
Learning: In docker-openwisp projects, ensure CELERY_SERVICE_NETWORK_MODE is set to an empty string "" (which Docker Compose treats as unset/null). This allows containers to connect via the Compose default network with correct service name DNS resolution. Using "bridge" as the value disables service name resolution and breaks communication between celery, dashboard, postgres, and redis. Apply this guideline to docker-compose.yml files in the repository and any similar Compose files where CELERY services rely on service name DNS.
Applied to files:
docker-compose.yml
📚 Learning: 2026-01-06T11:28:41.789Z
Learnt from: pandafy
Repo: openwisp/docker-openwisp PR: 549
File: docker-compose.yml:171-171
Timestamp: 2026-01-06T11:28:41.789Z
Learning: In docker-openwisp, the OpenVPN container (images/common/init_command.sh) exits silently with exit code 0 when VPN_DOMAIN environment variable is not set. Combined with restart policy "on-failure" in docker-compose.yml, this prevents the container from restarting when VPN is disabled.
Applied to files:
docker-compose.yml
📚 Learning: 2026-01-06T11:55:13.672Z
Learnt from: pandafy
Repo: openwisp/docker-openwisp PR: 549
File: docker-compose.yml:85-85
Timestamp: 2026-01-06T11:55:13.672Z
Learning: In docker-compose.yml files, avoid leaving CELERY_SERVICE_NETWORK_MODE as an empty string; an empty value is ignored and Docker Compose falls back to default networking. Do not rely on setting CELERY_SERVICE_NETWORK_MODE to 'bridge' for celery services, as this will not affect their networking. If specific networking is required for celery services, define and attach explicit networks in the compose file and reference them on the celery services.
Applied to files:
docker-compose.yml
🔇 Additional comments (2)
docker-compose.yml (1)
15-170: Consistent parameterization of image tags across all services — looks good.All 10 OpenWISP service images consistently use
${OPENWISP_VERSION:-edge}, and third-party images (postgis, influxdb, redis) are left unchanged. This cleanly addresses the pinning objective.Makefile (1)
24-26: Pull target correctly usesOPENWISP_VERSIONfor both pull and tag — nice fix.This properly addresses the earlier concern about always pulling
edgeand then retagging. Now both the pull source and the local tag use the same version variable, so pinning is respected end-to-end.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-compose.yml`:
- Line 15: The docker-compose image reference is hardcoded to openwisp while the
Makefile uses IMAGE_OWNER; update the image key in docker-compose.yml so it uses
the IMAGE_OWNER env var with OPENWISP_VERSION fallback (replace the literal
"openwisp/" prefix in the image line) e.g., use ${IMAGE_OWNER:-openwisp} for the
owner and ${OPENWISP_VERSION:-edge} for the tag, or alternatively
enforce/document that IMAGE_OWNER must remain "openwisp" and adjust
Makefile/README accordingly to keep behavior consistent; target the image line
in docker-compose.yml and the IMAGE_OWNER/OPENWISP_VERSION variables referenced
in your env/Makefile.
In `@Makefile`:
- Around line 4-6: The Makefile currently uses $(OPENWISP_VERSION) and
$(IMAGE_OWNER) without defaults which breaks if .env is missing; after the
existing include .env line, add conditional defaults using the ?= operator —
e.g. add lines setting OPENWISP_VERSION ?= edge and IMAGE_OWNER ?= openwisp (or
your desired default owner) so those variables get the fallback when not defined
in .env; ensure these defaults cover all usages of OPENWISP_VERSION and
IMAGE_OWNER (e.g., in the docker pull/docker tag image references and any other
spots where those variables are referenced).
- Around line 4-5: The Makefile currently uses the mandatory include "include
.env" which causes make to fail if .env is missing; change this to an optional
include by replacing the directive string "include .env" with the optional form
(e.g., "-include .env" or "sinclude .env") so make will silently skip the file
when it doesn't exist, leaving the rest of the Makefile behavior unchanged.
---
Nitpick comments:
In `@Makefile`:
- Around line 44-47: The recipe currently builds images with a hardcoded owner
then retags them, leaving a stale openwisp/...:latest artifact; update the
Makefile targets base-build and nfs-build so the initial docker build uses the
parameterized owner/tag (e.g., $(IMAGE_OWNER)/openwisp-base:$(OPENWISP_VERSION)
or $(IMAGE_OWNER)/openwisp-base:latest) instead of
openwisp/openwisp-base:latest, or explicitly document that openwisp/...:latest
is only a transient build artifact; modify the docker build --tag arguments and
corresponding docker tag commands accordingly to ensure the built image owner
matches $(IMAGE_OWNER) (or add an extra docker tag step if you still need a
separate latest tag).
- Around line 121-123: The Makefile currently hardcodes RELEASE_VERSION (e.g.,
RELEASE_VERSION = 25.10.0) which can drift from OPENWISP_VERSION in .env; update
the Makefile so RELEASE_VERSION is derived from a single source: either read
OPENWISP_VERSION from the .env (parse .env and export OPENWISP_VERSION into the
Makefile) or add a single canonical version file and have both the Makefile and
any .env loader read from it; modify the Makefile's RELEASE_VERSION assignment
and the release target (the release: recipe invoking make publish TAG=...) to
use that derived value (referencing RELEASE_VERSION and OPENWISP_VERSION in the
Makefile and release target) and add a one-line comment in the Makefile
documenting that RELEASE_VERSION is the published/tagged release and
OPENWISP_VERSION in .env is the consumed compose version to avoid future drift.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.env (1)
9-9: Add inline documentation for OPENWISP_VERSION.Consider adding a brief inline comment explaining valid values for OPENWISP_VERSION (e.g., stable release numbers, "edge" for development, "latest" for most recent stable) to guide users who are configuring this for the first time.
📝 Suggested documentation addition
# Image tag pinning +# IMAGE_OWNER: Docker Hub organization/user for OpenWISP images IMAGE_OWNER=openwisp +# OPENWISP_VERSION: Image tag version (e.g., "25.10.0", "latest", or "edge") OPENWISP_VERSION=edge🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env at line 9, Add an inline comment next to OPENWISP_VERSION in the .env to document acceptable values and meanings: explain that OPENWISP_VERSION can be a specific stable release number (e.g., 1.2.3), "edge" for development builds, or "latest" for the most recent stable, and optionally note default behavior and when to choose each option so users configuring OPENWISP_VERSION know which value to pick.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.envdeploy/auto-install.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- deploy/auto-install.sh
⏰ 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 comments (1)
.env (1)
7-9: The codebase consistently uses OPENWISP_VERSION (found in docker-compose.yml, Makefile, and .env), not DOCKER_TAG. No occurrences of DOCKER_TAG were found in any implementation files. Verify whether the PR description actually references DOCKER_TAG, as this claim cannot be confirmed through the codebase alone. If the PR description does mention DOCKER_TAG, it should be updated to reflect the actual variable name used in the implementation (OPENWISP_VERSION).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env:
- Around line 7-9: The default OPENWISP_VERSION is set to "edge", which
contradicts the PR's goal of version pinning; update the .env to use a stable
release tag (e.g., set OPENWISP_VERSION to "25.10.0") or remove the default so
users must explicitly set OPENWISP_VERSION, and if "edge" is intentionally kept
for development add an inline comment clarifying that IMAGE_OWNER and
OPENWISP_VERSION are for dev only.
---
Nitpick comments:
In @.env:
- Line 9: Add an inline comment next to OPENWISP_VERSION in the .env to document
acceptable values and meanings: explain that OPENWISP_VERSION can be a specific
stable release number (e.g., 1.2.3), "edge" for development builds, or "latest"
for the most recent stable, and optionally note default behavior and when to
choose each option so users configuring OPENWISP_VERSION know which value to
pick.
addressed
the intermediate
linking these 2 variables is unnecessary as they have different purposes, this would blend deployment configuration with release management |
- Added conditional defaults so the Makefile is self-contained - Added inline comments for OPENWISP_VERSION Related to openwisp#554
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
106-125:⚠️ Potential issue | 🟠 MajorUse
${IMAGE_OWNER}indocker-compose.ymlfor image references.The
publishtarget sources images from$(IMAGE_OWNER)/$${image}:$(OPENWISP_VERSION)(line 116), butdocker-compose.ymlhardcodesopenwisp/in all service image definitions. WhenIMAGE_OWNERis overridden,compose-build(which runsdocker compose build) will build images under the hardcodedopenwisp/registry, butpublishwill search for them under the customIMAGE_OWNER, causing a mismatch that breaks the build flow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 106 - 125, The issue is that docker-compose.yml hardcodes image names with openwisp/ while the Makefile publish target and compose-build expect images under $(IMAGE_OWNER); update docker-compose.yml so all service image definitions use the IMAGE_OWNER variable (e.g., ${IMAGE_OWNER}) instead of the hardcoded openwisp/ so that compose-build (invoked by the compose-build target) builds images with the same owner that publish (in the publish target) later tags and pushes; ensure IMAGE_OWNER has a sensible default and that compose-build / publish continue to work when IMAGE_OWNER is overridden.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.envMakefile
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: atif09
Repo: openwisp/docker-openwisp PR: 555
File: .env:7-9
Timestamp: 2026-02-17T17:08:06.832Z
Learning: In docker-openwisp, OPENWISP_VERSION=edge is intentionally set as the default in .env to maintain consistency with the existing TAG=edge default in the Makefile. Production users are expected to override OPENWISP_VERSION in their .env file to pin to a specific stable release tag.
Learnt from: atif09
Repo: openwisp/docker-openwisp PR: 555
File: Makefile:4-5
Timestamp: 2026-02-17T16:59:39.027Z
Learning: In the docker-openwisp Makefile, `include .env` is intentionally mandatory (not `-include .env`) because the .env file contains critical configurations that must be present for safe operation. Silent failures with empty values would be more dangerous than failing explicitly when the file is missing.
📚 Learning: 2026-02-17T17:08:06.832Z
Learnt from: atif09
Repo: openwisp/docker-openwisp PR: 555
File: .env:7-9
Timestamp: 2026-02-17T17:08:06.832Z
Learning: In docker-openwisp, OPENWISP_VERSION=edge is intentionally set as the default in .env to maintain consistency with the existing TAG=edge default in the Makefile. Production users are expected to override OPENWISP_VERSION in their .env file to pin to a specific stable release tag.
Applied to files:
Makefile
📚 Learning: 2026-02-17T16:59:39.027Z
Learnt from: atif09
Repo: openwisp/docker-openwisp PR: 555
File: Makefile:4-5
Timestamp: 2026-02-17T16:59:39.027Z
Learning: In all Makefiles, prefer 'include' (not '-include') for critical config files like .env. If the file is missing, the build should fail explicitly rather than silently proceed with empty values. This ensures configurations are present and avoids dangerous silent failures. Consider adding a clear comment near the line and validation steps in CI to verify .env presence.
Applied to files:
Makefile
📚 Learning: 2026-02-17T17:08:06.832Z
Learnt from: atif09
Repo: openwisp/docker-openwisp PR: 555
File: .env:7-9
Timestamp: 2026-02-17T17:08:06.832Z
Learning: In docker-openwisp, OPENWISP_VERSION should default to edge in the .env to align with the Makefile default TAG=edge. Production deployments must override OPENWISP_VERSION in their .env to pin to a specific stable release tag. During reviews, verify the default matches the Makefile default and that there is guidance or a documented override path for production environments.
Applied to files:
.env
📚 Learning: 2026-02-17T16:59:39.027Z
Learnt from: atif09
Repo: openwisp/docker-openwisp PR: 555
File: Makefile:4-5
Timestamp: 2026-02-17T16:59:39.027Z
Learning: In the docker-openwisp Makefile, `include .env` is intentionally mandatory (not `-include .env`) because the .env file contains critical configurations that must be present for safe operation. Silent failures with empty values would be more dangerous than failing explicitly when the file is missing.
Applied to files:
.env
🪛 checkmake (0.2.2)
Makefile
[warning] 10-10: Target "default" should be declared PHONY.
(phonydeclared)
⏰ 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 comments (7)
.env (1)
7-10: LGTM — clear documentation and sensible defaults.The inline comment on Line 9 documents the expected values well, and
OPENWISP_VERSION=edgeis consistent with the Makefile'sTAG = edgedefault. Based on learnings, production users are expected to override this in their.envfile to pin to a specific stable release tag.Makefile (6)
4-6: Intentional mandatory include — looks good.The mandatory
include .envandRELEASE_VERSIONconstant are well-placed. Based on learnings,include .env(not-include) is intentional to fail loudly when critical configuration is missing.
12-15: Fallback defaults are consistent with.envanddocker-compose.yml.
?=correctly defers to values frominclude .envwhen present, falling back toedgeandopenwispotherwise.
20-28: Pull target correctly usesOPENWISP_VERSIONfor both pull and retag.This properly addresses the pinning objective — the fetched image now matches the intended version.
46-54: Build-then-retag pattern is clear.Intermediate
:latesttags are local build artifacts, and the subsequentdocker tagto$(IMAGE_OWNER)/...:$(OPENWISP_VERSION)correctly bridges them into the parameterized naming scheme.
76-88: Clean target correctly removes the newIMAGE_OWNER-tagged images.
123-125: Release workflow is clean and well-structured.The two-step release — first publishing as
:latest, then retagging and pushing as:$(RELEASE_VERSION)withSKIP_BUILD=true— avoids redundant builds while ensuring both tags are available in the registry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Makefile`:
- Around line 106-125: The issue is that docker-compose.yml hardcodes image
names with openwisp/ while the Makefile publish target and compose-build expect
images under $(IMAGE_OWNER); update docker-compose.yml so all service image
definitions use the IMAGE_OWNER variable (e.g., ${IMAGE_OWNER}) instead of the
hardcoded openwisp/ so that compose-build (invoked by the compose-build target)
builds images with the same owner that publish (in the publish target) later
tags and pushes; ensure IMAGE_OWNER has a sensible default and that
compose-build / publish continue to work when IMAGE_OWNER is overridden.
There was a problem hiding this comment.
1. Does this branch solve the issue?
Yes, partially. The approach is correct:
- ✅ docker-compose.yml now uses
${OPENWISP_VERSION:-edge}instead of hardcoded:latest - ✅ auto-install.sh now persists
OPENWISP_VERSIONto.envfile instead of a separateVERSIONfile - ✅ Makefile
pulltarget now pulls the specific version tag
However, there's still a potential issue: If users don't set OPENWISP_VERSION in their .env, it defaults to edge (from the .env file). This is acceptable but could be confusing since edge is not a stable release.
2. Issues Found
A. Version Duplication (Critical)
The version is defined in three places with different purposes:
| File | Variable | Value | Purpose |
|---|---|---|---|
images/common/openwisp/__init__.py |
__openwisp_version__ |
"25.10.0" (hardcoded) |
Embedded in Docker image at build time |
.env |
OPENWISP_VERSION |
edge |
Used at runtime for pulling images |
Makefile |
RELEASE_VERSION |
25.10.0 |
Used for releases |
Problem: images/common/openwisp/__init__.py is never updated by any workflow. It contains stale version info.
Can we find a way to automatically put the right version in images/common/openwisp/__init__.py?
PS: I created a separate issue for this: #570
B. Makefile Variable Ordering Issue
USER = registry.gitlab.com/openwisp/docker-openwisp # Line 10
TAG = edge # Line 11
include .env # Line 3
OPENWISP_VERSION ?= edge # Line 13The include .env comes after USER and TAG definitions. If .env defined USER or TAG, they would be overridden. Currently it works, but it's fragile.
Recommendation: I think this is fine, we can just add a comment besides the include saying "The .env file can override variables in the Makefile, if needed".
C. pull Target Behavior Change
Before:
docker pull --quiet $(USER)/$${image}:$(TAG);
docker tag $(USER)/$${image}:$(TAG) openwisp/$${image}:latest;After:
docker pull --quiet $(USER)/$${image}:$(OPENWISP_VERSION);
docker tag $(USER)/$${image}:$(OPENWISP_VERSION) $(IMAGE_OWNER)/$${image}:$(OPENWISP_VERSION);Risk: If OPENWISP_VERSION doesn't exist on the registry (typo, deleted old tag), the command will fail. Previously it pulled :latest which always existed.
Question: would fallback to latest be ok @pandafy?
D. clean Target Missing New Images
The clean target now needs to remove additional tagged images:
$(IMAGE_OWNER)/openwisp-base:$(OPENWISP_VERSION) \
$(IMAGE_OWNER)/openwisp-nfs:$(OPENWISP_VERSION) \✅ This is already implemented (lines 82-83 in the diff).
E. Naming Confusion: RELEASE_VERSION vs OPENWISP_VERSION
OPENWISP_VERSION- comes from.env, used for pulling/pushingRELEASE_VERSION- hardcoded "25.10.0", used in release workflow
These should be more clearly documented or renamed to avoid confusion:
OPENWISP_VERSION→ could be calledIMAGE_TAG(the tag to pull/push)RELEASE_VERSION→ could stay as is (the version being released)
3. Security Considerations
- ✅ No security issues found
- ✅ No secrets hardcoded
- ✅ No new attack vectors introduced
4. Can we reuse the version from images/common/openwisp/__init__.py?
Answer: No, not directly, for these reasons:
-
It's not accessible at Makefile/runtime: The
__openwisp_version__is embedded in the built Docker image, not available at build time to the Makefile. -
It's already stale: The file shows
"25.10.0"but the latest release is"25.10.2". -
Wrong direction: The flow should be:
.envdefinesOPENWISP_VERSION→ used to pull images- Release workflow should update
images/common/openwisp/__init__.pyas part of the release
Better approach: Use a single source of truth - .env or a VERSION file - and automate the rest.
I created a separate issue: #570.
5. Summary & Recommendations
- Add fallback in
pulltarget if version doesn't exist - Rename variables for clarity (e.g.,
IMAGE_TAGinstead ofOPENWISP_VERSION) - Document the difference between
OPENWISP_VERSIONandRELEASE_VERSION
| echo -ne ${GRN}"OpenWISP Version (leave blank for latest): "${NON} | ||
| read openwisp_version | ||
| if [[ -z "$openwisp_version" ]]; then | ||
| openwisp_version=$(curl -L --silent https://api.github.com/repos/openwisp/docker-openwisp/releases/latest | jq -r .tag_name) | ||
| fi | ||
| echo -ne ${GRN}"OpenWISP Version (leave blank for latest stable release): "${NON} | ||
| read openwisp_version | ||
| if [[ -z "$openwisp_version" ]]; then | ||
| openwisp_version=$(curl -L --silent https://api.github.com/repos/openwisp/docker-openwisp/releases/latest | jq -r .tag_name) | ||
| fi |
There was a problem hiding this comment.
Can we get rid of blank space changes?
| OPENWISP_VERSION ?= edge | ||
| IMAGE_OWNER ?= openwisp |
There was a problem hiding this comment.
@atif09 what is the difference between USER and IMAGE_OWNER?
There was a problem hiding this comment.
USER is the full gitlab registry url, it cant double as a local image namespace, IMAGE_OWNER does that and being a separate variable, lets contributors override it with their own namespace locally
| --file ./images/openwisp_base/Dockerfile ./images/ \ | ||
| $$BUILD_ARGS | ||
| $$BUILD_ARGS; \ | ||
| docker tag openwisp/openwisp-base:latest $(IMAGE_OWNER)/openwisp-base:$(OPENWISP_VERSION) |
There was a problem hiding this comment.
I don't understand this change. Why is this required?
There was a problem hiding this comment.
without this docker tag , the $(IMAGE_OWNER)/openwisp-base:$(OPENWISP_VERSION) never exists and publish fails
There was a problem hiding this comment.
because publish sources from $(IMAGE_OWNER)/$${image}:${OPENWISP_VERSION} when tagging and pushing, and base-build only produces openwisp/openwisp-basse:latest
Added comments to clarify the purpose of RELEASE_VERSION and OPENWISP_VERSION and to note that .env can override Makefile variables if needed Related to openwisp#554
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Line 11: Add a .PHONY declaration for the Makefile target named "default" so
make will always run the compose-build recipe even if a file named "default"
exists; update the Makefile to include a .PHONY: default (you can also group
.PHONY: default compose-build) to mark the "default" target as phony.
- Around line 126-128: The release target in the Makefile relies on an implicit
OPENWISP_VERSION from the environment and can publish wrong images; modify the
release recipe so it explicitly passes OPENWISP_VERSION=$(RELEASE_VERSION) to
the publish calls (i.e., invoke make publish OPENWISP_VERSION=$(RELEASE_VERSION)
TAG=latest SKIP_TESTS=true and make publish OPENWISP_VERSION=$(RELEASE_VERSION)
TAG=$(RELEASE_VERSION) SKIP_BUILD=true SKIP_TESTS=true) so that publish (which
uses $(IMAGE_OWNER)/$${image}:$(OPENWISP_VERSION)) always uses the intended
RELEASE_VERSION rather than any preexisting .env value.
- Line 4: The comment on the `include .env` line is misleading: only variables
assigned with `?=` (e.g., `OPENWISP_VERSION`, `IMAGE_OWNER`) can be overridden
by .env, while variables set with plain `=` after the include (e.g.,
`RELEASE_VERSION`, `TAG`, `USER`, `SHELL`) will not be overridden; update the
comment to state this precisely and either move the `include .env` earlier or
change those assignments to `?=` if you want them overridable so
readers/operators know which variables can be pinned via .env.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Makefile
📜 Review details
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: atif09
Repo: openwisp/docker-openwisp PR: 555
File: .env:7-9
Timestamp: 2026-02-17T17:08:12.144Z
Learning: In docker-openwisp, OPENWISP_VERSION=edge is intentionally set as the default in .env to maintain consistency with the existing TAG=edge default in the Makefile. Production users are expected to override OPENWISP_VERSION in their .env file to pin to a specific stable release tag.
Learnt from: atif09
Repo: openwisp/docker-openwisp PR: 555
File: Makefile:4-5
Timestamp: 2026-02-17T16:59:43.808Z
Learning: In the docker-openwisp Makefile, `include .env` is intentionally mandatory (not `-include .env`) because the .env file contains critical configurations that must be present for safe operation. Silent failures with empty values would be more dangerous than failing explicitly when the file is missing.
📚 Learning: 2026-02-17T17:08:12.144Z
Learnt from: atif09
Repo: openwisp/docker-openwisp PR: 555
File: .env:7-9
Timestamp: 2026-02-17T17:08:12.144Z
Learning: In docker-openwisp, OPENWISP_VERSION=edge is intentionally set as the default in .env to maintain consistency with the existing TAG=edge default in the Makefile. Production users are expected to override OPENWISP_VERSION in their .env file to pin to a specific stable release tag.
Applied to files:
Makefile
📚 Learning: 2026-02-17T16:59:39.027Z
Learnt from: atif09
Repo: openwisp/docker-openwisp PR: 555
File: Makefile:4-5
Timestamp: 2026-02-17T16:59:39.027Z
Learning: In all Makefiles, prefer 'include' (not '-include') for critical config files like .env. If the file is missing, the build should fail explicitly rather than silently proceed with empty values. This ensures configurations are present and avoids dangerous silent failures. Consider adding a clear comment near the line and validation steps in CI to verify .env presence.
Applied to files:
Makefile
🪛 checkmake (0.2.2)
Makefile
[warning] 11-11: Target "default" should be declared PHONY.
(phonydeclared)
🔇 Additional comments (1)
Makefile (1)
29-31: Pull and tag both correctly referenceOPENWISP_VERSION.The previously flagged issue (pulling
$(TAG)then relabelling asOPENWISP_VERSION) has been resolved. Bothdocker pullanddocker tagnow use the same$(OPENWISP_VERSION), so pinned version semantics are consistent.
…env comment openwisp#554 Explicitly pass OPENWISP_VERSION= in release target and clarified which variables can be overriden via .env Related to openwisp#554
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Makefile (1)
11-11: Already discussed:.PHONYfordefaulttarget.The checkmake warning about
defaultlacking a.PHONYdeclaration was previously raised and intentionally deferred by the author for consistency with the rest of the Makefile (which globally lacks.PHONY). No action needed here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` at line 11, Add a .PHONY declaration for the default target to silence the checkmake warning: update the Makefile to include a line like ".PHONY: default" (or add "default" to an existing .PHONY list) so the target named "default" is explicitly marked phony; ensure the symbol referenced is the default target in the Makefile.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Makefile`:
- Line 11: Add a .PHONY declaration for the default target to silence the
checkmake warning: update the Makefile to include a line like ".PHONY: default"
(or add "default" to an existing .PHONY list) so the target named "default" is
explicitly marked phony; ensure the symbol referenced is the default target in
the Makefile.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Makefile
📜 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
🧠 Learnings (3)
📓 Common learnings
Learnt from: atif09
Repo: openwisp/docker-openwisp PR: 555
File: .env:7-9
Timestamp: 2026-02-17T17:08:12.144Z
Learning: In docker-openwisp, OPENWISP_VERSION=edge is intentionally set as the default in .env to maintain consistency with the existing TAG=edge default in the Makefile. Production users are expected to override OPENWISP_VERSION in their .env file to pin to a specific stable release tag.
Learnt from: atif09
Repo: openwisp/docker-openwisp PR: 555
File: Makefile:4-5
Timestamp: 2026-02-17T16:59:43.808Z
Learning: In the docker-openwisp Makefile, `include .env` is intentionally mandatory (not `-include .env`) because the .env file contains critical configurations that must be present for safe operation. Silent failures with empty values would be more dangerous than failing explicitly when the file is missing.
📚 Learning: 2026-02-17T17:08:12.144Z
Learnt from: atif09
Repo: openwisp/docker-openwisp PR: 555
File: .env:7-9
Timestamp: 2026-02-17T17:08:12.144Z
Learning: In docker-openwisp, OPENWISP_VERSION=edge is intentionally set as the default in .env to maintain consistency with the existing TAG=edge default in the Makefile. Production users are expected to override OPENWISP_VERSION in their .env file to pin to a specific stable release tag.
Applied to files:
Makefile
📚 Learning: 2026-02-17T16:59:39.027Z
Learnt from: atif09
Repo: openwisp/docker-openwisp PR: 555
File: Makefile:4-5
Timestamp: 2026-02-17T16:59:39.027Z
Learning: In all Makefiles, prefer 'include' (not '-include') for critical config files like .env. If the file is missing, the build should fail explicitly rather than silently proceed with empty values. This ensures configurations are present and avoids dangerous silent failures. Consider adding a clear comment near the line and validation steps in CI to verify .env presence.
Applied to files:
Makefile
🪛 checkmake (0.2.2)
Makefile
[warning] 11-11: Target "default" should be declared PHONY.
(phonydeclared)
🔇 Additional comments (3)
Makefile (3)
4-18: LGTM — variable declarations and.envintegration look correct.The
include .envis intentionally mandatory,OPENWISP_VERSION ?= edge/IMAGE_OWNER ?= openwispcorrectly use?=so.envtakes precedence, andRELEASE_VERSION/TAG/USERcorrectly use plain=to prevent accidental override. The inline comment on Line 4 accurately scopes what.envcan override.
29-31: LGTM — version pinning is correctly enforced in the pull target.Both the
docker pull(Line 29) anddocker tag(Line 30) now use$(OPENWISP_VERSION), which addresses the previously identified issue where$(TAG)was always pulled and then relabelled as a different version.
126-128: LGTM —releasetarget is now self-contained.Both invocations of
make publishnow explicitly passOPENWISP_VERSION=$(RELEASE_VERSION), ensuring the correct release images are sourced regardless of what.envhas set. This resolves the previously flagged implicit dependency.
3 participants
this change introduces a DOCKER_TAG environment variable that allows users to pin specific image versions in .env file, both 'docker compose pull' and 'make pull' now work according to this variable, ensuring consistent version behavior across all deployment methods
Checklist
Reference to Existing Issue
Closes #554.
Description of Changes
Screenshot
version pinning test:
with
DOCKER_TAG=25.10.0in .env