Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Content-Type: multipart/alternative;
 boundary="===============1698989698475403694=="
MIME-Version: 1.0
Subject: Re: Review Request 21375: CLOUDSTACK-6654: Configkey parameters are
 not
 validated
From: "ASF Subversion and Git Services" <asfbot@urd.zones.apache.org>
To: "Abhinandan Prateek" <aprateek@apache.org>,
 "Devdeep Singh" <devdeep.singh@citrix.com>
Cc: "Saksham Srivastava" <saksham.srivastava@citrix.com>,
 "ASF Subversion and Git Services" <asfbot@urd.zones.apache.org>,
 "cloudstack" <dev@cloudstack.apache.org>
Date: Thu, 12 Jun 2014 10:26:01 -0000
Message-ID: <20140612102601.22323.20351@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "ASF Subversion and Git Services" <noreply@reviews.apache.org>
References: <20140513130141.7395.55829@reviews.apache.org>
In-Reply-To: <20140513130141.7395.55829@reviews.apache.org>
Reply-To: "ASF Subversion and Git Services" <asfbot@urd.zones.apache.org>

--===============1698989698475403694==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit


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


Commit 9771e79b1bb9662fee4b95dd59432a9d77d42cd9 in cloudstack's branch refs/heads/4.4 from Saksham Srivastava
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=9771e79 ]

CLOUDSTACK-6654: Configkey parameters are not validated

(cherry picked from commit 5bcd017de6f421a6125406120b39fb8602276dc7)


- 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
> 
>


--===============1698989698475403694==--