From c59f1aff8d893b2151b565c4c63b830ea817eac3 Mon Sep 17 00:00:00 2001 From: jiaopengju Date: Sun, 29 Oct 2017 17:34:42 +0800 Subject: [PATCH] Add time format check in API Change-Id: Idca9266d503caa8096ca8d4265e813b4244be653 Closes-Bug: #1727672 --- karbor/api/v1/triggers.py | 14 +++++- karbor/common/config.py | 8 ++++ karbor/common/opts.py | 1 + .../triggers/timetrigger/time_trigger.py | 4 -- .../timetrigger/timeformats/calendar_time.py | 35 ++------------ .../timetrigger/timeformats/crontab_time.py | 13 +----- .../fullstack/test_scheduled_operations.py | 5 +- .../unit/api/v1/test_scheduled_operation.py | 2 + karbor/tests/unit/api/v1/test_triggers.py | 3 +- karbor/utils.py | 46 +++++++++++++++++++ 10 files changed, 81 insertions(+), 50 deletions(-) diff --git a/karbor/api/v1/triggers.py b/karbor/api/v1/triggers.py index cb11437c..7d21cde0 100644 --- a/karbor/api/v1/triggers.py +++ b/karbor/api/v1/triggers.py @@ -13,6 +13,8 @@ """The triggers api.""" from datetime import datetime + +from oslo_config import cfg from oslo_log import log as logging from oslo_utils import uuidutils from webob import exc @@ -26,6 +28,7 @@ from karbor.policies import triggers as trigger_policy from karbor.services.operationengine import api as operationengine_api from karbor import utils +CONF = cfg.CONF LOG = logging.getLogger(__name__) @@ -94,9 +97,18 @@ class TriggersController(wsgi.Controller): if not trigger_name or not trigger_type or not trigger_property: msg = _("Trigger name or type or property is not provided.") raise exc.HTTPBadRequest(explanation=msg) - self.validate_name_and_description(trigger_info) + trigger_format = trigger_property.get('format', None) + if trigger_format != CONF.time_format: + msg = _("Trigger format(%s) is invalid.") % trigger_format + raise exc.HTTPBadRequest(explanation=msg) + trigger_pattern = trigger_property.get('pattern', None) + if CONF.time_format == 'calendar': + utils.validate_calendar_time_format(trigger_pattern) + if CONF.time_format == 'crontab': + utils.validate_crontab_time_format(trigger_pattern) + trigger_property.setdefault( 'start_time', datetime.utcnow().replace(microsecond=0)) trigger_definition = { diff --git a/karbor/common/config.py b/karbor/common/config.py index 54cd9b4f..7c4e9183 100644 --- a/karbor/common/config.py +++ b/karbor/common/config.py @@ -73,6 +73,14 @@ global_opts = [ CONF.register_opts(global_opts) +global_trigger_opts = [ + cfg.StrOpt('time_format', + default='calendar', + choices=['crontab', 'calendar'], + help='The type of time format which is used to compute time') +] +CONF.register_opts(global_trigger_opts) + service_client_opts = [ cfg.StrOpt('service_name', diff --git a/karbor/common/opts.py b/karbor/common/opts.py index 27cd37f9..94f7dc70 100644 --- a/karbor/common/opts.py +++ b/karbor/common/opts.py @@ -65,6 +65,7 @@ _opts = [ karbor.common.config.core_opts, karbor.common.config.debug_opts, karbor.common.config.global_opts, + karbor.common.config.global_trigger_opts, karbor.api.common.api_common_opts, karbor.api.v1.protectables.query_instance_filters_opts, karbor.api.v1.providers.query_provider_filters_opts, diff --git a/karbor/services/operationengine/engine/triggers/timetrigger/time_trigger.py b/karbor/services/operationengine/engine/triggers/timetrigger/time_trigger.py index 5729fc61..b1fabc9d 100644 --- a/karbor/services/operationengine/engine/triggers/timetrigger/time_trigger.py +++ b/karbor/services/operationengine/engine/triggers/timetrigger/time_trigger.py @@ -39,10 +39,6 @@ time_trigger_opts = [ default=1800, help='The maximum window time'), - cfg.StrOpt('time_format', - default='calendar', - choices=['crontab', 'calendar'], - help='The type of time format which is used to compute time'), cfg.IntOpt('trigger_poll_interval', default=15, help='Interval, in seconds, in which Karbor will poll for ' diff --git a/karbor/services/operationengine/engine/triggers/timetrigger/timeformats/calendar_time.py b/karbor/services/operationengine/engine/triggers/timetrigger/timeformats/calendar_time.py index 54af7d17..7f947378 100644 --- a/karbor/services/operationengine/engine/triggers/timetrigger/timeformats/calendar_time.py +++ b/karbor/services/operationengine/engine/triggers/timetrigger/timeformats/calendar_time.py @@ -14,13 +14,11 @@ import os from datetime import timedelta from dateutil import rrule from icalendar import Calendar -from oslo_serialization import jsonutils from oslo_utils import timeutils -from karbor import exception -from karbor.i18n import _ from karbor.services.operationengine.engine.triggers.timetrigger import \ timeformats +from karbor import utils RATE = 2 @@ -46,21 +44,12 @@ class ICal(timeformats.TimeFormat): def __init__(self, start_time, pattern): super(ICal, self).__init__(start_time, pattern) - cal = Calendar.from_ical(self._decode_calendar_pattern(pattern)) + cal = Calendar.from_ical(utils.decode_calendar_pattern(pattern)) vevent = cal.walk('VEVENT')[0] self.dtstart = start_time self.min_freq = self._get_min_freq(vevent) self.rrule_obj = self._get_rrule_obj(vevent, start_time) - @staticmethod - def _decode_calendar_pattern(pattern): - try: - pattern.index('\\') - pattern_dict = jsonutils.loads('{"pattern": "%s"}' % pattern) - return pattern_dict["pattern"] - except Exception: - return pattern - @staticmethod def _get_rrule_obj(vevent, dtstart): rrules = vevent.get('RRULE') @@ -85,25 +74,7 @@ class ICal(timeformats.TimeFormat): :param pattern: The pattern of the icalendar time """ - try: - cal_obj = Calendar.from_ical(cls._decode_calendar_pattern(pattern)) - except Exception: - msg = (_("The trigger pattern(%s) is invalid") % pattern) - raise exception.InvalidInput(msg) - - try: - vevent = cal_obj.walk('VEVENT')[0] - except Exception: - msg = (_("The trigger pattern(%s) must include less than one " - "VEVENT component") % pattern) - raise exception.InvalidInput(msg) - - try: - vevent.decoded('RRULE') - except Exception: - msg = (_("The first VEVENT component of trigger pattern(%s) must " - "include less than one RRULE property") % pattern) - raise exception.InvalidInput(msg) + utils.validate_calendar_time_format(pattern) def compute_next_time(self, current_time): """Compute next time diff --git a/karbor/services/operationengine/engine/triggers/timetrigger/timeformats/crontab_time.py b/karbor/services/operationengine/engine/triggers/timetrigger/timeformats/crontab_time.py index e11467c8..790a7114 100644 --- a/karbor/services/operationengine/engine/triggers/timetrigger/timeformats/crontab_time.py +++ b/karbor/services/operationengine/engine/triggers/timetrigger/timeformats/crontab_time.py @@ -14,10 +14,9 @@ from croniter import croniter from datetime import datetime from oslo_utils import timeutils -from karbor import exception -from karbor.i18n import _ from karbor.services.operationengine.engine.triggers.timetrigger import \ timeformats +from karbor import utils class Crontab(timeformats.TimeFormat): @@ -29,15 +28,7 @@ class Crontab(timeformats.TimeFormat): @classmethod def check_time_format(cls, pattern): - if not pattern: - msg = (_("The trigger pattern is None")) - raise exception.InvalidInput(msg) - - try: - croniter(pattern) - except Exception: - msg = (_("The trigger pattern(%s) is invalid") % pattern) - raise exception.InvalidInput(msg) + utils.validate_crontab_time_format(pattern) def compute_next_time(self, current_time): time = current_time if current_time >= self._start_time else ( diff --git a/karbor/tests/fullstack/test_scheduled_operations.py b/karbor/tests/fullstack/test_scheduled_operations.py index 55e446a1..df71b8f2 100644 --- a/karbor/tests/fullstack/test_scheduled_operations.py +++ b/karbor/tests/fullstack/test_scheduled_operations.py @@ -24,7 +24,10 @@ from karbor.tests.fullstack import karbor_objects as objects from karbor.tests.fullstack import utils pattern = "BEGIN:VEVENT\nRRULE:FREQ=WEEKLY;INTERVAL=1;\nEND:VEVENT" -DEFAULT_PROPERTY = {'pattern': pattern} +DEFAULT_PROPERTY = { + 'pattern': pattern, + 'format': 'calendar' +} class ScheduledOperationsTest(karbor_base.KarborBaseTest): diff --git a/karbor/tests/unit/api/v1/test_scheduled_operation.py b/karbor/tests/unit/api/v1/test_scheduled_operation.py index 70744c6e..78c2eaea 100644 --- a/karbor/tests/unit/api/v1/test_scheduled_operation.py +++ b/karbor/tests/unit/api/v1/test_scheduled_operation.py @@ -11,6 +11,7 @@ # under the License. import mock +from oslo_config import cfg from oslo_utils import uuidutils from webob import exc @@ -51,6 +52,7 @@ class ScheduledOperationApiTest(base.TestCase): self.ctxt = context.RequestContext('demo', 'fakeproject', True) self.req = fakes.HTTPRequest.blank('/v1/scheduled_operations') + cfg.CONF.set_default('time_format', 'crontab') trigger = self._create_trigger() self._plan = self._create_plan(uuidutils.generate_uuid()) self.default_create_operation_param = { diff --git a/karbor/tests/unit/api/v1/test_triggers.py b/karbor/tests/unit/api/v1/test_triggers.py index 30e62836..2c090b35 100644 --- a/karbor/tests/unit/api/v1/test_triggers.py +++ b/karbor/tests/unit/api/v1/test_triggers.py @@ -9,7 +9,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. - +from oslo_config import cfg from webob import exc from karbor.api.v1 import triggers as trigger_api @@ -45,6 +45,7 @@ class TriggerApiTest(base.TestCase): self.controller.operationengine_api = FakeRemoteOperationApi() self.ctxt = context.RequestContext('demo', 'fakeproject', True) + cfg.CONF.set_default('time_format', 'crontab') self.req = fakes.HTTPRequest.blank('/v1/triggers') self.default_create_trigger_param = { "name": "123", diff --git a/karbor/utils.py b/karbor/utils.py index 6dc54a9f..de11bed2 100644 --- a/karbor/utils.py +++ b/karbor/utils.py @@ -19,9 +19,12 @@ import six import tempfile import webob.exc +from croniter import croniter +from icalendar import Calendar from keystoneclient import discover as ks_discover from oslo_config import cfg from oslo_log import log as logging +from oslo_serialization import jsonutils from oslo_utils import importutils from oslo_utils import strutils from oslo_utils import timeutils @@ -34,6 +37,49 @@ CONF = cfg.CONF LOG = logging.getLogger(__name__) +def decode_calendar_pattern(pattern): + try: + pattern.index('\\') + pattern_dict = jsonutils.loads('{"pattern": "%s"}' % pattern) + return pattern_dict["pattern"] + except Exception: + return pattern + + +def validate_calendar_time_format(pattern): + try: + cal_obj = Calendar.from_ical(decode_calendar_pattern(pattern)) + except Exception: + msg = (_("The trigger pattern(%s) is invalid") % pattern) + raise exception.InvalidInput(msg) + + try: + vevent = cal_obj.walk('VEVENT')[0] + except Exception: + msg = (_("The trigger pattern(%s) must include less than one " + "VEVENT component") % pattern) + raise exception.InvalidInput(msg) + + try: + vevent.decoded('RRULE') + except Exception: + msg = (_("The first VEVENT component of trigger pattern(%s) must " + "include less than one RRULE property") % pattern) + raise exception.InvalidInput(msg) + + +def validate_crontab_time_format(pattern): + if not pattern: + msg = (_("The trigger pattern is None")) + raise exception.InvalidInput(msg) + + try: + croniter(pattern) + except Exception: + msg = (_("The trigger pattern(%s) is invalid") % pattern) + raise exception.InvalidInput(msg) + + def find_config(config_path): """Find a configuration file using the given hint.