Matrix generator of SCM + PullRequest doesn't work

[IvanovOleg]

Checklist:

• [X] I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
• [X] I've included steps to reproduce the bug.
• [X] I've pasted the output of argocd version.

Describe the bug

I am trying to get PRs automatically discovered by an application set that uses a cobination of SCM and PullRequest generators in matrix. It looks like they don't work together because both of them output the branch variable which is different for SCM and PullRequest.

To Reproduce

apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: pr
spec:
  generators:
    - matrix:
        generators:
          - scmProvider:
              github:
                organization: org
                allBranches: false
                tokenRef:
                  secretName: creds-github-https
                  key: password
              filters:
                - repositoryMatch: ^pref
                  pathsExist: [deploy/overlays/pr/kustomization.yaml]
          - pullRequest:
              github:
                owner: org
                repo: '{{repository}}'
                tokenRef:
                  secretName: creds-github-https
                  key: password
  template:
    metadata:
      name: '{{repository}}-pr-{{number}}'
    spec:
      project: test
      source:
        repoURL: '{{url}}'
        targetRevision: '{{head_sha}}'
        path: 'deploy/overlays/pr'
      destination:
        server: https://kubernetes.default.svc
        namespace: '{{repository}}'
      syncPolicy:
        syncOptions:
          - CreateNamespace=true

Expected behavior

Apps are generated

Version

argocd: v2.5.2+148d8da
  GitCommit: 148d8da7a996f6c9f4d102fdd8e688c2ff3fd8c7
  GitTreeState: clean
  GoVersion: go1.18.7
  Compiler: gc
  Platform: linux/amd64
argocd-server: v2.5.2+
  GitCommit: 148d8da7a996f6c9f4d102fdd8e688c2ff3fd8c7
  GitTreeState: clean
  GoVersion: go1.[18]
  Compiler: gc
  Platform: linux/amd64
  Kustomize Version: v4.5.7 [20]
  Helm Version: v3.10.1+g9f88ccb
  Kubectl Version: v0.[24]

Logs

failed to combine string maps with merging params for the matrix generator: found duplicate key branch with different value, a: main ,b: prbranch

[XLordalX]

Temporary work-around is to set goTemplate: true in the ApplicationSet. Only downside is, you still can't access the pr branch variable.

[IvanovOleg]

@XLordalX Yes, that helps.

[dmpe]

There seems to be another bug, namely when using goTemplate: true, SCM branch value is being used instead of that of PR branch 1.

branch_slug does work though, because it is not being overwritten.

[XLordalX]

Indeed. I think both values from both generators should be accessible somehow. Something like {{ scm.branch }} and {{ pr.branch }}.

[barry-ar]

Hello @IvanovOleg
As @XLordalX told you previously you have to use the goTemplate variable which will allow you to use the variables of the generators scm at the level of the generators pull request

ArgoCD version must be greater than or equal to v2.5.5

apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: matrix-scm-pull-request
  namespace: argo-cd
spec:
  goTemplate: true
  generators:
  - matrix:
      generators:
      - scmProvider:
          cloneProtocol: https
          filters:
          - pathsExist:
            - values-staging.yaml
            repositoryMatch: ^repos
          github:
            allBranches: false
            organization: org
            tokenRef:
              key: token
              secretName: github-token

      - pullRequest:
          github:
            owner: org
            repo: '{{ .repository }}'
            tokenRef:
              secretName: github-token
              key: token
            labels:
            - preview
          requeueAfterSeconds: 30
  template:
    metadata:
      name: '{{ .branch_slug }}-{{ .repository }}'
      finalizers:
      - resources-finalizer.argocd.argoproj.io
      labels:
        type: pull-request
    spec:
      source:
        repoURL: '{{ .url }}'
        targetRevision: '{{ .branch_slug }}'
        path: chart/
        helm:
          parameters:
          - name: 'ingreshellohosts[0]'
            value: '{{ .branch_slug }}-{{ .repository }}.my-dns.com'
          - name: 'image'
            value: 'xxxxx.dkr.ecr.eu-west-1.amazonaws.com/{{ .repository }}:{{ .branch_slug }}'
          valueFiles:
          - ../values-staging.yaml
      project: staging
      destination:
        server: https://xxxxx.yyy.eu-west-1.eks.amazonaws.com
        namespace: '{{ .branch_slug }}-{{ .repository }}'
      syncPolicy:
        syncOptions:
        - CreateNamespace=true

Docs: goTemplate

[crenshaw-dev]

I think a prefix is one good option.

Another is this PR, which would allow aliasing individual parameters to avoid collisions: https://github.com/argoproj/argo-cd/pull/10754/files#diff-6a3e8102af454ad5d4c96a511468e359ed2652d82e8df5e4cc3dfc873e4429f4

[rmgpinto]

@IvanovOleg did you get this to work? I got the matrix generator working with a Git generator and Pull Request generator. Some issues:

• the application is only created when I open a PR;
• the application is deleted when I close the PR.

Do you know how to fix this?

[IvanovOleg]

@rmgpinto I guest that is an expected behavior. Since first generator just provides some extra data to a second one (they don't generate apps separately)

[joshuabaird]

This will also fix this issue: https://github.com/argoproj/argo-cd/pull/14558

[crenshaw-dev]

If someone wants to pop open a new PR based on mine and add docs+tests, that feature will be ready to ship. My 2.9 cycle is super packed, I'm not sure I'll be able to get this in if it's just me on it.

[okassov]

Are there any updates to solve this problem?

[LaloLoop]

Isn't this just a simple bug where the first generator overrides the values of the second one on the list? I expect generators placed later to have precedence over the ones defined first. I found this line, which looks different from what we want. When goTemplate: false, it does pass b as the values override on a.

I wonder what the design decision behind this test case was. That prevents us from using SCM and PR inside a Matrix generator. That's a killer combo, TBH.

Changing these two lines (a to b):

mergo.Merge(&tmp, a, mergo.WithOverride)
mergo.Merge(&tmp, b, mergo.WithOverride)

And adjusting the tests here should do the trick.

expected: []map[string]interface{}{
    {"booleanFalse": true, "booleanTrue": false, "stringFalse": "true", "stringTrue": "false"},
},

I did those modifications and re-ran all the tests, and everything passed.

[LaloLoop]

Hi, I figured out a way to get this working. I did quite a deep analysis of how the ApplicationSet generates parameters for each generator and how each gets overridden by the previous one. I have a setup like the following.

apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: pr-generator-with-branch-override
  namespace: argocd
spec:
  goTemplate: true
  generators:
    - matrix:
        generators:
          - scmProvider:
              cloneProtocol: https
               # Omitted for brevity
          - matrix:
              generators:
                - pullRequest:
                     # Omitted for brevity
                - clusters:
                    selector:
                      matchLabels:
                        argocd.argoproj.io/secret-type: cluster
                        environment: 'staging'
  template:
    metadata:
      name: '{{ printf "%s-pr-%s" .repository .number}}'
    spec:
      # Omitted for brevity
      source:
        repoURL: '{{ .url }}'
        targetRevision: '{{ .branch }}' #<- This gets overriden by the scmProvider generator.
        path: 'manifests/'
      destination:
        name: '{{ .name }}'
        namespace: 'default'

I copied the ArgoCD generateApplications function into a separate Go project, and I made it work locally. It connects to my cluster hosting ArgoCD so that it can discover all the Cluster secrets and SCM provider credentials to make it possible to generate parameters locally.

Upon debugging the code, I noticed there was a step where the branch variable was overwritten. The Matrix.Generators[0] parameters get replaced in the templating of the Matrix.Generator[1]. That means that, when the second Matrix generator is evaluated, all the templating variables were already replaced.

This last part made me think in a workaround. We can make a go template render a go template.

Since the Cluster generator can handle additional values, we can use that field to render a template that will eventually get replaced by the second Matrix 0th generator. The generators end up like this.

generators:
    - matrix:
        generators:
          - scmProvider:
              cloneProtocol: https
               # Omitted for brevity
          - matrix:
              generators:
                - pullRequest:
                     # Omitted for brevity
                - clusters:
                    values:
                      #This does the trick. It will get replaced during the second matrix param interpolation and properly get the pr branch.
                      pr_branch: '{{`{{ .branch }}`}}'
                    selector:
                      matchLabels:
                        argocd.argoproj.io/secret-type: cluster
                        environment: 'staging':
template:
  spec:
    source:
      repoURL: '{{ .url }}'
     # Proper PR value gets injected here.
      targetRevision: '{{ .values.pr_branch }}'

I'll keep on working on that small util to allow us to generate applicationsets locally. It could be useful to do a dry run prior to actually merging PRs.

I hope the above helps while a real solution is provided. This is more of an undocumented feature since the Matrix generator internals is not explained in the documentation, and using template: true makes it a bit more robust.

Thanks!

[yogeek]

Hi there, I am trying to achieve the same thing (discover a list of SCM repo for which I want 1 app created for each PR) and I see that this issue is still ongoin. What is the status here please ? Do I have to use the workaround provided by @LaloLoop ?

[macmiranda]

You can also just use

        targetRevision: '{{ .head_sha }}'

in your template spec without the need to add a cluster generator and the second interpolation, if that works for you.

[yogeek]

In my case, I really need the branch name (because I pass it as a parameter of an helm chart in the application template to managed a secondary appset which needs the branch name)

In most cases I guess I can use the branch_slug but it could lead to some errors as I will not be able to control the format of the branch names that users create so it would be better to have the real branch under another variable like pr_branch to be able to discriminate it from the SCMProvider branch.