Secure convert_unit() function

This secures the utils.convert_unit function. Following work has been done:

* Convert all parameters to decimal.Decimal and handle all parameters the
  same way ('offset' and 'value' could not be a fraction until now,
  and there was a TypeError when 'factor' was an int).

* Set a default value to factor argument to stay consistent with
  the offset parameter.

* Pass factor and offset as ints everywhere to keep consistency across
  collectors.

Change-Id: Idd31490ff01d42530cb6b54eef042a0746d12632
This commit is contained in:
Luka Peschke 2018-02-14 11:37:09 +01:00
parent de36c1d019
commit 15d0a7dd05
4 changed files with 87 additions and 21 deletions

View File

@ -261,8 +261,8 @@ class CeilometerCollector(collector.BaseCollector):
conv_data = METRICS_CONF['metrics_units']['image']
image_size_mb = ck_utils.convert_unit(
decimal.Decimal(image_stats.max),
conv_data['image.size'].get('factor', '1'),
conv_data['image.size'].get('offset', '0'),
conv_data['image.size'].get('factor', 1),
conv_data['image.size'].get('offset', 0),
)
except KeyError:
LOG.warning('Error when trying to use yaml metrology conf.')
@ -315,8 +315,8 @@ class CeilometerCollector(collector.BaseCollector):
conv_data = METRICS_CONF['metrics_units']['volume']
volume_stats.max = ck_utils.convert_unit(
decimal.Decimal(volume_stats.max),
conv_data['volume.size'].get('factor', '1'),
conv_data['volume.size'].get('offset', '0'),
conv_data['volume.size'].get('factor', 1),
conv_data['volume.size'].get('offset', 0),
)
volume_data.append(self.t_cloudkitty.format_item(
@ -374,8 +374,8 @@ class CeilometerCollector(collector.BaseCollector):
conv = METRICS_CONF['metrics_units']['network.bw.' + direction]
tap_bw_mb = ck_utils.convert_unit(
decimal.Decimal(tap_stat.max),
conv[resource_type].get('factor', '1'),
conv[resource_type].get('offset', '0'),
conv[resource_type].get('factor', 1),
conv[resource_type].get('offset', 0),
)
except KeyError:
LOG.warning('Error when trying to use yaml metrology conf.')
@ -489,8 +489,8 @@ class CeilometerCollector(collector.BaseCollector):
conv_data = METRICS_CONF['metrics_units']['radosgw.usage']
rgw_size = ck_utils.convert_unit(
decimal.Decimal(rgw_stats.max),
conv_data['radosgw.object.size'].get('factor', '1'),
conv_data['radosgw.object.size'].get('offset', '0'),
conv_data['radosgw.object.size'].get('factor', 1),
conv_data['radosgw.object.size'].get('offset', 0),
)
rgw_data.append(

View File

@ -299,8 +299,8 @@ class GnocchiCollector(collector.BaseCollector):
if isinstance(qty, str):
resource_data[qty] = ck_utils.convert_unit(
resource_data[qty],
conv_data.get('factor', '1'),
conv_data.get('offset', '0'),
conv_data.get('factor', 1),
conv_data.get('offset', 0),
)
# NOTE(mc): deprecated except part kept for backward compatibility.
except KeyError:

View File

@ -16,6 +16,9 @@
# @author: Stéphane Albert
#
import datetime
import decimal
import fractions
import itertools
import unittest
import mock
@ -136,3 +139,59 @@ class UtilsTimeCalculationsTest(unittest.TestCase):
calc_dt = ck_utils.iso2dt(self.date_iso)
check_dt = ck_utils.ts2dt(self.date_ts)
self.assertEqual(calc_dt, check_dt)
class ConvertUnitTest(unittest.TestCase):
"""Class testing the convert_unit and num2decimal function"""
possible_args = [
None, # Use default arg
'2/3',
decimal.Decimal(1.23),
'1.23',
2,
'2',
2.3,
]
def test_arg_types(self):
"""Test function with several arg combinations of different types"""
for fac, off in itertools.product(self.possible_args, repeat=2):
factor = fac if fac else 1
offset = off if off else 0
ck_utils.convert_unit(10, factor, offset)
def test_str_str_str(self):
result = ck_utils.convert_unit('1/2', '1/2', '1/2')
self.assertEqual(result, decimal.Decimal(0.5 * 0.5 + 0.5))
def test_str_float_float(self):
result = ck_utils.convert_unit('1/2', 0.5, 0.5)
self.assertEqual(result, decimal.Decimal(0.5 * 0.5 + 0.5))
def test_convert_str_float(self):
result = ck_utils.num2decimal('2.0')
self.assertEqual(result, decimal.Decimal(2.0))
def test_convert_str_int(self):
result = ck_utils.num2decimal('2')
self.assertEqual(result, decimal.Decimal(2))
def test_convert_str_fraction(self):
result = ck_utils.num2decimal('2/3')
self.assertEqual(result, decimal.Decimal(2.0 / 3))
def test_convert_fraction(self):
result = ck_utils.num2decimal(fractions.Fraction(1, 2))
self.assertEqual(result, decimal.Decimal(1.0 / 2))
def test_convert_float(self):
result = ck_utils.num2decimal(0.5)
self.assertEqual(result, decimal.Decimal(0.5))
def test_convert_int(self):
result = ck_utils.num2decimal(2)
self.assertEqual(result, decimal.Decimal(2))
def test_convert_decimal(self):
result = ck_utils.num2decimal(decimal.Decimal(2))
self.assertEqual(result, decimal.Decimal(2))

View File

@ -300,18 +300,25 @@ def tempdir(**kwargs):
six.text_type(e))
def convert_unit(value, factor, offset=0):
"""Return converted value depending on the provided factor and offset."""
# Check if factor is a fraction
if '/' in factor:
tmp = fractions.Fraction(factor)
numerator = decimal.Decimal(tmp.numerator)
denominator = decimal.Decimal(tmp.denominator)
factor = numerator / denominator
else:
factor = decimal.Decimal(factor)
def num2decimal(num):
"""Converts a number into a decimal.Decimal.
return (decimal.Decimal(value) * factor) + decimal.Decimal(offset)
The number may be an str in float, int or fraction format;
a fraction.Fraction, a decimal.Decimal, an int or a float.
"""
if isinstance(num, decimal.Decimal):
return num
if isinstance(num, str):
if '/' in num:
num = float(fractions.Fraction(num))
if isinstance(num, fractions.Fraction):
num = float(num)
return decimal.Decimal(num)
def convert_unit(value, factor=1, offset=0):
"""Return converted value depending on the provided factor and offset."""
return num2decimal(value) * num2decimal(factor) + num2decimal(offset)
def flat_dict(item, parent=None):