cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF Subversion and Git Services" <asf...@urd.zones.apache.org>
Subject Re: Review Request 21375: CLOUDSTACK-6654: Configkey parameters are not validated
Date Thu, 12 Jun 2014 06:06:58 GMT

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


Commit 41030e4e3e39de694837d1a6dc2f4905da228d98 in cloudstack's branch refs/heads/master from
Saksham Srivastava
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=41030e4 ]

CLOUDSTACK-6654: Configkey parameters are not validated


- ASF Subversion and Git Services


On May 13, 2014, 1:01 p.m., Saksham Srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21375/
> -----------------------------------------------------------
> 
> (Updated May 13, 2014, 1:01 p.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek and Devdeep Singh.
> 
> 
> Bugs: CLOUDSTACK-6654
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6654
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> ConfigKey variables values are not validated. So anything  like -5.6 or “abc”  as
the value of cpu/memory/storage overprovision factors can be set. Similarly for all of the
variables in ConfigKey.
> We have a verification mechanism but it is never executed. The code is unreachable in
the preset 4.4
> 
> In ConfigurationManagerImpl.java: validateConfigurationValue()
>  
> Config c = Config.getConfig(name);
>          if (c == null) {
> s_logger.warn("Did not find configuration " + name + " in Config.java. Perhaps moved
to ConfigDepot?");
> -            return null;
> }
> Since for the ConfigKey parameters ‘c’ is always null, we return null and do not
further validate.
> 
> Fix is to make sure type is validated by using  _configDepot.get(name)
> 
> Note: Configkey does not have a range flag. Each range param has to be considered as
per case basis.
> Added comments for the same.
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java 231b5e1 
> 
> Diff: https://reviews.apache.org/r/21375/diff/
> 
> 
> Testing
> -------
> 
> Tested both Configkey variables as well as old Config parameters.
> ConfigKey values are now validated before setting them in db.
> 
> The following status message appears when cpu.overprovisioning.factor is set to incorrect
value.
> There was an error trying to parse the float value for: cpu.overprovisioning.factor
> 
> Build passes.
> Findbug is clean.
> 
> 
> Thanks,
> 
> Saksham Srivastava
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message