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

Make openvpn_custom_dns work as defined in Readme #175

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

danielsimkus
Copy link

Hello,

I was using openvpn_custom_dns along with openvpn_set_dns as true and noticed it wasn't adding the servers to my openvpn config.

Looking at the code it previously treated openvpn_custom_dns as if it was a Boolean, and instead used openvpn_dns_servers to loop through the servers:

{% if openvpn_set_dns %}
{% if openvpn_custom_dns %}
{% for srv in openvpn_dns_servers %}
push "dhcp-option DNS {{ srv }}"

It seemed to be suggesting that in our configs we set:

openvpn_set_dns: true
openvpn_custom_dns: true
openvpn_dns_servers:
 - 8.8.8.8
 - 8.8.4.4

Which contradicted the docs which state:
Screenshot 2022-01-25 at 10 05 47


I've tried to change it so we don't use openvpn_dns_servers at all, and the check for if we either add custom-servers vs the default DNS is based entirely on whether or not openvpn_custom_dns has entries in it.
This makes it more in-line with what the README says

Potentially breaking changes

  • If people have themselves been using the undocumented openvpn_dns_servers (and openvpn_custom_dns:true) it will no longer work
  • If people have been passing a bad list to openvpn_custom_dns without it actually writing them to config previously, it would now actually insert them which could potentially break

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.

1 participant