feat: multisource support

[askhari]

It relates to PR #631 and includes the same changes as it's based on that code.

Also, this relates to the following issues:

• Git write-back filename configurable, it defaults to .argocd-source-{application}.yaml when using Helm #551
• Multiple helm sources: skipping app of type '' because it's not of supported source type #558
• Support Multiple Sources as per ArgoCD v2.6.0 New Field #513

This PR provides multisource support for git write back configurations on Helm application types.
Also it adds a new feature to provide a helm values file for the git write back. This makes it easier to keep value files up to date.
In order to use the Helm values feature, you'll need to provide a "helmvalues" prefix in the write-back-target header. You can take a look to the write-back-target documentation in this code to know how to configure it.

There are test for all the new code, they cover the most basic cases.
I tried to keep the same coding pattern and built the least invasive changes in the code flow.

Please give it a try and let's see if it can be merged. Any suggestions are more than welcome.

[codecov-commenter]

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (82d59ef) 65.35% compared to head (cfbb5c3) 66.41%.

Files	Patch %	Lines
pkg/argocd/argocd.go	72.22%	8 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #636      +/-   ##
==========================================
+ Coverage   65.35%   66.41%   +1.06%     
==========================================
  Files          22       22              
  Lines        2084     2138      +54     
==========================================
+ Hits         1362     1420      +58     
+ Misses        588      587       -1     
+ Partials      134      131       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[askhari]

Sorry I didn't notice that the coverage went down. I'll make the necessary changes to keep the coverage at the same rate.

[askhari]

Sorry I didn't notice that the coverage went down. I'll make the necessary changes to keep the coverage at the same rate.

I added some tests to raise the code coverage. Please, let me know if more changes are required in order to approve this PR. :)

[kaessert]

This is great stuff! Maybe @jannfis has the chance to look at it. I think many folks out there are quite desparately waiting for multi-source support. For us it would be a game-changer.

[kvlxj]

This looks really promising ! We've been looking for this feature for months and hoping multi sources will be supported.

This might not be the right place to ask but I cannot find any issue or PR concerning valuesObject support for helm source ? I tested with it a few months back but can't seem to make it work apparently with v2.8.x ArgoCD.

Thank you @askhari for working on this anyway :)

[neilborromeo]

We're waiting for this PR to fix #558 too. Thank you!

[joshiparth1000]

when can we expect this PR to be merged? we desperately need this for our development workflow.

[kaessert]

There is another issue which is worth pointing out: Image-updater will write correctly to the destination repo, however argocd will only pick up parameter files from the main repo. (In case you want to offload images to a separate repo which was our plan at first)

[devopsidiot]

Really need this for my use case! Greatly looking forward to the merge and release! Amazing work @askhari!

[askhari]

@kvlxj , I also did not find anything related to the valuesObject. It would be helpful as well, but I don't think we should include that change in this PR. Maybe we may work on that and make a new PR.

[askhari]

There is another issue which is worth pointing out: Image-updater will write correctly to the destination repo, however argocd will only pick up parameter files from the main repo. (In case you want to offload images to a separate repo which was our plan at first)

@atarax , when you pointed out that argocd will only pick up parameter files from the main repo, I think this only applies to the Parameters when they are written without using the "helmvalues" feature of this PR. Is that right?
I mean, if we use the "helmvalues" feature and write into a separate repository with the helm values file and in ArgoCD the application is configured with two repositories (one for the Chart and one for the values), then ArgoCD will render it correctly (afaik). Am I understanding it wrong?

[kaessert]

@askhari yes, and no :) it will work but it will break standard naming conventions. We only got it to work with something like:

image_name: ...
image_tag: ...

but for most apps it's nested

image:
  name: ...
  tag: ...

[askhari]

Yup, you are more than right. We suffered ourselves that one.

I used conflate library to easy the YAML and string manipulation to merge content because I'm not that good coding ^^U .

I understand that it would be much better to indent in YAML. Honestly, right now I don't see a good way to solve that problem. In our case, we just assumed that the name of the parameter would be that one because it was easy to change in our environments.

If it's a heavy issue for this to go on, I'll try to solve whenever I get some more time for it.

In any case, thank you @atarax for pointing it out. It helps to improve :)

[kaessert]

@askhari sure, you're welcome! :) And btw, i think it would be way easier to solve on argocd side. I think the whole topic of multi-source application is not very well explored yet. But i could imagine maybe a property there to point argocd to the repo where it should take the parameter-files from or make it possible to have paramter-files from multiple repositories.

