addons icon indicating copy to clipboard operation
addons copied to clipboard

nginx_proxy: Listen on IPv6 by default

Open mraerino opened this issue 1 year ago • 6 comments

This makes nginx listen on IPv6. That's it. There is no functionality change to the IPv4 path that has existed so far.

Benefits include:

  • The trusted_networks auth backend in core now works with IPv6 when using nginx
  • Enables using IPv6 to expose the homeassistant install to the world (with some extra config also allows getting a cert via LE)

I've used an extra listening directive instead of [::]:443 ipv6only=off because with that config ipv4 gets ipv6-mapped in access logs and people might not want that.

Related issues

https://github.com/home-assistant/supervisor/issues/2133

mraerino avatar Feb 18 '24 20:02 mraerino

I've used an extra listening directive instead of [::]:443 ipv6only=off because with that config ipv4 gets ipv6-mapped in access logs and people might not want that.

Yeah I agree, this makes sense. :+1:

Can you also adjust the CHANGELOG.md and bump version in config.yaml? I think this change probably deserves a major bump, IMHO.

agners avatar Feb 19 '24 12:02 agners

I think this change probably deserves a major bump, IMHO.

so you see this as a breaking change? what do you think could cause issues about this change? according to semver, new feature (like ipv6 support) would be considered minor bumps?

mraerino avatar Feb 19 '24 12:02 mraerino

so you see this as a breaking change? what do you think could cause issues about this change? according to semver, new feature (like ipv6 support) would be considered minor bumps?

I guess this is very much debatable. Semver is quite clear for libraries or software with a clear API. However, add-ons pack-up many pieces of software. Does a major bump of anyone of those mandate a major bump of the add-on? :thinking:

Now in this case it is really a default configuration change of the add-on. The outward facing "API" if we want to call it like that indeed changes. You definitely can argue this is only adding a feature, so a minor bump should be sufficient. :man_shrugging:

agners avatar Feb 19 '24 13:02 agners

i did the version bump

mraerino avatar Feb 19 '24 15:02 mraerino

@agners how can i help move this forward?

mraerino avatar Feb 25 '24 21:02 mraerino

First off, I need to add that currently IPv6 support works even without this PR. But that is because the add-ons have no IPv6 support at all. In that environment, it seems that Docker exposes the ports by default on all address families:

        "NetworkSettings": {
...
            "Ports": {
                "443/tcp": [
                    {
                        "HostIp": "0.0.0.0",
                        "HostPort": "1443"
                    },
                    {
                        "HostIp": "::",
                        "HostPort": "1443"
                    }
                ]
            },

As I've mentioned in https://github.com/home-assistant/supervisor/issues/2133#issuecomment-1970813281, I don't 100% understand your environment. But how does the NetworkSettings of NGNIX look in your environment?

In essence, this change will only become relevant once we add IPv6 support. Currently there is no IPv6 at all inside the container (not even a link local address). Have you tested this change on a installation with the current setup? I am not against merging that already today, but we should be careful to not break current installations :sweat_smile:

agners avatar Feb 29 '24 10:02 agners