Image Automation not honouring GitRepository exclusions

[jvelasquez]

Hi all,

First of all, thanks for the work.

I am using the latest version of Flux2 and image-automation-controller / reflector. Given the following resources:

---
# Source: environment/templates/env-sync.yaml
apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: GitRepository
metadata:
  name: dev-123
  namespace: flux-system
spec:
  interval: 1m0s
  ref:
    branch: master
  secretRef:
    name: flux-system
  url: https://gitlab.com/unodos/gitlab-ci/gitops
  ignore: |
    # exclude all but environment dev-123 patches.yaml
    /*
    !/cluster/environments/dev-123/patches.yaml
---
# Source: environment/templates/patches.yaml
apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: podinfo
  namespace: dev-123
spec:
  values:
    image:
      tag: 5.0.1 # {"$imagepolicy": "flux-system:dev-123-podinfo-policy:tag"}
# Source: environment/templates/image-automations.yaml
apiVersion: image.toolkit.fluxcd.io/v1alpha1
kind: ImageUpdateAutomation
metadata:
  name: dev-123-podinfo-automation
  namespace: flux-system
spec:
  interval: 1m
  checkout:
    branch: master
    gitRepositoryRef:
      name: dev-123
  update:
    setters: {}
  commit:
    authorName: UpdateBot
    authorEmail: [email protected]

I am seeing this error:

image-automation-controller-5849df9999-tftj5 manager {"level":"error","ts":"2020-12-21T13:18:30.688Z","logger":"controller","msg":"Reconciler error","reconcilerGroup":"image.toolkit.fluxcd.io","reconcilerKind":"ImageUpdateAutomation","controller":"imageupdateautomation","name":"dev-123-podinfo-automation","namespace":"flux-system","error":"/tmp/flux-system-dev-123935373182/helm/charts/environment/templates/image-automations.yaml: yaml: line 3: did not find expected key"}

Note that the path is out of scope for the Gitrepository CRD I have created (only includes cluster/environments/dev-123/patches.yaml). Is this an expected behavior?

Thanks!

[stefanprodan]

Related to #68

[squaremo]

The image automation uses the git repository type only for its specification of how to gain access to a git repository (i.e., the url and secretRef fields). It doesn't use other parts of the spec, and in some cases it would be nonsensical to do so -- e.g., what should it make of ref: {semver: "1.2" }. In other words, it reuses the type for convenience, but it's not really a great fit.

I can see why you would want it to respect the ignore field specifically; it would be handy for excluding things you don't want to be updated (there was a field in the automation object for including paths, but it wasn't implemented, and got removed). I can see some ways it would complicate matters -- if you have several automations working on different parts of the same git repo, it would force you to provide a repo object for each, for instance -- but broadly, useful. However, I am wary of going further in supporting _some_fields of GitRepository while ignoring others.

[jvelasquez]

Hi @squaremo, thanks for the comment.

The image automation uses the git repository type only for its specification of how to gain access to a git repository (i.e., the url and secretRef fields). It doesn't use other parts of the spec, and in some cases it would be nonsensical to do so -- e.g., what should it make of ref: {semver: "1.2" }. In other words, it reuses the type for convenience, but it's not really a great fit.

Two things I can see here:

1. I believe that the use of GitRepository in Automation should be then clarified in the documentation -> The image automation uses the git repository type only for its specification of how to gain access to a git repository (i.e., the url and secretRef fields)
2. does not makes sense ref: {semver: "1.2" } you are right. It is up to the user to f*** things up or not, stating it clear in the docu should suffice.

I can see why you would want it to respect the ignore field specifically; it would be handy for excluding things you don't want to be updated (there was a field in the automation object for including paths, but it wasn't implemented, and got removed). I can see some ways it would complicate matters -- if you have several automations working on different parts of the same git repo, it would force you to provide a repo object for each, for instance -- but broadly, useful. However, I am wary of going further in supporting _some_fields of GitRepository while ignoring others.

1. I think adding the GitRepositoryRef, makes the user believe this object will be managed as is in the ImageAutomation resource. Maybe allowing ignore would make sense (would this be such a major change or make the solution more complex?)

apiVersion: image.toolkit.fluxcd.io/v1alpha1
kind: ImageUpdateAutomation
metadata:
  name: aaa
  namespace: aaa
spec:
  interval: 1m
  checkout:
    branch: dev
    # by default spec.ignore will be blank.
    ignore: |
      /charts
      REDME.md
    gitRepositoryRef:
      name: flux-system
  update:
    setters: {}
  commit:
    authorName: UpdateBot
    authorEmail: [email protected]

[stefanprodan]

I think all fields in the GitRepository should apply to image automation, ref: {semver: "1.2" } makes sense if you want to use the latest released YAMLs and push to a different branch.

[squaremo]

I think all fields in the GitRepository should apply to image automation

I'm open to that; but I suspect it will lead to some awkward design, since those fields are not intended to serve the automation API.

So, we could come up with an interpretation of refs -- semver, commit and tag -- but they don't necessarily all make sense. Maybe someone out there wants to always branch from a specific commit, but then what does the automation push the new commits to? (Inevitably, the specification gets split over two different objects.)

Then we'd need to reinterpret interval (automation already has an interval, and it should stay separate to that of the git repository); timeout seems OK, verification might be useful, but maybe not the way it's used for a source.

It also means whenever you want to add anything to the GitRepository spec, you have to come up with an interpretation for automation, or have the worst-of-both-worlds situation of supporting some fields but not others.

[squaremo]

1. I think adding the GitRepositoryRef, makes the user believe this object will be managed as is in the ImageAutomation resource. Maybe allowing ignore would make sense (would this be such a major change or make the solution more complex?)

Reusing parts of the specification might make more sense -- that means we can import that bits that have some reasonable interpretation. This would probably mean dropping gitRepositoryRef in favour of just supplying the URL and secret in the automation object.

This feels like it's becoming a larger discussion. I'll create one in flux2. <-- fluxcd/flux2#665

NB I don't think that discussion blocks implementing ignore. The outcome, if anything changes, will be to change where it's specified rather than its implementation.