Merge lp:~justin-fathomdb/nova/justinsb-openstack-api-keys into lp:~hudson-openstack/nova/trunk
- justinsb-openstack-api-keys
- Merge into trunk
Status: | Work in progress |
---|---|
Proposed branch: | lp:~justin-fathomdb/nova/justinsb-openstack-api-keys |
Merge into: | lp:~hudson-openstack/nova/trunk |
Diff against target: |
956 lines (+679/-56) 15 files modified
nova/api/ec2/cloud.py (+14/-41) nova/api/keys.py (+93/-0) nova/api/openstack/__init__.py (+10/-0) nova/api/openstack/keys.py (+122/-0) nova/cloudpipe/pipelib.py (+2/-2) nova/compute/api.py (+2/-1) nova/db/api.py (+2/-2) nova/db/sqlalchemy/api.py (+17/-6) nova/tests/integrated/__init__.py (+20/-0) nova/tests/integrated/api/__init__.py (+20/-0) nova/tests/integrated/api/client.py (+130/-0) nova/tests/integrated/integrated_helpers.py (+164/-0) nova/tests/integrated/test_keys.py (+78/-0) nova/tests/test_api.py (+2/-1) nova/tests/test_cloud.py (+3/-3) |
To merge this branch: | bzr merge lp:~justin-fathomdb/nova/justinsb-openstack-api-keys |
Related bugs: | |
Related blueprints: |
Achieve Stability in Cactus
(Undefined)
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jay Pipes | Pending | ||
Review via email: mp+50662@code.launchpad.net |
This proposal supersedes a proposal from 2011-02-19.
Commit message
Description of the change
Support user SSH keys in OpenStack API (they're already supported in EC2, needed for integration tests). Disabled by FLAG option until approved by community, but needed for testing and hence stability.
Jay Pipes (jaypipes) wrote : Posted in a previous version of this proposal | # |
justinsb (justin-fathomdb) wrote : | # |
1) Quite right Jay & very sorry NASA/Anso guys! I started with a clean file and then didn't update the copyright as I refactored the code into there.
2) The idea of the Keys class is to make it look like volumes, network, images & compute. Each of the Web services instantiates a service.API class, and then makes calls on it. Although it's in-process at the moment, and thus could be a module, the intention is that this is an implementation detail. I renamed it to API to show this and added a comment. I did fix up the imports as you suggested.
3) This is necessary to support testing, which is needed for our goal of stability. However, until it has been approved by the community, it is disabled unless specifically enabled for testing, just like FLAGS.allow_
4) Done! (Taking advantage of #3!)
Unmerged revisions
- 710. By justinsb
-
Eventlet seems to screw up httplib2; switched back to httplib as used elsewhere in nova
- 709. By justinsb
-
Merged with head - resolved conflict with moving of context.user.id => context.user_id
- 708. By justinsb
-
NASA copyright was in wrong file & a few formatting issues
- 707. By justinsb
-
Clean up Keys to meet style guidelines, and add explanatory note.
The idea of the API class is to make keys 'feel' like the other services (volumes, compute, images, network). Currently it's implemented in-process, but this way the REST interfaces all look the same, irrespective of this implementation detail - 706. By justinsb
-
Don't allow keys to be part of the OpenStack API until it is approved, but support stability by allowing it to be tested using the same approach as allow_admin_api
- 705. By justinsb
-
Added NASA copyright into file into which I pasted their copy. Sorry guys!!
- 704. By justinsb
-
Added unit tests for keys
- 703. By justinsb
-
Fix broken unit tests
- 702. By justinsb
-
pep8 fixes
- 701. By justinsb
-
Support keys in OpenStack API (needed for integration tests)
Preview Diff
1 | === modified file 'nova/api/ec2/cloud.py' |
2 | --- nova/api/ec2/cloud.py 2011-02-19 07:15:42 +0000 |
3 | +++ nova/api/ec2/cloud.py 2011-02-21 22:19:33 +0000 |
4 | @@ -31,7 +31,6 @@ |
5 | from nova import compute |
6 | from nova import context |
7 | |
8 | -from nova import crypto |
9 | from nova import db |
10 | from nova import exception |
11 | from nova import flags |
12 | @@ -40,7 +39,7 @@ |
13 | from nova import utils |
14 | from nova import volume |
15 | from nova.compute import instance_types |
16 | - |
17 | +from nova.api import keys |
18 | |
19 | FLAGS = flags.FLAGS |
20 | flags.DECLARE('service_down_time', 'nova.scheduler.driver') |
21 | @@ -50,29 +49,6 @@ |
22 | InvalidInputException = exception.InvalidInputException |
23 | |
24 | |
25 | -def _gen_key(context, user_id, key_name): |
26 | - """Generate a key |
27 | - |
28 | - This is a module level method because it is slow and we need to defer |
29 | - it into a process pool.""" |
30 | - # NOTE(vish): generating key pair is slow so check for legal |
31 | - # creation before creating key_pair |
32 | - try: |
33 | - db.key_pair_get(context, user_id, key_name) |
34 | - raise exception.Duplicate(_("The key_pair %s already exists") |
35 | - % key_name) |
36 | - except exception.NotFound: |
37 | - pass |
38 | - private_key, public_key, fingerprint = crypto.generate_key_pair() |
39 | - key = {} |
40 | - key['user_id'] = user_id |
41 | - key['name'] = key_name |
42 | - key['public_key'] = public_key |
43 | - key['fingerprint'] = fingerprint |
44 | - db.key_pair_create(context, key) |
45 | - return {'private_key': private_key, 'fingerprint': fingerprint} |
46 | - |
47 | - |
48 | def ec2_id_to_id(ec2_id): |
49 | """Convert an ec2 ID (i-[base 16 number]) to an instance id (int)""" |
50 | return int(ec2_id.split('-')[-1], 16) |
51 | @@ -97,6 +73,7 @@ |
52 | image_service=self.image_service, |
53 | volume_api=self.volume_api, |
54 | hostname_factory=id_to_ec2_id) |
55 | + self.keys_api = keys.API() |
56 | self.setup() |
57 | |
58 | def __str__(self): |
59 | @@ -282,35 +259,31 @@ |
60 | 'description': 'fixme'}]} |
61 | |
62 | def describe_key_pairs(self, context, key_name=None, **kwargs): |
63 | - key_pairs = db.key_pair_get_all_by_user(context, context.user_id) |
64 | + key_pairs = self.keys_api.get_all_keys(context) |
65 | if not key_name is None: |
66 | key_pairs = [x for x in key_pairs if x['name'] in key_name] |
67 | |
68 | result = [] |
69 | for key_pair in key_pairs: |
70 | - # filter out the vpn keys |
71 | - suffix = FLAGS.vpn_key_suffix |
72 | - if context.is_admin or \ |
73 | - not key_pair['name'].endswith(suffix): |
74 | - result.append({ |
75 | - 'keyName': key_pair['name'], |
76 | - 'keyFingerprint': key_pair['fingerprint'], |
77 | - }) |
78 | + result.append({ |
79 | + 'keyName': key_pair['name'], |
80 | + 'keyFingerprint': key_pair['fingerprint'], |
81 | + }) |
82 | |
83 | return {'keypairsSet': result} |
84 | |
85 | def create_key_pair(self, context, key_name, **kwargs): |
86 | - LOG.audit(_("Create key pair %s"), key_name, context=context) |
87 | - data = _gen_key(context, context.user_id, key_name) |
88 | + data = self.keys_api.create_key_pair(context, key_name) |
89 | + private_key = data['private_key'] |
90 | + key = data['key'] |
91 | return {'keyName': key_name, |
92 | - 'keyFingerprint': data['fingerprint'], |
93 | - 'keyMaterial': data['private_key']} |
94 | - # TODO(vish): when context is no longer an object, pass it here |
95 | + 'keyFingerprint': key['fingerprint'], |
96 | + 'keyMaterial': private_key} |
97 | |
98 | def delete_key_pair(self, context, key_name, **kwargs): |
99 | - LOG.audit(_("Delete key pair %s"), key_name, context=context) |
100 | try: |
101 | - db.key_pair_destroy(context, context.user_id, key_name) |
102 | + key = self.keys_api.get_key_by_name(context, key_name) |
103 | + self.keys_api.delete_key_pair(context, key) |
104 | except exception.NotFound: |
105 | # aws returns true even if the key doesn't exist |
106 | pass |
107 | |
108 | === added file 'nova/api/keys.py' |
109 | --- nova/api/keys.py 1970-01-01 00:00:00 +0000 |
110 | +++ nova/api/keys.py 2011-02-21 22:19:33 +0000 |
111 | @@ -0,0 +1,93 @@ |
112 | +# vim: tabstop=4 shiftwidth=4 softtabstop=4 |
113 | + |
114 | +# Copyright 2010 United States Government as represented by the |
115 | +# Administrator of the National Aeronautics and Space Administration. |
116 | +# Copyright 2011 Justin Santa Barbara |
117 | +# All Rights Reserved. |
118 | +# |
119 | +# Licensed under the Apache License, Version 2.0 (the "License"); you may |
120 | +# not use this file except in compliance with the License. You may obtain |
121 | +# a copy of the License at |
122 | +# |
123 | +# http://www.apache.org/licenses/LICENSE-2.0 |
124 | +# |
125 | +# Unless required by applicable law or agreed to in writing, software |
126 | +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT |
127 | +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the |
128 | +# License for the specific language governing permissions and limitations |
129 | +# under the License. |
130 | + |
131 | + |
132 | +"""Function dealing with (crypto) keys""" |
133 | + |
134 | +from nova import crypto |
135 | +from nova import db |
136 | +from nova import exception |
137 | +from nova import flags |
138 | +from nova import log as logging |
139 | + |
140 | + |
141 | +FLAGS = flags.FLAGS |
142 | + |
143 | +LOG = logging.getLogger("nova.keys") |
144 | + |
145 | + |
146 | +def _gen_key(context, user_id, key_name): |
147 | + """Generate a key |
148 | + |
149 | + This is a module level method because it is slow and we need to defer |
150 | + it into a process pool.""" |
151 | + # NOTE(vish): generating key pair is slow so check for legal |
152 | + # creation before creating key_pair |
153 | + try: |
154 | + db.key_pair_get(context, user_id, name=key_name) |
155 | + raise exception.Duplicate(_("The key_pair %s already exists") |
156 | + % key_name) |
157 | + except exception.NotFound: |
158 | + pass |
159 | + private_key, public_key, fingerprint = crypto.generate_key_pair() |
160 | + key = {} |
161 | + key['user_id'] = user_id |
162 | + key['name'] = key_name |
163 | + key['public_key'] = public_key |
164 | + key['fingerprint'] = fingerprint |
165 | + created = db.key_pair_create(context, key) |
166 | + return {'private_key': private_key, 'key': created} |
167 | + |
168 | + |
169 | +class API(object): |
170 | + """Although keys is currently an in-process function, this abstraction |
171 | + supports the idea that it may not be in future, so that all the APIs |
172 | + look the same (volumes, compute, keys etc).""" |
173 | + def get_all_keys(self, context): |
174 | + key_pairs = db.key_pair_get_all_by_user(context, context.user_id) |
175 | + |
176 | + result = [] |
177 | + for key_pair in key_pairs: |
178 | + # filter out the vpn keys |
179 | + suffix = FLAGS.vpn_key_suffix |
180 | + if context.is_admin or not key_pair['name'].endswith(suffix): |
181 | + result.append(key_pair) |
182 | + |
183 | + return result |
184 | + |
185 | + def get_key_by_name(self, context, key_name): |
186 | + return db.key_pair_get(context, context.user_id, name=key_name) |
187 | + |
188 | + def get_key_by_id(self, context, key_id): |
189 | + return db.key_pair_get(context, context.user_id, id=key_id) |
190 | + |
191 | + def create_key_pair(self, context, key_name): |
192 | + LOG.audit(_("Create key pair %s"), key_name, context=context) |
193 | + return _gen_key(context, context.user_id, key_name) |
194 | + # TODO(vish): when context is no longer an object, pass it here |
195 | + |
196 | + def delete_key_pair(self, context, key): |
197 | + key_name = key['name'] |
198 | + LOG.audit(_("Delete key pair %s"), key_name, context=context) |
199 | + try: |
200 | + db.key_pair_destroy(context, context.user_id, name=key_name) |
201 | + except exception.NotFound: |
202 | + # aws returns true even if the key doesn't exist |
203 | + pass |
204 | + return True |
205 | |
206 | === modified file 'nova/api/openstack/__init__.py' |
207 | --- nova/api/openstack/__init__.py 2011-02-17 21:39:03 +0000 |
208 | +++ nova/api/openstack/__init__.py 2011-02-21 22:19:33 +0000 |
209 | @@ -32,6 +32,7 @@ |
210 | from nova.api.openstack import consoles |
211 | from nova.api.openstack import flavors |
212 | from nova.api.openstack import images |
213 | +from nova.api.openstack import keys |
214 | from nova.api.openstack import servers |
215 | from nova.api.openstack import shared_ip_groups |
216 | from nova.api.openstack import zones |
217 | @@ -42,6 +43,10 @@ |
218 | flags.DEFINE_bool('allow_admin_api', |
219 | False, |
220 | 'When True, this API service will accept admin operations.') |
221 | +flags.DEFINE_bool('allow_testing_api', |
222 | + False, |
223 | + 'When True, this API service will support unofficial API extensions that ' |
224 | + 'are intended solely for testing as part of the stability drive.') |
225 | |
226 | |
227 | class FaultWrapper(wsgi.Middleware): |
228 | @@ -85,6 +90,11 @@ |
229 | mapper.resource("zone", "zones", controller=zones.Controller(), |
230 | collection={'detail': 'GET'}) |
231 | |
232 | + if FLAGS.allow_testing_api: |
233 | + LOG.debug(_("Including testing operations in API.")) |
234 | + mapper.resource("key", "keys", controller=keys.Controller(), |
235 | + collection={'detail': 'GET'}) |
236 | + |
237 | mapper.resource("server", "servers", controller=servers.Controller(), |
238 | collection={'detail': 'GET'}, |
239 | member=server_members) |
240 | |
241 | === added file 'nova/api/openstack/keys.py' |
242 | --- nova/api/openstack/keys.py 1970-01-01 00:00:00 +0000 |
243 | +++ nova/api/openstack/keys.py 2011-02-21 22:19:33 +0000 |
244 | @@ -0,0 +1,122 @@ |
245 | +# Copyright 2011 Justin Santa Barbara |
246 | +# All Rights Reserved. |
247 | +# |
248 | +# Licensed under the Apache License, Version 2.0 (the "License"); you may |
249 | +# not use this file except in compliance with the License. You may obtain |
250 | +# a copy of the License at |
251 | +# |
252 | +# http://www.apache.org/licenses/LICENSE-2.0 |
253 | +# |
254 | +# Unless required by applicable law or agreed to in writing, software |
255 | +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT |
256 | +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the |
257 | +# License for the specific language governing permissions and limitations |
258 | +# under the License. |
259 | + |
260 | +from webob import exc |
261 | + |
262 | +from nova import exception |
263 | +from nova import flags |
264 | +from nova import log as logging |
265 | +from nova import wsgi |
266 | +from nova.api.openstack import common |
267 | +from nova.api.openstack import faults |
268 | +from nova.api import keys |
269 | + |
270 | + |
271 | +LOG = logging.getLogger("nova.api.keys") |
272 | + |
273 | +FLAGS = flags.FLAGS |
274 | + |
275 | + |
276 | +def _translate_detail_keys(inst): |
277 | + """ Maps keys for details view""" |
278 | + |
279 | + inst_dict = _translate_summary_keys(inst) |
280 | + |
281 | + # No additional data / lookups at the moment |
282 | + |
283 | + return inst_dict |
284 | + |
285 | + |
286 | +def _translate_summary_keys(inst): |
287 | + """ Maps keys for summary view""" |
288 | + inst_dict = {} |
289 | + inst_dict['id'] = inst['id'] |
290 | + inst_dict['name'] = inst['name'] |
291 | + inst_dict['fingerprint'] = inst['fingerprint'] |
292 | + |
293 | + return inst_dict |
294 | + |
295 | + |
296 | +class Controller(wsgi.Controller): |
297 | + """ The (crypto) Key API controller for the OpenStack API """ |
298 | + |
299 | + _serialization_metadata = { |
300 | + 'application/xml': { |
301 | + "attributes": { |
302 | + "key": ["id", "keyName", "keyFingerprint", "privateKey"]}}} |
303 | + |
304 | + def __init__(self): |
305 | + self.keys_api = keys.API() |
306 | + super(Controller, self).__init__() |
307 | + |
308 | + def show(self, req, id): |
309 | + """Return data about the given key""" |
310 | + context = req.environ['nova.context'] |
311 | + |
312 | + try: |
313 | + key = self.keys_api.get_key_by_id(context, id) |
314 | + except exception.NotFound: |
315 | + return faults.Fault(exc.HTTPNotFound()) |
316 | + |
317 | + return {'key': _translate_detail_keys(key)} |
318 | + |
319 | + def delete(self, req, id): |
320 | + """ Delete a key """ |
321 | + context = req.environ['nova.context'] |
322 | + try: |
323 | + key = self.keys_api.get_key_by_id(context, id) |
324 | + self.keys_api.delete_key_pair(context, key) |
325 | + except exception.NotFound: |
326 | + return faults.Fault(exc.HTTPNotFound()) |
327 | + return exc.HTTPAccepted() |
328 | + |
329 | + def index(self, req): |
330 | + """ Returns a summary list of keys for a given user """ |
331 | + return self._items(req, entity_maker=_translate_summary_keys) |
332 | + |
333 | + def detail(self, req): |
334 | + """ Returns a detailed list of keys for a given user """ |
335 | + return self._items(req, entity_maker=_translate_detail_keys) |
336 | + |
337 | + def _items(self, req, entity_maker): |
338 | + """Returns a list of keys for a given user. |
339 | + |
340 | + entity_maker - applied to each item to map to return format |
341 | + """ |
342 | + context = req.environ['nova.context'] |
343 | + |
344 | + key_pairs = self.keys_api.get_all_keys(context) |
345 | + limited_list = common.limited(key_pairs, req) |
346 | + res = [entity_maker(inst) for inst in limited_list] |
347 | + return {'keys': res} |
348 | + |
349 | + def create(self, req): |
350 | + """ Creates a new server for a given user """ |
351 | + context = req.environ['nova.context'] |
352 | + |
353 | + env = self._deserialize(req.body, req) |
354 | + if not env: |
355 | + return faults.Fault(exc.HTTPUnprocessableEntity()) |
356 | + |
357 | + key_name = env['key']['name'] |
358 | + |
359 | + data = self.keys_api.create_key_pair(context, key_name) |
360 | + private_key = data['private_key'] |
361 | + key = data['key'] |
362 | + |
363 | + retval = _translate_detail_keys(key) |
364 | + retval['privateKey'] = private_key |
365 | + |
366 | + return {'key': retval} |
367 | |
368 | === modified file 'nova/cloudpipe/pipelib.py' |
369 | --- nova/cloudpipe/pipelib.py 2011-01-12 22:35:09 +0000 |
370 | +++ nova/cloudpipe/pipelib.py 2011-02-21 22:19:33 +0000 |
371 | @@ -37,7 +37,7 @@ |
372 | from nova.auth import manager |
373 | # TODO(eday): Eventually changes these to something not ec2-specific |
374 | from nova.api.ec2 import cloud |
375 | - |
376 | +from nova.api import keys |
377 | |
378 | FLAGS = flags.FLAGS |
379 | flags.DEFINE_string('boot_script_template', |
380 | @@ -138,7 +138,7 @@ |
381 | def setup_key_pair(self, context): |
382 | key_name = '%s%s' % (context.project.id, FLAGS.vpn_key_suffix) |
383 | try: |
384 | - result = cloud._gen_key(context, context.user.id, key_name) |
385 | + result = keys._gen_key(context, context.user.id, key_name) |
386 | private_key = result['private_key'] |
387 | try: |
388 | key_dir = os.path.join(FLAGS.keys_path, context.user.id) |
389 | |
390 | === modified file 'nova/compute/api.py' |
391 | --- nova/compute/api.py 2011-02-19 05:00:58 +0000 |
392 | +++ nova/compute/api.py 2011-02-21 22:19:33 +0000 |
393 | @@ -132,7 +132,8 @@ |
394 | security_groups.append(group['id']) |
395 | |
396 | if key_data is None and key_name: |
397 | - key_pair = db.key_pair_get(context, context.user_id, key_name) |
398 | + key_pair = db.key_pair_get(context, context.user_id, |
399 | + name=key_name) |
400 | key_data = key_pair['public_key'] |
401 | |
402 | base_options = { |
403 | |
404 | === modified file 'nova/db/api.py' |
405 | --- nova/db/api.py 2011-02-17 22:14:07 +0000 |
406 | +++ nova/db/api.py 2011-02-21 22:19:33 +0000 |
407 | @@ -442,9 +442,9 @@ |
408 | return IMPL.key_pair_destroy_all_by_user(context, user_id) |
409 | |
410 | |
411 | -def key_pair_get(context, user_id, name): |
412 | +def key_pair_get(context, user_id, name=None, id=None): |
413 | """Get a key_pair or raise if it does not exist.""" |
414 | - return IMPL.key_pair_get(context, user_id, name) |
415 | + return IMPL.key_pair_get(context, user_id, name=name, id=id) |
416 | |
417 | |
418 | def key_pair_get_all_by_user(context, user_id): |
419 | |
420 | === modified file 'nova/db/sqlalchemy/api.py' |
421 | --- nova/db/sqlalchemy/api.py 2011-02-17 21:39:03 +0000 |
422 | +++ nova/db/sqlalchemy/api.py 2011-02-21 22:19:33 +0000 |
423 | @@ -931,7 +931,8 @@ |
424 | authorize_user_context(context, user_id) |
425 | session = get_session() |
426 | with session.begin(): |
427 | - key_pair_ref = key_pair_get(context, user_id, name, session=session) |
428 | + key_pair_ref = key_pair_get(context, user_id, name=name, |
429 | + session=session) |
430 | key_pair_ref.delete(session=session) |
431 | |
432 | |
433 | @@ -946,17 +947,27 @@ |
434 | |
435 | |
436 | @require_context |
437 | -def key_pair_get(context, user_id, name, session=None): |
438 | +def key_pair_get(context, user_id, name=None, id=None, session=None): |
439 | authorize_user_context(context, user_id) |
440 | |
441 | if not session: |
442 | session = get_session() |
443 | |
444 | - result = session.query(models.KeyPair).\ |
445 | + query = session.query(models.KeyPair).\ |
446 | filter_by(user_id=user_id).\ |
447 | - filter_by(name=name).\ |
448 | - filter_by(deleted=can_read_deleted(context)).\ |
449 | - first() |
450 | + filter_by(deleted=can_read_deleted(context)) |
451 | + |
452 | + if not name and not id: |
453 | + raise exception.Error('Name or Id is required') |
454 | + |
455 | + if name: |
456 | + query = query.filter_by(name=name) |
457 | + |
458 | + if id: |
459 | + query = query.filter_by(id=id) |
460 | + |
461 | + result = query.first() |
462 | + |
463 | if not result: |
464 | raise exception.NotFound(_('no keypair for user %(user_id)s,' |
465 | ' name %(name)s') % locals()) |
466 | |
467 | === added directory 'nova/tests/integrated' |
468 | === added file 'nova/tests/integrated/__init__.py' |
469 | --- nova/tests/integrated/__init__.py 1970-01-01 00:00:00 +0000 |
470 | +++ nova/tests/integrated/__init__.py 2011-02-21 22:19:33 +0000 |
471 | @@ -0,0 +1,20 @@ |
472 | +# vim: tabstop=4 shiftwidth=4 softtabstop=4 |
473 | + |
474 | +# Copyright (c) 2011 Justin Santa Barbara |
475 | +# |
476 | +# Licensed under the Apache License, Version 2.0 (the "License"); you may |
477 | +# not use this file except in compliance with the License. You may obtain |
478 | +# a copy of the License at |
479 | +# |
480 | +# http://www.apache.org/licenses/LICENSE-2.0 |
481 | +# |
482 | +# Unless required by applicable law or agreed to in writing, software |
483 | +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT |
484 | +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the |
485 | +# License for the specific language governing permissions and limitations |
486 | +# under the License. |
487 | + |
488 | +""" |
489 | +:mod:`integrated` -- Tests whole systems, using mock services where needed |
490 | +================================= |
491 | +""" |
492 | |
493 | === added directory 'nova/tests/integrated/api' |
494 | === added file 'nova/tests/integrated/api/__init__.py' |
495 | --- nova/tests/integrated/api/__init__.py 1970-01-01 00:00:00 +0000 |
496 | +++ nova/tests/integrated/api/__init__.py 2011-02-21 22:19:33 +0000 |
497 | @@ -0,0 +1,20 @@ |
498 | +# vim: tabstop=4 shiftwidth=4 softtabstop=4 |
499 | + |
500 | +# Copyright (c) 2011 Justin Santa Barbara |
501 | +# |
502 | +# Licensed under the Apache License, Version 2.0 (the "License"); you may |
503 | +# not use this file except in compliance with the License. You may obtain |
504 | +# a copy of the License at |
505 | +# |
506 | +# http://www.apache.org/licenses/LICENSE-2.0 |
507 | +# |
508 | +# Unless required by applicable law or agreed to in writing, software |
509 | +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT |
510 | +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the |
511 | +# License for the specific language governing permissions and limitations |
512 | +# under the License. |
513 | + |
514 | +""" |
515 | +:mod:`api` -- OpenStack API client, for testing rather than production |
516 | +================================= |
517 | +""" |
518 | |
519 | === added file 'nova/tests/integrated/api/client.py' |
520 | --- nova/tests/integrated/api/client.py 1970-01-01 00:00:00 +0000 |
521 | +++ nova/tests/integrated/api/client.py 2011-02-21 22:19:33 +0000 |
522 | @@ -0,0 +1,130 @@ |
523 | +# vim: tabstop=4 shiftwidth=4 softtabstop=4 |
524 | + |
525 | +# Copyright (c) 2011 Justin Santa Barbara |
526 | +# |
527 | +# Licensed under the Apache License, Version 2.0 (the "License"); you may |
528 | +# not use this file except in compliance with the License. You may obtain |
529 | +# a copy of the License at |
530 | +# |
531 | +# http://www.apache.org/licenses/LICENSE-2.0 |
532 | +# |
533 | +# Unless required by applicable law or agreed to in writing, software |
534 | +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT |
535 | +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the |
536 | +# License for the specific language governing permissions and limitations |
537 | +# under the License. |
538 | + |
539 | +from nova import exception |
540 | +import json |
541 | +import httplib |
542 | +from urlparse import urlparse |
543 | + |
544 | + |
545 | +class TestOpenStackClient(object): |
546 | + """ A really basic OpenStack API client that is under our control, |
547 | + so we can make changes / insert hooks for testing""" |
548 | + |
549 | + def __init__(self, auth_user, auth_key, auth_uri): |
550 | + super(TestOpenStackClient, self).__init__() |
551 | + self.auth_result = None |
552 | + self.auth_user = auth_user |
553 | + self.auth_key = auth_key |
554 | + self.auth_uri = auth_uri |
555 | + |
556 | + def request(self, url, method='GET', body=None, headers={}): |
557 | + parsed_url = urlparse(url) |
558 | + port = parsed_url.port |
559 | + hostname = parsed_url.hostname |
560 | + scheme = parsed_url.scheme |
561 | + |
562 | + if scheme == 'http': |
563 | + conn = httplib.HTTPConnection(hostname, |
564 | + port=port) |
565 | + elif scheme == 'https': |
566 | + conn = httplib.HTTPSConnection(hostname, |
567 | + port=port) |
568 | + else: |
569 | + raise exception.Error("Unknown scheme: %s" % url) |
570 | + |
571 | + relative_url = parsed_url.path |
572 | + if parsed_url.query: |
573 | + relative_url = relative_url + parsed_url.query |
574 | + conn.request(method, relative_url, body, headers) |
575 | + response = conn.getresponse() |
576 | + return response |
577 | + |
578 | + def _authenticate(self): |
579 | + if self.auth_result: |
580 | + return self.auth_result |
581 | + |
582 | + headers = {'X-Auth-User': self.auth_user, |
583 | + 'X-Auth-Key': self.auth_key} |
584 | + response = self.request(self.auth_uri, |
585 | + headers=headers) |
586 | + if not response.status in [204]: |
587 | + raise exception.Error("Authentication failure: %s\n%s" % |
588 | + (response.status, response.read())) |
589 | + |
590 | + auth_headers = {} |
591 | + for k, v in response.getheaders(): |
592 | + auth_headers[k] = v |
593 | + |
594 | + self.auth_result = auth_headers |
595 | + return self.auth_result |
596 | + |
597 | + def api_request(self, relative_uri, check_response_status=None, **kwargs): |
598 | + auth_result = self._authenticate() |
599 | + |
600 | + #TODO(justinsb): Why is this lower case?? |
601 | + base_uri = auth_result['X-Server-Management-Url'.lower()] |
602 | + full_uri = base_uri + relative_uri |
603 | + |
604 | + headers = kwargs.setdefault('headers', {}) |
605 | + #TODO(justinsb): Why is this lower case?? |
606 | + headers['X-Auth-Token'] = auth_result['X-Auth-Token'.lower()] |
607 | + |
608 | + response = self.request(full_uri, **kwargs) |
609 | + |
610 | + print "%s => code %s" % (relative_uri, response.status) |
611 | + |
612 | + if check_response_status: |
613 | + if not response.status in check_response_status: |
614 | + raise exception.Error("Unexpected status: %s\n%s" % |
615 | + (response.status, response.read())) |
616 | + |
617 | + return response |
618 | + |
619 | + def _decode_json(self, response): |
620 | + body = response.read() |
621 | + print body |
622 | + return json.loads(body) |
623 | + |
624 | + def api_get(self, relative_uri, **kwargs): |
625 | + kwargs.setdefault('check_response_status', [200]) |
626 | + response = self.api_request(relative_uri, **kwargs) |
627 | + return self._decode_json(response) |
628 | + |
629 | + def api_post(self, relative_uri, body, **kwargs): |
630 | + kwargs['method'] = 'POST' |
631 | + if body: |
632 | + headers = kwargs.setdefault('headers', {}) |
633 | + headers['Content-Type'] = 'application/json' |
634 | + kwargs['body'] = json.dumps(body) |
635 | + |
636 | + kwargs.setdefault('check_response_status', [200]) |
637 | + response = self.api_request(relative_uri, **kwargs) |
638 | + return self._decode_json(response) |
639 | + |
640 | + def api_delete(self, relative_uri, **kwargs): |
641 | + kwargs['method'] = 'DELETE' |
642 | + kwargs.setdefault('check_response_status', [200, 202]) |
643 | + return self.api_request(relative_uri, **kwargs) |
644 | + |
645 | + def get_keys_detail(self): |
646 | + return self.api_get('/keys/detail')['keys'] |
647 | + |
648 | + def post_key(self, key): |
649 | + return self.api_post('/keys', key)['key'] |
650 | + |
651 | + def delete_key(self, key_id): |
652 | + return self.api_delete('/keys/%s' % key_id) |
653 | |
654 | === added file 'nova/tests/integrated/integrated_helpers.py' |
655 | --- nova/tests/integrated/integrated_helpers.py 1970-01-01 00:00:00 +0000 |
656 | +++ nova/tests/integrated/integrated_helpers.py 2011-02-21 22:19:33 +0000 |
657 | @@ -0,0 +1,164 @@ |
658 | +# vim: tabstop=4 shiftwidth=4 softtabstop=4 |
659 | + |
660 | +# Copyright 2011 Justin Santa Barbara |
661 | +# Copyright 2010 United States Government as represented by the |
662 | +# Administrator of the National Aeronautics and Space Administration. |
663 | +# All Rights Reserved. |
664 | +# |
665 | +# Licensed under the Apache License, Version 2.0 (the "License"); you may |
666 | +# not use this file except in compliance with the License. You may obtain |
667 | +# a copy of the License at |
668 | +# |
669 | +# http://www.apache.org/licenses/LICENSE-2.0 |
670 | +# |
671 | +# Unless required by applicable law or agreed to in writing, software |
672 | +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT |
673 | +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the |
674 | +# License for the specific language governing permissions and limitations |
675 | +# under the License. |
676 | + |
677 | +""" |
678 | +Provides common functionality for integrated unit tests |
679 | +""" |
680 | + |
681 | +from nova.tests.integrated.api.client import TestOpenStackClient |
682 | +from nova import wsgi |
683 | +from nova import flags |
684 | +from nova.auth import manager |
685 | +import random |
686 | +import string |
687 | +from nova.exception import Error |
688 | +from nova.log import logging |
689 | +from nova.api import openstack # For FLAGS.allow_testing_api |
690 | + |
691 | +FLAGS = flags.FLAGS |
692 | + |
693 | + |
694 | +def generate_random_alphanumeric(length): |
695 | + """Creates a random string of specified length""" |
696 | + return ''.join(random.choice(string.ascii_uppercase + string.digits) |
697 | + for _x in range(length)) |
698 | + |
699 | + |
700 | +def generate_new_element(items, prefix): |
701 | + """Creates a random string with prefix, that is not in 'items' list""" |
702 | + while True: |
703 | + candidate = prefix + generate_random_alphanumeric(8) |
704 | + if not candidate in items: |
705 | + return candidate |
706 | + print "Random collision on %s" % candidate |
707 | + |
708 | + |
709 | +class IntegratedUnitTestContext(object): |
710 | + __INSTANCE = None |
711 | + |
712 | + def __init__(self): |
713 | + self.old_allow_testing_api = FLAGS.allow_testing_api |
714 | + FLAGS.allow_testing_api = True |
715 | + |
716 | + self.auth_manager = manager.AuthManager() |
717 | + |
718 | + self.auth_user, self.auth_key = self._create_unittest_user() |
719 | + |
720 | + # No way to currently pass this through the OpenStack API |
721 | + self.project_name = 'openstack' |
722 | + self._configure_project(self.project_name) |
723 | + |
724 | + self.auth_url = self._start_nova_api() |
725 | + |
726 | + self.openstack_api = TestOpenStackClient(self.auth_user, |
727 | + self.auth_key, |
728 | + self.auth_url) |
729 | + |
730 | + def _create_unittest_user(self): |
731 | + users = self.auth_manager.get_users() |
732 | + user_names = [user.name for user in users] |
733 | + auth_name = generate_new_element(user_names, 'unittest_user_') |
734 | + auth_key = generate_random_alphanumeric(16) |
735 | + |
736 | + # Right now there's a bug where auth_name and auth_key are reversed |
737 | + auth_key = auth_name |
738 | + |
739 | + self.auth_manager.create_user(auth_name, auth_name, auth_key, False) |
740 | + return auth_name, auth_key |
741 | + |
742 | + def _configure_project(self, project_name): |
743 | + projects = self.auth_manager.get_projects() |
744 | + project_names = [project.name for project in projects] |
745 | + if not project_name in project_names: |
746 | + project = self.auth_manager.create_project(project_name, |
747 | + self.auth_user, |
748 | + description=None, |
749 | + member_users=None) |
750 | + else: |
751 | + self.auth_manager.add_to_project(self.auth_user, project_name) |
752 | + |
753 | + def _start_nova_api(self): |
754 | + # Copied and pasted from bin/nova-api... yuk!! |
755 | + paste_config_file = wsgi.paste_config_file('nova-api.conf') |
756 | + |
757 | + API_ENDPOINTS = ['ec2', 'osapi'] |
758 | + |
759 | + LOG = logging.getLogger('nova.api') |
760 | + LOG.setLevel(logging.DEBUG) |
761 | + |
762 | + LOG.debug(_("Using paste.deploy config at: %s"), paste_config_file) |
763 | + apps = [] |
764 | + for api in API_ENDPOINTS: |
765 | + config = wsgi.load_paste_configuration(paste_config_file, api) |
766 | + if config is None: |
767 | + #LOG.debug(_("No paste configuration for app: %s"), api) |
768 | + continue |
769 | + LOG.debug(_("App Config: %(api)s\n%(config)r") % locals()) |
770 | + wsgi.paste_config_to_flags(config, { |
771 | + "verbose": FLAGS.verbose, |
772 | + "%s_host" % api: config.get('host', '0.0.0.0'), |
773 | + "%s_port" % api: getattr(FLAGS, "%s_port" % api)}) |
774 | + LOG.info(_("Running %s API"), api) |
775 | + app = wsgi.load_paste_app(paste_config_file, api) |
776 | + apps.append((app, getattr(FLAGS, "%s_port" % api), |
777 | + getattr(FLAGS, "%s_host" % api))) |
778 | + if len(apps) == 0: |
779 | + LOG.error(_("No known API applications configured in %s."), |
780 | + paste_config_file) |
781 | + return |
782 | + |
783 | + # NOTE(todd): redo logging config, verbose could be set in paste config |
784 | + #logging.basicConfig() |
785 | + wsgi_server = wsgi.Server() |
786 | + for app in apps: |
787 | + wsgi_server.start(*app) |
788 | + #server.wait() |
789 | + self.wsgi_server = wsgi_server |
790 | + self.wsgi_apps = apps |
791 | + return 'http://localhost:8774/v1.0' |
792 | + |
793 | + def get_openstack_api(self): |
794 | + return self.openstack_api |
795 | + |
796 | + @staticmethod |
797 | + def get(): |
798 | + if not IntegratedUnitTestContext.__INSTANCE: |
799 | + IntegratedUnitTestContext.startup() |
800 | + #raise Error("Must call IntegratedUnitTestContext::startup") |
801 | + return IntegratedUnitTestContext.__INSTANCE |
802 | + |
803 | + @staticmethod |
804 | + def startup(): |
805 | + if IntegratedUnitTestContext.__INSTANCE: |
806 | + raise Error("Multiple calls to IntegratedUnitTestContext.startup") |
807 | + IntegratedUnitTestContext.__INSTANCE = IntegratedUnitTestContext() |
808 | + |
809 | + def cleanup(self): |
810 | + FLAGS.allow_testing_api = self.old_allow_testing_api |
811 | + # TODO(justinsb): Shutdown WSGI & anything else we startup |
812 | + # self.wsgi_server.terminate() |
813 | + # self.wsgi_server = None |
814 | + pass |
815 | + |
816 | + @staticmethod |
817 | + def shutdown(): |
818 | + if not IntegratedUnitTestContext.__INSTANCE: |
819 | + raise Error("Must call IntegratedUnitTestContext::startup") |
820 | + IntegratedUnitTestContext.__INSTANCE.cleanup() |
821 | + IntegratedUnitTestContext.__INSTANCE = None |
822 | |
823 | === added file 'nova/tests/integrated/test_keys.py' |
824 | --- nova/tests/integrated/test_keys.py 1970-01-01 00:00:00 +0000 |
825 | +++ nova/tests/integrated/test_keys.py 2011-02-21 22:19:33 +0000 |
826 | @@ -0,0 +1,78 @@ |
827 | +# vim: tabstop=4 shiftwidth=4 softtabstop=4 |
828 | + |
829 | +# Copyright 2011 Justin Santa Barbara |
830 | +# All Rights Reserved. |
831 | +# |
832 | +# Licensed under the Apache License, Version 2.0 (the "License"); you may |
833 | +# not use this file except in compliance with the License. You may obtain |
834 | +# a copy of the License at |
835 | +# |
836 | +# http://www.apache.org/licenses/LICENSE-2.0 |
837 | +# |
838 | +# Unless required by applicable law or agreed to in writing, software |
839 | +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT |
840 | +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the |
841 | +# License for the specific language governing permissions and limitations |
842 | +# under the License. |
843 | + |
844 | +import unittest |
845 | +from nova import flags |
846 | +from nova.tests.integrated.integrated_helpers import IntegratedUnitTestContext |
847 | +from nova.tests.integrated.integrated_helpers import generate_new_element |
848 | + |
849 | +FLAGS = flags.FLAGS |
850 | +FLAGS.verbose = True |
851 | + |
852 | + |
853 | +class KeysTest(unittest.TestCase): |
854 | + def setUp(self): |
855 | + context = IntegratedUnitTestContext.get() |
856 | + self.api = context.get_openstack_api() |
857 | + |
858 | + def tearDown(self): |
859 | + pass |
860 | + |
861 | + def test_get_keys(self): |
862 | + """Simple check that listing keys works""" |
863 | + keys = self.api.get_keys_detail() |
864 | + for key in keys: |
865 | + print("key: %s" % key) |
866 | + |
867 | + def _find_key(self, key_name): |
868 | + """Return key with specified name or None if not found""" |
869 | + keys = self.api.get_keys_detail() |
870 | + for key in keys: |
871 | + if key['name'] == key_name: |
872 | + return key |
873 | + return None |
874 | + |
875 | + def test_create_and_delete_key(self): |
876 | + """Creates and deletes a key""" |
877 | + |
878 | + # Find unused key name |
879 | + keys = self.api.get_keys_detail() |
880 | + key_names = [key.name for key in keys] |
881 | + key_name = generate_new_element(key_names, 'unittest_') |
882 | + |
883 | + # Create key with name |
884 | + created_key = self.api.post_key({'key': {'name': key_name}}) |
885 | + print "created_key: %s" % created_key |
886 | + private_key = created_key['privateKey'] |
887 | + print "private_key: %s" % private_key |
888 | + self.assertTrue(private_key) |
889 | + self.assertTrue('BEGIN RSA PRIVATE KEY' in private_key) |
890 | + |
891 | + # Check it's there |
892 | + found_key = self._find_key(key_name) |
893 | + self.assertEqual(key_name, found_key['name']) |
894 | + |
895 | + # Delete the key |
896 | + self.api.delete_key(created_key['id']) |
897 | + |
898 | + # Check it's not there any more |
899 | + found_key = self._find_key(key_name) |
900 | + self.assertFalse(found_key) |
901 | + |
902 | + |
903 | +if __name__ == "__main__": |
904 | + unittest.main() |
905 | |
906 | === modified file 'nova/tests/test_api.py' |
907 | --- nova/tests/test_api.py 2011-02-15 12:18:27 +0000 |
908 | +++ nova/tests/test_api.py 2011-02-21 22:19:33 +0000 |
909 | @@ -31,6 +31,7 @@ |
910 | from nova.api.ec2 import cloud |
911 | from nova.api.ec2 import apirequest |
912 | from nova.auth import manager |
913 | +from nova.api import keys |
914 | |
915 | |
916 | class FakeHttplibSocket(object): |
917 | @@ -164,7 +165,7 @@ |
918 | user = self.manager.create_user('fake', 'fake', 'fake') |
919 | project = self.manager.create_project('fake', 'fake', 'fake') |
920 | # NOTE(vish): create depends on pool, so call helper directly |
921 | - cloud._gen_key(context.get_admin_context(), user.id, keyname) |
922 | + keys._gen_key(context.get_admin_context(), user.id, keyname) |
923 | |
924 | rv = self.ec2.get_all_key_pairs() |
925 | results = [k for k in rv if k.name == keyname] |
926 | |
927 | === modified file 'nova/tests/test_cloud.py' |
928 | --- nova/tests/test_cloud.py 2011-01-23 01:46:05 +0000 |
929 | +++ nova/tests/test_cloud.py 2011-02-21 22:19:33 +0000 |
930 | @@ -39,7 +39,7 @@ |
931 | from nova.compute import power_state |
932 | from nova.api.ec2 import cloud |
933 | from nova.objectstore import image |
934 | - |
935 | +from nova.api import keys |
936 | |
937 | FLAGS = flags.FLAGS |
938 | LOG = logging.getLogger('nova.tests.cloud') |
939 | @@ -85,7 +85,7 @@ |
940 | |
941 | def _create_key(self, name): |
942 | # NOTE(vish): create depends on pool, so just call helper directly |
943 | - return cloud._gen_key(self.context, self.context.user.id, name) |
944 | + return keys._gen_key(self.context, self.context.user.id, name) |
945 | |
946 | def test_describe_regions(self): |
947 | """Makes sure describe regions runs without raising an exception""" |
948 | @@ -232,7 +232,7 @@ |
949 | bio = BIO.MemoryBuffer() |
950 | public_key = db.key_pair_get(self.context, |
951 | self.context.user.id, |
952 | - 'test')['public_key'] |
953 | + name='test')['public_key'] |
954 | key.save_pub_key_bio(bio) |
955 | converted = crypto.ssl_pub_to_ssh_pub(bio.read()) |
956 | # assert key fields are equal |
A good enhancement to Nova, Justin! :) Some smallish notes:
1)
14 +# Copyright 2011 Justin Santa Barbara
115 +# All Rights Reserved.
Much of the code in the newly-added file nova/api/keys.py was written by the Anso/NASA guys that was moved from the nova/api/ ec2/cloud. py file. It would be best, in my opinion, to put this above your copyright line:
# Copyright 2010 United States Government as represented by the
# Administrator of the National Aeronautics and Space Administration.
In other words, it's good faith to put both your own copyright line as well as the NASA one above it to acknowledge that some significant portion of the code in the file can be attributed to Anso. Hope that makes sense. No worries on the copyright header stuff in all the other files (like nova/api/ openstack/ keys.py since you wrote all of that code yourself.
2)
I'd prefer to get rid of the Keys class in nova/api/keys.py. All of the methods of Keys should be simple module-level functions since there is no object-level or class-level data to the Keys class. This would also mean you could remove this line in nova/api/ ec2/cloud. py:
55 + self.keys_api = Keys()
and change the import in that same file from this:
7 +from nova.api.keys import Keys
to simply this:
from nova.api import keys
Then, in the Controller methods, instead of something like this:
64 + key_pairs = self.keys_ api.get_ all_keys( context)
just do this:
key_pairs = keys.get_ all_keys( context)
The above solves two problems:
a) from nova.api.keys import Keys breaks the rule in the HACKING file that you should only import modules, not objects (and yes, I know that rule is broken regularly in various parts of the source code, but still, it's a documented coding rule...)
b) Removes the unnecessary Keys class that does nothing more than provide a class-level namespace instead of the more appropriate module-level namespacing.
3)
218 + mapper. resource( "key", "keys", controller= keys.Controller (), {'detail' : 'GET'})
219 + collection=
While I think your addition of the keys resource to the OpenStack REST API is very beneficial, it's up to the mailing list and community to decide on changes to the API. Please propose this change/enhancement to the mailing list for approval. We can't approve this patch until this community approval is gained, regardless of how we personally feel the API would be improved. Hope you understand.
4)
Please add some test cases for the OpenStack API /keys stuff, too :)
Cheers!
jay