Alon Bar-Lev has posted comments on this change.

Change subject: utils: pki-resource: cleanup: levarage enum methods for content 
type
......................................................................


Patch Set 5:

(4 comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetCACertificateQuery.java
Line 12:     protected void executeQueryCommand() {
Line 13:         try {
Line 14:             getQueryReturnValue().setSucceeded(false);
Line 15:             getQueryReturnValue().setReturnValue(
Line 16:                 PKIResources.X509_PEM.getAsString()
no... these are two separate formats, not resources... I can get the engine CA 
as X509_PEM or X509_PEM_CA content type.
Line 17:             );
Line 18:             getQueryReturnValue().setSucceeded(true);
Line 19:         } catch (Exception e) {
Line 20:             getQueryReturnValue().setExceptionString(e.getMessage());


....................................................
File backend/manager/modules/root/src/main/webapp/WEB-INF/web.xml
Line 19:         <param-value>/pki-resource</param-value>
Line 20:     </init-param>
Line 21:     <init-param>
Line 22:         <param-name>attr-resource</param-name>
Line 23:         <param-value>X509_PEM_CA</param-value>
because the interface are the hard coded string... not the enum.
Line 24:     </init-param>
Line 25:   </servlet>
Line 26:   <servlet-mapping>
Line 27:     <servlet-name>PKIResourceServlet.ca</servlet-name>


....................................................
File 
backend/manager/modules/services/src/main/java/org/ovirt/engine/core/services/PKIResourceServlet.java
Line 42:             }
Line 43:             PKIResources r = PKIResources.valueOf(resourceString);
Line 44:             Format format = r.getFormat();
Line 45:             if (formatString != null) {
Line 46:                 format = Format.valueOf(formatString);
hmmm? how... the ca-certificate will remain... so that the X509-PEM. so 
interface stays intact.
Line 47:             }
Line 48: 
Line 49:             if (alias == null) {
Line 50:                 alias = r.getAlias();


....................................................
File 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/PKIResources.java
Line 25:  * </pre>
Line 26:  */
Line 27: public enum PKIResources {
Line 28: 
Line 29:     X509_PEM_CA("application/x-x509-cert", null, 
getCertificate(EngineLocalConfig.getInstance().getPKICACert()), Format.PEM),
no... I can get the ca certificate in any format... I can get the engine 
certificate in openssh or pem... not sure the naming conventions you outlined 
is correct...

in this approach enum should be:

 CA_IN_PEM_CA_CONTENT,
 CA_IN_PEM_CONTENT,
 ENGINE_IN_PEM,
 ENGINE_IN_OPENSSH,

and if I add DER format, I need to multiple all entries.
Line 30:     X509_PEM("application/x-x509-ca-cert", null, 
getCertificate(EngineLocalConfig.getInstance().getPKIEngineCert()), Format.PEM),
Line 31:     OPENSSH_PUBKEY("text/plain", "ovirt-engine", 
getCertificate(EngineLocalConfig.getInstance().getPKIEngineCert()), 
Format.OPENSSH);
Line 32: 
Line 33:     private final String contentType;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I24da0ff174599ffdeabbf5846eab429bf0d6510d
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to