grpc / grpc-java

The Java gRPC implementation. HTTP/2 based RPC
https://grpc.io/docs/languages/java/
Apache License 2.0
11.38k stars 3.83k forks source link

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

Closed cfredri4 closed 2 years ago

cfredri4 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

cfredri4 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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