gloo-ee chart renders invalid yaml when default gw proxy disabled and kubeResourceOverride is used

[jenshu]

Gloo Edge Version

1.12.x

Kubernetes Version

None

Describe the bug

Trying to install GlooEE with these values

gloo:
  gatewayProxies:
    gatewayProxy:
      disabled: true
    otherProxy:
      disabled: false
      kubeResourceOverride:
        spec:
          template:
            metadata:
              labels:
                foo: bar
license_key: foobar

results in an error Error: Failed to render chart: exit status 1: Error: unable to build kubernetes objects from release manifest: error parsing : invalid Yaml document separator: apiVersion: apps/v1 due to the --- separator not being on its own line:

# Source: gloo-ee/charts/gloo/templates/7-gateway-proxy-deployment.yaml
---apiVersion: apps/v1
kind: Deployment

This seems to only occur when all the following are true:

• using the gloo-ee chart (couldn't reproduce in the gloo chart, but we should double check that)
• when the default gatewayProxy is disabled
• the enabled proxy comes alphabetically after gatewayProxy (i.e. couldn't reproduce with a proxy named aProxy)
• the enabled proxy uses a kubeResourceOverride

Might be fixed by moving the divider to right after this line

We should add testing to ensure we're not making the same mistake in other templates too.

Steps to reproduce the bug

see above

Expected Behavior

render valid yaml

Additional Context

No response

[ben-taussig-solo]

I have just moved this issue to blocked. Some context:

• A community user had put up this PR with the intention of resolving the issue: https://github.com/solo-io/gloo/pull/9294
• Per Jenny's request in Slack, I put up a solo-projects PR with a failing test that should pass when this issue gets fixed: https://github.com/solo-io/solo-projects/pull/5960
• However, that test did not pass when I pulled in the helm chart generated from the community PR
• I put up another Gloo PR to see if I could easily resolve the issue; however, none of my changes were sufficient to get the s-p test passing. That PR is here: https://github.com/solo-io/gloo/pull/9305
• At this point, I think we need to regroup on this work -- I had picked this up impromptu and it has required more involved effort than I had expected

[sam-heilbron]

It would be nice if we expanded our manifest renderer utility: https://github.com/solo-io/go-utils/tree/main/helmutils, so that we could proactively catch issues like this in the future

[ben-taussig-solo]

After reviewing this slack thread, I believe that:

• This issue seems to describe a slightly different bug from the one the community user was experiencing
• The community PR should definitely resolve the user's helm formatting issue, but we don't necessarily understand the specific circumstances under which that issue occurs
• Because of this discrepancy, the PR I put up to test that the issue described here is resolved did not pass CI when we pulled in the helm chart generated from the community PR

[AlfredoFausto]

Can we prioritize this @nfuden @jbohanon??

[nfuden]

@kcbabo for prioritization question

[sam-heilbron]

This issue, still exists. There is a bug in our Helm chart, that is trigged when using the kubeResourceOverride. However, there was an enhancement made to the Helm API, to support the priority class name. The following issues were resolved by the linked PR:

• https://github.com/solo-io/gloo/issues/8677
• https://github.com/solo-io/gloo/issues/9010
• https://github.com/solo-io/gloo/issues/9823
• https://github.com/solo-io/gloo/issues/9731

The idea was that supporting the priorityClassName on the proxy would bypass the need to rely on the kubeResourceOverride. This was based on a conversation with @AlfredoFausto .

cc @BeauSelisker28 @kcbabo