Add functional tests

Adds functional tests to confirm the integration of Keystone with Glance.
Also adds bug fixes for issues discovered while creating the functional
tests.

Change-Id: Ie20f8c93a10fbbe085b2ea39373897a058572775

bin/glance

@@ -135,6 +135,8 @@ def print_image_formatted(client, image):
     print "Location: %s" % image['location']
     print "Disk format: %s" % image['disk_format']
     print "Container format: %s" % image['container_format']
+    if image['owner']:
+        print "Owner: %s" % image['owner']
     if len(image['properties']) > 0:
         for k, v in image['properties'].items():
             print "Property '%s': %s" % (k, v)
@@ -321,10 +323,10 @@ to spell field names correctly. :)"""
             fields.pop(field)
 
     base_image_fields = ['disk_format', 'container_format', 'name',
-                         'location']
+                         'location', 'owner']
     for field in base_image_fields:
         fvalue = fields.pop(field, None)
-        if fvalue:
+        if fvalue is not None:
             image_meta[field] = fvalue
 
     # Have to handle "boolean" values specially...
@@ -858,7 +860,7 @@ Displays a list of images shared with a given member"""
     c = get_client(options)
 
     try:
-        images = c.get_member_images(member_id)
+        members = c.get_member_images(member_id)
     except exception.NotFound:
         print "No images shared with member %s" % member_id
         return SUCCESS
@@ -886,8 +888,8 @@ Replaces the members of the image <ID> to be solely <MEMBER>.  If the
 "--can-share" option is given, <MEMBER> will be able to further share
 the image."""
     try:
-        image_id = args.pop()
         member_id = args.pop()
+        image_id = args.pop()
     except IndexError:
         print "Please specify the image ID and the member name"
         return FAILURE
@@ -914,8 +916,8 @@ def member_add(options, args):
 Adds the member <MEMBER> to the image <ID>.  If the "--can-share"
 option is given, <MEMBER> will be able to further share the image."""
     try:
-        image_id = args.pop()
         member_id = args.pop()
+        image_id = args.pop()
     except IndexError:
         print "Please specify the image ID and the member name"
         return FAILURE
@@ -940,8 +942,8 @@ def member_delete(options, args):
 
 Deletes the specified member of the image <ID>."""
     try:
-        image_id = args.pop()
         member_id = args.pop()
+        image_id = args.pop()
     except IndexError:
         print "Please specify the image ID and the member name"
         return FAILURE

glance/api/v1/images.py

@@ -30,6 +30,7 @@
                        HTTPConflict,
                        HTTPBadRequest,
                        HTTPForbidden,
+                       HTTPNoContent,
                        HTTPUnauthorized)
 
 from glance import api
@@ -560,6 +561,12 @@ def update(self, req, id, image_meta, image_data):
                 logger.error(line)
             self.notifier.error('image.update', msg)
             raise HTTPBadRequest(msg, request=req, content_type="text/plain")
+        except exception.NotFound, e:
+            msg = ("Failed to find image to update: %(e)s" % locals())
+            for line in msg.split('\n'):
+                logger.info(line)
+            self.notifier.info('image.update', msg)
+            raise HTTPNotFound(msg, request=req, content_type="text/plain")
         else:
             self.notifier.info('image.update', image_meta)
 
@@ -589,11 +596,19 @@ def delete(self, req, id):
         # of a saving or queued image, therefore don't ask a backend
         # to delete the image if the backend doesn't yet store it.
         # See https://bugs.launchpad.net/glance/+bug/747799
-        if image['location']:
-            schedule_delete_from_backend(image['location'], self.options,
-                                         req.context, id)
-        registry.delete_image_metadata(self.options, req.context, id)
-        self.notifier.info('image.delete', id)
+        try:
+            if image['location']:
+                schedule_delete_from_backend(image['location'], self.options,
+                                             req.context, id)
+            registry.delete_image_metadata(self.options, req.context, id)
+        except exception.NotFound, e:
+            msg = ("Failed to find image to delete: %(e)s" % locals())
+            for line in msg.split('\n'):
+                logger.info(line)
+            self.notifier.info('image.delete', msg)
+            raise HTTPNotFound(msg, request=req, content_type="text/plain")
+        else:
+            self.notifier.info('image.delete', id)
 
     def members(self, req, image_id):
         """
