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

Hide peers with empty endpoints #102

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Miroka96
Copy link

solves #101

@githubixx
Copy link
Owner

Thanks for the PR! Sorry for not getting back earlier! While this PR works fine for my setup I'd like to setup an additional Molecule test scenario fist to make sure that it works as intended. I can't test every edge case but I'd at least have a better "feeling" before merging it 😉 I'm not sure if the current Molecule test scenario really covers this case at least a little bit.

@provokateurin
Copy link

It's good to know that you have to set wireguard_port to "" otherwise it will still create then endpoint entries.

@provokateurin
Copy link

The same should be done for unmanaged peers:

diff --git a/templates/etc/wireguard/wg.conf.j2 b/templates/etc/wireguard/wg.conf.j2
index bc60039..ad758b3 100644
--- a/templates/etc/wireguard/wg.conf.j2
+++ b/templates/etc/wireguard/wg.conf.j2
@@ -88,6 +88,8 @@ Endpoint = {{host}}:{{wireguard_port}}
 
 # Peers not managed by Ansible from "wireguard_unmanaged_peers" variable
 {% for peer in wireguard_unmanaged_peers.keys() %}
+{%   if (wireguard_unmanaged_peers[peer].endpoint is defined and wireguard_unmanaged_peers[peer].endpoint != "") or (wireguard_endpoint is defined and wireguard_endpoint != "") %}
+
 [Peer]
 # {{ peer }}
 PublicKey = {{ wireguard_unmanaged_peers[peer].public_key }}
@@ -103,5 +105,6 @@ Endpoint = {{ wireguard_unmanaged_peers[peer].endpoint }}
 {%     if wireguard_unmanaged_peers[peer].persistent_keepalive is defined %}
 PersistentKeepalive = {{ wireguard_unmanaged_peers[peer].persistent_keepalive }}
 {%     endif %}
+{%   endif %}
 {%   endfor %}
 {% endif %}

@almereyda
Copy link

almereyda commented Dec 22, 2021

I had merged this with master into https://github.com/libresh/ansible-role-wireguard, but cannot confirm that leaving the host variable wireguard_endpoint out and setting wireguard_port to "" will also hide these clients.

A workaround is to add them as unmanaged clients.

@christf
Copy link

christf commented Jan 8, 2022

I am in the same boat and tested your PR. It is working well for me. setting the port to "" will yield a message about invalid ports as it must be an integer.
@Miroka96 thank you for this patch. it did help me a lot!

Any chance to get this merged soon?

@githubixx
Copy link
Owner

Hi! Can you please provide a test case that shows the problem? It would be very helpful to maybe make a copy of the existing Molecule test with two or three VMs with some host variables that will cause the problem e.g. The test doesn't need to work as most people most probably don't have QEMU/KVM around to actually test it. I just want to have a few variables to play around. Thx!

@varac
Copy link

varac commented Feb 1, 2022

Also looking forward for this, since the current behaviour breaks routing over a central server when two clients without endpoint try to connect to each other.

@varac
Copy link

varac commented Feb 1, 2022

I tested this PR and it works nicely.

gregorydlogan added a commit to gregorydlogan/ansible-role-wireguard that referenced this pull request Mar 5, 2024
…d into master

Pull request githubixx#102

  Hide peers with empty endpoints
@eugeneoden
Copy link

This PR helped me resolve routing issues when using empty endpoints for NAT'd peers without endpoints - seems safe to merge.

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.

7 participants