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

helm: allow configuration of smtp credentials via existing secret #4643

Conversation

hendrikheil
Copy link

@hendrikheil hendrikheil commented Jul 9, 2024

What this PR does

This PR adds three keys to the helm chart values: smtp.existingSecret, smtp.usernameKey and smtp.passwordKey which allows for secure configuration of the smtp integration.

We noticed this, as we wanted to set all integration keys through secrets. We tried to bypass the Helm chart behavior by defining environment variables through env directly. However as we use Argo CD which has to diff the current state with the desired state, it caused issues. Diffing requires environment key names to be unique, in order to properly create a diff, unfortunately. Instead of just not setting any of the environment keys that the chart is currently setting, I opted to just allow setting the smtp credentials through an existing secret, which is how the other integrations handle it too.

I also added the usernameKey to the secret, as we use Postmark for SMTP where the "password" is also the username, so it needs to be in a secret too.

Which issue(s) this PR closes

There is no related existing issue yet.

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • Added the relevant release notes label (see labels prefixed w/ release:). These labels dictate how your PR will
    show up in the autogenerated release notes.

@hendrikheil hendrikheil requested a review from a team July 9, 2024 16:18
@CLAassistant
Copy link

CLAassistant commented Jul 9, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@joeyorlando joeyorlando left a comment

Choose a reason for hiding this comment

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

LGTM 👍 thanks for the contribution @hendrikheil!

Copy link
Contributor

@joeyorlando joeyorlando left a comment

Choose a reason for hiding this comment

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

do you mind adding a few helm unit tests? (examples in here)

More instructions on how to run these tests locally here

@joeyorlando joeyorlando changed the title feat: allow configuration of smtp credentials via existing secret helm: allow configuration of smtp credentials via existing secret Jul 24, 2024
@joeyorlando joeyorlando added the release:patch PR will be added to "Other Changes" section of release notes label Jul 24, 2024
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the pr:stale Added to a PR that has been deemed "stale". Managed by the actions/stale GitHub Action label Aug 24, 2024
Copy link
Contributor

This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot closed this Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:stale Added to a PR that has been deemed "stale". Managed by the actions/stale GitHub Action release:patch PR will be added to "Other Changes" section of release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants