Allow basicAuth username/password to be sourced from secrets

[adam-homeboost]

We use terraform to control the configuration of this helm chart and it means that we have to check the raw username and password values into our terraform source code, which is a very bad practice. It would be much safer and cleaner if we had the option of using values from regular kubernetes secrets which can be managed in a much more secure way.

[LukaSvastVolue]

I second this feature request, seems like a trivial thing to do any updates? Basically the implementation can look similar to how this opencost helm-chart handles it: https://github.com/opencost/opencost-helm-chart/blob/main/charts/opencost/values.yaml#L147

So by referencing "secret_name", "username_key", "password_key"

[skl]

Would #47 help?

[LukaSvastVolue]

I dont think so #47 seems to only make secret creation optional, how would we pass the reference to the pre-existing secret than, but maybe I am missing something?

[skl]

Yes, I see. #47 I assumes that you would create the secret named as grafana-agent-credentials in the format defined here, but other parts of the chart template are conditional based on the username and password being non-empty in the values... so this looks like a valid feature request 👍

[adam-homeboost]

#47 does help. Its fine for me if the names are mandated, though some flexibility would be nice. The only quibble I would have is that in most charts I have seen "createX" means "create this resource as part of installing helm and generate the values for me randomly". For charts using "existing" secrets, conf names tend to be more along the lines of "existingSecret", etc...

As @skl says though, other parts of the chart that are conditional based on username/password being non-empty do need to be addressed.

[MounaGC]

Yes , this will be a useful feature. Our use case is -terraform to deploy helm charts in kubernetes and we use CSI driver secrets in kubernetes. With the current approach the only way we can pass the secret is using the data source in terraform.

[LukaSvastVolue]

Any status updates or rough estimate on this @skl @petewall?

[caleb-devops]

In addition to this change, can this chart be updated to support extraObjects? This would enable SealedSecrets or ExternalSecrets to be created with the Helm deployment.

[RobCannon]

As a workaround, I have created a new secret that is in the format of grafana-agent-credentials, which I named k8s-grafana-cloud. Then I override the volume mounts in some of the daemonsets. Here are the relevant changes to values.yaml:

        externalServices:
          prometheus:
            host: {{ .Values.prometheusHost }}
            basicAuth:
              username: "placeholder"
              password: "placeholder"
          loki:
            host: {{ .Values.lokiHost }}
            basicAuth:
              username: "placeholder"
              password: "placeholder"
        opencost:
          opencost:
            exporter:
              defaultClusterId: {{ .Values.accountName }}
            prometheus:
              secret_name: k8s-grafana-cloud
              external:
                url: "{{ .Values.prometheusHost }}/api/prom"
        grafana-agent:
          controller:
            volumes:
              extra:
              - name: grafana-agent-credentials
                secret:
                  secretName: k8s-grafana-cloud
        grafana-agent-logs:
          controller:
            volumes:
              extra:
              - name: grafana-agent-credentials
                secret:
                  secretName: k8s-grafana-cloud

We are using ExternalSecrets and AWS SSM Parameters, so I create the secret with this:

apiVersion: external-secrets.io/v1beta1
kind: ExternalSecret
metadata:
  name: k8s-grafana-cloud
spec:
  refreshInterval: 1h
  secretStoreRef:
    name: parameterstore
    kind: SecretStore
  target:
    name: k8s-grafana-cloud  # This is the name of the k8s secret to create
    creationPolicy: Owner
  data:
  - secretKey: loki_host   # This is the key to produce in the k8s secret
    remoteRef:
      conversionStrategy: Default
      decodingStrategy: None
      key: k8s-grafana-cloud  # This is the AWS secret name
      property: lokiHost  # This is the property of the AWS secret to read
  - secretKey: loki_password   # This is the key to produce in the k8s secret
    remoteRef:
      conversionStrategy: Default
      decodingStrategy: None
      key: k8s-grafana-cloud  # This is the AWS secret name
      property: grafanaToken  # This is the property of the AWS secret to read
  - secretKey: loki_username   # This is the key to produce in the k8s secret
    remoteRef:
      conversionStrategy: Default
      decodingStrategy: None
      key: k8s-grafana-cloud  # This is the AWS secret name
      property: lokiUserName  # This is the property of the AWS secret to read
  - secretKey: prometheus_host   # This is the key to produce in the k8s secret
    remoteRef:
      conversionStrategy: Default
      decodingStrategy: None
      key: k8s-grafana-cloud  # This is the AWS secret name
      property: prometheusHost  # This is the property of the AWS secret to read
  - secretKey: prometheus_password   # This is the key to produce in the k8s secret
    remoteRef:
      conversionStrategy: Default
      decodingStrategy: None
      key: k8s-grafana-cloud  # This is the AWS secret name
      property: grafanaToken  # This is the property of the AWS secret to read
  - secretKey: prometheus_username   # This is the key to produce in the k8s secret
    remoteRef:
      conversionStrategy: Default
      decodingStrategy: None
      key: k8s-grafana-cloud  # This is the AWS secret name
      property: prometheusUserName  # This is the property of the AWS secret to read

