Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple helm sources: skipping app of type '' because it's not of supported source type #558

Open
admssa opened this issue Apr 24, 2023 · 40 comments
Labels
bug Something isn't working

Comments

@admssa
Copy link

admssa commented Apr 24, 2023

Describe the bug
The problem is that when you store helm values files in a separate repo you use spes.sources instead of spes.source in your Application manifests. Thus instead of sourceType string in application status we will have sourceTypes array.

status:
  sourceTypes:
  - Helm

Unfortunately argocd-image-updater check for sourceType only:

func getApplicationType(app *v1alpha1.Application) ApplicationType {
sourceType := app.Status.SourceType
if st, set := app.Annotations[common.WriteBackTargetAnnotation]; set &&
strings.HasPrefix(st, common.KustomizationPrefix) {
sourceType = v1alpha1.ApplicationSourceTypeKustomize
}
if sourceType == v1alpha1.ApplicationSourceTypeKustomize {
return ApplicationTypeKustomize
} else if sourceType == v1alpha1.ApplicationSourceTypeHelm {
return ApplicationTypeHelm
} else {
return ApplicationTypeUnsupported
}
}

To Reproduce
Replace values surrounded by << >> and create Application with 2 sources and follow the logs:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: test
  namespace: argocd
  annotations:
    argocd-image-updater.argoproj.io/write-back-method: git
    argocd-image-updater.argoproj.io/image-list: test=<<REPO_URL/test>>
    argocd-image-updater.argoproj.io/test.force-update: "true"
    argocd-image-updater.argoproj.io/test.helm.image-tag: deployment.image.tag
    argocd-image-updater.argoproj.io/test.helm.image-name: deployment.image.repository
    argocd-image-updater.argoproj.io/test.update-strategy: latest
    argocd-image-updater.argoproj.io/test.allow-tags: regexp:(tested-dev-)
spec:
  project: default
  sources:
    - repoURL: <<rREPO_URL>>
      chart: test
      targetRevision: 0.1.0
      helm:
        releaseName: test
        valueFiles:
          - $values/dev/values/test.yaml
    - repoURL: "[email protected]:<<YOUR_ORG>>/gitops-backend.git"
      targetRevision: HEAD
      ref: values
  destination:
    server: "https://kubernetes.default.svc"
    namespace: dev
  syncPolicy:
    automated:
      prune: true
      selfHeal: true
      allowEmpty: false
    syncOptions:
      - Validate=true
      - CreateNamespace=true
      - PrunePropagationPolicy=foreground
      - PruneLast=true

Version
0.12.2

Logs

time="2023-04-24T08:48:37Z" level=info msg="Processing results: applications=0 images_considered=0 images_skipped=0 images_updated=0 errors=0"
time="2023-04-24T08:50:38Z" level=warning msg="skipping app 'test' of type '' because it's not of supported source type" application=test
@admssa admssa added the bug Something isn't working label Apr 24, 2023
@amgonzalezf
Copy link

I have the same issue, thanks for reporting it @admssa

Until this problem gets fixed, the only thing we can do in order to continue using the image updater with helm is to use only one source repository in the application manifest.

@skiv99
Copy link

skiv99 commented Apr 25, 2023

I have the same problem too. If we use spes.sources image updater can't commit a new image in a git repo.

@natalicot
Copy link

Thank you for reporting!
We have the same problem

@deferraz
Copy link
Contributor

deferraz commented May 8, 2023

This is related with #513, looks like there is a possible solution in #548

@TheDukeDK
Copy link

Same problem here. Pretty important for us as it breaks our preferred development flow.

@LarsBingBong
Copy link

Any news on this one?

@TheDukeDK
Copy link

What's the status on this one? We would really like to be able to use a common helm chart and the image updater together and this is blocking.

@SunnyBerlin
Copy link

