nextcloud / helm

A community maintained helm chart for deploying Nextcloud on Kubernetes.
GNU Affero General Public License v3.0
334 stars 269 forks source link

feat(nginx): add ipFamilies option for IPv4 and IPv6 #639

Closed mdallaire closed 1 month ago

mdallaire commented 2 months ago

Description of the change

Add an option to configure the listening IP address for the nginx container.

Benefits

This will allow setting the IP to a value like "[::]" for IPv6 deployments. The current generated configs force the address to 0.0.0.0

Possible drawbacks

None that I can think of. The default value is 0.0.0.0 which is equivalent to the configuration generated by the chart at the moment.

Applicable issues

Additional information

Checklist

wrenix commented 1 month ago

should we add an array of multiple adresses (dualstack) or listen just on both (ipv4 and ipv6)

provokateurin commented 1 month ago

Array sounds like a good idea, if we add it before merging this it also doesn't result in a breaking change. As for always listening on both, I don't know if that is a good idea. If ipv6 is disabled in the kernel the container will always fail. So array with both ipv4 and ipv6 by default would make sense to me, then it can still be reduced to ipv4 only if necessary.

mdallaire commented 1 month ago

That would be achieved with the ipv6only nginx option. I can add it to the chart.

mdallaire commented 1 month ago

It could also be a single option similar to the ipFamilies for services. This array with possible values of IPv4 and IPv6 or both would decide how both listening address and ipv6only is built in the configuration template.

Let me know how you prefer this be done and I will adjust the PR.

provokateurin commented 1 month ago

(A squash of all commits would be nice to have)

mdallaire commented 1 month ago

(A squash of all commits would be nice to have)

Done!

provokateurin commented 1 month ago

Agreed, we can easily fix this if there is a problem.

mdallaire commented 1 month ago

I am just thinking of dualstack, is not everywhere automatically on if listen [::]:80;.

When listen [::]:80; is set, nginx will only listen to IPv6 (the default value for the ipv6only option is on ref). This is why we have to set listen [::]:80 ipv6only=off; when both IPv4 and IPv6 are present in the array.