feat: Add support for existingSecret for htpasswd authentication#180
feat: Add support for existingSecret for htpasswd authentication#180juanpicado merged 2 commits intoverdaccio:masterfrom
Conversation
Add support for referencing an existing Kubernetes secret for htpasswd authentication, avoiding plain text passwords in `values.yaml`. - Add `secrets.existingSecretHtpasswd` to reference an existing secret - Add `secrets.existingSecretHtpasswdKey` to specify the key name (defaults to "htpasswd") - Update templates to support both generated and existing secrets - Add README documentation ```yaml secrets: existingSecretHtpasswd: "my-htpasswd-secret" existingSecretHtpasswdKey: "htpasswd" # Optional ``` Create the secret: ```bash kubectl create secret generic my-htpasswd-secret \ --from-file=htpasswd=/path/to/htpasswd ``` If both `secrets.htpasswd` and `secrets.existingSecretHtpasswd` are set, the existing secret takes precedence.
Andrea-Gallicchio
force-pushed
the
secrets
branch
from
47c516a to
c074381
Compare
|
@juanpicado Would you be the right person to review/merge this? 🙏🏻 |
|
Yep let me give it a read later :) and if I have no questions I will merge thanks |
There was a problem hiding this comment.
Pull request overview
This PR adds support for referencing an existing Kubernetes secret for htpasswd authentication in the Verdaccio Helm chart, providing a more secure alternative to storing plain text credentials in values.yaml.
Key Changes:
- Added
secrets.existingSecretHtpasswdandsecrets.existingSecretHtpasswdKeyconfiguration options to reference external secrets - Updated deployment and statefulset templates to conditionally use either generated or existing secrets
- Modified secret generation logic to prevent conflicts when using existing secrets
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/verdaccio/values.yaml | Adds configuration options for referencing existing htpasswd secrets |
| charts/verdaccio/templates/statefulset.yaml | Updates pod annotations, volume mounts, and secret references to support both generated and existing secrets |
| charts/verdaccio/templates/deployment.yaml | Updates pod annotations, volume mounts, and secret references to support both generated and existing secrets |
| charts/verdaccio/templates/htpasswd-secret.yaml | Modifies conditional to prevent secret generation when using existing secret |
| charts/verdaccio/Chart.yaml | Bumps chart version from 4.28.0 to 4.29.0 for this minor feature addition |
| README.md | Adds comprehensive documentation with usage examples for the new existing secret feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {{- if .Values.secrets.existingSecretHtpasswd }} | ||
| checksum/htpasswd-secret: {{ .Values.secrets.existingSecretHtpasswd }}-{{ .Values.secrets.existingSecretHtpasswdKey | default "htpasswd" }} | ||
| {{- end }} |
There was a problem hiding this comment.
The checksum annotation for existingSecretHtpasswd only concatenates the secret name and key, which won't detect changes to the actual secret content. This means pods won't automatically restart when the external secret is updated. Consider documenting that users need to manually trigger pod restarts when updating the external secret, or remove this checksum annotation entirely since Kubernetes doesn't automatically track changes to external secrets.
| {{- if .Values.secrets.existingSecretHtpasswd }} | |
| checksum/htpasswd-secret: {{ .Values.secrets.existingSecretHtpasswd }}-{{ .Values.secrets.existingSecretHtpasswdKey | default "htpasswd" }} | |
| {{- end }} |
| - mountPath: /verdaccio/storage/htpasswd | ||
| name: htpasswd | ||
| subPath: htpasswd | ||
| subPath: {{ if .Values.secrets.existingSecretHtpasswd }}{{ .Values.secrets.existingSecretHtpasswdKey | default "htpasswd" }}{{ else }}htpasswd{{ end }} |
There was a problem hiding this comment.
The inline conditional for subPath makes the template harder to read. Consider moving this logic to a helper template or using a clearer multi-line if-else structure for better maintainability.
| subPath: {{ if .Values.secrets.existingSecretHtpasswd }}{{ .Values.secrets.existingSecretHtpasswdKey | default "htpasswd" }}{{ else }}htpasswd{{ end }} | |
| {{- if .Values.secrets.existingSecretHtpasswd }} | |
| subPath: {{ .Values.secrets.existingSecretHtpasswdKey | default "htpasswd" }} | |
| {{- else }} | |
| subPath: htpasswd | |
| {{- end }} |
| - name: htpasswd | ||
| secret: | ||
| secretName: {{ include "verdaccio.fullname" . }}-htpasswd | ||
| secretName: {{ if .Values.secrets.existingSecretHtpasswd }}{{ .Values.secrets.existingSecretHtpasswd }}{{ else }}{{ include "verdaccio.fullname" . }}-htpasswd{{ end }} |
There was a problem hiding this comment.
The inline conditional for secretName makes the template harder to read. Consider moving this logic to a helper template or using a clearer multi-line if-else structure for better maintainability.
| secretName: {{ if .Values.secrets.existingSecretHtpasswd }}{{ .Values.secrets.existingSecretHtpasswd }}{{ else }}{{ include "verdaccio.fullname" . }}-htpasswd{{ end }} | |
| {{- if .Values.secrets.existingSecretHtpasswd }} | |
| secretName: {{ .Values.secrets.existingSecretHtpasswd }} | |
| {{- else }} | |
| secretName: {{ include "verdaccio.fullname" . }}-htpasswd | |
| {{- end }} |
| - mountPath: /verdaccio/storage/htpasswd | ||
| name: htpasswd | ||
| subPath: htpasswd | ||
| subPath: {{ if .Values.secrets.existingSecretHtpasswd }}{{ .Values.secrets.existingSecretHtpasswdKey | default "htpasswd" }}{{ else }}htpasswd{{ end }} |
There was a problem hiding this comment.
The inline conditional for subPath makes the template harder to read. Consider moving this logic to a helper template or using a clearer multi-line if-else structure for better maintainability.
| subPath: {{ if .Values.secrets.existingSecretHtpasswd }}{{ .Values.secrets.existingSecretHtpasswdKey | default "htpasswd" }}{{ else }}htpasswd{{ end }} | |
| subPath: {{- if .Values.secrets.existingSecretHtpasswd }} | |
| {{ .Values.secrets.existingSecretHtpasswdKey | default "htpasswd" }} | |
| {{- else }} | |
| htpasswd | |
| {{- end }} |
| - name: htpasswd | ||
| secret: | ||
| secretName: {{ include "verdaccio.fullname" . }}-htpasswd | ||
| secretName: {{ if .Values.secrets.existingSecretHtpasswd }}{{ .Values.secrets.existingSecretHtpasswd }}{{ else }}{{ include "verdaccio.fullname" . }}-htpasswd{{ end }} |
There was a problem hiding this comment.
The inline conditional for secretName makes the template harder to read. Consider moving this logic to a helper template or using a clearer multi-line if-else structure for better maintainability.
| secretName: {{ if .Values.secrets.existingSecretHtpasswd }}{{ .Values.secrets.existingSecretHtpasswd }}{{ else }}{{ include "verdaccio.fullname" . }}-htpasswd{{ end }} | |
| {{- if .Values.secrets.existingSecretHtpasswd }} | |
| secretName: {{ .Values.secrets.existingSecretHtpasswd }} | |
| {{- else }} | |
| secretName: {{ include "verdaccio.fullname" . }}-htpasswd | |
| {{- end }} |
| {{- if .Values.secrets.existingSecretHtpasswd }} | ||
| checksum/htpasswd-secret: {{ .Values.secrets.existingSecretHtpasswd }}-{{ .Values.secrets.existingSecretHtpasswdKey | default "htpasswd" }} | ||
| {{- end }} |
There was a problem hiding this comment.
The checksum annotation for existingSecretHtpasswd only concatenates the secret name and key, which won't detect changes to the actual secret content. This means pods won't automatically restart when the external secret is updated. Consider documenting that users need to manually trigger pod restarts when updating the external secret, or remove this checksum annotation entirely since Kubernetes doesn't automatically track changes to external secrets.
| {{- if .Values.secrets.existingSecretHtpasswd }} | |
| checksum/htpasswd-secret: {{ .Values.secrets.existingSecretHtpasswd }}-{{ .Values.secrets.existingSecretHtpasswdKey | default "htpasswd" }} | |
| {{- end }} |
Address review feedback from Copilot suggestions: - Remove checksum annotation for existingSecretHtpasswd that only tracked secret name/key but not actual content, which was misleading - Refactor inline conditionals for subPath and secretName to multi-line if-else blocks for better template readability - Add documentation note about manual pod restart requirement when updating external secrets
|
@juanpicado I applied Copilot's suggestions. Would you mind taking another look? Thanks! |
|
Please wait for next week (very busy this one) |
Summary
Add support for referencing an existing Kubernetes secret for htpasswd authentication, avoiding plain text passwords in
values.yaml.Changes
secrets.existingSecretHtpasswdto reference an existing secretsecrets.existingSecretHtpasswdKeyto specify the key name (defaults to "htpasswd")Usage
Create the secret:
If both
secrets.htpasswdandsecrets.existingSecretHtpasswdare set, the existing secret takes precedence.