-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[grafana] correct array formatting for grafana.ini #3352
Conversation
Could you link documentation, where array style is used? for org_mapping, I can only find a space separated string:
|
@jkroepke yeah this was documented here: https://grafana.com/docs/grafana/latest/setup-grafana/configure-security/configure-authentication/okta/#org-roles-mapping-example and seems to work that way... the helm chart currently is doing space-separated but inside square brackets, so not even the way you described either |
but with the join method, it will result into:
not
|
yeah, string-wrapping is optional as it's written and afaik only required when spaces are involved (?)... if we want to enforce string-wrapping i can update the PR accordingly, just wanted to only do the minimal needed so it could be made to work via values... in our patched version of the chart we're passing values in like this:
which produces
|
instead |
bd46fff
to
9a808f5
Compare
cool, i like it. |
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.
For conistancy reasons, could you has this to the config section (15 lines above) as well?
Signed-off-by: Nathan <[email protected]>
9a808f5
to
c24e59e
Compare
Signed-off-by: MH <[email protected]>
fixes array format in configmap grafana.ini.
previously produced:
org_mapping = ["a" "b" "c"]
, which grafana did not apply correctlynow produces:
org_mapping = ["a", "b", "c"]
, which grafana applies correctlyof note:
for some reason this only seems to work when applied as TOML style array lists
["a", "b", "c"]
, even though the file type is nominally ini, which would suggest the following would actually be appropriate:🤷