feat(listens) support multiple addresses and IPv6

rainest commented 7 months ago

What this PR does / why we need it:

Configure default IPv6 listens for Kong listeners by default. The controller defaults are unchanged as the controller settings do not support multiple listen addresses. Note that this diverges from upstream: kong.conf.default does not bind to IPv6.

Replace the hidden .address service setting with a new hidden .addresses setting. The new setting is a list whereas the old setting was a single string.

Honor address overrides for the admin listen. It previously forced you to the default 0.0.0.0 address or 127.0.0.1 when disabled.

Which issue this PR fixes

Fix #847.

Relevant to https://github.com/Kong/kubernetes-ingress-controller/issues/5138.

Special notes for your reviewer:

See listen_tests.txt for example settings using values.yaml.txt

Umbrella chart shenanigans mean we can't really check this in ingress easily. I stuffed the modified kong chart into ingress.tar.gz. It requires a nightly to actually work because of kic#5138:

helm install ana /tmp/symingress --set controller.ingressController.image.repository="kong/nightly-ingress-controller" --set controller.ingressController.image.tag="nightly" --set controller.ingressController.image.effectiveSemver=3.1.0

The old address field was never documented and AFAIK there shouldn't be much reason to use it (containers in Kubernetes shouldn't have reason to listen on anything other than all addresses or localhost only). The change to addresses is technically breaking but I'm considering it not so because it was effectively a developer-only setting.

Checklist

[x] PR is based off the current tip of the main branch.
[x] Changes are documented under the "Unreleased" header in CHANGELOG.md
[x] New or modified sections of values.yaml are documented in the README.md
[x] Commits follow the Kong commit message guidelines

czeslavo commented 7 months ago

Was making it an opt-in setting (bind IPv6 only if explicitly asked for) an option? I'm not sure if that's a good direction to diverge from the defaults Kong uses.

pmalek commented 7 months ago

Was making it an opt-in setting (bind IPv6 only if explicitly asked for) an option? I'm not sure if that's a good direction to diverge from the defaults Kong uses.

I share the same concern. Do we really want to flip this default for all users? Will that work on all machines, e.g. what if I have IPv6 disabled? Is that a noop for me then?

Would it be possible to make this a proper chart value, accepting a list with the default having a single entry: 0.0.0.0. Add a comment in values.yaml saying that if IPv6 is desired then add [::] to that list. I believe the implementation in the templates would basically remain the same but it would allow users to configure that. WDYT?

rainest commented 7 months ago

tl;dr

I think we should avoid yet one (or N=Service count) more config setting you have to set just to get a working install if we can. If we can support IPv6-only clusters out of the box without you needing to toggle on "IPv6 mode" explicitly, we should.
My intuition is that not many installs would need to override defaults to explicitly disable an address family the cluster supports. If we get reports that people actually do need that, we can add the settings in later, but adding them preemptively probably just gets us a setting nobody will actually use.
Because some nuances around how Services work, this effectively only matters for IPv6-only clusters. Dual-stack clusters still default to IPv4 only in practice.
There are no obvious technical issues with using the new defaults on dual stack or IPv4-only clusters. The container environment always supports dual stack. Its IPv6 address goes unused if the cluster isn't set up to use it.
Discovery does unfortunately have a duplicated update bug with dual stack, though the Service nuances mean that doesn't show up in practice as-is.

I'd like to avoid requiring a toggle to support something that should arguably work out of the box if it's available in the environment.

Unfortunately absent ongoing user surveys we don't have a good way to float new features, so turning it on and seeing if it causes issues for someone is the less than ideal way to see if we need a toggle.

Even with one I'd say it should default to IPv6 enabled, since it's only relevant if the wider environment supports it. If you have a dual-stack cluster but have somehow broken your networking such that only IPv4 actually works, you should probably disable IPv6 at the environment level rather than expecting applications to not try using IPv6.

The controller lacking support for multiple listens de facto still requires changing defaults for v6-only clusters, but ideally we address that in the future.

I don't want to formally expose addresses ever (AFAIK there shouldn't ever be a reason to bind to explicit addresses instead of wildcard/wildcard local insisde the container), but we could expose https://kubernetes.io/docs/concepts/services-networking/dual-stack/#services and infer the appropriate listens from the enabled families.

