OpenStack Compute (nova)

Merge lp:~cbehrens/nova/lp855115 into lp:~hudson-openstack/nova/trunk

  1. lp855115
  2. Merge into trunk
Proposed by Chris Behrens
Status: Merged
Approved by: Vish Ishaya
Approved revision: 1546
Merged at revision: 1612
Proposed branch: lp:~cbehrens/nova/lp855115
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 175 lines (+68/-12)
5 files modified
nova/api/ec2/__init__.py (+4/-0)
nova/api/openstack/contrib/floating_ips.py (+6/-1)
nova/db/sqlalchemy/api.py (+11/-10)
nova/tests/api/openstack/contrib/test_floating_ips.py (+1/-1)
nova/tests/test_network.py (+46/-0)
To merge this branch: bzr merge lp:~cbehrens/nova/lp855115
Reviewer Review Type Date Requested Status
Vish Ishaya (community) Approve
Paul Voccio (community) Approve
Kevin L. Mitchell (community) Approve
Review via email: mp+76475@code.launchpad.net

Description of the change

Fixes lp:855115 -- issue with disassociating floating ips.

To post a comment you must log in.
Revision history for this message
Kevin L. Mitchell (klmitch) wrote :

Looks OK to me...

review: Approve
Revision history for this message
Paul Voccio (pvo) wrote :

lgtm

review: Approve
Revision history for this message
Vish Ishaya (vishvananda) wrote :

awesome, lgtm

review: Approve
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :
Download full text (129.5 KiB)

The attempt to merge lp:~cbehrens/nova/lp855115 into lp:nova failed. Below is the output from the failed tests.

CloudTestCase
    test_ajax_console OK 1.75
    test_allocate_address OK 0.26
    test_associate_disassociate_address OK 1.60
    test_authorize_revoke_security_group_ingress_by_id OK 0.33
    test_authorize_security_group_fail_missing_source_group OK 0.25
    test_authorize_security_group_ingress OK 0.26
    test_authorize_security_group_ingress_already_exists OK 0.32
    test_authorize_security_group_ingress_ip_permissions_groups OK 0.32
    test_authorize_security_group_ingress_ip_permissions_ip_rangesOK 0.27
    test_authorize_security_group_ingress_missing_group_name_or_idOK 0.18
    test_authorize_security_group_ingress_missing_protocol_paramsOK 0.39
    test_console_output OK 1.23
    test_create_delete_security_group OK 0.26
    test_create_image OK 4.05
    test_create_snapshot OK 0.44
    test_create_volume_from_snapshot OK 0.46
    test_delete_key_pair OK 0.39
    test_delete_security_group_by_id OK 0.23
    test_delete_security_group_no_params OK 0.20
    test_delete_security_group_with_bad_group_id OK 0.22
    test_delete_security_group_with_bad_name OK 0.22
    test_delete_snapshot OK 0.43
    test_deregister_image OK 0.19
    test_deregister_image_wrong_container_type OK 0.21
    test_describe_addresses OK 0.49
    test_describe_availability_zones OK 0.23
    test_describe_image_attribute OK 0.18
    test_describe_image_attribute_block_device_mapping OK 0.19
    test_describe_image_attribute_root_device_name OK 0.18
    test_describe_image_mapping OK 0.19
    test_describe_images OK 0.19
    test_describe_instance_attribute OK 0.18
    test_describe_instances OK 0.35
    test_describe_instances_bdm OK 1.39
    test_describe_instances_deleted OK 0.22
    test_describe_key_pairs OK 0.57
    test_describe_regions OK 0.19
    test_describe_security_group_ingress_groups OK 0.43
    test_describe_security_groups OK 0.27
    test_describe_security_groups_by_id OK 0.32
    test_describe_snapshots OK 0.23
    test_describe_volumes OK 0.25
  ...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'nova/api/ec2/__init__.py'
