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.

Sign up for GitHub

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

Jump to bottom

[grafana]: Fix: Assert "no leaked secrets" must allow secret refer… #2910

Closed
wants to merge 1 commit into from
Closed

[grafana]: Fix: Assert "no leaked secrets" must allow secret refer… #2910

wants to merge 1 commit into from

Conversation

YvesZelros
Copy link
Contributor

@YvesZelros YvesZelros commented Jan 16, 2024 •
edited
Loading

What does this PR do?

Fix regression on Grafana Chart introduce last week by #2867

The assert no leaked secrets must allow secret reference as documented in Grafana =>

https://github.com/grafana/helm-charts/blob/main/charts/grafana/README.md#how-to-securely-reference-secrets-in-grafanaini

  auth.google:
    client_id: $__file{/etc/secrets/auth_google/client_id}
    client_secret: $__file{/etc/secrets/auth_google/client_secret}

@CLAassistant
Copy link

CLAassistant commented Jan 16, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@YvesZelros YvesZelros force-pushed the fix/grafana_assertNoLeakedSecrets branch from 24371b1 to a8d4285 Compare January 16, 2024 13:50
@YvesZelros YvesZelros mentioned this pull request Jan 16, 2024
Merged
@YvesZelros
fix(grafana): Assert "no leaked secrets" must allow secret reference …
11bc120 
…$__file{...}

Signed-off-by: Yves Galante <yves.galante@zelros.com>
@YvesZelros YvesZelros force-pushed the fix/grafana_assertNoLeakedSecrets branch from bb21075 to 11bc120 Compare January 16, 2024 14:12
@z0rc
Copy link
Contributor

z0rc commented Jan 16, 2024

This isn't sufficient. There are three providers: env, file and vault. See https://grafana.com/docs/grafana/latest/setup-grafana/configure-grafana/#variable-expansion for details. I believe chart should allow all of them.

@z0rc
Copy link
Contributor

z0rc commented Jan 16, 2024

Also this PR is probably a dupe of #2904.

@YvesZelros YvesZelros closed this Jan 16, 2024
@YvesZelros YvesZelros deleted the fix/grafana_assertNoLeakedSecrets branch January 16, 2024 14:23
@YvesZelros
Copy link
Contributor Author

@z0rc Thanks, I see it to late .... I close my PR

@YvesZelros
Copy link
Contributor Author

@z0rc Not that PR not support vault

cwrau added a commit to teutonet/teutonet-helm-charts that referenced this pull request Jan 31, 2024
@cwrau
fix(base-cluster/grafana): oidc secret handling
8512269 
Closes #751, see grafana/helm-charts#2867
The update to kube-prometheus-stack is necessary, otherwise
grafana/helm-charts#2910 is not included
@cwrau cwrau mentioned this pull request Jan 31, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maorfr maorfr Awaiting requested review from maorfr maorfr is a code owner

@torstenwalter torstenwalter Awaiting requested review from torstenwalter torstenwalter is a code owner

@Xtigyro Xtigyro Awaiting requested review from Xtigyro Xtigyro is a code owner

@zanhsieh zanhsieh Awaiting requested review from zanhsieh zanhsieh is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@YvesZelros @CLAassistant @z0rc