Skip to content

Commit

Permalink
Merge "Wrapped unexpected exceptions (bug 955411)"
Browse files Browse the repository at this point in the history
  • Loading branch information
Jenkins authored and openstack-gerrit committed Mar 20, 2012
2 parents da04fc0 + 009d661 commit 5ea232a
Show file tree
Hide file tree
Showing 14 changed files with 121 additions and 55 deletions.
2 changes: 1 addition & 1 deletion keystone/catalog/backends/sql.py
Expand Up @@ -125,7 +125,7 @@ def delete_endpoint(self, endpoint_id):
endpoint_ref = session.query(Endpoint)
endpoint_ref = endpoint_ref.filter_by(id=endpoint_id).first()
if not endpoint_ref:
raise exception.NotFound('Endpoint not found')
raise exception.EndpointNotFound(endpoint_id=endpoint_id)
with session.begin():
session.delete(endpoint_ref)
session.flush()
Expand Down
11 changes: 3 additions & 8 deletions keystone/catalog/core.py
Expand Up @@ -19,8 +19,6 @@

import uuid

import webob.exc

from keystone import config
from keystone import exception
from keystone import identity
Expand Down Expand Up @@ -131,7 +129,7 @@ def get_services(self, context):
def get_service(self, context, service_id):
service_ref = self.catalog_api.get_service(context, service_id)
if not service_ref:
raise webob.exc.HTTPNotFound()
raise exception.ServiceNotFound(service_id=service_id)
return {'OS-KSADM:service': service_ref}

def delete_service(self, context, service_id):
Expand Down Expand Up @@ -168,11 +166,8 @@ def create_endpoint(self, context, endpoint):
endpoint_ref['id'] = endpoint_id

service_id = endpoint_ref['service_id']
try:
service = self.catalog_api.get_service(context, service_id)
except exception.ServiceNotFound:
msg = 'No service exists with id %s' % service_id
raise webob.exc.HTTPBadRequest(msg)
if not self.catalog_api.service_exists(context, service_id):
raise exception.ServiceNotFound(service_id=service_id)

new_endpoint_ref = self.catalog_api.create_endpoint(
context, endpoint_id, endpoint_ref)
Expand Down
11 changes: 7 additions & 4 deletions keystone/common/ldap/core.py
Expand Up @@ -16,6 +16,7 @@

import ldap

from keystone import exception
from keystone.common import logging
from keystone.common.ldap import fakeldap

Expand Down Expand Up @@ -140,14 +141,16 @@ def affirm_unique(self, values):
if values['name'] is not None:
entity = self.get_by_name(values['name'])
if entity is not None:
raise Exception('%s with id %s already exists'
% (self.options_name, values['id']))
raise exception.Conflict(type=self.options_name,
details='Duplicate name, %s.' %
values['name'])

if values['id'] is not None:
entity = self.get(values['id'])
if entity is not None:
raise Exception('%s with id %s already exists'
% (self.options_name, values['id']))
raise exception.Conflict(type=self.options_name,
details='Duplicate ID, %s.' %
values['id'])

def create(self, values):
conn = self.get_connection()
Expand Down
1 change: 1 addition & 0 deletions keystone/common/sql/core.py
Expand Up @@ -43,6 +43,7 @@
String = sql.String
ForeignKey = sql.ForeignKey
DateTime = sql.DateTime
IntegrityError = sql.exc.IntegrityError


# Special Fields
Expand Down
3 changes: 3 additions & 0 deletions keystone/common/wsgi.py
Expand Up @@ -185,6 +185,9 @@ def __call__(self, req):
except exception.Error as e:
LOG.warning(e)
return render_exception(e)
except Exception as e:
logging.exception(e)
return render_exception(exception.UnexpectedError(exception=e))

if result is None:
return render_response(status=(204, 'No Content'))
Expand Down
20 changes: 7 additions & 13 deletions keystone/contrib/ec2/core.py
Expand Up @@ -36,8 +36,6 @@

import uuid

import webob.exc

from keystone import catalog
from keystone import config
from keystone import exception
Expand Down Expand Up @@ -114,12 +112,9 @@ def check_signature(self, creds_ref, credentials):
credentials['host'] = hostname
signature = signer.generate(credentials)
if not utils.auth_str_equal(credentials.signature, signature):
# TODO(termie): proper exception
msg = 'Invalid signature'
raise webob.exc.HTTPUnauthorized(explanation=msg)
raise exception.Unauthorized(message='Invalid EC2 signature.')
else:
msg = 'Signature not supplied'
raise webob.exc.HTTPUnauthorized(explanation=msg)
raise exception.Unauthorized(message='EC2 signature not supplied.')

def authenticate(self, context, credentials=None,
ec2Credentials=None):
Expand Down Expand Up @@ -152,8 +147,7 @@ def authenticate(self, context, credentials=None,
creds_ref = self.ec2_api.get_credential(context,
credentials['access'])
if not creds_ref:
msg = 'Access key not found'
raise webob.exc.HTTPUnauthorized(explanation=msg)
raise exception.Unauthorized(message='EC2 access key not found.')

self.check_signature(creds_ref, credentials)

Expand Down Expand Up @@ -263,7 +257,7 @@ def _assert_identity(self, context, user_id):
:param context: standard context
:param user_id: id of user
:raises webob.exc.HTTPForbidden: when token is invalid
:raises exception.Forbidden: when token is invalid
"""
try:
Expand All @@ -273,7 +267,7 @@ def _assert_identity(self, context, user_id):
raise exception.Unauthorized()
token_user_id = token_ref['user'].get('id')
if not token_user_id == user_id:
raise webob.exc.HTTPForbidden()
raise exception.Forbidden()

def _is_admin(self, context):
"""Wrap admin assertion error return statement.
Expand All @@ -294,9 +288,9 @@ def _assert_owner(self, context, user_id, credential_id):
:param context: standard context
:param user_id: expected credential owner
:param credential_id: id of credential object
:raises webob.exc.HTTPForbidden: on failure
:raises exception.Forbidden: on failure
"""
cred_ref = self.ec2_api.get_credential(context, credential_id)
if not user_id == cred_ref['user_id']:
raise webob.exc.HTTPForbidden()
raise exception.Forbidden()
50 changes: 45 additions & 5 deletions keystone/exception.py
Expand Up @@ -58,26 +58,66 @@ class Unauthorized(Error):


class Forbidden(Error):
"""You are not authorized to perform the requested action: %(action)s"""
"""You are not authorized to perform the requested action."""
code = 403
title = 'Not Authorized'


class ForbiddenAction(Forbidden):
"""You are not authorized to perform the requested action: %(action)s"""


class NotFound(Error):
"""Could not find: %(target)s"""
code = 404
title = 'Not Found'


class EndpointNotFound(NotFound):
"""Could not find endopint: %(endpoint_id)s"""


class RoleNotFound(NotFound):
"""Could not find role: %(role_id)s"""


class ServiceNotFound(NotFound):
"""Could not find service: %(service_id)s"""


class TenantNotFound(NotFound):
"""Could not find tenant: %(tenant_id)s"""


class TokenNotFound(NotFound):
"""Could not find token: %(token_id)s"""


class UserNotFound(NotFound):
"""Could not find user: %(user_id)s"""


class Conflict(Error):
"""Conflict occurred attempting to store %(type)s.
%(details)s
"""
code = 409
title = 'Conflict'


class NotImplemented(Error):
"""The action you have requested has not been implemented."""
code = 501
action = 'Not Implemented'


class TokenNotFound(NotFound):
"""Could not find token: %(token_id)s"""
class UnexpectedError(Error):
"""An unexpected error prevented the server from fulfilling your request.
%(exception)s
class ServiceNotFound(NotFound):
"""Could not find service: %(service_id)s"""
"""
code = 500
title = 'Internal Server Error'
19 changes: 13 additions & 6 deletions keystone/identity/backends/kvs.py
Expand Up @@ -14,6 +14,7 @@
# License for the specific language governing permissions and limitations
# under the License.

from keystone import exception
from keystone import identity
from keystone.common import kvs
from keystone.common import utils
Expand Down Expand Up @@ -151,9 +152,11 @@ def remove_role_from_user_and_tenant(self, user_id, tenant_id, role_id):
# CRUD
def create_user(self, user_id, user):
if self.get_user(user_id):
raise Exception('Duplicate id')
msg = 'Duplicate ID, %s.' % user_id
raise exception.Conflict(type='user', details=msg)
if self.get_user_by_name(user['name']):
raise Exception('Duplicate name')
msg = 'Duplicate name, %s.' % user['name']
raise exception.Conflict(type='user', details=msg)
user = _ensure_hashed_password(user)
self.db.set('user-%s' % user_id, user)
self.db.set('user_name-%s' % user['name'], user)
Expand All @@ -166,7 +169,8 @@ def update_user(self, user_id, user):
if 'name' in user:
existing = self.db.get('user_name-%s' % user['name'])
if existing and user_id != existing['id']:
raise Exception('Duplicate name')
msg = 'Duplicate name, %s.' % user['name']
raise exception.Conflict(type='user', details=msg)
# get the old name and delete it too
old_user = self.db.get('user-%s' % user_id)
new_user = old_user.copy()
Expand All @@ -189,9 +193,11 @@ def delete_user(self, user_id):

def create_tenant(self, tenant_id, tenant):
if self.get_tenant(tenant_id):
raise Exception('Duplicate id')
msg = 'Duplicate ID, %s.' % tenant_id
raise exception.Conflict(type='tenant', details=msg)
if self.get_tenant_by_name(tenant['name']):
raise Exception('Duplicate name')
msg = 'Duplicate name, %s.' % tenant['name']
raise exception.Conflict(type='tenant', details=msg)
self.db.set('tenant-%s' % tenant_id, tenant)
self.db.set('tenant_name-%s' % tenant['name'], tenant)
return tenant
Expand All @@ -200,7 +206,8 @@ def update_tenant(self, tenant_id, tenant):
if 'name' in tenant:
existing = self.db.get('tenant_name-%s' % tenant['name'])
if existing and tenant_id != existing['id']:
raise Exception('Duplicate name')
msg = 'Duplicate name, %s.' % tenant['name']
raise exception.Conflict(type='tenant', details=msg)
# get the old name and delete it too
old_tenant = self.db.get('tenant-%s' % tenant_id)
new_tenant = old_tenant.copy()
Expand Down
2 changes: 1 addition & 1 deletion keystone/identity/backends/ldap/core.py
Expand Up @@ -545,7 +545,7 @@ def get_by_name(self, name, filter=None):
def add_user(self, role_id, user_id, tenant_id=None):
user = self.user_api.get(user_id)
if user is None:
raise exception.NotFound('User %s not found' % (user_id,))
raise exception.UserNotFound(user_id=user_id)
role_dn = self._subrole_id_to_dn(role_id, tenant_id)
conn = self.get_connection()
user_dn = self.user_api._id_to_dn(user_id)
Expand Down
23 changes: 23 additions & 0 deletions keystone/identity/backends/sql.py
Expand Up @@ -15,8 +15,10 @@
# under the License.

import copy
import functools

from keystone import identity
from keystone import exception
from keystone.common import sql
from keystone.common import utils
from keystone.common.sql import migration
Expand All @@ -35,6 +37,19 @@ def _ensure_hashed_password(user_ref):
return user_ref


def handle_conflicts(type='object'):
"""Converts IntegrityError into HTTP 409 Conflict."""
def decorator(method):
@functools.wraps(method)
def wrapper(*args, **kwargs):
try:
return method(*args, **kwargs)
except sql.IntegrityError as e:
raise exception.Conflict(type=type, details=str(e))
return wrapper
return decorator


class User(sql.ModelBase, sql.DictBase):
__tablename__ = 'user'
id = sql.Column(sql.String(64), primary_key=True)
Expand Down Expand Up @@ -280,6 +295,7 @@ def remove_role_from_user_and_tenant(self, user_id, tenant_id, role_id):
self.create_metadata(user_id, tenant_id, metadata_ref)

# CRUD
@handle_conflicts(type='user')
def create_user(self, user_id, user):
user = _ensure_hashed_password(user)
session = self.get_session()
Expand All @@ -289,6 +305,7 @@ def create_user(self, user_id, user):
session.flush()
return user_ref.to_dict()

@handle_conflicts(type='user')
def update_user(self, user_id, user):
session = self.get_session()
with session.begin():
Expand All @@ -311,6 +328,7 @@ def delete_user(self, user_id):
session.delete(user_ref)
session.flush()

@handle_conflicts(type='tenant')
def create_tenant(self, tenant_id, tenant):
session = self.get_session()
with session.begin():
Expand All @@ -319,6 +337,7 @@ def create_tenant(self, tenant_id, tenant):
session.flush()
return tenant_ref.to_dict()

@handle_conflicts(type='tenant')
def update_tenant(self, tenant_id, tenant):
session = self.get_session()
with session.begin():
Expand All @@ -340,6 +359,7 @@ def delete_tenant(self, tenant_id):
session.delete(tenant_ref)
session.flush()

@handle_conflicts(type='metadata')
def create_metadata(self, user_id, tenant_id, metadata):
session = self.get_session()
with session.begin():
Expand All @@ -349,6 +369,7 @@ def create_metadata(self, user_id, tenant_id, metadata):
session.flush()
return metadata

@handle_conflicts(type='metadata')
def update_metadata(self, user_id, tenant_id, metadata):
session = self.get_session()
with session.begin():
Expand All @@ -367,13 +388,15 @@ def delete_metadata(self, user_id, tenant_id):
self.db.delete('metadata-%s-%s' % (tenant_id, user_id))
return None

@handle_conflicts(type='role')
def create_role(self, role_id, role):
session = self.get_session()
with session.begin():
session.add(Role(**role))
session.flush()
return role

@handle_conflicts(type='role')
def update_role(self, role_id, role):
session = self.get_session()
with session.begin():
Expand Down

0 comments on commit 5ea232a

Please sign in to comment.