First time today wanted to play with image-updater , but we also plan to use multiple sources : (((

@filipprosovsky
Copy link

Tested @deferraz PR#578 by building image from feature branch. However, sources worked, but only with argocd-image-updater.argoproj.io/write-back-method: argocd.
If the first source is helm repository, it cannot push file to git. I further edited the code to try override repoURL (I can imagine adding an annotation to specify to which repository to push the image updater source file). The file was pushed to git (you also need to specify spec.sources[0].path so it is placed to correct location in git. This could possibly be set by annotation when using helm? as path is "ignored" as described in ArgoCD documentation). However, even after file is pushed with correct image and tags specified in annotations, the image is not changed in application, no error shown.

@askhari
Copy link
Contributor

askhari commented Oct 31, 2023

@filipprosovsky , I opened this PR #636 that may help with the "git" write back method. Just let me know if that helps.

@unfernandito
Copy link

Hi.

It's there any workaroun to make multiple sources work with this tool?

@Danielkiss9
Copy link

2024 here :)
I see a fix was merged to master - any news on when it will be released as a stable version?

@larssb
Copy link

larssb commented Feb 22, 2024

Any news?

@bashirmindee
Copy link

any news ?

@bart-braidwell
Copy link

looking forward for that fix.

@TheDukeDK
Copy link

Checking in as well. Can see that PR #636 is merged. Does that cover all the needed use cases? Is there a release where this functionality is targeted?

@dominykasn
Copy link

Update please?

@rounakdatta
Copy link

A new release has been published a few days back, please check if it solves for you. For me, the fix is working!

@mecsys
Copy link

mecsys commented May 20, 2024

Works! I tested (0.13.0 image) argocd app updates and write back changes to git repo. Thanks argocd image updater team!

@zagr0
Copy link

zagr0 commented May 20, 2024

Hi @mecsys , could you please share application example that works for your scenario. Because for me when values.yaml file with tag field is in separate repo I got error like described in the issue

@mecsys
Copy link

mecsys commented May 20, 2024

@zagr0

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: podinfo
  namespace: argo-cd
  annotations:
    argocd-image-updater.argoproj.io/image-list: podinfo=ghcr.io/stefanprodan/podinfo:6.x.x
    argocd-image-updater.argoproj.io/podinfo.helm.image-name: image.repository
    argocd-image-updater.argoproj.io/podinfo.helm.image-tag: image.tag
    # argocd app strategy
    argocd-image-updater.argoproj.io/write-back-method: argocd
    # git strategy
    # argocd-image-updater.argoproj.io/write-back-method: git:secret:argo-cd/argocd-repo-creds-ops-mecsys    
    # argocd-image-updater.argoproj.io/git-repository: [email protected]:mecsys/mecsys-gitops.git
    # argocd-image-updater.argoproj.io/git-branch: main:image-updater{{range .Images}}-{{.Name}}-{{.NewTag}}{{end}}
spec:
  destination:
    namespace: podinfo
    server: https://kubernetes.default.svc
  project: infra
  sources:
    - chart: podinfo
      repoURL: https://stefanprodan.github.io/podinfo
      targetRevision: 6.5.1
      path: infrastructure/production/apps/ # this is for image updater to know where to store the .argocd-source-appName.yaml file.
      helm:
        releaseName: podinfo
        valueFiles:
          - $values/infrastructure/production/infra-values/podinfo/values.yaml        
    - ref: values
      repoURL: [email protected]:mecsys/mecsys-gitops.git
      targetRevision: main
  syncPolicy:
    automated:
      prune: true
      selfHeal: true
    syncOptions:
      - CreateNamespace=true

image

and for write-back-method == git

image

The last one i trying investigate why argocd is not sync after merge.

@zagr0
Copy link

zagr0 commented May 20, 2024

@mecsys , thank you! It becomes more clear now. I see it still works with parameter overrides, but my expectations were to have changes commited directly in values.yaml file in the image.tag

@askhari
Copy link
Contributor

askhari commented May 23, 2024

Hi @zagr0 ,

I think that the annotation "argocd-image-updater.argoproj.io/write-back-target: helmvalues" is missing in the configuration. This annotation is used to specify the values file you want to use for the writeback.

I checked the argocd-image-updater documentation for release v0.13.0 and it seems that it's not updated with the changes of the PR #636 . This is a direct link to the file of that PR with the "helmvalues" documentation. I hope this helps.

Cheers!

@zagr0
Copy link

zagr0 commented May 23, 2024

Hi @askhari , thank you! I tried it, but it leads to error: "Could not update application spec: could not find an image-name annotation for image foo/bar". Hoping this issue should be a fix for that

@askhari
Copy link
Contributor

askhari commented May 23, 2024

@zagr0 , could you please share the current configuration you are using for the application manifest please? That may help to understand if there is some misconfiguration or the code does not support it yet.

@zagr0
Copy link

zagr0 commented May 23, 2024

sure, my app is:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: stage-app
  annotations:
    argocd-image-updater.argoproj.io/image-list: main=my.repo.domain:5353/common/app
    argocd-image-updater.argoproj.io/main.pull-secret: pullsecret:argocd/registry
    argocd-image-updater.argoproj.io/main.helm.image-name: image.repository
    argocd-image-updater.argoproj.io/main.helm.image-tag: image.tag
    argocd-image-updater.argoproj.io/main.update-strategy: newest-build
    argocd-image-updater.argoproj.io/write-back-method: git:secret:argocd/git-creds
    argocd-image-updater.argoproj.io/write-back-target: helmvalues:./stage/app-helm/values.yaml
    argocd-image-updater.argoproj.io/git-repository: https://my.repo.domain/deploy.git
    argocd-image-updater.argoproj.io/git-branch: master
spec:
  project: stage
  sources:
    - repoURL: https://my.repo.domain/api/v4/projects/123/packages/helm/stable
      chart: common
      targetRevision: 2.*
      helm:
        releaseName: app
        valueFiles:
          - $values/stage/app-helm/values.yaml
    - repoURL: https://my.repo.domain/deploy.git
      targetRevision: master
      ref: values
  destination:
    server: https://kubernetes.default.svc
    namespace: stage
  syncPolicy:
    syncOptions:
      - CreateNamespace=true

the repo tree:

stage
  app-helm
    values.yaml

values file content:

image:
  repository: "my.repo.domain:5353/common/app"
  tag: "dev"

@askhari
Copy link
Contributor

askhari commented May 23, 2024

@zagr0 , maybe this comment will help to understand the behaviour.

The thing is that argocd-image-updater uses to different pieces of information that should match in order to work properly, one is the "alias" name that you use for the image and the other one is the docker image name. If those names do not match, then it will throw that error.
You may tray to change the alias name and see what happens. If you do that, you'll also need to change the references to the alias name in the rest of the annotations. I don't know if this changes have a big impact on your configurations, but you may try.

I understant that this is a bug that should be addresses, I just did not have more time to spend since the last time I did the PR. I'm sorry about that.

I hope the info will help.

@zagr0
Copy link

zagr0 commented May 23, 2024

@askhari , thank you very much to make it clear! Unfortunately I'm not able use alias with the value of image name common/app. Because of annotations key naming constrains:

metadata.annotations: Invalid value: a qualified name must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]') with an optional DNS subdomain prefix and '/' (e.g. 'example.com/MyName')

@askhari
Copy link
Contributor

askhari commented May 24, 2024

Hi @zagr0 ,

I did a quick read to the code again and I'm not really quite sure if this will work, but could you try to use this configuration for your annotations and see what happens please?

annotations: argocd-image-updater.argoproj.io/image-list: my.repo.domain:5353/common/app argocd-image-updater.argoproj.io/common_app.pull-secret: pullsecret:argocd/registry argocd-image-updater.argoproj.io/common_app.helm.image-name: image.repository argocd-image-updater.argoproj.io/common_app.helm.image-tag: image.tag argocd-image-updater.argoproj.io/common/app.helm.image-name: image.repository argocd-image-updater.argoproj.io/common/app.helm.image-tag: image.tag argocd-image-updater.argoproj.io/common_app.update-strategy: newest-build argocd-image-updater.argoproj.io/write-back-method: git:secret:argocd/git-creds argocd-image-updater.argoproj.io/write-back-target: helmvalues:./stage/app-helm/values.yaml argocd-image-updater.argoproj.io/git-repository: https://my.repo.domain/deploy.git argocd-image-updater.argoproj.io/git-branch: master

I know that this configuration is a bit crappy as there are two annotations that just repeat themselves with a slight different syntax. There is a function called normalizedSymbolicName() that substitutes the "/" character for "_", but it's not used in all the references used to build annotation names in the code. For what I read in the code I think it may work only for this case and configuration.
This behaviour comes from the details explained in this conversation on the original PR. I may change the c.ImageName for a img.normalizedSymbolicName() . It may solve the duplicated annotations, but it won't solve the problem of using an alias name for the images. That would need some more digging and changes into the code.

Not sure if this helps. I'm sorry for the inconveniences.

@zagr0
Copy link

zagr0 commented May 24, 2024

hi @askhari ,

I tried the example but got error complaining about annotation key names: metadata.annotations: Invalid value...
To be honest I didn't catch the idea, could you please check if formatted annotations I applied are correct?

annotations:
  argocd-image-updater.argoproj.io/image-list: my.repo.domain:5353/common/app
  argocd-image-updater.argoproj.io/common_app.pull-secret: pullsecret:argocd/registry
  argocd-image-updater.argoproj.io/common_app.helm.image-name: image.repository
  argocd-image-updater.argoproj.io/common_app.helm.image-tag: image.tag
  argocd-image-updater.argoproj.io/common/app.helm.image-name: image.repository
  argocd-image-updater.argoproj.io/common/app.helm.image-tag: image.tag
  argocd-image-updater.argoproj.io/common_app.update-strategy: newest-build
  argocd-image-updater.argoproj.io/write-back-method: git:secret:argocd/git-creds
  argocd-image-updater.argoproj.io/write-back-target: helmvalues:./stage/app-helm/values.yaml
  argocd-image-updater.argoproj.io/git-repository: https://my.repo.domain/deploy.git
  argocd-image-updater.argoproj.io/git-branch: master

@askhari
Copy link
Contributor

askhari commented May 24, 2024

Hi @zagr0 ,

It was my mistake thinking that I could use the "/" character. I'm sorry for the error.

I though it was worth to try it because the code reads the annotations in multiple parts. But in some of them uses a function which replaces the "/" from the image name with "_" characters. And in other parts of the code uses directly the ImageName value without replacing the "/".
In the end it's not possible to use another slash in the annotation name because it just allows one.

Again, sorry because I may wasted some of your time with this tests. I'll try to free some time to work on a fix for this asap, but I'm not sure if it will be this week.

@askhari
Copy link
Contributor

askhari commented May 27, 2024

@zagr0 , I opened #725 which aims to fix the problem of not finding the right annotations.

Let's see if I did the PR in the right way, because it's the first time I try to send a fix for a specific version.

@askhari
Copy link
Contributor

askhari commented Jun 5, 2024

@zagr0 , the PR #725 has been merged into master but there isn't a new tag yet. You may try building the master branch or the code of the PR. It should solve the issue when using image aliases.

@zagr0
Copy link

zagr0 commented Jun 5, 2024

Thank you very much @askhari ! Most probably there are image builds with latest tag from master, I will check it.

@zagr0
Copy link

zagr0 commented Jun 13, 2024

Hi @askhari ,

I have verified with my scenario and now there is no issues with finding image name, this part works, but the changes committed to values.yaml file looks really strange.

Image updater was not able to discover image: hierarchy and tag inside and just added new entries of image.tag and image.repository in the file:

...
image:
  pullPolicy: IfNotPresent
  repository: my.repo.domain/common/app
  tag: 0.0.6-SNAPSHOT_3ced321a
image.repository: my.repo.domain/common/app
image.tag: 0.0.5
...

In addition to that image updater recreated content of the values.yaml to format it, ordered all values alphabetically.

@zagr0
Copy link

zagr0 commented Jun 13, 2024

here is the issue

@askhari
Copy link
Contributor

askhari commented Jun 16, 2024

@zagr0 , thank you for the feedback.

As I understood there are two different issues (please correct me if I'm wrong):

  1. Right now, this feature does not search into the YAML hierarchy. Instead, it searches for a key that includes the dots.
  2. And it messes up with the order of the keys in the YAML file when writing it back to the repository.

The first thing was fine for us, but I understand that it makes more sense to navigate the hierarchy.
The second one it a bit trickier to solve because it's the natural behaviour of the golang YAML library. To solve it we need to use MapSlice type and it makes more complicated (at least for me).

I'm already working to solve this two issues on this branch of my fork and I'll make a PR shortly: https://github.com/askhari/argocd-image-updater/tree/fix/helm-values-wbc.

Again, thank you for noticing it.

Cheers!

@askhari
Copy link
Contributor

askhari commented Jun 16, 2024

Hi @zagr0 ,

I opened #748 to solve these problems. Let's see if it gets approved :)

Cheers!

@askhari
Copy link
Contributor

askhari commented Jul 2, 2024

@admssa , now that #725 and #748 have been merged into master, could you please test if they solve the problem you explained in this issue?
If the problem is solved, we may close this issue :) .

@larssb
Copy link

larssb commented Jul 2, 2024

@askhari thank you so much for working on this! Aren't we waiting for a new version of the image-updater here...to get the fixes you've done?

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests