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

use new getListenerFromPlugin signature #83

Closed
wants to merge 4 commits into from

Conversation

MayCXC
Copy link

@MayCXC MayCXC commented Dec 13, 2024

this is for when caddyserver/caddy#6651 gets released

@MayCXC MayCXC force-pushed the update-new-func-signature branch 2 times, most recently from 14e06e5 to cbd3bb3 Compare December 14, 2024 05:00
@jurekl
Copy link

jurekl commented Dec 27, 2024

I think that in each of the getXXXListener functions the following correction should be made, otherwise I can't compile the plugin code

...
na, err := caddy.JoinNetworkAddress(network, host, portRange)
...

caddy.JoinNetworkAddress() returns a string, so that's definitely not correct, but it will probably be fine after changing it to:

...
na, err := caddy.ParseNetworkAddress(caddy.JoinNetworkAddress(network, host, portRange))
...

EDIT:
After this change, I can formally build this plugin, but it does not work as it should :
Example error:
Error: loading initial config: loading new config: http app module: start: starting HTTP/3 QUIC listener: tsnet: address invalid AddrPort: missing port in address

Since I am actively using the caddy-tailscale module, I have to revert to the caddy commit from before the ListenerFunc declaration change for now :-(

@jurekl
Copy link

jurekl commented Jan 15, 2025

Last commit 2 months ago, I wonder if project caddy-tailscale is still active/alive? After the aforementioned significant change in the caddy function declaration, building this plugin in its current form is impossible.
It seemed to me that this plugin has many very satisfied and active users. Meanwhile, after the aforementioned change in caddy, we already have two new caddy tags, the last one 2.9.1.
This PR does not solve the problem I mentioned above, it simply does not compile. I am very curious if in this situation users of this plugin do not update caddy, like me?
Or maybe there is another solution, something I do not know about? I would be happy to use the knowledge/practice/experience of others :-)
Best regards...

MayCXC and others added 4 commits January 24, 2025 18:37
Signed-off-by: Aaron Paterson <[email protected]>
Updates tailscale/tailscale#8905

Signed-off-by: Will Norris <[email protected]>
Signed-off-by: Aaron Paterson <[email protected]>
This reverts commit 48c1c24.

Signed-off-by: Aaron Paterson <[email protected]>
Signed-off-by: Aaron Paterson <[email protected]>
@MayCXC MayCXC force-pushed the update-new-func-signature branch from d423cd9 to 665bec7 Compare January 24, 2025 23:37
@MayCXC MayCXC marked this pull request as ready for review January 24, 2025 23:37
@MayCXC
Copy link
Author

MayCXC commented Jan 24, 2025

Last commit 2 months ago, I wonder if project caddy-tailscale is still active/alive? After the aforementioned significant change in the caddy function declaration, building this plugin in its current form is impossible. It seemed to me that this plugin has many very satisfied and active users. Meanwhile, after the aforementioned change in caddy, we already have two new caddy tags, the last one 2.9.1. This PR does not solve the problem I mentioned above, it simply does not compile. I am very curious if in this situation users of this plugin do not update caddy, like me? Or maybe there is another solution, something I do not know about? I would be happy to use the knowledge/practice/experience of others :-) Best regards...

My apologies, and thanks for #89 . The missing ParseNetworkAddress was a git error, but I have not seen anything like the AddrPort error you got after. I can try to debug it if you can provide an example configuration to reproduce it, otherwise I think this PR should be closed in favor of yours.

@MayCXC MayCXC closed this Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants