From 04b50db56ee90c0f4dd685a8f45883522260164f Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 11 Jul 2011 14:27:01 -0700 Subject: Replace 'like' support with 'regexp' matching done in python. Since 'like' would result in a full table scan anyway, this is a bit more flexible. Make search options and matching a little more generic Return 404 when --fixed_ip doesn't match any instance, instead of a 500 only when the IP isn't in the FixedIps table. --- nova/api/ec2/cloud.py | 23 ++++++++++++++++++----- nova/api/openstack/servers.py | 20 +++++++++++--------- 2 files changed, 29 insertions(+), 14 deletions(-) (limited to 'nova/api') diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 9be30cf75..9efbb5985 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -118,8 +118,9 @@ class CloudController(object): def _get_mpi_data(self, context, project_id): result = {} + search_opts = {'project_id': project_id} for instance in self.compute_api.get_all(context, - project_id=project_id): + search_opts=search_opts): if instance['fixed_ips']: line = '%s slots=%d' % (instance['fixed_ips'][0]['address'], instance['vcpus']) @@ -145,7 +146,12 @@ class CloudController(object): def get_metadata(self, address): ctxt = context.get_admin_context() - instance_ref = self.compute_api.get_all(ctxt, fixed_ip=address) + search_opts = {'fixed_ip': address} + try: + instance_ref = self.compute_api.get_all(ctxt, + search_opts=search_opts) + except exception.NotFound: + instance_ref = None if instance_ref is None: return None @@ -816,11 +822,18 @@ class CloudController(object): instances = [] for ec2_id in instance_id: internal_id = ec2utils.ec2_id_to_id(ec2_id) - instance = self.compute_api.get(context, - instance_id=internal_id) + try: + instance = self.compute_api.get(context, + instance_id=internal_id) + except exception.NotFound: + continue instances.append(instance) else: - instances = self.compute_api.get_all(context, **kwargs) + try: + instances = self.compute_api.get_all(context, + search_opts=kwargs) + except exception.NotFound: + instances = [] for instance in instances: if not context.is_admin: if instance['image_ref'] == str(FLAGS.vpn_image_id): diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index fc1ab8d46..d259590a5 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -52,6 +52,8 @@ class Controller(object): servers = self._items(req, is_detail=False) except exception.Invalid as err: return exc.HTTPBadRequest(explanation=str(err)) + except exception.NotFound: + return exc.HTTPNotFound() return servers def detail(self, req): @@ -60,6 +62,8 @@ class Controller(object): servers = self._items(req, is_detail=True) except exception.Invalid as err: return exc.HTTPBadRequest(explanation=str(err)) + except exception.NotFound as err: + return exc.HTTPNotFound() return servers def _get_view_builder(self, req): @@ -77,16 +81,14 @@ class Controller(object): builder - the response model builder """ query_str = req.str_GET - reservation_id = query_str.get('reservation_id') - project_id = query_str.get('project_id') - fixed_ip = query_str.get('fixed_ip') - recurse_zones = utils.bool_from_str(query_str.get('recurse_zones')) + recurse_zones = utils.bool_from_str( + query_str.get('recurse_zones', False)) + # Pass all of the options on to compute's 'get_all' + search_opts = query_str + # Reset this after converting from string to bool + search_opts['recurse_zones'] = recurse_zones instance_list = self.compute_api.get_all( - req.environ['nova.context'], - reservation_id=reservation_id, - project_id=project_id, - fixed_ip=fixed_ip, - recurse_zones=recurse_zones) + req.environ['nova.context'], search_opts=search_opts) limited_list = self._limit_items(instance_list, req) builder = self._get_view_builder(req) servers = [builder.build(inst, is_detail)['server'] -- cgit From d2265cbe65f1b3940b37966245da13b9714234ef Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Sun, 17 Jul 2011 16:12:59 -0700 Subject: Refactored OS API code to allow checking of invalid query string paremeters and admin api/context to the index/detail calls. v1.0 still ignores unknown parameters, but v1.1 will return 400/BadRequest on unknown options. admin_api only commands are treated as unknown parameters if FLAGS.enable_admin_api is False. If enable_admin_api is True, non-admin context requests return 403/Forbidden. Fixed EC2 API code to handle search options to compute_api.get_all() more correctly. Reverted compute_api.get_all to ignore unknown options, since the OS API now does the verification. Updated tests. --- nova/api/ec2/cloud.py | 23 +++++--- nova/api/openstack/servers.py | 123 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 124 insertions(+), 22 deletions(-) (limited to 'nova/api') diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 0d24f0938..76725370a 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -800,11 +800,16 @@ class CloudController(object): return [{label: x} for x in lst] def describe_instances(self, context, **kwargs): - return self._format_describe_instances(context, **kwargs) + # Optional DescribeInstances argument + instance_id = kwargs.get('instance_id', None) + return self._format_describe_instances(context, + instance_id=instance_id) def describe_instances_v6(self, context, **kwargs): - kwargs['use_v6'] = True - return self._format_describe_instances(context, **kwargs) + # Optional DescribeInstancesV6 argument + instance_id = kwargs.get('instance_id', None) + return self._format_describe_instances(context, + instance_id=instance_id, use_v6=True) def _format_describe_instances(self, context, **kwargs): return {'reservationSet': self._format_instances(context, **kwargs)} @@ -814,7 +819,8 @@ class CloudController(object): assert len(i) == 1 return i[0] - def _format_instances(self, context, instance_id=None, **kwargs): + def _format_instances(self, context, instance_id=None, use_v6=False, + **search_opts): # TODO(termie): this method is poorly named as its name does not imply # that it will be making a variety of database calls # rather than simply formatting a bunch of instances that @@ -827,14 +833,15 @@ class CloudController(object): internal_id = ec2utils.ec2_id_to_id(ec2_id) try: instance = self.compute_api.get(context, - instance_id=internal_id) + instance_id=internal_id, + search_opts=search_opts) except exception.NotFound: continue instances.append(instance) else: try: instances = self.compute_api.get_all(context, - search_opts=kwargs) + search_opts=search_opts) except exception.NotFound: instances = [] for instance in instances: @@ -856,7 +863,7 @@ class CloudController(object): fixed_addr = fixed['address'] if fixed['floating_ips']: floating_addr = fixed['floating_ips'][0]['address'] - if fixed['network'] and 'use_v6' in kwargs: + if fixed['network'] and use_v6: i['dnsNameV6'] = ipv6.to_global( fixed['network']['cidr_v6'], fixed['virtual_interface']['address'], @@ -1014,7 +1021,7 @@ class CloudController(object): 'AvailabilityZone'), block_device_mapping=kwargs.get('block_device_mapping', {})) return self._format_run_instances(context, - instances[0]['reservation_id']) + instance_id=instances[0]['reservation_id']) def _do_instance(self, action, context, ec2_id): instance_id = ec2utils.ec2_id_to_id(ec2_id) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 8a947c0e0..218037d14 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -39,6 +39,41 @@ LOG = logging.getLogger('nova.api.openstack.servers') FLAGS = flags.FLAGS +def check_option_permissions(context, specified_options, + user_api_options, admin_api_options): + """Check whether or not entries in 'specified_options' are valid + based on the allowed 'user_api_options' and 'admin_api_options'. + + All inputs are lists of option names + + Returns: exception.InvalidInput for an invalid option or + exception.AdminRequired for needing admin privs + """ + + # We pretend we don't know about admin_api_options if the admin + # API is not enabled. + if FLAGS.enable_admin_api: + known_options = user_api_options + admin_api_options + else: + known_options = user_api_options + + # Check for unknown query string params. + spec_unknown_opts = [for opt in specified_options + if opt not in known_options] + if spec_unknown_opts: + unknown_opt_str = ", ".join(spec_unknown_opts) + raise exception.InvalidInput(reason=_( + "Unknown options specified: %(unknown_opt_str)")) + + # Check for admin context for the admin commands + if not context.is_admin: + spec_admin_opts = [for opt in specified_options + if opt in admin_api_options] + if spec_admin_opts: + admin_opt_str = ", ".join(admin_opts) + raise exception.AdminRequired() + + class Controller(object): """ The Server API controller for the OpenStack API """ @@ -51,9 +86,9 @@ class Controller(object): try: servers = self._items(req, is_detail=False) except exception.Invalid as err: - return exc.HTTPBadRequest(explanation=str(err)) + return faults.Fault(exc.HTTPBadRequest(explanation=str(err))) except exception.NotFound: - return exc.HTTPNotFound() + return faults.Fault(exc.HTTPNotFound()) return servers def detail(self, req): @@ -61,9 +96,9 @@ class Controller(object): try: servers = self._items(req, is_detail=True) except exception.Invalid as err: - return exc.HTTPBadRequest(explanation=str(err)) + return faults.Fault(exc.HTTPBadRequest(explanation=str(err))) except exception.NotFound as err: - return exc.HTTPNotFound() + return faults.Fault(exc.HTTPNotFound()) return servers def _get_view_builder(self, req): @@ -75,20 +110,17 @@ class Controller(object): def _action_rebuild(self, info, request, instance_id): raise NotImplementedError() - def _items(self, req, is_detail): - """Returns a list of servers for a given user. + def _get_items(self, context, req, is_detail, search_opts=None): + """Returns a list of servers. builder - the response model builder """ - query_str = req.str_GET - recurse_zones = utils.bool_from_str( - query_str.get('recurse_zones', False)) - # Pass all of the options on to compute's 'get_all' - search_opts = query_str - # Reset this after converting from string to bool - search_opts['recurse_zones'] = recurse_zones + + if search_opts is None: + search_opts = {} + instance_list = self.compute_api.get_all( - req.environ['nova.context'], search_opts=search_opts) + context, search_opts=search_opts) limited_list = self._limit_items(instance_list, req) builder = self._get_view_builder(req) servers = [builder.build(inst, is_detail)['server'] @@ -422,6 +454,41 @@ class ControllerV10(Controller): return faults.Fault(exc.HTTPNotFound()) return exc.HTTPAccepted() + def _items(self, req, is_detail): + """Returns a list of servers based on the request. + + Checks for search options and permissions on the options. + """ + + search_opts = {} + search_opts.update(req.str_GET) + + user_api = ['project_id', 'fixed_ip', 'recurse_zones', + 'reservation_id', 'name', 'fresh', 'ip', 'ip6'] + admin_api = ['instance_name'] + + context = req.environ['nova.context'] + + try: + check_option_permissions(context, search_opt.keys(), + user_api, admin_api) + except exception.InvalidInput: + # FIXME(comstud): I refactored code in here to support + # new search options, and the original code ignored + # invalid options. So, I've left it this way for now. + # The v1.1 implementation will return an error in this + # case.. + pass + except exception.AdminRequired, e: + raise faults.Fault(exc.HTTPForbidden(detail=str(e))) + + # Convert recurse_zones into a boolean + search_opts['recurse_zones'] = utils.bool_from_str( + search_opts.get('recurse_zones', False)) + + return self._get_items(context, req, is_detail, + search_opts=search_opts) + def _image_ref_from_req_data(self, data): return data['server']['imageId'] @@ -493,6 +560,34 @@ class ControllerV11(Controller): except exception.NotFound: return faults.Fault(exc.HTTPNotFound()) + def _items(self, req, is_detail): + """Returns a list of servers based on the request. + + Checks for search options and permissions on the options. + """ + + search_opts = {} + search_opts.update(req.str_GET) + + user_api = ['image', 'flavor', 'name', 'status', + 'reservation_id', 'changes-since', 'ip', 'ip6'] + admin_api = ['ip', 'ip6', 'instance_name'] + + context = req.environ['nova.context'] + + try: + check_option_permissions(context, search_opt.keys(), + user_api, admin_api) + except exception.InvalidInput, e: + raise faults.Fault(exc.HTTPBadRequest(detail=str(e))) + except exception.AdminRequired, e: + raise faults.Fault(exc.HTTPForbidden(detail=str(e))) + + # NOTE(comstud): Making recurse_zones always be True in v1.1 + search_opts['recurse_zones'] = True + return self._get_items(context, req, is_detail, + search_opts=search_opts) + def _image_ref_from_req_data(self, data): return data['server']['imageRef'] -- cgit From 102a0e5b9d6ce22a5fc5a00fc260bbe1e3592222 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 18 Jul 2011 02:45:10 -0700 Subject: added searching by 'image', 'flavor', and 'status' reverted ip/ip6 searching to be admin only --- nova/api/openstack/servers.py | 18 +++++++++++++----- nova/api/openstack/views/servers.py | 15 +-------------- 2 files changed, 14 insertions(+), 19 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 218037d14..fb1ce2529 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -119,6 +119,14 @@ class Controller(object): if search_opts is None: search_opts = {} + # If search by 'status', we need to convert it to 'state' + # If the status is unknown, bail + status = search_opts.pop('status', None) + if status is not None: + search_opts['state'] = power_state.states_from_status(status) + if len(search_opts['state']) == 0: + raise exception.InvalidInput(reason=_( + 'Invalid server status')) instance_list = self.compute_api.get_all( context, search_opts=search_opts) limited_list = self._limit_items(instance_list, req) @@ -464,8 +472,8 @@ class ControllerV10(Controller): search_opts.update(req.str_GET) user_api = ['project_id', 'fixed_ip', 'recurse_zones', - 'reservation_id', 'name', 'fresh', 'ip', 'ip6'] - admin_api = ['instance_name'] + 'reservation_id', 'name', 'fresh', 'status'] + admin_api = ['ip', 'ip6', 'instance_name'] context = req.environ['nova.context'] @@ -570,7 +578,7 @@ class ControllerV11(Controller): search_opts.update(req.str_GET) user_api = ['image', 'flavor', 'name', 'status', - 'reservation_id', 'changes-since', 'ip', 'ip6'] + 'reservation_id', 'changes-since'] admin_api = ['ip', 'ip6', 'instance_name'] context = req.environ['nova.context'] @@ -579,9 +587,9 @@ class ControllerV11(Controller): check_option_permissions(context, search_opt.keys(), user_api, admin_api) except exception.InvalidInput, e: - raise faults.Fault(exc.HTTPBadRequest(detail=str(e))) + raise faults.Fault(exc.HTTPBadRequest(explanation=str(e))) except exception.AdminRequired, e: - raise faults.Fault(exc.HTTPForbidden(detail=str(e))) + raise faults.Fault(exc.HTTPForbidden(explanation=str(e))) # NOTE(comstud): Making recurse_zones always be True in v1.1 search_opts['recurse_zones'] = True diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 67fb6a84e..1883ce2a5 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -60,25 +60,12 @@ class ViewBuilder(object): def _build_detail(self, inst): """Returns a detailed model of a server.""" - power_mapping = { - None: 'BUILD', - power_state.NOSTATE: 'BUILD', - power_state.RUNNING: 'ACTIVE', - power_state.BLOCKED: 'ACTIVE', - power_state.SUSPENDED: 'SUSPENDED', - power_state.PAUSED: 'PAUSED', - power_state.SHUTDOWN: 'SHUTDOWN', - power_state.SHUTOFF: 'SHUTOFF', - power_state.CRASHED: 'ERROR', - power_state.FAILED: 'ERROR', - power_state.BUILDING: 'BUILD', - } inst_dict = { 'id': inst['id'], 'name': inst['display_name'], 'addresses': self.addresses_builder.build(inst), - 'status': power_mapping[inst.get('state')]} + 'status': power_state.status_from_state(inst.get('state'))} ctxt = nova.context.get_admin_context() compute_api = nova.compute.API() -- cgit From 68ca0a6e770eadf1ed56aa9d0bef14c5ca16e172 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 18 Jul 2011 02:49:42 -0700 Subject: add image and flavor searching to v1.0 api fixed missing updates from cut n paste in some doc strings --- nova/api/openstack/servers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index fb1ce2529..b9347a014 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -472,7 +472,8 @@ class ControllerV10(Controller): search_opts.update(req.str_GET) user_api = ['project_id', 'fixed_ip', 'recurse_zones', - 'reservation_id', 'name', 'fresh', 'status'] + 'reservation_id', 'name', 'fresh', 'status', + 'image', 'flavor'] admin_api = ['ip', 'ip6', 'instance_name'] context = req.environ['nova.context'] -- cgit From a6968a100d2a2409094f7b434a88c700ebb876f3 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 18 Jul 2011 02:59:03 -0700 Subject: flavor needs to be converted to int from query string value --- nova/api/openstack/servers.py | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index b9347a014..f470c59e3 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -494,6 +494,9 @@ class ControllerV10(Controller): # Convert recurse_zones into a boolean search_opts['recurse_zones'] = utils.bool_from_str( search_opts.get('recurse_zones', False)) + # convert flavor into an int + if 'flavor' in search_opts: + search_opts['flavor'] = int(search_opts['flavor']) return self._get_items(context, req, is_detail, search_opts=search_opts) @@ -594,6 +597,9 @@ class ControllerV11(Controller): # NOTE(comstud): Making recurse_zones always be True in v1.1 search_opts['recurse_zones'] = True + # convert flavor into an int + if 'flavor' in search_opts: + search_opts['flavor'] = int(search_opts['flavor']) return self._get_items(context, req, is_detail, search_opts=search_opts) -- cgit From bfee5105a2e557a28a605778599e99308f2a126e Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 18 Jul 2011 03:02:50 -0700 Subject: typos --- nova/api/openstack/servers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index f470c59e3..9bfcf585b 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -58,7 +58,7 @@ def check_option_permissions(context, specified_options, known_options = user_api_options # Check for unknown query string params. - spec_unknown_opts = [for opt in specified_options + spec_unknown_opts = [opt for opt in specified_options if opt not in known_options] if spec_unknown_opts: unknown_opt_str = ", ".join(spec_unknown_opts) @@ -67,7 +67,7 @@ def check_option_permissions(context, specified_options, # Check for admin context for the admin commands if not context.is_admin: - spec_admin_opts = [for opt in specified_options + spec_admin_opts = [opt for opt in specified_options if opt in admin_api_options] if spec_admin_opts: admin_opt_str = ", ".join(admin_opts) -- cgit From edaeb96d6ce9c14b1f70a71c219d0353b59ed270 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 18 Jul 2011 03:08:23 -0700 Subject: more typos --- nova/api/openstack/servers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 9bfcf585b..771939624 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -52,7 +52,7 @@ def check_option_permissions(context, specified_options, # We pretend we don't know about admin_api_options if the admin # API is not enabled. - if FLAGS.enable_admin_api: + if FLAGS.allow_admin_api: known_options = user_api_options + admin_api_options else: known_options = user_api_options @@ -479,7 +479,7 @@ class ControllerV10(Controller): context = req.environ['nova.context'] try: - check_option_permissions(context, search_opt.keys(), + check_option_permissions(context, search_opts.keys(), user_api, admin_api) except exception.InvalidInput: # FIXME(comstud): I refactored code in here to support -- cgit From 043cfae7737a977f7f03d75910742f741b832323 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 18 Jul 2011 03:36:08 -0700 Subject: missed power_state import in api fixed reversed compare in power_state --- nova/api/openstack/servers.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 771939624..5a4dcbd9e 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -32,6 +32,7 @@ import nova.api.openstack.views.images import nova.api.openstack.views.servers from nova.api.openstack import wsgi import nova.api.openstack +from nova.compute import power_state from nova.scheduler import api as scheduler_api @@ -125,8 +126,9 @@ class Controller(object): if status is not None: search_opts['state'] = power_state.states_from_status(status) if len(search_opts['state']) == 0: - raise exception.InvalidInput(reason=_( - 'Invalid server status')) + reason = _('Invalid server status: %(status)s') % locals() + LOG.error(reason) + raise exception.InvalidInput(reason=reason) instance_list = self.compute_api.get_all( context, search_opts=search_opts) limited_list = self._limit_items(instance_list, req) @@ -482,7 +484,7 @@ class ControllerV10(Controller): check_option_permissions(context, search_opts.keys(), user_api, admin_api) except exception.InvalidInput: - # FIXME(comstud): I refactored code in here to support + # NOTE(comstud): I refactored code in here to support # new search options, and the original code ignored # invalid options. So, I've left it this way for now. # The v1.1 implementation will return an error in this -- cgit From 5a2add5c6011ce94f4727037c193274d21351cb2 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 18 Jul 2011 04:13:22 -0700 Subject: another typo --- nova/api/openstack/servers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 8df4ce31d..8c1638e21 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -588,7 +588,7 @@ class ControllerV11(Controller): context = req.environ['nova.context'] try: - check_option_permissions(context, search_opt.keys(), + check_option_permissions(context, search_opts.keys(), user_api, admin_api) except exception.InvalidInput, e: raise faults.Fault(exc.HTTPBadRequest(explanation=str(e))) -- cgit From 0b9048bc3285b86a073da9aa9327815319aaa184 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Tue, 19 Jul 2011 12:44:00 -0700 Subject: allow 'marker' and 'limit' in search options. fix log format error --- nova/api/openstack/servers.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 8c1638e21..17a3df344 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -64,8 +64,10 @@ def check_option_permissions(context, specified_options, if opt not in known_options] if spec_unknown_opts: unknown_opt_str = ", ".join(spec_unknown_opts) + LOG.error(_("Received request for unknown options " + "'%(unknown_opt_str)s'") % locals()) raise exception.InvalidInput(reason=_( - "Unknown options specified: %(unknown_opt_str)")) + "Unknown options specified: %(unknown_opt_str)s")) # Check for admin context for the admin commands if not context.is_admin: @@ -73,6 +75,9 @@ def check_option_permissions(context, specified_options, if opt in admin_api_options] if spec_admin_opts: admin_opt_str = ", ".join(admin_opts) + LOG.error(_("Received request for admin options " + "'%(admin_opt_str)s' from non-admin context") % + locals()) raise exception.AdminRequired() @@ -471,9 +476,9 @@ class ControllerV10(Controller): search_opts = {} search_opts.update(req.str_GET) - user_api = ['project_id', 'fixed_ip', 'recurse_zones', - 'reservation_id', 'name', 'fresh', 'status', - 'image', 'flavor'] + user_api = ['marker', 'limit', 'project_id', 'fixed_ip', + 'recurse_zones', 'reservation_id', 'name', 'fresh', + 'status', 'image', 'flavor'] admin_api = ['ip', 'ip6', 'instance_name'] context = req.environ['nova.context'] @@ -581,8 +586,8 @@ class ControllerV11(Controller): search_opts = {} search_opts.update(req.str_GET) - user_api = ['image', 'flavor', 'name', 'status', - 'reservation_id', 'changes-since'] + user_api = ['marker', 'limit', 'image', 'flavor', 'name', + 'status', 'reservation_id', 'changes-since'] admin_api = ['ip', 'ip6', 'instance_name'] context = req.environ['nova.context'] -- cgit From 7630aa8acc376364375ef48a3d955a7c21f50b04 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 20 Jul 2011 11:02:00 -0700 Subject: added API tests for search options fixed a couple of bugs the tests caught --- nova/api/openstack/servers.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 17a3df344..ed3f82039 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -67,14 +67,15 @@ def check_option_permissions(context, specified_options, LOG.error(_("Received request for unknown options " "'%(unknown_opt_str)s'") % locals()) raise exception.InvalidInput(reason=_( - "Unknown options specified: %(unknown_opt_str)s")) + "Unknown options specified: %(unknown_opt_str)s") % + locals()) # Check for admin context for the admin commands if not context.is_admin: spec_admin_opts = [opt for opt in specified_options if opt in admin_api_options] if spec_admin_opts: - admin_opt_str = ", ".join(admin_opts) + admin_opt_str = ", ".join(spec_admin_opts) LOG.error(_("Received request for admin options " "'%(admin_opt_str)s' from non-admin context") % locals()) -- cgit From b1099b43f34e41676b0508267e9ad40b2c3415e3 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 20 Jul 2011 11:32:43 -0700 Subject: ec2 fixes --- nova/api/ec2/cloud.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'nova/api') diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 76725370a..9aef5079c 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -832,9 +832,7 @@ class CloudController(object): for ec2_id in instance_id: internal_id = ec2utils.ec2_id_to_id(ec2_id) try: - instance = self.compute_api.get(context, - instance_id=internal_id, - search_opts=search_opts) + instance = self.compute_api.get(context, internal_id) except exception.NotFound: continue instances.append(instance) @@ -1021,7 +1019,7 @@ class CloudController(object): 'AvailabilityZone'), block_device_mapping=kwargs.get('block_device_mapping', {})) return self._format_run_instances(context, - instance_id=instances[0]['reservation_id']) + reservation_id=instances[0]['reservation_id']) def _do_instance(self, action, context, ec2_id): instance_id = ec2utils.ec2_id_to_id(ec2_id) -- cgit From a1cb17bf98359fae760800f8467c897d859b6994 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 20 Jul 2011 12:16:23 -0700 Subject: minor fixups --- nova/api/openstack/servers.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index ed3f82039..8ec74b387 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -64,11 +64,10 @@ def check_option_permissions(context, specified_options, if opt not in known_options] if spec_unknown_opts: unknown_opt_str = ", ".join(spec_unknown_opts) - LOG.error(_("Received request for unknown options " - "'%(unknown_opt_str)s'") % locals()) - raise exception.InvalidInput(reason=_( - "Unknown options specified: %(unknown_opt_str)s") % - locals()) + reason = _("Received request for unknown options " + "'%(unknown_opt_str)s'") % locals() + LOG.error(reason) + raise exception.InvalidInput(reason=reason) # Check for admin context for the admin commands if not context.is_admin: @@ -136,6 +135,11 @@ class Controller(object): reason = _('Invalid server status: %(status)s') % locals() LOG.error(reason) raise exception.InvalidInput(reason=reason) + + # Don't pass these along to compute API, if they exist. + search_opts.pop('changes-since', None) + search_opts.pop('fresh', None) + instance_list = self.compute_api.get_all( context, search_opts=search_opts) limited_list = self._limit_items(instance_list, req) -- cgit From 994e219ab0b25d48b31484a43a0ac12099cf226e Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 3 Aug 2011 00:46:38 -0700 Subject: rework OS API checking of search options --- nova/api/openstack/servers.py | 181 ++++++++++++++++-------------------------- 1 file changed, 70 insertions(+), 111 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 2573fc48c..b028d3a40 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -41,44 +41,46 @@ LOG = logging.getLogger('nova.api.openstack.servers') FLAGS = flags.FLAGS -def check_option_permissions(context, specified_options, - user_api_options, admin_api_options): - """Check whether or not entries in 'specified_options' are valid - based on the allowed 'user_api_options' and 'admin_api_options'. +def check_admin_search_options(context, search_options, admin_api_options): + """Check for any 'admin_api_options' specified in 'search_options'. + + If admin api is not enabled, we should pretend that we know nothing + about those options.. Ie, they don't exist in user-facing API. To + achieve this, we will strip any admin options that we find from + search_options + + If admin api is enabled, we should require admin context for any + admin options specified, and return an exception in this case. + + If any exist and admin api is not enabled, strip them from + search_options (has the effect of treating them like they don't exist). + + search_options is a dictionary of "search_option": value + admin_api_options is a list + + Returns: None if options are okay. + Modifies: admin options could be stripped from search_options + Raises: exception.AdminRequired for needing admin context + """ - All inputs are lists of option names + if not FLAGS.allow_admin_api: + # Remove any admin_api_options from search_options + for option in admin_api_options: + search_options.pop(option, None) + return - Returns: exception.InvalidInput for an invalid option or - exception.AdminRequired for needing admin privs - """ + # allow_admin_api is True and admin context? Any command is okay. + if context.is_admin: + return - # We pretend we don't know about admin_api_options if the admin - # API is not enabled. - if FLAGS.allow_admin_api: - known_options = user_api_options + admin_api_options - else: - known_options = user_api_options - - # Check for unknown query string params. - spec_unknown_opts = [opt for opt in specified_options - if opt not in known_options] - if spec_unknown_opts: - unknown_opt_str = ", ".join(spec_unknown_opts) - reason = _("Received request for unknown options " - "'%(unknown_opt_str)s'") % locals() - LOG.error(reason) - raise exception.InvalidInput(reason=reason) - - # Check for admin context for the admin commands - if not context.is_admin: - spec_admin_opts = [opt for opt in specified_options - if opt in admin_api_options] - if spec_admin_opts: - admin_opt_str = ", ".join(spec_admin_opts) - LOG.error(_("Received request for admin options " - "'%(admin_opt_str)s' from non-admin context") % - locals()) - raise exception.AdminRequired() + spec_admin_opts = [opt for opt in search_options.iterkeys() + if opt in admin_api_options] + if spec_admin_opts: + admin_opt_str = ", ".join(spec_admin_opts) + LOG.error(_("Received request for admin-only search options " + "'%(admin_opt_str)s' from non-admin context") % + locals()) + raise exception.AdminRequired() class Controller(object): @@ -91,7 +93,7 @@ class Controller(object): def index(self, req): """ Returns a list of server names and ids for a given user """ try: - servers = self._items(req, is_detail=False) + servers = self._servers_from_request(req, is_detail=False) except exception.Invalid as err: return faults.Fault(exc.HTTPBadRequest(explanation=str(err))) except exception.NotFound: @@ -101,7 +103,7 @@ class Controller(object): def detail(self, req): """ Returns a list of server details for a given user """ try: - servers = self._items(req, is_detail=True) + servers = self._servers_from_request(req, is_detail=True) except exception.Invalid as err: return faults.Fault(exc.HTTPBadRequest(explanation=str(err))) except exception.NotFound as err: @@ -117,10 +119,9 @@ class Controller(object): def _action_rebuild(self, info, request, instance_id): raise NotImplementedError() - def _get_items(self, context, req, is_detail, search_opts=None): - """Returns a list of servers. - - builder - the response model builder + def _servers_search(self, context, req, is_detail, search_opts=None): + """Returns a list of servers, taking into account any search + options specified. """ if search_opts is None: @@ -147,6 +148,34 @@ class Controller(object): for inst in limited_list] return dict(servers=servers) + def _servers_from_request(self, req, is_detail): + """Returns a list of servers based on the request. + + Checks for search options and permissions on the options. + """ + + search_opts = {} + search_opts.update(req.str_GET) + + admin_api = ['ip', 'ip6', 'instance_name'] + + context = req.environ['nova.context'] + + try: + check_admin_search_options(context, search_opts, admin_api) + except exception.AdminRequired, e: + raise exc.HTTPForbidden(detail=str(e)) + + # Convert recurse_zones into a boolean + search_opts['recurse_zones'] = utils.bool_from_str( + search_opts.get('recurse_zones', False)) + # convert flavor into an int + if 'flavor' in search_opts: + search_opts['flavor'] = int(search_opts['flavor']) + + return self._servers_search(context, req, is_detail, + search_opts=search_opts) + @scheduler_api.redirect_handler def show(self, req, id): """ Returns server details by server id """ @@ -469,45 +498,6 @@ class ControllerV10(Controller): raise exc.HTTPNotFound() return webob.Response(status_int=202) - def _items(self, req, is_detail): - """Returns a list of servers based on the request. - - Checks for search options and permissions on the options. - """ - - search_opts = {} - search_opts.update(req.str_GET) - - user_api = ['marker', 'limit', 'project_id', 'fixed_ip', - 'recurse_zones', 'reservation_id', 'name', 'fresh', - 'status', 'image', 'flavor'] - admin_api = ['ip', 'ip6', 'instance_name'] - - context = req.environ['nova.context'] - - try: - check_option_permissions(context, search_opts.keys(), - user_api, admin_api) - except exception.InvalidInput: - # NOTE(comstud): I refactored code in here to support - # new search options, and the original code ignored - # invalid options. So, I've left it this way for now. - # The v1.1 implementation will return an error in this - # case.. - pass - except exception.AdminRequired, e: - raise faults.Fault(exc.HTTPForbidden(detail=str(e))) - - # Convert recurse_zones into a boolean - search_opts['recurse_zones'] = utils.bool_from_str( - search_opts.get('recurse_zones', False)) - # convert flavor into an int - if 'flavor' in search_opts: - search_opts['flavor'] = int(search_opts['flavor']) - - return self._get_items(context, req, is_detail, - search_opts=search_opts) - def _image_ref_from_req_data(self, data): return data['server']['imageId'] @@ -572,37 +562,6 @@ class ControllerV11(Controller): except exception.NotFound: raise exc.HTTPNotFound() - def _items(self, req, is_detail): - """Returns a list of servers based on the request. - - Checks for search options and permissions on the options. - """ - - search_opts = {} - search_opts.update(req.str_GET) - - user_api = ['marker', 'limit', 'image', 'flavor', 'name', - 'status', 'reservation_id', 'changes-since'] - admin_api = ['ip', 'ip6', 'instance_name'] - - context = req.environ['nova.context'] - - try: - check_option_permissions(context, search_opts.keys(), - user_api, admin_api) - except exception.InvalidInput, e: - raise faults.Fault(exc.HTTPBadRequest(explanation=str(e))) - except exception.AdminRequired, e: - raise faults.Fault(exc.HTTPForbidden(explanation=str(e))) - - # NOTE(comstud): Making recurse_zones always be True in v1.1 - search_opts['recurse_zones'] = True - # convert flavor into an int - if 'flavor' in search_opts: - search_opts['flavor'] = int(search_opts['flavor']) - return self._get_items(context, req, is_detail, - search_opts=search_opts) - def _image_ref_from_req_data(self, data): return data['server']['imageRef'] -- cgit From a5390a5b1cb95ca9aee6e2f99572498dd60b48e5 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 3 Aug 2011 00:53:13 -0700 Subject: remove faults.Fault wrapper on exceptions --- nova/api/openstack/servers.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index b028d3a40..6ad549c97 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -95,9 +95,9 @@ class Controller(object): try: servers = self._servers_from_request(req, is_detail=False) except exception.Invalid as err: - return faults.Fault(exc.HTTPBadRequest(explanation=str(err))) + return exc.HTTPBadRequest(explanation=str(err)) except exception.NotFound: - return faults.Fault(exc.HTTPNotFound()) + return exc.HTTPNotFound() return servers def detail(self, req): @@ -105,9 +105,9 @@ class Controller(object): try: servers = self._servers_from_request(req, is_detail=True) except exception.Invalid as err: - return faults.Fault(exc.HTTPBadRequest(explanation=str(err))) + return exc.HTTPBadRequest(explanation=str(err)) except exception.NotFound as err: - return faults.Fault(exc.HTTPNotFound()) + return exc.HTTPNotFound() return servers def _build_view(self, req, instance, is_detail=False): -- cgit From c0851f2ec5be12c43cc96367e22220d25589e4ae Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 01:36:12 -0700 Subject: cleanup checking of options in the API before calling compute_api's get_all() --- nova/api/openstack/servers.py | 104 ++++++++++++++++++------------------------ 1 file changed, 45 insertions(+), 59 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 965cf0bfc..e3e829e81 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -44,55 +44,25 @@ LOG = logging.getLogger('nova.api.openstack.servers') FLAGS = flags.FLAGS -def check_admin_search_options(context, search_options, admin_api_options): - """Check for any 'admin_api_options' specified in 'search_options'. - - If admin api is not enabled, we should pretend that we know nothing - about those options.. Ie, they don't exist in user-facing API. To - achieve this, we will strip any admin options that we find from - search_options - - If admin api is enabled, we should require admin context for any - admin options specified, and return an exception in this case. - - If any exist and admin api is not enabled, strip them from - search_options (has the effect of treating them like they don't exist). - - search_options is a dictionary of "search_option": value - admin_api_options is a list - - Returns: None if options are okay. - Modifies: admin options could be stripped from search_options - Raises: exception.AdminRequired for needing admin context - """ - - if not FLAGS.allow_admin_api: - # Remove any admin_api_options from search_options - for option in admin_api_options: - search_options.pop(option, None) - return - - # allow_admin_api is True and admin context? Any command is okay. - if context.is_admin: - return - - spec_admin_opts = [opt for opt in search_options.iterkeys() - if opt in admin_api_options] - if spec_admin_opts: - admin_opt_str = ", ".join(spec_admin_opts) - LOG.error(_("Received request for admin-only search options " - "'%(admin_opt_str)s' from non-admin context") % - locals()) - raise exception.AdminRequired() - - class Controller(object): - """ The Server API controller for the OpenStack API """ + """ The Server API base controller class for the OpenStack API """ + + servers_search_options = [] def __init__(self): self.compute_api = compute.API() self.helper = helper.CreateInstanceHelper(self) + def _check_servers_options(self, search_options): + if FLAGS.allow_admin_api and context.is_admin: + # Allow all options + return + # Otherwise, strip out all unknown options + unknown_options = [opt for opt in search_options + if opt not in self.servers_search_options] + for opt in unknown_options: + search_options.pop(opt, None) + def index(self, req): """ Returns a list of server names and ids for a given user """ try: @@ -131,21 +101,38 @@ class Controller(object): search_opts = {} # If search by 'status', we need to convert it to 'state' - # If the status is unknown, bail - status = search_opts.pop('status', None) - if status is not None: + # If the status is unknown, bail. + # Leave 'state' in search_opts so compute can pass it on to + # child zones.. + if 'status' in search_opts: + status = search_opts['status'] search_opts['state'] = power_state.states_from_status(status) if len(search_opts['state']) == 0: reason = _('Invalid server status: %(status)s') % locals() LOG.error(reason) raise exception.InvalidInput(reason=reason) - # Don't pass these along to compute API, if they exist. - search_opts.pop('changes-since', None) - search_opts.pop('fresh', None) + # By default, compute's get_all() will return deleted instances. + # If an admin hasn't specified a 'deleted' search option, we need + # to filter out deleted instances by setting the filter ourselves. + # ... Unless 'changes-since' is specified, because 'changes-since' + # should return recently deleted images according to the API spec. + + if 'deleted' not in search_opts: + # Admin hasn't specified deleted filter + if 'changes-since' not in search_opts: + # No 'changes-since', so we need to find non-deleted servers + search_opts['deleted'] = '^False$' + else: + # This is the default, but just in case.. + search_opts['deleted'] = '^True$' instance_list = self.compute_api.get_all( context, search_opts=search_opts) + + # FIXME(comstud): 'changes-since' is not fully implemented. Where + # should this be filtered? + limited_list = self._limit_items(instance_list, req) servers = [self._build_view(req, inst, is_detail)['server'] for inst in limited_list] @@ -160,21 +147,12 @@ class Controller(object): search_opts = {} search_opts.update(req.str_GET) - admin_api = ['ip', 'ip6', 'instance_name'] - context = req.environ['nova.context'] - - try: - check_admin_search_options(context, search_opts, admin_api) - except exception.AdminRequired, e: - raise exc.HTTPForbidden(detail=str(e)) + self._check_servers_options(context, search_opts) # Convert recurse_zones into a boolean search_opts['recurse_zones'] = utils.bool_from_str( search_opts.get('recurse_zones', False)) - # convert flavor into an int - if 'flavor' in search_opts: - search_opts['flavor'] = int(search_opts['flavor']) return self._servers_search(context, req, is_detail, search_opts=search_opts) @@ -581,6 +559,10 @@ class Controller(object): class ControllerV10(Controller): + """v1.0 OpenStack API controller""" + + servers_search_options = ["reservation_id", "fixed_ip", + "name", "recurse_zones"] @scheduler_api.redirect_handler def delete(self, req, id): @@ -645,6 +627,10 @@ class ControllerV10(Controller): class ControllerV11(Controller): + """v1.1 OpenStack API controller""" + + servers_search_options = ["reservation_id", "name", "recurse_zones", + "status", "image", "flavor", "changes-since"] @scheduler_api.redirect_handler def delete(self, req, id): -- cgit From b1b919d42d8c359fc9ae981b44466d269fc688a6 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 13:59:57 -0700 Subject: test fixes and typos --- nova/api/openstack/common.py | 33 +++++++++++++++++++++++++++++++++ nova/api/openstack/servers.py | 9 ++++----- nova/api/openstack/views/servers.py | 3 +-- 3 files changed, 38 insertions(+), 7 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index efa4ab385..bbf46261b 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -25,6 +25,7 @@ from nova import exception from nova import flags from nova import log as logging from nova.api.openstack import wsgi +from nova.compute import power_state as compute_power_state LOG = logging.getLogger('nova.api.openstack.common') @@ -35,6 +36,38 @@ XML_NS_V10 = 'http://docs.rackspacecloud.com/servers/api/v1.0' XML_NS_V11 = 'http://docs.openstack.org/compute/api/v1.1' +_STATUS_MAP = { + None: 'BUILD', + compute_power_state.NOSTATE: 'BUILD', + compute_power_state.RUNNING: 'ACTIVE', + compute_power_state.BLOCKED: 'ACTIVE', + compute_power_state.SUSPENDED: 'SUSPENDED', + compute_power_state.PAUSED: 'PAUSED', + compute_power_state.SHUTDOWN: 'SHUTDOWN', + compute_power_state.SHUTOFF: 'SHUTOFF', + compute_power_state.CRASHED: 'ERROR', + compute_power_state.FAILED: 'ERROR', + compute_power_state.BUILDING: 'BUILD', +} + + +def status_from_power_state(power_state): + """Map the power state to the server status string""" + return _STATUS_MAP[power_state] + + +def power_states_from_status(status): + """Map the server status string to a list of power states""" + power_states = [] + for power_state, status_map in _STATUS_MAP.iteritems(): + # Skip the 'None' state + if power_state is None: + continue + if status.lower() == status_map.lower(): + power_states.append(power_state) + return power_states + + def get_pagination_params(request): """Return marker, limit tuple from request. diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index e3e829e81..c842fcc01 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -31,7 +31,6 @@ from nova.api.openstack import create_instance_helper as helper from nova.api.openstack import ips from nova.api.openstack import wsgi from nova.compute import instance_types -from nova.compute import power_state from nova.scheduler import api as scheduler_api import nova.api.openstack import nova.api.openstack.views.addresses @@ -53,7 +52,7 @@ class Controller(object): self.compute_api = compute.API() self.helper = helper.CreateInstanceHelper(self) - def _check_servers_options(self, search_options): + def _check_servers_options(self, context, search_options): if FLAGS.allow_admin_api and context.is_admin: # Allow all options return @@ -106,7 +105,7 @@ class Controller(object): # child zones.. if 'status' in search_opts: status = search_opts['status'] - search_opts['state'] = power_state.states_from_status(status) + search_opts['state'] = common.power_states_from_status(status) if len(search_opts['state']) == 0: reason = _('Invalid server status: %(status)s') % locals() LOG.error(reason) @@ -122,10 +121,10 @@ class Controller(object): # Admin hasn't specified deleted filter if 'changes-since' not in search_opts: # No 'changes-since', so we need to find non-deleted servers - search_opts['deleted'] = '^False$' + search_opts['deleted'] = False else: # This is the default, but just in case.. - search_opts['deleted'] = '^True$' + search_opts['deleted'] = True instance_list = self.compute_api.get_all( context, search_opts=search_opts) diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 64be00f86..8222f6766 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -20,7 +20,6 @@ import hashlib import os from nova import exception -from nova.compute import power_state import nova.compute import nova.context from nova.api.openstack import common @@ -65,7 +64,7 @@ class ViewBuilder(object): inst_dict = { 'id': inst['id'], 'name': inst['display_name'], - 'status': power_state.status_from_state(inst.get('state'))} + 'status': common.status_from_power_state(inst.get('state'))} ctxt = nova.context.get_admin_context() compute_api = nova.compute.API() -- cgit From e36232aed703eca43c6eb6df02a5c2aa0a1ac649 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 14:40:06 -0700 Subject: fix OS API tests --- nova/api/openstack/servers.py | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index c842fcc01..e10e5bc86 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -59,9 +59,14 @@ class Controller(object): # Otherwise, strip out all unknown options unknown_options = [opt for opt in search_options if opt not in self.servers_search_options] + unk_opt_str = ", ".join(unknown_options) + log_msg = _("Stripping out options '%(unk_opt_str)s' from servers " + "query") % locals() + LOG.debug(log_msg) for opt in unknown_options: search_options.pop(opt, None) + def index(self, req): """ Returns a list of server names and ids for a given user """ try: -- cgit From 2b45204e593f9330c8b961cfae3ad5af0bd36642 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 14:47:05 -0700 Subject: doc string fix --- nova/api/openstack/servers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index e10e5bc86..b89b24047 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -145,7 +145,8 @@ class Controller(object): def _servers_from_request(self, req, is_detail): """Returns a list of servers based on the request. - Checks for search options and permissions on the options. + Checks for search options and strips out options that should + not be available to non-admins. """ search_opts = {} -- cgit From 64966cbd83cbde6a240dad4ac786fe7a6a116f2f Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 17:30:07 -0700 Subject: pep8 fixes --- nova/api/openstack/servers.py | 1 - 1 file changed, 1 deletion(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 63791bcd1..b54897b8a 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -66,7 +66,6 @@ class Controller(object): for opt in unknown_options: search_options.pop(opt, None) - def index(self, req): """ Returns a list of server names and ids for a given user """ try: -- cgit From 8aeec07c2a5f8a5f1cfb049e20caa29295496606 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 17:36:39 -0700 Subject: add comment for servers_search_options list in the OS API Controllers. --- nova/api/openstack/servers.py | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index b54897b8a..ae0b103bd 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -46,6 +46,10 @@ FLAGS = flags.FLAGS class Controller(object): """ The Server API base controller class for the OpenStack API """ + # These are a list of possible query string paramters to the + # /servers query that a user should be able to do. Specify this + # in your subclasses. When admin api is off, unknown options will + # get filtered out without error. servers_search_options = [] def __init__(self): @@ -565,6 +569,9 @@ class Controller(object): class ControllerV10(Controller): """v1.0 OpenStack API controller""" + # These are a list of possible query string paramters to the + # /servers query that a user should be able to do. When admin api + # is off, unknown options will get filtered out without error. servers_search_options = ["reservation_id", "fixed_ip", "name", "recurse_zones"] @@ -633,6 +640,9 @@ class ControllerV10(Controller): class ControllerV11(Controller): """v1.1 OpenStack API controller""" + # These are a list of possible query string paramters to the + # /servers query that a user should be able to do. When admin api + # is off, unknown options will get filtered out without error. servers_search_options = ["reservation_id", "name", "recurse_zones", "status", "image", "flavor", "changes-since"] -- cgit From fd9a761f25c6095d1ea47e09cdac503683b44bfc Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 17:59:22 -0700 Subject: pep8 fix --- nova/api/openstack/servers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index ae0b103bd..b3776ed44 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -46,7 +46,7 @@ FLAGS = flags.FLAGS class Controller(object): """ The Server API base controller class for the OpenStack API """ - # These are a list of possible query string paramters to the + # These are a list of possible query string paramters to the # /servers query that a user should be able to do. Specify this # in your subclasses. When admin api is off, unknown options will # get filtered out without error. @@ -569,7 +569,7 @@ class Controller(object): class ControllerV10(Controller): """v1.0 OpenStack API controller""" - # These are a list of possible query string paramters to the + # These are a list of possible query string paramters to the # /servers query that a user should be able to do. When admin api # is off, unknown options will get filtered out without error. servers_search_options = ["reservation_id", "fixed_ip", @@ -640,7 +640,7 @@ class ControllerV10(Controller): class ControllerV11(Controller): """v1.1 OpenStack API controller""" - # These are a list of possible query string paramters to the + # These are a list of possible query string paramters to the # /servers query that a user should be able to do. When admin api # is off, unknown options will get filtered out without error. servers_search_options = ["reservation_id", "name", "recurse_zones", -- cgit From b2fe710c59ba266b9afd67db1cae60a6db5c71e3 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Fri, 5 Aug 2011 15:06:07 -0700 Subject: rename _check_servers_options, add some comments and small cleanup in the db get_by_filters call --- nova/api/openstack/servers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index b3776ed44..9a3872113 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -56,7 +56,7 @@ class Controller(object): self.compute_api = compute.API() self.helper = helper.CreateInstanceHelper(self) - def _check_servers_options(self, context, search_options): + def _remove_invalid_options(self, context, search_options): if FLAGS.allow_admin_api and context.is_admin: # Allow all options return @@ -156,7 +156,7 @@ class Controller(object): search_opts.update(req.str_GET) context = req.environ['nova.context'] - self._check_servers_options(context, search_opts) + self._remove_invalid_options(context, search_opts) # Convert recurse_zones into a boolean search_opts['recurse_zones'] = utils.bool_from_str( -- cgit From ca152762aa73a93583be2ada237cf8bbbcc99220 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 6 Jul 2011 05:41:47 -0700 Subject: clean up OS API servers getting --- nova/api/openstack/servers.py | 94 +++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 57 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 9a3872113..ce04a1eab 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -46,34 +46,14 @@ FLAGS = flags.FLAGS class Controller(object): """ The Server API base controller class for the OpenStack API """ - # These are a list of possible query string paramters to the - # /servers query that a user should be able to do. Specify this - # in your subclasses. When admin api is off, unknown options will - # get filtered out without error. - servers_search_options = [] - def __init__(self): self.compute_api = compute.API() self.helper = helper.CreateInstanceHelper(self) - def _remove_invalid_options(self, context, search_options): - if FLAGS.allow_admin_api and context.is_admin: - # Allow all options - return - # Otherwise, strip out all unknown options - unknown_options = [opt for opt in search_options - if opt not in self.servers_search_options] - unk_opt_str = ", ".join(unknown_options) - log_msg = _("Stripping out options '%(unk_opt_str)s' from servers " - "query") % locals() - LOG.debug(log_msg) - for opt in unknown_options: - search_options.pop(opt, None) - def index(self, req): """ Returns a list of server names and ids for a given user """ try: - servers = self._servers_from_request(req, is_detail=False) + servers = self._get_servers(req, is_detail=False) except exception.Invalid as err: return exc.HTTPBadRequest(explanation=str(err)) except exception.NotFound: @@ -83,7 +63,7 @@ class Controller(object): def detail(self, req): """ Returns a list of server details for a given user """ try: - servers = self._servers_from_request(req, is_detail=True) + servers = self._get_servers(req, is_detail=True) except exception.Invalid as err: return exc.HTTPBadRequest(explanation=str(err)) except exception.NotFound as err: @@ -99,13 +79,21 @@ class Controller(object): def _action_rebuild(self, info, request, instance_id): raise NotImplementedError() - def _servers_search(self, context, req, is_detail, search_opts=None): + def _get_servers(self, req, is_detail): """Returns a list of servers, taking into account any search options specified. """ - if search_opts is None: - search_opts = {} + search_opts = {} + search_opts.update(req.str_GET) + + context = req.environ['nova.context'] + remove_invalid_options(context, search_opts, + self._get_server_search_options()) + + # Convert recurse_zones into a boolean + search_opts['recurse_zones'] = utils.bool_from_str( + search_opts.get('recurse_zones', False)) # If search by 'status', we need to convert it to 'state' # If the status is unknown, bail. @@ -145,26 +133,6 @@ class Controller(object): for inst in limited_list] return dict(servers=servers) - def _servers_from_request(self, req, is_detail): - """Returns a list of servers based on the request. - - Checks for search options and strips out options that should - not be available to non-admins. - """ - - search_opts = {} - search_opts.update(req.str_GET) - - context = req.environ['nova.context'] - self._remove_invalid_options(context, search_opts) - - # Convert recurse_zones into a boolean - search_opts['recurse_zones'] = utils.bool_from_str( - search_opts.get('recurse_zones', False)) - - return self._servers_search(context, req, is_detail, - search_opts=search_opts) - @scheduler_api.redirect_handler def show(self, req, id): """ Returns server details by server id """ @@ -569,12 +537,6 @@ class Controller(object): class ControllerV10(Controller): """v1.0 OpenStack API controller""" - # These are a list of possible query string paramters to the - # /servers query that a user should be able to do. When admin api - # is off, unknown options will get filtered out without error. - servers_search_options = ["reservation_id", "fixed_ip", - "name", "recurse_zones"] - @scheduler_api.redirect_handler def delete(self, req, id): """ Destroys a server """ @@ -636,16 +598,14 @@ class ControllerV10(Controller): """ Determine the admin password for a server on creation """ return self.helper._get_server_admin_password_old_style(server) + def _get_server_search_options(self): + """Return server search options allowed by non-admin""" + return 'reservation_id', 'fixed_ip', 'name', 'recurse_zones' + class ControllerV11(Controller): """v1.1 OpenStack API controller""" - # These are a list of possible query string paramters to the - # /servers query that a user should be able to do. When admin api - # is off, unknown options will get filtered out without error. - servers_search_options = ["reservation_id", "name", "recurse_zones", - "status", "image", "flavor", "changes-since"] - @scheduler_api.redirect_handler def delete(self, req, id): """ Destroys a server """ @@ -812,6 +772,11 @@ class ControllerV11(Controller): """ Determine the admin password for a server on creation """ return self.helper._get_server_admin_password_new_style(server) + def _get_server_search_options(self): + """Return server search options allowed by non-admin""" + return ('reservation_id', 'name', 'recurse_zones', + 'status', 'image', 'flavor', 'changes-since') + class HeadersSerializer(wsgi.ResponseHeadersSerializer): @@ -982,3 +947,18 @@ def create_resource(version='1.0'): deserializer = wsgi.RequestDeserializer(body_deserializers) return wsgi.Resource(controller, deserializer, serializer) + + +def remove_invalid_options(context, search_options, allowed_search_options): + """Remove search options that are not valid for non-admin API/context""" + if FLAGS.allow_admin_api and context.is_admin: + # Allow all options + return + # Otherwise, strip out all unknown options + unknown_options = [opt for opt in search_options + if opt not in allowed_search_options] + unk_opt_str = ", ".join(unknown_options) + log_msg = _("Removing options '%(unk_opt_str)s' from query") % locals() + LOG.debug(log_msg) + for opt in unknown_options: + search_options.pop(opt, None) -- cgit From 1c90eb34085dbb69f37e2f63dea7496afabb06b3 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 6 Jul 2011 06:20:38 -0700 Subject: clean up compute_api.get_all filter name remappings. ditch fixed_ip one-off code. fixed ec2 api call to this to compensate --- nova/api/ec2/cloud.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nova/api') diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 966c3a564..db49baffa 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -242,7 +242,7 @@ class CloudController(object): search_opts=search_opts) except exception.NotFound: instance_ref = None - if instance_ref is None: + if not instance_ref: return None # This ensures that all attributes of the instance -- cgit From 09772f5bf3140a6f4cbaace50ead8d25a874cbb0 Mon Sep 17 00:00:00 2001 From: Jake Dahn Date: Fri, 5 Aug 2011 14:37:44 -0700 Subject: If ip is deallocated from project, but attached to a fixed ip, it is now detached --- nova/api/openstack/contrib/floating_ips.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'nova/api') diff --git a/nova/api/openstack/contrib/floating_ips.py b/nova/api/openstack/contrib/floating_ips.py index 3d8049324..616388e80 100644 --- a/nova/api/openstack/contrib/floating_ips.py +++ b/nova/api/openstack/contrib/floating_ips.py @@ -97,8 +97,13 @@ class FloatingIPController(object): def delete(self, req, id): context = req.environ['nova.context'] - ip = self.network_api.get_floating_ip(context, id) + try: + if 'fixed_ip' in ip: + self.disassociate(req, id, '') + except: + pass + self.network_api.release_floating_ip(context, address=ip) return {'released': { -- cgit From ccea6c91b2314311587466d67d20f1583ddba1ee Mon Sep 17 00:00:00 2001 From: Jake Dahn Date: Fri, 5 Aug 2011 15:28:10 -0700 Subject: adding logging to exception in delete method --- nova/api/openstack/contrib/floating_ips.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/contrib/floating_ips.py b/nova/api/openstack/contrib/floating_ips.py index 616388e80..49ab88bb6 100644 --- a/nova/api/openstack/contrib/floating_ips.py +++ b/nova/api/openstack/contrib/floating_ips.py @@ -24,6 +24,9 @@ from nova.api.openstack import faults from nova.api.openstack import extensions +LOG = logging.getLogger('nova.api.openstack.contrib.floating_ips') + + def _translate_floating_ip_view(floating_ip): result = {'id': floating_ip['id'], 'ip': floating_ip['address']} @@ -98,11 +101,12 @@ class FloatingIPController(object): def delete(self, req, id): context = req.environ['nova.context'] ip = self.network_api.get_floating_ip(context, id) + try: if 'fixed_ip' in ip: self.disassociate(req, id, '') - except: - pass + except Exception, e: + LOG.exception(_("Error disassociating fixed_ip %s"), e) self.network_api.release_floating_ip(context, address=ip) -- cgit From fe7f229c8ad91b1ae9187b8c541fdefd535eed9b Mon Sep 17 00:00:00 2001 From: Jake Dahn Date: Fri, 5 Aug 2011 16:20:53 -0700 Subject: moving try/except block, and changing syntax of except statement --- nova/api/openstack/contrib/floating_ips.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/contrib/floating_ips.py b/nova/api/openstack/contrib/floating_ips.py index 49ab88bb6..996a42abe 100644 --- a/nova/api/openstack/contrib/floating_ips.py +++ b/nova/api/openstack/contrib/floating_ips.py @@ -101,12 +101,12 @@ class FloatingIPController(object): def delete(self, req, id): context = req.environ['nova.context'] ip = self.network_api.get_floating_ip(context, id) - - try: - if 'fixed_ip' in ip: + + if 'fixed_ip' in ip: + try: self.disassociate(req, id, '') - except Exception, e: - LOG.exception(_("Error disassociating fixed_ip %s"), e) + except Exception as e: + LOG.exception(_("Error disassociating fixed_ip %s"), e) self.network_api.release_floating_ip(context, address=ip) -- cgit From c600b2cf3697fc3587fe5519fda8dd4b82d67234 Mon Sep 17 00:00:00 2001 From: Jake Dahn Date: Mon, 8 Aug 2011 12:10:14 -0700 Subject: adding forgotten import for logging --- nova/api/openstack/contrib/floating_ips.py | 1 + 1 file changed, 1 insertion(+) (limited to 'nova/api') diff --git a/nova/api/openstack/contrib/floating_ips.py b/nova/api/openstack/contrib/floating_ips.py index 996a42abe..52c9c6cf9 100644 --- a/nova/api/openstack/contrib/floating_ips.py +++ b/nova/api/openstack/contrib/floating_ips.py @@ -18,6 +18,7 @@ from webob import exc from nova import exception +from nova import log as logging from nova import network from nova import rpc from nova.api.openstack import faults -- cgit