-
Notifications
You must be signed in to change notification settings - Fork 30
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
Allow the configuration of an exit node and lan access. #8
base: main
Are you sure you want to change the base?
Conversation
Thanks for the PR and apologies for the delay on this; reviewing now. |
if [ -v TAILSCALE_USE_EXIT_NODE ]; then | ||
echo "[!] using ${TAILSCALE_USE_EXIT_NODE} as an exit node." | ||
FLAGS="${FLAGS} --exit-node=${TAILSCALE_USE_EXIT_NODE}" | ||
|
||
if [ -v TAILSCALE_EXIT_NODE_ALLOW_LAN_ACCESS ]; then | ||
echo '[!] allowing exit node LAN access.' | ||
FLAGS="${FLAGS} --exit-node-allow-lan-access=${TAILSCALE_EXIT_NODE_ALLOW_LAN_ACCESS}" | ||
fi | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of checking that the env is set, we should also ensure its value is true
Something like:
if [ "${TAILSCALE_USE_EXIT_NODE}" = "true" ]; then
echo "[!] using exit node."
FLAGS="${FLAGS} --exit-node"
if [ "${TAILSCALE_EXIT_NODE_ALLOW_LAN_ACCESS}" = "true" ] || \
[ "${TAILSCALE_EXIT_NODE_ALLOW_LAN_ACCESS}" = "false" ]; then
echo '[!] configuring exit node LAN access.'
FLAGS="${FLAGS} --exit-node-allow-lan-access=${TAILSCALE_EXIT_NODE_ALLOW_LAN_ACCESS}"
else
echo '[!] TAILSCALE_EXIT_NODE_ALLOW_LAN_ACCESS is not set to true or false. Skipping this setting.'
fi
else
echo "[!] TAILSCALE_USE_EXIT_NODE is not set to true. Skipping exit node configuration."
fi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In your example, wouldn't [ "${TAILSCALE_EXIT_NODE_ALLOW_LAN_ACCESS}" = "false" ]
never be true, given we are checking for true before executing that branch?
EDIT: Nevermind, my brain didn't read the full variable. Fixing up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, --exit-node
isn't a true/false flag, it's a setting with a string argument containing the IP address or name of the exit node to use. This change wouldn't make sense in that context.
https://tailscale.com/kb/1103/exit-nodes/#step-4-use-the-exit-node
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good catch. Let's use -n
check for non-empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And change TAILSCALE_USE_EXIT_NODE
to TAILSCALE_EXIT_NODE
. The name made me think it was a boolean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An empty value is valid and is used to disable the feature in the configuration. If we check for empty values we wouldn't be able to turn off the feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was under the impression that up
required the complete set of desired settings, so omitting it should clear it out. If we were using set
that would not be the case. But I tested this and checked tailscale debug prefs
between and you're correct we need to set it to an empty value to disable it.
We could change it for both of the settings to provide the default value if they are unset, or maybe a better solution would be to supply --reset
and only pass the desired settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like transparently passing whatever option the user configures is the best way versus trying to catch all the edge cases. Maybe there's room for another change to provide a --reset
option, but I feel like that's out of scope for this specific PR. Thoughts?
Explicit checking on LAN ACCESS being true or false
I’ve been trying the same base on this PR and struggling with it,
if ! [ -e /dev/net/tun ]; then
FLAGS="$FLAGS --tun=userspace-networking"
if [ -v TAILSCALE_USE_EXIT_NODE ]; then
FLAGS="$FLAGS --socks5-server=localhost:1055 --outbound-http-proxy-listen=localhost:1055"
fi
fi
# configure proxy
sleep 3 # give tailscale a chance to start up
if [ -v TAILSCALE_USE_EXIT_NODE ]; then
if [ -d /var/run/s6/container_environment ]; then
printf "socks5://localhost:1055/" > /var/run/s6/container_environment/ALL_PROXY
printf "http://localhost:1055/" > /var/run/s6/container_environment/HTTP_PROXY
printf "http://localhost:1055/" > /var/run/s6/container_environment/http_proxy
fi
fi |
@LimeDrive thanks for sharing. Based on your work it sounds like my PR might not even be appropriate. |
@LimeDrive do you have a fork where I can try out your changes? Did you find that the proxy settings were always required for exit node use, or only when limited to userspace networking? |
That was the fork I used for testing: https://github.com/LimeDrive/tailscale-mod/pkgs/container/tailscale-mod/131781718?tag=main Proxy settings should only be added with user space networking on init of the mod. I gave up after testing mod with an exit node and found it more efficient to share the container VPN network with Docker for this kind of setup. |
This PR might be dead but this seemed relevant and I didn't want to open a new issue. While working on my own container, I ended up reading through all the files in this project as I figured out how everything fits together, s6, etc. That's how I came across this variable name typo in the svc-tailscale-up script in the exit node section. I don't have an opinion on whether or not this feature belongs in the docker-mod and I know it's undocumented, but I just wanted to mention it.
|
No description provided.