-
-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor tailscale nginx-auth module #293954
Refactor tailscale nginx-auth module #293954
Conversation
1fcfcac
to
c4dca65
Compare
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/3618 |
81238e5
to
0e92ac0
Compare
0e92ac0
to
56b0fe8
Compare
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.
Good news! Thanks for bringing this to attention.
Do you think it would make sense to wait for results of the upstream issue you linked? I.e. merging this before a rename means, that the program is confusingly still called tailscale-nginx-auth and that it's missing the '/auth-forward' endpoint, suggested in tailscale/tailscale#4680.
56b0fe8
to
0e92ac0
Compare
Thank you both for reviewing.
Caddy officially documents support for the feature. Their suggested workaround for lacking The current upstream situation isn't ideal, but I think it's good enough. There have not been many signs of life on the upstream ticket in the past year, and it is unclear to me if/when improvements are coming. |
0e92ac0
to
811045b
Compare
I think I've addressed all feedback now. |
811045b
to
f2968e6
Compare
f2968e6
to
f5bef73
Compare
7cf2818
to
f39d198
Compare
Got my maintainers entry added in another PR, so I've just rebased this. |
This additional module allows the tailscale auth proxy to be configured independently of nginx. The tailscale auth proxy works with both caddy and traefik. All prior nginx/tailscale-auth options are retained as aliases.
f39d198
to
3cf6c4d
Compare
I've rebased again to resolve conflicts with #303841. I also removed a remaining |
Description of changes
Tailscale's nginx-auth proxy, despite the name, actually works just fine with caddy and traefik (see tailscale/tailscale#4680). However, the current nixos module is closely coupled with nginx. This change breaks that coupling.
All options of the existing module except
virtualHosts
(which is nginx-specific) have been moved to a new module without any nginx coupling. The options remain exposed on the nginx-specific module as aliases.closes #290353
@phaer
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 馃憤 reaction to pull requests you find important.