Fix: Set flyteadmin gRPC port to 80 in ingress if using TLS between load balancer and backend

[fg91]

Describe your changes

Flyte can be configured to use TLS between the ingress (or rather load balancer) and the flyteadmin backend.

This is required for instance in GKE clusters when using the GCE ingress controller (instead of nginx) since gRPC requires http2 and http2 between GCE load balancers and backends in GKE clusters requires TLS (source):

Note: To ensure the load balancer can make a correct HTTP2 request to your backend, your backend must be configured with SSL.

Whether flyteadmin uses TLS (and with which certificate) is controlled in the helm values file via:

configmap:
  adminServer:
    security:
      secure: true
      ssl:
        certificateFile: "/etc/tls/cert.pem"
        keyFile: "/etc/tls/key.pem"

(The certificate and key needs to be mounted into flyteadmin e.g. via a secret. Can be self-signed.)

However, in case TLS is enabled, flyteadmin doesn't seve the http server on port 80 and the gRPC server on port 81 but actually a single http(s) server that wraps both of them on port 80!

---

Details:

In flyteadmin's main serve entrypoint there is a decision gate whether serverConfig.Security.Secure.

• If no, we go into serveGatewayInsecure
  • In this case, the gRPC server is started in a go-routine here while the separate http server is started afterwards here.
• If, however, serverConfig.Security.Secure is true, we instead go into serveGatewaySecure
  • Instead of separate http and gRPC servers, a single one wrapping both is started here with TLS. This server routes to the actual http or gRPC server here.

In the second case, all requests for flyteadmin, including gRPC, have to be sent to port 80.

---

This is not accounted for in the ingress template of the helm chart. Activating TLS for flyteadmin, thus, breaks the deployment.

This PR fixes this.

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

[fg91]

Not needed anymore for #3965 as the recommended deployment with IAP now uses this reference architecture using an istio ingress behind the GCP load balancer.
Still a bug though.

[davidmirror-ops]

@fg91 I haven't reproduce the behavior you describe but the code is clear. I'd say that even if not needed for GKE Ingress controller, it's still a bug.

Thanks

[davidmirror-ops]

@fg91 could you make helm and push again please?

[fg91]

@fg91 I haven't reproduce the behavior you describe but the code is clear. I'd say that even if not needed for GKE Ingress controller, it's still a bug.

I wonder whether somebody actually lets flyteadmin handle TLS itself instead of doing this at the ingress. I only tried this because the GCE ingress requires TLS betwenn ingress and backend when http2 is used which in turn is required by grpc.

That being said, when I couldn't reach flyteadmin anymore after having turned on TLS in the flyteadmin config, I would have never guessed that the port changed (not documented) and only realised this happens when stepping through flyteadmin with a debugger. Which was definitely not the first thing I tried 😄
So in case somebody else does in fact need this, let's finish this PR ..

@fg91 could you make helm and push again please?

Done

[davidmirror-ops]

Thank you