jojochuang commented on PR #11544: URL: https://github.com/apache/ozone/pull/11544#issuecomment-6221502626
## Reviewer notes (plain language) ### Simple examples **Good CSR (valid key, profile OK)** - Before: every check passed, but the inspection future never completed with success — it stayed *incomplete*. The CA server only looked for *failure*, so “not failed” → sign anyway. - After: future completes with `null` (= approved) → sign. **Same outcome for operators**, correct API behavior. **Bad CSR (e.g. signature does not match key)** - Before: first check fails → future completes exceptionally → no cert. Later checks still ran (extra logs). - After: fail fast on first error → no cert. **Same denial**, less noise. **Broken / unfinished inspection (e.g. future never completes)** - Before: incomplete future → treated like approval path → **could sign without a finished inspection** (latent bug; mocked in `testRequestCertificateRejectsUnfinishedInspection`). - After: `!isDone()` → reject with “CSR inspection did not complete.” **Fail closed.** Analogy: bouncer must say “OK” or “no”; the clerk used to stamp if they didn’t hear “no.” Now they require an explicit “OK” (completed success) or reject incomplete/failed inspection. ### Bottom line **This is not a security risk on today’s production path** with the synchronous `DefaultApprover`: invalid CSRs already failed exceptionally before signing; valid CSRs were already being signed. The patch fixes **contract correctness**, **fail-fast on inspection**, and **defense in depth** so incomplete inspection is never treated as approval (relevant for follow-up from #11486 and any future async approver). No config or expected behavior change for normal cert enrollment after upgrade. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
