Use strict DNS for mesh gateways with hostnames

[andrewstucki]

Description

This fixes #17557. In an attempt to support mesh gateways fronted by AWS load balancers, a code path for peered mesh gateways was introduced in #14917 that leverages envoy clusters backed by the LOGICAL_DNS cluster discovery type. Problematically, when replicas of mesh gateways exist in a peered connection, the dialing peer will hit this code path and attempt to add multiple endpoints for the targeted mesh gateways. Envoy, however, doesn't support multiple endpoints using LOGICAL_DNS and will start spitting out errors applying the xDS it receives from Consul.

In Kubernetes, when a mesh gateway restarts then, it will never finish initializing and get marked as healthy, so its pod will continually restart and the gateway becomes unusable.

Because this requires using hostnames rather than IP addresses for the WAN addresses registered for mesh gateways, it likely impacts mostly Consul users on AWS, where hostnames are used for LoadBalancer services and thus registered for LoadBalancer type mesh gateways. This will also affect users who manually (or with annotations) register mesh gateways with mutiple FQDNs.

Note that this appears to only affect the dialing cluster in a peered connection, the accepting clusters use a different code path that only ever uses a single mesh gateway target and doesn't attempt to load-balance between multiple mesh gateways.

Testing & Reproduction steps

I was able to recreate this pretty easily outside of AWS by pinning the FQDN of the mesh gateways in the accepting cluster via something like:

meshGateway:
  enabled: true
  replicas: 2
  wanAddress:
    source: "Static"
    static: "gateway.nanosleep.cloud"

which gives this for my dialing cluster:

curl https://${DC2_CONSUL}/v1/peerings ... | jq
...
"PeerServerAddresses": [
  "gateway.nanosleep.cloud:443",
  "gateway.nanosleep.cloud:443"
],
...

And dialing cluster Consul logs then show:

2023-10-17T22:37:29.016Z [ERROR] agent.envoy.xds.mesh_gateway: got error response from envoy proxy: service_id=default/default/consul-consul-mesh-gateway-6ff745887b-5c5s2 typeUrl=type.googleapis.com/envoy.config.cluster.v3.Cluster xdsVersion=v3 nonce=00000006 error="rpc error: code = Internal desc = Error adding/updating cluster(s) server.dc1.peering.303380e1-f1a6-fb04-4ca6-c562e4951539.consul: LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint"

and dialing cluster mesh gateway:

2023-10-17T22:36:20.838Z+00:00 [warning] envoy.config(14) gRPC config for type.googleapis.com/envoy.config.cluster.v3.Cluster rejected: Error adding/updating cluster(s) server.dc1.peering.303380e1-f1a6-fb04-4ca6-c562e4951539.consul: LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint

Swapping to STRICT_DNS allows the mesh gateway to finish configuration and boot properly.

Links

Strict DNS in envoy.

PR Checklist

• updated test coverage
• external facing docs updated
• appropriate backport labels added
• not a security concern

[freddygv]

Is there a reason why this wouldn't impact all the callers of the hostnameEndpoints function? Would include connect proxies, terminating gateways, and mesh gateways used for WAN federation

[andrewstucki]

@freddygv do you mean to say that you think that this would likely manifest in places other than just mesh gateways? The change that I made is just scoped to makeExternalHostnameCluster (not hostnameEndpoints) which is only called in constructing clusters for terminating gateways and mesh gateways. I kept terminating gateways the same to keep the scope of this small and because I can't think of a way of generating an external service that has more than a single address associated with it, which would be needed to trigger this.

Are the cases you mentioned places where we push both clusters that can have multiple primary addresses associated with them and leverage LOGICAL_DNS? If so, I'm kind of surprised this wouldn't have been run into earlier, but I can take a look at the code paths to make sure.

Edit: and to clarify -- I think the normal use-cases you mentioned (i.e. connect proxies, etc.) generally resolve services solely by IP, so they would use EDS-based service discovery, rather than LOGICAL_DNS no?

[andrewstucki]

So, to follow up, it looks like the other code paths that actually leverage LOGICAL_DNS all go through configureClusterWithHostnames which has some logic since your PR #8084 (3+ years ago 😄) that just takes the first healthy endpoint in the list, logs a warning message and uses it. So, as far as I can see the only remaining uses are in makeExternalHostnameCluster -- which means only dialing mesh gateways and potentially(?) terminating gateways (which I'm not sure how to trigger this with) would be affected by this.

[sarahalsmiller]

Does it make sense for discoveryType to be added to clusterOpts instead of as a separate entry field? Otherwise looks good to me, thanks for knocking this out.

[andrewstucki]

I don't think so, mainly b/c clusterOps is used a bunch of places in terms of cluster generation and most existing functions would never use it -- I don't like the idea of having a "catch all" options structure where only a couple of functions actually use certain fields on the structure, it makes usage harder to track down IMO.

[sarahalsmiller]

Small nit question, but otherwise looks good to me

[hc-github-team-consul-core]

Backport failed @andrewstucki. Run: https://github.com/hashicorp/consul/actions/runs/6631399210