Merge lp:~xtoddx/nova/imagedownload into lp:~hudson-openstack/nova/trunk

Proposed by Todd Willey
Status: Merged
Approved by: Jay Pipes
Approved revision: 368
Merged at revision: 373
Proposed branch: lp:~xtoddx/nova/imagedownload
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 107 lines (+13/-11)
1 file modified
nova/objectstore/handler.py (+13/-11)
To merge this branch: bzr merge lp:~xtoddx/nova/imagedownload
Reviewer Review Type Date Requested Status
Jay Pipes (community) Approve
Devin Carlen (community) Approve
Review via email: mp+38901@code.launchpad.net

Commit message

Authorize image access instead of just blindly giving it away.

To post a comment you must log in.
Revision history for this message
Devin Carlen (devcamcar) wrote :

lgtm

review: Approve
lp:~xtoddx/nova/imagedownload updated
368. By Todd Willey

Construct exception instead of raising a class.

Revision history for this message
Jay Pipes (jaypipes) wrote :

it's not necessary to use constructor parens when raising exceptions that take no arguments:

jpipes@serialcoder:~$ python
Python 2.6.5 (r265:79063, Apr 16 2010, 13:57:41)
[GCC 4.4.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> class SomeException(Exception):
... pass
...
>>> raise SomeException
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
__main__.SomeException
>>> raise SomeException()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
__main__.SomeException

That said, nothing wrong with the parens :)

lgtm.

review: Approve
Revision history for this message
Soren Hansen (soren) wrote :

