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

Make Proxy and Funnel port configurable #277

Conversation

lmagyar
Copy link
Contributor

@lmagyar lmagyar commented Oct 19, 2023

Proposed Changes

Also made the port configurable. Proxy and Funnel are using the same port, so making 2 port configs and dropping the current proxy: bool? and funnel: bool? configs can't be done. So 443 by default is configured as port, but this won't start proxy or funnel automatically.

Related Issues

fixes #276

@lmagyar lmagyar marked this pull request as draft October 19, 2023 21:30
@lmagyar lmagyar force-pushed the pr-make-proxy-and-funnel-port-configurable branch from 9105408 to 8a64752 Compare October 19, 2023 21:44
tailscale/config.yaml Outdated Show resolved Hide resolved
@lmagyar
Copy link
Contributor Author

lmagyar commented Oct 19, 2023

Finally moved all non port related code into the #273 PR, because now it's much clearer which service and PR does what.
Now both are ready to review.

@lmagyar lmagyar marked this pull request as ready for review October 19, 2023 22:17
@frenck frenck marked this pull request as draft October 22, 2023 17:12
@lmagyar lmagyar force-pushed the pr-make-proxy-and-funnel-port-configurable branch from 8a64752 to fdf1524 Compare October 22, 2023 17:29
@lmagyar
Copy link
Contributor Author

lmagyar commented Oct 22, 2023

Rebased it, and ready to review.

@lmagyar lmagyar marked this pull request as ready for review October 22, 2023 17:30
@lmagyar lmagyar force-pushed the pr-make-proxy-and-funnel-port-configurable branch 2 times, most recently from c0c7332 to 278d907 Compare October 23, 2023 19:53
Copy link

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

@github-actions github-actions bot added the stale There has not been activity on this issue or PR for quite some time. label Nov 23, 2023
@lmagyar
Copy link
Contributor Author

lmagyar commented Nov 23, 2023

Not stale, decision is to be made:

  • move this option from port config to plain config like proxy_and_funnel_port: port? (because this is not a real host port, it is available only in the tailnet)
  • or forget it in favor of PR Use new tailscale cli arguments and merge proxy and funnel into longrun serve service #306 (which allows using the cli for serve config, but I think it is a big step for an average user to use command line to configure a basic thing like a port, so I prefer to move this option to a plain option and leave advanced command line for more experienced users)

@github-actions github-actions bot removed the stale There has not been activity on this issue or PR for quite some time. label Nov 24, 2023
@lmagyar lmagyar force-pushed the pr-make-proxy-and-funnel-port-configurable branch from 278d907 to f6853db Compare December 16, 2023 18:11
@frenck
Copy link
Member

frenck commented Jan 7, 2024

I actually think we need to move this to an add-on option, which also allows us to guard that the port number is in one of the accepted ones.

I feel like this is a bit too much abusing a port feature that was not meant for this usecase (and allows for any port, even if it doesn't make sense).

../Frenck

@frenck
Copy link
Member

frenck commented Jan 7, 2024

which allows using the cli for serve config

We should never expose that.

@frenck frenck added the new-feature New features or options. label Jan 7, 2024
@frenck frenck marked this pull request as draft January 7, 2024 13:34
@lmagyar lmagyar force-pushed the pr-make-proxy-and-funnel-port-configurable branch from f6853db to 5a4a3f0 Compare January 7, 2024 17:54
@lmagyar
Copy link
Contributor Author

lmagyar commented Jan 7, 2024

I've used proxy_and_funnel_port: list(443|8443|10000)? option to configure it.

Issues:

  • This limits the port number to this 3, even when only Proxy (tailscale serve) is used, though this is a Funnel restriction. But I think this will cause less complaints than enabling any port for Proxy that runs on to error when the user later enables Funnel.
  • The generated YAML configuration looks a bit ugly for an integer port number: proxy_and_funnel_port: "8443"
  • If the user edit the YAML and enters proxy_and_funnel_port: "8443" the UI will not give an error, but the non-YAML view can't select the right "8443" from the list. The add-on works certainly. This seems to be a UI bug around int/str values.

Otherwise ready to review.

@lmagyar lmagyar marked this pull request as ready for review January 7, 2024 18:24
@lmagyar lmagyar changed the title Proxy and Funnel port is configurable Make Proxy and Funnel port configurable Jan 7, 2024
@frenck
Copy link
Member

frenck commented Jan 7, 2024

Hmm right, the list is considered a list of strings by the SU.

Copy link
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's see how it behaves. Thanks, @lmagyar 👍

../Frenck

@frenck frenck merged commit d6d8a51 into hassio-addons:main Jan 7, 2024
11 checks passed
@lmagyar
Copy link
Contributor Author

lmagyar commented Jan 7, 2024

Wait, wait!

We can use a regex!

@lmagyar
Copy link
Contributor Author

lmagyar commented Jan 7, 2024

Give me a few minutes to test it.

@lmagyar lmagyar deleted the pr-make-proxy-and-funnel-port-configurable branch January 7, 2024 19:19
@github-actions github-actions bot locked and limited conversation to collaborators Jan 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new-feature New features or options.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tailscale Add-on's embedded Nginx proxy intercepts all in-bound HTTPS traffic from tailnet
3 participants