From 1c546b10b88256c96467c541f725fd753397e09d Mon Sep 17 00:00:00 2001 From: gordon chung Date: Mon, 29 Jun 2015 14:29:27 -0400 Subject: [PATCH] set default limit to meter/sample queries this patch makes existing limit options in meter and samples apis mandatory. if not set explicitly, a default limit will be applied. DocImpact ApiImpact Change-Id: Ie354e8e64a6f8677a0be7e75c34e8901cdffbc6a Implements: blueprint mandatory-limit --- ceilometer/api/app.py | 4 ++ ceilometer/api/controllers/v2/meters.py | 3 +- ceilometer/api/controllers/v2/samples.py | 3 +- ceilometer/api/controllers/v2/utils.py | 18 ++++++ .../api/v2/test_list_events_scenarios.py | 14 ----- .../api/v2/test_list_meters_scenarios.py | 62 +++++++++++++++++++ 6 files changed, 86 insertions(+), 18 deletions(-) diff --git a/ceilometer/api/app.py b/ceilometer/api/app.py index 102bcc6f..c657b00b 100644 --- a/ceilometer/api/app.py +++ b/ceilometer/api/app.py @@ -46,6 +46,10 @@ API_OPTS = [ cfg.BoolOpt('pecan_debug', default=False, help='Toggle Pecan Debug Middleware.'), + cfg.IntOpt('default_api_return_limit', + default=1000, + help='Default maximum number of items returned by API request.' + ), ] CONF.register_opts(OPTS) diff --git a/ceilometer/api/controllers/v2/meters.py b/ceilometer/api/controllers/v2/meters.py index 294377ce..f94e72e6 100644 --- a/ceilometer/api/controllers/v2/meters.py +++ b/ceilometer/api/controllers/v2/meters.py @@ -285,8 +285,7 @@ class MeterController(rest.RestController): rbac.enforce('get_samples', pecan.request) q = q or [] - if limit and limit < 0: - raise base.ClientSideError(_("Limit must be positive")) + limit = v2_utils.enforce_limit(limit) kwargs = v2_utils.query_to_kwargs(q, storage.SampleFilter.__init__) kwargs['meter'] = self.meter_name f = storage.SampleFilter(**kwargs) diff --git a/ceilometer/api/controllers/v2/samples.py b/ceilometer/api/controllers/v2/samples.py index eba9b787..198bb74e 100644 --- a/ceilometer/api/controllers/v2/samples.py +++ b/ceilometer/api/controllers/v2/samples.py @@ -122,8 +122,7 @@ class SamplesController(rest.RestController): q = q or [] - if limit and limit < 0: - raise base.ClientSideError(_("Limit must be positive")) + limit = utils.enforce_limit(limit) kwargs = utils.query_to_kwargs(q, storage.SampleFilter.__init__) f = storage.SampleFilter(**kwargs) return map(Sample.from_db_model, diff --git a/ceilometer/api/controllers/v2/utils.py b/ceilometer/api/controllers/v2/utils.py index 2f9cb63b..659a20a0 100644 --- a/ceilometer/api/controllers/v2/utils.py +++ b/ceilometer/api/controllers/v2/utils.py @@ -23,6 +23,8 @@ import datetime import functools import inspect +from oslo_config import cfg +from oslo_log import log from oslo_utils import timeutils import pecan import six @@ -30,8 +32,24 @@ import wsme from ceilometer.api.controllers.v2 import base from ceilometer.api import rbac +from ceilometer.i18n import _, _LI from ceilometer import utils +LOG = log.getLogger(__name__) +cfg.CONF.import_opt('default_api_return_limit', 'ceilometer.api.app', + group='api') + + +def enforce_limit(limit): + """Ensure limit is defined and is valid. if not, set a default.""" + if limit is None: + limit = cfg.CONF.api.default_api_return_limit + LOG.info(_LI('No limit value provided, result set will be' + 'limited to %(limit)d.'), {limit: limit}) + if limit and limit < 0: + raise base.ClientSideError(_("Limit must be positive")) + return limit + def get_auth_project(on_behalf_of=None): # when an alarm is created by an admin on behalf of another tenant diff --git a/ceilometer/tests/api/v2/test_list_events_scenarios.py b/ceilometer/tests/api/v2/test_list_events_scenarios.py index 5aff5db0..274d4eea 100644 --- a/ceilometer/tests/api/v2/test_list_events_scenarios.py +++ b/ceilometer/tests/api/v2/test_list_events_scenarios.py @@ -20,7 +20,6 @@ import datetime import mock from oslo_utils import timeutils import six -import webtest.app from ceilometer.publisher import utils from ceilometer import sample @@ -87,19 +86,6 @@ class TestListEvents(v2.FunctionalTest, data = self.get_json('/meters/instance/') self.assertEqual(2, len(data)) - def test_all_limit(self): - data = self.get_json('/meters/instance?limit=1') - self.assertEqual(1, len(data)) - - def test_all_limit_negative(self): - self.assertRaises(webtest.app.AppError, - self.get_json, - '/meters/instance?limit=-2') - - def test_all_limit_bigger(self): - data = self.get_json('/meters/instance?limit=42') - self.assertEqual(2, len(data)) - def test_empty_project(self): data = self.get_json('/meters/instance', q=[{'field': 'project_id', diff --git a/ceilometer/tests/api/v2/test_list_meters_scenarios.py b/ceilometer/tests/api/v2/test_list_meters_scenarios.py index 56d78fb5..74d773e2 100644 --- a/ceilometer/tests/api/v2/test_list_meters_scenarios.py +++ b/ceilometer/tests/api/v2/test_list_meters_scenarios.py @@ -61,6 +61,68 @@ class TestValidateUserInput(v2.FunctionalTest, 'type': 'integer'}]) +class TestListMetersRestriction(v2.FunctionalTest, + tests_db.MixinTestsWithBackendScenarios): + + def setUp(self): + super(TestListMetersRestriction, self).setUp() + self.CONF.set_override('default_api_return_limit', 10, group='api') + for i in range(20): + s = sample.Sample( + 'volume.size', + 'gauge', + 'GiB', + 5 + i, + 'user-id', + 'project1', + 'resource-id', + timestamp=(datetime.datetime(2012, 9, 25, 10, 30) + + datetime.timedelta(seconds=i)), + resource_metadata={'display_name': 'test-volume', + 'tag': 'self.sample', + }, + source='source1', + ) + msg = utils.meter_message_from_counter( + s, self.CONF.publisher.telemetry_secret, + ) + self.conn.record_metering_data(msg) + + def test_meter_limit(self): + data = self.get_json('/meters/volume.size?limit=1') + self.assertEqual(1, len(data)) + + def test_meter_limit_negative(self): + self.assertRaises(webtest.app.AppError, + self.get_json, + '/meters/volume.size?limit=-2') + + def test_meter_limit_bigger(self): + data = self.get_json('/meters/volume.size?limit=42') + self.assertEqual(20, len(data)) + + def test_meter_default_limit(self): + data = self.get_json('/meters/volume.size') + self.assertEqual(10, len(data)) + + def test_sample_limit(self): + data = self.get_json('/samples?limit=1') + self.assertEqual(1, len(data)) + + def test_sample_limit_negative(self): + self.assertRaises(webtest.app.AppError, + self.get_json, + '/samples?limit=-2') + + def test_sample_limit_bigger(self): + data = self.get_json('/samples?limit=42') + self.assertEqual(20, len(data)) + + def test_sample_default_limit(self): + data = self.get_json('/samples') + self.assertEqual(10, len(data)) + + class TestListMeters(v2.FunctionalTest, tests_db.MixinTestsWithBackendScenarios):