Multiple conf files - not processed correctly in entrypoint.sh

[BobSammers]

There is processing in entrypoint.sh that looks to be intended to take the first file in /etc/wireguard to pass to wg-quick up, but this doesn't seem to work if there actually is more than one config file: the container fails to start.

The reason is line 17:

config=`echo $configs | head -n 1`

It looks like it should take the first of (potentially) several lines in the $configs variable, but echo doesn't print newline chars, instead replacing them with spaces. This means that when the variable does contain several lines, only a single long line is sent to head and subsequent processing fails.

It can be fixed by substituting printf which preserves newlines (and works more consistently across shells, I believe):

config=`printf "$configs" | head -n 1`

I'm sure most people will only have one config file, so it won't have a big impact (I only spotted it by chance), but I thought I'd file a report.

Edit
A bit of experimentation suggests that the issue can also be solved by just putting inverted comas around the variable in the original line:

config=`echo "$configs" | head -n 1`

also works.

[jordanpotter]

Hey @BobSammers, thank you for creating this issue and for the detailed explanation!

Now that I'm thinking about it... should we even allow multiple config files? When a user accidentally includes multiple files, it seems unexpected to pick the first one alphabetically (which feels random). Maybe instead we error out so the user is forced to specify only one file.

Thoughts?

[BobSammers]

That's a good question - a bigger-picture look at this bit of detail is probably sensible.

I'm not sure. It's obviously a valid requirement to want to have several definitions - if you're peering to a commercial VPN service with lots of endpoints you might want to vary the target server based on load or geography. Of course, that's only useful if there is a mechanism for choosing the endpoint, rather than picking one alphabetically*. The straightforward solution is to do what you do in the README examples, and simply mount only the required conf into the container.

That aside, it seems to me a possible alternative to raising an error on finding more than one conf file is to leave the processing as it is (the working version, obvs!), but allow the user to specify an ENV variable with the name of a config in it, bypassing the find if a config with that name is located.

I suggest this for two reasons:

• It considers the widest range of input to be valid, rather than, instead, introducing a new error condition
• It offers possibilities to anyone who wants access to more than one config inside the container (perhaps to vary the endpoint on the fly, manually or automatically).

It hopefully wouldn't be much different in terms of complexity to implement this over detecting and reporting the new error condition. Famous last words,

*Does find actually return lexicographical results? I've a feeling it's an undefined order, although it's entirely possible that it typically returns output that happens to be sorted by the implementation. It could be passed through sort to make sure, if that's going to be documented.

[jordanpotter]

Hey @BobSammers, please forgive me for the late reply. And thank you for sharing your perspective and considering all the options!

Currently, the container will error if multiple config files are provided. Given that the documentation and examples are built around a single config file, I'd like to keep this behavior. It keeps the image simple to explain and test, and if a user needs to switch between config files, they can update the file they mount.

However, as you highlighted, when multiple config files are detected the code fails awkwardly. I'll make a PR so the code fails more gracefully/explicitly 👍

If a future user has a specific need for multiple config files that cannot be solved via mounts, I'd encourage them to create a new issue and we'll discuss and prioritize it.

Thanks again for all your help!

[jordanpotter]

Here's the PR for your review: #51

[jordanpotter]

PR #51 merged, going to close this out. Thanks again!!