SCM Provider with Pull Requests #466

fardin01 · 2022-01-19T10:22:12Z

Problem

Both the SCM Provider generator and Pull Request generator are very good tools for different and valid use cases, but the limitation is that they cannot be used together in a Matrix generator:

apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: myapp
spec:
  generators:
    - matrix:
        generators:
        - scmProvider:
            github:
              organization: myorg
        - pullRequest:
            github:
              owner: myorg
              repo: < Cannot do this here! >
  template:
   ....

We are trying to adopt/migrate to ArgoCD and this is sort of a major blocker. Our testing ecosystem depends on engineers being able to deploy PRs to validate features. According to Pull Request generator docs, "this [the genrator] fits well with the style of building a test environment when you create a pull request". However, when using ApplicationSets, we cannot really have one ApplicationSet with SCM generator and one with Pull Request generator.

Proposed solution

Add an extra attribute to SCM generator:

  generators:
  - scmProvider:
      github:
        organization: myorg
        allBranches: false
        allPullRequests: true

Similar to allBranches, allPullRequests would create a new ArgoCD app for the open PRs, and could also have a PRMatch filter.

So it would be like embedding the Pull Request generator into the SCM generator.

Any feedback/comments would be appreciated.

The text was updated successfully, but these errors were encountered:

fardin01 · 2022-01-19T12:03:53Z

A better alternative: Change the Pull Request generator to accept a pattern/regex for repo attribute, and if left empty it should find all the repos. Using this in conjunction with SCM generator will most likely hit the rate limits within seconds, so it must be very well optimized.

fardin01 · 2022-01-19T13:20:55Z

The above alternative is probably a good enhancement to the Pull Request generator in general. But embedding the Pull Request generator into the SCM generator has the added benefit of using the SCM generator's filters and listing pull requests only for the filtered repos.

fardin01 · 2022-01-19T16:28:52Z

@jgwest Would love your thoughts before I start working on this :)

This commit adds pull request support to SCM generator so the generator can create ArgoCD apps for PRs as well. Fixes argoproj#466

This commit adds pull request support to SCM generator so the generator can create ArgoCD apps for PRs as well. Fixes argoproj#466 Signed-off-by: Fardin Khanjani <fardin.khanjani@tradeshift.com>

crenshaw-dev · 2022-02-16T19:59:34Z

@fardin01 how would you feel about adding templating support to the second generator in a matrix generator? Basically pass each variable set produced by the first generator to the second generator and perform substitution before running the generator.

Something like this:

apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: myapp
spec:
  generators:
    - matrix:
        generators:
        - scmProvider:
            github:
              organization: myorg
        - pullRequest:
            github:
              owner: myorg
              repo: "{{repoURL}}"
  template:
   ....

can create ArgoCD apps for PRs as well. Fixes argoproj#466 Signed-off-by: Fardin Khanjani fardin.khanjani@tradeshift.com

can create ArgoCD apps for PRs as well. Fixes argoproj#466 Signed-off-by: Fardin Khanjani <fardin.khanjani@tradeshift.com>

…enerator can create ArgoCD apps for PRs as well. Fixes argoproj#466 Signed-off-by: Fardin Khanjani <fardin.khanjani@tradeshift.com>

fardin01 · 2022-02-28T09:28:08Z

@fardin01 how would you feel about adding templating support to the second generator in a matrix generator? Basically pass each variable set produced by the first generator to the second generator and perform substitution before running the generator.

Something like this:
apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: myapp
spec:
  generators:
    - matrix:
        generators:
        - scmProvider:
            github:
              organization: myorg
        - pullRequest:
            github:
              owner: myorg
              repo: "{{repoURL}}"
  template:
   ....

@crenshaw-dev I think templating support in the second generator would be an excellent idea generally, but not so much in this specific use-case, because the matrix generator (at least at this moment) supports only two child generators. What if someone wants to use the SCM Provider (with PR support) and e.g. cluster generator (which probably is a common real-world scenario)? If the SCM Provider supports PRs by itself, then it would become more useful in a matrix generator. What do you think?

I'd also appreciate it if you could take a look at the PR for this issue.

crenshaw-dev · 2022-03-02T16:33:14Z

@fardin01 will review!

I think there might be a couple possible fixes for the matrix generator limitations:

use a nested matrix generator, providing up to 3 generators
add support for N generators in a matrix generator

But those are certainly more involved than your fix. Since your change just adds a field that we could later deprecate to go another direction, I think it's probably quite safe.

…enerator can create ArgoCD apps for PRs as well. Fixes argoproj#466 Signed-off-by: Fardin Khanjani <fardin.khanjani@tradeshift.com>

fardin01 added a commit to fardin01/applicationset that referenced this issue Jan 20, 2022

Add pull request support to SCM generator

4fd7635

This commit adds pull request support to SCM generator so the generator can create ArgoCD apps for PRs as well. Fixes argoproj#466

fardin01 added a commit to fardin01/applicationset that referenced this issue Jan 20, 2022

Add pull request support to SCM generator

adbe406

This commit adds pull request support to SCM generator so the generator can create ArgoCD apps for PRs as well. Fixes argoproj#466

fardin01 linked a pull request Jan 20, 2022 that will close this issue

feat: Add pull request support to SCM generator #469

Draft

fardin01 added a commit to fardin01/applicationset that referenced this issue Feb 23, 2022

This commit adds pull request support to SCM generator so the generator

9c5702d

can create ArgoCD apps for PRs as well. Fixes argoproj#466 Signed-off-by: Fardin Khanjani fardin.khanjani@tradeshift.com

fardin01 added a commit to fardin01/applicationset that referenced this issue Feb 23, 2022

This commit adds pull request support to SCM generator so the generator

52bb958

can create ArgoCD apps for PRs as well. Fixes argoproj#466 Signed-off-by: Fardin Khanjani <fardin.khanjani@tradeshift.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SCM Provider with Pull Requests #466

SCM Provider with Pull Requests #466

fardin01 commented Jan 19, 2022

fardin01 commented Jan 19, 2022

fardin01 commented Jan 19, 2022

fardin01 commented Jan 19, 2022

crenshaw-dev commented Feb 16, 2022

fardin01 commented Feb 28, 2022

crenshaw-dev commented Mar 2, 2022

SCM Provider with Pull Requests #466

SCM Provider with Pull Requests #466

Comments

fardin01 commented Jan 19, 2022

Problem

Proposed solution

fardin01 commented Jan 19, 2022

fardin01 commented Jan 19, 2022

fardin01 commented Jan 19, 2022

crenshaw-dev commented Feb 16, 2022

fardin01 commented Feb 28, 2022

crenshaw-dev commented Mar 2, 2022