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

Improve helm secrets template #8598

Merged
merged 6 commits into from Mar 21, 2024
Merged

Improve helm secrets template #8598

merged 6 commits into from Mar 21, 2024

Conversation

yashgorana
Copy link
Contributor

@yashgorana yashgorana commented Mar 18, 2024

Description

Part of https://github.com/OpenMined/Heartbeat/issues/1113

  • Helm chart no longer includes dev credentials anymore for deployment safety.
  • global.useDefaultSecrets is changed to global.randomizedSecrets.
  • common.secrets.set is should be a bit more clear when dealing with using generated passwords vs. default static passwords
  • All dev k8s deployments will use packages/grid/helm/values.dev.yaml which includes the dev secrets

Affected Dependencies

List any dependencies that are required for this change.

How has this been tested?

  • Describe the tests that you ran to verify your changes.
  • Provide instructions so we can reproduce.
  • List any relevant details for your test configuration.

Checklist

@yashgorana yashgorana changed the title Yash/helm secrets tpl Improve helm secrets template Mar 18, 2024
rootPassword: example
# custom secret values
secret:
rootPassword: null
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Charts will no longer ship with a dev default value. It must be explicitly provided by the cluster owner if randomizedSecrets=false else helm will error out because of this line

tox.ini Show resolved Hide resolved
@@ -11,6 +11,7 @@ data:
defaultRootPassword: {{ include "common.secrets.set" (dict
"secret" $secretName
"key" "defaultRootPassword"
"default" .Values.node.defaultSecret.defaultRootPassword
"randomDefault" .Values.global.randomizedSecrets
Copy link
Collaborator

Choose a reason for hiding this comment

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

MAYBE: it would be good to setup application level secret randomization
instead of global.randomizedSecrets.

it could be
.Values.global.node.randomizedSecrets

Where for example if users , need to set a custom secret only for syft container, they could do it easily.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This could be done similarly for mongo, seaweedfs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm ... looks like it didn't get pushed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where for example if users , need to set a custom secret only for syft container, they could do it easily.

for that they can use what you had implemented, provide a custom Secret and use it through values.backend.secretKeyName.

Copy link
Collaborator

@rasswanth-s rasswanth-s left a comment

Choose a reason for hiding this comment

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

Well Done @yashgorana

@rasswanth-s rasswanth-s merged commit cea40a0 into dev Mar 21, 2024
34 checks passed
@rasswanth-s rasswanth-s deleted the yash/helm-secrets-tpl branch March 21, 2024 08:25
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.

None yet

2 participants