-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14976/#review28176
-----------------------------------------------------------



api/src/com/cloud/event/EventTypes.java
<https://reviews.apache.org/r/14976/#comment54828>

    please add events for certificate upload and delete events as well



api/src/com/cloud/network/lb/CertService.java
<https://reviews.apache.org/r/14976/#comment54829>

    Should the 'validate' method be internal to certificate service?



api/src/com/cloud/network/lb/LoadBalancingRule.java
<https://reviews.apache.org/r/14976/#comment54830>

    can you please add new constructors which takes ssl cert and protocol. If 
we extend same constructor then its resulting unnecessary changes in internal 
lb, elastic lb code as well.



server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java
<https://reviews.apache.org/r/14976/#comment54818>

    need a better error message. its actually mismatch in owners of lb rule, 
certificate and caller



server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java
<https://reviews.apache.org/r/14976/#comment54833>

    remove the comments if not required


- i dont see certificate details in the load balancer response obtained from 
listLoadBalancerRules.Does it make sense to give the certificate details if 
there is a cert assigined to load balancer rule?

- i dont see code to add a network offering with 'SslTermination'capability. 
Also list network offering, should show if LB service with SSL termination is 
supported by the offering. 

- Please add Apache license header to all files.

- Murali Reddy


On Oct. 30, 2013, 8:34 p.m., Syed Ahmed wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14976/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2013, 8:34 p.m.)
> 
> 
> Review request for cloudstack, Darren Shepherd, Murali Reddy, and Sheng Yang.
> 
> 
> Bugs: CLOUDSTACK-4821
>     https://issues.apache.org/jira/browse/CLOUDSTACK-4821
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> This is the second patch for SSL termination support. This patch impletements 
> the assginement of certificate to loadbalancers. Support for netscaler is 
> also added. Due to the version of netscaler API in CS, I could not add 
> support for certificate chain. This should not be a big change however. We 
> can discuss this.
> 
> 
> NOTE: Because I cannot diff with my local branch, this patch also includes 
> the first patch which includes certificate management logic ... sorry 
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/agent/api/to/LoadBalancerTO.java df2f8a8 
>   api/src/com/cloud/event/EventTypes.java a762606 
>   api/src/com/cloud/network/Network.java 49f380b 
>   api/src/com/cloud/network/lb/CertService.java PRE-CREATION 
>   api/src/com/cloud/network/lb/LoadBalancingRule.java 4b37782 
>   api/src/com/cloud/network/lb/LoadBalancingRulesService.java 59d5c8d 
>   api/src/com/cloud/network/lb/SslCert.java PRE-CREATION 
>   api/src/org/apache/cloudstack/api/ApiConstants.java c75e6a0 
>   
> api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignCertToLoadBalancerCmd.java
>  PRE-CREATION 
>   
> api/src/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java
>  a368436 
>   
> api/src/org/apache/cloudstack/api/command/user/loadbalancer/DeleteSslCertCmd.java
>  PRE-CREATION 
>   
> api/src/org/apache/cloudstack/api/command/user/loadbalancer/ListSslCertsCmd.java
>  PRE-CREATION 
>   
> api/src/org/apache/cloudstack/api/command/user/loadbalancer/RemoveCertFromLoadBalancerCmd.java
>  PRE-CREATION 
>   
> api/src/org/apache/cloudstack/api/command/user/loadbalancer/UploadSslCertCmd.java
>  PRE-CREATION 
>   api/src/org/apache/cloudstack/api/response/SslCertResponse.java 
> PRE-CREATION 
>   client/tomcatconf/applicationContext.xml.in 2a3520b 
>   client/tomcatconf/nonossComponentContext.xml.in 9d1da95 
>   core/src/com/cloud/agent/api/routing/LoadBalancerConfigCommand.java 3a51e8a 
>   
> engine/components-api/src/com/cloud/network/lb/LoadBalancingRulesManager.java 
> 3e32585 
>   engine/schema/src/com/cloud/network/dao/LoadBalancerCertMapDao.java 
> PRE-CREATION 
>   engine/schema/src/com/cloud/network/dao/LoadBalancerCertMapDaoImpl.java 
> PRE-CREATION 
>   engine/schema/src/com/cloud/network/dao/LoadBalancerCertMapVO.java 
> PRE-CREATION 
>   engine/schema/src/com/cloud/network/dao/LoadBalancerVO.java fee88cf 
>   engine/schema/src/com/cloud/network/dao/SslCertDao.java PRE-CREATION 
>   engine/schema/src/com/cloud/network/dao/SslCertDaoImpl.java PRE-CREATION 
>   engine/schema/src/com/cloud/network/dao/SslCertVO.java PRE-CREATION 
>   
> plugins/network-elements/elastic-loadbalancer/src/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java
>  ab414de 
>   
> plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerVMManagerImpl.java
>  b6269eb 
>   
> plugins/network-elements/internal-loadbalancer/test/org/apache/cloudstack/internallbelement/InternalLbElementTest.java
>  f170fee 
>   
> plugins/network-elements/internal-loadbalancer/test/org/apache/cloudstack/internallbvmmgr/InternalLBVMManagerTest.java
>  82f90fb 
>   
> plugins/network-elements/netscaler/src/com/cloud/network/element/NetscalerElement.java
>  d63b14f 
>   
> plugins/network-elements/netscaler/src/com/cloud/network/resource/NetscalerResource.java
>  fe072e1 
>   server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java 
> dd48930 
>   server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java c685ee3 
>   server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 
> 3dfcad5 
>   server/src/com/cloud/server/ManagementServerImpl.java 699f469 
>   
> server/src/org/apache/cloudstack/network/lb/ApplicationLoadBalancerManagerImpl.java
>  2385edc 
>   server/src/org/apache/cloudstack/network/lb/CertServiceImpl.java 
> PRE-CREATION 
>   server/test/org/apache/cloudstack/lb/ApplicationLoadBalancerTest.java 
> 9b46e68 
>   
> server/test/org/apache/cloudstack/network/lb/ApplicationLoadBalancerTest.java 
> PRE-CREATION 
>   server/test/org/apache/cloudstack/network/lb/CertServiceTest.java 
> PRE-CREATION 
>   server/test/resources/certs/bad_format_cert.crt PRE-CREATION 
>   server/test/resources/certs/dsa_self_signed.crt PRE-CREATION 
>   server/test/resources/certs/dsa_self_signed.key PRE-CREATION 
>   server/test/resources/certs/expired_cert.crt PRE-CREATION 
>   server/test/resources/certs/non_x509_pem.crt PRE-CREATION 
>   server/test/resources/certs/root_chain.crt PRE-CREATION 
>   server/test/resources/certs/rsa_ca_signed.crt PRE-CREATION 
>   server/test/resources/certs/rsa_ca_signed.key PRE-CREATION 
>   server/test/resources/certs/rsa_ca_signed2.crt PRE-CREATION 
>   server/test/resources/certs/rsa_ca_signed2.key PRE-CREATION 
>   server/test/resources/certs/rsa_random_pkey.key PRE-CREATION 
>   server/test/resources/certs/rsa_self_signed.crt PRE-CREATION 
>   server/test/resources/certs/rsa_self_signed.key PRE-CREATION 
>   server/test/resources/certs/rsa_self_signed_with_pwd.crt PRE-CREATION 
>   server/test/resources/certs/rsa_self_signed_with_pwd.key PRE-CREATION 
>   setup/db/db/schema-421to430.sql aaebf96 
>   utils/src/com/cloud/utils/net/NetUtils.java f590425 
> 
> Diff: https://reviews.apache.org/r/14976/diff/
> 
> 
> Testing
> -------
> 
> Testing was done using a VPX on my setup. 
> 
> 
> Thanks,
> 
> Syed Ahmed
> 
>

Reply via email to