Skip to content

observer: add CCADB CRL prober#8644

Merged
jsha merged 5 commits intomainfrom
crl-fetch-observer
Mar 11, 2026
Merged

observer: add CCADB CRL prober#8644
jsha merged 5 commits intomainfrom
crl-fetch-observer

Conversation

@jsha
Copy link
Copy Markdown
Contributor

@jsha jsha commented Feb 24, 2026

Prior work: letsencrypt/crl-monitor#88

Fixes #8618

Comment thread observer/probers/ccadb/ccadb.go Fixed
@jsha jsha marked this pull request as ready for review February 24, 2026 06:38
@jsha jsha requested a review from a team as a code owner February 24, 2026 06:38
@jsha jsha requested a review from aarongable February 24, 2026 06:38
Copy link
Copy Markdown
Contributor

@aarongable aarongable left a comment

Choose a reason for hiding this comment

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

I have a few minor comments about the code, but nothing big.

My major concern is that this simply doesn't feel like a prober. It's doing so much work, and has so many opportunities for failure, all of which get wrapped up in big accumulated errors. In my mind, the ideal prober is short, simple, only emits metrics (not meaningful logs), and we have alerts for every single metric it exposes. I understand that it is useful for this to live within boulder-observer, so we don't have to deploy and monitor Yet Another Component, but I'm registering my moderate discomfort with the complexity here.

Comment thread cmd/boulder-observer/main.go Outdated
Comment thread observer/probers/ccadb/intermediates.pem Outdated
Comment thread observer/probers/ccadb/ccadb.go
Comment thread observer/probers/ccadb/ccadb.go Outdated
Comment thread observer/probers/ccadb/ccadb.go Outdated
Comment thread observer/probers/ccadb/ccadb.go Outdated
Comment thread observer/probers/ccadb/ccadb.go Outdated
Comment thread observer/probers/ccadb/ccadb.go Outdated
aarongable
aarongable previously approved these changes Feb 27, 2026
Comment thread observer/probers/ccadb/ccadb.go
@beautifulentropy beautifulentropy self-requested a review February 27, 2026 15:33
Copy link
Copy Markdown
Member

@beautifulentropy beautifulentropy left a comment

Choose a reason for hiding this comment

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

Overall, the structure of this change looks good to me. I do have a few comments, mostly concerning some places where we could panic. I also noticed there isn't a corresponding update to cmd/boulder-observer/README.md.

Comment thread observer/probers/ccadb/retryhttp.go Outdated
Comment thread observer/probers/ccadb/ccadb.go
Comment thread observer/probers/ccadb/ccadb.go Outdated
Comment thread observer/probers/ccadb/ccadb.go Outdated
Comment thread observer/probers/ccadb/retryhttp.go
@jsha
Copy link
Copy Markdown
Contributor Author

jsha commented Mar 11, 2026

Oops sorry I had one more change to push, the README updates! Thanks for all the great feedback BTW.

@beautifulentropy beautifulentropy self-requested a review March 11, 2026 21:31
@jsha jsha merged commit 00dd199 into main Mar 11, 2026
29 checks passed
@jsha jsha deleted the crl-fetch-observer branch March 11, 2026 22:26
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.

boulder-observer: add tool to fetch all CRLs from CCADB

4 participants