Alon Bar-Lev has posted comments on this change.
Change subject: WIP Support foreman SSL provider
......................................................................
Patch Set 3: (7 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/BaseProviderProxy.java
Line 74: );
Line 75: conn = (HttpsURLConnection) url.openConnection();
Line 76: conn.setSSLSocketFactory(ctx.getSocketFactory());
Line 77: conn.connect();
Line 78: return chain;
Have you checked the issue with missing root at www.google.com or
www.microsoft.com?
Line 79: } catch (SSLHandshakeException e) {
Line 80: return chain;
Line 81: } catch (NoSuchAlgorithmException e) {
Line 82: handleException(VdcBllErrors.PROVIDER_FAILURE, e);
Line 75: conn = (HttpsURLConnection) url.openConnection();
Line 76: conn.setSSLSocketFactory(ctx.getSocketFactory());
Line 77: conn.connect();
Line 78: return chain;
Line 79: } catch (SSLHandshakeException e) {
Why do you return chain in error?
Line 80: return chain;
Line 81: } catch (NoSuchAlgorithmException e) {
Line 82: handleException(VdcBllErrors.PROVIDER_FAILURE, e);
Line 83: } catch (KeyManagementException e) {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/ExternalTrustStoreInitializer.java
Line 10: import org.ovirt.engine.core.utils.EngineLocalConfig;
Line 11:
Line 12: public class ExternalTrustStoreInitializer {
Line 13:
Line 14: private static final String FILE_URL_PREFIX = "file:";
file://
Line 15:
Line 16: public static String getTrustStorePath() {
Line 17: File varDir = EngineLocalConfig.getInstance().getVarDir();
Line 18: return varDir + "/" + "external_truststore";
Line 36: // Passing null stream will create a new empty trust
store
Line 37: trustStore.load(null,
getTrustStorePassword().toCharArray());
Line 38: trustStore.store(out,
getTrustStorePassword().toCharArray());
Line 39: } catch (Exception e) {
Line 40: e.printStackTrace();
print within j2ee?
Line 41: }
Line 42: finally {
Line 43: if (out != null) {
Line 44: try {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/ImportProviderCertificateChainCommand.java
Line 80: ks = KeyStore.getInstance(KeyStore.getDefaultType());
Line 81: ks.load(in, trustStorePassword.toCharArray());
Line 82: out = new FileOutputStream(trustStore);
Line 83: // In case there is only one certificate, we insert it.
Line 84: // Otherwise, we need to insert the entire chain
except the end certificate
I think this should be explicitly specified at UI, not implicit by logic...
Line 85: int numberOfCertificatesToInsert = chain.size() == 1 ?
1 : chain.size() - 1;
Line 86: for (int certIndex = 0; certIndex <
numberOfCertificatesToInsert; ++certIndex) {
Line 87: Certificate certificate = chain.get(certIndex);
Line 88: String alias = Guid.NewGuid().toString();
Line 96:
handleException(VdcBllErrors.PROVIDER_IMPORT_CERTIFICATE_CHAIN_ERROR, e);
Line 97: } catch (IOException e) {
Line 98:
handleException(VdcBllErrors.PROVIDER_IMPORT_CERTIFICATE_CHAIN_ERROR, e);
Line 99: } catch (KeyStoreException e) {
Line 100:
handleException(VdcBllErrors.PROVIDER_IMPORT_CERTIFICATE_CHAIN_ERROR, e);
Again, I see no value in same action for multiple (all) exceptions.
Line 101: }
Line 102: finally {
Line 103: if (in != null) {
Line 104: try {
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/EngineLocalConfig.java
Line 62: public boolean isHttpEnabled() {
Line 63: return getBoolean("ENGINE_HTTP_ENABLED");
Line 64: }
Line 65:
Line 66: public boolean isEnableSniExtension() {
no need for this, right?
Line 67: return getBoolean("ENGINE_ENABLE_SNI_EXTENSION");
Line 68: }
Line 69:
Line 70: public int getHttpPort() {
--
To view, visit http://gerrit.ovirt.org/15128
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I35343409d74a4f90aae726b46781f27ce08a981a
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
