[env] Update VPN environment variables to OPENVPN for WireGuard suppo… #520
Conversation
openwisp#227 Changed VPN_* environment variables to OPENVPN_* to distinguish them from upcoming WireGuard VPN configurations. This prepares the codebase for WireGuard support by making OpenVPN-specific variables clearly identifiable. Changes: - VPN_DOMAIN → OPENVPN_DOMAIN - VPN_NAME → OPENVPN_NAME - VPN_CLIENT_NAME → OPENVPN_CLIENT_NAME Updated across configuration files, Docker images, scripts, and documentation. Fixes openwisp#227
📝 WalkthroughWalkthroughThis pull request systematically renames environment variables across the codebase from generic Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
deploy/auto-install.sh (1)
149-156: Use OPENVPN_DOMAIN in all branches.The default and “disable” branches still write
VPN_DOMAIN, so.envnever updatesOPENVPN_DOMAINwhen the user leaves it blank or disables it. This breaks the rename and keeps stale OpenVPN settings.🐛 Proposed fix
- if [[ -z "$vpn_domain" ]]; then - set_env "VPN_DOMAIN" "openvpn.${domain}" + if [[ -z "$vpn_domain" ]]; then + set_env "OPENVPN_DOMAIN" "openvpn.${domain}" set_env CELERY_SERVICE_NETWORK_MODE "service:openvpn" elif [[ "${vpn_domain,,}" == "n" ]]; then - set_env "VPN_DOMAIN" "" + set_env "OPENVPN_DOMAIN" "" set_env CELERY_SERVICE_NETWORK_MODE "" else set_env "OPENVPN_DOMAIN" "$vpn_domain" fiimages/openwisp_dashboard/load_init_data.py (1)
241-245: Update VPN enablement check to OPENVPN_DOMAIN.
is_vpn_enabledstill readsVPN_DOMAIN, so VPN setup will be skipped when onlyOPENVPN_DOMAINis set.🐛 Proposed fix
- is_vpn_enabled = os.environ.get("VPN_DOMAIN", "") != "" + is_vpn_enabled = os.environ.get("OPENVPN_DOMAIN", "") != ""
🤖 Fix all issues with AI agents
In `@docs/user/settings.rst`:
- Around line 58-61: Rename the reST anchor .. _vpn_domain: to ..
_openvpn_domain: in the settings documentation and update the corresponding
cross-reference :ref:`OpenVPN <vpn_domain>` to :ref:`OpenVPN <openvpn_domain>`
so the anchor name matches the environment variable OPENVPN_DOMAIN and the
reference resolves correctly.
In `@images/common/init_command.sh`:
- Line 32: The test using bash-only [[ ]] around OPENVPN_DOMAIN is incompatible
with the /bin/sh shebang; replace the conditional in the init script that checks
OPENVPN_DOMAIN (the line using [[ -z "$OPENVPN_DOMAIN" ]]) with a
POSIX-compatible test using [ -z "$OPENVPN_DOMAIN" ] and ensure proper quoting
and an exit (e.g., exit 1) remains after the test to preserve behavior in the
init_command.sh flow.
🧹 Nitpick comments (1)
docs/user/settings.rst (1)
999-1005: Update description for consistency.For consistency with the renaming and the description of
OPENVPN_NAME(line 994), line 1002 should say "Name of the OpenVPN client template" instead of "Name of the VPN client template".📝 Suggested fix
-- **Explanation:** Name of the VPN client template that will be visible on +- **Explanation:** Name of the OpenVPN client template that will be visible on the OpenWISP dashboard.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.envdeploy/auto-install.shdocs/user/settings.rstimages/common/init_command.shimages/openwisp_base/Dockerfileimages/openwisp_dashboard/load_init_data.pyimages/openwisp_openvpn/supervisord.conf
🧰 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:
images/openwisp_openvpn/supervisord.conf.envimages/openwisp_base/Dockerfiledeploy/auto-install.shimages/common/init_command.sh
🪛 dotenv-linter (4.0.0)
.env
[warning] 10-10: [UnorderedKey] The OPENVPN_DOMAIN key should go before the SSH_PRIVATE_KEY_PATH key
(UnorderedKey)
[warning] 46-46: [UnorderedKey] The OPENVPN_NAME key should go before the OPENWISP_GEOCODING_CHECK key
(UnorderedKey)
[warning] 47-47: [UnorderedKey] The OPENVPN_CLIENT_NAME key should go before the OPENVPN_DOMAIN key
(UnorderedKey)
🪛 Shellcheck (0.11.0)
images/common/init_command.sh
[warning] 32-32: In POSIX sh, [[ ]] is undefined.
(SC3010)
⏰ 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 (5)
images/openwisp_openvpn/supervisord.conf (1)
21-21: Rename looks consistent.Switching to
ENV_OPENVPN_NAMEaligns with the new env naming.images/openwisp_base/Dockerfile (1)
154-157: LGTM.OpenVPN env defaults are renamed consistently.
.env (1)
10-10: LGTM.Env key renames are consistent with the new OpenVPN naming.
Also applies to: 45-47
docs/user/settings.rst (2)
28-28: LGTM!The prefix list correctly updated to reflect the OpenVPN-specific naming convention.
991-997: LGTM!The section correctly renamed and the explanation properly specifies "OpenVPN Server" for clarity.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| .. _vpn_domain: | ||
|
|
||
| ``VPN_DOMAIN`` | ||
| ~~~~~~~~~~~~~~ | ||
| ``OPENVPN_DOMAIN`` | ||
| ~~~~~~~~~~~~~~~~~~ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the current state of the file around lines 58-61
sed -n '55,70p' docs/user/settings.rstRepository: openwisp/docker-openwisp
Length of output: 446
🏁 Script executed:
# Search for cross-references to vpn_domain anchor
rg -n ':ref:`.*vpn_domain.*`' --glob '*.rst'Repository: openwisp/docker-openwisp
Length of output: 50
🏁 Script executed:
# Search for any references to vpn_domain in the codebase
rg -n 'vpn_domain' --glob '*.rst'Repository: openwisp/docker-openwisp
Length of output: 163
🏁 Script executed:
# Check if openvpn_domain anchor or references already exist
rg -n 'openvpn_domain' --glob '*.rst'Repository: openwisp/docker-openwisp
Length of output: 50
🏁 Script executed:
# Examine the context around the vpn_domain reference in quickstart.rst
sed -n '50,65p' docs/user/quickstart.rstRepository: openwisp/docker-openwisp
Length of output: 897
Update the reStructuredText anchor and cross-reference to match the variable name.
The anchor on line 58 of docs/user/settings.rst uses .. _vpn_domain: while the environment variable is named OPENVPN_DOMAIN. This inconsistency should be resolved by renaming the anchor to .. _openvpn_domain:. Additionally, the cross-reference in docs/user/quickstart.rst:58 that uses :ref:OpenVPN <vpn_domain> must be updated to `:ref:`OpenVPN <openvpn_domain> to maintain the working reference.
Changes required
In docs/user/settings.rst line 58:
-.. _vpn_domain:
+.. _openvpn_domain:In docs/user/quickstart.rst line 58:
-<vpn_domain>` services.
+<openvpn_domain>` services.🤖 Prompt for AI Agents
In `@docs/user/settings.rst` around lines 58 - 61, Rename the reST anchor ..
_vpn_domain: to .. _openvpn_domain: in the settings documentation and update the
corresponding cross-reference :ref:`OpenVPN <vpn_domain>` to :ref:`OpenVPN
<openvpn_domain>` so the anchor name matches the environment variable
OPENVPN_DOMAIN and the reference resolves correctly.
| fi | ||
| elif [ "$MODULE_NAME" = 'openvpn' ]; then | ||
| if [[ -z "$VPN_DOMAIN" ]]; then exit; fi | ||
| if [[ -z "$OPENVPN_DOMAIN" ]]; then exit; fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the shebang and content of the file
head -n 35 images/common/init_command.sh | cat -nRepository: openwisp/docker-openwisp
Length of output: 1216
Fix POSIX shell incompatibility: use [ ] instead of [[ ]].
Line 32 uses bash-specific [[ ]] test syntax, but the shebang is /bin/sh. This will cause a syntax error in dash (the default /bin/sh in most container environments). Replace with the POSIX-compatible [ -z "$OPENVPN_DOMAIN" ].
Suggested fix
- if [[ -z "$OPENVPN_DOMAIN" ]]; then exit; fi
+ if [ -z "$OPENVPN_DOMAIN" ]; then exit; fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ -z "$OPENVPN_DOMAIN" ]]; then exit; fi | |
| if [ -z "$OPENVPN_DOMAIN" ]; then exit; fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 32-32: In POSIX sh, [[ ]] is undefined.
(SC3010)
🤖 Prompt for AI Agents
In `@images/common/init_command.sh` at line 32, The test using bash-only [[ ]]
around OPENVPN_DOMAIN is incompatible with the /bin/sh shebang; replace the
conditional in the init script that checks OPENVPN_DOMAIN (the line using [[ -z
"$OPENVPN_DOMAIN" ]]) with a POSIX-compatible test using [ -z "$OPENVPN_DOMAIN"
] and ensure proper quoting and an exit (e.g., exit 1) remains after the test to
preserve behavior in the init_command.sh flow.
1 participant
…rt #227
Changed VPN_* environment variables to OPENVPN_* to distinguish them from upcoming WireGuard VPN configurations. This prepares the codebase for WireGuard support by making OpenVPN-specific variables clearly identifiable.
Checklist
Reference to Existing Issue
Closes #227
Description of Changes
Updated across configuration files, Docker images, scripts, and documentation.
Screenshot
![Screenshot 2025-10-06 at 10 37 28 PM]()
-bd9005bea847" />