argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
17.91k stars 5.46k forks source link

ArgoCD dislikes a broken template patch #17637

Open ckav370 opened 7 months ago

ckav370 commented 7 months ago

Checklist:

Describe the bug

ArgoCD is managing itself.

Using a template patch to set sync policy in an applicationset and make it configurable across different apps. It seems there was an issue with the templatePatch which has caused ArgoCD to throw an error and the UI to no longer be available

TypeError: Cannot read properties of undefined (reading 'chart')
    at https://argocd/main.4176eed80be48b27f252.js:2:1930731
    at Array.map (<anonymous>)
    at Object.children (https://argocd/main.4176eed80be48b27f252.js:2:1929919)
    at Yr.render (https://argocd/main.4176eed80be48b27f252.js:2:1478481)
    at zi (https://argocd/main.4176eed80be48b27f252.js:2:771988)
    at Hi (https://argocd/main.4176eed80be48b27f252.js:2:771783)
    at Es (https://argocd/main.4176eed80be48b27f252.js:2:807407)
    at Sl (https://argocd/main.4176eed80be48b27f252.js:2:798866)
    at xl (https://argocd/main.4176eed80be48b27f252.js:2:798791)
    at hl (https://argocd/main.4176eed80be48b27f252.js:2:795814)

Example template Patch (using helm, not kustomize hence the escape characters)

      syncPolicy:
        syncOptions:
          - CreateNamespace=true
  templatePatch: |-
    spec:
    {{`{{ if .autoSync }}
      syncPolicy:
        syncOptions:
          - CreateNamespace=true
        automated:
          prune: true
          selfHeal: true
    {{ end }}`}}

To Reproduce

Adding an invalid template patch like above which has a patch for an existing sync policy, sync and watch the UI become unavailable

Expected behavior

Template patch is anything invalid which I would expect the applicationset controller to report and degraded or out of sync, not throwing an error which renders the UI inaccessible

The fix was to define the templatePatch properly, eg:

      syncPolicy:
        syncOptions:
          - CreateNamespace=true
  templatePatch: |-
    {{`{{ if .autoSync }}
    spec:
      syncPolicy:
        syncOptions:
        - CreateNamespace=true
      syncPolicy:
        automated:
          prune: true
          selfHeal: true
    {{ end }}`}}

But it seems very unexpected behaviour that a misconfigured applicationset such as this should cause the UI to become unavailable

Screenshots

Once the broken patch has been synced by ArgoCD, this is what the UI does. Once the broken template patch has been reverted, the UI returns to usual working state

Screenshot 2024-03-27 at 09 11 27

Version


argocd: v2.10.1+a79e0ea.dirty
  BuildDate: 2024-02-14T22:23:04Z
  GitCommit: a79e0eaca415461dc36615470cecc25d6d38cefb
  GitTreeState: dirty
  GoVersion: go1.21.7
  Compiler: gc
  Platform: darwin/arm64
argocd-server: v2.10.2+fcf5d8c
  BuildDate: 2024-03-01T21:24:51Z
  GitCommit: fcf5d8c2381b68ab1621b90be63913b12cca2eb7
  GitTreeState: clean
  GoVersion: go1.21.3
  Compiler: gc
  Platform: linux/amd64
  Kustomize Version: v5.2.1 2023-10-19T20:13:51Z
  Helm Version: v3.14.2+gc309b6f
  Kubectl Version: v0.26.11
  Jsonnet Version: v0.20.0

Logs Surprisingly, nothing stand out in the logs

Killpit commented 7 months ago

It's not an issue related to ArgoCD, it's related to JavaScript/TypeScript. And one piece of advice, I'd recommend you update your go and Kubernetes versions up to date. I think you should look at the error for JavaScript and then we can proceed from here.

ckav370 commented 7 months ago

FYI- this vanilla Argo running the recommended upstream container image as per the helm chart, this also includes Go. Running on EKS version 1.28

crenshaw-dev commented 7 months ago

@Killpit Argo CD ships with JavaScript for the UI. This problem should be unrelated to the go and Kubernetes versions. @ckav370 has already posted the (redacted) JS error.

Killpit commented 7 months ago

I know, it's just a piece of advice, I think we need to see the application he created.

jgwest commented 7 months ago

Dupe of https://github.com/argoproj/argo-cd/issues/15545 (albeit w/o the same repro steps)?

Killpit commented 7 months ago

Dupe of #15545 (albeit w/o the same repro steps)?

Looks like it, yup it's quite same, we don't know other tools that are being used here and I think this issue can be a good reference point for @ckav370

olehArabskyi commented 7 months ago

I have received same error. In my case it was an issue with one of ArgoCD CRD "Application". After deletion of broken CRD the error is gone.

ckav370 commented 6 months ago

I actually noticed this error again by adding a small change to an applicationset but refreshing the UI is resolved. After fixing the template patch from the original issue, I updated an appset adding releaseName: "{{{{.values.env}}}}" as below which seems to be the only change. This error has not reoccured yet and i have combed the repo server logs but see nothing unusual

apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: name
  namespace: argocd
spec:
  syncPolicy:
    preserveResourcesOnDeletion: true
  ignoreApplicationDifferences:
    - jsonPointers:
        - /spec/source/targetRevision
  goTemplate: true
  goTemplateOptions: ["missingkey=error"]
  generators:
  - matrix:
      generators:
      - list:
          elements:
          - cluster: cluster
            url: url
            branch: main
            autoSync: false
      - git:
          repoURL: https://github.com/repo.git
          revision: main
          files:
          - path: env/cluster/*.yaml
          values:
            env: |-
              {{"{{`{{ trimSuffix \".yaml\" .path.filename }}`}}"}}
            app: |-
              {{"{{`{{ index .path.segments 1 }}`}}"}}
  template:
    metadata:
      name: "{{ `{{.values.app}}` }}-{{ `{{.values.env}}` }}"
    spec:
      project: my-project
      source:
        repoURL: https://github.com/repos.git
        targetRevision: "{{ `{{ .branch }}` }}" 
        path: charts/{{ `{{.values.env}}`}}
        helm:
          releaseName: "{{ `{{.values.env}}` }}" -> adding this 
          valueFiles:
          - values.yaml
          - "../../env/{{ `{{.cluster}}` }}/{{ `{{.values.env}}` }}.yaml" 
      destination:
        server: "{{ `{{ .url }}` }}"
        namespace: "{{ `{{.values.env}}` }}"
      syncPolicy:
        syncOptions:
          - CreateNamespace=true
          - ServerSideApply=true
  templatePatch: |-
    {{`{{ if .autoSync }}
    spec:
      syncPolicy:
        syncOptions:
        - CreateNamespace=true
        - ServerSideApply=true
      syncPolicy:
        automated:
          prune: true
          selfHeal: true
    {{ end }}`}}
PhamQuang-512 commented 3 months ago

i encounter the same error when not specify spec.source in templatePatch

apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: my-test-applicationset
  namespace: argocd
spec:
  goTemplate: true
  goTemplateOptions: ["missingkey=error"]
  syncPolicy:
    preserveResourcesOnDeletion: true
  generators:
    - matrix:
        generators:
          - list:
              elements:
                - repoURL: <my-repo>
          - list:
              elements:
                - name: 'reloader'
                  namespace: reloader
                  clusterName: in-cluster
                  path: others/reloader-1.0.121
                  releaseName: reloader
                  autoSync: false
                  valueFiles:
                    - values.yaml
  template:
    metadata:
      name: '{{.name}}'
    spec:
      project: default
      source:
        repoURL: '{{.repoURL}}'
        targetRevision: HEAD  
        path: '{{.path}}'
        helm:
          releaseName: '{{.releaseName}}'
          valueFiles:
          - values.yaml
      destination:
        name: '{{.clusterName}}'
        namespace: '{{.namespace}}'
  templatePatch: |
    spec:
    {{- if .autoSync }}
      syncPolicy:
        automated:
          prune: {{ .prune | default "false" }}
          selfHeal: {{ .selfHeal | default "false" }}
          allowEmpty: {{ .allowEmpty | default "false" }}
    {{- end }}

i just found that i need to change in templatePatch like this:

  templatePatch: |
    {{- if .autoSync }}
    spec:
      syncPolicy:
        automated:
          prune: {{ .prune | default "false" }}
          selfHeal: {{ .selfHeal | default "false" }}
          allowEmpty: {{ .allowEmpty | default "false" }}
    {{- end }}

and it will work, but still need to review this feature because it affects the UI if not carefully config

andrii-korotkov-verkada commented 2 days ago

ArgoCD versions 2.10 and below have reached EOL. Can you upgrade and tell us if the issue is still present, please?