Alon Bar-Lev has posted comments on this change.

Change subject: pki: remove the need to store ssh public key
......................................................................


Patch Set 10: (14 inline comments)

....................................................
File backend/manager/conf/ca/installCA.sh
Line 87
Line 88
Line 89
Line 90
Line 91
OK. Will fix this.


....................................................
File backend/manager/modules/root/pom.xml
Line 34:     <dependency>
Line 35:       <groupId>commons-codec</groupId>
Line 36:       <artifactId>commons-codec</artifactId>
Line 37:       <scope>provided</scope>
Line 38:     </dependency>
Done
Line 39:     <dependency>
Line 40:       <groupId>${engine.groupId}</groupId>
Line 41:       <artifactId>common</artifactId>
Line 42:       <version>${engine.version}</version>


....................................................
File 
backend/manager/modules/root/src/main/java/org/ovirt/engine/core/pki/PKIResource.java
Line 24:  * - output-format output format.
Line 25:  *   - X509-PEM
Line 26:  *   - SSH
Line 27:  * - output-alias
Line 28:  */
Changed to something simpler.
Line 29: public class PKIResource extends HttpServlet {
Line 30:     // Serialization id:
Line 31:     private static final long serialVersionUID = 6242595311775511209L;
Line 32: 


Line 25:  *   - X509-PEM
Line 26:  *   - SSH
Line 27:  * - output-alias
Line 28:  */
Line 29: public class PKIResource extends HttpServlet {
Done
Line 30:     // Serialization id:
Line 31:     private static final long serialVersionUID = 6242595311775511209L;
Line 32: 
Line 33:     // The log:


Line 38: 
Line 39:         InputStream in = null;
Line 40: 
Line 41:         String resourceLocation = 
getServletConfig().getInitParameter("resource-location");
Line 42:         String outputFormat = 
getServletConfig().getInitParameter("output-format");
Done
Line 43:         try {
Line 44:             in = new FileInputStream(resourceLocation);
Line 45: 
Line 46:             CertificateFactory cf = 
CertificateFactory.getInstance("X.509");


Line 43:         try {
Line 44:             in = new FileInputStream(resourceLocation);
Line 45: 
Line 46:             CertificateFactory cf = 
CertificateFactory.getInstance("X.509");
Line 47:             Certificate certificate = cf.generateCertificate(in);
Done
Line 48: 
Line 49:             if (outputFormat.equals("X509-PEM")) {
Line 50:                 response.setContentType("application/x-x509-ca-cert");
Line 51:                 // do not let println to use platform specific new line


Line 45: 
Line 46:             CertificateFactory cf = 
CertificateFactory.getInstance("X.509");
Line 47:             Certificate certificate = cf.generateCertificate(in);
Line 48: 
Line 49:             if (outputFormat.equals("X509-PEM")) {
Done
Line 50:                 response.setContentType("application/x-x509-ca-cert");
Line 51:                 // do not let println to use platform specific new line
Line 52:                 response.getWriter().print(
Line 53:                     String.format(


Line 50:                 response.setContentType("application/x-x509-ca-cert");
Line 51:                 // do not let println to use platform specific new line
Line 52:                 response.getWriter().print(
Line 53:                     String.format(
Line 54:                         (
They create proper indentation, distinguish parameters.
Line 55:                             "-----BEGIN CERTIFICATE-----\n" +
Line 56:                             "%1$s" +
Line 57:                             "-----END CERTIFICATE-----\n"
Line 58:                         ),


Line 61:                             new byte[] { (byte)'\n' }
Line 62:                         ).encodeToString(
Line 63:                             certificate.getEncoded()
Line 64:                         )
Line 65:                     )
I disagree, this method provides you to see the output template at one glance, 
as this is not critical path code I suggest readability in favour of 
optimization.
Line 66:                 );
Line 67:             }
Line 68:             else if (outputFormat.equals("SSH")) {
Line 69:                 response.setContentType("text/plain");


Line 64:                         )
Line 65:                     )
Line 66:                 );
Line 67:             }
Line 68:             else if (outputFormat.equals("SSH")) {
Done
Line 69:                 response.setContentType("text/plain");
Line 70:                 // do not let println to use platform specific new line
Line 71:                 response.getWriter().print(
Line 72:                     OpenSSHUtils.getKeyString(


Line 70:                 // do not let println to use platform specific new line
Line 71:                 response.getWriter().print(
Line 72:                     OpenSSHUtils.getKeyString(
Line 73:                         certificate.getPublicKey(),
Line 74:                         
getServletConfig().getInitParameter("output-alias")
Done
Line 75:                     )
Line 76:                 );
Line 77:             }
Line 78:             else {


Line 75:                     )
Line 76:                 );
Line 77:             }
Line 78:             else {
Line 79:                 throw new IllegalArgumentException("Unsupported output 
format " + outputFormat);
But this is the result, see bellow.
Line 80:             }
Line 81:         }
Line 82:         catch(Exception e) {
Line 83:             log.error(


Line 78:             else {
Line 79:                 throw new IllegalArgumentException("Unsupported output 
format " + outputFormat);
Line 80:             }
Line 81:         }
Line 82:         catch(Exception e) {
Short scopes with "e" is similar to use of "i", "j", or any obvious use.
Line 83:             log.error(
Line 84:                 String.format(
Line 85:                     "Cannot send public key resource '%1$s' format 
'%2$s'",
Line 86:                     resourceLocation,


....................................................
File packaging/fedora/setup/engine-setup.py
Line 858
Line 859
Line 860
Line 861
Line 862
Done


--
To view, visit http://gerrit.ovirt.org/4853
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I328baded92b2e7c5169bc87e7c19680f598389b9
Gerrit-PatchSet: 10
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Itamar Heim <ih...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Laszlo Hornyak <lhorn...@redhat.com>
Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to