2--- nova/api/ec2/__init__.py 2011-09-20 22:52:03 +0000
3+++ nova/api/ec2/__init__.py 2011-09-21 20:25:31 +0000
4@@ -386,6 +386,10 @@
5 LOG.debug(_('InvalidPortRange raised: %s'), unicode(ex),
6 context=context)
7 return self._error(req, context, type(ex).__name__, unicode(ex))
8+ except exception.NotAuthorized as ex:
9+ LOG.info(_('NotAuthorized raised: %s'), unicode(ex),
10+ context=context)
11+ return self._error(req, context, type(ex).__name__, unicode(ex))
12 except Exception as ex:
13 extra = {'environment': req.environ}
14 LOG.exception(_('Unexpected error raised: %s'), unicode(ex),
15
16=== modified file 'nova/api/openstack/contrib/floating_ips.py'
17--- nova/api/openstack/contrib/floating_ips.py 2011-09-08 20:18:08 +0000
18+++ nova/api/openstack/contrib/floating_ips.py 2011-09-21 20:25:31 +0000
19@@ -144,6 +144,8 @@
20 address)
21 except exception.ApiError, e:
22 raise webob.exc.HTTPBadRequest(explanation=e.message)
23+ except exception.NotAuthorized, e:
24+ raise webob.exc.HTTPUnauthorized()
25
26 return webob.Response(status_int=202)
27
28@@ -162,7 +164,10 @@
29
30 floating_ip = self.network_api.get_floating_ip_by_ip(context, address)
31 if floating_ip.get('fixed_ip'):
32- self.network_api.disassociate_floating_ip(context, address)
33+ try:
34+ self.network_api.disassociate_floating_ip(context, address)
35+ except exception.NotAuthorized, e:
36+ raise webob.exc.HTTPUnauthorized()
37
38 return webob.Response(status_int=202)
39
40
41=== modified file 'nova/db/sqlalchemy/api.py'
42--- nova/db/sqlalchemy/api.py 2011-09-19 22:32:45 +0000
43+++ nova/db/sqlalchemy/api.py 2011-09-21 20:25:31 +0000
44@@ -534,7 +534,6 @@
45 fixed_address, host):
46 session = get_session()
47 with session.begin():
48- # TODO(devcamcar): How to ensure floating_id belongs to user?
49 floating_ip_ref = floating_ip_get_by_address(context,
50 floating_address,
51 session=session)
52@@ -550,7 +549,6 @@
53 def floating_ip_deallocate(context, address):
54 session = get_session()
55 with session.begin():
56- # TODO(devcamcar): How to ensure floating id belongs to user?
57 floating_ip_ref = floating_ip_get_by_address(context,
58 address,
59 session=session)
60@@ -564,7 +562,6 @@
61 def floating_ip_destroy(context, address):
62 session = get_session()
63 with session.begin():
64- # TODO(devcamcar): Ensure address belongs to user.
65 floating_ip_ref = floating_ip_get_by_address(context,
66 address,
67 session=session)
68@@ -575,8 +572,6 @@
69 def floating_ip_disassociate(context, address):
70 session = get_session()
71 with session.begin():
72- # TODO(devcamcar): Ensure address belongs to user.
73- # Does get_floating_ip_by_address handle this?
74 floating_ip_ref = floating_ip_get_by_address(context,
75 address,
76 session=session)
77@@ -645,17 +640,23 @@
78
79 @require_context
80 def floating_ip_get_by_address(context, address, session=None):
81- # TODO(devcamcar): Ensure the address belongs to user.
82 if not session:
83 session = get_session()
84
85 result = session.query(models.FloatingIp).\
86- options(joinedload_all('fixed_ip.network')).\
87- filter_by(address=address).\
88- filter_by(deleted=can_read_deleted(context)).\
89- first()
90+ options(joinedload_all('fixed_ip.network')).\
91+ filter_by(address=address).\
92+ filter_by(deleted=can_read_deleted(context)).\
93+ first()
94+
95 if not result:
96 raise exception.FloatingIpNotFoundForAddress(address=address)
97+
98+ # If the floating IP has a project ID set, check to make sure
99+ # the non-admin user has access.
100+ if result.project_id and is_user_context(context):
101+ authorize_project_context(context, result.project_id)
102+
103 return result
104
105
106
107=== modified file 'nova/tests/api/openstack/contrib/test_floating_ips.py'
108--- nova/tests/api/openstack/contrib/test_floating_ips.py 2011-09-07 23:38:41 +0000
109+++ nova/tests/api/openstack/contrib/test_floating_ips.py 2011-09-21 20:25:31 +0000
110@@ -278,7 +278,7 @@
111 req.body = json.dumps(body)
112 req.headers["content-type"] = "application/json"
113 resp = req.get_response(fakes.wsgi_app())
114- self.assertEqual(resp.status_int, 400)
115+ self.assertEqual(resp.status_int, 401)
116
117 def test_associate_floating_ip_to_instance_no_project_id(self):
118 def fake_fixed_ip_get_by_address(ctx, address, session=None):
119
120=== modified file 'nova/tests/test_network.py'
121--- nova/tests/test_network.py 2011-09-15 19:25:52 +0000
122+++ nova/tests/test_network.py 2011-09-21 20:25:31 +0000
123@@ -436,6 +436,52 @@
124 self.network.add_fixed_ip_to_instance(self.context, 1, HOST,
125 networks[0]['id'])
126
127+ def test_ip_association_and_allocation_of_other_project(self):
128+ """Makes sure that we cannot deallocaate or disassociate
129+ a public ip of other project"""
130+
131+ context1 = context.RequestContext('user', 'project1')
132+ context2 = context.RequestContext('user', 'project2')
133+
134+ address = '1.2.3.4'
135+ float_addr = db.floating_ip_create(context1.elevated(),
136+ {'address': address,
137+ 'project_id': context1.project_id})
138+
139+ instance = db.instance_create(context1,
140+ {'project_id': 'project1'})
141+
142+ fix_addr = db.fixed_ip_associate_pool(context1.elevated(),
143+ 1, instance['id'])
144+
145+ # Associate the IP with non-admin user context
146+ self.assertRaises(exception.NotAuthorized,
147+ self.network.associate_floating_ip,
148+ context2,
149+ float_addr,
150+ fix_addr)
151+
152+ # Deallocate address from other project
153+ self.assertRaises(exception.NotAuthorized,
154+ self.network.deallocate_floating_ip,
155+ context2,
156+ float_addr)
157+
158+ # Now Associates the address to the actual project
159+ self.network.associate_floating_ip(context1, float_addr, fix_addr)
160+
161+ # Now try dis-associating from other project
162+ self.assertRaises(exception.NotAuthorized,
163+ self.network.disassociate_floating_ip,
164+ context2,
165+ float_addr)
166+
167+ # Clean up the ip addresses
168+ self.network.deallocate_floating_ip(context1, float_addr)
169+ self.network.deallocate_fixed_ip(context1, fix_addr)
170+ db.floating_ip_destroy(context1.elevated(), float_addr)
171+ db.fixed_ip_disassociate(context1.elevated(), fix_addr)
172+
173
174 class CommonNetworkTestCase(test.TestCase):
175 def fake_create_fixed_ips(self, context, network_id):