tailscale-android
tailscale-android copied to clipboard
Implement USE_EXIT_NODE intent
Closes https://github.com/tailscale/tailscale/issues/8143. I map friendly labels from intent extras to tailscale node IDs, with empty string or not specifying the exitNode intent extra as the "no exit node" action. When an error is encountered, we will push a notification with a friendly message to the status notification channel. The tasker syntax I tested with locally is:
Action: com.tailscale.ipn.USE_EXIT_NODE
Package: com.tailscale.ipn
Class: com.tailscale.ipn.IPNReceiver
Target: Broadcast Receiver
Extra: exitNode:exitNodeLabelOrEmpty
Extra: allowLanAccess:trueOrFalse
@DentonGentry hey, sorry for the ping, just trying to get someone to look at this pr.
I understand and do intend to. We are finishing the 1.56 client release this week. We would want something like this to go in at the start of a new development cycle to potentially appear in 1.58, we need to finish 1.56 first.
I understand and do intend to. We are finishing the 1.56 client release this week. We would want something like this to go in at the start of a new development cycle to potentially appear in 1.58, we need to finish 1.56 first.
Perfectly fine. I just hadn't heard anything so I was making sure this was on the radar 🙂.
Hey @DentonGentry, no pressure intended, but do you have an idea when you'll be taking a look at this?
I hope that the pull request will be merged soon. It's a feature I'm eagerly awaiting. I use MacroDroid for automations, depending on wifi connection etc. At home, "Use exit node" should be off, but on the road, please turn it on.
@DentonGentry @kari-ts, any status updates?
@DentonGentry @kari-ts, I'm trying to be patient here, but the radio silence is really starting to get frustrating. Can I please at least get an idea of when you'll look at this? I don't want to keep resolving merge conflicts if this is just never going to be looked at.
I'm personally dying for this feature. Can it please get added asap?
@333fred thanks for the work here.
I'm making some changes that impact exit nodes now (https://github.com/tailscale/tailscale-android/pull/172, https://github.com/tailscale/tailscale-android/pull/176) - right now, whenever we want to make any changes to prefs, we call SetPrefs, which is intended for setting prefs for the first time, rather than editing the prefs. I also would like to prevent users from being able to use an exit node if they're currently running an exit node, and vice versa.
Once we switch to using EditPrefs instead of SetPrefs, if a user is using an exit node and tries to run an exit node or vice versa, we will fail this check. For this PR, this error should be handled and some UI telling the user why the action failed should be shown.
I see. These intents are normally run in the background, while tailscale isn't open, or indeed while the phone screen isn't even on; I'm not sure what UI would be best to try and show. Either toasts or notifications are likely options here. Perhaps a notification would be best, as toasts can easily be missed?
Adding @sonovawolf for UI input
@sonovawolf thoughts on the UI suggestions? I won't have time to work on the changes this weekend, but may be able to get to it some evening next week if I have a direction to go.
Hey @333fred! We'd prefer to use notifications to communicate those errors.
@kari-ts I've had time to address the notification feedback; given the number of changes to the app since I initially submitted this PR, I simply rewrote the UseExitNodeWorker
in Kotlin and force-pushed (I was also missing the Signed-off-by
in my commits). This is ready for review.
@agottardo strings extracted.
Thanks for the reviews! Anything else you need from me before merging this?
Thanks for the reviews! Anything else you need from me before merging this?
@333fred we'll take care of merging this as soon as QA has signed off on the 1.66 release, to land this PR in the next beta version (and ultimately the 1.68 release). 👍🏻
@333fred sorry to bother you, but we had to refactor quite a few files to fix some last-minute bugs for the release. Can you please rebase this fixing the conflicts, and I'll get it merged? Thanks.
Sorry, I've been getting ready for vacation and am now actually on vacation, so it'll be a bit before I can get to a real computer to rebase.
@agottardo I'm back from vacation and was able to rebase this. As a general piece of feedback, since the contribution guidelines at this point are "we're working on it", trying to get this in has not been a good experience. I would really like to suggest a branching strategy that allows you to work on features for future versions without having to wait for stabilization of the current version; I'm waiting with a bit of dread for a month from now when you tell me that you're ready to merge my PR, but the underlying architecture has been refactored again and I need to do another rewrite of things, as I've needed to do twice now (once majorly with the app rewrite, and once minorly with the notification and app lifetime changes you made in the past month since I updated my PR again).
Thank you @333fred !
Thanks @barnstar, @agottardo, and @kari-ts! Glad to see this finally make it in!