Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add Tailscale ZTA module #2207
Add Tailscale ZTA module #2207
Changes from all commits
d18f4f7
b5636dd
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 my local testing (against Mac App Store-installed tailscale) it was enough to set
LIVEBOOK_IP
to$(tailscale ip --1)
so maybe we show that here too?As an aside, @josevalim: would it be possible to just set
LIVEBOOK_IDENTITY_PROVIDER=tailscale
and do the rest in Elixir e.g. when the app boots? I think it would be a much better UI if users don't have to copy paste a bunch of shell commands.Similarly, perhaps we don't need to require users to set
LIVEBOOK_IP
but can do that ourselves internally.That being said I think it is totally fine to start with something really simple if verbose and improve this as more people use 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.
Let's update the examples to use
tailscale ip --1
but require those env vars to be set. I don't want to rely onlsof
for macos and the setting ofLIVEBOOK_IP
happens at a very different place which I am not sure it is worth coupling.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.
It works if you have a Tailscale alias in your shell, I got this command here because it (should) work on all types of MacOS installs without requiring an alias to be set.
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 point, the CLI doesn’t get installed by default. Fine by me.
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.
Or in our instructions we do the alias. I think that’s better. And if we can somehow get the port and password out of the cli with less code that’s even better. But no strong opinion.
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.
if the socket path is incorrect (see my comment above), the error is pretty bad:
my suggestion is to handle this particular scenario early on, perhaps something like this:
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.
Added!