Gloo-ee mixes extraAnnotations for two gatewayProxy service resources

[wsliu-talend]

Describe the bug
If we config two gatewayProxy instances under gloo.gatewayProxies in extra value file and install it through helm chart, the extraAnnotations values of gloo.gatewayProxies.gatewayProxy will be added to another gloo.gatewayProxies.gatewayProxyAny.

To Reproduce
Steps to reproduce the behavior:

1. Prepare value.yml file as below:

  gloo:
    gatewayProxies:
      gatewayProxy:
        service:
          name: "gateway-proxy"
          extraAnnotations:
            key_a: value_a
      gatewayProxyFoo:
        service:
          name: "gateway-proxy-foo"
          extraAnnotations:
            key_b: value_b

2. Install gloo-ee 1.7.9 through helm chart
   helm install gloo glooe/gloo-ee --version 1.7.9 -f value.yml --namespace gloo-system
   --create-namespace --set-string license_key=YOUR_LICENSE_KEY
   To demonstrate the issue, we can also get the template only:
   helm template "gloo" glooe/gloo-ee -f value.yml --version 1.7.9
3. Check k8s service resouce named "gateway-proxy-foo", the annotations are as below:

     annotations:
       key_a: "value_a"
       key_b: "value_b"

Expected behavior
The annotations for k8s service resouce "gateway-proxy-foo" should be:

     annotations:
       key_b: "value_b"

Additional context
Add any other context about the problem here, e.g.

• Gloo Edge version
  gloo-ee-1.7.9 helm chart
• Kubernetes version
  AWS eks 1.18

[wsliu-talend]

It can be reproduced in gloo-ee-1.7.0 helm chart, but not in gloo-ee-1.6.31 helm chart.

[jon-walton]

This is not fixed in ee 1.8.1. The deepCopy was moved outside of the loop

the following values.yaml

gloo:
  gatewayProxies:
    gatewayProxy:
      disabled: true

    gatewayProxyPublic:
      disabled: false

    gatewayProxyInternal:
      disabled: false
      service:
        extraAnnotations:
          a: b

results in the extraAnnotations being applied to both services

# Source: gloo-ee/charts/gloo/templates/8-gateway-proxy-service.yaml
apiVersion: v1
kind: Service
metadata:
  labels:
    app: gloo
    gloo: gateway-proxy
    gateway-proxy-id: gateway-proxy-internal
  name: gateway-proxy-internal
  namespace: default
  annotations:
    a: "b"
    prometheus.io/path: "/metrics"
    prometheus.io/port: "8081"
    prometheus.io/scrape: "true"
spec:

---
# Source: gloo-ee/charts/gloo/templates/8-gateway-proxy-service.yaml
apiVersion: v1
kind: Service
metadata:
  labels:
    app: gloo
    gloo: gateway-proxy
    gateway-proxy-id: gateway-proxy-public
  name: gateway-proxy-public
  namespace: default
  annotations:
    a: "b"
    prometheus.io/path: "/metrics"
    prometheus.io/port: "8081"
    prometheus.io/scrape: "true"
spec:

this fixes it (gloo-ee\charts\gloo\templates\8-gateway-proxy-service.yaml)

+{{- range $name, $gatewaySpec := .Values.gatewayProxies }}
{{- $defaultGatewayProxySafe := deepCopy $gatewayProxy -}}
{{- $_ := unset $defaultGatewayProxySafe.service "extraAnnotations" -}}
-{{- range $name, $gatewaySpec := .Values.gatewayProxies }}
{{- $spec := deepCopy $gatewaySpec | mergeOverwrite $defaultGatewayProxySafe -}}

[ben-taussig-solo]

Reopening this issue After some discussion, I have re-closed the issue, and have opened #5396 in its place.

As discussed in @kdorosh's comment on #4953, that PR only partially resolved this issue

as discussed offline, I think [defaulting to deep map merges when in some cases this can cause incorrect behavior] is a symptom of perhaps a larger problem in our chart

gloo:
  gatewayProxies:
    gateway-proxy-internal:
      service:
        extraAnnotations:
          a: value_a
          b: value_b
          c: value_c
    gatewayProxy:
      service:
        extraAnnotations:
          a: value_a
          b: value_b_2

The default gateway proxy (gatewayProxy) after templating ends up having:

a: value_a
b: value_b_2
c: value_c

As you can see, the default gateway proxy inherits the annotation c: value_c from gateway-proxy-internal.

One proposition for resolving this issue has been to relocate inherited annotations under a different attribute, I.E.,:

service:
  extraAnnotations:
    a: value_a
    b: value_b_2
  defaultAnnotations:
    c: value_c