-
Notifications
You must be signed in to change notification settings - Fork 167
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
fix: loki auth for multi-tenancy #1662
Conversation
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.
Currently apl is leveraging loki-distributed chart (however in apl the chart is called loki)
The original repo (https://github.com/grafana/helm-charts/tree/loki-distributed-0.69.16/charts/loki) has been deprecated and new on is at
https://github.com/grafana/loki/blob/main/production/helm/loki/values.yaml
There, they are calling the chart just "loki" :)
In a new repo we can find a detailed instruction
######################################################################################################################
#
# Loki Configuration
#
# There are several ways to pass configuration to Loki, listing them here in order of our preference for how
# you should use this chart.
# 1. Use the templated value of loki.config below and the corresponding override sections which follow.
# This allows us to set a lot of important Loki configurations and defaults and also allows us to maintain them
# over time as Loki changes and evolves.
# 2. Use the loki.structuredConfig section.
# This will completely override the templated value of loki.config, so you MUST provide the entire Loki config
# including any configuration that we set in loki.config unless you explicitly are trying to change one of those
# values and are not able to do so with the templated sections.
# If you choose this approach the burden is on you to maintain any changes we make to the templated config.
# 3. Use an existing secret or configmap to provide the configuration.
# This option is mostly provided for folks who have external processes which provide or modify the configuration.
# When using this option you can specify a different name for loki.generatedConfigObjectName and configObjectName
# if you have a process which takes the generated config and modifies it, or you can stop the chart from generating
# a config entirely by setting loki.generatedConfigObjectName to
#
######################################################################################################################
I understand that community decided to cover changes in the Loki config file. It makes sense but it does not explain inability to overwrite the individual parameters. Thefore, I create a GitHub issue at grafana/loki#13723
I also verified the templated changes with the following script:
export ENV_DIR=$PWD/tests/fixtures
gco main
otomi template loki-main -l name=loki
gco loki-sr-fix-loki-auth-1
otomi template loki-sr-fix-loki-auth-1 -l name=loki
# compare
dyff between helmfile.d/loki-main/helmfile-10.monitoring-65f7013d-loki/loki-distributed/templates/configmap.yaml helmfile.d/loki-sr-fix-loki-auth-1/helmfile-10.monitoring-65f7013d-loki/loki-distributed/templates/configmap.yaml --omit-header --ignore-order-changes
data.config.yaml
± value change in multiline text (two inserts, three deletions)
- auth_enabled: false
+ auth_enabled: true
- shared_store: filesystem
+ shared_store: s3
- ruler:
- alertmanager_url: https://alertmanager.xx
- external_url: https://alertmanager.xx
- ring:
- kvstore:
- store: memberlist
- rule_path: /tmp/loki/scratch
- storage:
- local:
- directory: /etc/loki/rules
- type: local
Action points:
- From the above dyff comparison I see that the
ruler
section has been removed. Was it intentional ? - We should not make any change in the chart itself. I propose to follow community advise and generate the whole Loki config in out loki.gotmpl file. I can help with that. Just let me know.
- If the Loki issue is accepted then we can contribute to Loki helm chart. Once it is merged the we will be able to simplify our loki.gotmpl file
Using the |
This PR fixes the broken Loki multi-tenancy. The problem is the setup of the Loki Distributed Helm Chart. Most of the Loki configuration options are combined in just one value
{{ .Values.loki.config }}
. Theauth-enabled
in one of them. In theloki.config
other Chart values can be used. But not all Loki configuration options are supported in the values.yaml. Using values within values in the values.yaml makes it very difficult to modify values using a .gotmpl.If any of you has an idea to go around this issue (other than trying to change this in the upstream Grafana helm-charts project), please let me know.
Another option would be (as previously done) provide the full configuration values in the loki.gotmpl. But if we want users to modify config, this would not be the preferred way.