ApplicationSet Controller crashes when tag is not closed; panic: Cannot find end tag="}}"

[johnkeates]

Describe the bug

When feeding a mis-formatted ApplicationSet to the controller it panics and crashes causing the ApplicaitonSet Controller Pod to restart.
Mis-formatting in this case means: not closing a variable tag.

To Reproduce

1. Create an ApplicationSet with a Generator that emits values
2. Use a value as an in-line values argument to a Helm chart within the Application Template
3. Instead of properly closing the tag with a }} only use a }
4. In my case, open a new tag (also with {{) and then again fail to close it properly (only use })

The ApplicationSet Controller panics and gets restarted after it crashes.

The (indeed faulty) template contains at the bottom of a list of in-line Helm values:

            - name: POSTGRESQL_URL
              value: "{{values.postgresUrl}}"
            - name: POSTGRESQL_USERNAME
              value: "{{values.postgresUsername}"  
            - name: POSTGRESQL_PASSWORD
              value: "{{values.postgresPassword}"

Expected behavior

The ApplicationSet controller should return an error about a malformed template and continue operating as normal.

Version

argocd: v2.7.4+a33baa3.dirty
  BuildDate: 2023-06-05T23:45:09Z
  GitCommit: a33baa301fe61b899dc8bbad9e554efbc77e0991
  GitTreeState: dirty
  GoVersion: go1.20.4
  Compiler: gc
  Platform: darwin/arm64
argocd-server: v2.6.7+5bcd846
  BuildDate: 2023-03-23T14:57:27Z
  GitCommit: 5bcd846fa16e4b19d8f477de7da50ec0aef320e5
  GitTreeState: clean
  GoVersion: go1.18.10
  Compiler: gc
  Platform: linux/amd64
  Kustomize Version: v4.5.7 2022-08-02T16:35:54Z
  Helm Version: v3.10.3+g835b733
  Kubectl Version: v0.24.2
  Jsonnet Version: v0.19.1

Logs
Error message contains (redacted):

panic: Cannot find end tag="}}" in the template="requiredSystemValues:\n  dumbBlockOfDataBecauseHelmLookupsAreBroken: '{{metadata.annotations.deploymentTargets}}'\nimage:\n  name: {{values.ecrRepo}}\n  tag: {{values.imageTag}}\nreadinessProbe:\n  httpGet:\n    path: /status\n    port: 5000\n  periodSeconds: 5\n  initialDelaySeconds: 10\npodAnnotations:\n  prometheus.io/scrape: \"true\"\n  prometheus.io/path: \"/metrics\"\n  prometheus.io/port: \"9095\"\ncommonLabels:\n  app: \"{{values.serviceName}}\"\n  team: \"{{values.team}}\"\n  language: \"{{values.language}}\"\ncontainerPort: 5000\nautoscaling:\n  enabled: true\n  minReplicas: \"{{values.minReplicaCount}}\"\n  maxReplicas: \"{{values.maxReplicaCount}}\"\n  targetCPUUtilizationPercentage: \"80\" #CPU Scaler\n  metricType: Utilization  #Default=AverageValue\nresources:\n  requests:\n    cpu: \"{{values.cpu}}\"\n    memory: \"{{values.ram}}\"\n  limits:\n    memory: \"{{values.ram}}\"\ntraffic:\n  trusted:\n    exposed: true\n  consumer:\n    exposed: false\n  thirdParty:\n    exposed: false\nenv:\n- name: CONTAINER_SPACE\n  value: 'placeholder\n- name: CONTAINER_ENVIRONMENT\n  value: \"{{metadata.labels.environment}}\"\n- name: LOG4J_FORMAT_MSG_NO_LOOKUPS\n  value: 'true'\n- name: LOG_JSON_TCP_HOST\n  value: fluent-bit.fluent-bit.svc.cluster.local:5170\n- name: KAFKA_DSN\n  value: \"{{values.kafkaHosts}}\"\n- name: KAFKA_GROUP_ID\n  value: \"{{values.kafkaGroupId}}\"\n- name: PLACEHOLDER_REDACTED_1\n  value: \"{{values.kafkaTopic}}\"\n- name: PLACEHOLDER_REDACTED_11\n  value: \"{{values.placeholderBaseUrl}}\"\n- name: PLACEHOLDER_REDACTED_111\n  value: \"{{values.mySpecialtoken}}\"\n- name: PLACEHOLDER_REDACTED_2\n  value: \"{{values.placeholderClientId}}\"\n- name: PLACEHOLDER_REDACTED_11111\n  value: \"{{values.placeholderClientSecret}}\"\n- name: PLACEHOLDER_REDACTED_111111\n  value: \"{{values.placeholderGrantType}}\"\n- name: POSTGRESQL_URL\n  value: \"{{values.postgresUrl}}\"\n- name: POSTGRESQL_USERNAME\n  value: \"{{values.postgresUsername}\"  \n- name: POSTGRESQL_PASSWORD\n  value: \"{{values.postgresPassword}\"\n" starting from "values.postgresUsername}\"  \n- name: POSTGRESQL_PASSWORD\n  value: \"{{values.postgresPassword}\"\n"

[crenshaw-dev]

I think we need to:

1. write a unit test to demonstrate the bug
2. add logic here to recover from fasttemplate's panic and simply return an error

[MasonLegere]

I can do it quickly - was already working on it early today.

[MasonLegere]

I believe that the error is bubbling up here as the templated value won't match the regex leading to something uncaught as it's passed on later to Helm. Changing the regex could work but seems gross to me so far 🤷‍♂️