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

Support a list of values as string in the generators selectors #20943

Open
OpenGuidou opened this issue Nov 25, 2024 · 5 comments · May be fixed by #20972
Open

Support a list of values as string in the generators selectors #20943

OpenGuidou opened this issue Nov 25, 2024 · 5 comments · May be fixed by #20972
Labels
component:application-sets Bulk application management related enhancement New feature or request

Comments

@OpenGuidou
Copy link
Contributor

OpenGuidou commented Nov 25, 2024

Summary

With the current state of applicationset templating, it's not possible to provide values as a list in a selector matchExpressions values, coming from variables defined in the generator or from another generator.

Motivation

Let's take the example of this applicationset:

spec:
  generators:
  - list:
      elements:
        - cluster: engineering-dev
          url: https://kubernetes.default.svc
          envs:
            - staging
            - prod
      selector:
        matchExpressions:
          - key: env
            operator: In
            values:
              - {{ .envs }}

It's not possible to provide the envs variable in the selector values.

Proposal

I would propose to add a field in the matchExpression, to support a list of values as string:

spec:
  generators:
  - list:
      elements:
        - cluster: engineering-dev
          url: https://kubernetes.default.svc
          envs:
            - staging
            - prod
      selector:
        matchExpressions:
          - key: env
            operator: In
            valuesString: "staging,prod"

This would be a comma-separated list, as a string, that could be filled using templating functions:

spec:
  goTemplate: true
  generators:
  - list:
      elements:
        - cluster: engineering-dev
          url: https://kubernetes.default.svc
          envs:
            - staging
            - prod
      selector:
        matchExpressions:
          - key: env
            operator: In
            valuesString: `{{- join "," .envs }}`
@OpenGuidou OpenGuidou added the enhancement New feature or request label Nov 25, 2024
@andrii-korotkov-verkada andrii-korotkov-verkada added the component:application-sets Bulk application management related label Nov 25, 2024
@damsien
Copy link

damsien commented Nov 26, 2024

Hello, can I take this issue? 🙂
@OpenGuidou can you confirm that we should have either values or valuesString (both can't be used at the same time).

@OpenGuidou
Copy link
Contributor Author

Sure, go ahead !

It's Indeed a good idea to check only one is provided

@damsien
Copy link

damsien commented Nov 26, 2024

Thanks!
/assign

@damsien
Copy link

damsien commented Nov 26, 2024

I took a look at the issue. I am not sure that it is a good idea to add the valuesString field since it is redundant. Why not using in this way?

spec:
  goTemplate: true
  generators:
  - list:
      elements:
        - cluster: engineering-dev
          url: https://kubernetes.default.svc
          envs:
            - staging
            - prod
      selector:
        matchExpressions:
          - key: env
            operator: In
            values: [`{{- range $index, $env := .envs }}{{ if $index }}, {{ end }}"{{ $env }}"{{- end }}`]

The thing is we want to be as much close to the native Kubernetes API as possible. The struct for the matchExpressions does not include the valuesString field.

@OpenGuidou
Copy link
Contributor Author

You would still end up with a list of a single string.
The idea would be to extend the metav1 LabelSelector, and convert the valuesString to values before applying the selector

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

Successfully merging a pull request may close this issue.

3 participants