From 7f666230e37745b174998a485fe1d7626c4862ae Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Wed, 25 Aug 2010 16:45:59 +0000 Subject: A few small changes to install_venv to let venv builds work on the tarmac box. --- tools/install_venv.py | 4 ++++ tools/pip-requires | 1 + 2 files changed, 5 insertions(+) diff --git a/tools/install_venv.py b/tools/install_venv.py index 1f0fa3cc7..e764efff6 100644 --- a/tools/install_venv.py +++ b/tools/install_venv.py @@ -88,6 +88,10 @@ def create_virtualenv(venv=VENV): def install_dependencies(venv=VENV): print 'Installing dependencies with pip (this can take a while)...' + # Install greenlet by hand - just listing it in the requires file does not + # get it in stalled in the right order + run_command(['tools/with_venv.sh', 'pip', 'install', '-E', venv, 'greenlet'], + redirect_output=False) run_command(['tools/with_venv.sh', 'pip', 'install', '-E', venv, '-r', PIP_REQUIRES], redirect_output=False) run_command(['tools/with_venv.sh', 'pip', 'install', '-E', venv, TWISTED_NOVA], diff --git a/tools/pip-requires b/tools/pip-requires index 13e8e5f45..9853252dc 100644 --- a/tools/pip-requires +++ b/tools/pip-requires @@ -18,3 +18,4 @@ wsgiref==0.1.2 zope.interface==3.6.1 mox==0.5.0 -f http://pymox.googlecode.com/files/mox-0.5.0.tar.gz +greenlet==0.3.1 -- cgit From 9330ebc110aeb7591567c66939b39f4345b5778d Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Fri, 10 Sep 2010 04:52:48 -0700 Subject: added modify project command to allow project manager and description to be updated --- nova/auth/fakeldap.py | 5 ++++- nova/auth/ldapdriver.py | 18 ++++++++++++++++++ nova/auth/manager.py | 20 ++++++++++++++++++++ nova/tests/auth_unittest.py | 6 ++++++ 4 files changed, 48 insertions(+), 1 deletion(-) diff --git a/nova/auth/fakeldap.py b/nova/auth/fakeldap.py index bfc3433c5..2791dfde6 100644 --- a/nova/auth/fakeldap.py +++ b/nova/auth/fakeldap.py @@ -33,6 +33,7 @@ SCOPE_ONELEVEL = 1 # not implemented SCOPE_SUBTREE = 2 MOD_ADD = 0 MOD_DELETE = 1 +MOD_REPLACE = 2 class NO_SUCH_OBJECT(Exception): # pylint: disable-msg=C0103 @@ -175,7 +176,7 @@ class FakeLDAP(object): Args: dn -- a dn attrs -- a list of tuples in the following form: - ([MOD_ADD | MOD_DELETE], attribute, value) + ([MOD_ADD | MOD_DELETE | MOD_REPACE], attribute, value) """ redis = datastore.Redis.instance() @@ -185,6 +186,8 @@ class FakeLDAP(object): values = _from_json(redis.hget(key, k)) if cmd == MOD_ADD: values.append(v) + elif cmd == MOD_REPLACE: + values = [v] else: values.remove(v) values = redis.hset(key, k, _to_json(values)) diff --git a/nova/auth/ldapdriver.py b/nova/auth/ldapdriver.py index 74ba011b5..cc8e2caa3 100644 --- a/nova/auth/ldapdriver.py +++ b/nova/auth/ldapdriver.py @@ -202,6 +202,24 @@ class LdapDriver(object): self.conn.add_s('cn=%s,%s' % (name, FLAGS.ldap_project_subtree), attr) return self.__to_project(dict(attr)) + def modify_project(self, project_id, manager_uid=None, description=None): + """Modify an existing project""" + if not manager_uid and not description: + return + attr = [] + if manager_uid: + if not self.__user_exists(manager_uid): + raise exception.NotFound("Project can't be modified because " + "manager %s doesn't exist" % + manager_uid) + manager_dn = self.__uid_to_dn(manager_uid) + attr.append((self.ldap.MOD_REPLACE, 'projectManager', manager_dn)) + if description: + attr.append((self.ldap.MOD_REPLACE, 'description', description)) + self.conn.modify_s('cn=%s,%s' % (project_id, + FLAGS.ldap_project_subtree), + attr) + def add_to_project(self, uid, project_id): """Add user to project""" dn = 'cn=%s,%s' % (project_id, FLAGS.ldap_project_subtree) diff --git a/nova/auth/manager.py b/nova/auth/manager.py index 284b29502..d094bb7e1 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -525,6 +525,26 @@ class AuthManager(object): if project_dict: return Project(**project_dict) + def modify_project(self, project, manager_user=None, description=None): + """Modify a project + + @type name: Project or project_id + @param project: The project to modify. + + @type manager_user: User or uid + @param manager_user: This user will be the new project manager. + + @type description: str + @param project: This will be the new description of the project. + + """ + if manager_user: + manager_user = User.safe_id(manager_user) + with self.driver() as drv: + drv.modify_project(Project.safe_id(project), + manager_user, + description) + def add_to_project(self, user, project): """Add user to project""" with self.driver() as drv: diff --git a/nova/tests/auth_unittest.py b/nova/tests/auth_unittest.py index 0b404bfdc..2fc780640 100644 --- a/nova/tests/auth_unittest.py +++ b/nova/tests/auth_unittest.py @@ -206,6 +206,12 @@ class AuthTestCase(test.BaseTestCase): self.assert_(len(self.manager.get_projects()) > 1) self.assertEqual(len(self.manager.get_projects('test2')), 1) + def test_220_can_modify_project(self): + self.manager.modify_project('testproj', 'test2', 'new description') + project = self.manager.get_project('testproj') + self.assertEqual(project.project_manager_id, 'test2') + self.assertEqual(project.description, 'new description') + def test_299_can_delete_project(self): self.manager.delete_project('testproj') self.assertFalse(filter(lambda p: p.name == 'testproj', self.manager.get_projects())) -- cgit From bc265bbc9b3b42e46e044c18252218a375192123 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Fri, 10 Sep 2010 17:12:49 -0700 Subject: multi-region flag for describe regions --- nova/endpoint/cloud.py | 15 ++++++++++++--- nova/flags.py | 15 +++++++++------ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 8e2beb1e3..180af0540 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -174,9 +174,18 @@ class CloudController(object): @rbac.allow('all') def describe_regions(self, context, region_name=None, **kwargs): - # TODO(vish): region_name is an array. Support filtering - return {'regionInfo': [{'regionName': 'nova', - 'regionUrl': FLAGS.ec2_url}]} + if FLAGS.region_list: + regions = [] + for region in FLAGS.region_list: + name, _sep, url = region.partition(',') + regions.append({'regionName': name, + 'regionUrl': url}) + else: + regions = [{'regionName': 'nova', + 'regionUrl': FLAGS.ec2_url}] + if region_name: + regions = [r for r in regions if r['regionName'] in region_name] + return {'regionInfo': regions } @rbac.allow('all') def describe_snapshots(self, diff --git a/nova/flags.py b/nova/flags.py index 2bca36f7e..19dcb96ba 100644 --- a/nova/flags.py +++ b/nova/flags.py @@ -34,7 +34,7 @@ class FlagValues(gflags.FlagValues): Unknown flags will be ignored when parsing the command line, but the command line will be kept so that it can be replayed if new flags are defined after the initial parsing. - + """ def __init__(self): @@ -50,7 +50,7 @@ class FlagValues(gflags.FlagValues): # leftover args at the end sneaky_unparsed_args = {"value": None} original_argv = list(argv) - + if self.IsGnuGetOpt(): orig_getopt = getattr(getopt, 'gnu_getopt') orig_name = 'gnu_getopt' @@ -81,7 +81,7 @@ class FlagValues(gflags.FlagValues): args = argv[:1] finally: setattr(getopt, orig_name, orig_getopt) - + # Store the arguments for later, we'll need them for new flags # added at runtime self.__dict__['__stored_argv'] = original_argv @@ -92,7 +92,7 @@ class FlagValues(gflags.FlagValues): def SetDirty(self, name): """Mark a flag as dirty so that accessing it will case a reparse.""" self.__dict__['__dirty'].append(name) - + def IsDirty(self, name): return name in self.__dict__['__dirty'] @@ -113,12 +113,12 @@ class FlagValues(gflags.FlagValues): for k in self.__dict__['__dirty']: setattr(self, k, getattr(new_flags, k)) self.ClearDirty() - + def __setitem__(self, name, flag): gflags.FlagValues.__setitem__(self, name, flag) if self.WasAlreadyParsed(): self.SetDirty(name) - + def __getitem__(self, name): if self.IsDirty(name): self.ParseNewFlags() @@ -166,6 +166,9 @@ def DECLARE(name, module_string, flag_values=FLAGS): # Define any app-specific flags in their own files, docs at: # http://code.google.com/p/python-gflags/source/browse/trunk/gflags.py#39 +DEFINE_list('region_list', + [], + 'list of region,url pairs') DEFINE_string('connection_type', 'libvirt', 'libvirt, xenapi or fake') DEFINE_integer('s3_port', 3333, 's3 port') DEFINE_string('s3_host', '127.0.0.1', 's3 host') -- cgit From ee206cd08bd2d82bb5d64b84b6804ba51ab56b37 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Fri, 10 Sep 2010 18:51:22 -0700 Subject: moved keypairs to db using the same interface --- nova/auth/manager.py | 36 +++++++++++++++--------------------- nova/db/api.py | 23 +++++++++++++++++++++++ nova/db/sqlalchemy/api.py | 32 ++++++++++++++++++++++++++++++++ nova/db/sqlalchemy/models.py | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 21 deletions(-) diff --git a/nova/auth/manager.py b/nova/auth/manager.py index d5fbec7c5..4cb23bea6 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -668,42 +668,36 @@ class AuthManager(object): with self.driver() as drv: if not drv.get_user(uid): raise exception.NotFound("User %s doesn't exist" % user) - if drv.get_key_pair(uid, key_name): - raise exception.Duplicate("The keypair %s already exists" - % key_name) + try: + db.keypair_get(None, uid, key_name) + raise exception.Duplicate("The keypair %s already exists" + % key_name) + except exception.NotFound: + pass private_key, public_key, fingerprint = crypto.generate_key_pair() self.create_key_pair(uid, key_name, public_key, fingerprint) return private_key, fingerprint def create_key_pair(self, user, key_name, public_key, fingerprint): """Creates a key pair for user""" - with self.driver() as drv: - kp_dict = drv.create_key_pair(User.safe_id(user), - key_name, - public_key, - fingerprint) - if kp_dict: - return KeyPair(**kp_dict) + key = {} + key['user_id'] = User.safe_id(user) + key['name'] = key_name + key['public_key'] = public_key + key['fingerprint'] = fingerprint + return db.keypair_create(None, key) def get_key_pair(self, user, key_name): """Retrieves a key pair for user""" - with self.driver() as drv: - kp_dict = drv.get_key_pair(User.safe_id(user), key_name) - if kp_dict: - return KeyPair(**kp_dict) + return db.keypair_get(None, User.safe_id(user), key_name) def get_key_pairs(self, user): """Retrieves all key pairs for user""" - with self.driver() as drv: - kp_list = drv.get_key_pairs(User.safe_id(user)) - if not kp_list: - return [] - return [KeyPair(**kp_dict) for kp_dict in kp_list] + return db.keypair_get_all_by_user(None, User.safe_id(user)) def delete_key_pair(self, user, key_name): """Deletes a key pair for user""" - with self.driver() as drv: - drv.delete_key_pair(User.safe_id(user), key_name) + return db.keypair_destroy(None, User.safe_id(user), key_name) def get_credentials(self, user, project=None): """Get credential zip for user in project""" diff --git a/nova/db/api.py b/nova/db/api.py index d81673fad..1db978c52 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -254,6 +254,29 @@ def instance_update(context, instance_id, values): return IMPL.instance_update(context, instance_id, values) +################### + + +def keypair_create(context, values): + """Create a keypair from the values dictionary.""" + return IMPL.keypair_create(context, values) + + +def keypair_destroy(context, user_id, name): + """Destroy the keypair or raise if it does not exist.""" + return IMPL.keypair_destroy(context, user_id, name) + + +def keypair_get(context, user_id, name): + """Get a keypair or raise if it does not exist.""" + return IMPL.keypair_get(context, user_id, name) + + +def keypair_get_all_by_user(context, user_id): + """Get all keypairs by user.""" + return IMPL.keypair_get_all_by_user(context, user_id) + + #################### diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 02ebdd222..b3a307043 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -355,6 +355,38 @@ def instance_update(_context, instance_id, values): ################### +def keypair_create(_context, values): + keypair_ref = models.Keypair() + for (key, value) in values.iteritems(): + keypair_ref[key] = value + keypair_ref.save() + return keypair_ref + + +def keypair_destroy(_context, user_id, name): + session = get_session() + with session.begin(): + keypair_ref = models.Keypair.find_by_args(user_id, + name, + session=session) + keypair_ref.delete(session=session) + + +def keypair_get(_context, user_id, name): + return models.Keypair.find_by_args(user_id, name) + + +def keypair_get_all_by_user(_context, user_id): + session = get_session() + return session.query(models.Keypair + ).filter_by(user_id=user_id + ).filter_by(deleted=False + ).all() + + +################### + + def network_count(_context): return models.Network.count() diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 6818f838c..81c0a77a8 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -284,6 +284,42 @@ class ExportDevice(BASE, NovaBase): uselist=False)) +class Keypair(BASE, NovaBase): + """Represents a keypair""" + __tablename__ = 'keypairs' + id = Column(Integer, primary_key=True) + name = Column(String(255)) + + user_id = Column(String(255)) + + fingerprint = Column(String(255)) + public_key = Column(Text) + + @property + def str_id(self): + return '%s.%s' % (self.user_id, self.name) + + @classmethod + def find_by_str(cls, str_id, session=None, deleted=False): + user_id, _sep, name = str_id.partition('.') + return cls.find_by_str(user_id, name, session, deleted) + + @classmethod + def find_by_args(cls, user_id, name, session=None, deleted=False): + if not session: + session = get_session() + try: + return session.query(cls + ).filter_by(user_id=user_id + ).filter_by(name=name + ).filter_by(deleted=deleted + ).one() + except exc.NoResultFound: + new_exc = exception.NotFound("No model for user %s, name %s" % + (user_id, name)) + raise new_exc.__class__, new_exc, sys.exc_info()[2] + + class Network(BASE, NovaBase): """Represents a network""" __tablename__ = 'networks' -- cgit From d3273e594daf5f94f09c7904bac53fbb895ffeb6 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Fri, 10 Sep 2010 18:55:11 -0700 Subject: remove keypair from driver --- nova/auth/ldapdriver.py | 60 ------------------------------------------------- nova/auth/manager.py | 23 ------------------- 2 files changed, 83 deletions(-) diff --git a/nova/auth/ldapdriver.py b/nova/auth/ldapdriver.py index 74ba011b5..4e9afc858 100644 --- a/nova/auth/ldapdriver.py +++ b/nova/auth/ldapdriver.py @@ -99,13 +99,6 @@ class LdapDriver(object): dn = FLAGS.ldap_user_subtree return self.__to_user(self.__find_object(dn, query)) - def get_key_pair(self, uid, key_name): - """Retrieve key pair by uid and key name""" - dn = 'cn=%s,%s' % (key_name, - self.__uid_to_dn(uid)) - attr = self.__find_object(dn, '(objectclass=novaKeyPair)') - return self.__to_key_pair(uid, attr) - def get_project(self, pid): """Retrieve project by id""" dn = 'cn=%s,%s' % (pid, @@ -119,12 +112,6 @@ class LdapDriver(object): '(objectclass=novaUser)') return [self.__to_user(attr) for attr in attrs] - def get_key_pairs(self, uid): - """Retrieve list of key pairs""" - attrs = self.__find_objects(self.__uid_to_dn(uid), - '(objectclass=novaKeyPair)') - return [self.__to_key_pair(uid, attr) for attr in attrs] - def get_projects(self, uid=None): """Retrieve list of projects""" pattern = '(objectclass=novaProject)' @@ -154,21 +141,6 @@ class LdapDriver(object): self.conn.add_s(self.__uid_to_dn(name), attr) return self.__to_user(dict(attr)) - def create_key_pair(self, uid, key_name, public_key, fingerprint): - """Create a key pair""" - # TODO(vish): possibly refactor this to store keys in their own ou - # and put dn reference in the user object - attr = [ - ('objectclass', ['novaKeyPair']), - ('cn', [key_name]), - ('sshPublicKey', [public_key]), - ('keyFingerprint', [fingerprint]), - ] - self.conn.add_s('cn=%s,%s' % (key_name, - self.__uid_to_dn(uid)), - attr) - return self.__to_key_pair(uid, dict(attr)) - def create_project(self, name, manager_uid, description=None, member_uids=None): """Create a project""" @@ -265,19 +237,10 @@ class LdapDriver(object): """Delete a user""" if not self.__user_exists(uid): raise exception.NotFound("User %s doesn't exist" % uid) - self.__delete_key_pairs(uid) self.__remove_from_all(uid) self.conn.delete_s('uid=%s,%s' % (uid, FLAGS.ldap_user_subtree)) - def delete_key_pair(self, uid, key_name): - """Delete a key pair""" - if not self.__key_pair_exists(uid, key_name): - raise exception.NotFound("Key Pair %s doesn't exist for user %s" % - (key_name, uid)) - self.conn.delete_s('cn=%s,uid=%s,%s' % (key_name, uid, - FLAGS.ldap_user_subtree)) - def delete_project(self, project_id): """Delete a project""" project_dn = 'cn=%s,%s' % (project_id, FLAGS.ldap_project_subtree) @@ -288,10 +251,6 @@ class LdapDriver(object): """Check if user exists""" return self.get_user(uid) != None - def __key_pair_exists(self, uid, key_name): - """Check if key pair exists""" - return self.get_key_pair(uid, key_name) != None - def __project_exists(self, project_id): """Check if project exists""" return self.get_project(project_id) != None @@ -341,13 +300,6 @@ class LdapDriver(object): """Check if group exists""" return self.__find_object(dn, '(objectclass=groupOfNames)') != None - def __delete_key_pairs(self, uid): - """Delete all key pairs for user""" - keys = self.get_key_pairs(uid) - if keys != None: - for key in keys: - self.delete_key_pair(uid, key['name']) - @staticmethod def __role_to_dn(role, project_id=None): """Convert role to corresponding dn""" @@ -472,18 +424,6 @@ class LdapDriver(object): 'secret': attr['secretKey'][0], 'admin': (attr['isAdmin'][0] == 'TRUE')} - @staticmethod - def __to_key_pair(owner, attr): - """Convert ldap attributes to KeyPair object""" - if attr == None: - return None - return { - 'id': attr['cn'][0], - 'name': attr['cn'][0], - 'owner_id': owner, - 'public_key': attr['sshPublicKey'][0], - 'fingerprint': attr['keyFingerprint'][0]} - def __to_project(self, attr): """Convert ldap attributes to Project object""" if attr == None: diff --git a/nova/auth/manager.py b/nova/auth/manager.py index 4cb23bea6..ef6a5a486 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -154,29 +154,6 @@ class User(AuthBase): self.admin) -class KeyPair(AuthBase): - """Represents an ssh key returned from the datastore - - Even though this object is named KeyPair, only the public key and - fingerprint is stored. The user's private key is not saved. - """ - - def __init__(self, id, name, owner_id, public_key, fingerprint): - AuthBase.__init__(self) - self.id = id - self.name = name - self.owner_id = owner_id - self.public_key = public_key - self.fingerprint = fingerprint - - def __repr__(self): - return "KeyPair('%s', '%s', '%s', '%s', '%s')" % (self.id, - self.name, - self.owner_id, - self.public_key, - self.fingerprint) - - class Project(AuthBase): """Represents a Project returned from the datastore""" -- cgit From adb9cf9e71908844fd720e6f9bab9588610878e1 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Fri, 10 Sep 2010 19:03:35 -0700 Subject: delete keypairs when a user is deleted --- nova/auth/manager.py | 8 ++++++-- nova/db/api.py | 5 +++++ nova/db/sqlalchemy/api.py | 8 ++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/nova/auth/manager.py b/nova/auth/manager.py index ef6a5a486..e2bb748b0 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -620,9 +620,13 @@ class AuthManager(object): return User(**user_dict) def delete_user(self, user): - """Deletes a user""" + """Deletes a user + + Additionally deletes all users keypairs""" + uid = User.safe_id(user) + db.keypair_destroy_all_by_user(None, uid) with self.driver() as drv: - drv.delete_user(User.safe_id(user)) + drv.delete_user(uid) def generate_key_pair(self, user, key_name): """Generates a key pair for a user diff --git a/nova/db/api.py b/nova/db/api.py index 1db978c52..e96d803db 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -267,6 +267,11 @@ def keypair_destroy(context, user_id, name): return IMPL.keypair_destroy(context, user_id, name) +def keypair_destroy_all_by_user(context, user_id): + """Destroy all keypairs by user.""" + return IMPL.keypair_destroy_all_by_user(context, user_id) + + def keypair_get(context, user_id, name): """Get a keypair or raise if it does not exist.""" return IMPL.keypair_get(context, user_id, name) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index b3a307043..4fd1bf216 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -372,6 +372,14 @@ def keypair_destroy(_context, user_id, name): keypair_ref.delete(session=session) +def keypair_destroy_all_by_user(_context, user_id): + session = get_session() + with session.begin(): + # TODO(vish): do we have to use sql here? + session.execute('update keypairs set deleted=1 where user_id=:id', + {'id': user_id}) + + def keypair_get(_context, user_id, name): return models.Keypair.find_by_args(user_id, name) -- cgit From 8e834931087c54585a7aa2716c7a0708fd658f30 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Fri, 10 Sep 2010 22:13:36 -0700 Subject: move keypair generation out of auth and fix tests --- nova/auth/manager.py | 70 -------------------------------------------- nova/endpoint/cloud.py | 48 +++++++++++++++++++++--------- nova/tests/api_unittest.py | 7 +++-- nova/tests/auth_unittest.py | 31 -------------------- nova/tests/cloud_unittest.py | 53 ++++++++++++++++++++++++++++----- 5 files changed, 83 insertions(+), 126 deletions(-) diff --git a/nova/auth/manager.py b/nova/auth/manager.py index e2bb748b0..fb87847d5 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -128,24 +128,6 @@ class User(AuthBase): def is_project_manager(self, project): return AuthManager().is_project_manager(self, project) - def generate_key_pair(self, name): - return AuthManager().generate_key_pair(self.id, name) - - def create_key_pair(self, name, public_key, fingerprint): - return AuthManager().create_key_pair(self.id, - name, - public_key, - fingerprint) - - def get_key_pair(self, name): - return AuthManager().get_key_pair(self.id, name) - - def delete_key_pair(self, name): - return AuthManager().delete_key_pair(self.id, name) - - def get_key_pairs(self): - return AuthManager().get_key_pairs(self.id) - def __repr__(self): return "User('%s', '%s', '%s', '%s', %s)" % (self.id, self.name, @@ -628,58 +610,6 @@ class AuthManager(object): with self.driver() as drv: drv.delete_user(uid) - def generate_key_pair(self, user, key_name): - """Generates a key pair for a user - - Generates a public and private key, stores the public key using the - key_name, and returns the private key and fingerprint. - - @type user: User or uid - @param user: User for which to create key pair. - - @type key_name: str - @param key_name: Name to use for the generated KeyPair. - - @rtype: tuple (private_key, fingerprint) - @return: A tuple containing the private_key and fingerprint. - """ - # NOTE(vish): generating key pair is slow so check for legal - # creation before creating keypair - uid = User.safe_id(user) - with self.driver() as drv: - if not drv.get_user(uid): - raise exception.NotFound("User %s doesn't exist" % user) - try: - db.keypair_get(None, uid, key_name) - raise exception.Duplicate("The keypair %s already exists" - % key_name) - except exception.NotFound: - pass - private_key, public_key, fingerprint = crypto.generate_key_pair() - self.create_key_pair(uid, key_name, public_key, fingerprint) - return private_key, fingerprint - - def create_key_pair(self, user, key_name, public_key, fingerprint): - """Creates a key pair for user""" - key = {} - key['user_id'] = User.safe_id(user) - key['name'] = key_name - key['public_key'] = public_key - key['fingerprint'] = fingerprint - return db.keypair_create(None, key) - - def get_key_pair(self, user, key_name): - """Retrieves a key pair for user""" - return db.keypair_get(None, User.safe_id(user), key_name) - - def get_key_pairs(self, user): - """Retrieves all key pairs for user""" - return db.keypair_get_all_by_user(None, User.safe_id(user)) - - def delete_key_pair(self, user, key_name): - """Deletes a key pair for user""" - return db.keypair_destroy(None, User.safe_id(user), key_name) - def get_credentials(self, user, project=None): """Get credential zip for user in project""" if not isinstance(user, User): diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 6ca6855ca..172c65d79 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -29,13 +29,13 @@ import time from twisted.internet import defer +from nova import crypto from nova import db from nova import exception from nova import flags from nova import rpc from nova import utils from nova.auth import rbac -from nova.auth import manager from nova.compute.instance_types import INSTANCE_TYPES from nova.endpoint import images @@ -44,14 +44,30 @@ FLAGS = flags.FLAGS flags.DECLARE('storage_availability_zone', 'nova.volume.manager') -def _gen_key(user_id, key_name): - """ Tuck this into AuthManager """ +def _gen_key(context, user_id, key_name): + """Generate a key + + This is a module level method because it is slow and we need to defer + it into a process pool.""" try: - mgr = manager.AuthManager() - private_key, fingerprint = mgr.generate_key_pair(user_id, key_name) + # NOTE(vish): generating key pair is slow so check for legal + # creation before creating keypair + try: + db.keypair_get(context, user_id, key_name) + raise exception.Duplicate("The keypair %s already exists" + % key_name) + except exception.NotFound: + pass + private_key, public_key, fingerprint = crypto.generate_key_pair() + key = {} + key['user_id'] = user_id + key['name'] = key_name + key['public_key'] = public_key + key['fingerprint'] = fingerprint + db.keypair_create(context, key) + return {'private_key': private_key, 'fingerprint': fingerprint} except Exception as ex: return {'exception': ex} - return {'private_key': private_key, 'fingerprint': fingerprint} class CloudController(object): @@ -177,18 +193,18 @@ class CloudController(object): @rbac.allow('all') def describe_key_pairs(self, context, key_name=None, **kwargs): - key_pairs = context.user.get_key_pairs() + key_pairs = db.keypair_get_all_by_user(context, context.user.id) if not key_name is None: - key_pairs = [x for x in key_pairs if x.name in key_name] + key_pairs = [x for x in key_pairs if x['name'] in key_name] result = [] for key_pair in key_pairs: # filter out the vpn keys suffix = FLAGS.vpn_key_suffix - if context.user.is_admin() or not key_pair.name.endswith(suffix): + if context.user.is_admin() or not key_pair['name'].endswith(suffix): result.append({ - 'keyName': key_pair.name, - 'keyFingerprint': key_pair.fingerprint, + 'keyName': key_pair['name'], + 'keyFingerprint': key_pair['fingerprint'], }) return {'keypairsSet': result} @@ -204,14 +220,18 @@ class CloudController(object): dcall.callback({'keyName': key_name, 'keyFingerprint': kwargs['fingerprint'], 'keyMaterial': kwargs['private_key']}) - pool.apply_async(_gen_key, [context.user.id, key_name], + # TODO(vish): when context is no longer an object, pass it here + pool.apply_async(_gen_key, [None, context.user.id, key_name], callback=_complete) return dcall @rbac.allow('all') def delete_key_pair(self, context, key_name, **kwargs): - context.user.delete_key_pair(key_name) - # aws returns true even if the key doens't exist + try: + db.keypair_destroy(context, context.user.id, key_name) + except exception.NotFound: + # aws returns true even if the key doesn't exist + pass return True @rbac.allow('all') diff --git a/nova/tests/api_unittest.py b/nova/tests/api_unittest.py index 462d1b295..fdb9e21d8 100644 --- a/nova/tests/api_unittest.py +++ b/nova/tests/api_unittest.py @@ -41,8 +41,8 @@ FLAGS = flags.FLAGS # it's pretty damn circuitous so apologies if you have to fix # a bug in it # NOTE(jaypipes) The pylint disables here are for R0913 (too many args) which -# isn't controllable since boto's HTTPRequest needs that many -# args, and for the version-differentiated import of tornado's +# isn't controllable since boto's HTTPRequest needs that many +# args, and for the version-differentiated import of tornado's # httputil. # NOTE(jaypipes): The disable-msg=E1101 and E1103 below is because pylint is # unable to introspect the deferred's return value properly @@ -224,7 +224,8 @@ class ApiEc2TestCase(test.BaseTestCase): for x in range(random.randint(4, 8))) user = self.manager.create_user('fake', 'fake', 'fake') project = self.manager.create_project('fake', 'fake', 'fake') - self.manager.generate_key_pair(user.id, keyname) + # NOTE(vish): create depends on pool, so call helper directly + cloud._gen_key(None, user.id, keyname) rv = self.ec2.get_all_key_pairs() results = [k for k in rv if k.name == keyname] diff --git a/nova/tests/auth_unittest.py b/nova/tests/auth_unittest.py index b54e68274..1b4e12677 100644 --- a/nova/tests/auth_unittest.py +++ b/nova/tests/auth_unittest.py @@ -17,8 +17,6 @@ # under the License. import logging -from M2Crypto import BIO -from M2Crypto import RSA from M2Crypto import X509 import unittest @@ -65,35 +63,6 @@ class AuthTestCase(test.BaseTestCase): 'export S3_URL="http://127.0.0.1:3333/"\n' + 'export EC2_USER_ID="test1"\n') - def test_006_test_key_storage(self): - user = self.manager.get_user('test1') - user.create_key_pair('public', 'key', 'fingerprint') - key = user.get_key_pair('public') - self.assertEqual('key', key.public_key) - self.assertEqual('fingerprint', key.fingerprint) - - def test_007_test_key_generation(self): - user = self.manager.get_user('test1') - private_key, fingerprint = user.generate_key_pair('public2') - key = RSA.load_key_string(private_key, callback=lambda: None) - bio = BIO.MemoryBuffer() - public_key = user.get_key_pair('public2').public_key - key.save_pub_key_bio(bio) - converted = crypto.ssl_pub_to_ssh_pub(bio.read()) - # assert key fields are equal - self.assertEqual(public_key.split(" ")[1].strip(), - converted.split(" ")[1].strip()) - - def test_008_can_list_key_pairs(self): - keys = self.manager.get_user('test1').get_key_pairs() - self.assertTrue(filter(lambda k: k.name == 'public', keys)) - self.assertTrue(filter(lambda k: k.name == 'public2', keys)) - - def test_009_can_delete_key_pair(self): - self.manager.get_user('test1').delete_key_pair('public') - keys = self.manager.get_user('test1').get_key_pairs() - self.assertFalse(filter(lambda k: k.name == 'public', keys)) - def test_010_can_list_users(self): users = self.manager.get_users() logging.warn(users) diff --git a/nova/tests/cloud_unittest.py b/nova/tests/cloud_unittest.py index 29947e03c..4bad25c2b 100644 --- a/nova/tests/cloud_unittest.py +++ b/nova/tests/cloud_unittest.py @@ -17,13 +17,18 @@ # under the License. import logging +from M2Crypto import BIO +from M2Crypto import RSA import StringIO import time + from tornado import ioloop from twisted.internet import defer import unittest from xml.etree import ElementTree +from nova import crypto +from nova import db from nova import flags from nova import rpc from nova import test @@ -54,16 +59,21 @@ class CloudTestCase(test.BaseTestCase): proxy=self.compute) self.injected.append(self.compute_consumer.attach_to_tornado(self.ioloop)) - try: - manager.AuthManager().create_user('admin', 'admin', 'admin') - except: pass - admin = manager.AuthManager().get_user('admin') - project = manager.AuthManager().create_project('proj', 'admin', 'proj') - self.context = api.APIRequestContext(handler=None,project=project,user=admin) + self.manager = manager.AuthManager() + self.user = self.manager.create_user('admin', 'admin', 'admin', True) + self.project = self.manager.create_project('proj', 'admin', 'proj') + self.context = api.APIRequestContext(handler=None, + user=self.user, + project=self.project) def tearDown(self): - manager.AuthManager().delete_project('proj') - manager.AuthManager().delete_user('admin') + self.manager.delete_project(self.project) + self.manager.delete_user(self.user) + super(CloudTestCase, self).setUp() + + def _create_key(self, name): + # NOTE(vish): create depends on pool, so just call helper directly + return cloud._gen_key(self.context, self.context.user.id, name) def test_console_output(self): if FLAGS.connection_type == 'fake': @@ -76,6 +86,33 @@ class CloudTestCase(test.BaseTestCase): self.assert_(output) rv = yield self.compute.terminate_instance(instance_id) + + def test_key_generation(self): + result = self._create_key('test') + private_key = result['private_key'] + key = RSA.load_key_string(private_key, callback=lambda: None) + bio = BIO.MemoryBuffer() + public_key = db.keypair_get(self.context, + self.context.user.id, + 'test')['public_key'] + key.save_pub_key_bio(bio) + converted = crypto.ssl_pub_to_ssh_pub(bio.read()) + # assert key fields are equal + self.assertEqual(public_key.split(" ")[1].strip(), + converted.split(" ")[1].strip()) + + def test_describe_key_pairs(self): + self._create_key('test1') + self._create_key('test2') + result = self.cloud.describe_key_pairs(self.context) + keys = result["keypairsSet"] + self.assertTrue(filter(lambda k: k['keyName'] == 'test1', keys)) + self.assertTrue(filter(lambda k: k['keyName'] == 'test2', keys)) + + def test_delete_key_pair(self): + self._create_key('test') + self.cloud.delete_key_pair(self.context, 'test') + def test_run_instances(self): if FLAGS.connection_type == 'fake': logging.debug("Can't test instances without a real virtual env.") -- cgit From 38070f19036dcf24367429bcc79ffb55fad4b3cd Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Fri, 10 Sep 2010 22:42:51 -0700 Subject: it is called regionEndpoint, and use pipe as a separator --- nova/endpoint/cloud.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 180af0540..02eb50b19 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -177,12 +177,12 @@ class CloudController(object): if FLAGS.region_list: regions = [] for region in FLAGS.region_list: - name, _sep, url = region.partition(',') + name, _sep, url = region.partition('|') regions.append({'regionName': name, - 'regionUrl': url}) + 'regionEndpoint': url}) else: regions = [{'regionName': 'nova', - 'regionUrl': FLAGS.ec2_url}] + 'regionEndpoint': FLAGS.ec2_url}] if region_name: regions = [r for r in regions if r['regionName'] in region_name] return {'regionInfo': regions } -- cgit From 66c583b1883af6e3452271df4b302fd32d1ee25d Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 11 Sep 2010 04:18:30 -0700 Subject: fixed old key reference and made keypair name constistent -> key_pair --- nova/auth/manager.py | 4 ++-- nova/cloudpipe/pipelib.py | 4 ++-- nova/crypto.py | 2 +- nova/db/api.py | 30 +++++++++++++++--------------- nova/db/sqlalchemy/api.py | 28 ++++++++++++++-------------- nova/db/sqlalchemy/models.py | 6 +++--- nova/endpoint/cloud.py | 21 ++++++++++----------- nova/tests/cloud_unittest.py | 2 +- 8 files changed, 48 insertions(+), 49 deletions(-) diff --git a/nova/auth/manager.py b/nova/auth/manager.py index fb87847d5..4e321c1bd 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -604,9 +604,9 @@ class AuthManager(object): def delete_user(self, user): """Deletes a user - Additionally deletes all users keypairs""" + Additionally deletes all users key_pairs""" uid = User.safe_id(user) - db.keypair_destroy_all_by_user(None, uid) + db.key_pair_destroy_all_by_user(None, uid) with self.driver() as drv: drv.delete_user(uid) diff --git a/nova/cloudpipe/pipelib.py b/nova/cloudpipe/pipelib.py index 2867bcb21..de6a97fb6 100644 --- a/nova/cloudpipe/pipelib.py +++ b/nova/cloudpipe/pipelib.py @@ -58,7 +58,7 @@ class CloudPipe(object): z.write(FLAGS.boot_script_template,'autorun.sh') z.close() - key_name = self.setup_keypair(project.project_manager_id, project_id) + key_name = self.setup_key_pair(project.project_manager_id, project_id) zippy = open(zippath, "r") context = api.APIRequestContext(handler=None, user=project.project_manager, project=project) @@ -74,7 +74,7 @@ class CloudPipe(object): security_groups=["vpn-secgroup"]) zippy.close() - def setup_keypair(self, user_id, project_id): + def setup_key_pair(self, user_id, project_id): key_name = '%s%s' % (project_id, FLAGS.vpn_key_suffix) try: private_key, fingerprint = self.manager.generate_key_pair(user_id, key_name) diff --git a/nova/crypto.py b/nova/crypto.py index b05548ea1..1c6fe57ad 100644 --- a/nova/crypto.py +++ b/nova/crypto.py @@ -18,7 +18,7 @@ """ Wrappers around standard crypto, including root and intermediate CAs, -SSH keypairs and x509 certificates. +SSH key_pairs and x509 certificates. """ import base64 diff --git a/nova/db/api.py b/nova/db/api.py index e96d803db..507f70dd5 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -257,29 +257,29 @@ def instance_update(context, instance_id, values): ################### -def keypair_create(context, values): - """Create a keypair from the values dictionary.""" - return IMPL.keypair_create(context, values) +def key_pair_create(context, values): + """Create a key_pair from the values dictionary.""" + return IMPL.key_pair_create(context, values) -def keypair_destroy(context, user_id, name): - """Destroy the keypair or raise if it does not exist.""" - return IMPL.keypair_destroy(context, user_id, name) +def key_pair_destroy(context, user_id, name): + """Destroy the key_pair or raise if it does not exist.""" + return IMPL.key_pair_destroy(context, user_id, name) -def keypair_destroy_all_by_user(context, user_id): - """Destroy all keypairs by user.""" - return IMPL.keypair_destroy_all_by_user(context, user_id) +def key_pair_destroy_all_by_user(context, user_id): + """Destroy all key_pairs by user.""" + return IMPL.key_pair_destroy_all_by_user(context, user_id) -def keypair_get(context, user_id, name): - """Get a keypair or raise if it does not exist.""" - return IMPL.keypair_get(context, user_id, name) +def key_pair_get(context, user_id, name): + """Get a key_pair or raise if it does not exist.""" + return IMPL.key_pair_get(context, user_id, name) -def keypair_get_all_by_user(context, user_id): - """Get all keypairs by user.""" - return IMPL.keypair_get_all_by_user(context, user_id) +def key_pair_get_all_by_user(context, user_id): + """Get all key_pairs by user.""" + return IMPL.key_pair_get_all_by_user(context, user_id) #################### diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 4fd1bf216..ce97f6710 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -355,38 +355,38 @@ def instance_update(_context, instance_id, values): ################### -def keypair_create(_context, values): - keypair_ref = models.Keypair() +def key_pair_create(_context, values): + key_pair_ref = models.KeyPair() for (key, value) in values.iteritems(): - keypair_ref[key] = value - keypair_ref.save() - return keypair_ref + key_pair_ref[key] = value + key_pair_ref.save() + return key_pair_ref -def keypair_destroy(_context, user_id, name): +def key_pair_destroy(_context, user_id, name): session = get_session() with session.begin(): - keypair_ref = models.Keypair.find_by_args(user_id, + key_pair_ref = models.KeyPair.find_by_args(user_id, name, session=session) - keypair_ref.delete(session=session) + key_pair_ref.delete(session=session) -def keypair_destroy_all_by_user(_context, user_id): +def key_pair_destroy_all_by_user(_context, user_id): session = get_session() with session.begin(): # TODO(vish): do we have to use sql here? - session.execute('update keypairs set deleted=1 where user_id=:id', + session.execute('update key_pairs set deleted=1 where user_id=:id', {'id': user_id}) -def keypair_get(_context, user_id, name): - return models.Keypair.find_by_args(user_id, name) +def key_pair_get(_context, user_id, name): + return models.KeyPair.find_by_args(user_id, name) -def keypair_get_all_by_user(_context, user_id): +def key_pair_get_all_by_user(_context, user_id): session = get_session() - return session.query(models.Keypair + return session.query(models.KeyPair ).filter_by(user_id=user_id ).filter_by(deleted=False ).all() diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 81c0a77a8..0ecc48bae 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -284,9 +284,9 @@ class ExportDevice(BASE, NovaBase): uselist=False)) -class Keypair(BASE, NovaBase): - """Represents a keypair""" - __tablename__ = 'keypairs' +class KeyPair(BASE, NovaBase): + """Represents a public key pair for ssh""" + __tablename__ = 'key_pairs' id = Column(Integer, primary_key=True) name = Column(String(255)) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 172c65d79..f30565aca 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -51,10 +51,10 @@ def _gen_key(context, user_id, key_name): it into a process pool.""" try: # NOTE(vish): generating key pair is slow so check for legal - # creation before creating keypair + # creation before creating key_pair try: - db.keypair_get(context, user_id, key_name) - raise exception.Duplicate("The keypair %s already exists" + db.key_pair_get(context, user_id, key_name) + raise exception.Duplicate("The key_pair %s already exists" % key_name) except exception.NotFound: pass @@ -64,7 +64,7 @@ def _gen_key(context, user_id, key_name): key['name'] = key_name key['public_key'] = public_key key['fingerprint'] = fingerprint - db.keypair_create(context, key) + db.key_pair_create(context, key) return {'private_key': private_key, 'fingerprint': fingerprint} except Exception as ex: return {'exception': ex} @@ -193,7 +193,7 @@ class CloudController(object): @rbac.allow('all') def describe_key_pairs(self, context, key_name=None, **kwargs): - key_pairs = db.keypair_get_all_by_user(context, context.user.id) + key_pairs = db.key_pair_get_all_by_user(context, context.user.id) if not key_name is None: key_pairs = [x for x in key_pairs if x['name'] in key_name] @@ -228,7 +228,7 @@ class CloudController(object): @rbac.allow('all') def delete_key_pair(self, context, key_name, **kwargs): try: - db.keypair_destroy(context, context.user.id, key_name) + db.key_pair_destroy(context, context.user.id, key_name) except exception.NotFound: # aws returns true even if the key doesn't exist pass @@ -545,11 +545,10 @@ class CloudController(object): launch_time = time.strftime('%Y-%m-%dT%H:%M:%SZ', time.gmtime()) key_data = None if kwargs.has_key('key_name'): - key_pair = context.user.get_key_pair(kwargs['key_name']) - if not key_pair: - raise exception.ApiError('Key Pair %s not found' % - kwargs['key_name']) - key_data = key_pair.public_key + key_pair_ref = db.key_pair_get(context, + context.user.id, + kwargs['key_name']) + key_data = key_pair_ref['public_key'] # TODO: Get the real security group of launch in here security_group = "default" diff --git a/nova/tests/cloud_unittest.py b/nova/tests/cloud_unittest.py index 4bad25c2b..e56ea6ac2 100644 --- a/nova/tests/cloud_unittest.py +++ b/nova/tests/cloud_unittest.py @@ -92,7 +92,7 @@ class CloudTestCase(test.BaseTestCase): private_key = result['private_key'] key = RSA.load_key_string(private_key, callback=lambda: None) bio = BIO.MemoryBuffer() - public_key = db.keypair_get(self.context, + public_key = db.key_pair_get(self.context, self.context.user.id, 'test')['public_key'] key.save_pub_key_bio(bio) -- cgit From 619e9fd636854b55e7f3334f93ed759ff82759f0 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 11 Sep 2010 04:48:37 -0700 Subject: fixed typo network => network_manager in cloud.py --- nova/endpoint/cloud.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 622b4e2a4..0eedd9fec 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -613,7 +613,7 @@ class CloudController(object): # NOTE(vish): Currently, nothing needs to be done on the # network node until release. If this changes, # we will need to cast here. - self.network.deallocate_fixed_ip(context, address) + self.network_manager.deallocate_fixed_ip(context, address) host = instance_ref['host'] if host: -- cgit From 5b9908ff2601adfac3565ff900ef254df27102b9 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 11 Sep 2010 06:29:13 -0700 Subject: fixed reversed admin logic on describe instances --- nova/endpoint/cloud.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 0eedd9fec..a25598dc8 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -366,7 +366,7 @@ class CloudController(object): instances = db.instance_get_by_reservation(context, reservation_id) else: - if not context.user.is_admin(): + if context.user.is_admin(): instances = db.instance_get_all(context) else: instances = db.instance_get_by_project(context, -- cgit From 06a799d2668723bbaead7ca2afbfb4b0cbf28abb Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 11 Sep 2010 18:16:10 -0700 Subject: use a string version of key name when constructing mpi dict because None doesn't work well in lookup --- nova/endpoint/cloud.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 622b4e2a4..45291ca34 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -88,10 +88,11 @@ class CloudController(object): if instance['fixed_ip']: line = '%s slots=%d' % (instance['fixed_ip']['str_id'], INSTANCE_TYPES[instance['instance_type']]['vcpus']) - if instance['key_name'] in result: - result[instance['key_name']].append(line) + key = str(instance['key_name']) + if key in result: + result[key].append(line) else: - result[instance['key_name']] = [line] + result[key] = [line] return result def get_metadata(self, address): -- cgit From dff482e992b25580728955ae83ea2e38a18e7736 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 11 Sep 2010 20:54:12 -0700 Subject: manage command for project quotas --- bin/nova-manage | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/bin/nova-manage b/bin/nova-manage index 325245ac4..c7ef33a48 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -50,7 +50,6 @@ """ CLI interface for nova management. - Connects to the running ADMIN api in the api daemon. """ import os @@ -68,7 +67,9 @@ if os.path.exists(os.path.join(possible_topdir, 'nova', '__init__.py')): sys.path.insert(0, possible_topdir) from nova import db +from nova import exception from nova import flags +from nova import quota from nova import utils from nova.auth import manager from nova.cloudpipe import pipelib @@ -186,6 +187,13 @@ class RoleCommands(object): class UserCommands(object): """Class for managing users.""" + @staticmethod + def _print_export(user): + """Print export variables to use with API.""" + print 'export EC2_ACCESS_KEY=%s' % user.access + print 'export EC2_SECRET_KEY=%s' % user.secret + + def __init__(self): self.manager = manager.AuthManager() @@ -193,13 +201,13 @@ class UserCommands(object): """creates a new admin and prints exports arguments: name [access] [secret]""" user = self.manager.create_user(name, access, secret, True) - print_export(user) + self._print_export(user) def create(self, name, access=None, secret=None): """creates a new user and prints exports arguments: name [access] [secret]""" user = self.manager.create_user(name, access, secret, False) - print_export(user) + self._print_export(user) def delete(self, name): """deletes an existing user @@ -211,7 +219,7 @@ class UserCommands(object): arguments: name""" user = self.manager.get_user(name) if user: - print_export(user) + self._print_export(user) else: print "User %s doesn't exist" % name @@ -222,12 +230,6 @@ class UserCommands(object): print user.name -def print_export(user): - """Print export variables to use with API.""" - print 'export EC2_ACCESS_KEY=%s' % user.access - print 'export EC2_SECRET_KEY=%s' % user.secret - - class ProjectCommands(object): """Class for managing projects.""" @@ -262,6 +264,19 @@ class ProjectCommands(object): for project in self.manager.get_projects(): print project.name + def quota(self, project_id, key=None, value=None): + """Set or display quotas for project + arguments: project_id [key] [value]""" + if key: + quo = {'project_id': project_id, key: value} + try: + db.quota_update(None, project_id, quo) + except exception.NotFound: + db.quota_create(None, quo) + project_quota = quota._get_quota(None, project_id) + for key, value in project_quota.iteritems(): + print '%s: %s' % (key, value) + def remove(self, project, user): """Removes user from project arguments: project user""" @@ -274,6 +289,7 @@ class ProjectCommands(object): with open(filename, 'w') as f: f.write(zip_file) + class FloatingIpCommands(object): """Class for managing floating ip.""" @@ -306,6 +322,7 @@ class FloatingIpCommands(object): floating_ip['address'], instance) + CATEGORIES = [ ('user', UserCommands), ('project', ProjectCommands), -- cgit From 6cbf8b736cc2c9929c2ad69ddc8e8b4fc2d0f4ae Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sun, 12 Sep 2010 23:09:15 -0700 Subject: removed second copy of ProcessExecutionError --- nova/utils.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/nova/utils.py b/nova/utils.py index 8939043e6..d18dd9843 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -39,17 +39,6 @@ from nova.exception import ProcessExecutionError FLAGS = flags.FLAGS TIME_FORMAT = "%Y-%m-%dT%H:%M:%SZ" -class ProcessExecutionError(IOError): - def __init__( self, stdout=None, stderr=None, exit_code=None, cmd=None, - description=None): - if description is None: - description = "Unexpected error while running command." - if exit_code is None: - exit_code = '-' - message = "%s\nCommand: %s\nExit code: %s\nStdout: %r\nStderr: %r" % ( - description, cmd, exit_code, stdout, stderr) - IOError.__init__(self, message) - def import_class(import_str): """Returns a class from a string including module and class""" mod_str, _sep, class_str = import_str.rpartition('.') -- cgit From 86cd30b749e6da78d4ceb6c77f2116975429a81a Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Mon, 13 Sep 2010 01:15:35 -0700 Subject: renamed _get_quota to get_quota and moved int(size) into quota.py --- bin/nova-manage | 2 +- nova/endpoint/cloud.py | 1 - nova/quota.py | 9 +++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bin/nova-manage b/bin/nova-manage index c7ef33a48..824e00ac5 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -273,7 +273,7 @@ class ProjectCommands(object): db.quota_update(None, project_id, quo) except exception.NotFound: db.quota_create(None, quo) - project_quota = quota._get_quota(None, project_id) + project_quota = quota.get_quota(None, project_id) for key, value in project_quota.iteritems(): print '%s: %s' % (key, value) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 749bf5f9c..1618b784b 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -284,7 +284,6 @@ class CloudController(object): @rbac.allow('projectmanager', 'sysadmin') def create_volume(self, context, size, **kwargs): # check quota - size = int(size) if quota.allowed_volumes(context, 1, size) < 1: logging.warn("Quota exceeeded for %s, tried to create %sG volume", context.project.id, size) diff --git a/nova/quota.py b/nova/quota.py index f0e51feeb..edbb83111 100644 --- a/nova/quota.py +++ b/nova/quota.py @@ -37,7 +37,7 @@ flags.DEFINE_integer('quota_gigabytes', 1000, flags.DEFINE_integer('quota_floating_ips', 10, 'number of floating ips allowed per project') -def _get_quota(context, project_id): +def get_quota(context, project_id): rval = {'instances': FLAGS.quota_instances, 'cores': FLAGS.quota_cores, 'volumes': FLAGS.quota_volumes, @@ -57,7 +57,7 @@ def allowed_instances(context, num_instances, instance_type): project_id = context.project.id used_instances, used_cores = db.instance_data_get_for_project(context, project_id) - quota = _get_quota(context, project_id) + quota = get_quota(context, project_id) allowed_instances = quota['instances'] - used_instances allowed_cores = quota['cores'] - used_cores type_cores = instance_types.INSTANCE_TYPES[instance_type]['vcpus'] @@ -72,9 +72,10 @@ def allowed_volumes(context, num_volumes, size): project_id = context.project.id used_volumes, used_gigabytes = db.volume_data_get_for_project(context, project_id) - quota = _get_quota(context, project_id) + quota = get_quota(context, project_id) allowed_volumes = quota['volumes'] - used_volumes allowed_gigabytes = quota['gigabytes'] - used_gigabytes + size = int(size) num_gigabytes = num_volumes * size allowed_volumes = min(allowed_volumes, int(allowed_gigabytes // size)) @@ -85,7 +86,7 @@ def allowed_floating_ips(context, num_floating_ips): """Check quota and return min(num_floating_ips, allowed_floating_ips)""" project_id = context.project.id used_floating_ips = db.floating_ip_count_by_project(context, project_id) - quota = _get_quota(context, project_id) + quota = get_quota(context, project_id) allowed_floating_ips = quota['floating_ips'] - used_floating_ips return min(num_floating_ips, allowed_floating_ips) -- cgit From 2774466197a0dda3763569fe7aa1a578baf5e059 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Mon, 13 Sep 2010 02:15:02 -0700 Subject: added missing yield in detach_volume --- nova/compute/manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 954227b42..24538e4f1 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -189,7 +189,7 @@ class ComputeManager(manager.Manager): volume_id) instance_ref = self.db.instance_get(context, instance_id) volume_ref = self.db.volume_get(context, volume_id) - self.driver.detach_volume(instance_ref['str_id'], - volume_ref['mountpoint']) + yield self.driver.detach_volume(instance_ref['str_id'], + volume_ref['mountpoint']) self.db.volume_detached(context, volume_id) defer.returnValue(True) -- cgit From 3d68f1f74cd7fe6ddb9eec003a9e31f8ad036b27 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Tue, 14 Sep 2010 16:26:19 -0400 Subject: Add ratelimiting package into Nova. After Austin it'll be pulled out into PyPI. --- nova/api/rackspace/ratelimiting/__init__.py | 103 ++++++++++++++++++++++++++++ nova/api/rackspace/ratelimiting/tests.py | 60 ++++++++++++++++ 2 files changed, 163 insertions(+) create mode 100644 nova/api/rackspace/ratelimiting/__init__.py create mode 100644 nova/api/rackspace/ratelimiting/tests.py diff --git a/nova/api/rackspace/ratelimiting/__init__.py b/nova/api/rackspace/ratelimiting/__init__.py new file mode 100644 index 000000000..176e7d66e --- /dev/null +++ b/nova/api/rackspace/ratelimiting/__init__.py @@ -0,0 +1,103 @@ +"""Rate limiting of arbitrary actions.""" + +import time +import urllib +import webob.dec +import webob.exc + + +# Convenience constants for the limits dictionary passed to Limiter(). +PER_SECOND = 1 +PER_MINUTE = 60 +PER_HOUR = 60 * 60 +PER_DAY = 60 * 60 * 24 + +class Limiter(object): + + """Class providing rate limiting of arbitrary actions.""" + + def __init__(self, limits): + """Create a rate limiter. + + limits: a dict mapping from action name to a tuple. The tuple contains + the number of times the action may be performed, and the time period + (in seconds) during which the number must not be exceeded for this + action. Example: dict(reboot=(10, ratelimiting.PER_MINUTE)) would + allow 10 'reboot' actions per minute. + """ + self.limits = limits + self._levels = {} + + def perform(self, action_name, username='nobody'): + """Attempt to perform an action by the given username. + + action_name: the string name of the action to perform. This must + be a key in the limits dict passed to the ctor. + + username: an optional string name of the user performing the action. + Each user has her own set of rate limiting counters. Defaults to + 'nobody' (so that if you never specify a username when calling + perform(), a single set of counters will be used.) + + Return None if the action may proceed. If the action may not proceed + because it has been rate limited, return the float number of seconds + until the action would succeed. + """ + # Think of rate limiting as a bucket leaking water at 1cc/second. The + # bucket can hold as many ccs as there are seconds in the rate + # limiting period (e.g. 3600 for per-hour ratelimits), and if you can + # perform N actions in that time, each action fills the bucket by + # 1/Nth of its volume. You may only perform an action if the bucket + # would not overflow. + now = time.time() + key = '%s:%s' % (username, action_name) + last_time_performed, water_level = self._levels.get(key, (now, 0)) + # The bucket leaks 1cc/second. + water_level -= (now - last_time_performed) + if water_level < 0: + water_level = 0 + num_allowed_per_period, period_in_secs = self.limits[action_name] + # Fill the bucket by 1/Nth its capacity, and hope it doesn't overflow. + capacity = period_in_secs + new_level = water_level + (capacity * 1.0 / num_allowed_per_period) + if new_level > capacity: + # Delay this many seconds. + return new_level - capacity + self._levels[key] = (now, new_level) + return None + + +# If one instance of this WSGIApps is unable to handle your load, put a +# sharding app in front that shards by username to one of many backends. + +class WSGIApp(object): + + """Application that tracks rate limits in memory. Send requests to it of + this form: + + POST /limiter// + + and receive a 200 OK, or a 403 Forbidden with an X-Wait-Seconds header + containing the number of seconds to wait before the action would succeed. + """ + + def __init__(self, limiter): + """Create the WSGI application using the given Limiter instance.""" + self.limiter = limiter + + @webob.dec.wsgify + def __call__(req): + parts = req.path_info.split('/') + # format: /limiter// + if req.method != 'POST': + raise webob.exc.HTTPMethodNotAllowed() + if len(parts) != 4 or parts[1] != 'limiter': + raise webob.exc.HTTPNotFound() + username = parts[2] + action_name = urllib.unquote(parts[3]) + delay = self.limiter.perform(action_name, username) + if delay: + return webob.exc.HTTPForbidden( + headers={'X-Wait-Seconds': delay}) + else: + return '' # 200 OK diff --git a/nova/api/rackspace/ratelimiting/tests.py b/nova/api/rackspace/ratelimiting/tests.py new file mode 100644 index 000000000..1983cdea8 --- /dev/null +++ b/nova/api/rackspace/ratelimiting/tests.py @@ -0,0 +1,60 @@ +import ratelimiting +import time +import unittest + +class Test(unittest.TestCase): + + def setUp(self): + self.limits = { + 'a': (5, ratelimiting.PER_SECOND), + 'b': (5, ratelimiting.PER_MINUTE), + 'c': (5, ratelimiting.PER_HOUR), + 'd': (1, ratelimiting.PER_SECOND), + 'e': (100, ratelimiting.PER_SECOND)} + self.rl = ratelimiting.Limiter(self.limits) + + def exhaust(self, action, times_until_exhausted, **kwargs): + for i in range(times_until_exhausted): + when = self.rl.perform(action, **kwargs) + self.assertEqual(when, None) + num, period = self.limits[action] + delay = period * 1.0 / num + # Verify that we are now thoroughly delayed + for i in range(10): + when = self.rl.perform(action, **kwargs) + self.assertAlmostEqual(when, delay, 2) + + def test_second(self): + self.exhaust('a', 5) + time.sleep(0.2) + self.exhaust('a', 1) + time.sleep(1) + self.exhaust('a', 5) + + def test_minute(self): + self.exhaust('b', 5) + + def test_one_per_period(self): + def allow_once_and_deny_once(): + when = self.rl.perform('d') + self.assertEqual(when, None) + when = self.rl.perform('d') + self.assertAlmostEqual(when, 1, 2) + return when + time.sleep(allow_once_and_deny_once()) + time.sleep(allow_once_and_deny_once()) + allow_once_and_deny_once() + + def test_we_can_go_indefinitely_if_we_spread_out_requests(self): + for i in range(200): + when = self.rl.perform('e') + self.assertEqual(when, None) + time.sleep(0.01) + + def test_users_get_separate_buckets(self): + self.exhaust('c', 5, username='alice') + self.exhaust('c', 5, username='bob') + self.exhaust('c', 5, username='chuck') + self.exhaust('c', 0, username='chuck') + self.exhaust('c', 0, username='bob') + self.exhaust('c', 0, username='alice') -- cgit From 8138a35d3672e08640762b7533c1c527568d0b4f Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Tue, 14 Sep 2010 18:59:02 -0400 Subject: RateLimitingMiddleware --- nova/api/rackspace/__init__.py | 52 ++++++++++++++++++++++++++++++++- nova/tests/api/rackspace/__init__.py | 56 ++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 1 deletion(-) diff --git a/nova/api/rackspace/__init__.py b/nova/api/rackspace/__init__.py index b4d666d63..e35109b43 100644 --- a/nova/api/rackspace/__init__.py +++ b/nova/api/rackspace/__init__.py @@ -31,6 +31,7 @@ from nova import flags from nova import wsgi from nova.api.rackspace import flavors from nova.api.rackspace import images +from nova.api.rackspace import ratelimiting from nova.api.rackspace import servers from nova.api.rackspace import sharedipgroups from nova.auth import manager @@ -40,7 +41,7 @@ class API(wsgi.Middleware): """WSGI entry point for all Rackspace API requests.""" def __init__(self): - app = AuthMiddleware(APIRouter()) + app = AuthMiddleware(RateLimitingMiddleware(APIRouter())) super(API, self).__init__(app) @@ -65,6 +66,55 @@ class AuthMiddleware(wsgi.Middleware): return self.application +class RateLimitingMiddleware(wsgi.Middleware): + """Rate limit incoming requests according to the OpenStack rate limits.""" + + def __init__(self, application): + super(RateLimitingMiddleware, self).__init__(application) + #TODO(gundlach): These limits were based on limitations of Cloud + #Servers. We should revisit them in Nova. + self.limiter = ratelimiting.Limiter(limits={ + 'DELETE': (100, ratelimiting.PER_MINUTE), + 'PUT': (10, ratelimiting.PER_MINUTE), + 'POST': (10, ratelimiting.PER_MINUTE), + 'POST servers': (50, ratelimiting.PER_DAY), + 'GET changes-since': (3, ratelimiting.PER_MINUTE), + }) + + @webob.dec.wsgify + def __call__(self, req): + """Rate limit the request. + + If the request should be rate limited, return a 413 status with a + Retry-After header giving the time when the request would succeed. + """ + username = req.headers['X-Auth-User'] + action_name = self.get_action_name(req) + if not action_name: # not rate limited + return self.application + delay = self.limiter.perform(action_name, username=username) + if action_name == 'POST servers': + # "POST servers" is a POST, so it counts against "POST" too. + delay2 = self.limiter.perform('POST', username=username) + delay = max(delay or 0, delay2 or 0) + if delay: + # TODO(gundlach): Get the retry-after format correct. + raise webob.exc.HTTPRequestEntityTooLarge(headers={ + 'Retry-After': time.time() + delay}) + else: + return self.application + + def get_action_name(self, req): + """Return the action name for this request.""" + if req.method == 'GET' and 'changes-since' in req.GET: + return 'GET changes-since' + if req.method == 'POST' and req.path_info.starts_with('/servers'): + return 'POST servers' + if req.method in ['PUT', 'POST', 'DELETE']: + return req.method + return None + + class APIRouter(wsgi.Router): """ Routes requests on the Rackspace API to the appropriate controller diff --git a/nova/tests/api/rackspace/__init__.py b/nova/tests/api/rackspace/__init__.py index e69de29bb..f7537a4e7 100644 --- a/nova/tests/api/rackspace/__init__.py +++ b/nova/tests/api/rackspace/__init__.py @@ -0,0 +1,56 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2010 OpenStack LLC. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import unittest + +from nova.api.rackspace.ratelimiting import RateLimitingMiddleware +from nova.tests.api.test_helper import * +from webob import Request + +class RateLimitingMiddlewareTest(unittest.TestCase): + def setUp(self): + self.middleware = RateLimitingMiddleware(APIStub()) + self.stubs = stubout.StubOutForTesting() + + def tearDown(self): + self.stubs.UnsetAll() + + def test_get_action_name(self): + middleware = RateLimitingMiddleware(APIStub()) + def verify(method, url, action_name): + req = Request(url) + req.method = method + action = middleware.get_action_name(req) + self.assertEqual(action, action_name) + verify('PUT', '/servers/4', 'PUT') + verify('DELETE', '/servers/4', 'DELETE') + verify('POST', '/images/4', 'POST') + verify('POST', '/servers/4', 'POST servers') + verify('GET', '/foo?a=4&changes-since=never&b=5', 'GET changes-since') + verify('GET', '/foo?a=4&monkeys-since=never&b=5', None) + verify('GET', '/servers/4', None) + verify('HEAD', '/servers/4', None) + + def TODO_test_call(self): + pass + #mw = make_middleware() + #req = build_request('DELETE', '/servers/4') + #for i in range(5): + # resp = req.get_response(mw) + # assert resp is OK + #resp = req.get_response(mw) + #assert resp is rate limited -- cgit From 63ad073efd0b20f59f02bc37182c0180cac3f405 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Wed, 15 Sep 2010 09:25:53 -0400 Subject: RateLimitingMiddleware tests --- nova/api/rackspace/__init__.py | 24 ++++++++++----- nova/api/rackspace/ratelimiting/tests.py | 3 ++ nova/tests/api/rackspace/__init__.py | 51 +++++++++++++++++++++----------- 3 files changed, 52 insertions(+), 26 deletions(-) diff --git a/nova/api/rackspace/__init__.py b/nova/api/rackspace/__init__.py index e35109b43..66d80a5b7 100644 --- a/nova/api/rackspace/__init__.py +++ b/nova/api/rackspace/__init__.py @@ -92,23 +92,31 @@ class RateLimitingMiddleware(wsgi.Middleware): action_name = self.get_action_name(req) if not action_name: # not rate limited return self.application - delay = self.limiter.perform(action_name, username=username) - if action_name == 'POST servers': - # "POST servers" is a POST, so it counts against "POST" too. - delay2 = self.limiter.perform('POST', username=username) - delay = max(delay or 0, delay2 or 0) + delay = self.get_delay(action_name, username) if delay: # TODO(gundlach): Get the retry-after format correct. raise webob.exc.HTTPRequestEntityTooLarge(headers={ 'Retry-After': time.time() + delay}) - else: - return self.application + return self.application + + def get_delay(self, action_name, username): + """Return the delay for the given action and username, or None if + the action would not be rate limited. + """ + if action_name == 'POST servers': + # "POST servers" is a POST, so it counts against "POST" too. + # Attempt the "POST" first, lest we are rate limited by "POST" but + # use up a precious "POST servers" call. + delay = self.limiter.perform("POST", username=username) + if delay: + return delay + return self.limiter.perform(action_name, username=username) def get_action_name(self, req): """Return the action name for this request.""" if req.method == 'GET' and 'changes-since' in req.GET: return 'GET changes-since' - if req.method == 'POST' and req.path_info.starts_with('/servers'): + if req.method == 'POST' and req.path_info.startswith('/servers'): return 'POST servers' if req.method in ['PUT', 'POST', 'DELETE']: return req.method diff --git a/nova/api/rackspace/ratelimiting/tests.py b/nova/api/rackspace/ratelimiting/tests.py index 1983cdea8..545e1d1b6 100644 --- a/nova/api/rackspace/ratelimiting/tests.py +++ b/nova/api/rackspace/ratelimiting/tests.py @@ -58,3 +58,6 @@ class Test(unittest.TestCase): self.exhaust('c', 0, username='chuck') self.exhaust('c', 0, username='bob') self.exhaust('c', 0, username='alice') + +if __name__ == '__main__': + unittest.main() diff --git a/nova/tests/api/rackspace/__init__.py b/nova/tests/api/rackspace/__init__.py index f7537a4e7..2fab1a4da 100644 --- a/nova/tests/api/rackspace/__init__.py +++ b/nova/tests/api/rackspace/__init__.py @@ -17,22 +17,15 @@ import unittest -from nova.api.rackspace.ratelimiting import RateLimitingMiddleware +from nova.api.rackspace import RateLimitingMiddleware from nova.tests.api.test_helper import * from webob import Request class RateLimitingMiddlewareTest(unittest.TestCase): - def setUp(self): - self.middleware = RateLimitingMiddleware(APIStub()) - self.stubs = stubout.StubOutForTesting() - - def tearDown(self): - self.stubs.UnsetAll() - def test_get_action_name(self): middleware = RateLimitingMiddleware(APIStub()) def verify(method, url, action_name): - req = Request(url) + req = Request.blank(url) req.method = method action = middleware.get_action_name(req) self.assertEqual(action, action_name) @@ -45,12 +38,34 @@ class RateLimitingMiddlewareTest(unittest.TestCase): verify('GET', '/servers/4', None) verify('HEAD', '/servers/4', None) - def TODO_test_call(self): - pass - #mw = make_middleware() - #req = build_request('DELETE', '/servers/4') - #for i in range(5): - # resp = req.get_response(mw) - # assert resp is OK - #resp = req.get_response(mw) - #assert resp is rate limited + def exhaust(self, middleware, method, url, username, times): + req = Request.blank(url, dict(REQUEST_METHOD=method), + headers={'X-Auth-User': username}) + for i in range(times): + resp = req.get_response(middleware) + self.assertEqual(resp.status_int, 200) + resp = req.get_response(middleware) + self.assertEqual(resp.status_int, 413) + self.assertTrue('Retry-After' in resp.headers) + + def test_single_action(self): + middleware = RateLimitingMiddleware(APIStub()) + self.exhaust(middleware, 'DELETE', '/servers/4', 'usr1', 100) + self.exhaust(middleware, 'DELETE', '/servers/4', 'usr2', 100) + + def test_POST_servers_action_implies_POST_action(self): + middleware = RateLimitingMiddleware(APIStub()) + self.exhaust(middleware, 'POST', '/servers/4', 'usr1', 10) + self.exhaust(middleware, 'POST', '/images/4', 'usr2', 10) + self.assertTrue(set(middleware.limiter._levels) == + set(['usr1:POST', 'usr1:POST servers', 'usr2:POST'])) + + def test_POST_servers_action_correctly_ratelimited(self): + middleware = RateLimitingMiddleware(APIStub()) + # Use up all of our "POST" allowance for the minute, 5 times + for i in range(5): + self.exhaust(middleware, 'POST', '/servers/4', 'usr1', 10) + # Reset the 'POST' action counter. + del middleware.limiter._levels['usr1:POST'] + # All 50 daily "POST servers" actions should be all used up + self.exhaust(middleware, 'POST', '/servers/4', 'usr1', 0) -- cgit From fd4d5787d5b6f6e550d33c13eb76f4562a87a118 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Wed, 15 Sep 2010 11:23:08 -0400 Subject: Test the WSGIApp --- nova/api/rackspace/ratelimiting/__init__.py | 2 +- nova/api/rackspace/ratelimiting/tests.py | 69 ++++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/nova/api/rackspace/ratelimiting/__init__.py b/nova/api/rackspace/ratelimiting/__init__.py index 176e7d66e..64d5fff2c 100644 --- a/nova/api/rackspace/ratelimiting/__init__.py +++ b/nova/api/rackspace/ratelimiting/__init__.py @@ -86,7 +86,7 @@ class WSGIApp(object): self.limiter = limiter @webob.dec.wsgify - def __call__(req): + def __call__(self, req): parts = req.path_info.split('/') # format: /limiter// if req.method != 'POST': diff --git a/nova/api/rackspace/ratelimiting/tests.py b/nova/api/rackspace/ratelimiting/tests.py index 545e1d1b6..f924e7805 100644 --- a/nova/api/rackspace/ratelimiting/tests.py +++ b/nova/api/rackspace/ratelimiting/tests.py @@ -1,8 +1,10 @@ -import ratelimiting import time import unittest +import webob -class Test(unittest.TestCase): +import nova.api.rackspace.ratelimiting as ratelimiting + +class LimiterTest(unittest.TestCase): def setUp(self): self.limits = { @@ -59,5 +61,68 @@ class Test(unittest.TestCase): self.exhaust('c', 0, username='bob') self.exhaust('c', 0, username='alice') + +class WSGIAppTest(unittest.TestCase): + + def setUp(self): + test = self + class FakeLimiter(object): + def __init__(self): + self._action = self._username = self._delay = None + def mock(self, action, username, delay): + self._action = action + self._username = username + self._delay = delay + def perform(self, action, username): + test.assertEqual(action, self._action) + test.assertEqual(username, self._username) + return self._delay + self.limiter = FakeLimiter() + self.app = ratelimiting.WSGIApp(self.limiter) + + def test_invalid_methods(self): + requests = [] + for method in ['GET', 'PUT', 'DELETE']: + req = webob.Request.blank('/limits/michael/breakdance', + dict(REQUEST_METHOD=method)) + requests.append(req) + for req in requests: + self.assertEqual(req.get_response(self.app).status_int, 405) + + def test_invalid_urls(self): + requests = [] + for prefix in ['limit', '', 'limiter2', 'limiter/limits', 'limiter/1']: + req = webob.Request.blank('/%s/michael/breakdance' % prefix, + dict(REQUEST_METHOD='POST')) + requests.append(req) + for req in requests: + self.assertEqual(req.get_response(self.app).status_int, 404) + + def verify(self, url, username, action, delay=None): + """Make sure that POSTing to the given url causes the given username + to perform the given action. Make the internal rate limiter return + delay and make sure that the WSGI app returns the correct response. + """ + req = webob.Request.blank(url, dict(REQUEST_METHOD='POST')) + self.limiter.mock(action, username, delay) + resp = req.get_response(self.app) + if not delay: + self.assertEqual(resp.status_int, 200) + else: + self.assertEqual(resp.status_int, 403) + self.assertEqual(resp.headers['X-Wait-Seconds'], delay) + + def test_good_urls(self): + self.verify('/limiter/michael/hoot', 'michael', 'hoot') + + def test_escaping(self): + self.verify('/limiter/michael/jump%20up', 'michael', 'jump up') + + def test_response_to_delays(self): + self.verify('/limiter/michael/hoot', 'michael', 'hoot', 1) + self.verify('/limiter/michael/hoot', 'michael', 'hoot', 1.56) + self.verify('/limiter/michael/hoot', 'michael', 'hoot', 1000) + + if __name__ == '__main__': unittest.main() -- cgit From f200587ce068482ab94e777154de3ac777269fa0 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Wed, 15 Sep 2010 13:54:38 -0400 Subject: Add support for middleware proxying to a ratelimiting.WSGIApp, for deployments that use more than one API Server and thus can't store ratelimiting counters in memory. --- nova/api/rackspace/__init__.py | 29 ++++-- nova/api/rackspace/ratelimiting/__init__.py | 21 ++++- nova/api/rackspace/ratelimiting/tests.py | 140 +++++++++++++++++++++++++--- nova/tests/api/rackspace/__init__.py | 8 ++ 4 files changed, 173 insertions(+), 25 deletions(-) diff --git a/nova/api/rackspace/__init__.py b/nova/api/rackspace/__init__.py index 66d80a5b7..ac5365310 100644 --- a/nova/api/rackspace/__init__.py +++ b/nova/api/rackspace/__init__.py @@ -69,17 +69,26 @@ class AuthMiddleware(wsgi.Middleware): class RateLimitingMiddleware(wsgi.Middleware): """Rate limit incoming requests according to the OpenStack rate limits.""" - def __init__(self, application): + def __init__(self, application, service_host=None): + """Create a rate limiting middleware that wraps the given application. + + By default, rate counters are stored in memory. If service_host is + specified, the middleware instead relies on the ratelimiting.WSGIApp + at the given host+port to keep rate counters. + """ super(RateLimitingMiddleware, self).__init__(application) - #TODO(gundlach): These limits were based on limitations of Cloud - #Servers. We should revisit them in Nova. - self.limiter = ratelimiting.Limiter(limits={ - 'DELETE': (100, ratelimiting.PER_MINUTE), - 'PUT': (10, ratelimiting.PER_MINUTE), - 'POST': (10, ratelimiting.PER_MINUTE), - 'POST servers': (50, ratelimiting.PER_DAY), - 'GET changes-since': (3, ratelimiting.PER_MINUTE), - }) + if not service_host: + #TODO(gundlach): These limits were based on limitations of Cloud + #Servers. We should revisit them in Nova. + self.limiter = ratelimiting.Limiter(limits={ + 'DELETE': (100, ratelimiting.PER_MINUTE), + 'PUT': (10, ratelimiting.PER_MINUTE), + 'POST': (10, ratelimiting.PER_MINUTE), + 'POST servers': (50, ratelimiting.PER_DAY), + 'GET changes-since': (3, ratelimiting.PER_MINUTE), + }) + else: + self.limiter = ratelimiting.WSGIAppProxy(service_host) @webob.dec.wsgify def __call__(self, req): diff --git a/nova/api/rackspace/ratelimiting/__init__.py b/nova/api/rackspace/ratelimiting/__init__.py index 64d5fff2c..f843bac0f 100644 --- a/nova/api/rackspace/ratelimiting/__init__.py +++ b/nova/api/rackspace/ratelimiting/__init__.py @@ -1,5 +1,6 @@ """Rate limiting of arbitrary actions.""" +import httplib import time import urllib import webob.dec @@ -98,6 +99,24 @@ class WSGIApp(object): delay = self.limiter.perform(action_name, username) if delay: return webob.exc.HTTPForbidden( - headers={'X-Wait-Seconds': delay}) + headers={'X-Wait-Seconds': "%.2f" % delay}) else: return '' # 200 OK + + +class WSGIAppProxy(object): + + """Limiter lookalike that proxies to a ratelimiting.WSGIApp.""" + + def __init__(self, service_host): + """Creates a proxy pointing to a ratelimiting.WSGIApp at the given + host.""" + self.service_host = service_host + + def perform(self, action, username='nobody'): + conn = httplib.HTTPConnection(self.service_host) + conn.request('POST', '/limiter/%s/%s' % (username, action)) + resp = conn.getresponse() + if resp.status == 200: + return None # no delay + return float(resp.getheader('X-Wait-Seconds')) diff --git a/nova/api/rackspace/ratelimiting/tests.py b/nova/api/rackspace/ratelimiting/tests.py index f924e7805..13a47989b 100644 --- a/nova/api/rackspace/ratelimiting/tests.py +++ b/nova/api/rackspace/ratelimiting/tests.py @@ -1,3 +1,5 @@ +import httplib +import StringIO import time import unittest import webob @@ -62,22 +64,25 @@ class LimiterTest(unittest.TestCase): self.exhaust('c', 0, username='alice') +class FakeLimiter(object): + """Fake Limiter class that you can tell how to behave.""" + def __init__(self, test): + self._action = self._username = self._delay = None + self.test = test + def mock(self, action, username, delay): + self._action = action + self._username = username + self._delay = delay + def perform(self, action, username): + self.test.assertEqual(action, self._action) + self.test.assertEqual(username, self._username) + return self._delay + + class WSGIAppTest(unittest.TestCase): def setUp(self): - test = self - class FakeLimiter(object): - def __init__(self): - self._action = self._username = self._delay = None - def mock(self, action, username, delay): - self._action = action - self._username = username - self._delay = delay - def perform(self, action, username): - test.assertEqual(action, self._action) - test.assertEqual(username, self._username) - return self._delay - self.limiter = FakeLimiter() + self.limiter = FakeLimiter(self) self.app = ratelimiting.WSGIApp(self.limiter) def test_invalid_methods(self): @@ -110,7 +115,7 @@ class WSGIAppTest(unittest.TestCase): self.assertEqual(resp.status_int, 200) else: self.assertEqual(resp.status_int, 403) - self.assertEqual(resp.headers['X-Wait-Seconds'], delay) + self.assertEqual(resp.headers['X-Wait-Seconds'], "%.2f" % delay) def test_good_urls(self): self.verify('/limiter/michael/hoot', 'michael', 'hoot') @@ -124,5 +129,112 @@ class WSGIAppTest(unittest.TestCase): self.verify('/limiter/michael/hoot', 'michael', 'hoot', 1000) +class FakeHttplibSocket(object): + """a fake socket implementation for httplib.HTTPResponse, trivial""" + + def __init__(self, response_string): + self._buffer = StringIO.StringIO(response_string) + + def makefile(self, _mode, _other): + """Returns the socket's internal buffer""" + return self._buffer + + +class FakeHttplibConnection(object): + """A fake httplib.HTTPConnection + + Requests made via this connection actually get translated and routed into + our WSGI app, we then wait for the response and turn it back into + an httplib.HTTPResponse. + """ + def __init__(self, app, host, is_secure=False): + self.app = app + self.host = host + + def request(self, method, path, data='', headers={}): + req = webob.Request.blank(path) + req.method = method + req.body = data + req.headers = headers + req.host = self.host + # Call the WSGI app, get the HTTP response + resp = str(req.get_response(self.app)) + # For some reason, the response doesn't have "HTTP/1.0 " prepended; I + # guess that's a function the web server usually provides. + resp = "HTTP/1.0 %s" % resp + sock = FakeHttplibSocket(resp) + self.http_response = httplib.HTTPResponse(sock) + self.http_response.begin() + + def getresponse(self): + return self.http_response + + +def wire_HTTPConnection_to_WSGI(host, app): + """Monkeypatches HTTPConnection so that if you try to connect to host, you + are instead routed straight to the given WSGI app. + + After calling this method, when any code calls + + httplib.HTTPConnection(host) + + the connection object will be a fake. Its requests will be sent directly + to the given WSGI app rather than through a socket. + + Code connecting to hosts other than host will not be affected. + + This method may be called multiple times to map different hosts to + different apps. + """ + class HTTPConnectionDecorator(object): + """Wraps the real HTTPConnection class so that when you instantiate + the class you might instead get a fake instance.""" + def __init__(self, wrapped): + self.wrapped = wrapped + def __call__(self, connection_host, *args, **kwargs): + if connection_host == host: + return FakeHttplibConnection(app, host) + else: + return self.wrapped(connection_host, *args, **kwargs) + httplib.HTTPConnection = HTTPConnectionDecorator(httplib.HTTPConnection) + + +class WSGIAppProxyTest(unittest.TestCase): + + def setUp(self): + """Our WSGIAppProxy is going to call across an HTTPConnection to a + WSGIApp running a limiter. The proxy will send input, and the proxy + should receive that same input, pass it to the limiter who gives a + result, and send the expected result back. + + The HTTPConnection isn't real -- it's monkeypatched to point straight + at the WSGIApp. And the limiter isn't real -- it's a fake that + behaves the way we tell it to. + """ + self.limiter = FakeLimiter(self) + app = ratelimiting.WSGIApp(self.limiter) + wire_HTTPConnection_to_WSGI('100.100.100.100:80', app) + self.proxy = ratelimiting.WSGIAppProxy('100.100.100.100:80') + + def test_200(self): + self.limiter.mock('conquer', 'caesar', None) + when = self.proxy.perform('conquer', 'caesar') + self.assertEqual(when, None) + + def test_403(self): + self.limiter.mock('grumble', 'proletariat', 1.5) + when = self.proxy.perform('grumble', 'proletariat') + self.assertEqual(when, 1.5) + + def test_failure(self): + self.limiter.mock('murder', 'brutus', None) + try: + when = self.proxy.perform('stab', 'brutus') + except AssertionError: + pass + else: + self.fail("I didn't perform the action I expected") + + if __name__ == '__main__': unittest.main() diff --git a/nova/tests/api/rackspace/__init__.py b/nova/tests/api/rackspace/__init__.py index 2fab1a4da..622cb4335 100644 --- a/nova/tests/api/rackspace/__init__.py +++ b/nova/tests/api/rackspace/__init__.py @@ -21,7 +21,9 @@ from nova.api.rackspace import RateLimitingMiddleware from nova.tests.api.test_helper import * from webob import Request + class RateLimitingMiddlewareTest(unittest.TestCase): + def test_get_action_name(self): middleware = RateLimitingMiddleware(APIStub()) def verify(method, url, action_name): @@ -69,3 +71,9 @@ class RateLimitingMiddlewareTest(unittest.TestCase): del middleware.limiter._levels['usr1:POST'] # All 50 daily "POST servers" actions should be all used up self.exhaust(middleware, 'POST', '/servers/4', 'usr1', 0) + + def test_proxy_ctor_works(self): + middleware = RateLimitingMiddleware(APIStub()) + self.assertEqual(middleware.limiter.__class__.__name__, "Limiter") + middleware = RateLimitingMiddleware(APIStub(), service_host='foobar') + self.assertEqual(middleware.limiter.__class__.__name__, "WSGIAppProxy") -- cgit From 7437df558f3277e21a4c34a5b517a1cae5dd5a74 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Wed, 15 Sep 2010 17:17:20 -0400 Subject: Support querying version list --- nova/api/__init__.py | 13 +++++++++++++ nova/tests/api/__init__.py | 5 +++-- run_tests.py | 4 +++- tools/pip-requires | 4 ++-- 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/nova/api/__init__.py b/nova/api/__init__.py index b9b9e3988..9f116dada 100644 --- a/nova/api/__init__.py +++ b/nova/api/__init__.py @@ -21,6 +21,7 @@ Root WSGI middleware for all API controllers. """ import routes +import webob.dec from nova import wsgi from nova.api import ec2 @@ -32,6 +33,18 @@ class API(wsgi.Router): def __init__(self): mapper = routes.Mapper() + mapper.connect("/", controller=self.versions) mapper.connect("/v1.0/{path_info:.*}", controller=rackspace.API()) mapper.connect("/ec2/{path_info:.*}", controller=ec2.API()) super(API, self).__init__(mapper) + + @webob.dec.wsgify + def versions(self, req): + """Respond to a request for all OpenStack API versions.""" + response = { + "versions": [ + dict(status="CURRENT", id="v1.0")]} + metadata = { + "application/xml": { + "attributes": dict(version=["status", "id"])}} + return wsgi.Serializer(req.environ, metadata).to_content_type(response) diff --git a/nova/tests/api/__init__.py b/nova/tests/api/__init__.py index 59c4adc3d..4682c094e 100644 --- a/nova/tests/api/__init__.py +++ b/nova/tests/api/__init__.py @@ -52,8 +52,9 @@ class Test(unittest.TestCase): result = webob.Request.blank('/test/cloud').get_response(api.API()) self.assertNotEqual(result.body, "/cloud") - def test_query_api_version(self): - pass + def test_query_api_versions(self): + result = webob.Request.blank('/').get_response(api.API()) + self.assertTrue('CURRENT' in result.body) if __name__ == '__main__': unittest.main() diff --git a/run_tests.py b/run_tests.py index 77aa9088a..cf37b820e 100644 --- a/run_tests.py +++ b/run_tests.py @@ -50,8 +50,10 @@ from nova import flags from nova import twistd from nova.tests.access_unittest import * -from nova.tests.auth_unittest import * from nova.tests.api_unittest import * +from nova.tests.api import * +from nova.tests.api.rackspace import * +from nova.tests.auth_unittest import * from nova.tests.cloud_unittest import * from nova.tests.compute_unittest import * from nova.tests.flags_unittest import * diff --git a/tools/pip-requires b/tools/pip-requires index 13e8e5f45..9b8027451 100644 --- a/tools/pip-requires +++ b/tools/pip-requires @@ -6,14 +6,14 @@ amqplib==0.6.1 anyjson==0.2.4 boto==2.0b1 carrot==0.10.5 -eventlet==0.9.10 +eventlet==0.9.12 lockfile==0.8 python-daemon==1.5.5 python-gflags==1.3 redis==2.0.0 routes==1.12.3 tornado==1.0 -webob==0.9.8 +WebOb==0.9.8 wsgiref==0.1.2 zope.interface==3.6.1 mox==0.5.0 -- cgit From 5ff47a4513c3b5a7f8f90c417e1e62113797de8c Mon Sep 17 00:00:00 2001 From: Jesse Andrews Date: Sun, 19 Sep 2010 18:23:41 -0700 Subject: updated docstring --- nova/flags.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/flags.py b/nova/flags.py index efa14f9d7..64dd9d456 100644 --- a/nova/flags.py +++ b/nova/flags.py @@ -169,7 +169,7 @@ def DECLARE(name, module_string, flag_values=FLAGS): DEFINE_list('region_list', [], - 'list of region,url pairs') + 'list of region|url pairs separated by commas') DEFINE_string('connection_type', 'libvirt', 'libvirt, xenapi or fake') DEFINE_integer('s3_port', 3333, 's3 port') DEFINE_string('s3_host', '127.0.0.1', 's3 host') -- cgit From ae760b13c5382f2f4719dde445235c156cc27d18 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Mon, 20 Sep 2010 14:49:05 -0400 Subject: Use assertRaises --- nova/api/rackspace/ratelimiting/tests.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/nova/api/rackspace/ratelimiting/tests.py b/nova/api/rackspace/ratelimiting/tests.py index 13a47989b..4c9510917 100644 --- a/nova/api/rackspace/ratelimiting/tests.py +++ b/nova/api/rackspace/ratelimiting/tests.py @@ -227,13 +227,10 @@ class WSGIAppProxyTest(unittest.TestCase): self.assertEqual(when, 1.5) def test_failure(self): - self.limiter.mock('murder', 'brutus', None) - try: - when = self.proxy.perform('stab', 'brutus') - except AssertionError: - pass - else: - self.fail("I didn't perform the action I expected") + def shouldRaise(): + self.limiter.mock('murder', 'brutus', None) + self.proxy.perform('stab', 'brutus') + self.assertRaises(AssertionError, shouldRaise) if __name__ == '__main__': -- cgit From fc93548e99dea561dbf2f198b0fccc84467dbf8b Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Mon, 20 Sep 2010 17:02:32 -0400 Subject: Undo run_tests.py modification in the hopes of making this merge --- run_tests.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/run_tests.py b/run_tests.py index cf37b820e..77aa9088a 100644 --- a/run_tests.py +++ b/run_tests.py @@ -50,10 +50,8 @@ from nova import flags from nova import twistd from nova.tests.access_unittest import * -from nova.tests.api_unittest import * -from nova.tests.api import * -from nova.tests.api.rackspace import * from nova.tests.auth_unittest import * +from nova.tests.api_unittest import * from nova.tests.cloud_unittest import * from nova.tests.compute_unittest import * from nova.tests.flags_unittest import * -- cgit From ce1a8086f7ec947dd148855910a1a5a9696e33f7 Mon Sep 17 00:00:00 2001 From: Todd Willey Date: Mon, 20 Sep 2010 23:56:17 -0400 Subject: Don't use something the shell will escape as a separator. | is now =. --- nova/endpoint/cloud.py | 2 +- nova/flags.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index b28bb0dc3..2b67af96f 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -158,7 +158,7 @@ class CloudController(object): if FLAGS.region_list: regions = [] for region in FLAGS.region_list: - name, _sep, url = region.partition('|') + name, _sep, url = region.partition('=') regions.append({'regionName': name, 'regionEndpoint': url}) else: diff --git a/nova/flags.py b/nova/flags.py index 64dd9d456..c5dee2855 100644 --- a/nova/flags.py +++ b/nova/flags.py @@ -169,7 +169,7 @@ def DECLARE(name, module_string, flag_values=FLAGS): DEFINE_list('region_list', [], - 'list of region|url pairs separated by commas') + 'list of region=url pairs separated by commas') DEFINE_string('connection_type', 'libvirt', 'libvirt, xenapi or fake') DEFINE_integer('s3_port', 3333, 's3 port') DEFINE_string('s3_host', '127.0.0.1', 's3 host') -- cgit