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

feat: advanced secret and configmap refs in data sources #1055

Merged
merged 3 commits into from May 19, 2023

Conversation

pb82
Copy link
Collaborator

@pb82 pb82 commented May 16, 2023

This removes the .spec.secrets field of data sources and replaces it with a more flexible solution relying on Secret and Config Map Key selectors and path based applying.

Thanks @addreas for the initial implementation, this is mostly based on your PR #999

@pb82 pb82 requested review from NissesSenap, HVBE and weisdd May 16, 2023 13:07
@pb82 pb82 force-pushed the datasource-env-vars-rewrite branch from b494b84 to d752ed9 Compare May 17, 2023 12:24
Copy link
Collaborator

@HVBE HVBE left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pb82 pb82 enabled auto-merge May 19, 2023 08:40
@pb82 pb82 merged commit 9435e83 into master May 19, 2023
9 checks passed
@pb82 pb82 deleted the datasource-env-vars-rewrite branch May 19, 2023 08:45
@sathieu
Copy link

sathieu commented May 23, 2023

It would be great to have the same feature for dashboards.

This would allow to import configmaps matching specific labels, i.e:

apiVersion: v1
kind: ConfigMap
metadata:
  name: foobar
  labels:
    grafana_dashboard: '1'
# ...

like the grafana helm chart, see here.

@NissesSenap
Copy link
Collaborator

NissesSenap commented May 23, 2023

@sathieu we don't plan to support this feature. The CR does things for us and it would add a bunch of extra complexity.
We could on the other hand add s configmap that contains the json, and you would point to that cm from the CR. And we would be willing to accept that PR if you are up for it.

You can find an example of this in #999

This way you will have to write a short script that find the configmaps according to your label and create a CR for them, which should be rather straight forward.
I don't see any issues to even accept that script to this repo.

At the same time it's not that painful to do the same thing and just create a CR according to our standards.

@sathieu
Copy link

sathieu commented May 23, 2023

Thanks for your response @NissesSenap.

I don't think I'll be able to contribute such a big feature, which is probably as complex as this PR #1055.

@NissesSenap
Copy link
Collaborator

@sathieu feel free to create an issue around it and we will see how many other users that wants it.
And we will try to prioritize accordingly.

@rafilkmp3
Copy link

How can I add dashboards with the label grafana_dashboard: "1" using grafana-operator ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants