From 9338e5c046679d665537b4355f0279d14aedd62d Mon Sep 17 00:00:00 2001 From: Dinesh Bhor Date: Tue, 16 Feb 2016 01:36:15 -0800 Subject: [PATCH] Add check to limit maximum value of max_rows Currently 'glance-manage db purge' fails if given a very large number for max_rows. Moved and renamed validate_mysql_int() from glance.common.utils to glance.db.sqlalchemy.api as _validate_db_int() since it is related to database only so that it can be used to validate max_rows for maximum limit. Closes-Bug: #1543937 Change-Id: Id16694807c180632c1785e9b1ebe8d1c79d885ab --- glance/cmd/manage.py | 5 ++++- glance/common/utils.py | 25 ------------------------- glance/db/sqlalchemy/api.py | 24 ++++++++++++++++++++++-- glance/tests/unit/test_glance_manage.py | 16 ++++++++++++++++ 4 files changed, 42 insertions(+), 28 deletions(-) diff --git a/glance/cmd/manage.py b/glance/cmd/manage.py index 671eadb943..8b150a9b7b 100644 --- a/glance/cmd/manage.py +++ b/glance/cmd/manage.py @@ -176,7 +176,10 @@ class DbCommands(object): if max_rows < 1: sys.exit(_("Minimal rows limit is 1.")) ctx = context.get_admin_context(show_deleted=True) - db_api.purge_deleted_rows(ctx, age_in_days, max_rows) + try: + db_api.purge_deleted_rows(ctx, age_in_days, max_rows) + except exception.Invalid as exc: + sys.exit(exc.msg) class DbLegacyCommands(object): diff --git a/glance/common/utils.py b/glance/common/utils.py index 84c50eb2b9..758eca21d9 100644 --- a/glance/common/utils.py +++ b/glance/common/utils.py @@ -548,31 +548,6 @@ def no_4byte_params(f): return wrapper -def validate_mysql_int(*args, **kwargs): - """ - Make sure that all arguments are less than 2 ** 31 - 1. - - This limitation is introduced because mysql stores INT in 4 bytes. - If the validation fails for some argument, exception.Invalid is raised with - appropriate information. - """ - max_int = (2 ** 31) - 1 - for param in args: - if param > max_int: - msg = _("Value %(value)d out of range, " - "must not exceed %(max)d") % {"value": param, - "max": max_int} - raise exception.Invalid(msg) - - for param_str in kwargs: - param = kwargs.get(param_str) - if param and param > max_int: - msg = _("'%(param)s' value out of range, " - "must not exceed %(max)d") % {"param": param_str, - "max": max_int} - raise exception.Invalid(msg) - - def stash_conf_values(): """ Make a copy of some of the current global CONF's settings. diff --git a/glance/db/sqlalchemy/api.py b/glance/db/sqlalchemy/api.py index 5552fa190d..a926f35763 100644 --- a/glance/db/sqlalchemy/api.py +++ b/glance/db/sqlalchemy/api.py @@ -105,6 +105,23 @@ def get_session(autocommit=True, expire_on_commit=False): expire_on_commit=expire_on_commit) +def _validate_db_int(**kwargs): + """Make sure that all arguments are less than or equal to 2 ** 31 - 1. + + This limitation is introduced because databases stores INT in 4 bytes. + If the validation fails for some argument, exception.Invalid is raised with + appropriate information. + """ + max_int = (2 ** 31) - 1 + + for param_key, param_value in kwargs.items(): + if param_value and param_value > max_int: + msg = _("'%(param)s' value out of range, " + "must not exceed %(max)d.") % {"param": param_key, + "max": max_int} + raise exception.Invalid(msg) + + def clear_db_env(): """ Unset global configuration variables for database. @@ -731,8 +748,8 @@ def _validate_image(values, mandatory_status=True): raise exception.Invalid(msg) # validate integer values to eliminate DBError on save - utils.validate_mysql_int(min_disk=values.get('min_disk'), - min_ram=values.get('min_ram')) + _validate_db_int(min_disk=values.get('min_disk'), + min_ram=values.get('min_ram')) return values @@ -1273,6 +1290,9 @@ def purge_deleted_rows(context, age_in_days, max_rows, session=None): Deletes rows of table images, table tasks and all dependent tables according to given age for relevant models. """ + # check max_rows for its maximum limit + _validate_db_int(max_rows=max_rows) + session = session or get_session() metadata = MetaData(get_engine()) deleted_age = timeutils.utcnow() - datetime.timedelta(days=age_in_days) diff --git a/glance/tests/unit/test_glance_manage.py b/glance/tests/unit/test_glance_manage.py index f5e5bfc42d..e7f958c7a2 100644 --- a/glance/tests/unit/test_glance_manage.py +++ b/glance/tests/unit/test_glance_manage.py @@ -56,3 +56,19 @@ class DBCommandsTestCase(test_utils.BaseTestCase): expected = ("Invalid int value for max_rows: " "%(max_rows)s") % {'max_rows': max_rows} self.assertEqual(expected, ex.code) + + @mock.patch.object(db_api, 'purge_deleted_rows') + @mock.patch.object(context, 'get_admin_context') + def test_purge_max_rows(self, mock_context, mock_db_purge): + mock_context.return_value = self.context + value = (2 ** 31) - 1 + self.commands.purge(age_in_days=1, max_rows=value) + mock_db_purge.assert_called_once_with(self.context, 1, value) + + def test_purge_command_exceeded_maximum_rows(self): + # value(2 ** 31) is greater than max_rows(2147483647) by 1. + value = 2 ** 31 + ex = self.assertRaises(SystemExit, self.commands.purge, age_in_days=1, + max_rows=value) + expected = "'max_rows' value out of range, must not exceed 2147483647." + self.assertEqual(expected, ex.code)