Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make password secret optional #45

Merged
merged 5 commits into from
May 14, 2024
Merged

Conversation

danj-replicated
Copy link
Contributor

@danj-replicated danj-replicated commented May 13, 2024

new value: passwordSecretRef takes a secret ref i.e: map[key, name] to tell kots what secret to use instead of the default.

if set, it overrides password secret generation

createPasswordSecret defaults to true as to not break current behaviour
Comment on lines 31 to 36
{{- if .Values.passwordSecretRef }}
{{- with .Values.passwordSecretRef }}
key: {{ .key }}
name: {{ .name }}
{{- end }}
{{- else }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we will also need to somehow tell KOTS what this new secret name is as it updates it at runtime when the password is changed. The kotsadm-password name is currently hard-coded in a few places.

One example where it updates the secret: https://github.com/replicatedhq/kots/blob/v1.109.0/pkg/password/password.go#L80

Comment on lines +28 to +32
{{- if .Values.passwordSecretRef }}
if kubectl get secret kotsadm-password -n {{ .Release.Namespace }} -o jsonpath='{.metadata.labels.app\.kubernetes\.io/managed-by}' | grep -q "Helm"; then
kubectl annotate secret kotsadm-password -n {{ .Release.Namespace }} helm.sh/resource-policy=keep
fi
{{- end }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would this effectively keep the secret around, but it would no longer be used by KOTS? is there a reason why we want to keep it around? kinda thinking out loud here...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't think we're deprecating the secret entirely? just pre-generating the secret if needed from EC

Copy link
Contributor

@cbodonnell cbodonnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - pending the KOTS change to leverage the new env vars

@danj-replicated danj-replicated merged commit 861db4a into main May 14, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants