> On Dec. 20, 2012, 6:45 a.m., Hugo Trippaers wrote:
> > Heya,
> > 
> > Good catch this one.
> > 
> > Please do not use the INSERT IGNORE syntax. This is Mysql specific and 
> > would prevent using any other database in the future. Isn't setting a 
> > random password a better solution?
> > 
> > Adding the logging statements is good.
> > 
> > Removing the NOT NULL requirements on the password field would lower (a 
> > very small) security threshold, which is something we shouldn't do.
> > 
> > Cheers,
> > 
> > Hugo

Agree with Hugo here. We should not remove NOT NULL constraints on password 
field in User table. Inserting a random password will make it work since later 
on CloudStartupServlet.enableAdmin will update it to encoded version of 
"password". Ideally, in production system, we should allow user to provide a 
non-default password in setting up management server.


- Min


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


On Nov. 30, 2012, 11:09 a.m., Hiroaki Kawai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8293/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2012, 11:09 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Current com.cloud.server.ConfigurationServerImpl#saveUser tries to insert an 
> admin user entity into database, but the sql leaves password field 
> unspecified. This will always fail because the database schema declares 
> password field NOT NULL. This patch will fix database schema, and add missing 
> logging when this sql execution failed.
> 
> This happens only on current master:HEAD.
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/server/ConfigurationServerImpl.java d2f0bb0 
>   setup/db/create-schema.sql fff084e 
> 
> Diff: https://reviews.apache.org/r/8293/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hiroaki Kawai
> 
>

Reply via email to