Skip to content

Conversation

@ecolternv
Copy link
Contributor

#195 - Don't include imagePullSecret with empty name in service chart

Description

The OSMO service chart https://github.com/NVIDIA/OSMO/tree/main/deployments/charts/service always includes an imagePullSecret for its deployments. Even if those deployments dont need an imagePullSecret and .Values.global.imagePullSecret is empty

      imagePullSecrets:
      - name: {{ .Values.global.imagePullSecret }}

This fixes the above code to not include an imagePullSecret if none is set

      {{- if .Values.global.imagePullSecret }}
      imagePullSecrets:
      - name: {{ .Values.global.imagePullSecret }}
      {{- end }}

For some versions of helm/kubernetes, including an empty imagePullSecret or one with an invalid names simply omits a warning and deployment is still able to succeed

I1230 15:21:35.844213  145324 warnings.go:110] "Warning: spec.template.spec.imagePullSecrets[0].name: invalid empty name \"\""

But for others, it seems to cause the helm deploy to fail altogether

Error: UPGRADE FAILED: failed to create patch: map: map[] does not contain declared merge key: name &&
failed to create patch: map: map[] does not contain declared merge key: name && failed to create patch: map:
map[] does not contain declared merge key: name && failed to create patch: map:
map[] does not contain declared merge key: name && failed to create patch: map:
map[] does not contain declared merge key: name

Issue #195

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@ecolternv ecolternv requested a review from a team December 30, 2025 20:47
@ecolternv ecolternv merged commit a0a3009 into main Jan 5, 2026
11 checks passed
@ecolternv ecolternv deleted the ecolter/image-pull-secret-fix branch January 5, 2026 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants