Merge lp:~tpatil/nova/add-remove-securitygroup-instance into lp:~hudson-openstack/nova/trunk
- add-remove-securitygroup-instance
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Jesse Andrews |
Approved revision: | 1428 |
Merged at revision: | 1469 |
Proposed branch: | lp:~tpatil/nova/add-remove-securitygroup-instance |
Merge into: | lp:~hudson-openstack/nova/trunk |
Diff against target: |
698 lines (+503/-21) 7 files modified
nova/api/openstack/contrib/security_groups.py (+98/-21) nova/api/openstack/create_instance_helper.py (+31/-0) nova/compute/api.py (+72/-0) nova/db/api.py (+6/-0) nova/db/sqlalchemy/api.py (+15/-0) nova/exception.py (+10/-0) nova/tests/api/openstack/contrib/test_security_groups.py (+271/-0) |
To merge this branch: | bzr merge lp:~tpatil/nova/add-remove-securitygroup-instance |
Related bugs: | |
Related blueprints: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jesse Andrews (community) | Approve | ||
Vish Ishaya (community) | Approve | ||
Review via email: mp+71967@code.launchpad.net |
Commit message
Description of the change
Added OS APIs to associate/
I will add views to return list of security groups associated with the servers later after this branch is merged into trunk. The reason I will do this later is because my previous merge proposal (https:/
Jesse Andrews (anotherjesse) wrote : | # |
We need to update https:/
Jesse Andrews (anotherjesse) wrote : | # |
As noted by bcwaldon & blamar in https:/
166 +
167 + def get_actions(self):
168 + """Return the actions the extension adds, as required by contract."""
169 + actions = [
170 + extensions.
171 + self._add_
172 + extensions.
173 + self._remove_
174 + ]
175 +
176 + return actions
Other than that LGTM
Tushar Patil (tpatil) wrote : | # |
> As noted by bcwaldon & blamar in
> https:/
> redux/+merge/71922 we should switch the add/remove to being server actions
> like:
>
> 166 +
> 167 + def get_actions(self):
> 168 + """Return the actions the extension adds, as required by
> contract."""
> 169 + actions = [
> 170 + extensions.
> "addFloatingIp",
> 171 + self._add_
> 172 + extensions.
> "removeFloatingIp",
> 173 +
> self._remove_
> 174 + ]
> 175 +
> 176 + return actions
>
> Other than that LGTM
I have implemented add/remove security groups as server actions and fixed all relevant unit tests.
Please review.
Preview Diff
1 | === modified file 'nova/api/openstack/contrib/security_groups.py' | |||
2 | --- nova/api/openstack/contrib/security_groups.py 2011-08-12 00:04:33 +0000 | |||
3 | +++ nova/api/openstack/contrib/security_groups.py 2011-08-20 22:39:52 +0000 | |||
4 | @@ -25,10 +25,11 @@ | |||
5 | 25 | from nova import exception | 25 | from nova import exception |
6 | 26 | from nova import flags | 26 | from nova import flags |
7 | 27 | from nova import log as logging | 27 | from nova import log as logging |
8 | 28 | from nova import rpc | ||
9 | 28 | from nova.api.openstack import common | 29 | from nova.api.openstack import common |
10 | 29 | from nova.api.openstack import extensions | 30 | from nova.api.openstack import extensions |
11 | 30 | from nova.api.openstack import wsgi | 31 | from nova.api.openstack import wsgi |
13 | 31 | 32 | from nova.compute import power_state | |
14 | 32 | 33 | ||
15 | 33 | from xml.dom import minidom | 34 | from xml.dom import minidom |
16 | 34 | 35 | ||
17 | @@ -73,33 +74,28 @@ | |||
18 | 73 | context, rule)] | 74 | context, rule)] |
19 | 74 | return security_group | 75 | return security_group |
20 | 75 | 76 | ||
21 | 77 | def _get_security_group(self, context, id): | ||
22 | 78 | try: | ||
23 | 79 | id = int(id) | ||
24 | 80 | security_group = db.security_group_get(context, id) | ||
25 | 81 | except ValueError: | ||
26 | 82 | msg = _("Security group id should be integer") | ||
27 | 83 | raise exc.HTTPBadRequest(explanation=msg) | ||
28 | 84 | except exception.NotFound as exp: | ||
29 | 85 | raise exc.HTTPNotFound(explanation=unicode(exp)) | ||
30 | 86 | return security_group | ||
31 | 87 | |||
32 | 76 | def show(self, req, id): | 88 | def show(self, req, id): |
33 | 77 | """Return data about the given security group.""" | 89 | """Return data about the given security group.""" |
34 | 78 | context = req.environ['nova.context'] | 90 | context = req.environ['nova.context'] |
44 | 79 | try: | 91 | security_group = self._get_security_group(context, id) |
36 | 80 | id = int(id) | ||
37 | 81 | security_group = db.security_group_get(context, id) | ||
38 | 82 | except ValueError: | ||
39 | 83 | msg = _("Security group id is not integer") | ||
40 | 84 | return exc.HTTPBadRequest(explanation=msg) | ||
41 | 85 | except exception.NotFound as exp: | ||
42 | 86 | return exc.HTTPNotFound(explanation=unicode(exp)) | ||
43 | 87 | |||
45 | 88 | return {'security_group': self._format_security_group(context, | 92 | return {'security_group': self._format_security_group(context, |
46 | 89 | security_group)} | 93 | security_group)} |
47 | 90 | 94 | ||
48 | 91 | def delete(self, req, id): | 95 | def delete(self, req, id): |
49 | 92 | """Delete a security group.""" | 96 | """Delete a security group.""" |
50 | 93 | context = req.environ['nova.context'] | 97 | context = req.environ['nova.context'] |
60 | 94 | try: | 98 | security_group = self._get_security_group(context, id) |
52 | 95 | id = int(id) | ||
53 | 96 | security_group = db.security_group_get(context, id) | ||
54 | 97 | except ValueError: | ||
55 | 98 | msg = _("Security group id is not integer") | ||
56 | 99 | return exc.HTTPBadRequest(explanation=msg) | ||
57 | 100 | except exception.SecurityGroupNotFound as exp: | ||
58 | 101 | return exc.HTTPNotFound(explanation=unicode(exp)) | ||
59 | 102 | |||
61 | 103 | LOG.audit(_("Delete security group %s"), id, context=context) | 99 | LOG.audit(_("Delete security group %s"), id, context=context) |
62 | 104 | db.security_group_destroy(context, security_group.id) | 100 | db.security_group_destroy(context, security_group.id) |
63 | 105 | 101 | ||
64 | @@ -226,9 +222,9 @@ | |||
65 | 226 | security_group_rule = db.security_group_rule_create(context, values) | 222 | security_group_rule = db.security_group_rule_create(context, values) |
66 | 227 | 223 | ||
67 | 228 | self.compute_api.trigger_security_group_rules_refresh(context, | 224 | self.compute_api.trigger_security_group_rules_refresh(context, |
69 | 229 | security_group_id=security_group['id']) | 225 | security_group_id=security_group['id']) |
70 | 230 | 226 | ||
72 | 231 | return {'security_group_rule': self._format_security_group_rule( | 227 | return {"security_group_rule": self._format_security_group_rule( |
73 | 232 | context, | 228 | context, |
74 | 233 | security_group_rule)} | 229 | security_group_rule)} |
75 | 234 | 230 | ||
76 | @@ -336,6 +332,11 @@ | |||
77 | 336 | 332 | ||
78 | 337 | 333 | ||
79 | 338 | class Security_groups(extensions.ExtensionDescriptor): | 334 | class Security_groups(extensions.ExtensionDescriptor): |
80 | 335 | |||
81 | 336 | def __init__(self): | ||
82 | 337 | self.compute_api = compute.API() | ||
83 | 338 | super(Security_groups, self).__init__() | ||
84 | 339 | |||
85 | 339 | def get_name(self): | 340 | def get_name(self): |
86 | 340 | return "SecurityGroups" | 341 | return "SecurityGroups" |
87 | 341 | 342 | ||
88 | @@ -351,6 +352,82 @@ | |||
89 | 351 | def get_updated(self): | 352 | def get_updated(self): |
90 | 352 | return "2011-07-21T00:00:00+00:00" | 353 | return "2011-07-21T00:00:00+00:00" |
91 | 353 | 354 | ||
92 | 355 | def _addSecurityGroup(self, input_dict, req, instance_id): | ||
93 | 356 | context = req.environ['nova.context'] | ||
94 | 357 | |||
95 | 358 | try: | ||
96 | 359 | body = input_dict['addSecurityGroup'] | ||
97 | 360 | group_name = body['name'] | ||
98 | 361 | instance_id = int(instance_id) | ||
99 | 362 | except ValueError: | ||
100 | 363 | msg = _("Server id should be integer") | ||
101 | 364 | raise exc.HTTPBadRequest(explanation=msg) | ||
102 | 365 | except TypeError: | ||
103 | 366 | msg = _("Missing parameter dict") | ||
104 | 367 | raise webob.exc.HTTPBadRequest(explanation=msg) | ||
105 | 368 | except KeyError: | ||
106 | 369 | msg = _("Security group not specified") | ||
107 | 370 | raise webob.exc.HTTPBadRequest(explanation=msg) | ||
108 | 371 | |||
109 | 372 | if not group_name or group_name.strip() == '': | ||
110 | 373 | msg = _("Security group name cannot be empty") | ||
111 | 374 | raise webob.exc.HTTPBadRequest(explanation=msg) | ||
112 | 375 | |||
113 | 376 | try: | ||
114 | 377 | self.compute_api.add_security_group(context, instance_id, | ||
115 | 378 | group_name) | ||
116 | 379 | except exception.SecurityGroupNotFound as exp: | ||
117 | 380 | return exc.HTTPNotFound(explanation=unicode(exp)) | ||
118 | 381 | except exception.InstanceNotFound as exp: | ||
119 | 382 | return exc.HTTPNotFound(explanation=unicode(exp)) | ||
120 | 383 | except exception.Invalid as exp: | ||
121 | 384 | return exc.HTTPBadRequest(explanation=unicode(exp)) | ||
122 | 385 | |||
123 | 386 | return exc.HTTPAccepted() | ||
124 | 387 | |||
125 | 388 | def _removeSecurityGroup(self, input_dict, req, instance_id): | ||
126 | 389 | context = req.environ['nova.context'] | ||
127 | 390 | |||
128 | 391 | try: | ||
129 | 392 | body = input_dict['removeSecurityGroup'] | ||
130 | 393 | group_name = body['name'] | ||
131 | 394 | instance_id = int(instance_id) | ||
132 | 395 | except ValueError: | ||
133 | 396 | msg = _("Server id should be integer") | ||
134 | 397 | raise exc.HTTPBadRequest(explanation=msg) | ||
135 | 398 | except TypeError: | ||
136 | 399 | msg = _("Missing parameter dict") | ||
137 | 400 | raise webob.exc.HTTPBadRequest(explanation=msg) | ||
138 | 401 | except KeyError: | ||
139 | 402 | msg = _("Security group not specified") | ||
140 | 403 | raise webob.exc.HTTPBadRequest(explanation=msg) | ||
141 | 404 | |||
142 | 405 | if not group_name or group_name.strip() == '': | ||
143 | 406 | msg = _("Security group name cannot be empty") | ||
144 | 407 | raise webob.exc.HTTPBadRequest(explanation=msg) | ||
145 | 408 | |||
146 | 409 | try: | ||
147 | 410 | self.compute_api.remove_security_group(context, instance_id, | ||
148 | 411 | group_name) | ||
149 | 412 | except exception.SecurityGroupNotFound as exp: | ||
150 | 413 | return exc.HTTPNotFound(explanation=unicode(exp)) | ||
151 | 414 | except exception.InstanceNotFound as exp: | ||
152 | 415 | return exc.HTTPNotFound(explanation=unicode(exp)) | ||
153 | 416 | except exception.Invalid as exp: | ||
154 | 417 | return exc.HTTPBadRequest(explanation=unicode(exp)) | ||
155 | 418 | |||
156 | 419 | return exc.HTTPAccepted() | ||
157 | 420 | |||
158 | 421 | def get_actions(self): | ||
159 | 422 | """Return the actions the extensions adds""" | ||
160 | 423 | actions = [ | ||
161 | 424 | extensions.ActionExtension("servers", "addSecurityGroup", | ||
162 | 425 | self._addSecurityGroup), | ||
163 | 426 | extensions.ActionExtension("servers", "removeSecurityGroup", | ||
164 | 427 | self._removeSecurityGroup) | ||
165 | 428 | ] | ||
166 | 429 | return actions | ||
167 | 430 | |||
168 | 354 | def get_resources(self): | 431 | def get_resources(self): |
169 | 355 | resources = [] | 432 | resources = [] |
170 | 356 | 433 | ||
171 | 357 | 434 | ||
172 | === modified file 'nova/api/openstack/create_instance_helper.py' | |||
173 | --- nova/api/openstack/create_instance_helper.py 2011-08-18 20:41:02 +0000 | |||
174 | +++ nova/api/openstack/create_instance_helper.py 2011-08-20 22:39:52 +0000 | |||
175 | @@ -111,6 +111,15 @@ | |||
176 | 111 | if personality: | 111 | if personality: |
177 | 112 | injected_files = self._get_injected_files(personality) | 112 | injected_files = self._get_injected_files(personality) |
178 | 113 | 113 | ||
179 | 114 | sg_names = [] | ||
180 | 115 | security_groups = server_dict.get('security_groups') | ||
181 | 116 | if security_groups is not None: | ||
182 | 117 | sg_names = [sg['name'] for sg in security_groups if sg.get('name')] | ||
183 | 118 | if not sg_names: | ||
184 | 119 | sg_names.append('default') | ||
185 | 120 | |||
186 | 121 | sg_names = list(set(sg_names)) | ||
187 | 122 | |||
188 | 114 | try: | 123 | try: |
189 | 115 | flavor_id = self.controller._flavor_id_from_req_data(body) | 124 | flavor_id = self.controller._flavor_id_from_req_data(body) |
190 | 116 | except ValueError as error: | 125 | except ValueError as error: |
191 | @@ -164,6 +173,7 @@ | |||
192 | 164 | reservation_id=reservation_id, | 173 | reservation_id=reservation_id, |
193 | 165 | min_count=min_count, | 174 | min_count=min_count, |
194 | 166 | max_count=max_count, | 175 | max_count=max_count, |
195 | 176 | security_group=sg_names, | ||
196 | 167 | user_data=user_data, | 177 | user_data=user_data, |
197 | 168 | availability_zone=availability_zone)) | 178 | availability_zone=availability_zone)) |
198 | 169 | except quota.QuotaError as error: | 179 | except quota.QuotaError as error: |
199 | @@ -174,6 +184,8 @@ | |||
200 | 174 | except exception.FlavorNotFound as error: | 184 | except exception.FlavorNotFound as error: |
201 | 175 | msg = _("Invalid flavorRef provided.") | 185 | msg = _("Invalid flavorRef provided.") |
202 | 176 | raise exc.HTTPBadRequest(explanation=msg) | 186 | raise exc.HTTPBadRequest(explanation=msg) |
203 | 187 | except exception.SecurityGroupNotFound as error: | ||
204 | 188 | raise exc.HTTPBadRequest(explanation=unicode(error)) | ||
205 | 177 | # Let the caller deal with unhandled exceptions. | 189 | # Let the caller deal with unhandled exceptions. |
206 | 178 | 190 | ||
207 | 179 | def _handle_quota_error(self, error): | 191 | def _handle_quota_error(self, error): |
208 | @@ -465,6 +477,10 @@ | |||
209 | 465 | if personality is not None: | 477 | if personality is not None: |
210 | 466 | server["personality"] = personality | 478 | server["personality"] = personality |
211 | 467 | 479 | ||
212 | 480 | security_groups = self._extract_security_groups(server_node) | ||
213 | 481 | if security_groups is not None: | ||
214 | 482 | server["security_groups"] = security_groups | ||
215 | 483 | |||
216 | 468 | return server | 484 | return server |
217 | 469 | 485 | ||
218 | 470 | def _extract_personality(self, server_node): | 486 | def _extract_personality(self, server_node): |
219 | @@ -481,3 +497,18 @@ | |||
220 | 481 | return personality | 497 | return personality |
221 | 482 | else: | 498 | else: |
222 | 483 | return None | 499 | return None |
223 | 500 | |||
224 | 501 | def _extract_security_groups(self, server_node): | ||
225 | 502 | """Marshal the security_groups attribute of a parsed request""" | ||
226 | 503 | node = self.find_first_child_named(server_node, "security_groups") | ||
227 | 504 | if node is not None: | ||
228 | 505 | security_groups = [] | ||
229 | 506 | for sg_node in self.find_children_named(node, "security_group"): | ||
230 | 507 | item = {} | ||
231 | 508 | name_node = self.find_first_child_named(sg_node, "name") | ||
232 | 509 | if name_node: | ||
233 | 510 | item["name"] = self.extract_text(name_node) | ||
234 | 511 | security_groups.append(item) | ||
235 | 512 | return security_groups | ||
236 | 513 | else: | ||
237 | 514 | return None | ||
238 | 484 | 515 | ||
239 | === modified file 'nova/compute/api.py' | |||
240 | --- nova/compute/api.py 2011-08-19 18:37:39 +0000 | |||
241 | +++ nova/compute/api.py 2011-08-20 22:39:52 +0000 | |||
242 | @@ -613,6 +613,78 @@ | |||
243 | 613 | self.db.queue_get_for(context, FLAGS.compute_topic, host), | 613 | self.db.queue_get_for(context, FLAGS.compute_topic, host), |
244 | 614 | {'method': 'refresh_provider_fw_rules', 'args': {}}) | 614 | {'method': 'refresh_provider_fw_rules', 'args': {}}) |
245 | 615 | 615 | ||
246 | 616 | def _is_security_group_associated_with_server(self, security_group, | ||
247 | 617 | instance_id): | ||
248 | 618 | """Check if the security group is already associated | ||
249 | 619 | with the instance. If Yes, return True. | ||
250 | 620 | """ | ||
251 | 621 | |||
252 | 622 | if not security_group: | ||
253 | 623 | return False | ||
254 | 624 | |||
255 | 625 | instances = security_group.get('instances') | ||
256 | 626 | if not instances: | ||
257 | 627 | return False | ||
258 | 628 | |||
259 | 629 | inst_id = None | ||
260 | 630 | for inst_id in (instance['id'] for instance in instances \ | ||
261 | 631 | if instance_id == instance['id']): | ||
262 | 632 | return True | ||
263 | 633 | |||
264 | 634 | return False | ||
265 | 635 | |||
266 | 636 | def add_security_group(self, context, instance_id, security_group_name): | ||
267 | 637 | """Add security group to the instance""" | ||
268 | 638 | security_group = db.security_group_get_by_name(context, | ||
269 | 639 | context.project_id, | ||
270 | 640 | security_group_name) | ||
271 | 641 | # check if the server exists | ||
272 | 642 | inst = db.instance_get(context, instance_id) | ||
273 | 643 | #check if the security group is associated with the server | ||
274 | 644 | if self._is_security_group_associated_with_server(security_group, | ||
275 | 645 | instance_id): | ||
276 | 646 | raise exception.SecurityGroupExistsForInstance( | ||
277 | 647 | security_group_id=security_group['id'], | ||
278 | 648 | instance_id=instance_id) | ||
279 | 649 | |||
280 | 650 | #check if the instance is in running state | ||
281 | 651 | if inst['state'] != power_state.RUNNING: | ||
282 | 652 | raise exception.InstanceNotRunning(instance_id=instance_id) | ||
283 | 653 | |||
284 | 654 | db.instance_add_security_group(context.elevated(), | ||
285 | 655 | instance_id, | ||
286 | 656 | security_group['id']) | ||
287 | 657 | rpc.cast(context, | ||
288 | 658 | db.queue_get_for(context, FLAGS.compute_topic, inst['host']), | ||
289 | 659 | {"method": "refresh_security_group_rules", | ||
290 | 660 | "args": {"security_group_id": security_group['id']}}) | ||
291 | 661 | |||
292 | 662 | def remove_security_group(self, context, instance_id, security_group_name): | ||
293 | 663 | """Remove the security group associated with the instance""" | ||
294 | 664 | security_group = db.security_group_get_by_name(context, | ||
295 | 665 | context.project_id, | ||
296 | 666 | security_group_name) | ||
297 | 667 | # check if the server exists | ||
298 | 668 | inst = db.instance_get(context, instance_id) | ||
299 | 669 | #check if the security group is associated with the server | ||
300 | 670 | if not self._is_security_group_associated_with_server(security_group, | ||
301 | 671 | instance_id): | ||
302 | 672 | raise exception.SecurityGroupNotExistsForInstance( | ||
303 | 673 | security_group_id=security_group['id'], | ||
304 | 674 | instance_id=instance_id) | ||
305 | 675 | |||
306 | 676 | #check if the instance is in running state | ||
307 | 677 | if inst['state'] != power_state.RUNNING: | ||
308 | 678 | raise exception.InstanceNotRunning(instance_id=instance_id) | ||
309 | 679 | |||
310 | 680 | db.instance_remove_security_group(context.elevated(), | ||
311 | 681 | instance_id, | ||
312 | 682 | security_group['id']) | ||
313 | 683 | rpc.cast(context, | ||
314 | 684 | db.queue_get_for(context, FLAGS.compute_topic, inst['host']), | ||
315 | 685 | {"method": "refresh_security_group_rules", | ||
316 | 686 | "args": {"security_group_id": security_group['id']}}) | ||
317 | 687 | |||
318 | 616 | @scheduler_api.reroute_compute("update") | 688 | @scheduler_api.reroute_compute("update") |
319 | 617 | def update(self, context, instance_id, **kwargs): | 689 | def update(self, context, instance_id, **kwargs): |
320 | 618 | """Updates the instance in the datastore. | 690 | """Updates the instance in the datastore. |
321 | 619 | 691 | ||
322 | === modified file 'nova/db/api.py' | |||
323 | --- nova/db/api.py 2011-08-12 19:00:48 +0000 | |||
324 | +++ nova/db/api.py 2011-08-20 22:39:52 +0000 | |||
325 | @@ -570,6 +570,12 @@ | |||
326 | 570 | security_group_id) | 570 | security_group_id) |
327 | 571 | 571 | ||
328 | 572 | 572 | ||
329 | 573 | def instance_remove_security_group(context, instance_id, security_group_id): | ||
330 | 574 | """Disassociate the given security group from the given instance.""" | ||
331 | 575 | return IMPL.instance_remove_security_group(context, instance_id, | ||
332 | 576 | security_group_id) | ||
333 | 577 | |||
334 | 578 | |||
335 | 573 | def instance_action_create(context, values): | 579 | def instance_action_create(context, values): |
336 | 574 | """Create an instance action from the values dictionary.""" | 580 | """Create an instance action from the values dictionary.""" |
337 | 575 | return IMPL.instance_action_create(context, values) | 581 | return IMPL.instance_action_create(context, values) |
338 | 576 | 582 | ||
339 | === modified file 'nova/db/sqlalchemy/api.py' | |||
340 | --- nova/db/sqlalchemy/api.py 2011-08-18 21:57:52 +0000 | |||
341 | +++ nova/db/sqlalchemy/api.py 2011-08-20 22:39:52 +0000 | |||
342 | @@ -1502,6 +1502,19 @@ | |||
343 | 1502 | 1502 | ||
344 | 1503 | 1503 | ||
345 | 1504 | @require_context | 1504 | @require_context |
346 | 1505 | def instance_remove_security_group(context, instance_id, security_group_id): | ||
347 | 1506 | """Disassociate the given security group from the given instance""" | ||
348 | 1507 | session = get_session() | ||
349 | 1508 | |||
350 | 1509 | session.query(models.SecurityGroupInstanceAssociation).\ | ||
351 | 1510 | filter_by(instance_id=instance_id).\ | ||
352 | 1511 | filter_by(security_group_id=security_group_id).\ | ||
353 | 1512 | update({'deleted': True, | ||
354 | 1513 | 'deleted_at': utils.utcnow(), | ||
355 | 1514 | 'updated_at': literal_column('updated_at')}) | ||
356 | 1515 | |||
357 | 1516 | |||
358 | 1517 | @require_context | ||
359 | 1505 | def instance_action_create(context, values): | 1518 | def instance_action_create(context, values): |
360 | 1506 | """Create an instance action from the values dictionary.""" | 1519 | """Create an instance action from the values dictionary.""" |
361 | 1507 | action_ref = models.InstanceActions() | 1520 | action_ref = models.InstanceActions() |
362 | @@ -2437,6 +2450,7 @@ | |||
363 | 2437 | filter_by(deleted=can_read_deleted(context),).\ | 2450 | filter_by(deleted=can_read_deleted(context),).\ |
364 | 2438 | filter_by(id=security_group_id).\ | 2451 | filter_by(id=security_group_id).\ |
365 | 2439 | options(joinedload_all('rules')).\ | 2452 | options(joinedload_all('rules')).\ |
366 | 2453 | options(joinedload_all('instances')).\ | ||
367 | 2440 | first() | 2454 | first() |
368 | 2441 | else: | 2455 | else: |
369 | 2442 | result = session.query(models.SecurityGroup).\ | 2456 | result = session.query(models.SecurityGroup).\ |
370 | @@ -2444,6 +2458,7 @@ | |||
371 | 2444 | filter_by(id=security_group_id).\ | 2458 | filter_by(id=security_group_id).\ |
372 | 2445 | filter_by(project_id=context.project_id).\ | 2459 | filter_by(project_id=context.project_id).\ |
373 | 2446 | options(joinedload_all('rules')).\ | 2460 | options(joinedload_all('rules')).\ |
374 | 2461 | options(joinedload_all('instances')).\ | ||
375 | 2447 | first() | 2462 | first() |
376 | 2448 | if not result: | 2463 | if not result: |
377 | 2449 | raise exception.SecurityGroupNotFound( | 2464 | raise exception.SecurityGroupNotFound( |
378 | 2450 | 2465 | ||
379 | === modified file 'nova/exception.py' | |||
380 | --- nova/exception.py 2011-08-16 12:47:35 +0000 | |||
381 | +++ nova/exception.py 2011-08-20 22:39:52 +0000 | |||
382 | @@ -541,6 +541,16 @@ | |||
383 | 541 | message = _("Security group with rule %(rule_id)s not found.") | 541 | message = _("Security group with rule %(rule_id)s not found.") |
384 | 542 | 542 | ||
385 | 543 | 543 | ||
386 | 544 | class SecurityGroupExistsForInstance(Invalid): | ||
387 | 545 | message = _("Security group %(security_group_id)s is already associated" | ||
388 | 546 | " with the instance %(instance_id)s") | ||
389 | 547 | |||
390 | 548 | |||
391 | 549 | class SecurityGroupNotExistsForInstance(Invalid): | ||
392 | 550 | message = _("Security group %(security_group_id)s is not associated with" | ||
393 | 551 | " the instance %(instance_id)s") | ||
394 | 552 | |||
395 | 553 | |||
396 | 544 | class MigrationNotFound(NotFound): | 554 | class MigrationNotFound(NotFound): |
397 | 545 | message = _("Migration %(migration_id)s could not be found.") | 555 | message = _("Migration %(migration_id)s could not be found.") |
398 | 546 | 556 | ||
399 | 547 | 557 | ||
400 | === modified file 'nova/tests/api/openstack/contrib/test_security_groups.py' | |||
401 | --- nova/tests/api/openstack/contrib/test_security_groups.py 2011-08-12 00:04:33 +0000 | |||
402 | +++ nova/tests/api/openstack/contrib/test_security_groups.py 2011-08-20 22:39:52 +0000 | |||
403 | @@ -15,10 +15,13 @@ | |||
404 | 15 | # under the License. | 15 | # under the License. |
405 | 16 | 16 | ||
406 | 17 | import json | 17 | import json |
407 | 18 | import mox | ||
408 | 19 | import nova | ||
409 | 18 | import unittest | 20 | import unittest |
410 | 19 | import webob | 21 | import webob |
411 | 20 | from xml.dom import minidom | 22 | from xml.dom import minidom |
412 | 21 | 23 | ||
413 | 24 | from nova import exception | ||
414 | 22 | from nova import test | 25 | from nova import test |
415 | 23 | from nova.api.openstack.contrib import security_groups | 26 | from nova.api.openstack.contrib import security_groups |
416 | 24 | from nova.tests.api.openstack import fakes | 27 | from nova.tests.api.openstack import fakes |
417 | @@ -51,6 +54,28 @@ | |||
418 | 51 | return {'security_group': sg} | 54 | return {'security_group': sg} |
419 | 52 | 55 | ||
420 | 53 | 56 | ||
421 | 57 | def return_server(context, server_id): | ||
422 | 58 | return {'id': server_id, 'state': 0x01, 'host': "localhost"} | ||
423 | 59 | |||
424 | 60 | |||
425 | 61 | def return_non_running_server(context, server_id): | ||
426 | 62 | return {'id': server_id, 'state': 0x02, | ||
427 | 63 | 'host': "localhost"} | ||
428 | 64 | |||
429 | 65 | |||
430 | 66 | def return_security_group(context, project_id, group_name): | ||
431 | 67 | return {'id': 1, 'name': group_name, "instances": [ | ||
432 | 68 | {'id': 1}]} | ||
433 | 69 | |||
434 | 70 | |||
435 | 71 | def return_security_group_without_instances(context, project_id, group_name): | ||
436 | 72 | return {'id': 1, 'name': group_name} | ||
437 | 73 | |||
438 | 74 | |||
439 | 75 | def return_server_nonexistant(context, server_id): | ||
440 | 76 | raise exception.InstanceNotFound(instance_id=server_id) | ||
441 | 77 | |||
442 | 78 | |||
443 | 54 | class TestSecurityGroups(test.TestCase): | 79 | class TestSecurityGroups(test.TestCase): |
444 | 55 | def setUp(self): | 80 | def setUp(self): |
445 | 56 | super(TestSecurityGroups, self).setUp() | 81 | super(TestSecurityGroups, self).setUp() |
446 | @@ -325,6 +350,252 @@ | |||
447 | 325 | response = self._delete_security_group(11111111) | 350 | response = self._delete_security_group(11111111) |
448 | 326 | self.assertEquals(response.status_int, 404) | 351 | self.assertEquals(response.status_int, 404) |
449 | 327 | 352 | ||
450 | 353 | def test_associate_by_non_existing_security_group_name(self): | ||
451 | 354 | body = dict(addSecurityGroup=dict(name='non-existing')) | ||
452 | 355 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
453 | 356 | req.headers['Content-Type'] = 'application/json' | ||
454 | 357 | req.method = 'POST' | ||
455 | 358 | req.body = json.dumps(body) | ||
456 | 359 | response = req.get_response(fakes.wsgi_app()) | ||
457 | 360 | self.assertEquals(response.status_int, 404) | ||
458 | 361 | |||
459 | 362 | def test_associate_by_invalid_server_id(self): | ||
460 | 363 | body = dict(addSecurityGroup=dict(name='test')) | ||
461 | 364 | self.stubs.Set(nova.db, 'security_group_get_by_name', | ||
462 | 365 | return_security_group) | ||
463 | 366 | req = webob.Request.blank('/v1.1/servers/invalid/action') | ||
464 | 367 | req.headers['Content-Type'] = 'application/json' | ||
465 | 368 | req.method = 'POST' | ||
466 | 369 | req.body = json.dumps(body) | ||
467 | 370 | response = req.get_response(fakes.wsgi_app()) | ||
468 | 371 | self.assertEquals(response.status_int, 400) | ||
469 | 372 | |||
470 | 373 | def test_associate_without_body(self): | ||
471 | 374 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
472 | 375 | body = dict(addSecurityGroup=None) | ||
473 | 376 | self.stubs.Set(nova.db, 'instance_get', return_server) | ||
474 | 377 | req.headers['Content-Type'] = 'application/json' | ||
475 | 378 | req.method = 'POST' | ||
476 | 379 | req.body = json.dumps(body) | ||
477 | 380 | response = req.get_response(fakes.wsgi_app()) | ||
478 | 381 | self.assertEquals(response.status_int, 400) | ||
479 | 382 | |||
480 | 383 | def test_associate_no_security_group_name(self): | ||
481 | 384 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
482 | 385 | body = dict(addSecurityGroup=dict()) | ||
483 | 386 | self.stubs.Set(nova.db, 'instance_get', return_server) | ||
484 | 387 | req.headers['Content-Type'] = 'application/json' | ||
485 | 388 | req.method = 'POST' | ||
486 | 389 | req.body = json.dumps(body) | ||
487 | 390 | response = req.get_response(fakes.wsgi_app()) | ||
488 | 391 | self.assertEquals(response.status_int, 400) | ||
489 | 392 | |||
490 | 393 | def test_associate_security_group_name_with_whitespaces(self): | ||
491 | 394 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
492 | 395 | body = dict(addSecurityGroup=dict(name=" ")) | ||
493 | 396 | self.stubs.Set(nova.db, 'instance_get', return_server) | ||
494 | 397 | req.headers['Content-Type'] = 'application/json' | ||
495 | 398 | req.method = 'POST' | ||
496 | 399 | req.body = json.dumps(body) | ||
497 | 400 | response = req.get_response(fakes.wsgi_app()) | ||
498 | 401 | self.assertEquals(response.status_int, 400) | ||
499 | 402 | |||
500 | 403 | def test_associate_non_existing_instance(self): | ||
501 | 404 | self.stubs.Set(nova.db, 'instance_get', return_server_nonexistant) | ||
502 | 405 | body = dict(addSecurityGroup=dict(name="test")) | ||
503 | 406 | self.stubs.Set(nova.db, 'security_group_get_by_name', | ||
504 | 407 | return_security_group) | ||
505 | 408 | req = webob.Request.blank('/v1.1/servers/10000/action') | ||
506 | 409 | req.headers['Content-Type'] = 'application/json' | ||
507 | 410 | req.method = 'POST' | ||
508 | 411 | req.body = json.dumps(body) | ||
509 | 412 | response = req.get_response(fakes.wsgi_app()) | ||
510 | 413 | self.assertEquals(response.status_int, 404) | ||
511 | 414 | |||
512 | 415 | def test_associate_non_running_instance(self): | ||
513 | 416 | self.stubs.Set(nova.db, 'instance_get', return_non_running_server) | ||
514 | 417 | self.stubs.Set(nova.db, 'security_group_get_by_name', | ||
515 | 418 | return_security_group_without_instances) | ||
516 | 419 | body = dict(addSecurityGroup=dict(name="test")) | ||
517 | 420 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
518 | 421 | req.headers['Content-Type'] = 'application/json' | ||
519 | 422 | req.method = 'POST' | ||
520 | 423 | req.body = json.dumps(body) | ||
521 | 424 | response = req.get_response(fakes.wsgi_app()) | ||
522 | 425 | self.assertEquals(response.status_int, 400) | ||
523 | 426 | |||
524 | 427 | def test_associate_already_associated_security_group_to_instance(self): | ||
525 | 428 | self.stubs.Set(nova.db, 'instance_get', return_server) | ||
526 | 429 | self.stubs.Set(nova.db, 'security_group_get_by_name', | ||
527 | 430 | return_security_group) | ||
528 | 431 | body = dict(addSecurityGroup=dict(name="test")) | ||
529 | 432 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
530 | 433 | req.headers['Content-Type'] = 'application/json' | ||
531 | 434 | req.method = 'POST' | ||
532 | 435 | req.body = json.dumps(body) | ||
533 | 436 | response = req.get_response(fakes.wsgi_app()) | ||
534 | 437 | self.assertEquals(response.status_int, 400) | ||
535 | 438 | |||
536 | 439 | def test_associate(self): | ||
537 | 440 | self.stubs.Set(nova.db, 'instance_get', return_server) | ||
538 | 441 | self.mox.StubOutWithMock(nova.db, 'instance_add_security_group') | ||
539 | 442 | nova.db.instance_add_security_group(mox.IgnoreArg(), | ||
540 | 443 | mox.IgnoreArg(), | ||
541 | 444 | mox.IgnoreArg()) | ||
542 | 445 | self.stubs.Set(nova.db, 'security_group_get_by_name', | ||
543 | 446 | return_security_group_without_instances) | ||
544 | 447 | self.mox.ReplayAll() | ||
545 | 448 | |||
546 | 449 | body = dict(addSecurityGroup=dict(name="test")) | ||
547 | 450 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
548 | 451 | req.headers['Content-Type'] = 'application/json' | ||
549 | 452 | req.method = 'POST' | ||
550 | 453 | req.body = json.dumps(body) | ||
551 | 454 | response = req.get_response(fakes.wsgi_app()) | ||
552 | 455 | self.assertEquals(response.status_int, 202) | ||
553 | 456 | |||
554 | 457 | def test_associate_xml(self): | ||
555 | 458 | self.stubs.Set(nova.db, 'instance_get', return_server) | ||
556 | 459 | self.mox.StubOutWithMock(nova.db, 'instance_add_security_group') | ||
557 | 460 | nova.db.instance_add_security_group(mox.IgnoreArg(), | ||
558 | 461 | mox.IgnoreArg(), | ||
559 | 462 | mox.IgnoreArg()) | ||
560 | 463 | self.stubs.Set(nova.db, 'security_group_get_by_name', | ||
561 | 464 | return_security_group_without_instances) | ||
562 | 465 | self.mox.ReplayAll() | ||
563 | 466 | |||
564 | 467 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
565 | 468 | req.headers['Content-Type'] = 'application/xml' | ||
566 | 469 | req.method = 'POST' | ||
567 | 470 | req.body = """<addSecurityGroup> | ||
568 | 471 | <name>test</name> | ||
569 | 472 | </addSecurityGroup>""" | ||
570 | 473 | response = req.get_response(fakes.wsgi_app()) | ||
571 | 474 | self.assertEquals(response.status_int, 202) | ||
572 | 475 | |||
573 | 476 | def test_disassociate_by_non_existing_security_group_name(self): | ||
574 | 477 | body = dict(removeSecurityGroup=dict(name='non-existing')) | ||
575 | 478 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
576 | 479 | req.headers['Content-Type'] = 'application/json' | ||
577 | 480 | req.method = 'POST' | ||
578 | 481 | req.body = json.dumps(body) | ||
579 | 482 | response = req.get_response(fakes.wsgi_app()) | ||
580 | 483 | self.assertEquals(response.status_int, 404) | ||
581 | 484 | |||
582 | 485 | def test_disassociate_by_invalid_server_id(self): | ||
583 | 486 | body = dict(removeSecurityGroup=dict(name='test')) | ||
584 | 487 | self.stubs.Set(nova.db, 'security_group_get_by_name', | ||
585 | 488 | return_security_group) | ||
586 | 489 | req = webob.Request.blank('/v1.1/servers/invalid/action') | ||
587 | 490 | req.headers['Content-Type'] = 'application/json' | ||
588 | 491 | req.method = 'POST' | ||
589 | 492 | req.body = json.dumps(body) | ||
590 | 493 | response = req.get_response(fakes.wsgi_app()) | ||
591 | 494 | self.assertEquals(response.status_int, 400) | ||
592 | 495 | |||
593 | 496 | def test_disassociate_without_body(self): | ||
594 | 497 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
595 | 498 | body = dict(removeSecurityGroup=None) | ||
596 | 499 | self.stubs.Set(nova.db, 'instance_get', return_server) | ||
597 | 500 | req.headers['Content-Type'] = 'application/json' | ||
598 | 501 | req.method = 'POST' | ||
599 | 502 | req.body = json.dumps(body) | ||
600 | 503 | response = req.get_response(fakes.wsgi_app()) | ||
601 | 504 | self.assertEquals(response.status_int, 400) | ||
602 | 505 | |||
603 | 506 | def test_disassociate_no_security_group_name(self): | ||
604 | 507 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
605 | 508 | body = dict(removeSecurityGroup=dict()) | ||
606 | 509 | self.stubs.Set(nova.db, 'instance_get', return_server) | ||
607 | 510 | req.headers['Content-Type'] = 'application/json' | ||
608 | 511 | req.method = 'POST' | ||
609 | 512 | req.body = json.dumps(body) | ||
610 | 513 | response = req.get_response(fakes.wsgi_app()) | ||
611 | 514 | self.assertEquals(response.status_int, 400) | ||
612 | 515 | |||
613 | 516 | def test_disassociate_security_group_name_with_whitespaces(self): | ||
614 | 517 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
615 | 518 | body = dict(removeSecurityGroup=dict(name=" ")) | ||
616 | 519 | self.stubs.Set(nova.db, 'instance_get', return_server) | ||
617 | 520 | req.headers['Content-Type'] = 'application/json' | ||
618 | 521 | req.method = 'POST' | ||
619 | 522 | req.body = json.dumps(body) | ||
620 | 523 | response = req.get_response(fakes.wsgi_app()) | ||
621 | 524 | self.assertEquals(response.status_int, 400) | ||
622 | 525 | |||
623 | 526 | def test_disassociate_non_existing_instance(self): | ||
624 | 527 | self.stubs.Set(nova.db, 'instance_get', return_server_nonexistant) | ||
625 | 528 | body = dict(removeSecurityGroup=dict(name="test")) | ||
626 | 529 | self.stubs.Set(nova.db, 'security_group_get_by_name', | ||
627 | 530 | return_security_group) | ||
628 | 531 | req = webob.Request.blank('/v1.1/servers/10000/action') | ||
629 | 532 | req.headers['Content-Type'] = 'application/json' | ||
630 | 533 | req.method = 'POST' | ||
631 | 534 | req.body = json.dumps(body) | ||
632 | 535 | response = req.get_response(fakes.wsgi_app()) | ||
633 | 536 | self.assertEquals(response.status_int, 404) | ||
634 | 537 | |||
635 | 538 | def test_disassociate_non_running_instance(self): | ||
636 | 539 | self.stubs.Set(nova.db, 'instance_get', return_non_running_server) | ||
637 | 540 | self.stubs.Set(nova.db, 'security_group_get_by_name', | ||
638 | 541 | return_security_group) | ||
639 | 542 | body = dict(removeSecurityGroup=dict(name="test")) | ||
640 | 543 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
641 | 544 | req.headers['Content-Type'] = 'application/json' | ||
642 | 545 | req.method = 'POST' | ||
643 | 546 | req.body = json.dumps(body) | ||
644 | 547 | response = req.get_response(fakes.wsgi_app()) | ||
645 | 548 | self.assertEquals(response.status_int, 400) | ||
646 | 549 | |||
647 | 550 | def test_disassociate_already_associated_security_group_to_instance(self): | ||
648 | 551 | self.stubs.Set(nova.db, 'instance_get', return_server) | ||
649 | 552 | self.stubs.Set(nova.db, 'security_group_get_by_name', | ||
650 | 553 | return_security_group_without_instances) | ||
651 | 554 | body = dict(removeSecurityGroup=dict(name="test")) | ||
652 | 555 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
653 | 556 | req.headers['Content-Type'] = 'application/json' | ||
654 | 557 | req.method = 'POST' | ||
655 | 558 | req.body = json.dumps(body) | ||
656 | 559 | response = req.get_response(fakes.wsgi_app()) | ||
657 | 560 | self.assertEquals(response.status_int, 400) | ||
658 | 561 | |||
659 | 562 | def test_disassociate(self): | ||
660 | 563 | self.stubs.Set(nova.db, 'instance_get', return_server) | ||
661 | 564 | self.mox.StubOutWithMock(nova.db, 'instance_remove_security_group') | ||
662 | 565 | nova.db.instance_remove_security_group(mox.IgnoreArg(), | ||
663 | 566 | mox.IgnoreArg(), | ||
664 | 567 | mox.IgnoreArg()) | ||
665 | 568 | self.stubs.Set(nova.db, 'security_group_get_by_name', | ||
666 | 569 | return_security_group) | ||
667 | 570 | self.mox.ReplayAll() | ||
668 | 571 | |||
669 | 572 | body = dict(removeSecurityGroup=dict(name="test")) | ||
670 | 573 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
671 | 574 | req.headers['Content-Type'] = 'application/json' | ||
672 | 575 | req.method = 'POST' | ||
673 | 576 | req.body = json.dumps(body) | ||
674 | 577 | response = req.get_response(fakes.wsgi_app()) | ||
675 | 578 | self.assertEquals(response.status_int, 202) | ||
676 | 579 | |||
677 | 580 | def test_disassociate_xml(self): | ||
678 | 581 | self.stubs.Set(nova.db, 'instance_get', return_server) | ||
679 | 582 | self.mox.StubOutWithMock(nova.db, 'instance_remove_security_group') | ||
680 | 583 | nova.db.instance_remove_security_group(mox.IgnoreArg(), | ||
681 | 584 | mox.IgnoreArg(), | ||
682 | 585 | mox.IgnoreArg()) | ||
683 | 586 | self.stubs.Set(nova.db, 'security_group_get_by_name', | ||
684 | 587 | return_security_group) | ||
685 | 588 | self.mox.ReplayAll() | ||
686 | 589 | |||
687 | 590 | req = webob.Request.blank('/v1.1/servers/1/action') | ||
688 | 591 | req.headers['Content-Type'] = 'application/xml' | ||
689 | 592 | req.method = 'POST' | ||
690 | 593 | req.body = """<removeSecurityGroup> | ||
691 | 594 | <name>test</name> | ||
692 | 595 | </removeSecurityGroup>""" | ||
693 | 596 | response = req.get_response(fakes.wsgi_app()) | ||
694 | 597 | self.assertEquals(response.status_int, 202) | ||
695 | 598 | |||
696 | 328 | 599 | ||
697 | 329 | class TestSecurityGroupRules(test.TestCase): | 600 | class TestSecurityGroupRules(test.TestCase): |
698 | 330 | def setUp(self): | 601 | def setUp(self): |
Awesome stuff.
Nice tests.