[build] cmd/genbotcert: add detail to hostname mismatch error
Dmitri Shuralyov (Gerrit)
New activity on the change![]( "Open in Gerrit")
Related details
- Carlos Amedee
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Carlos Amedee (Gerrit)
Carlos Amedee voted and added 3 comments![]( "Open in Gerrit")
Votes added by Carlos Amedee
| Code-Review | +2 |
3 comments
func generateCert(ctx context.Context, csrPath, hostname string) error {Just out of curiosity, why did you invert these two values? Was it to sort them alphabetically?
if err := cr.CheckSignature(); err != nil {Good explicit check.
Related details
- Dmitri Shuralyov
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dmitri Shuralyov (Gerrit)
Dmitri Shuralyov voted and added 3 comments![]( "Open in Gerrit")
Votes added by Dmitri Shuralyov
| Auto-Submit | +1 |
3 comments
Thanks.
func generateCert(ctx context.Context, csrPath, hostname string) error {Just out of curiosity, why did you invert these two values? Was it to sort them alphabetically?
It wasn't about alphabetical order; my intent was to leave the parameters in what seemed like a descending order, placing the core parameter towards the front, secondary parameter later. That is, I thought for purposes of `readAndCheckCSR` the csrPath is the main input. Hostname is only being passed in to double-check that the CSR was generated for the intended hostname.
I try to refrain from reordering parameters of the same time type, since it's more risky and not always an improvement to readability for other people, but here it seemed worth doing alongside the refactoring for tests.
if err := cr.CheckSignature(); err != nil {Good explicit check.
For posterity, I'll add a note here that I don't expect it to be neccessary because if it fails here, it shouldn't succeed in the future step of creating the certificate remotely.
But it seemed consistent to add a local check here since we're already checking other things that should always match when a sufficiently new version of genbotcert was used to create the CSR.
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gopher Robot (Gerrit)
Gopher Robot submitted the change![]( "Open in Gerrit")
Change information
cmd/genbotcert: add detail to hostname mismatch error
Previously, if the CSR was somehow generated without a .bots.golang.org
suffix, the hostname check would catch that it didn't match but print a
message that's not very clear. Make it more detailed.
Also check some other fields that should match provided that genbotcert,
an appropriately-new version thereof, was used to create the CSR rather
than something else.
- M cmd/genbotcert/genbotcert.go
- A cmd/genbotcert/genbotcert_test.go
Code-Review: +1 by Dmitri Shuralyov, +2 by Carlos Amedee
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |