feat(tls): support inline PEM content for TLS configuration#1381
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the TLS configuration functionality to support inline PEM content in addition to file paths. Users can now provide certificate and key data directly as strings containing PEM-encoded content, rather than being limited to file system paths.
Key Changes
- Added a new
readPEMhelper function that detects whether input is inline PEM content or a file path - Modified
makeCertPoolandloadCertificatefunctions to use the newreadPEMhelper - Removed deprecated
BuildNameToCertificate()call
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func readPEM(input string) ([]byte, error) { | ||
| if strings.Contains(input, "-----BEGIN") { | ||
| return []byte(input), nil | ||
| } | ||
|
|
||
| return os.ReadFile(input) | ||
| } |
There was a problem hiding this comment.
The new readPEM function and the modified behavior of makeCertPool and loadCertificate functions lack documentation. Since these functions now accept either file paths or inline PEM content, this dual behavior should be documented with comments explaining the expected input formats and how the function distinguishes between them.
| preview := input | ||
| if len(preview) > 20 { | ||
| preview = preview[:20] + "..." | ||
| } | ||
| return nil, fmt.Errorf( | ||
| "could not parse any PEM certificates %q: %v", certFile, err) | ||
| "could not parse any PEM certificates from %q: %v", preview, err) |
There was a problem hiding this comment.
The error message may expose sensitive PEM content (private keys or certificates) in logs when the preview is displayed. When inline PEM content fails to parse, the preview could contain parts of sensitive cryptographic material. Consider sanitizing the preview to avoid logging sensitive data, or use a generic message that doesn't include the actual content.
| } | ||
| return nil, fmt.Errorf( | ||
| "could not parse any PEM certificates %q: %v", certFile, err) | ||
| "could not parse any PEM certificates from %q: %v", preview, err) |
There was a problem hiding this comment.
The variable err is nil at this point. When AppendCertsFromPEM returns false, it doesn't set an error - it simply indicates that no certificates were successfully parsed. Including %v with a nil err in the error message will print <nil>, which is misleading and unhelpful to users.
| "could not parse any PEM certificates from %q: %v", preview, err) | |
| "could not parse any PEM certificates from %q", preview) |
No description provided.