Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of IPv6 support #151

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

tricovictor
Copy link

openvpn_ipv6_enabled is defined for ipv6 support.
Also openvpm_ipv6_route_ranges, where we define an array of network ranges to use.
For example:
openvpn_ipv6_route_ranges:

  • 2000:1::/64
  • 2000:3::/64

@tricovictor
Copy link
Author

Missing \ n to run molecule correctly here

@nkakouros
Copy link
Collaborator

@tricovictor Do you need sth from me to fix the PR?

@tricovictor
Copy link
Author

Hi @nkakouros. I not is acquainted with Molecule but the task "Set IPv6 forwarding in the sysctl file and reload if necessary" in /task/system/fordwarding.yml is reporting:

errorIpv6

I suppose it is solved as the task but from IPV4 by placing:
when: 'not lookup('env', 'IN_MOLECULE') | d(true, true) | bool'
and add openvpn_ipv6_server is defined

sysctl_set: true
state: present
reload: true
when: not lookup('env', 'IN_MOLECULE') | d(true, true) | bool and openvpn_ipv6_server is defined
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you break this when into a list?

Choose a reason for hiding this comment

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

Done in c1cbc8c

@@ -8,3 +8,12 @@
state: present
reload: true
when: not lookup('env', 'IN_MOLECULE') | d(true, true) | bool

- name: Set IPv6 forwarding in the sysctl file and reload if necessary
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you also rename the previous task to say IPv4?

Choose a reason for hiding this comment

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

Done in c1cbc8c

@@ -14,6 +15,10 @@ port {{ openvpn_port }}
# TCP or UDP server?
proto {{ openvpn_proto }}

{% if openvpn_ipv6_enabled %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add | bool for consistency.

Choose a reason for hiding this comment

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

Done in c1cbc8c

@@ -31,6 +36,9 @@ cipher {{ openvpn_cipher }}
# most systems, the VPN will not function unless you partially or fully disable
# the firewall for the TUN/TAP interface.
dev {{ openvpn_dev }}
{% if openvpn_ipv6_enabled %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add | bool for consistency.

Choose a reason for hiding this comment

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

Done in c1cbc8c

@@ -31,6 +36,9 @@ cipher {{ openvpn_cipher }}
# most systems, the VPN will not function unless you partially or fully disable
# the firewall for the TUN/TAP interface.
dev {{ openvpn_dev }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you leave the non-ipv6 options outside of a conditional? Does this work?

Choose a reason for hiding this comment

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

Yes, this works

@@ -73,6 +81,11 @@ topology {{ openvpn_topology }}
# 10.8.0.1. Comment this line out if you are ethernet bridging. See the man
# page for more info.
server {{ openvpn_server }}
{% if openvpn_ipv6_enabled and openvpn_ipv6_server is defined %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

add | bool

Choose a reason for hiding this comment

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

Done in c1cbc8c

{% if openvpn_ipv6_enabled and openvpn_ipv6_server is defined %}
server-ipv6 {{ openvpn_ipv6_server }}
ifconfig-ipv6 {{ openvpn_ipv6_ifconfig }}
push "route-ipv6-default {{ openvpn_ipv6_route_default }}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I couldn't find an option named route-ipv6-default. Where is this documented?

Also, why do you need lines 86-87 unconditionally when ipv6 is used?

Choose a reason for hiding this comment

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

openvpn_ipv6_server is defined will always be true. This variable is defined in defaults/main.yml as an empty string, what will matter in this template is if it's still empty or not.

According to the OpenVPN wiki

There are 2 ways to add IPv6 addressing and pool options to the server, similar to what OpenVPN supports for IPv4: using a helper-directive, and by expanding the helper-directive. The expansion is required if you do not wish to use the automatic values the helper-directive supplies.

  • therefore ifconfig-ipv6 will be a complementary and optional configuration
  • route-ipv6-default doesn't exist, it's optional, and can be pushed with the variable openvpn_ipv6_route_ranges which is defined as an empty list by default.

Choose a reason for hiding this comment

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

Done in c1cbc8c

@santiagomr
Copy link

I added to this PR the fix to the IP forwarding discussed here, since it's necessary for both IPv4 (#159) and IPv6.

Do you think that something else is missing to merge this PR? Please note that we (@UdelaRInterior) are an organization. @tricovictor, @ulvida, and I (@santiagomr) are working together on this fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants