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

feature: Enable client certificate rotation by using tls.Config@GetClientCertificate #523

Merged

Conversation

benkelukas
Copy link
Contributor

@benkelukas benkelukas commented May 29, 2024

Enable client certificate rotation by providing a callback to tls.Config@GetClientCertificate when setting up Temporal connection

Reason for This PR

Closes: #522

Description of Changes

Added a callback for GetClientCertificate to enable client certificate rotation when mTLS connection fails, e.g. after it changes in the referenced cert and key files

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.

PR Checklist

[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]

  • All commits in this PR are signed (git commit -s).
  • The reason for this PR is clearly provided (issue no. or explanation).
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this PR.
  • Any user-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.

@CLAassistant
Copy link

CLAassistant commented May 29, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Lukas Benke seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@rustatian rustatian self-requested a review May 29, 2024 08:04
@rustatian rustatian added the C-enhancement Category: enhancement. Meaning improvements of current module, transport, etc.. label May 29, 2024
tls.go Outdated Show resolved Hide resolved
…ientCertificate instead of building and passing the key pair statically
@benkelukas benkelukas force-pushed the use-get-client-certificate-for-tls branch from 1ede8da to fee8e51 Compare May 29, 2024 08:08
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 36.49%. Comparing base (c8e0bcf) to head (4b385c2).
Report is 213 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master     #523       +/-   ##
===========================================
+ Coverage   22.67%   36.49%   +13.81%     
===========================================
  Files           4       13        +9     
  Lines         269      959      +690     
===========================================
+ Hits           61      350      +289     
- Misses        206      539      +333     
- Partials        2       70       +68     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rustatian
Copy link
Collaborator

Thanks, @benkelukas 👍 LGTM, will be released next week with RR v2024.1.3.

@rustatian rustatian enabled auto-merge May 29, 2024 09:04
@rustatian rustatian self-requested a review May 29, 2024 09:04
@rustatian rustatian merged commit e09223c into temporalio:master May 29, 2024
9 checks passed
@benkelukas
Copy link
Contributor Author

Thanks @rustatian - that's great news! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: enhancement. Meaning improvements of current module, transport, etc..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[💡 FEATURE REQUEST]: Enable TLS client certificate rotation
4 participants