Consul service mesh upstreams are not respecting the global proxy-defaults

[crhino]

Overview of the Issue

Users expect to be able to set the protocol in the proxy-defaults config entry and have it apply to upstream services. However, it currently does not.

Reproduction Steps

1. Set up a global proxy defaults:

kind = "proxy-defaults"
name = "global"
namespace = "default"
config {
  protocol = "http"
}

2. Configure upstreams for a service on the mesh, without specifying the protocol:

"upstreams": [
  {
    "destination_name": "counting",
    "local_bind_port": 5000
  },
  {
    "destination_name": "fortio",
    "local_bind_port": 6000
  }
]

3. Check the envoy configuration for the service's sidecar proxy and see that the upstream listeners are not configured with http connection manager, but instead have the regular tcp_proxy config:

        "filters": [
          {
           "name": "envoy.tcp_proxy",
           "config": {
            "stat_prefix": "upstream_fortio_tcp",
            "cluster": "fortio.default.chris1.internal.cedad92f-c48c-8527-afb6-e3023da43538.consul"
           }
          }
         ]

[crhino]

Ah, I believe I see the problem here. We check IsDefault() here and proceed into a case that does not respect the chain.Protocol, only the upstream protocol parsed from the Upstream.Config.

IsDefault() will return true when only a proxy-defaults config entry is defined, showcased by this testcase

Potentially we could make IsDefault() return false if a non tcp protocol is set? Not sure if that has second-order consequences yet or not. Another option is to respect the chain.Protocol in the function referenced.

[mikemorris]

Do we know if this is a regression, or just unexpected behavior that differs from docs?

[rboyer]

It is broken in 1.6.5, 1.7.0, 1.7.1, 1.7.2, and 1.7.3

[rboyer]

Reproduction:

$ consul agent -dev -hcl 'enable_central_service_config = true' &

$  echo '
kind = "proxy-defaults"
name = "global"
config {
protocol = "http"
}
' | consul config write -

$ cat foo.hcl
service {
  name = "foo"
  port = 8888

  connect {
    sidecar_service {
      proxy {
        upstreams = [
          {
            destination_name = "web"
            local_bind_port  = 5000
          },
        ]
      }
    }
  }
}

$ consul services register foo.hcl 
$ consul connect envoy -sidecar-for foo

Then check localhost:19000/config_dump in the listener section

[rboyer]

I'm thinking the real bug is in agent/consul/config_endpoint.go:ResolveServiceConfig. That is part of ServiceManager's workflow whereby the service registration blob you present the Agent is merged with proxy-defaults+service-defaults for itself and all upstreams before actually being registered. Later these are used when crafting the discovery chain compilation request.

[rboyer]

Let's see if the CI gods reject: #7938

[rboyer]

Do we know if this is a regression, or just unexpected behavior that differs from docs?

I think this was a behavioral omission when proxy-defaults and service-defaults were written in 1.5.x. Outside of egressing L7 stats to datadog/prometheus/etc usually you would add some config entry specific to the service which is enough to bypass the bug.

[david-yu]

Thanks @rboyer!