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

ApplicationSet template is missing toYaml/fromYaml functions #11993

Closed
alexbde opened this issue Jan 16, 2023 · 8 comments · Fixed by #15063
Closed

ApplicationSet template is missing toYaml/fromYaml functions #11993

alexbde opened this issue Jan 16, 2023 · 8 comments · Fixed by #15063
Assignees
Labels
component:applications-set Bulk application management related enhancement New feature or request

Comments

@alexbde
Copy link
Contributor

alexbde commented Jan 16, 2023

We do have an use case which I would have thought is a pretty common one: we have an application Helm chart and multiple locations with application configurations. E.g., a git repository with general/common configurations (valid for all stages) & another git repository with stage-specific configurations.

Today we’re using the app-of-apps pattern with Helm charts to read & merge those configs/values.yaml files. Minimal example:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata: ...
spec:
  source:
    helm:
      values: |
        {{- .Values.stage | merge .Values.general | toYaml | nindent 8 }}
...

We tried to move to ApplicationSet instead of app-of-apps because it looks like the securer and more future-proof concept. We were very happy we can now use Go template with Argo CD >= 2.5 and the merging is working perfectly. Sadly, we noticed there is no toYaml/fromYaml function. We’re super close to the goal with the right result, but couldn’t simply print it.

apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata: ...
spec:
  generators:
  - ... // merge or matrix with multiple (git) generators
  goTemplate: true
  template:
    metadata: ...
    spec:
      source:
        helm:
          values: |
            {{ (omit . "path") | toYaml }} // not working, missing function, so close!
...

Would it be possible to add this function to the upcoming 2.6 release? There is already a toJson function, which would at least make it work (because json is a subset of yaml). To make it beautiful and more native it would be great to have a toYaml/fromYaml function.

@crenshaw-dev
Copy link
Collaborator

@alexbde I'm also surprised those functions aren't available. I'd welcome a PR to add them. Might event consider cherry-picking back to 2.5 if it's a pretty minimal PR.

@alexbde
Copy link
Contributor Author

alexbde commented Jan 20, 2023

@crenshaw-dev Yeah, so was I, I would have actually expected it in the sprig library. I'm not a go developer at all, but I will try to have a look 👍

To whom it may concern: there are toJson/fromJson methods added with argo-cd 2.6, which are an okay-ish workaround (because JSON < YAML) to make it work at least - it's just not beautiful in the UI, because it's JSON ;)

@blakepettersson
Copy link
Member

I saw that @alexbde put in an issue in Masterminds/sprig#358, which would be the ideal place to put it. Maybe as a workaround until then we could add those functions directly in utils.go and once (if) it is upstream we could then use that implementation?

@blakepettersson
Copy link
Member

There's now a PR in Masterminds/sprig#360

@alexbde
Copy link
Contributor Author

alexbde commented Feb 9, 2023

Wow! Thank you very much for taking care of that @blakepettersson ❤️ Fingers crossed the sprig maintainers will pick it up soon :)

@todaywasawesome todaywasawesome added the enhancement New feature or request label Jun 28, 2023
@crenshaw-dev crenshaw-dev added the component:applications-set Bulk application management related label Jun 30, 2023
@crenshaw-dev
Copy link
Collaborator

I don't see any movement from sprig. Let's do this in Argo CD now, and we can switch to the sprig implementation later if they add it.

@yebolenko
Copy link

any update on this?

@speedfl
Copy link
Contributor

speedfl commented Sep 11, 2023

I created the PR. I hope to merge as part of next release (2.9)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:applications-set Bulk application management related enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants