Browse Source

Validate attributes restrictions

Since Nailgun contains attributes restriction
mechanism it's possible to verify attributes
restrictions. This commit applies restrictions
checks into validation for both node attributes
and cluster attributes.

Change-Id: I269da9a7a7df5fea336c07784b37d6ced1641993
Closes-Bug: #1567394
(cherry picked from commit 7e3fe2c308)
tags/9.0^2
Artur Svechnikov 3 years ago
parent
commit
f3217aa07a

+ 5
- 1
nailgun/nailgun/api/v1/handlers/node.py View File

@@ -351,7 +351,11 @@ class NodeAttributesHandler(BaseHandler):
351 351
         """
352 352
         node = self.get_object_or_404(objects.Node, node_id)
353 353
 
354
-        data = self.checked_data(node=node)
354
+        if not node.cluster:
355
+            raise self.http(400, "Node '{}' doesn't belong to any cluster"
356
+                            .format(node.id))
357
+
358
+        data = self.checked_data(node=node, cluster=node.cluster)
355 359
         objects.Node.update_attributes(node, data)
356 360
 
357 361
         return objects.Node.get_attributes(node)

+ 19
- 2
nailgun/nailgun/api/v1/validators/base.py View File

@@ -141,14 +141,31 @@ class BasicAttributesValidator(BasicValidator):
141 141
         return attrs
142 142
 
143 143
     @classmethod
144
-    def validate_attributes(cls, data):
145
-        """Validate attributes."""
144
+    def validate_attributes(cls, data, models=None, force=False):
145
+        """Validate attributes.
146
+
147
+        :param data: attributes
148
+        :type data: dict
149
+        :param models: models which are used in
150
+                       restrictions conditions
151
+        :type models: dict
152
+        :param force: don't check restrictions
153
+        :type force: bool
154
+        """
146 155
         for attrs in six.itervalues(data):
147 156
             if not isinstance(attrs, dict):
148 157
                 continue
149 158
             for attr_name, attr in six.iteritems(attrs):
150 159
                 cls.validate_attribute(attr_name, attr)
151 160
 
161
+        # If settings are present restrictions can be checked
162
+        if models and not force:
163
+            restrict_err = restrictions.AttributesRestriction.check_data(
164
+                models, data)
165
+            if restrict_err:
166
+                raise errors.InvalidData(
167
+                    "Some restrictions didn't pass verification: {}"
168
+                    .format(restrict_err))
152 169
         return data
153 170
 
154 171
     @classmethod

+ 12
- 1
nailgun/nailgun/api/v1/validators/cluster.py View File

@@ -28,6 +28,7 @@ from nailgun.db.sqlalchemy.models import Node
28 28
 from nailgun.errors import errors
29 29
 from nailgun import objects
30 30
 from nailgun.plugins.manager import PluginManager
31
+from nailgun.settings import settings
31 32
 
32 33
 
33 34
 class ClusterValidator(base.BasicValidator):
@@ -232,11 +233,21 @@ class ClusterAttributesValidator(base.BasicAttributesValidator):
232 233
             )
233 234
 
234 235
         attrs = d
236
+        models = None
237
+
235 238
         if cluster is not None:
236 239
             attrs = objects.Cluster.get_updated_editable_attributes(cluster, d)
237 240
             cls.validate_provision(cluster, attrs)
238 241
             cls.validate_allowed_attributes(cluster, d, force)
239
-        cls.validate_attributes(attrs.get('editable', {}))
242
+
243
+            models = {
244
+                'settings': attrs.get('editable', {}),
245
+                'cluster': cluster,
246
+                'version': settings.VERSION,
247
+                'networking_parameters': cluster.network_config,
248
+            }
249
+
250
+        cls.validate_attributes(attrs.get('editable', {}), models, force=force)
240 251
 
241 252
         return d
242 253
 

+ 10
- 2
nailgun/nailgun/api/v1/validators/node.py View File

@@ -26,6 +26,7 @@ from nailgun.db.sqlalchemy.models import Node
26 26
 from nailgun.db.sqlalchemy.models import NodeNICInterface
27 27
 from nailgun.errors import errors
28 28
 from nailgun import objects
29
+from nailgun.settings import settings
29 30
 from nailgun import utils
30 31
 
31 32
 
@@ -442,10 +443,17 @@ class NodeDeploymentValidator(TaskDeploymentValidator,
442 443
 class NodeAttributesValidator(base.BasicAttributesValidator):
443 444
 
444 445
     @classmethod
445
-    def validate(cls, data, node):
446
+    def validate(cls, data, node, cluster):
446 447
         data = cls.validate_json(data)
447 448
         full_data = utils.dict_merge(objects.Node.get_attributes(node), data)
448
-        attrs = cls.validate_attributes(full_data)
449
+
450
+        models = {
451
+            'settings': objects.Cluster.get_editable_attributes(cluster),
452
+            'cluster': cluster,
453
+            'version': settings.VERSION,
454
+            'networking_parameters': cluster.network_config,
455
+        }
456
+        attrs = cls.validate_attributes(full_data, models=models)
449 457
 
450 458
         cls._validate_cpu_pinning(node, attrs)
451 459
         cls._validate_hugepages(node, attrs)

+ 57
- 1
nailgun/nailgun/test/integration/test_attributes.py View File

@@ -180,6 +180,60 @@ class TestClusterAttributes(BaseIntegrationTest):
180 180
         )
181 181
         self.assertEqual(400, resp.status_code)
182 182
 
183
+    def test_failing_attributes_with_restrictions(self):
184
+        cluster = self.env.create_cluster(api=False)
185
+        objects.Cluster.patch_attributes(cluster, {
186
+            'editable': {
187
+                'test': {
188
+                    'comp1': {
189
+                        'description': 'desc',
190
+                        'label': 'Comp 1',
191
+                        'type': 'checkbox',
192
+                        'value': False,
193
+                        'weight': 10,
194
+                    },
195
+                    'comp2': {
196
+                        'description': 'desc',
197
+                        'label': 'Comp 2',
198
+                        'type': 'checkbox',
199
+                        'value': False,
200
+                        'weight': 20,
201
+                        'restrictions': ["settings:test.comp1.value == true"],
202
+                    },
203
+                },
204
+            },
205
+        })
206
+        resp = self.app.patch(
207
+            reverse(
208
+                'ClusterAttributesHandler',
209
+                kwargs={'cluster_id': cluster.id}),
210
+            params=jsonutils.dumps({
211
+                'editable': {
212
+                    'test': {
213
+                        'comp1': {
214
+                            'value': True
215
+                        },
216
+                        'comp2': {
217
+                            'value': True
218
+                        },
219
+                    },
220
+                },
221
+            }),
222
+            headers=self.default_headers,
223
+            expect_errors=True
224
+        )
225
+        self.assertEqual(400, resp.status_code)
226
+
227
+        extended_restr = {
228
+            'condition': "settings:test.comp1.value == true",
229
+            'action': 'disable',
230
+        }
231
+        self.assertIn(
232
+            "Validation failed for attribute 'Comp 2': restriction with action"
233
+            "='{}' and condition='{}' failed due to attribute value='True'"
234
+            .format(extended_restr['action'], extended_restr['condition']),
235
+            resp.json_body['message'])
236
+
183 237
     def test_get_default_attributes(self):
184 238
         cluster = self.env.create_cluster(api=True)
185 239
         release = self.db.query(Release).get(
@@ -781,7 +835,9 @@ class TestAttributesWithPlugins(BaseIntegrationTest):
781 835
                                         'label': 'label',
782 836
                                         'value': '1',
783 837
                                         'weight': 25,
784
-                                        'restrictions': [{'action': 'hide'}]
838
+                                        'restrictions': [{
839
+                                            'condition': 'true',
840
+                                            'action': 'hide'}]
785 841
                                     }
786 842
                                 }]
787 843
                             },

+ 14
- 4
nailgun/nailgun/test/integration/test_cluster_changes_handler.py View File

@@ -1639,8 +1639,13 @@ class TestHandlers(BaseIntegrationTest):
1639 1639
                 kwargs={'cluster_id': cluster['id']}),
1640 1640
             params=jsonutils.dumps({
1641 1641
                 'editable': {
1642
-                    'storage': {'volumes_ceph': {'value': True},
1643
-                                'osd_pool_size': {'value': '3'}}}}),
1642
+                    'storage': {
1643
+                        'volumes_ceph': {'value': True},
1644
+                        'osd_pool_size': {'value': '3'},
1645
+                        'volumes_lvm': {'value': False},
1646
+                    }
1647
+                }
1648
+            }),
1644 1649
             headers=self.default_headers)
1645 1650
 
1646 1651
         task = self.env.launch_deployment()
@@ -1701,8 +1706,13 @@ class TestHandlers(BaseIntegrationTest):
1701 1706
                 kwargs={'cluster_id': cluster['id']}),
1702 1707
             params=jsonutils.dumps({
1703 1708
                 'editable': {
1704
-                    'storage': {'volumes_ceph': {'value': True},
1705
-                                'osd_pool_size': {'value': '1'}}}}),
1709
+                    'storage': {
1710
+                        'volumes_ceph': {'value': True},
1711
+                        'osd_pool_size': {'value': '1'},
1712
+                        'volumes_lvm': {'value': False},
1713
+                    }
1714
+                }
1715
+            }),
1706 1716
             headers=self.default_headers)
1707 1717
 
1708 1718
         task = self.env.launch_deployment()

+ 2
- 1
nailgun/nailgun/test/integration/test_node_handler.py View File

@@ -426,7 +426,8 @@ class TestHandlers(BaseIntegrationTest):
426 426
         self.assertEqual(fake_attributes, resp.json_body)
427 427
 
428 428
     def test_put_node_attributes(self):
429
-        node = self.env.create_node(api=False)
429
+        self.env.create(nodes_kwargs=[{}])
430
+        node = self.env.nodes[-1]
430 431
         fake_attributes = {
431 432
             'group1': {
432 433
                 'metadata': {},

+ 36
- 7
nailgun/nailgun/test/unit/test_node_attributes_validator.py View File

@@ -17,13 +17,34 @@
17 17
 import json
18 18
 import mock
19 19
 from nailgun.api.v1.validators import node as node_validator
20
+from nailgun import consts
20 21
 from nailgun.errors import errors
22
+from nailgun import objects
21 23
 from nailgun.test import base
22 24
 
23 25
 
24 26
 validator = node_validator.NodeAttributesValidator.validate
25 27
 
26 28
 
29
+def mock_cluster_attributes(func):
30
+    def wrapper(*args, **kwargs):
31
+        attr_mock = mock.patch.object(
32
+            objects.Cluster,
33
+            'get_editable_attributes',
34
+            return_value={
35
+                'common': {
36
+                    'libvirt_type': {
37
+                        'value': consts.HYPERVISORS.kvm,
38
+                    }
39
+                }
40
+            }
41
+        )
42
+        with attr_mock:
43
+            func(*args, **kwargs)
44
+
45
+    return wrapper
46
+
47
+
27 48
 class BaseNodeAttributeValidatorTest(base.BaseTestCase):
28 49
     def setUp(self):
29 50
         super(BaseNodeAttributeValidatorTest, self).setUp()
@@ -61,16 +82,19 @@ class BaseNodeAttributeValidatorTest(base.BaseTestCase):
61 82
             }
62 83
         }
63 84
         self.node = mock.Mock(meta=meta, attributes=attributes)
85
+        self.cluster = mock.Mock()
64 86
 
65 87
 
66 88
 class TestNodeAttributesValidatorHugepages(BaseNodeAttributeValidatorTest):
67 89
 
90
+    @mock_cluster_attributes
68 91
     def test_defaults(self):
69 92
         data = {}
70 93
 
71 94
         self.assertNotRaises(errors.InvalidData, validator,
72
-                             json.dumps(data), self.node)
95
+                             json.dumps(data), self.node, self.cluster)
73 96
 
97
+    @mock_cluster_attributes
74 98
     def test_valid_hugepages(self):
75 99
         data = {
76 100
             'hugepages': {
@@ -87,8 +111,9 @@ class TestNodeAttributesValidatorHugepages(BaseNodeAttributeValidatorTest):
87 111
         }
88 112
 
89 113
         self.assertNotRaises(errors.InvalidData, validator,
90
-                             json.dumps(data), self.node)
114
+                             json.dumps(data), self.node, self.cluster)
91 115
 
116
+    @mock_cluster_attributes
92 117
     def test_too_much_hugepages(self):
93 118
         data = {
94 119
             'hugepages': {
@@ -103,8 +128,9 @@ class TestNodeAttributesValidatorHugepages(BaseNodeAttributeValidatorTest):
103 128
 
104 129
         self.assertRaisesWithMessageIn(
105 130
             errors.InvalidData, 'Not enough memory for components',
106
-            validator, json.dumps(data), self.node)
131
+            validator, json.dumps(data), self.node, self.cluster)
107 132
 
133
+    @mock_cluster_attributes
108 134
     def test_dpdk_requires_too_much(self):
109 135
         data = {
110 136
             'hugepages': {
@@ -116,10 +142,11 @@ class TestNodeAttributesValidatorHugepages(BaseNodeAttributeValidatorTest):
116 142
 
117 143
         self.assertRaisesWithMessageIn(
118 144
             errors.InvalidData, 'could not require more memory than node has',
119
-            validator, json.dumps(data), self.node)
145
+            validator, json.dumps(data), self.node, self.cluster)
120 146
 
121 147
 
122 148
 class TestNodeAttributesValidatorCpuPinning(BaseNodeAttributeValidatorTest):
149
+    @mock_cluster_attributes
123 150
     def test_valid_data(self):
124 151
         data = {
125 152
             'cpu_pinning': {
@@ -128,8 +155,9 @@ class TestNodeAttributesValidatorCpuPinning(BaseNodeAttributeValidatorTest):
128 155
         }
129 156
 
130 157
         self.assertNotRaises(errors.InvalidData, validator,
131
-                             json.dumps(data), self.node)
158
+                             json.dumps(data), self.node, self.cluster)
132 159
 
160
+    @mock_cluster_attributes
133 161
     def test_no_cpu_for_os(self):
134 162
         pinned_count = self.node.meta['cpu']['total']
135 163
 
@@ -141,8 +169,9 @@ class TestNodeAttributesValidatorCpuPinning(BaseNodeAttributeValidatorTest):
141 169
 
142 170
         self.assertRaisesWithMessageIn(
143 171
             errors.InvalidData, 'at least one cpu',
144
-            validator, json.dumps(data), self.node)
172
+            validator, json.dumps(data), self.node, self.cluster)
145 173
 
174
+    @mock_cluster_attributes
146 175
     def test_one_cpu_for_os(self):
147 176
         pinned_count = self.node.meta['cpu']['total'] - 1
148 177
 
@@ -153,4 +182,4 @@ class TestNodeAttributesValidatorCpuPinning(BaseNodeAttributeValidatorTest):
153 182
         }
154 183
 
155 184
         self.assertNotRaises(errors.InvalidData, validator,
156
-                             json.dumps(data), self.node)
185
+                             json.dumps(data), self.node, self.cluster)

+ 86
- 32
nailgun/nailgun/utils/restrictions.py View File

@@ -176,7 +176,8 @@ class RestrictionBase(object):
176 176
         :type action: string
177 177
         :param strict: disallow undefined variables in condition
178 178
         :type strict: bool
179
-        :returns: dict -- object with 'result' as bool and 'message' as dict
179
+        :returns: dict -- object with 'result' as list of satisfied
180
+                  restrictions and 'message' as dict
180 181
         """
181 182
         satisfied = []
182 183
 
@@ -197,9 +198,9 @@ class RestrictionBase(object):
197 198
                 filterd_by_action_restrictions)
198 199
 
199 200
         return {
200
-            'result': bool(satisfied),
201
-            'message': '. '.join([item.get('message') for item in
202
-                                  satisfied if item.get('message')])
201
+            'result': satisfied,
202
+            'message': '. '.join(item.get('message') for item in
203
+                                 satisfied if item.get('message'))
203 204
         }
204 205
 
205 206
     @staticmethod
@@ -235,6 +236,28 @@ class RestrictionBase(object):
235 236
 
236 237
 class AttributesRestriction(RestrictionBase):
237 238
 
239
+    @staticmethod
240
+    def _group_attribute(data):
241
+        return 'metadata' in data
242
+
243
+    @classmethod
244
+    def _get_label(cls, data):
245
+        if cls._group_attribute(data):
246
+            return data['metadata'].get('label')
247
+        return data.get('label')
248
+
249
+    @classmethod
250
+    def _get_value(cls, data):
251
+        if cls._group_attribute(data):
252
+            return data['metadata'].get('enabled', True)
253
+        return data.get('value')
254
+
255
+    @classmethod
256
+    def _get_restrictions(cls, data):
257
+        if cls._group_attribute(data):
258
+            return data['metadata'].get('restrictions', [])
259
+        return data.get('restrictions', [])
260
+
238 261
     @classmethod
239 262
     def check_data(cls, models, data):
240 263
         """Check cluster attributes data
@@ -243,42 +266,73 @@ class AttributesRestriction(RestrictionBase):
243 266
         :type models: dict
244 267
         :param data: cluster attributes object
245 268
         :type data: list|dict
246
-        :retruns: func -- generator which produces errors
269
+        :returns: list of errors
247 270
         """
248 271
         def find_errors(data=data):
249 272
             """Generator which traverses through cluster attributes tree
250 273
 
251
-            Also checks restrictions for attributes and values for correctness
252
-            with regex
274
+            Checks regex for each attribute. Check restrictions for each
275
+            enabled attribute with type 'checkbox'. All hidden attributes
276
+            are skipped as well as disabled group attrbites. Similar
277
+            logic is used on UI
253 278
             """
254
-            if isinstance(data, dict):
255
-                restr = cls.check_restrictions(
256
-                    models, data.get('restrictions', []))
257
-                if restr.get('result'):
258
-                    # TODO(apopovych): handle restriction message
259
-                    return
260
-                else:
261
-                    attr_type = data.get('type')
262
-                    if (
263
-                        attr_type == 'text_list' or
264
-                        attr_type == 'textarea_list'
265
-                    ):
266
-                        err = cls.check_fields_length(data)
267
-                        if err is not None:
268
-                            yield err
269
-
270
-                    regex_error = cls.validate_regex(data)
271
-                    if regex_error is not None:
272
-                        yield regex_error
273
-
274
-                    for key, value in six.iteritems(data):
275
-                        if key not in ['restrictions', 'regex']:
276
-                            for err in find_errors(value):
277
-                                yield err
278
-            elif isinstance(data, list):
279
+            if isinstance(data, list):
279 280
                 for item in data:
280 281
                     for err in find_errors(item):
281 282
                         yield err
283
+                return
284
+
285
+            if not isinstance(data, dict):
286
+                return
287
+
288
+            group_attribute = cls._group_attribute(data)
289
+            value = cls._get_value(data)
290
+            label = cls._get_label(data)
291
+            restrictions = cls._get_restrictions(data)
292
+
293
+            # hidden attributes should be skipped
294
+            if cls.check_restrictions(
295
+                    models, restrictions, action='hide')['result']:
296
+                return
297
+
298
+            if group_attribute and not value:
299
+                # disabled group attribute should be skipped as well
300
+                # as all sub-attributes
301
+                return
302
+
303
+            attr_type = data.get('type')
304
+            if attr_type in ['text_list', 'textarea_list']:
305
+                err = cls.check_fields_length(data)
306
+                if err is not None:
307
+                    yield err
308
+
309
+            regex_error = cls.validate_regex(data)
310
+            if regex_error is not None:
311
+                yield regex_error
312
+
313
+            # restrictions with 'disable' action should be checked only for
314
+            # enabled attributes which type is 'checkbox' or group attributes
315
+            if (attr_type == 'checkbox' or group_attribute) and value:
316
+                restr = cls.check_restrictions(
317
+                    models, restrictions, action='disable')
318
+
319
+                for item in restr['result']:
320
+                    error = (
321
+                        "restriction with action='{}' and condition="
322
+                        "'{}' failed due to attribute value='{}'"
323
+                        .format(item['action'], item['condition'], value)
324
+                    )
325
+                    if item.get('message'):
326
+                        error = item['message']
327
+
328
+                    yield ("Validation failed for attribute '{}':"
329
+                           " {}".format(label, error))
330
+
331
+            for key, value in six.iteritems(data):
332
+                if key == 'metadata':
333
+                    continue
334
+                for err in find_errors(value):
335
+                    yield err
282 336
 
283 337
         return list(find_errors())
284 338
 

Loading…
Cancel
Save