[Helm] Self-Monitoring fails when loki.auth_enabled is true and/or gateway.basicAuth.enabled is true

[bossm8]

Describe the bug
Logs of the self monitoring deployment cannot be shipped to loki tenancy is activated (default) and/or if the gateway basic authentication is enabled.

Issue:

1. The grafana agent is not configured with basic authentication credentials but only with the tenant_id field which sets the X-Scope-OrgID header source-code, promtail-doc.
2. The default gateway config overrides the X-Scope-OrgID header with the basic auth username which is resulting in an empty header value source-code
3. The grafana agent cannot push logs to loki as now the org id header is set to an empty value

Relevant agent log:

ts=2023-10-12T13:01:44.570096519Z caller=client.go:430 level=error component=logs logs_config=loki/loki component=client host=loki-gateway.loki.svc.cluster.local msg="final error sending batch" status=401 tenant=self-monitoring error="server returned HTTP status 401 Unauthorized (401): no org id"

Gateway logs with the default configuration:

10.42.0.104 - - [12/Oct/2023:12:45:03 +0000]  401 "POST /loki/api/v1/push HTTP/1.1" 10 "-" "GrafanaAgent/" "-"

Gateway logs when the proxy_set_header directive in the gateways nginx.conf is commented:

10.42.0.104 - - [12/Oct/2023:12:41:42 +0000]  204 "POST /loki/api/v1/push HTTP/1.1" 0 "-" "GrafanaAgent/" "-"

To Reproduce
Steps to reproduce the behavior:

1. Use the loki helm chart with the following config (for local deployment with rancher for example):

minio:
  enabled: true

write:
  replicas: 2
  autoscaling:
    enabled: true
    minReplicas: 2
    maxReplicas: 4
    maxReplicas: 6
  affinity: ""

read:
  replicas: 2
  autoscaling:
    enabled: true
    minReplicas: 2
    maxReplicas: 4
    maxReplicas: 6
  affinity: ""

backend:
  replicas: 3
  autoscaling:
    enabled: true
    minReplicas: 3
    maxReplicas: 6
  affinity: ""

kubectl create ns loki && helm install --values loki.yml -n loki loki grafana/loki

2. Check the logs of the agent:

kubectl logs -n loki -l operator.agent.grafana.com/type=logs

Expected behavior
The default configuration works to send logs with the agent to loki (loki.auth_enabled: true). I.e. the agent configuration contains at least basic_auth.username so that nginx set's the correct header. And if basic auth on the gateway is enabled also the password.
Inspect the config:

kubectl get secrets -n loki loki-logs-config -o go-template='{{ index .data "agent.yml" }}' | base64 -d

Generated configuration with the default loki multi tenancy configuration (above):

logs:
    configs:
        - clients:
            - external_labels:
                cluster: loki
              tenant_id: self-monitoring
              url: http://loki-gateway.loki.svc.cluster.local/loki/api/v1/push
          name: loki/loki
          scrape_configs:
            (...)

Expected when gateway basic auth is not enabled:

logs:
    configs:
        - clients:
            - external_labels:
                cluster: loki
              tenant_id: self-monitoring
              url: http://loki-gateway.loki.svc.cluster.local/loki/api/v1/push
              basic_auth:
                username: self-monitoring
                (password: if needed some random value)
          name: loki/loki
          scrape_configs:
            (...)

(This could also be fixed by adjusting the default nginx config mentioned in the comment below)

Expected when gateway basic auth is enabled:

logs:
    configs:
        - clients:
            - external_labels:
                cluster: loki
              tenant_id: self-monitoring
              url: http://loki-gateway.loki.svc.cluster.local/loki/api/v1/push
              basic_auth:
                username: self-monitoring
                password: some-password-set
          name: loki/loki
          scrape_configs:
            (...)

The password could either be set on the selfMonitoring tenant or in the loki tenants, but then it should be documented that the tenant the self monitoring uses must be configured with a password in the loki tenants (relevant source-code.

Environment:

• Infrastructure: Kubernetes on rancher desktop
• Deployment tool: helm

Screenshots, Promtail config, or terminal output
See above

[bossm8]

Note: The current workaround I use to enable multitenancy without basic authentication on the gateway is to conditionally set the orgID header on nginx:

gateway:
  nginxConfig:
    httpSnippet: |
      {{ if .Values.loki.tenants }}
      map $http_x_scope_orgid $new_x_scope_orgid {
        default $http_x_scope_orgid;   # use the existing header value by default
        ""      $remote_user;          # if not set, use the $remote_user variable
      }
      proxy_set_header X-Scope-OrgID $new_x_scope_orgid;
      {{ end }}

[adippl]

Setting X-Scope-OrgID by default seems like a bad idea.

Chart already seems to know how to pass tenant and password to the chart. Implementation seems to be broken and not exposed via values file.

loki/production/helm/loki/templates/loki-canary/daemonset.yaml

Line 78 in 6b46e37

name: {{ include "enterprise-logs.selfMonitoringTenantSecret" $ }}

I manually patched -pass= argument to the pod and got working authentication on daemonset.apps/loki-canary pods.

    spec:
      containers:
      - args:
        - -addr=loki-gateway.loki.svc.k8s3.acmelab.top.:80
        - -labelname=pod
        - -labelvalue=$(POD_NAME)
        - -user=self-monitoring
        - -tenant-id=self-monitoring
        - -pass=<<<PASSWORD>>>

Chart already configures -user= and -tenant-id when authentication is enabled It could also configure -pass= from value

loki/production/helm/loki/templates/loki-canary/daemonset.yaml

Lines 51 to 53 in 6b46e37

	{{- else if $.Values.loki.auth_enabled }}
	- -user={{ $.Values.monitoring.selfMonitoring.tenant.name }}
	- -tenant-id={{ $.Values.monitoring.selfMonitoring.tenant.name }}

[JollyJoker974-2]

I have the same problem.
waiting for the fix proposed by @adippl

[kyrofa]

Yeah same! Loki can't monitor itself with the default config. That gives me pause regarding the production-readiness of this chart, especially given how long this has been an issue.

[LeszekBlazewski]

The potential way to fix this provided by @adippl looks great, @adippl are you interested in contributing or should someone else look into implementing this and opening a PR?