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

Post selectors broken when using goTemplate: true #12524

Closed
3 tasks done
Roshick opened this issue Feb 18, 2023 · 9 comments · Fixed by #13584
Closed
3 tasks done

Post selectors broken when using goTemplate: true #12524

Roshick opened this issue Feb 18, 2023 · 9 comments · Fixed by #13584
Labels
bug Something isn't working component:applications-set Bulk application management related

Comments

@Roshick
Copy link
Contributor

Roshick commented Feb 18, 2023

Checklist:

  • I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
  • I've included steps to reproduce the bug.
  • I've pasted the output of argocd version.

Describe the bug

When goTemplate is enabled, the post selectors do not work anymore.

I assume this problem is caused by the params needing to be prefixed with a dot when goTemplate is active, unfortunately prefixing a selectorKey is impossible as this will fail during creation in the library's validation for the requirements

This issue will be the exact same problem, seeing that goTemplate in this case is also enabled. The assumption that keys with dots are completely unsupported is incorrect, when goTemplate is disabled keys with dots work perfectly fine (just not prefixed).

To Reproduce

Simply create an applicationset that uses both goTemplate: true and post selectors, neither version (with or without prefix) will work

Version

ArgoCD v2.6.2

@Roshick Roshick added the bug Something isn't working label Feb 18, 2023
@crenshaw-dev crenshaw-dev added the component:applications-set Bulk application management related label Apr 2, 2023
@uzemeltetes-doto
Copy link

uzemeltetes-doto commented May 11, 2023

I'm also facing this problem, and I cannot disable Go templating, because I use dynamic list generator (v2.7.0rc2):

apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: infra
  namespace: argocd
spec:
  goTemplate: true
  generators:
    - matrix:
        generators:
          - git:
              repoURL: https://gitlab.com/***/k8s-infra.git
              revision: HEAD
              pathParamPrefix: componentvalues
              files:
              - path: "clusters/*/*/components/values.yaml"
          - list:
              elements: []
              elementsYaml: "{{ .components | toJson }}"
      selector:
        matchExpressions:
        - {key: .componentvalues.cluster.name, operator: Exists}

Someone please look into this bug.

@m13t
Copy link
Contributor

m13t commented May 11, 2023

So this looks to be that the labels are being filtered out if they contain nested properties and only top-level string key value pairs are being kept.

I’m not overly familiar with the codebase but at a quick glance this looks like for the generator spec function that filters these out, you could simply dot append the keys for nested values rather than discard them.

I’m happy to put a PR in for this assuming there aren’t some wider ramifications I’m unaware of?

@uzemeltetes-doto
Copy link

uzemeltetes-doto commented May 12, 2023

This is the error message from ApplicationSet controller when I apply the ApplicationSet manifest above:

time="2023-05-12T11:32:00Z" level=error msg="error generating application from params" error="error parsing label selector: key: Invalid value: \".componentvalues.cluster.name\": name part 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]')"

@m13t
Copy link
Contributor

m13t commented May 12, 2023

I think that's expected as the label isn't expecting a Go template expression there.

@crenshaw-dev
Copy link
Collaborator

I'm not super familiar with how this bug is working under the hood. But it's definitely a bug. Just an oversight from when we added go templates. I'd be happy to review any PR that makes it possible/intuitive to select when using go templates.

@m13t
Copy link
Contributor

m13t commented May 12, 2023

I think it's to do with the structure of the values is different between Go template and old templates.
I can see that Go templates come in as nested maps but without go templates the keys come in to the generators pre-joined with a period. I'm working on a fix to join the keys in the generator spec processor. As these don't need to be dynamic it should be fairly straight forward and result in the same behaviour as non-Go templates.

@uzemeltetes-doto
Copy link

I did some testing, and found that keys that do not contain dots work, but keys with dots don't.

This works:

      selector:
        matchExpressions:
        - {key: topleveltestkey, operator: Exists}

