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

[FEAT] Enable SERVER_ALLOWEDIPS_PEER_ declaration by comma #331

Closed
1 task done
thalesmaoa opened this issue Apr 24, 2024 · 9 comments
Closed
1 task done

[FEAT] Enable SERVER_ALLOWEDIPS_PEER_ declaration by comma #331

thalesmaoa opened this issue Apr 24, 2024 · 9 comments
Labels
enhancement New feature or request

Comments

@thalesmaoa
Copy link

Is this a new feature request?

  • I have searched the existing issues

Wanted change

I'm using .env file to declare peer as follows:

# Define 1 PEER
PEER="client1"
PEERS_CLIENT="$PEERS_CLIENT,$PEER"

# Define 2 PEER
PEER="client2"
PEERS_CLIENT="$PEERS_CLIENT,$PEER"
PEER_ALLOWEDIP="SERVER_ALLOWEDIPS_PEER_$PEER=110.254.11.0/24"

# Define 3 PEER
PEER="client3"
PEERS_CLIENT="$PEERS_CLIENT,$PEER"
PEER_ALLOWEDIP="$PEER_ALLOWEDIP,SERVER_ALLOWEDIPS_PEER_$PEER=120.254.23.0/24"

Reason for change

SERVER_ALLOWEDIPS_PEER_ variable is not identified for multiple peers:

[Peer]
# peer_client2
PublicKey = 
PresharedKey = 
AllowedIPs = 11.254.254.4/32,10.254.1.0/24,SERVER_ALLOWEDIPS_PEER_client3=xx.xx.xx.x/24,SERVER_ALLOWEDIPS_PEER_client2=xx.xx.xx.x/24

Proposed code change

No response

@thalesmaoa thalesmaoa added the enhancement New feature or request label Apr 24, 2024
Copy link

Thanks for opening your first issue here! Be sure to follow the relevant issue templates, or risk having this issue marked as invalid.

@aptalca
Copy link
Member

aptalca commented Apr 24, 2024

No idea what that .env is meant to accomplish but the variable is explained here: https://github.com/linuxserver/docker-wireguard?tab=readme-ov-file#site-to-site-vpn

@thalesmaoa
Copy link
Author

Sure, this is a feature request.

The .env file is supported by docker to add enviromental variables during docker deploy. Docker .env doc.

For now, to make a site-to-site, there must be individuals calls for each client:

SERVER_ALLOWEDIPS_PEER_laptop="192.168.1.0/24,192.168.2.0/24"
SERVER_ALLOWEDIPS_PEER_smartphone="192.168.1.0/24,192.168.3.0/24"
SERVER_ALLOWEDIPS_PEER_desktop="192.168.1.0/24,192.168.4.0/24"

For 3 clients, this is ok to admin. When this number grows a bit, it is kind of difficult to track.

My suggestion is to make like peer declaration, by comma separated.

PEERS_CLIENT="laptop,smartphone,desktop"

The problem is that subnets are comma separated.

SERVER_ALLOWEDIPS=SERVER_ALLOWEDIPS_PEER_${i}

Is is possible to reimplement this line to identify different clients with dot-comma?

In other words, map like peers:

mapfile -t PEERS_ARRAY < <(seq 1 "${PEERS}")

@aptalca
Copy link
Member

aptalca commented Apr 24, 2024

so instead of setting

SERVER_ALLOWEDIPS_PEER_laptop="192.168.1.0/24,192.168.2.0/24"
SERVER_ALLOWEDIPS_PEER_smartphone="192.168.1.0/24,192.168.3.0/24"
SERVER_ALLOWEDIPS_PEER_desktop="192.168.1.0/24,192.168.4.0/24"

you want to set

SERVER_ALLOWEDIPS='laptop="192.168.1.0/24,192.168.2.0/24",smartphone="192.168.1.0/24,192.168.3.0/24",desktop="192.168.1.0/24,192.168.4.0/24"'

?
that barely saves any keystrokes, and the ones saved are all copy/pasteable. It would add a significant complication to the algorithm for virtually no gain.

@drizuid
Copy link
Member

drizuid commented Apr 24, 2024

IMHO this FR would only add complexity that we have to support. I am not in favor.

@thalesmaoa
Copy link
Author

My suggestion is to keep the original var, but remap it as an array breaking by substring.

I will try to make some code for suggestions to clarify.

@thalesmaoa
Copy link
Author

As an example:

I just want to merge all SERVER_ALLOWEDIPS_PEER_ in one variable as follows:

PEER_ALLOWEDIP="SERVER_ALLOWEDIPS_PEER_client1=10.254.1.0/24,SERVER_ALLOWEDIPS_PEER_client2=101.54.13.0/24,SERVER_ALLOWEDIPS_PEER_client3=110.254.11.0/24"
$ echo $PEER_ALLOWEDIP
SERVER_ALLOWEDIPS_PEER_client1=10.254.1.0/24,SERVER_ALLOWEDIPS_PEER_client2=101.54.13.0/24,SERVER_ALLOWEDIPS_PEER_client3=110.254.11.0/24

Now, I just need to extract the corresponding peer information:

PEER="client1"
substring=$(echo "$PEER_ALLOWEDIP" | grep -o "\bSERVER_ALLOWEDIPS_PEER_${PEER}=[^,]*")
echo "Substring for $PEER: $substring"
Substring for client1: SERVER_ALLOWEDIPS_PEER_client1=10.254.1.0/24

SERVER_ALLOWEDIPS in

SERVER_ALLOWEDIPS=SERVER_ALLOWEDIPS_PEER_${i}

can be replaced by substring.

Of course my code is not good and is just to present a point.

If the variable is not defined, it looks for this new pattern, take the substring and change nothing else.

@aptalca
Copy link
Member

aptalca commented Apr 24, 2024

The way you describe it actually doesn't even save any keystrokes. I see no benefit to this, but a bunch of risk and development cost.

Plus, your suggestion would break if you try to add multiple comma separated ranges for a single peer.

Anyway, this is not something we will add due to high cost to benefit ratio.

@aptalca aptalca closed this as not planned Won't fix, can't repro, duplicate, stale Apr 24, 2024
@thalesmaoa
Copy link
Author

Sure, thx anyway. I will try to find another way to fix my problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests

3 participants