Skip to content
New issue

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

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

Already on GitHub? # to your account

Potential fix for issue https://github.com/Unleash/unleash/issues/9111 #180

Merged
merged 4 commits into from
Jan 24, 2025

Conversation

vikrambindal
Copy link
Contributor

@vikrambindal vikrambindal commented Jan 17, 2025

Fix existingSecrets implementation in unleash-edge chart

Description

This PR fixes the implementation of existingSecrets in the unleash-edge Helm chart which currently causes Kubernetes validation errors when deploying with secret configurations. The issue is documented in unleash#9111.

Changes

  1. Modified deploment.yaml default to use envFrom to load secrets
{{- if Values.existingSecrets }}
envFrom:
  - secretRef:
      name: {{ .Values.existingSecrets }}
{{- end }}
  1. Updated the deployment template to use envFrom instead of trying to merge secrets into env section, which prevents validation errors with environment variables.

Testing

  • Deployed chart with empty secrets configuration (works)
  • Deployed chart with secrets configured:
existingSecrets: custom-user-secret  # references existing secret
  • Verified deployment completes without validation errors
  • Verified environment variables are properly set in the container

Additional Notes

  • This is a breaking change as it modifies how secrets are configured
  • Users will need to update their values.yaml to use the new secret reference format
  • The new implementation is more aligned with Kubernetes best practices for secret management

Related Issues

Copy link
Member

@chriswk chriswk left a comment

Choose a reason for hiding this comment

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

Thank you for this. Can I ask you to please bump the major version in the Chart file as well, so our tests run green and this fix can be available as soon as I merge.

@chriswk chriswk self-assigned this Jan 21, 2025
@vikrambindal
Copy link
Contributor Author

Thank you for this. Can I ask you to please bump the major version in the Chart file as well, so our tests run green and this fix can be available as soon as I merge.

Have updated the major version, but felt it should be more like 2.8.0 ?

@chriswk
Copy link
Member

chriswk commented Jan 22, 2025

Have updated the major version, but felt it should be more like 2.8.0 ?

We're breaking the setting/configuration format, so it is a major version.

Fixing empty, null, quote string check for envFrom
@chriswk chriswk merged commit e5f7920 into Unleash:main Jan 24, 2025
11 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

fix: existingSecrets implementation causes env var validation errors in Deployment
2 participants