But this doesn't, no applications are generated:

      selector:
        matchExpressions:
        - {key: toplevel.subkey, operator: Exists}

Or we just have to use different syntax for matchExpressions with subkeys? I didn't find any info about it yet.

@m13t
Copy link
Contributor

m13t commented May 12, 2023

@uzemeltetes-doto no it should work with dots but there's a bug. Working on a fix at the moment.

m13t added a commit to m13t/argo-cd that referenced this issue May 25, 2023
Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
crenshaw-dev pushed a commit that referenced this issue May 27, 2023
* fixes #12524

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* refactor keepOnlyStringLabels function into more generic map flattening function

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* updated USERS.md

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

* use flatten library to replace custom flatten function

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

---------

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>
m13t added a commit to m13t/argo-cd that referenced this issue May 30, 2023
…roj#13584)

* fixes argoproj#12524

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* refactor keepOnlyStringLabels function into more generic map flattening function

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* updated USERS.md

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

* use flatten library to replace custom flatten function

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

---------

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>
m13t added a commit to m13t/argo-cd that referenced this issue May 30, 2023
…roj#13584)

* fixes argoproj#12524

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* refactor keepOnlyStringLabels function into more generic map flattening function

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* updated USERS.md

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

* use flatten library to replace custom flatten function

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

---------

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>
m13t added a commit to m13t/argo-cd that referenced this issue May 30, 2023
…roj#13584)

* fixes argoproj#12524

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* refactor keepOnlyStringLabels function into more generic map flattening function

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* updated USERS.md

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

* use flatten library to replace custom flatten function

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

---------

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>
crenshaw-dev added a commit that referenced this issue May 30, 2023
…y-pick #13584) (#13822)