An easier way to do this would be welcome.

[petewall]

Hello everybody! I really would love to get a user-friendly and intuitive approach for this in the chart. It really is top of mind.

I am waiting for the Grafana Agent squad to merge in this: grafana/agent#4854

When that makes it to an official release, this is going to be the plan:

In the default case:

• secret created by the chart
• secret detected by the agent and used in the service components (instead of being volume mounted to the agent pods)

The values file adds these options:

externalServices:
  prometheus:
    secret:
      name: grafana-agent-credentials
      create: true
    host: http:
    hostKey: prometheus_host
    basicAuth:
      usernameKey: prometheus_username
      passwordKey: prometheus_password
    tenantId: 123
    tenantIdKey: prometheus_tenantId

And likewise for Loki and Tempo services. That would allow for:

1. Default secret creation from credentials set in the values file
2. Use an existing secret with an arbitrary name
3. Use different secrets for prometheus, loki, and tempo

[zeitlinger]

The grafana agent issue has been fixed and a release has been created.

@petewall can this be implemented now?

[petewall]

Nearly there. I'm polishing up the PR and waiting for a new Helm chart release from the Agent team.

[mischavandenburg]

Hello @petewall , is there any update on this? We would love to start using this chart but we are currently hindered because of the need of checking in credentials to our code.

[petewall]

Yes! This is coming. While implementing the change, I ran into an issue with the way that the Grafana Agent accesses elements in the secrets or configmaps.
This is fixed in River version 0.3.0. The next version of Grafana Agent will have that and I'll be able to complete the work on this change.

Thank you for your patience!

[mischavandenburg]

Sounds good @petewall , thank you for your efforts!

[petewall]

This is now released in https://github.com/grafana/k8s-monitoring-helm/releases/tag/v0.5.0!
Thank you, everyone, for waiting for this release. I'd be thrilled if you tried it out and gave me your impressions.

[danielloader]

Tried it and my first experiences:

• It's quite hard to make this work with external secrets sourced from AWS
• Ended up having to put the username, password and host in the secret and source it wholesale as a syncwave on argocd, before the rest of the objects are applied.
• It all boils down to cross field ownership, if I let the helm chart create the secrets it's hard to let external secrets merge in the fields in a way that argocd will permit since the gitops source of truth starts to break down - it can be loosely mitigated with ignoreDifferences settings but then you're ignoring if the password/username fields even exists at all, leading to potential failures to start the agent if aws secrets doesn't work for whatever reason.

The final solution I went with is to disable secrets creation in the helm chart completely and opt for the three fields being sourced, the only downside I can see from this is I've had to put an inline comment on the values file that the hostname in the chart values isn't used, but is necessary to pass the checks in the helm template.

Other than these niggles caused by ArgoCD and External Secrets operator, it's all working as expected now! I can just source one set of secrets from a single source of truth and spawn multiple clusters inconsequentially.

[caleb-devops]

Thank you @petewall, this is working well!

Can we also get support for extraObjects? This would help to create the secrets using ExternalSecrets or SealedSecrets. I can submit a PR for it if you like.

[mischavandenburg]

Thank you very much @petewall , it works well! I'm deploying this to AKS synching secrets from Azure Key Vaults using akv2k8s.

I wrote a post about how I did it here:

https://mischavandenburg.com/zet/grafana-agent-with-custom-secrets-akv2k8s/

Although this might have been a slight bug during the first iterations of this chart, I did run into the following error in my first attempts. The logs of the Grafana Agent showed:

unsupported protocol scheme \"\""

After a lot of debugging I decided to completely rebuild the deployment from scratch, and then I found out that I had put the externalServices.prometheus and externalServices.secret objects in a different order which maybe interfered with the rest of the values file somehow.

[petewall]

This is complete now, so closing this issue.
if you have problems related to external secrets or other things, feel free to open a new issue and we'll handle it there!

[Routhinator]

This change added a bug where if the tenantId is not needed (and thus not defined) the field is not created in the secret by the chart:

https://github.com/grafana/k8s-monitoring-helm/blob/main/charts/k8s-monitoring/templates/metrics-service-credentials.yaml#L18

This sends the grafana-agent pods in a crashloop:

Error: /etc/agent/config.river:826:73: field "tenantId" does not exist

825 |     url = nonsensitive(remote.kubernetes.secret.logs_service.data["host"]) + "/loki/api/v1/push"
826 |     tenant_id = nonsensitive(remote.kubernetes.secret.logs_service.data["tenantId"])
    |                                                                         ^^^^^^^^^^
827 | 

Error: /etc/agent/config.river:795:94: field "tenantId" does not exist

794 |     url = nonsensitive(remote.kubernetes.secret.metrics_service.data["host"]) + "/api/v1/write"
795 |     headers = { "X-Scope-OrgID" = nonsensitive(remote.kubernetes.secret.metrics_service.data["tenantId"]) }
    |                                                                                              ^^^^^^^^^^
796 | 

I managed to work around this by adding a tenantId: "0" to my values.yaml