It is extra template logic and extra config burden (especially if we do it "properly" and let you set it per-Service--I'd probably ignore that and expose a single setting/require you use the same setting for every Service) though, so if we can live without it, we save one more setting that <1% of installs will actually use.

Will that work on all machines, e.g. what if I have IPv6 disabled?

Yeah. The only problem case is if you're using a single-stack listen for only the protocol your cluster doesn't support. More detail below, but from Kong's perspective its environment does support either address family. It can bind to IPv6 even if those container-internal addresses aren't reachable from outside. Having a link-local IPv6 address doesn't interfere with IPv4 traffic.

For many dual-stack clusters we actually won't use IPv6 at the cluster level anyway. Kubernetes has a an apparently hard-coded default Service configuration that defaults to single-stack only based on the first IP range you configure. If you configure IPv4 addresses first, you'll actually only use IPv4, at least for Services, unless we do expose the family/stack Service settings.

If you configure IPv6 first, you were presumably in a broken state similar to IPv6-only clusters, since the Service would effectively be IPv6-only and clients would only try to talk v6 and fail because there was no corresponding container listen. I assume these were probably uncommon in practice given the apparent absence of complaints about us not working on dual-stack clusters.

Since we're using Service Endpoints for discovery, you'll only get the address family the Service supports even if the Pod has both families assigned (AFAICT this holds for both normal and headless Services equally). Most installs will still use IPv4 only even on dual-stack clusters.

Testing discovery with a manually-configured dual stack admin Service does indicate that it's a bit boneheaded about this though. IIRC the code is just listing every endpoint and assuming every address it finds is a Pod (which is true in single stack), and the updates then fire twice. In a single proxy replica environment:

2024-01-19T00:11:53Z    debug   events  successfully applied Kong configuration to https://[fd00:10:244::10]:8444   {"v": 1, "type": "Normal", "object": {"kind":"Pod","namespace":"default","name":"ana-controller-6cd6c6dd9b-kptm9","apiVersion":"v1"}, "reason": "KongConfigurationSucceeded"}
2024-01-19T00:11:53Z    debug   events  successfully applied Kong configuration to https://10.244.0.16:8444 {"v": 1, "type": "Normal", "object": {"kind":"Pod","namespace":"default","name":"ana-controller-6cd6c6dd9b-kptm9","apiVersion":"v1"}, "reason": "KongConfigurationSucceeded"}

2024-01-19T00:11:56Z    debug   events  successfully applied Kong configuration to https://10.244.0.16:8444 {"v": 1, "type": "Normal", "object": {"kind":"Pod","namespace":"default","name":"ana-controller-6cd6c6dd9b-kptm9","apiVersion":"v1"}, "reason": "KongConfigurationSucceeded"}
2024-01-19T00:11:56Z    debug   events  successfully applied Kong configuration to https://[fd00:10:244::10]:8444   {"v": 1, "type": "Normal", "object": {"kind":"Pod","namespace":"default","name":"ana-controller-6cd6c6dd9b-kptm9","apiVersion":"v1"}, "reason": "KongConfigurationSucceeded"}

On an IPv4-only KIND instance, I'm using the (new) default dual-stack admin bind. The controller uses the IPv4 cluster IP because there's no v6 IP available and successfully talks to the admin API:

$ kubectl logs ana-gateway-llrq8 | grep admin_listen
2024/01/18 22:18:54 [debug] admin_listen = {"0.0.0.0:8444 http2 ssl","[::]:8444 http2 ssl"}

$ kubectl logs ana-controller-zxghl | grep 8444
...
2024-01-18T22:18:57Z    info    Successfully synced configuration to Kong   {"url": "https://10.244.0.12:8444", "update_strategy": "InMemory", "v": 0}

$ cat /tmp/v4.conf                             
kind: Cluster          
apiVersion: kind.x-k8s.io/v1alpha4
networking:
  ipFamily: ipv4
  apiServerAddress: 127.0.0.1

Going into more detail, TIL that containers do not inherit their host's network capabilities, which I guess makes sense. On the kubelet:

root@boop-control-plane:/# sysctl -A 2>/dev/null | grep net.ipv6.conf.default.disable_ipv6
net.ipv6.conf.default.disable_ipv6 = 1

versus in the proxy container:

kong@ana-gateway-llrq8:/$ sysctl -A 2>/dev/null | grep net.ipv6.conf.default.disable_ipv6  
net.ipv6.conf.default.disable_ipv6 = 0

Can't see a whole lot further in the container since we lack most of the standard Linux network tools (and I can't figure it out from /proc info) and NGINX isn't reporting low-level bind info on success, but I'm guessing that we still get a self-assigned v6 address and bind to that. For comparison, using an explicit address we don't have in the container definitely fails:

nginx: [emerg] bind() to 9.9.9.9:8999 failed (99: Cannot assign requested address)

In that sense it does probably make sense for upstream kong.conf.default to default to IPv4 only, since it may well end up being installed into a v4-only environment and would fail without overrides. In our case, however, we can rely on the container always supporting both and don't need to be as conservative.

rainest commented 7 months ago

Rebase to fix conflicts and added a release as well, since someone wanted the other thing that got merged.

lel-amri commented 4 months ago

Thanks for making IPv6 easier to use. I'm hitting an issue at work on an IPv4 only cluster. It seems that the "container environment" does not set any IPv6 address in our pods networks namespaces. AFAIK, the network setup (lo included) is handled by the CNI, which, in our case, is explicitly told to not set IPv6. @rainest do you have a quote from a specification that says "the container environment always supports dual stack"? If that's not the case, I'd consider this default configuration a bug.

Kong / charts