@@ -718,7 +733,6 @@ def add_member(self, req, image_id, member, body=None):
         can_share = None
         if body and 'member' in body and 'can_share' in body['member']:
             can_share = bool(body['member']['can_share'])
-
         try:
             registry.add_member(self.options, req.context, image_id, member,
                                 can_share)

glance/registry/client.py

@@ -141,7 +141,10 @@ def get_member_images(self, member_id):
 
     def replace_members(self, image_id, member_data):
         """Replaces Registry's information about image membership"""
-        if 'memberships' not in member_data.keys():
+        if isinstance(member_data, (list, tuple)):
+            member_data = dict(memberships=list(member_data))
+        elif (isinstance(member_data, dict) and
+              'memberships' not in member_data):
             member_data = dict(memberships=[member_data])
 
         body = json.dumps(member_data)

glance/registry/context.py

@@ -48,14 +48,27 @@ def is_image_visible(self, image):
 
             # Figure out if this image is shared with that tenant
             try:
-                db_api.image_member_find(self, image.id, self.owner)
-                return True
+                tmp = db_api.image_member_find(self, image.id, self.owner)
+                return not tmp['deleted']
             except exception.NotFound:
                 pass
 
         # Private image
         return False
 
+    def is_image_mutable(self, image):
+        """Return True if the image is mutable in this context."""
+        # Is admin == image mutable
+        if self.is_admin:
+            return True
+
+        # No owner == image not mutable
+        if image.owner is None or self.owner is None:
+            return False
+
+        # Image only mutable by its owner
+        return image.owner == self.owner
+
     def is_image_sharable(self, image, **kwargs):
         """Return True if the image can be shared to others in this context."""
         # Only allow sharing if we have an owner

glance/registry/db/api.py

@@ -39,6 +39,7 @@
 _ENGINE = None
 _MAKER = None
 BASE = models.BASE
+logger = None
 
 # attributes common to all models
 BASE_MODEL_ATTRS = set(['id', 'created_at', 'updated_at', 'deleted_at',
@@ -64,6 +65,7 @@ def configure_db(options):
     :param options: Mapping of configuration options
     """
     global _ENGINE
+    global logger
     if not _ENGINE:
         debug = config.get_option(
             options, 'debug', type='bool', default=False)
@@ -82,6 +84,13 @@ def configure_db(options):
         models.register_models(_ENGINE)
 
 
+def check_mutate_authorization(context, image_ref):
+    if not context.is_image_mutable(image_ref):
+        logger.info(_("Attempted to modify image user did not own."))
+        msg = _("You do not own this image")
+        raise exception.NotAuthorized(msg)
+
+
 def get_session(autocommit=True, expire_on_commit=False):
     """Helper method to grab session"""
     global _MAKER, _ENGINE
@@ -112,6 +121,10 @@ def image_destroy(context, image_id):
     session = get_session()
     with session.begin():
         image_ref = image_get(context, image_id, session=session)
+
+        # Perform authorization check
+        check_mutate_authorization(context, image_ref)
+
         image_ref.delete(session=session)
 
         for prop_ref in image_ref.properties:
@@ -216,7 +229,8 @@ def image_get_all(context, filters=None, marker=None, limit=None,
         the_filter = [models.Image.is_public == filters['is_public']]
         if filters['is_public'] and context.owner is not None:
             the_filter.extend([(models.Image.owner == context.owner),
-                               models.Image.members.any(member=context.owner)])
+                               models.Image.members.any(member=context.owner,
+                                                        deleted=False)])
         if len(the_filter) > 1:
             query = query.filter(or_(*the_filter))
         else:
@@ -320,13 +334,20 @@ def _image_update(context, values, image_id, purge_props=False):
 
         if image_id:
             image_ref = image_get(context, image_id, session=session)
+
+            # Perform authorization check
+            check_mutate_authorization(context, image_ref)
         else:
             if 'size' in values:
                 values['size'] = int(values['size'])
 
             values['is_public'] = bool(values.get('is_public', False))
             image_ref = models.Image()
 
+        # Need to canonicalize ownership
+        if 'owner' in values and not values['owner']:
+            values['owner'] = None
+
         if image_id:
             # Don't drop created_at if we're passing it in...
             _drop_protected_attrs(models.Image, values)
@@ -438,6 +459,7 @@ def _image_member_update(context, memb_ref, values, session=None):
 
 def image_member_delete(context, memb_ref, session=None):
     """Delete an ImageMember object"""
+    session = session or get_session()
     memb_ref.update(dict(deleted=True))
     memb_ref.save(session=session)
     return memb_ref
@@ -470,7 +492,6 @@ def image_member_find(context, image_id, member, session=None):
         # RequestContext.is_image_visible(), so avoid recursive calls
         return session.query(models.ImageMember).\
                         options(joinedload(models.ImageMember.image)).\
-                        filter_by(deleted=_deleted(context)).\
                         filter_by(image_id=image_id).\
                         filter_by(member=member).\
                         one()

glance/registry/server.py

@@ -422,8 +422,9 @@ def replace_members(self, req, image_id, body):
             raise exc.HTTPUnauthorized(_("No authenticated user"))
 
         # Make sure the image exists
+        session = db_api.get_session()
         try:
-            image = db_api.image_get(req.context, image_id)
+            image = db_api.image_get(req.context, image_id, session=session)
         except exception.NotFound:
             raise exc.HTTPNotFound()
         except exception.NotAuthorized:
@@ -468,7 +469,8 @@ def replace_members(self, req, image_id, body):
             try:
                 membership = db_api.image_member_find(req.context,
                                                       datum['image_id'],
-                                                      datum['member_id'])
+                                                      datum['member'],
+                                                      session=session)
 
                 # Are we overriding can_share?
                 if datum['can_share'] is None:
@@ -490,14 +492,15 @@ def replace_members(self, req, image_id, body):
             if memb['id'] in existing:
                 # Just update the membership in place
                 update = existing[memb['id']]['values']
-                db_api.image_member_update(req.context, memb, update)
+                db_api.image_member_update(req.context, memb, update,
+                                           session=session)
             else:
                 # Outdated one; needs to be deleted
-                db_api.image_member_delete(memb)
+                db_api.image_member_delete(req.context, memb, session=session)
 
         # Now add the non-existant ones
         for memb in add:
-            db_api.image_member_create(req.context, memb)
+            db_api.image_member_create(req.context, memb, session=session)
 
         # Make an appropriate result
         return exc.HTTPNoContent()
@@ -550,15 +553,18 @@ def add_member(self, req, image_id, member, body=None):
 
         # Look up an existing membership...
         try:
+            session = db_api.get_session()
             membership = db_api.image_member_find(req.context,
-                                                  image_id, member)
+                                                  image_id, member,
+                                                  session=session)
             if can_share is not None:
                 values = dict(can_share=can_share)
-                db_api.image_member_update(req.context, membership, values)
+                db_api.image_member_update(req.context, membership, values,
+                                           session=session)
         except exception.NotFound:
             values = dict(image_id=image['id'], member=member,
                           can_share=bool(can_share))
-            db_api.image_member_create(req.context, values)
+            db_api.image_member_create(req.context, values, session=session)
 
         # Make an appropriate result
         return exc.HTTPNoContent()
@@ -592,9 +598,14 @@ def delete_member(self, req, image_id, member):
 
         # Look up an existing membership
         try:
-            membership = db_api.image_member_find(req.context,
-                                                  image_id, member)
-            db_api.image_member_delete(req.context, membership)
+            session = db_api.get_session()
+            member_ref = db_api.image_member_find(req.context,
+                                                  image_id,
+                                                  member,
+                                                  session=session)
+            db_api.image_member_delete(req.context,
+                                       member_ref,
+                                       session=session)
         except exception.NotFound:
             pass
 
@@ -665,10 +676,12 @@ def make_member_list(members, **attr_map):
     """
 
     def _fetch_memb(memb, attr_map):
-        return dict([(k, memb[v]) for k, v in attr_map if v in memb.keys()])
+        return dict([(k, memb[v]) for k, v in attr_map.items()
+                                  if v in memb.keys()])
 
     # Return the list of members with the given attribute mapping
-    return [_fetch_memb(memb, attr_map) for memb in members]
+    return [_fetch_memb(memb, attr_map) for memb in members
+            if not memb.deleted]
 
 
 def app_factory(global_conf, **local_conf):