core: Change getListenerFromPlugin signature

[MayCXC]

I wanted #6622 to go into the next release 100%, because it cleaned up an unnecessary func added in #6573 . this PR separates the address, port range, and port offset in getListenerFromPlugin . this lets plugins create listeners in the caddy pool if they want, and map port ranges to their own groups of listen addresses (e.g systemd socket units) in a direct way.

[mohammed90]

this PR renames NetworkAddress JoinHostPort to JoinAt

Can you share more on the reasoning for the rename? It's one of the oldest methods in Caddy codebase, and I worry the rename will break plugins relying on it.

[MayCXC]

this PR renames NetworkAddress JoinHostPort to JoinAt

Can you share more on the reasoning for the rename? It's one of the oldest methods in Caddy codebase, and I worry the rename will break plugins relying on it.

the rename is just to make its syntax distinct from net.JoinHostPort, because it has no host or port parameters. I did intend for the change to affect plugins, because when the host port and offset get passed together to getListenerFromPlugin, it is confusing which should be passed to net.JoinHostPort v.s. na.JoinHostPort without the rename.

[mholt]

the rename is just to make its syntax distinct from net.JoinHostPort, because it has no host or port parameters.

True, but arguably, that's even more reason to keep the HostPort part in the name, since it's not obvious that's what it's joining otherwise.

I think we should leave this one, since it's been around a while and may be used in a lot of places.

[MayCXC]

& to address your concern, currently any plugins passing a port offset to na.JoinHostPort are making one up, because before this PR they did not know the actual offset of the port they were passed, or the full port range. so that break is intentional / overlapping with the getListenerFromPlugin change.

[MayCXC]

the rename is just to make its syntax distinct from net.JoinHostPort, because it has no host or port parameters.

True, but arguably, that's even more reason to keep the HostPort part in the name, since it's not obvious that's what it's joining otherwise.

I think we should leave this one, since it's been around a while and may be used in a lot of places.

I'm with it if we can figure out where the offset is coming from. if plugins actually use it, it probably needs to be changed to the new offset parameter in this branch.

[mohammed90]

I'm with it if we can figure out where the offset is coming from

The NetworkAddress is an address family and IP address and a port range, where the offset is 0 at the first port in the range, and it extends to NetworkAddress#PortRangeSize. Typical usage is to loop for PortRangeSize iterations and do JoinHostPort at the offset of the index of the iteration.

I admit, it's my fault for not making the method return an error if the offset join result goes beyond the defined range.

[MayCXC]

I'm with it if we can figure out where the offset is coming from

The NetworkAddress is an IP address and a port range, where the offset is 0 at the first port in the range, and it extends to NetworkAddress#PortRangeSize. Typical usage is to loop for PortRangeSize iterations and do JoinHostPort at the offset of the index of the iteration.

I admit, it's my fault for not making the method return an error if the offset join result goes beyond the defined range.

I think I get it, and that port range would come from config / a custom module? I only considered plugins using RegisterNetwork, so if plugins are reading port ranges from somewhere else I'll revert.

[mohammed90]

I think I get it, and that port range would come from config / a custom module?

It comes from user configuration. It's the value set with bind in the Caddyfile and apps.http.servers.[index/name].listen in the JSON structure.

[MayCXC]

ok, RegisterNetwork plugins will be fine with the current func name. next I will go back in time and propose JoinHostPortAt

[MayCXC]

preemptive caddy-tailscale update: MayCXC/caddy-tailscale@afda8ef

[mholt]

Thanks, let's try this!