[devopsidiot]

Thank you so much @askhari and @atarax - It's working in my environment. Now it's time to get my projects actually doing the thing! 😄

[devopsidiot]

@askhari - I couldn't get it to work 😞
Here's my application.yaml

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name:test
  namespace: argo
  annotations:
    argocd-image-updater.argoproj.io/image-list: testalias=account.dkr.ecr.us-east-1.amazonaws.com/test
    argocd-image-updater.argoproj.io/testalias.helm.image-name: image_name
    argocd-image-updater.argoproj.io/testalias.helm.image-tag: image_tag
    argocd-image-updater.argoproj.io/testalias.allow-tags: regexp:develop-v.*
    argocd-image-updater.argoproj.io/testalias.update-strategy: latest
    argocd-image-updater.argoproj.io/testalias.force-update: "true"
    argocd-image-updater.argoproj.io/testalias.pull-secret: ext:/scripts/ecr-login.sh
    argocd-image-updater.argoproj.io/write-back-method: git:secret:argo-image-updater/argocd-image-updater-git-creds
    argocd-image-updater.argoproj.io/git-repository: [email protected]:company/gitops.git
    argocd-image-updater.argoproj.io/write-back-target: "helmvalues:./values-rnd.yaml"
    argocd-image-updater.argoproj.io/git-branch: rnd
  finalizers:
  - resources-finalizer.argocd.argoproj.io
spec:
  sources:
    - repoURL: http://chartmuseum.chartmuseum.svc:8080
      targetRevision: 0.1.33
      chart: world-eater
      helm:
        valueFiles:
        - $values/flux/rnd-cluster/workloads/big-test/values-rnd.yaml
    - repoURL: [email protected]:company/gitops.git
      targetRevision: rnd
      ref: values

Here's my values.yaml file that I think would relate:

---
global:
  namespace: sandbox

label: test

configMapRef: test-configmap
secretRef: test-secret

image_name: accountid.dkr.ecr.us-east-1.amazonaws.com/test
image_tag: develop-v0.20.0-33

the image is unable to find the image-name in the annotation or values file:

time="2023-11-27T17:10:39Z" level=error msg="Could not update application spec: could not find an image-name annotation for test" application=test

@atarax - you said you had some issues with the annotations - what finally fixed it for you?

[devopsidiot]

Yeah, I've got argocd upgraded to the latest version and changed up my values.yaml file to adopt the image_name & image_tag like @atarax said they got it to work and I'm still getting the error: level=error msg="Could not update application spec: could not find an image-name annotation for image test" application=test

Would really appreciate some suggestions.

[askhari]

Yeah, I've got argocd upgraded to the latest version and changed up my values.yaml file to adopt the image_name & image_tag like @atarax said they got it to work and I'm still getting the error: level=error msg="Could not update application spec: could not find an image-name annotation for image test" application=test

Would really appreciate some suggestions.

@devopsidiot , the error messages says that the docker image name is "test" instead of "testalias" (where it says "annotation for image test").
Given that in the application manifest the annotations point to a "testalias" (for example in "argocd-image-updater.argoproj.io/testalias.helm.image-name: image_name"), then argocd-image-updated cannot find any annotation related to the "test" image and throws that error.

Usually, the name of the image is retrieved from the field "Status.Summary.Images" at the application manifest. If you use the "force-update" annotation, it also uses the image alias.

Maybe this information will help you :).

[jannfis]

Wow! Thank you so much for this awesome PR, @askhari!

Sorry for coming back so late to this one. I don't have anything to complain, this is a great quality contribution.

LGTM!

[askhari]

Thank you @jannfis! :)

Hopefully this may help some people and I'm really happy about that.

[SamPeng87]

Yeah, I've got argocd upgraded to the latest version and changed up my values.yaml file to adopt the image_name & image_tag like @atarax said they got it to work and I'm still getting the error: level=error msg="Could not update application spec: could not find an image-name annotation for image test" application=test
Would really appreciate some suggestions.

@devopsidiot , the error messages says that the docker image name is "test" instead of "testalias" (where it says "annotation for image test"). Given that in the application manifest the annotations point to a "testalias" (for example in "argocd-image-updater.argoproj.io/testalias.helm.image-name: image_name"), then argocd-image-updated cannot find any annotation related to the "test" image and throws that error.

Usually, the name of the image is retrieved from the field "Status.Summary.Images" at the application manifest. If you use the "force-update" annotation, it also uses the image alias.

Maybe this information will help you :).

We have this, too。

I understand what you're saying about not finding the right image. However, the image alias is already configured in image-list. I understand that this image alias should be the base rule. Not anything else.

[SamPeng87]

I mean's,that c.ImageName is mistake,not use image-list defined alias,may be use c.ImageAlias is right?

[SamPeng87]

I review more code,I found if you use like ContainerImage's normalizedSymbolicNam is good idea,If there are multiple layers of "/" in image, it is not legal to use the annotation name.There's no image list in this place?

[askhari]

@SamPeng87, afaik we could not use directly the "ImageAlias" because there are cases where it might be empty or people just do not use it.

The image list iterated in that "for" sentence is retrieved by the "GetImagesFromApplication" function, which builds a list using the name of each docker image found in the "app..Status.Summary.Images" (which is a structure that comes directly from ArgoCD) or using the "argocd-image-updater.argoproj.io/image-list" annotation.

As the "GetImagesFromApplication" was already used, I reused it to reproduce the same behavior.

Given that this happens only using the "helmvalues" method, it should not affect any other default behaviors of argocd-image-updater (at least I hope it won't affect).

I understand that we may improve this new feature to use "ImageAlias" whenever the annotations use an alias for the docker image. I'll note that and work on it later on.

Thank you very much for noticing it! :)

[pavel-kurnosov]

If i don't set helmvalues in write-back-target then it is able to commit, it finds proper annotations and etc. But as soon as I set write-back-target and specify values.yaml it is failing with error: "Could not update application spec: could not find an image-name annotation for". Should we register a bug for that usecase?

[askhari]

@pavel-kurnosov , this is the same as @SamPeng87 noticed. I'll take a look into it when I have some more time.
If you want to open an issue I think it's ok, then we'll be able to reference it when pushing the PR to solve this.

Thank you!

[dominicroessner]

Since this is merged now, any estimate on when we can expect a helm release with these changes?

[devopsidiot]

Honestly @dominicroessner, if it comes, it might be 1-6 months later. Even though this particular PR doesn't work with my use case and @askhari is going to iterate on some more stuff, I pulled this down and complied locally and pushed it to ECR and referenced that container in the helm chart. I'd suggest doing the same.

[ks-yim]

Hi, @jannfis sorry to bother you, but can you please share the next release schedule which will include this and the other commits so far?

Compiling a custom image is our best as @devopsidiot suggested?

[aburan28]

Any update on when this will be released?

[vitaly-dt]

Same here, any update on when will it be released?

[theoribeiro]

I know multisource is still in beta but this is a really useful feature for our org. Would love to have this in the next release, any updates?

[adiii717]

Any update on when this will be released? please

[Sovietaced]

This introduces a regression. The old code would observe the app status which included computed fields for source type. The new code looks at the application spec which in our case does not have any of these fields defined.

[Sovietaced]

I don't know if its related but we are using an ApplicationSet where you simply pass the path, repoURL, etc: https://argo-cd.readthedocs.io/en/stable/operator-manual/applicationset/Template/#template-fields. I'm not aware of any options to specify kustomize/helm specific things.

[Sovietaced]

This seems to fix the issue for me.

// getApplicationSourceType returns the source type of the application
func getApplicationSourceType(app *v1alpha1.Application) v1alpha1.ApplicationSourceType {

	if st, set := app.Annotations[common.WriteBackTargetAnnotation]; set &&
		strings.HasPrefix(st, common.KustomizationPrefix) {
		return v1alpha1.ApplicationSourceTypeKustomize
	}

	if app.Spec.HasMultipleSources() {
		for _, st := range app.Status.SourceTypes {
			if st == v1alpha1.ApplicationSourceTypeHelm {
				return v1alpha1.ApplicationSourceTypeHelm
			} else if st == v1alpha1.ApplicationSourceTypeKustomize {
				return v1alpha1.ApplicationSourceTypeKustomize
			} else if st == v1alpha1.ApplicationSourceTypePlugin {
				return v1alpha1.ApplicationSourceTypePlugin
			}
		}
		return v1alpha1.ApplicationSourceTypeDirectory
	}

	return app.Status.SourceType
}

[mecsys]

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

[hofq]

How does Kustomize Support work?

Having "kustomization" as git write-back-target does not work and putting "kustomization:/path/in/repo" does end in an crash loop of the application, as the error is not handled.