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

Support for PKCS12/PFX certificates #125

Merged

Conversation

EnriqueJoseSanjuaneloRobles-TomTom
Copy link
Contributor

Issue #123

In case a certificate in PKCS12 format is specified (using the same extension as PEM formats), the helper will try to decode its content (given the PEM decoding failed).
A test case is included is also added as well.

@EnriqueJoseSanjuaneloRobles-TomTom EnriqueJoseSanjuaneloRobles-TomTom changed the title Support for pfx certificates Support for PKCS12/PFX certificates Nov 7, 2022
@EnriqueJoseSanjuaneloRobles-TomTom EnriqueJoseSanjuaneloRobles-TomTom changed the title Support for PKCS12/PFX certificates Support for PKCS12/PFX certificates https://github.com/joe-elliott/cert-exporter/pull/125 Nov 7, 2022
@EnriqueJoseSanjuaneloRobles-TomTom EnriqueJoseSanjuaneloRobles-TomTom changed the title Support for PKCS12/PFX certificates https://github.com/joe-elliott/cert-exporter/pull/125 Support for PKCS12/PFX certificates Nov 7, 2022
Copy link
Owner

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

Thanks for this improvement!

Two small comments.

src/exporters/certHelpers.go Show resolved Hide resolved
// Parse as PKCS ?
pfx_blocks, err := decodeFromPKCS(certBytes)
if err != nil {
return metrics, fmt.Errorf("Failed to parse certificate")
Copy link
Owner

Choose a reason for hiding this comment

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

let's include an indication that there was an attempt to parse as both pem and the new format in the error message for clarity.

also let's include the reported error. something like:
fmt.Errorf("failed to parse as pem and pkcs12: %w", err)

Copy link
Owner

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for the feature improvement.

@@ -37,40 +39,84 @@ func secondsToExpiryFromCertAsBase64String(s string) ([]certMetric, error) {

func secondsToExpiryFromCertAsBytes(certBytes []byte) ([]certMetric, error) {
var metrics []certMetric

parsed, metrics, err := parseAsPEM(certBytes)
Copy link
Owner

Choose a reason for hiding this comment

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

i like this refactor, it makes this function more readable

src/exporters/certHelpers.go Outdated Show resolved Hide resolved
src/exporters/certHelpers.go Outdated Show resolved Hide resolved
@joe-elliott joe-elliott merged commit 233f1dd into joe-elliott:master Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants