Mailing-List: contact cloudstack-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: cloudstack-dev@incubator.apache.org
Content-Type: multipart/alternative;
 boundary="===============6107438993846575506=="
MIME-Version: 1.0
Subject: Re: Review Request: (CLOUDSTACK-1475) update keystore in SSVM and
 change
 download iso/template url after Update SSL Certificate
From: "Wei Zhou" <w.zhou@leaseweb.com>
To: "Jayapal Reddy" <jayapalreddy.uradi@citrix.com>,
 "Nitin Mehta" <nitin.mehta@citrix.com>
Cc: "cloudstack" <cloudstack-dev@incubator.apache.org>,
 "Wei Zhou" <w.zhou@leaseweb.com>
Date: Mon, 18 Mar 2013 11:05:14 -0000
Message-ID: <20130318110514.6651.41250@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Wei Zhou" <noreply@reviews.apache.org>
References: <20130315055640.1180.75336@reviews.apache.org>
In-Reply-To: <20130315055640.1180.75336@reviews.apache.org>
Reply-To: "Wei Zhou" <w.zhou@leaseweb.com>

--===============6107438993846575506==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable


> On March 15, 2013, 5:56 a.m., Nitin Mehta wrote:
> > server/src/com/cloud/storage/download/DownloadMonitorImpl.java, line 202
> > <https://reviews.apache.org/r/9696/diff/2/?file=3D264874#file264874line=
202>
> >
> >     can you please use SecondaryStorageVmManager instead ?
> =

> Wei Zhou wrote:
>     I can define a new constant in SecondaryStorageVmManager which is sam=
e to ConsoleProxyManager.CERTIFICATE_NAME, but I think it is not necessary.
> =

> Nitin Mehta wrote:
>     I would rather put it in SecondaryStorageVmManager so that other devs=
 are not confused and the design is more modularized and maintainable.

CPVM and SSVM use the same SSL certificate with name =3D ConsoleProxyManage=
r.CERTIFICATE_NAME =3D "CPVMCertificate".
ConsoleProxyManager.CERTIFICATE_NAME is also used in com.cloud.storage.seco=
ndary.SecondaryStorageManagerImpl.generateSetupCommand(Long).
To compatible with lower version of cloudstack, it is difficult to change t=
he certificate name.


> On March 15, 2013, 5:56 a.m., Nitin Mehta wrote:
> > server/src/com/cloud/configuration/Config.java, line 120
> > <https://reviews.apache.org/r/9696/diff/2/?file=3D264869#file264869line=
120>
> >
> >     are there any dependencies on this flag in the code ?
> >     We need to remove this flag during migration as well.
> =

> Wei Zhou wrote:
>     consoleproxy.url.domain is not used in any source codes. We use "comp=
any.com" which is set in SSL certificate update as the domain suffix of con=
sole url.
> =

> Nitin Mehta wrote:
>     But if someone is upgrading to this version s/he will have this entry=
 in the DB and hence in the global setting correct ? Can you please put a d=
elete statement so that people upgrading do not get confused by this entry =
? You can look into the upgrade files for example

Of course. At first I would like to ensure the fixed version (4.0.2/4.1.0/m=
aster), then I willcreate an patch including the removement.


> On March 15, 2013, 5:56 a.m., Nitin Mehta wrote:
> > server/src/com/cloud/storage/upload/UploadMonitorImpl.java, line 225
> > <https://reviews.apache.org/r/9696/diff/2/?file=3D264875#file264875line=
225>
> >
> >     can you put an example here...seems some hardcoding
> =

> Wei Zhou wrote:
>     The list "token" is the result I split the download url of ISO/Templa=
te by "/". For example, url is https://10-11-101-112.realhostip.com/userdat=
a/2fdd9a70-9c4a-4a04-b1d5-1e41c221a1f9.iso. the token[2] is 10-11-101-112.r=
ealhostip.com.
> =

> Nitin Mehta wrote:
>     Wei - Can you please put this as a comment in the code please - this =
would greatly help devs to understand in the future ?

Of course.


- Wei


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


On March 15, 2013, 9:54 a.m., Wei Zhou wrote:
> =

> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9696/
> -----------------------------------------------------------
> =

> (Updated March 15, 2013, 9:54 a.m.)
> =

> =

> Review request for cloudstack, Nitin Mehta and Jayapal Reddy.
> =

> =

> Description
> -------
> =

> This patch is for issue CLOUDSTACK-1475 (RegisterISO error after Update S=
SL Certificate)
> on CloudStack 4.0.1. =

> =

> =

> Changes include:
> (1) update realhostip.keystore in SSVM (see the change in config_ssl.sh)
> (2) change suffix of download iso/template url rom realhostip.com to doma=
in_suffix in SSL Certificate.
> (3) validate download URL because ssvm publicip or domain suffix may chan=
ge.
> =

> =

> This addresses bug CLOUDSTACK-1475.
> =

> =

> Diffs
> -----
> =

>   agent/src/com/cloud/agent/resource/consoleproxy/ConsoleProxyResource.ja=
va 48f5079 =

>   console-proxy/scripts/config_ssl.sh 8d80c47 =

>   core/src/com/cloud/storage/resource/CifsSecondaryStorageResource.java c=
606fca =

>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 15=
5210d =

>   server/src/com/cloud/configuration/Config.java dbcc97a =

>   server/src/com/cloud/consoleproxy/AgentBasedConsoleProxyManager.java 01=
b4720 =

>   server/src/com/cloud/consoleproxy/AgentBasedStandaloneConsoleProxyManag=
er.java 6172780 =

>   server/src/com/cloud/consoleproxy/StaticConsoleProxyManager.java d2df83=
c =

>   server/src/com/cloud/server/ConfigurationServerImpl.java 3368c9b =

>   server/src/com/cloud/storage/download/DownloadMonitorImpl.java 2736777 =

>   server/src/com/cloud/storage/upload/UploadMonitorImpl.java 4231be8 =

> =

> Diff: https://reviews.apache.org/r/9696/diff/
> =

> =

> Testing
> -------
> =

> Testing manually ok.
> =

> =

> To test:
> (1) generate update the SSL certificate and it.  see "17.3.1. Changing th=
e Console Proxy SSL Certificate and Domain" part in CloudPlatform3.0.6Admin=
Guide
> http://support.citrix.com/servlet/KbServlet/download/33425-102-696517/Clo=
udPlatform3.0.6AdminGuide.pdf
> =

> (2) visit instance via console. =

> =

> (3) Download ISO/Template. The browser will show the download url.
> Before patch: the domain suffix of url always be "realhostip.com"
> after patch: the domain suffix of url is "company.com" which you set in s=
tep(1).
> =

> (4) Register ISO/Template using the url in step(3).
> Before patch: When the domain suffix is not "realhostip.com", it fails wi=
th error message "sun.security.validator.ValidatorException: PKIX path buil=
ding failed: sun.security.provider.certpath.SunCertPathBuilderException: un=
able to find valid certification path to requested target".
> after patch: successful.
> =

> (5) Destroy SSVM, and a new one will be created. =

> Before patch: the url in step(3) does not change. the url still be the ip=
 address of old SSVM, and old domain suffix.
> after patch: the url will contain the ip address of new SSVM. If the "com=
pany.com" changes, the url will also contain the new domain suffix.
> =

> (6) If you do not have a DNS server (which can resolve company.com domain=
), please add an entry in /etc/hosts file of the client.
> aaa-bbb-ccc-ddd aaa-bbb-ccc-ddd.company.com        # aaa.bbb.ccc.ddd is t=
he console proxy ip. and ssvm as well.
> =

> =

> We need to restart management-server after Update SSL Certificate.
> =

> =

> Thanks,
> =

> Wei Zhou
> =

>


--===============6107438993846575506==--