ImagePullSecret for loki memcached with error

[V1ct0rHM]

Describe the bug
The bug in this situation arises from a mismatch between the structure of the values provided in the values.yaml file and the way the template is trying to access them.

In the template, the .pullSecrets field is being accessed under $.ctx.Values.image.pullSecrets. However, in the values.yaml file, the imagePullSecrets field is defined directly under the top-level key.

Because of this mismatch, when the template tries to access .pullSecrets, it encounters a nil pointer error because it expects the field to be nested under image and pullSecrets, but it's not structured that way in the values.yaml file.

To fix this bug, the template needs to be adjusted to directly access the imagePullSecrets field from the values.yaml file without assuming any nesting. This ensures that the template matches the structure of the provided values, preventing the nil pointer error.

To Reproduce
Steps to reproduce the behavior:

1. Started Loki helm chart 6.2.0
2. Started Promtail 6.15.5
3. No queries

Expected behavior
The expected behavior is that the Helm chart template correctly retrieves the imagePullSecrets field from the values.yaml file and uses it to populate the imagePullSecrets field in the generated Kubernetes manifests.

Specifically, when the template is rendered, it should iterate over the list of image pull secrets provided in the values.yaml file and include them in the generated StatefulSet manifest under the spec.template.spec.imagePullSecrets field.

The corrected behavior ensures that the Helm chart can be customized with the appropriate image pull secrets, allowing the deployment to access private Docker registries or other protected image sources as needed.

Environment:

• Infrastructure: EKS 1.28
• Deployment tool: Helm and ArgoCD

Screenshots, Promtail config, or terminal output

I have a proposed change here:

loki/production/helm/loki/templates/memcached/_memcached-statefulset.tpl

Line 75 in 397aa56

{{- range $.ctx.Values.image.pullSecrets }}

To => {{- range $.ctx.Values.imagePullSecrets }}

[zesty-henitzhaki]

image:
  pullSecrets:
  - regcred

imagePullSecrets:
- name: regcred

Quick Solution..
Until the repair

[V1ct0rHM]

image:
  pullSecrets:
  - regcred

imagePullSecrets:
- name: regcred

Quick Solution..
Until the repair

I did this for now, thanks lmk if you want me to open a PR

[EvertonSA]

this is a very old discussion, I prefer imagePullSecrets but many grafana charts are using image.PullSecrets.

not a simple change as many people would be affected

[EvertonSA]

see https://github.com/grafana/agent/pull/5978/files and grafana/helm-charts#2832

[fmannaixn]

What is holding up Haibread's PR from being reviewed and merged? Should I update it and bump it to 6.6.7? Is there any other reason it didn't get reviewed by the Loki team?

This isn't a semantic discussion about whether the chart should use imagePullSecrets or image.pullSecrets, it's a bug where the template checks if Values.imagePullSecrets is defined, which is the value the other templates in the chart use, but then iterates over the nil value of Values.image.pullSecrets. I agree there is some inconsistency that could be cleaned up among the LGTM products, but this issue is purely a bug in this one template.