Allon Mureinik has posted comments on this change. Change subject: pki: cleanup the ca interface ......................................................................
Patch Set 1: I would prefer that you didn't submit this (4 inline comments) .................................................... File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/hostinstall/OpenSslCAWrapper.java Line 20: Line 21: public class OpenSslCAWrapper { Line 22: Line 23: public static String SignCertificateRequest(String request, String label) Line 24: throws FileNotFoundException, UnsupportedEncodingException, IOException { please fix indentation. Also, I'd just leave "throws IOException", and javadoc the others (since they extend it anyway). Line 25: Line 26: File pkicertdir = new File(Config.resolveCABasePath(), "certs"); Line 27: File pkireqdir = new File(Config.resolveCABasePath(), "requests"); Line 28: String reqFileName = String.format("%1$sreq.pem", label); Line 25: Line 26: File pkicertdir = new File(Config.resolveCABasePath(), "certs"); Line 27: File pkireqdir = new File(Config.resolveCABasePath(), "requests"); Line 28: String reqFileName = String.format("%1$sreq.pem", label); Line 29: String certFileName = String.format("%1$scert.pem", label); Consider using constants. Line 30: Line 31: OutputStream os = null; Line 32: try { Line 33: os = new FileOutputStream( Line 49: } Line 50: } Line 51: Line 52: if ( Line 53: !new OpenSslCAWrapper().SignCertificateRequest( Calling new inside of a static block is usally indicative of something gone wrong. Why not change SignCertificateRequest to be a static method? Line 54: reqFileName, Line 55: Config.<Integer> GetValue(ConfigValues.VdsCertificateValidityInYears) * 365, Line 56: certFileName Line 57: ) Line 55: Config.<Integer> GetValue(ConfigValues.VdsCertificateValidityInYears) * 365, Line 56: certFileName Line 57: ) Line 58: ) { Line 59: throw new RuntimeException("Certificate enrollment failed"); can't we have a more reasonable exception? Line 60: } Line 61: Line 62: return FileUtil.readAllText(new File(pkicertdir, certFileName).getPath()); Line 63: } -- To view, visit http://gerrit.ovirt.org/9162 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8c9285ed3a0640fea17a4ce629d6deb532430c4 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Douglas Schilling Landgraf <dougsl...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches