Add java truststore support to additional certificates #1996
Conversation
Much like appending to ca-certificates.crt, this adds support for adding additional certificates to JKS files so that Java can use the additional certificates as well. The behavior is the same in that only existing truststore files will be appended to and the process is otherwise skipped entirely.
markusthoemmes
force-pushed
the
cert-truststores
branch
from
fe8cd8b to
be45c5a
Compare
konradzapalowicz
left a comment
konradzapalowicz
left a comment
There was a problem hiding this comment.
I like the code, it plugs nicely into the existing implementation. I've left feedback related to understanding how the corner cases are expected to be handled where I felt I needed additional clarity. Thanks for coding it ⭐
| } | ||
|
|
||
| // Default password for Java cacerts truststore. | ||
| javaTruststorePassword = []byte("changeit") |
There was a problem hiding this comment.
should it be changed now and if not then could you expand the docs and write when
There was a problem hiding this comment.
This is the default truststore password, it's not a prompt to change it in the future.
| } | ||
|
|
||
| // Write all modified Java truststores back to disk. | ||
| for _, ts := range existingTruststores { |
There was a problem hiding this comment.
it feels that this could be a private func, wouldn't testing benefit from that?
There was a problem hiding this comment.
I don't see testing benefiting from that tbh. I've only externalized loadJavaTruststores for scoping reasons.
2 participants
Much like appending to ca-certificates.crt, this adds support for adding additional certificates to JKS files so that Java can use the additional certificates as well. The behavior is the same in that only existing truststore files will be appended to and the process is otherwise skipped entirely.