Set a description and/or commit message, otherwise this'll never get merged.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'nova/objectstore/handler.py'
--- nova/objectstore/handler.py 2010-10-01 12:57:17 +0000
+++ nova/objectstore/handler.py 2010-10-19 23:59:41 +0000
@@ -119,7 +119,7 @@
119 # Authorization Header format: 'AWS <access>:<secret>'119 # Authorization Header format: 'AWS <access>:<secret>'
120 authorization_header = request.getHeader('Authorization')120 authorization_header = request.getHeader('Authorization')
121 if not authorization_header:121 if not authorization_header:
122 raise exception.NotAuthorized122 raise exception.NotAuthorized()
123 auth_header_value = authorization_header.split(' ')[1]123 auth_header_value = authorization_header.split(' ')[1]
124 access, _ignored, secret = auth_header_value.rpartition(':')124 access, _ignored, secret = auth_header_value.rpartition(':')
125 am = manager.AuthManager()125 am = manager.AuthManager()
@@ -134,7 +134,7 @@
134 return context.RequestContext(user, project)134 return context.RequestContext(user, project)
135 except exception.Error as ex:135 except exception.Error as ex:
136 logging.debug("Authentication Failure: %s", ex)136 logging.debug("Authentication Failure: %s", ex)
137 raise exception.NotAuthorized137 raise exception.NotAuthorized()
138138
139class ErrorHandlingResource(resource.Resource):139class ErrorHandlingResource(resource.Resource):
140 """Maps exceptions to 404 / 401 codes. Won't work for140 """Maps exceptions to 404 / 401 codes. Won't work for
@@ -209,7 +209,7 @@
209 return error.NoResource(message="No such bucket").render(request)209 return error.NoResource(message="No such bucket").render(request)
210210
211 if not bucket_object.is_authorized(request.context):211 if not bucket_object.is_authorized(request.context):
212 raise exception.NotAuthorized212 raise exception.NotAuthorized()
213213
214 prefix = get_argument(request, "prefix", u"")214 prefix = get_argument(request, "prefix", u"")
215 marker = get_argument(request, "marker", u"")215 marker = get_argument(request, "marker", u"")
@@ -239,7 +239,7 @@
239 bucket_object = bucket.Bucket(self.name)239 bucket_object = bucket.Bucket(self.name)
240240
241 if not bucket_object.is_authorized(request.context):241 if not bucket_object.is_authorized(request.context):
242 raise exception.NotAuthorized242 raise exception.NotAuthorized()
243243
244 bucket_object.delete()244 bucket_object.delete()
245 request.setResponseCode(204)245 request.setResponseCode(204)
@@ -262,7 +262,7 @@
262 logging.debug("Getting object: %s / %s", self.bucket.name, self.name)262 logging.debug("Getting object: %s / %s", self.bucket.name, self.name)
263263
264 if not self.bucket.is_authorized(request.context):264 if not self.bucket.is_authorized(request.context):
265 raise exception.NotAuthorized265 raise exception.NotAuthorized()
266266
267 obj = self.bucket[urllib.unquote(self.name)]267 obj = self.bucket[urllib.unquote(self.name)]
268 request.setHeader("Content-Type", "application/unknown")268 request.setHeader("Content-Type", "application/unknown")
@@ -280,7 +280,7 @@
280 logging.debug("Putting object: %s / %s", self.bucket.name, self.name)280 logging.debug("Putting object: %s / %s", self.bucket.name, self.name)
281281
282 if not self.bucket.is_authorized(request.context):282 if not self.bucket.is_authorized(request.context):
283 raise exception.NotAuthorized283 raise exception.NotAuthorized()
284284
285 key = urllib.unquote(self.name)285 key = urllib.unquote(self.name)
286 request.content.seek(0, 0)286 request.content.seek(0, 0)
@@ -301,7 +301,7 @@
301 self.name)301 self.name)
302302
303 if not self.bucket.is_authorized(request.context):303 if not self.bucket.is_authorized(request.context):
304 raise exception.NotAuthorized304 raise exception.NotAuthorized()
305305
306 del self.bucket[urllib.unquote(self.name)]306 del self.bucket[urllib.unquote(self.name)]
307 request.setResponseCode(204)307 request.setResponseCode(204)
@@ -318,6 +318,8 @@
318318
319 def render_GET(self, request):319 def render_GET(self, request):
320 """Returns the image file"""320 """Returns the image file"""
321 if not self.img.is_authorized(request.context, True):
322 raise exception.NotAuthorized()
321 return static.File(self.img.image_path,323 return static.File(self.img.image_path,
322 defaultType='application/octet-stream'324 defaultType='application/octet-stream'
323 ).render_GET(request)325 ).render_GET(request)
@@ -369,12 +371,12 @@
369 image_path = os.path.join(FLAGS.images_path, image_id)371 image_path = os.path.join(FLAGS.images_path, image_id)
370 if not image_path.startswith(FLAGS.images_path) or \372 if not image_path.startswith(FLAGS.images_path) or \
371 os.path.exists(image_path):373 os.path.exists(image_path):
372 raise exception.NotAuthorized374 raise exception.NotAuthorized()
373375
374 bucket_object = bucket.Bucket(image_location.split("/")[0])376 bucket_object = bucket.Bucket(image_location.split("/")[0])
375377
376 if not bucket_object.is_authorized(request.context):378 if not bucket_object.is_authorized(request.context):
377 raise exception.NotAuthorized379 raise exception.NotAuthorized()
378380
379 p = multiprocessing.Process(target=image.Image.register_aws_image,381 p = multiprocessing.Process(target=image.Image.register_aws_image,
380 args=(image_id, image_location, request.context))382 args=(image_id, image_location, request.context))
@@ -389,7 +391,7 @@
389 image_object = image.Image(image_id)391 image_object = image.Image(image_id)
390 if not image_object.is_authorized(request.context):392 if not image_object.is_authorized(request.context):
391 logging.debug("not authorized for render_POST in images")393 logging.debug("not authorized for render_POST in images")
392 raise exception.NotAuthorized394 raise exception.NotAuthorized()
393395
394 operation = get_argument(request, 'operation', u'')396 operation = get_argument(request, 'operation', u'')
395 if operation:397 if operation:
@@ -411,7 +413,7 @@
411 image_object = image.Image(image_id)413 image_object = image.Image(image_id)
412414
413 if not image_object.is_authorized(request.context):415 if not image_object.is_authorized(request.context):
414 raise exception.NotAuthorized416 raise exception.NotAuthorized()
415417
416 image_object.delete()418 image_object.delete()
417419