[fix] WireGuard auto tunnel provisioning broken with IPv6 #721#1255
Open
devangpratap wants to merge 7 commits intoopenwisp:masterfrom
Open
[fix] WireGuard auto tunnel provisioning broken with IPv6 #721#1255devangpratap wants to merge 7 commits intoopenwisp:masterfrom
devangpratap wants to merge 7 commits intoopenwisp:masterfrom
Conversation
When using an IPv6 subnet from OpenWISP IPAM for a WireGuard VPN, the auto-generated client template incorrectly set the address family to 'ipv4' with mask 32 because netjsonconfig's wireguard_auto_client hardcodes these values regardless of IP version. After calling wireguard_auto_client, the resulting config is now post-processed to set family='ipv6' and mask=128 when the VPN subnet is IPv6. A regression test is also included. Fixes openwisp#721
Contributor
|
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:
📝 WalkthroughWalkthroughThe change updates AbstractVpn.auto_client so that when generating a WireGuard auto-client configuration for an IPv6 subnet and a provided template_backend_class, any WireGuard interface in the produced output has its address entries annotated with family "ipv6" and mask 128. Three tests were added verifying auto_client produces a WireGuard interface address with family "ipv6" and mask 128 for WireGuard, VXLAN+WireGuard, and a generic VPN template when given an IPv6 subnet. CHANGES.rst was updated to record the bugfix. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openwisp_controller/config/tests/test_vpn.py`:
- Around line 652-655: The test currently assumes the WireGuard interface is at
interfaces[0]; instead, find the interface in auto["interfaces"] whose "type" ==
"wireguard" and then set addresses = that_interface["addresses"] (and fail the
test with an informative message if no such interface is found) so the
subsequent assertions on addresses[0]["family"] and addresses[0]["mask"] are
resilient to backend ordering changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0d7ba452-5e76-45e2-a78f-7a4f4de4a40b
📒 Files selected for processing (2)
openwisp_controller/config/base/vpn.pyopenwisp_controller/config/tests/test_vpn.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). (11)
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-15T15:05:49.557Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/management/commands/clear_last_ip.py:38-42
Timestamp: 2026-01-15T15:05:49.557Z
Learning: In Django projects, when using select_related() to traverse relations (for example, select_related("organization__config_settings")), the traversed relation must not be deferred. If you also use .only() in the same query, include the relation name or FK field (e.g., "organization" or "organization_id") in the .only() list to avoid the error "Field X cannot be both deferred and traversed using select_related at the same time." Apply this guideline to Django code in openwisp_controller/config/management/commands/clear_last_ip.py and similar modules by ensuring any select_related with an accompanying only() includes the related field names to prevent deferred/traversed conflicts.
Applied to files:
openwisp_controller/config/base/vpn.pyopenwisp_controller/config/tests/test_vpn.py
📚 Learning: 2026-02-17T19:13:10.088Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:10.088Z
Learning: In reviews for the openwisp/openwisp-controller repository, do not propose changes based on Ruff warnings. The project does not use Ruff as its linter; ignore Ruff-related suggestions and follow the repository’s established linting and configuration rules. This guidance applies to all Python files under the openwisp_controller directory.
Applied to files:
openwisp_controller/config/base/vpn.pyopenwisp_controller/config/tests/test_vpn.py
📚 Learning: 2026-01-15T15:07:17.354Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/estimated_location/tests/tests.py:172-175
Timestamp: 2026-01-15T15:07:17.354Z
Learning: In this repository, flake8 enforces E501 (line too long) via setup.cfg (max-line-length = 88) while ruff ignores E501 via ruff.toml. Therefore, use '# noqa: E501' on lines that intentionally exceed 88 characters to satisfy flake8 without affecting ruff checks. This applies to Python files across the project (any .py) and is relevant for tests as well. Use sparingly and only where breaking lines is not feasible without hurting readability or functionality.
Applied to files:
openwisp_controller/config/base/vpn.pyopenwisp_controller/config/tests/test_vpn.py
🔇 Additional comments (1)
openwisp_controller/config/base/vpn.py (1)
632-637: IPv6 WireGuard address normalization is correctly implemented.This is a good, targeted fix: it only applies for IPv6 subnets and rewrites WireGuard interface addresses to
family="ipv6"with/128, matching expected client semantics.
Pass matching organization to both the VPN and its subnet in
test_auto_client_ipv6 so they satisfy the org-relation validation
(_validate_org_relation("subnet")) in openwisp-users.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
openwisp_controller/config/tests/test_vpn.py (1)
654-654:⚠️ Potential issue | 🟡 MinorMake the IPv6 regression assertion order-independent.
Line 654 assumes the WireGuard interface is always at index
0. That can make this regression test flaky if interface ordering changes.Suggested hardening
- addresses = auto["interfaces"][0]["addresses"] + wg_interface = next( + (i for i in auto["interfaces"] if i.get("type") == "wireguard"), None + ) + self.assertIsNotNone( + wg_interface, "WireGuard interface missing in auto_client output" + ) + addresses = wg_interface["addresses"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openwisp_controller/config/tests/test_vpn.py` at line 654, The test currently assumes the WireGuard interface is at interfaces[0] by using auto["interfaces"][0]["addresses"]; change the assertion to be order-independent by locating the interface that contains the expected IPv6 addresses (e.g., search auto["interfaces"] for an interface entry whose "addresses" contains the target IPv6 / whose "family" == "inet6" or whose "name" matches the WireGuard interface name) and then assert against that interface's "addresses" list; update any references to auto["interfaces"][0]["addresses"] to use the found_interface variable (or equivalent) so the test no longer depends on list ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@openwisp_controller/config/tests/test_vpn.py`:
- Line 654: The test currently assumes the WireGuard interface is at
interfaces[0] by using auto["interfaces"][0]["addresses"]; change the assertion
to be order-independent by locating the interface that contains the expected
IPv6 addresses (e.g., search auto["interfaces"] for an interface entry whose
"addresses" contains the target IPv6 / whose "family" == "inet6" or whose "name"
matches the WireGuard interface name) and then assert against that interface's
"addresses" list; update any references to auto["interfaces"][0]["addresses"] to
use the found_interface variable (or equivalent) so the test no longer depends
on list ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: adfa8ee3-c5dd-41bd-b469-f09c51641a8c
📒 Files selected for processing (1)
openwisp_controller/config/tests/test_vpn.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). (11)
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-15T15:05:49.557Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/management/commands/clear_last_ip.py:38-42
Timestamp: 2026-01-15T15:05:49.557Z
Learning: In Django projects, when using select_related() to traverse relations (for example, select_related("organization__config_settings")), the traversed relation must not be deferred. If you also use .only() in the same query, include the relation name or FK field (e.g., "organization" or "organization_id") in the .only() list to avoid the error "Field X cannot be both deferred and traversed using select_related at the same time." Apply this guideline to Django code in openwisp_controller/config/management/commands/clear_last_ip.py and similar modules by ensuring any select_related with an accompanying only() includes the related field names to prevent deferred/traversed conflicts.
Applied to files:
openwisp_controller/config/tests/test_vpn.py
📚 Learning: 2026-02-17T19:13:10.088Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:10.088Z
Learning: In reviews for the openwisp/openwisp-controller repository, do not propose changes based on Ruff warnings. The project does not use Ruff as its linter; ignore Ruff-related suggestions and follow the repository’s established linting and configuration rules. This guidance applies to all Python files under the openwisp_controller directory.
Applied to files:
openwisp_controller/config/tests/test_vpn.py
📚 Learning: 2026-01-15T15:07:17.354Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/estimated_location/tests/tests.py:172-175
Timestamp: 2026-01-15T15:07:17.354Z
Learning: In this repository, flake8 enforces E501 (line too long) via setup.cfg (max-line-length = 88) while ruff ignores E501 via ruff.toml. Therefore, use '# noqa: E501' on lines that intentionally exceed 88 characters to satisfy flake8 without affecting ruff checks. This applies to Python files across the project (any .py) and is relevant for tests as well. Use sparingly and only where breaking lines is not feasible without hurting readability or functionality.
Applied to files:
openwisp_controller/config/tests/test_vpn.py
Code Review SummaryStatus: No Issues Found | Recommendation: Merge OverviewThis PR effectively fixes WireGuard auto tunnel provisioning broken with IPv6 subnets (issue #721). Changes Reviewed
Implementation Quality
Files Reviewed (4 files)
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
openwisp_controller/config/tests/test_vpn.py (1)
643-657: 🧹 Nitpick | 🔵 TrivialMake the assertion resilient to interface ordering.
The test assumes
auto["interfaces"][0]is the WireGuard interface. If backend ordering changes, this could cause false negatives. Consider finding the interface by type:- addresses = auto["interfaces"][0]["addresses"] + wg_interface = next( + (i for i in auto["interfaces"] if i.get("type") == "wireguard"), None + ) + self.assertIsNotNone(wg_interface, "WireGuard interface not found") + addresses = wg_interface["addresses"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openwisp_controller/config/tests/test_vpn.py` around lines 643 - 657, In test_auto_client_ipv6 update the assertions to find the WireGuard interface from auto["interfaces"] instead of assuming index 0: call vpn.auto_client(template_backend_class=template.backend_class) as before, then locate the interface object where its type/name indicates WireGuard (e.g., check interface.get("type") == "wireguard" or interface.get("name") contains "wg") and run the assertions against that interface's "addresses" list (length == 1, family == "ipv6", mask == 128) so the test no longer depends on interface ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openwisp_controller/config/tests/test_vpn.py`:
- Around line 1015-1031: The test test_auto_client_ipv6 assumes the WireGuard
interface is at auto["interfaces"][0]; instead, locate the interface by its type
before asserting addresses: call vpn.auto_client(...) to get auto, find the
interface in auto["interfaces"] where interface["type"] == "wireguard" (or the
correct interface type used elsewhere), then inspect that interface's
"addresses" to assert length, family ("ipv6") and mask (128); update references
to avoid using fixed index and use a filter/next pattern on the interfaces list.
---
Duplicate comments:
In `@openwisp_controller/config/tests/test_vpn.py`:
- Around line 643-657: In test_auto_client_ipv6 update the assertions to find
the WireGuard interface from auto["interfaces"] instead of assuming index 0:
call vpn.auto_client(template_backend_class=template.backend_class) as before,
then locate the interface object where its type/name indicates WireGuard (e.g.,
check interface.get("type") == "wireguard" or interface.get("name") contains
"wg") and run the assertions against that interface's "addresses" list (length
== 1, family == "ipv6", mask == 128) so the test no longer depends on interface
ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4ddcf04c-e7c8-4bfe-9b8f-26322f68c343
📒 Files selected for processing (2)
CHANGES.rstopenwisp_controller/config/tests/test_vpn.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). (11)
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-15T15:05:49.557Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/management/commands/clear_last_ip.py:38-42
Timestamp: 2026-01-15T15:05:49.557Z
Learning: In Django projects, when using select_related() to traverse relations (for example, select_related("organization__config_settings")), the traversed relation must not be deferred. If you also use .only() in the same query, include the relation name or FK field (e.g., "organization" or "organization_id") in the .only() list to avoid the error "Field X cannot be both deferred and traversed using select_related at the same time." Apply this guideline to Django code in openwisp_controller/config/management/commands/clear_last_ip.py and similar modules by ensuring any select_related with an accompanying only() includes the related field names to prevent deferred/traversed conflicts.
Applied to files:
openwisp_controller/config/tests/test_vpn.py
📚 Learning: 2026-02-17T19:13:10.088Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:10.088Z
Learning: In reviews for the openwisp/openwisp-controller repository, do not propose changes based on Ruff warnings. The project does not use Ruff as its linter; ignore Ruff-related suggestions and follow the repository’s established linting and configuration rules. This guidance applies to all Python files under the openwisp_controller directory.
Applied to files:
openwisp_controller/config/tests/test_vpn.py
📚 Learning: 2026-01-15T15:07:17.354Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/estimated_location/tests/tests.py:172-175
Timestamp: 2026-01-15T15:07:17.354Z
Learning: In this repository, flake8 enforces E501 (line too long) via setup.cfg (max-line-length = 88) while ruff ignores E501 via ruff.toml. Therefore, use '# noqa: E501' on lines that intentionally exceed 88 characters to satisfy flake8 without affecting ruff checks. This applies to Python files across the project (any .py) and is relevant for tests as well. Use sparingly and only where breaking lines is not feasible without hurting readability or functionality.
Applied to files:
openwisp_controller/config/tests/test_vpn.py
🔇 Additional comments (1)
CHANGES.rst (1)
7-11: LGTM!The changelog entry is properly formatted and accurately describes the bugfix for WireGuard auto tunnel provisioning with IPv6 subnets. The RST formatting and link structure are consistent with other entries in the file.
CI Failure AnalysisHello @devangpratap, There are two distinct failures in the CI logs:
Failures & Remediation
|
… helper Pop 'organization' and 'subnet' from kwargs before passing them explicitly to _create_vpn, preventing 'got multiple values for keyword argument' error when test_auto_client_ipv6 supplies these via vpn_options.
CI Failure AnalysisHello @devangpratap, Build/Infrastructure/Other: GitHub Actions job timed outExplanation: Remediation: # Re-run the CI job |
Find WireGuard interface by name instead of assuming index 0, as suggested by CodeRabbit review.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
openwisp_controller/config/tests/test_vpn.py (1)
654-656:⚠️ Potential issue | 🟡 MinorUse the interface
typeinstead of a"wg"name substring.These tests are still coupled to a naming convention, so they can fail if the WireGuard interface is renamed while the generated schema remains valid. Filtering by
type == "wireguard"is the stable discriminator here.Suggested test hardening
- wg_interface = next( - (i for i in auto["interfaces"] if "wg" in i.get("name", "")), None - ) + wg_interface = next( + (i for i in auto["interfaces"] if i.get("type") == "wireguard"), None + ) self.assertIsNotNone(wg_interface, "WireGuard interface not found")Also applies to: 1032-1034
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openwisp_controller/config/tests/test_vpn.py` around lines 654 - 656, The test currently selects the WireGuard interface by name substring using wg_interface = next((i for i in auto["interfaces"] if "wg" in i.get("name", "")), None); change this to filter by the interface type instead (e.g., next((i for i in auto["interfaces"] if i.get("type") == "wireguard"), None)) so the test uses the stable discriminator "type" == "wireguard" when locating the interface in the auto["interfaces"] list (apply the same change for the other occurrence around lines 1032-1034).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@openwisp_controller/config/tests/test_vpn.py`:
- Around line 654-656: The test currently selects the WireGuard interface by
name substring using wg_interface = next((i for i in auto["interfaces"] if "wg"
in i.get("name", "")), None); change this to filter by the interface type
instead (e.g., next((i for i in auto["interfaces"] if i.get("type") ==
"wireguard"), None)) so the test uses the stable discriminator "type" ==
"wireguard" when locating the interface in the auto["interfaces"] list (apply
the same change for the other occurrence around lines 1032-1034).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6b0e3ec2-da8c-44f4-8463-5e5b92dcee34
📒 Files selected for processing (1)
openwisp_controller/config/tests/test_vpn.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). (11)
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-15T15:05:49.557Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/management/commands/clear_last_ip.py:38-42
Timestamp: 2026-01-15T15:05:49.557Z
Learning: In Django projects, when using select_related() to traverse relations (for example, select_related("organization__config_settings")), the traversed relation must not be deferred. If you also use .only() in the same query, include the relation name or FK field (e.g., "organization" or "organization_id") in the .only() list to avoid the error "Field X cannot be both deferred and traversed using select_related at the same time." Apply this guideline to Django code in openwisp_controller/config/management/commands/clear_last_ip.py and similar modules by ensuring any select_related with an accompanying only() includes the related field names to prevent deferred/traversed conflicts.
Applied to files:
openwisp_controller/config/tests/test_vpn.py
📚 Learning: 2026-02-17T19:13:10.088Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:10.088Z
Learning: In reviews for the openwisp/openwisp-controller repository, do not propose changes based on Ruff warnings. The project does not use Ruff as its linter; ignore Ruff-related suggestions and follow the repository’s established linting and configuration rules. This guidance applies to all Python files under the openwisp_controller directory.
Applied to files:
openwisp_controller/config/tests/test_vpn.py
📚 Learning: 2026-01-15T15:07:17.354Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/estimated_location/tests/tests.py:172-175
Timestamp: 2026-01-15T15:07:17.354Z
Learning: In this repository, flake8 enforces E501 (line too long) via setup.cfg (max-line-length = 88) while ruff ignores E501 via ruff.toml. Therefore, use '# noqa: E501' on lines that intentionally exceed 88 characters to satisfy flake8 without affecting ruff checks. This applies to Python files across the project (any .py) and is relevant for tests as well. Use sparingly and only where breaking lines is not feasible without hurting readability or functionality.
Applied to files:
openwisp_controller/config/tests/test_vpn.py
CI Failure AnalysisHello @devangpratap, The CI pipeline failed due to a Test Failure. Explanation: The Remediation:
If the issue persists after these steps, it might be necessary to add more detailed logging within the Selenium test setup and teardown to capture the exact state leading to the crash. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When using an IPv6 subnet from OpenWISP IPAM for a WireGuard VPN, the auto-generated client template incorrectly set the address family to 'ipv4' with mask 32 because netjsonconfig's wireguard_auto_client hardcodes these values regardless of IP version.
After calling wireguard_auto_client, the resulting config is now post-processed to set family='ipv6' and mask=128 when the VPN subnet is IPv6. A regression test is also included.
Checklist
Reference to Existing Issue
Closes #721
Description of Changes
The wireguard_auto_client method in netjsonconfig's OpenWrt backend hardcodes family: ipv4 and mask: 32 when generating the WireGuard interface address, regardless of the actual IP version of the assigned subnet. When a user sets up a WireGuard VPN using an IPv6 IPAM subnet and auto-configures a VPN client template, the generated template gets an incorrect IPv4 address family. If the user then manually corrects the family to ipv6, schema validation fails.
The fix post-processes the output of wireguard_auto_client in AbstractVpn.auto_client() (openwisp_controller/config/base/vpn.py): when self.subnet.subnet.version == 6, all WireGuard interface addresses in the auto-generated config are updated to family: ipv6 and mask: 128.
Screenshot
N/A