Setting both wireguard_endpoint to "" and wireguard_port to a non default port results in invalid template output

[daanh432]

The way the template is built. If you set the wireguard_endpoint host_var to an empty string (meaning that that host shouldn't have an endpoint) the if statements end up with in undesired option.

Whilst having a custom port it is not possible to remove the endpoint by setting the wireguard_endpoint to an empty string.

{%       if hostvars[host].wireguard_endpoint is defined and hostvars[host].wireguard_endpoint != "" %}
Endpoint = {{hostvars[host].wireguard_endpoint}}:{{hostvars[host].wireguard_port}}
{%       else %}
Endpoint = {{host}}:{{hostvars[host].wireguard_port}}
{%       endif %}

[aut0]

Any reason this is not being addressed?

[githubixx]

I've no test case for it :wink: That means I first need to setup a Molecule test case to be able to reproduce the behavior. I normally want to first see the result in a test environment before I merge something that changes the "core" template. But I don't have that much time to create a Molecule test where I need to guess what values covers the PRs case. So I normally only work on such PRs if I've "too much time" or someone provides a Molecule test case that I can use as a template.

[aut0]

Understood. I propose to add the "bug" label.

[githubixx]

@daanh432 Can you provide some test variables that helps to reproduce the issue?

[daanh432]

It has to do with some hosts not having a valid endpoint to connect to because they are behind a firewall for example. This is fine within the Wireguard world because of the ability to use persistent keepalive which will allow the reverse connection anyway without it having an endpoint defined.

With the template in the main branch at this time, setting the endpoint to an empty string and port to a valid value results in it falling back to Endpoint = {{host}}:{{hostvars[host].wireguard_port}}.

So for one of your hosts set the variables as follows (the port value is irrelevant it just needs to be defined):

wireguard_endpoint: ""
wireguard_port: 5555

This will have the unintended output of:

Endpoint = 1.2.3.4:5555

The expected output would be leaving out the Endpoint entirely because we're explicitly telling it to not have an endpoint address.

The pull request I opened a while ago splits up the check for defined and empty string. If it is undefined it will still use Endpoint = {{host}}:{{hostvars[host].wireguard_port}}.

But if it is defined with an empty string it will leave out the endpoint definition entirely and output # No endpoint defined for this peer.

In short: as of right now it is only possible to remove the endpoint statement in its entirety by not using a custom port. With the change in #177 it is also possible to remove the endpoint statement when having a globally defined custom port.

I would like to help you out with the Molecule test but I have had no available time yet to look into the workings of Molecule.

[daanh432]

This issue has also been mentioned in an earlier PR. So maybe it is more worthwhile to follow up on that pull request instead.