-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add trusted platform module (TPM) support to TLS package #12801
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
❌ Your patch check has failed because the patch coverage (93.75%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #12801 +/- ##
=======================================
Coverage 91.43% 91.43%
=======================================
Files 487 489 +2
Lines 26811 26855 +44
=======================================
+ Hits 24514 24555 +41
- Misses 1814 1816 +2
- Partials 483 484 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
certificate, err := tls.X509KeyPair(certPem, keyPem) | ||
if err != nil { | ||
return tls.Certificate{}, fmt.Errorf("failed to load TLS cert and key PEMs: %w", err) | ||
if c.TPMConfig.Enabled { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does TPM configuration need to be accounted for in the list of cases at the top of this function that returns an error if multiple certs are configured?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no
I was interested in the statement
It looks like the code just needs to modify the certificate used with TLS. In theory, we could define some kind of TLS-extension API, if we wanted the TPM dependency to be an optional one. 🤷 I'm not sure what process this repo uses to manage SBOM. |
70746d5
to
9917ad1
Compare
@codeboten thanks for the review. I have fixed the build but I am not able to get 95% code coverage without making the code a bit odd. Could you please re-review? |
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
There are conflicts in |
Description
Add trusted platform module (TPM) support to TLS package.
Link to tracking issue
Resolves open-telemetry/opentelemetry-collector-contrib#38682
Replaces open-telemetry/opentelemetry-collector-contrib#39059
TPM cannot be implemented as extension open-telemetry/opentelemetry-collector-contrib#38682 because it overrides the entire
http.transport
and therefore invalidates other extensions/authenticators.Testing
Documentation