* fix(appset): Post selector with Go templates in ApplicationSet (#13584)

* fixes #12524

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* refactor keepOnlyStringLabels function into more generic map flattening function

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* updated USERS.md

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

* use flatten library to replace custom flatten function

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

---------

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

* fix merge

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

* re-add deleted test

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

---------

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>
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 pushed a commit that referenced this issue Jun 1, 2023
…y-pick #13584) (#13823)

* fix(appset): Post selector with Go templates in ApplicationSet (#13584)

* fixes #12524

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* refactor keepOnlyStringLabels function into more generic map flattening function

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* updated USERS.md

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

* use flatten library to replace custom flatten function

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

---------

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

* fixed tests

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

---------

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>
crenshaw-dev pushed a commit that referenced this issue Jun 1, 2023
…y-pick #13584) (#13824)

* fix(appset): Post selector with Go templates in ApplicationSet (#13584)

* fixes #12524

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* refactor keepOnlyStringLabels function into more generic map flattening function

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* updated USERS.md

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

* use flatten library to replace custom flatten function

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

---------

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

* fixed tests

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* fixed missing import

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

---------

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>
schakrad pushed a commit to schakrad/argo-cd that referenced this issue Jul 24, 2023
…y-pick argoproj#13584) (argoproj#13822)

* fix(appset): Post selector with Go templates in ApplicationSet (argoproj#13584)

* fixes argoproj#12524

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* refactor keepOnlyStringLabels function into more generic map flattening function

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* updated USERS.md

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

* use flatten library to replace custom flatten function

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

---------

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

* fix merge

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

* re-add deleted test

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

---------

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: schakrad <58915923+schakrad@users.noreply.github.com>
yyzxw pushed a commit to yyzxw/argo-cd that referenced this issue Aug 9, 2023
…roj#13584)

* fixes argoproj#12524

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* refactor keepOnlyStringLabels function into more generic map flattening function

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* updated USERS.md

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

* use flatten library to replace custom flatten function

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

---------

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>
@uzemeltetes-doto
Copy link

Unfortunately this still seems to be an issue in v2.8.4:

time="2023-10-12T07:32:24Z" level=error msg="error generating application from params" error="error parsing label selector: key: Invalid value: \".testkey\": name part 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]')" generator="{nil nil nil nil nil nil &MatrixGenerator{Generators:[]ApplicationSetNestedGenerator{ApplicationSetNestedGenerator{List:nil,Clusters:nil,Git:&GitGenerator{RepoURL:https://gitlab.com/doto1/k8s-infra.git,Directories:[]GitDirectoryGeneratorItem{},Files:[]GitFileGeneratorItem{GitFileGeneratorItem{Path:clusters/*/*/components/values.yaml,},},Revision:HEAD,RequeueAfterSeconds:nil,Template:ApplicationSetTemplate{ApplicationSetTemplateMeta:ApplicationSetTemplateMeta{Name:,Namespace:,Labels:map[string]string{},Annotations:map[string]string{},Finalizers:[],},Spec:ApplicationSpec{Source:nil,Destination:ApplicationDestination{Server:,Namespace:,Name:,},Project:,SyncPolicy:nil,IgnoreDifferences:[]ResourceIgnoreDifferences{},Info:[]Info{},RevisionHistoryLimit:nil,Sources:[]ApplicationSource{},},},PathParamPrefix:componentvalues,Values:map[string]string{},},SCMProvider:nil,ClusterDecisionResource:nil,PullRequest:nil,Matrix:nil,Merge:nil,Selector:nil,Plugin:nil,},ApplicationSetNestedGenerator{List:&ListGenerator{Elements:[]JSON{},Template:ApplicationSetTemplate{ApplicationSetTemplateMeta:ApplicationSetTemplateMeta{Name:,Namespace:,Labels:map[string]string{},Annotations:map[string]string{},Finalizers:[],},Spec:ApplicationSpec{Source:nil,Destination:ApplicationDestination{Server:,Namespace:,Name:,},Project:,SyncPolicy:nil,IgnoreDifferences:[]ResourceIgnoreDifferences{},Info:[]Info{},RevisionHistoryLimit:nil,Sources:[]ApplicationSource{},},},ElementsYaml:{{ .components | toJson }},},Clusters:nil,Git:nil,SCMProvider:nil,ClusterDecisionResource:nil,PullRequest:nil,Matrix:nil,Merge:nil,Selector:nil,Plugin:nil,},},Template:ApplicationSetTemplate{ApplicationSetTemplateMeta:ApplicationSetTemplateMeta{Name:,Namespace:,Labels:map[string]string{},Annotations:map[string]string{},Finalizers:[],},Spec:ApplicationSpec{Source:nil,Destination:ApplicationDestination{Server:,Namespace:,Name:,},Project:,SyncPolicy:nil,IgnoreDifferences:[]ResourceIgnoreDifferences{},Info:[]Info{},RevisionHistoryLimit:nil,Sources:[]ApplicationSource{},},},} nil &LabelSelector{MatchLabels:map[string]string{},MatchExpressions:[]LabelSelectorRequirement{LabelSelectorRequirement{Key:.testkey,Operator:Exists,Values:[],},},} nil}"

Must use dot prefix because of generator:

apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: infra
  namespace: argocd
spec:
  goTemplate: true
  generators:
    - matrix:
        generators:
          - git:
              repoURL: https://gitlab.com/***/k8s-infra.git
              revision: HEAD
              pathParamPrefix: componentvalues
              files:
              - path: "clusters/*/*/components/values.yaml"
          - list:
              elements: []
              elementsYaml: "{{ .components | toJson }}"
      selector:
        matchExpressions:
        - {key: .testkey, operator: Exists}

tesla59 pushed a commit to tesla59/argo-cd that referenced this issue Dec 16, 2023
…roj#13584)

* fixes argoproj#12524

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* refactor keepOnlyStringLabels function into more generic map flattening function

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>

* updated USERS.md

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

* use flatten library to replace custom flatten function

Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>

---------

Signed-off-by: Lewis Marsden-Lambert <lewis.lambert@zserve.co.uk>
Signed-off-by: Lewis Marsden-Lambert <lewis.marsden-lambert@smartpension.co.uk>
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.

4 participants