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] Support Go Templating in spec.generators #11661

Closed
llavaud opened this issue Dec 12, 2022 · 2 comments · Fixed by #12287
Closed

[ApplicationSet] Support Go Templating in spec.generators #11661

llavaud opened this issue Dec 12, 2022 · 2 comments · Fixed by #12287
Labels
bug Something isn't working component:applications-set Bulk application management related

Comments

@llavaud
Copy link

llavaud commented Dec 12, 2022

Summary

Support Go Templating in ApplicationSet spec.generators

Motivation

I would like to be able to do some Go Templating on the parameters generated by another generator like in the following example:

spec:
  goTemplate: true
  generators:
    - matrix:
        generators:
          - git:
              repoURL: https://github.com/argoproj/applicationset.git
              revision: HEAD
              files:
                - path: "examples/git-generator-files-discovery/cluster-config/**/config.json"
          - clusters:
              selector:
                matchLabels:
                  argocd.argoproj.io/secret-type: cluster
                  kubernetes.io/environment: '{{default "foo" .my_label}}'

Proposal

Implemented in the same way as the support of Go Templating in spec.template ?

@llavaud llavaud added the enhancement New feature or request label Dec 12, 2022
@llavaud
Copy link
Author

llavaud commented Feb 3, 2023

after digging a little into the code, it seems that goTemplate must be supported in spec.generators.

with the following spec, I got a parsing error:

spec:
  goTemplate: true
  generators:
    - matrix:
        generators:
          - git:
              repoURL: https://github.com/myrepo.git
              revision: master
              files:
                - path: instances/*/argocd.yaml
          - clusters:
              selector:
                matchLabels:
                  argocd.argoproj.io/secret-type: cluster
                  cloud_provider: '{{default "aws" .cloud_provider}}'

It fails with the following error:

failed to get params for second generator in the matrix generator: child generator returned an error on parameter generation: failed to parse template {"clusters":{"selector":{"matchLabels":{"argocd.argoproj.io/secret-type":"cluster","cloud_provider":"{{default \"aws\" .cloud_provider}}"}},"template":{"metadata":{},"spec":{"source":{"repoURL":""},"destination":{} ,"project":""}}}}: template: :1: unexpected "\\" in operand

It seems to failed here when parsing the manifest.

@crenshaw-dev crenshaw-dev added bug Something isn't working and removed enhancement New feature or request labels Feb 3, 2023
@crenshaw-dev
Copy link
Collaborator

iirc the problem is that the "second generator" templating is using the old fashioned method of "marshal everything to JSON, template over it, then unmarshal back." It's janky and breaks easily. Instead we should use the same field-wise templating that we use in the template field.

crenshaw-dev added a commit to crenshaw-dev/argo-cd that referenced this issue Feb 3, 2023
…11661)

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
@crenshaw-dev crenshaw-dev added the component:applications-set Bulk application management related label Feb 15, 2023
lsoica pushed a commit to lsoica/argo-cd that referenced this issue Feb 17, 2023
…11661)

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
crenshaw-dev added a commit that referenced this issue Mar 8, 2023
…12287)

* fix: use field-wise templating for child matrix generators (#11661)

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* test shouldn't use go template

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* Update applicationset/utils/utils.go

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
gcp-cherry-pick-bot bot pushed a commit that referenced this issue Mar 8, 2023
…12287)

* fix: use field-wise templating for child matrix generators (#11661)

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* test shouldn't use go template

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* Update applicationset/utils/utils.go

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
crenshaw-dev added a commit that referenced this issue Mar 8, 2023
…12287) (#12771)

* fix: use field-wise templating for child matrix generators (#11661)



* test shouldn't use go template



* Update applicationset/utils/utils.go



---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
crenshaw-dev added a commit that referenced this issue Mar 8, 2023
…12287)

* fix: use field-wise templating for child matrix generators (#11661)

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* test shouldn't use go template

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* Update applicationset/utils/utils.go

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
crenshaw-dev added a commit that referenced this issue Mar 22, 2023
…rom a git file (#12428) (#12490)

* fix: use field-wise templating for child matrix generators (#11661)

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* test shouldn't use go template

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* feat: extend List generator with ElementsJsonBase64

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: proper field name and crd update

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: indentation

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: remove b64 encoding. Based on #12287

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: generated with codegen

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: reset some of the generated files

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: elementsyaml to cover both yaml and json

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: regenerate code

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

* Regenerate code

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

* fix: update ApplicationSet docs

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

* fix: elementsyaml to elementsYaml to be more consistent with other fields

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>
Signed-off-by: laurentiusoica <laurentiu@soica.ro>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
crenshaw-dev added a commit that referenced this issue May 1, 2023
* fix: use field-wise templating for child matrix generators (#11661)

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* test shouldn't use go template

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* feat: extend List generator with ElementsJsonBase64

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: proper field name and crd update

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: indentation

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: remove b64 encoding. Based on #12287

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: generated with codegen

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: reset some of the generated files

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: elementsyaml to cover both yaml and json

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: regenerate code

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

* Regenerate code

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

* fix: update ApplicationSet docs

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

* fix: elementsyaml to elementsYaml to be more consistent with other fields

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

* fix: preserve field order

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>
Signed-off-by: laurentiusoica <laurentiu@soica.ro>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
yyzxw pushed a commit to yyzxw/argo-cd that referenced this issue Aug 9, 2023
…11661) (argoproj#12287)

* fix: use field-wise templating for child matrix generators (argoproj#11661)

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* test shouldn't use go template

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* Update applicationset/utils/utils.go

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
yyzxw pushed a commit to yyzxw/argo-cd that referenced this issue Aug 9, 2023
…rom a git file (argoproj#12428) (argoproj#12490)

* fix: use field-wise templating for child matrix generators (argoproj#11661)

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* test shouldn't use go template

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* feat: extend List generator with ElementsJsonBase64

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: proper field name and crd update

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: indentation

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: remove b64 encoding. Based on argoproj#12287

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: generated with codegen

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: reset some of the generated files

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: elementsyaml to cover both yaml and json

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: regenerate code

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

* Regenerate code

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

* fix: update ApplicationSet docs

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

* fix: elementsyaml to elementsYaml to be more consistent with other fields

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>
Signed-off-by: laurentiusoica <laurentiu@soica.ro>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
yyzxw pushed a commit to yyzxw/argo-cd that referenced this issue Aug 9, 2023
* fix: use field-wise templating for child matrix generators (argoproj#11661)

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* test shouldn't use go template

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* feat: extend List generator with ElementsJsonBase64

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: proper field name and crd update

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: indentation

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: remove b64 encoding. Based on argoproj#12287

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: generated with codegen

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: reset some of the generated files

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: elementsyaml to cover both yaml and json

Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>

* fix: regenerate code

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

* Regenerate code

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

* fix: update ApplicationSet docs

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

* fix: elementsyaml to elementsYaml to be more consistent with other fields

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

* fix: preserve field order

Signed-off-by: laurentiusoica <laurentiu@soica.ro>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Laurentiu Soica <laurentiu@soica.ro>
Signed-off-by: laurentiusoica <laurentiu@soica.ro>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component:applications-set Bulk application management related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants