xds: suspected incompatibility for server+Istio (invalid traffic direction: UNSPECIFIED)

[cfredri4]

I have a grpc-java (1.46.0) server in an Istio (1.13.2) mesh that works fine using Envoy proxy, but which fails if I convert it to use xds instead:

2022-05-10 13:34:54.929+0000 [main] DEBUG io.grpc.xds.XdsLogger - [logOnly] [xds-client<6>] Failed processing LDS Response version 2022-05-10T13:32:29Z/4243 nonce L+ztZYHLHCU=6de6948c-3e51-46d8-a376-50e792d5aadd. Errors:
LDS response Listener 'xds.istio.io/grpc/lds/inbound/0.0.0.0:9091' validation error: Listener xds.istio.io/grpc/lds/inbound/0.0.0.0:9091 with invalid traffic direction: UNSPECIFIED

The LDS response is attached below, clearly there is no trafficDirection specified but the exact same setup works fine using Envoy.
If I comment out this check then everything works fine using xds also. Should this check be changed to allow UNSPECIFIED as well?

(ping @sanjaypujare)

2022-05-10 13:34:54.821+0000 [main] TRACE io.grpc.xds.XdsLogger - [logOnly] [xds-client<9>: (unix:///etc/istio/proxy/XDS)] Received LDS response:
{
  "versionInfo": "2022-05-10T13:32:29Z/4243",
  "resources": [{
    "@type": "type.googleapis.com/envoy.config.listener.v3.Listener",
    "name": "xds.istio.io/grpc/lds/inbound/0.0.0.0:9091",
    "address": {
      "socketAddress": {
        "address": "0.0.0.0",
        "portValue": 9091
      }
    },
    "filterChains": [{
      "filters": [{
        "name": "inbound-hcmmtls",
        "typedConfig": {
          "@type": "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager",
          "routeConfig": {
            "name": "inbound",
            "virtualHosts": [{
              "domains": ["*"],
              "routes": [{
                "match": {
                  "prefix": "/"
                },
                "nonForwardingAction": {
                }
              }]
            }]
          },
          "httpFilters": [{
            "name": "envoy.filters.http.router",
            "typedConfig": {
              "@type": "type.googleapis.com/envoy.extensions.filters.http.router.v3.Router"
            }
          }]
        }
      }],
      "transportSocket": {
        "name": "envoy.transport_sockets.tls",
        "typedConfig": {
          "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.DownstreamTlsContext",
          "commonTlsContext": {
            "combinedValidationContext": {
              "defaultValidationContext": {
              },
              "validationContextCertificateProviderInstance": {
                "instanceName": "default",
                "certificateName": "ROOTCA"
              }
            },
            "tlsCertificateCertificateProviderInstance": {
              "instanceName": "default",
              "certificateName": "default"
            }
          },
          "requireClientCertificate": true
        }
      },
      "name": "inbound-mtls"
    }]
  }],
  "typeUrl": "type.googleapis.com/envoy.config.listener.v3.Listener",
  "nonce": "L+ztZYHLHCU\u003d6de6948c-3e51-46d8-a376-50e792d5aadd",
  "controlPlane": {
    "identifier": "{\"Component\":\"istiod\",\"ID\":\"istiod-rev1-13-2-696fcc6445-77gpw\",\"Info\":{\"version\":\"1.13.2\",\"revision\":\"91533d04e894ff86b80acd6d7a4517b144f9e19a\",\"golang_version\":\"go1.17.8\",\"status\":\"Clean\",\"tag\":\"1.13.2\"}}"
  }
}

[sanjaypujare]

@cfredri4 thanks for trying this out.

If I comment out this check then everything works fine using xds also. Should this check be changed to allow UNSPECIFIED as well?

I would expect the control plane (Istio in this case) to use the appropriate direction and in this case i.e. for the server side listener it has to be TrafficDirection.INBOUND .

So I would like to find out from @costinm if this can be fixed in Istio or if there is a reason why this listener direction has to be unspecified even in case of Envoy. If it has to be unspecified in case of Envoy (since it creates a single listener for both inbound and outbound traffic) then can Istio treat the proxyless case specially to it sets the direction as inbound for server side listener?

@cfredri4 based on what we hear we'll need to make this change cross language since all gRPC languages currently enforce this check and that will need to be changed.

[sanjaypujare]

the check doesn't exist in Go and I don't know if it exists in Core/C++ (@yashykt any idea?)

It looks like the gRFC does not mention it either. So I am inclined to make this change unless Istio can make the change quickly. @cfredri4 if you want you can go ahead and create a PR

[costinm]

Not sure what traffic direction is used for - but better to be consistent with go and c++.

It would be GREAT if we could have some yaml files with all the fields supported by each implementation, at least as a reference.

[sanjaypujare]

I think this is fixed by #9173. If not pls reopen

[cfredri4]

With the changes in #9173 then Java will still reject TrafficDirection.OUTBOUND, but Go and C++ will still accept this as they have no check at all.
Is it important that all languages are consistent? And should then Go and C++ implement a similar check, or should the check in Java actually be removed altogether?

[sanjaypujare]

Good question. Having an outbound listener doesn't make sense for a server side listener. I'd wait until there is an actual issue before trying to change anything

[yashykt]

Core/C++ does not make any check on traffic direction