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

Parse raw certificate subjects before debug logging #11

Merged
merged 1 commit into from
Mar 4, 2016

Conversation

riyazdf
Copy link

@riyazdf riyazdf commented Mar 2, 2016

When initializing the certificate pool, it would be helpful to print the parsed pkix.Name to debug logs instead of the raw DER bytes (example of previous output here: notaryproject/notary#611)

Signed-off-by: Riyaz Faizullabhoy riyaz.faizullabhoy@docker.com

@calavera
Copy link
Contributor

calavera commented Mar 3, 2016

why do we even do all work that for a debug log? If we really really want to log those names, can we run that code only if debugging is enabled?

@riyazdf
Copy link
Author

riyazdf commented Mar 4, 2016

Agreed! I concur that we could run this code only if debugging is enabled, though we could also just change the debug log to output the number of trusted certificates (len(certPool.Subjects())) because the current debug statement is hard to read.

I'll work on the former, but let me know if you prefer a different way forward :)

@calavera
Copy link
Contributor

calavera commented Mar 4, 2016

either of those options sounds good to me.

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
@riyazdf
Copy link
Author

riyazdf commented Mar 4, 2016

@calavera I think the number of certs is much cleaner, so I've updated the debug log accordingly

@calavera
Copy link
Contributor

calavera commented Mar 4, 2016

awesome, thanks!

Merging 🎉

calavera added a commit that referenced this pull request Mar 4, 2016
Parse raw certificate subjects before debug logging
@calavera calavera merged commit f549a93 into docker:master Mar 4, 2016
@riyazdf riyazdf deleted the subject-debug-log branch March 4, 2016 22:49
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