feat: --skip-appset-generation-errors to tolerate ungeneratable ApplicationSets

[szhekpisov-katanox]

Adds --skip-appset-generation-errors (default false). When enabled, an ApplicationSet that fails to generate logs a warning and is skipped instead of aborting the whole run.

This is useful for ApplicationSets whose generators need cluster secrets or live data that isn't available in the ephemeral cluster — e.g. pullRequest / scmProvider generators — which would otherwise fail the entire diff.

Split out of #452 per review.

[dag-andersen]

Hi again @szhekpisov-katanox :)

I do not doubt that this can be a real problem, but would you mind sharing a concrete ApplicationSet example that fails when running this tool? In my experience, most generators that do not find anything simply generate zero Applications instead of returning an error.

That would make it much easier to understand the failure type and add an integration test for it later.

[szhekpisov-katanox]

Hi again @szhekpisov-katanox :)

I do not doubt that this can be a real problem, but would you mind sharing a concrete ApplicationSet example that fails when running this tool? In my experience, most generators that do not find anything simply generate zero Applications instead of returning an error.

That would make it much easier to understand the failure type and add an integration test for it later.

apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: skip-appset-errors-demo
  namespace: argocd
spec:
  goTemplate: true
  goTemplateOptions: ["missingkey=error"]
  generators:
    - pullRequest:
        github:
          owner: dag-andersen
          repo: argocd-diff-preview
          tokenRef:
            secretName: this-secret-does-not-exist
            key: token
        requeueAfterSeconds: 30
  template:
    metadata:
      name: "skip-appset-errors-demo-pr-{{.number}}"
    spec:
      project: default
      destination:
        server: https://kubernetes.default.svc
        namespace: "demo-pr-{{.number}}"
      # Public source so the template would render fine if the generator ever
      # produced parameters — the repro is entirely about the generation error.
      source:
        repoURL: https://github.com/argoproj/argocd-example-apps.git
        targetRevision: HEAD
        path: guestbook
      syncPolicy:
        syncOptions:
          - CreateNamespace=true

The GitHub pullRequest generator authenticates using a token read from a Kubernetes Secret (tokenRef). In argocd-diff-preview's ephemeral cluster that Secret does not exist, so the ApplicationSet controller returns a hard error ("error fetching Secret token: secret ... not found") rather than an empty result. Without --skip-appset-generation-errors this error aborts the entire diff run — even for unrelated ApplicationSets.

[dag-andersen]

Hi @szhekpisov-katanox!

Again, thank you so much for the effort and thoughts you have put into your PRs. I really like when people are willing to discuss this tool with me! That is the only way we can make it better.

I have been thinking a bit about this feature.

My overall philosophy is that I want users to trust the tool and never feel like: "Why didn't it catch that???". In general, I would rather be pessimistic when filtering Applications and so on. Better to render too many Applications than accidentally miss one. I want to avoid false negatives at basically all cost. I believe this builds trust in the tool, even if it makes the tool a bit slower or some features a bit more cumbersome.

With that in mind, I usually prefer not to create any kind of --skip-xxx-errors flag, since it could conceal a real error that should have been detected.

So in general, I would always suggest that people add argocd-diff-preview/render: "never" to the Applications/ApplicationSets that they know will fail, instead of simply using an --ignore-errors flag.
That makes the decision explicit and local to the problematic resource, instead of allowing the whole run to ignore an entire class of errors.

However, if we know that there are certain Applications/ApplicationSets that are problematic, then we should maybe add a flag (or simply make it the default) that ignores errors for that type of Application. For example, it might be a good idea to always skip ApplicationSet pullRequest generators.

What do you think? :)

Let me know if there are other cases we need to handle, or if you have other examples of problematic ApplicationSets that we may need to handle. Otherwise, we may simply end up creating the --skip-appset-generation-errors flag.

[szhekpisov-katanox]

Hi @dag-andersen!

Thank you for your response on that. So far the pullRequest generator is the only case I've hit. I don't think it should be skipped automatically though — I'd rather leave the decision to the user per resource, rather than have the tool silently drop a whole generator type.

And I agree, making exclusion specific makes sense to me — let's drop this PR then. The only thing I would suggest in that case is to add a hint in the error: if an ApplicationSet like the one above fails, it can simply be excluded from evaluation with the argocd-diff-preview/render: "never" label. WDYT?