summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorVitalii Solodilov <mcdkr@yandex.ru>2018-07-29 18:17:51 +0400
committerVitalii Solodilov <mcdkr@yandex.ru>2018-07-30 11:55:35 +0400
commit4bf03d8e7df0ecc418b7069819cf3b59898c1eb8 (patch)
treea1e66c9c7d0da2439506ec230ae1d06312237d9b
parentb01d6c6642b01461851aee5ac7709185c478e320 (diff)
Remove extra a specification validation
Currently when we get a specification using the instantiate_spec function, we always validate their schema and semantics over and over again. To prevent it we add new validate parameter to a Spec class. The validate parameter must be True when we create a workflow, workbook or action using a mistral-api. In all other cases, it must be False. Change-Id: Ia450ea9635bc75c204fe031cfeeab154f1d03862 Closes-Bug: #1738769 Signed-off-by: Vitalii Solodilov <mcdkr@yandex.ru>
Notes
Notes (review): Code-Review+2: Andras Kovi <akovi@nokia.com> Code-Review+2: Dougal Matthews <dougal@redhat.com> Workflow+1: Dougal Matthews <dougal@redhat.com> Code-Review+2: Renat Akhmerov <renat.akhmerov@gmail.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Tue, 31 Jul 2018 08:27:18 +0000 Reviewed-on: https://review.openstack.org/586883 Project: openstack/mistral Branch: refs/heads/master
-rw-r--r--mistral/api/controllers/v2/validation.py2
-rw-r--r--mistral/lang/base.py47
-rw-r--r--mistral/lang/parser.py30
-rw-r--r--mistral/lang/v2/actions.py4
-rw-r--r--mistral/lang/v2/on_clause.py4
-rw-r--r--mistral/lang/v2/policies.py4
-rw-r--r--mistral/lang/v2/publish.py4
-rw-r--r--mistral/lang/v2/retry_policy.py4
-rw-r--r--mistral/lang/v2/task_defaults.py4
-rw-r--r--mistral/lang/v2/tasks.py20
-rw-r--r--mistral/lang/v2/workbook.py4
-rw-r--r--mistral/lang/v2/workflows.py8
-rw-r--r--mistral/services/workbooks.py8
-rw-r--r--mistral/services/workflows.py8
14 files changed, 90 insertions, 61 deletions
diff --git a/mistral/api/controllers/v2/validation.py b/mistral/api/controllers/v2/validation.py
index 5c91fda..e436913 100644
--- a/mistral/api/controllers/v2/validation.py
+++ b/mistral/api/controllers/v2/validation.py
@@ -32,7 +32,7 @@ class SpecValidationController(rest.RestController):
32 definition = pecan.request.text 32 definition = pecan.request.text
33 33
34 try: 34 try:
35 self._parse_func(definition) 35 self._parse_func(definition, validate=True)
36 except exc.DSLParsingException as e: 36 except exc.DSLParsingException as e:
37 return {'valid': False, 'error': str(e)} 37 return {'valid': False, 'error': str(e)}
38 38
diff --git a/mistral/lang/base.py b/mistral/lang/base.py
index 4fdb03a..57fed49 100644
--- a/mistral/lang/base.py
+++ b/mistral/lang/base.py
@@ -52,7 +52,7 @@ ALL = (
52PARAMS_PTRN = re.compile("([-_\w]+)=(%s)" % "|".join(ALL)) 52PARAMS_PTRN = re.compile("([-_\w]+)=(%s)" % "|".join(ALL))
53 53
54 54
55def instantiate_spec(spec_cls, data): 55def instantiate_spec(spec_cls, data, validate=False):
56 """Instantiates specification accounting for specification hierarchies. 56 """Instantiates specification accounting for specification hierarchies.
57 57
58 :param spec_cls: Specification concrete or base class. In case if base 58 :param spec_cls: Specification concrete or base class. In case if base
@@ -60,17 +60,22 @@ def instantiate_spec(spec_cls, data):
60 _polymorphic_key and _polymorphic_value in order to find a concrete 60 _polymorphic_key and _polymorphic_value in order to find a concrete
61 class that needs to be instantiated. 61 class that needs to be instantiated.
62 :param data: Raw specification data as a dictionary. 62 :param data: Raw specification data as a dictionary.
63 :type data: dict
64 :param validate: If it equals False then semantics and schema validation
65 will be skipped
66 :type validate: bool
63 """ 67 """
64 68
65 if issubclass(spec_cls, BaseSpecList): 69 if issubclass(spec_cls, BaseSpecList):
66 # Ignore polymorphic search for specification lists because 70 # Ignore polymorphic search for specification lists because
67 # it doesn't make sense for them. 71 # it doesn't make sense for them.
68 return spec_cls(data) 72 return spec_cls(data, validate)
69 73
70 if not hasattr(spec_cls, '_polymorphic_key'): 74 if not hasattr(spec_cls, '_polymorphic_key'):
71 spec = spec_cls(data) 75 spec = spec_cls(data, validate)
72 76
73 spec.validate_semantics() 77 if validate:
78 spec.validate_semantics()
74 79
75 return spec 80 return spec
76 81
@@ -101,9 +106,10 @@ def instantiate_spec(spec_cls, data):
101 ) 106 )
102 107
103 if cls._polymorphic_value == data.get(key_name, key_default): 108 if cls._polymorphic_value == data.get(key_name, key_default):
104 spec = cls(data) 109 spec = cls(data, validate)
105 110
106 spec.validate_semantics() 111 if validate:
112 spec.validate_semantics()
107 113
108 return spec 114 return spec
109 115
@@ -176,10 +182,12 @@ class BaseSpec(object):
176 182
177 return schema 183 return schema
178 184
179 def __init__(self, data): 185 def __init__(self, data, validate):
180 self._data = data 186 self._data = data
187 self._validate = validate
181 188
182 self.validate_schema() 189 if validate:
190 self.validate_schema()
183 191
184 def validate_schema(self): 192 def validate_schema(self):
185 """Validates DSL entity schema that this specification represents. 193 """Validates DSL entity schema that this specification represents.
@@ -227,10 +235,10 @@ class BaseSpec(object):
227 def _spec_property(self, prop_name, spec_cls): 235 def _spec_property(self, prop_name, spec_cls):
228 prop_val = self._data.get(prop_name) 236 prop_val = self._data.get(prop_name)
229 237
230 return ( 238 if prop_val is not None:
231 instantiate_spec(spec_cls, prop_val) if prop_val is not None 239 return instantiate_spec(spec_cls, prop_val, self._validate)
232 else None 240 else:
233 ) 241 return None
234 242
235 def _group_spec(self, spec_cls, *prop_names): 243 def _group_spec(self, spec_cls, *prop_names):
236 if not prop_names: 244 if not prop_names:
@@ -244,7 +252,7 @@ class BaseSpec(object):
244 if prop_val: 252 if prop_val:
245 data[prop_name] = prop_val 253 data[prop_name] = prop_val
246 254
247 return instantiate_spec(spec_cls, data) 255 return instantiate_spec(spec_cls, data, self._validate)
248 256
249 def _inject_version(self, prop_names): 257 def _inject_version(self, prop_names):
250 for prop_name in prop_names: 258 for prop_name in prop_names:
@@ -322,8 +330,8 @@ class BaseListSpec(BaseSpec):
322 "required": ["version"], 330 "required": ["version"],
323 } 331 }
324 332
325 def __init__(self, data): 333 def __init__(self, data, validate):
326 super(BaseListSpec, self).__init__(data) 334 super(BaseListSpec, self).__init__(data, validate)
327 335
328 self.items = [] 336 self.items = []
329 337
@@ -331,7 +339,10 @@ class BaseListSpec(BaseSpec):
331 if k != 'version': 339 if k != 'version':
332 v['name'] = k 340 v['name'] = k
333 self._inject_version([k]) 341 self._inject_version([k])
334 self.items.append(instantiate_spec(self.item_class, v)) 342
343 self.items.append(
344 instantiate_spec(self.item_class, v, validate)
345 )
335 346
336 def validate_schema(self): 347 def validate_schema(self):
337 super(BaseListSpec, self).validate_schema() 348 super(BaseListSpec, self).validate_schema()
@@ -357,7 +368,7 @@ class BaseSpecList(object):
357 368
358 _version = '2.0' 369 _version = '2.0'
359 370
360 def __init__(self, data): 371 def __init__(self, data, validate):
361 self.items = {} 372 self.items = {}
362 373
363 for k, v in data.items(): 374 for k, v in data.items():
@@ -369,7 +380,7 @@ class BaseSpecList(object):
369 v['name'] = k 380 v['name'] = k
370 v['version'] = self._version 381 v['version'] = self._version
371 382
372 self.items[k] = instantiate_spec(self.item_class, v) 383 self.items[k] = instantiate_spec(self.item_class, v, validate)
373 384
374 def item_keys(self): 385 def item_keys(self):
375 return self.items.keys() 386 return self.items.keys()
diff --git a/mistral/lang/parser.py b/mistral/lang/parser.py
index dc92c18..0af0bf9 100644
--- a/mistral/lang/parser.py
+++ b/mistral/lang/parser.py
@@ -80,15 +80,17 @@ def _get_spec_version(spec_dict):
80# Factory methods to get specifications either from raw YAML formatted text or 80# Factory methods to get specifications either from raw YAML formatted text or
81# from dictionaries parsed from YAML formatted text. 81# from dictionaries parsed from YAML formatted text.
82 82
83def get_workbook_spec(spec_dict): 83def get_workbook_spec(spec_dict, validate):
84 if _get_spec_version(spec_dict) == V2_0: 84 if _get_spec_version(spec_dict) == V2_0:
85 return base.instantiate_spec(wb_v2.WorkbookSpec, spec_dict) 85 return base.instantiate_spec(
86 wb_v2.WorkbookSpec, spec_dict, validate
87 )
86 88
87 return None 89 return None
88 90
89 91
90def get_workbook_spec_from_yaml(text): 92def get_workbook_spec_from_yaml(text, validate=True):
91 return get_workbook_spec(parse_yaml(text)) 93 return get_workbook_spec(parse_yaml(text), validate)
92 94
93 95
94def get_action_spec(spec_dict): 96def get_action_spec(spec_dict):
@@ -106,12 +108,14 @@ def get_action_spec_from_yaml(text, action_name):
106 return get_action_spec(spec_dict) 108 return get_action_spec(spec_dict)
107 109
108 110
109def get_action_list_spec(spec_dict): 111def get_action_list_spec(spec_dict, validate):
110 return base.instantiate_spec(actions_v2.ActionListSpec, spec_dict) 112 return base.instantiate_spec(
113 actions_v2.ActionListSpec, spec_dict, validate
114 )
111 115
112 116
113def get_action_list_spec_from_yaml(text): 117def get_action_list_spec_from_yaml(text, validate=True):
114 return get_action_list_spec(parse_yaml(text)) 118 return get_action_list_spec(parse_yaml(text), validate=validate)
115 119
116 120
117def get_workflow_spec(spec_dict): 121def get_workflow_spec(spec_dict):
@@ -130,16 +134,18 @@ def get_workflow_spec(spec_dict):
130 return None 134 return None
131 135
132 136
133def get_workflow_list_spec(spec_dict): 137def get_workflow_list_spec(spec_dict, validate):
134 return base.instantiate_spec(wf_v2.WorkflowListSpec, spec_dict) 138 return base.instantiate_spec(
139 wf_v2.WorkflowListSpec, spec_dict, validate
140 )
135 141
136 142
137def get_workflow_spec_from_yaml(text): 143def get_workflow_spec_from_yaml(text):
138 return get_workflow_spec(parse_yaml(text)) 144 return get_workflow_spec(parse_yaml(text))
139 145
140 146
141def get_workflow_list_spec_from_yaml(text): 147def get_workflow_list_spec_from_yaml(text, validate=True):
142 return get_workflow_list_spec(parse_yaml(text)) 148 return get_workflow_list_spec(parse_yaml(text), validate)
143 149
144 150
145def get_task_spec(spec_dict): 151def get_task_spec(spec_dict):
diff --git a/mistral/lang/v2/actions.py b/mistral/lang/v2/actions.py
index 2d37d9d..a736967 100644
--- a/mistral/lang/v2/actions.py
+++ b/mistral/lang/v2/actions.py
@@ -34,8 +34,8 @@ class ActionSpec(base.BaseSpec):
34 "additionalProperties": False 34 "additionalProperties": False
35 } 35 }
36 36
37 def __init__(self, data): 37 def __init__(self, data, validate):
38 super(ActionSpec, self).__init__(data) 38 super(ActionSpec, self).__init__(data, validate)
39 39
40 self._name = data['name'] 40 self._name = data['name']
41 self._description = data.get('description') 41 self._description = data.get('description')
diff --git a/mistral/lang/v2/on_clause.py b/mistral/lang/v2/on_clause.py
index cb65312..e5c1bb7 100644
--- a/mistral/lang/v2/on_clause.py
+++ b/mistral/lang/v2/on_clause.py
@@ -94,8 +94,8 @@ class OnClauseSpec(base.BaseSpec):
94 ] 94 ]
95 } 95 }
96 96
97 def __init__(self, data): 97 def __init__(self, data, validate):
98 super(OnClauseSpec, self).__init__(data) 98 super(OnClauseSpec, self).__init__(data, validate)
99 99
100 if not isinstance(data, dict): 100 if not isinstance(data, dict):
101 # Old simple schema. 101 # Old simple schema.
diff --git a/mistral/lang/v2/policies.py b/mistral/lang/v2/policies.py
index 775b07e..b18e0d1 100644
--- a/mistral/lang/v2/policies.py
+++ b/mistral/lang/v2/policies.py
@@ -37,8 +37,8 @@ class PoliciesSpec(base.BaseSpec):
37 def get_schema(cls, includes=['definitions']): 37 def get_schema(cls, includes=['definitions']):
38 return super(PoliciesSpec, cls).get_schema(includes) 38 return super(PoliciesSpec, cls).get_schema(includes)
39 39
40 def __init__(self, data): 40 def __init__(self, data, validate):
41 super(PoliciesSpec, self).__init__(data) 41 super(PoliciesSpec, self).__init__(data, validate)
42 42
43 self._retry = self._spec_property('retry', retry_policy.RetrySpec) 43 self._retry = self._spec_property('retry', retry_policy.RetrySpec)
44 self._wait_before = data.get('wait-before', 0) 44 self._wait_before = data.get('wait-before', 0)
diff --git a/mistral/lang/v2/publish.py b/mistral/lang/v2/publish.py
index a3adc10..9515aa3 100644
--- a/mistral/lang/v2/publish.py
+++ b/mistral/lang/v2/publish.py
@@ -29,8 +29,8 @@ class PublishSpec(base.BaseSpec):
29 "additionalProperties": False 29 "additionalProperties": False
30 } 30 }
31 31
32 def __init__(self, data): 32 def __init__(self, data, validate):
33 super(PublishSpec, self).__init__(data) 33 super(PublishSpec, self).__init__(data, validate)
34 34
35 self._branch = self._data.get('branch') 35 self._branch = self._data.get('branch')
36 self._global = self._data.get('global') 36 self._global = self._data.get('global')
diff --git a/mistral/lang/v2/retry_policy.py b/mistral/lang/v2/retry_policy.py
index 224d188..cc69149 100644
--- a/mistral/lang/v2/retry_policy.py
+++ b/mistral/lang/v2/retry_policy.py
@@ -54,10 +54,10 @@ class RetrySpec(base.BaseSpec):
54 def get_schema(cls, includes=['definitions']): 54 def get_schema(cls, includes=['definitions']):
55 return super(RetrySpec, cls).get_schema(includes) 55 return super(RetrySpec, cls).get_schema(includes)
56 56
57 def __init__(self, data): 57 def __init__(self, data, validate):
58 data = self._transform_retry_one_line(data) 58 data = self._transform_retry_one_line(data)
59 59
60 super(RetrySpec, self).__init__(data) 60 super(RetrySpec, self).__init__(data, validate)
61 61
62 self._break_on = data.get('break-on') 62 self._break_on = data.get('break-on')
63 self._count = data.get('count') 63 self._count = data.get('count')
diff --git a/mistral/lang/v2/task_defaults.py b/mistral/lang/v2/task_defaults.py
index acb1b48..08f8bee 100644
--- a/mistral/lang/v2/task_defaults.py
+++ b/mistral/lang/v2/task_defaults.py
@@ -53,8 +53,8 @@ class TaskDefaultsSpec(base.BaseSpec):
53 def get_schema(cls, includes=['definitions']): 53 def get_schema(cls, includes=['definitions']):
54 return super(TaskDefaultsSpec, cls).get_schema(includes) 54 return super(TaskDefaultsSpec, cls).get_schema(includes)
55 55
56 def __init__(self, data): 56 def __init__(self, data, validate):
57 super(TaskDefaultsSpec, self).__init__(data) 57 super(TaskDefaultsSpec, self).__init__(data, validate)
58 58
59 self._policies = self._group_spec( 59 self._policies = self._group_spec(
60 policies.PoliciesSpec, 60 policies.PoliciesSpec,
diff --git a/mistral/lang/v2/tasks.py b/mistral/lang/v2/tasks.py
index 96ee804..70ad9f1 100644
--- a/mistral/lang/v2/tasks.py
+++ b/mistral/lang/v2/tasks.py
@@ -107,8 +107,8 @@ class TaskSpec(base.BaseSpec):
107 ] 107 ]
108 } 108 }
109 109
110 def __init__(self, data): 110 def __init__(self, data, validate):
111 super(TaskSpec, self).__init__(data) 111 super(TaskSpec, self).__init__(data, validate)
112 112
113 self._name = data['name'] 113 self._name = data['name']
114 self._description = data.get('description') 114 self._description = data.get('description')
@@ -248,10 +248,14 @@ class TaskSpec(base.BaseSpec):
248 spec = None 248 spec = None
249 249
250 if state == states.SUCCESS and self._publish: 250 if state == states.SUCCESS and self._publish:
251 spec = publish.PublishSpec({'branch': self._publish}) 251 spec = publish.PublishSpec(
252 {'branch': self._publish},
253 validate=self._validate
254 )
252 elif state == states.ERROR and self._publish_on_error: 255 elif state == states.ERROR and self._publish_on_error:
253 spec = publish.PublishSpec( 256 spec = publish.PublishSpec(
254 {'branch': self._publish_on_error} 257 {'branch': self._publish_on_error},
258 validate=self._validate
255 ) 259 )
256 260
257 return spec 261 return spec
@@ -291,8 +295,8 @@ class DirectWorkflowTaskSpec(TaskSpec):
291 _direct_workflow_schema 295 _direct_workflow_schema
292 ) 296 )
293 297
294 def __init__(self, data): 298 def __init__(self, data, validate):
295 super(DirectWorkflowTaskSpec, self).__init__(data) 299 super(DirectWorkflowTaskSpec, self).__init__(data, validate)
296 300
297 self._join = data.get('join') 301 self._join = data.get('join')
298 302
@@ -376,8 +380,8 @@ class ReverseWorkflowTaskSpec(TaskSpec):
376 _reverse_workflow_schema 380 _reverse_workflow_schema
377 ) 381 )
378 382
379 def __init__(self, data): 383 def __init__(self, data, validate):
380 super(ReverseWorkflowTaskSpec, self).__init__(data) 384 super(ReverseWorkflowTaskSpec, self).__init__(data, validate)
381 385
382 self._requires = data.get('requires', []) 386 self._requires = data.get('requires', [])
383 387
diff --git a/mistral/lang/v2/workbook.py b/mistral/lang/v2/workbook.py
index 72c93d5..22a501a 100644
--- a/mistral/lang/v2/workbook.py
+++ b/mistral/lang/v2/workbook.py
@@ -51,8 +51,8 @@ class WorkbookSpec(base.BaseSpec):
51 "additionalProperties": False 51 "additionalProperties": False
52 } 52 }
53 53
54 def __init__(self, data): 54 def __init__(self, data, validate):
55 super(WorkbookSpec, self).__init__(data) 55 super(WorkbookSpec, self).__init__(data, validate)
56 56
57 self._inject_version(['actions', 'workflows']) 57 self._inject_version(['actions', 'workflows'])
58 58
diff --git a/mistral/lang/v2/workflows.py b/mistral/lang/v2/workflows.py
index 2d60627..af1f1ca 100644
--- a/mistral/lang/v2/workflows.py
+++ b/mistral/lang/v2/workflows.py
@@ -44,8 +44,8 @@ class WorkflowSpec(base.BaseSpec):
44 "additionalProperties": False 44 "additionalProperties": False
45 } 45 }
46 46
47 def __init__(self, data): 47 def __init__(self, data, validate):
48 super(WorkflowSpec, self).__init__(data) 48 super(WorkflowSpec, self).__init__(data, validate)
49 49
50 self._name = data['name'] 50 self._name = data['name']
51 self._description = data.get('description') 51 self._description = data.get('description')
@@ -152,8 +152,8 @@ class DirectWorkflowSpec(WorkflowSpec):
152 } 152 }
153 } 153 }
154 154
155 def __init__(self, data): 155 def __init__(self, data, validate):
156 super(DirectWorkflowSpec, self).__init__(data) 156 super(DirectWorkflowSpec, self).__init__(data, validate)
157 157
158 # Init simple dictionary based caches for inbound and 158 # Init simple dictionary based caches for inbound and
159 # outbound task specifications. In fact, we don't need 159 # outbound task specifications. In fact, we don't need
diff --git a/mistral/services/workbooks.py b/mistral/services/workbooks.py
index f55333c..77fafeb 100644
--- a/mistral/services/workbooks.py
+++ b/mistral/services/workbooks.py
@@ -18,7 +18,9 @@ from mistral.services import actions
18 18
19 19
20def create_workbook_v2(definition, namespace='', scope='private'): 20def create_workbook_v2(definition, namespace='', scope='private'):
21 wb_spec = spec_parser.get_workbook_spec_from_yaml(definition) 21 wb_spec = spec_parser.get_workbook_spec_from_yaml(
22 definition, validate=True
23 )
22 24
23 wb_values = _get_workbook_values( 25 wb_values = _get_workbook_values(
24 wb_spec, 26 wb_spec,
@@ -36,7 +38,9 @@ def create_workbook_v2(definition, namespace='', scope='private'):
36 38
37 39
38def update_workbook_v2(definition, namespace='', scope='private'): 40def update_workbook_v2(definition, namespace='', scope='private'):
39 wb_spec = spec_parser.get_workbook_spec_from_yaml(definition) 41 wb_spec = spec_parser.get_workbook_spec_from_yaml(
42 definition, validate=True
43 )
40 44
41 values = _get_workbook_values(wb_spec, definition, scope, namespace) 45 values = _get_workbook_values(wb_spec, definition, scope, namespace)
42 46
diff --git a/mistral/services/workflows.py b/mistral/services/workflows.py
index c9e9a1d..7a49ccb 100644
--- a/mistral/services/workflows.py
+++ b/mistral/services/workflows.py
@@ -54,7 +54,9 @@ def sync_db():
54def create_workflows(definition, scope='private', is_system=False, 54def create_workflows(definition, scope='private', is_system=False,
55 run_in_tx=True, namespace=''): 55 run_in_tx=True, namespace=''):
56 LOG.debug("Creating workflows") 56 LOG.debug("Creating workflows")
57 wf_list_spec = spec_parser.get_workflow_list_spec_from_yaml(definition) 57 wf_list_spec = spec_parser.get_workflow_list_spec_from_yaml(
58 definition, validate=True
59 )
58 db_wfs = [] 60 db_wfs = []
59 61
60 if run_in_tx: 62 if run_in_tx:
@@ -98,7 +100,9 @@ def update_workflows(definition, scope='private', identifier=None,
98 namespace=''): 100 namespace=''):
99 LOG.debug("Updating workflows") 101 LOG.debug("Updating workflows")
100 102
101 wf_list_spec = spec_parser.get_workflow_list_spec_from_yaml(definition) 103 wf_list_spec = spec_parser.get_workflow_list_spec_from_yaml(
104 definition, validate=True
105 )
102 wfs = wf_list_spec.get_workflows() 106 wfs = wf_list_spec.get_workflows()
103 107
104 if identifier and len(wfs) > 1: 108 if identifier and len(wfs) > 1: