Istio Compat: Edge Helm Chart Does Not Correctly Generate excludeInboundPorts annotation for gateway proxies with custom names

[bgottfried91]

Gloo Edge Version

1.12.x (beta)

Kubernetes Version

1.23.x

Describe the bug

#6195 and #6196 added support for full compatibility between a GE Gateway Proxy and services in the Mesh, but the Helm chart changes made in them to ensure that the sidecar injected into each Gateway Proxy doesn't capture traffic inbound to the proxy from outside the cluster only works for a Gateway Proxy with the default name gatewayProxy.

Steps to reproduce the bug

If you run the following command: helm helm template gloo-ee glooe/gloo-ee --version 1.12.0-beta7 --set global.istioIntegration.enableIstioSidecarOnGateway=true

you will see that the Deployment named "gateway-proxy" has an annotation for traffic.sidecar.istio.io/excludeInboundPorts: with a value of 8080,8443 (the default inbound ports):

apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app: gloo
    gloo: gateway-proxy
    gateway-proxy-id: gateway-proxy
  name: gateway-proxy
  namespace: default
spec:
  replicas: 1
  selector:
    matchLabels:
      gloo: gateway-proxy
      gateway-proxy-id: gateway-proxy
  template:
    metadata:
      labels:
        gloo: gateway-proxy
        gateway-proxy-id: gateway-proxy
        gateway-proxy: live
        sidecar.istio.io/inject: "true"

      annotations:
        prometheus.io/path: /metrics
        prometheus.io/port: "8081"
        prometheus.io/scrape: "true"
        traffic.sidecar.istio.io/excludeInboundPorts: 8080,8443

If you run the same template, but with a values file that disables the default Gateway Proxy and adds an additional Gateway Proxy called corp-gw:

global:
  istioIntegration:
    enableIstioSidecarOnGateway: true
gloo:
  gatewayProxies:
    corpGw: # Proxy name for private access (intranet facing)
      disabled: false # overwrite the "default" value in the merge step

The resulting Deployment for corp-gw will generate the traffic.sidecar.istio.io/excludeInboundPorts annotation with no value:

apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app: gloo
    gloo: gateway-proxy
    gateway-proxy-id: corp-gw
  name: corp-gw
  namespace: default
spec:
  replicas: 1
  selector:
    matchLabels:
      gloo: gateway-proxy
      gateway-proxy-id: corp-gw
  template:
    metadata:
      labels:
        gloo: gateway-proxy
        gateway-proxy-id: corp-gw
        gateway-proxy: live
        sidecar.istio.io/inject: "true"

      annotations:
        prometheus.io/path: /metrics
        prometheus.io/port: "8081"
        prometheus.io/scrape: "true"
        traffic.sidecar.istio.io/excludeInboundPorts:

despite this gateway (presumably) also having default inbound ports of 8080 and 8443.

Expected Behavior

The traffic.sidecar.istio.io/excludeInboundPorts: annotation should be populated appropriately regardless of what Gateway Proxy is being acted upon.

Additional Context

The issue appears to be due to using the incorrect Helm variable . This is the block in the Helm chart that generates the annotation:

{{- $gatewaySpec := (index . 2) }}
{{- with (first .) }}
{{- $global := .Values.global }}
{{- $settings := .Values.settings }}
{{- $isUpgrade := .Values.gateway.upgrade }}
{{- $gatewayProxy := .Values.gatewayProxies.gatewayProxy -}}
{{- $spec := deepCopy $gatewaySpec | mergeOverwrite (deepCopy $gatewayProxy) -}}
{{- $ports := list }}
{{- if not (empty $gatewaySpec.podTemplate) }}
  {{- $ports = (list $gatewaySpec.podTemplate.httpPort $gatewaySpec.podTemplate.httpsPort $gatewaySpec.podTemplate.extraPorts) }}
  {{- if not (empty $gatewaySpec.podTemplate.nodeSelector) }}
    {{- $_ := set $spec.podTemplate "nodeSelector" $gatewaySpec.podTemplate.nodeSelector }}
  {{- end }}

Note that we merge the $gatewaySpec variable (which presumably contains the block for the Gateway Proxy being generated) with the $gatewayProxy block and store that into $spec, so that any values not filled in for the specific Gateway Proxy being created will get the default values from gatewayProxy. But then when setting the $ports variable, we go back to using $gatewaySpec instead of $spec. I tested modifying the chart to reference $spec inside the $ports assignment block and it generates the annotation appropriately once that's done.