Add fake Akamai purge server for integration testing - #3946
Conversation
cpu
left a comment
cpu
left a comment
There was a problem hiding this comment.
Only small nits. The substance of the PR is 💯 - thanks @rolandshoemaker
| var allPurgeURLs []string | ||
| for _, status := range statuses { | ||
| // It's possible that, if our ticks are fast enough (mainly in tests), we | ||
| // will get a status where the ocspLastUpdated == revokedDate and has already |
There was a problem hiding this comment.
nit: I think "we will get a status where the ocspLastUpdated == revokedDate and has already been revoked." is missing a word:
| // will get a status where the ocspLastUpdated == revokedDate and has already | |
| // will get a certificate status where the ocspLastUpdated == revokedDate and the certificate has already |
| "akamaiBaseURL": "http://localhost:6789", | ||
| "akamaiClientToken": "its-a-token", | ||
| "akamaiClientSecret": "its-a-secret", | ||
| "akamaiAccessToken": "idk-how-this-is-different-from-client-token-but-okay", |
| purgeData = response.json() | ||
| if os.environ.get('BOULDER_CONFIG_DIR', '').startswith("test/config-next"): | ||
| if len(purgeData["V3"]) is not 1: | ||
| raise Exception("Unexpected number of Akamai v3 purges") |
There was a problem hiding this comment.
Should there also be a check that when the config is config-next that there are no v2 purges (and vice-versa for config)?
| v2Purges := [][]string{} | ||
| v3Purges := [][]string{} | ||
| mu := sync.Mutex{} | ||
| http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
I think it would be nicer to see this broken up into separate handler functions registered on a mux. Right now its a pretty long function and has to do its own path checking.
| return | ||
| } | ||
| err = akamai.CheckSignature(*secret, "http://"+*listenAddr, r, body) | ||
| if err != nil { |
There was a problem hiding this comment.
nit: this if and err = assignment could be combined.
| return | ||
| } | ||
| err = json.Unmarshal(body, &purgeRequest) | ||
| if err != nil { |
There was a problem hiding this comment.
nit: this if and err = assignment could be combined.
| } | ||
|
|
||
| if updater.ccu != nil { | ||
| if updater.ccu != nil && len(allPurgeURLs) > 0 { |
There was a problem hiding this comment.
Just curious: why was this len check needed? Was it fixing another bug?
There was a problem hiding this comment.
Kind of. We don't do any special empty set logic for findRevokedCertificatesToUpdate so even when it returned an empty list of statuses we would call out to the purger. Because of how we manage batching it wouldn't actually end up sending a request (although it's entirely possible we did in the past, didn't really bother checking that) but we should save a CPU cycle or two by avoiding needing to do that.
There was a problem hiding this comment.
(Probably at some point we should fix the logic in findRevokedCertificatesToUpdate and above, but that gets a bit more involved than I wanted to do here)
| HTTPStatus int | ||
| EstimatedSeconds int | ||
| }{ | ||
| PurgeID: "welcome-to-the-purge", |
cpu
left a comment
cpu
left a comment
There was a problem hiding this comment.
Thanks @rolandshoemaker - looks great!
3 participants
Fixes #3916.