-
Notifications
You must be signed in to change notification settings - Fork 9
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
apps: Grafana- add config option for using external dex with user grafana #2406
base: main
Are you sure you want to change the base?
apps: Grafana- add config option for using external dex with user grafana #2406
Conversation
auth_url: https://{{ .Values.grafana.user.oidc.externalDexDomain }}{{ $trailingDot }}/auth | ||
token_url: https://{{ .Values.grafana.user.oidc.externalDexDomain }}{{ $trailingDot }}/token | ||
api_url: https://{{ .Values.grafana.user.oidc.externalDexDomain }}{{ $trailingDot }}/api | ||
{{- end }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should you add these settings to the grafana-ops.yaml.gotmpl aswell?
As otherwise i'm wondering why the keys aren't just added to the grafana.user
section as they do nothing for the ops grafana version currently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs at least schema for the new options.
useInternalDex: true | ||
externalDexDomain: set-me |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can get a starting point for schema by:
yq4 -i '.properties.grafana.properties.ops.properties.oidc.properties.useInternalDex.type="boolean"' config/schemas/config.yaml
yq4 -i '.properties.grafana.properties.ops.properties.oidc.properties.externalDexDomain.type="string"' config/schemas/config.yaml
@@ -112,9 +112,15 @@ grafana.ini: | |||
client_secret: $__env{clientSecret} | |||
use_pkce: true | |||
scopes: {{ .Values.grafana.user.oidc.scopes }} | |||
{{- if .Values.grafana.user.oidc.useInternalDex }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be simpler if this was conditional on externalDexDomain
being set instead of having two settings?
What is the reason for pointing Grafana at a custom dex instead of pointing Dex at another OIDC instance? |
Warning
This is a public repository, ensure not to disclose:
What kind of PR is this?
Required: Mark one of the following that is applicable:
Optional: Mark one or more of the following that are applicable:
Important
Breaking changes should be marked
kind/admin-change
orkind/dev-change
depending on typeCritical security fixes should be marked with
kind/security
What does this PR do / why do we need this PR?
In this PR we add a config option in user grafana to use external dex
Information to reviewers
Checklist