From f7aaf1fa04331522aee2158e372940df92f45cb0 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Thu, 27 Jun 2013 21:00:05 +0000 Subject: [PATCH] Make flavors is_public option actually work When you create a flavor, you can set an is_public flag to be True or False. It is True by default. When False, the intention is that the flavor is only accessible by an admin, unless you use the flavor_access API extension to grant access to specific tenants. Unfortunately, the only place in the code where this was being enforced was when listing flavors through the API. It would filter out the non-public ones for a non-admin. Otherwise, the flavor was accessible. You could get the details, and you could boot an instance with it, if you figured out a valid flavor ID. This patch adds enforcement down in the db layer. It also fixes one place in the API where the context wasn't passed down to enable the enforcement to happen. Fix bug 1194093. master -> grizzly (cherry picked from commit b65d506a5f9d9b2b20777a9aceb44a8ffed6a5de) Conflicts: nova/api/openstack/compute/contrib/flavor_access.py nova/api/openstack/compute/contrib/flavormanage.py nova/api/openstack/compute/flavors.py nova/compute/api.py nova/db/sqlalchemy/api.py nova/tests/api/openstack/compute/contrib/test_flavor_access.py nova/tests/api/openstack/compute/contrib/test_flavor_disabled.py nova/tests/api/openstack/compute/contrib/test_flavor_manage.py nova/tests/api/openstack/compute/contrib/test_flavor_rxtx.py nova/tests/api/openstack/compute/contrib/test_flavor_swap.py nova/tests/api/openstack/compute/contrib/test_flavorextradata.py nova/tests/api/openstack/compute/test_flavors.py nova/tests/db/test_db_api.py grizzly -> folsom (cherry picked from commit 6df1b7a2a1413a98bffc8b8e0b947f3c90e3bbf5) Conflicts: nova/db/sqlalchemy/api.py nova/tests/api/openstack/compute/test_flavors.py Change-Id: I5b37fa0bb19683fe1642fd81222547d4a317054e --- .../api/openstack/compute/contrib/flavor_access.py | 3 ++- nova/api/openstack/compute/contrib/flavormanage.py | 2 +- nova/api/openstack/compute/flavors.py | 4 +++- nova/compute/api.py | 2 +- nova/compute/instance_types.py | 2 +- nova/db/api.py | 4 ++-- nova/db/sqlalchemy/api.py | 26 +++++++++++++++------- .../compute/contrib/test_flavor_access.py | 2 +- .../compute/contrib/test_flavor_disabled.py | 2 +- .../compute/contrib/test_flavor_manage.py | 3 ++- .../openstack/compute/contrib/test_flavor_rxtx.py | 2 +- .../openstack/compute/contrib/test_flavor_swap.py | 2 +- .../compute/contrib/test_flavorextradata.py | 2 +- nova/tests/api/openstack/compute/test_flavors.py | 4 ++-- 14 files changed, 37 insertions(+), 23 deletions(-) diff --git a/nova/api/openstack/compute/contrib/flavor_access.py b/nova/api/openstack/compute/contrib/flavor_access.py index 9991408..26cd77f 100644 --- a/nova/api/openstack/compute/contrib/flavor_access.py +++ b/nova/api/openstack/compute/contrib/flavor_access.py @@ -99,7 +99,8 @@ class FlavorAccessController(object): authorize(context) try: - flavor = instance_types.get_instance_type_by_flavor_id(flavor_id) + flavor = instance_types.get_instance_type_by_flavor_id(flavor_id, + ctxt=context) except exception.FlavorNotFound: explanation = _("Flavor not found.") raise webob.exc.HTTPNotFound(explanation=explanation) diff --git a/nova/api/openstack/compute/contrib/flavormanage.py b/nova/api/openstack/compute/contrib/flavormanage.py index e7731cc..79551b1 100644 --- a/nova/api/openstack/compute/contrib/flavormanage.py +++ b/nova/api/openstack/compute/contrib/flavormanage.py @@ -43,7 +43,7 @@ class FlavorManageController(wsgi.Controller): try: flavor = instance_types.get_instance_type_by_flavor_id( - id, read_deleted="no") + id, ctxt=context, read_deleted="no") except exception.NotFound, e: raise webob.exc.HTTPNotFound(explanation=e.format_message()) diff --git a/nova/api/openstack/compute/flavors.py b/nova/api/openstack/compute/flavors.py index 8aa57a2..d51b48a 100644 --- a/nova/api/openstack/compute/flavors.py +++ b/nova/api/openstack/compute/flavors.py @@ -84,7 +84,9 @@ class Controller(wsgi.Controller): def show(self, req, id): """Return data about the given flavor id.""" try: - flavor = instance_types.get_instance_type_by_flavor_id(id) + context = req.environ['nova.context'] + flavor = instance_types.get_instance_type_by_flavor_id(id, + ctxt=context) req.cache_db_flavor(flavor) except exception.NotFound: raise webob.exc.HTTPNotFound() diff --git a/nova/compute/api.py b/nova/compute/api.py index 5319d04..ca78830 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1080,7 +1080,7 @@ class API(base.Base): #NOTE(bcwaldon): this doesn't really belong in this class def get_instance_type(self, context, instance_type_id): """Get an instance type by instance type id.""" - return instance_types.get_instance_type(instance_type_id) + return instance_types.get_instance_type(instance_type_id, ctxt=context) def get(self, context, instance_id): """Get a single instance with the given instance_id.""" diff --git a/nova/compute/instance_types.py b/nova/compute/instance_types.py index 6869672..5be97c1 100644 --- a/nova/compute/instance_types.py +++ b/nova/compute/instance_types.py @@ -163,7 +163,7 @@ def get_instance_type_by_flavor_id(flavorid, ctxt=None, read_deleted="yes"): if ctxt is None: ctxt = context.get_admin_context(read_deleted=read_deleted) - return db.instance_type_get_by_flavor_id(ctxt, flavorid) + return db.instance_type_get_by_flavor_id(ctxt, flavorid, read_deleted) def get_instance_type_access_by_flavor_id(flavorid, ctxt=None): diff --git a/nova/db/api.py b/nova/db/api.py index 9f2ff73..40db686 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -1460,9 +1460,9 @@ def instance_type_get_by_name(context, name): return IMPL.instance_type_get_by_name(context, name) -def instance_type_get_by_flavor_id(context, id): +def instance_type_get_by_flavor_id(context, id, read_deleted=None): """Get instance type by flavor id.""" - return IMPL.instance_type_get_by_flavor_id(context, id) + return IMPL.instance_type_get_by_flavor_id(context, id, read_deleted) def instance_type_destroy(context, name): diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 7fcc4f8..ea32168 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -3910,7 +3910,7 @@ def instance_type_create(context, values): pass try: instance_type_get_by_flavor_id(context, values['flavorid'], - session) + read_deleted='no', session=session) raise exception.InstanceTypeExists(name=values['name']) except exception.FlavorNotFound: pass @@ -3952,9 +3952,16 @@ def _dict_with_extra_specs(inst_type_query): def _instance_type_get_query(context, session=None, read_deleted=None): - return model_query(context, models.InstanceTypes, session=session, + query = model_query(context, models.InstanceTypes, session=session, read_deleted=read_deleted).\ - options(joinedload('extra_specs')) + options(joinedload('extra_specs')) + if not context.is_admin: + the_filter = [models.InstanceTypes.is_public == True] + the_filter.extend([ + models.InstanceTypes.projects.any(project_id=context.project_id) + ]) + query = query.filter(or_(*the_filter)) + return query @require_context @@ -4029,9 +4036,11 @@ def instance_type_get_by_name(context, name, session=None): @require_context -def instance_type_get_by_flavor_id(context, flavor_id, session=None): +def instance_type_get_by_flavor_id(context, flavor_id, read_deleted, + session=None): """Returns a dict describing specific flavor_id""" - result = _instance_type_get_query(context, session=session).\ + result = _instance_type_get_query(context, read_deleted=read_deleted, + session=session).\ filter_by(flavorid=flavor_id).\ first() @@ -4083,7 +4092,7 @@ def instance_type_access_add(context, flavor_id, project_id): session = get_session() with session.begin(): instance_type_ref = instance_type_get_by_flavor_id(context, flavor_id, - session=session) + read_deleted='no', session=session) instance_type_id = instance_type_ref['id'] access_ref = _instance_type_access_query(context, session=session).\ filter_by(instance_type_id=instance_type_id).\ @@ -4111,7 +4120,7 @@ def instance_type_access_remove(context, flavor_id, project_id): session = get_session() with session.begin(): instance_type_ref = instance_type_get_by_flavor_id(context, flavor_id, - session=session) + read_deleted='no', session=session) instance_type_id = instance_type_ref['id'] access_ref = _instance_type_access_query(context, session=session).\ filter_by(instance_type_id=instance_type_id).\ @@ -4447,7 +4456,8 @@ def instance_type_extra_specs_update_or_create(context, flavor_id, specs): session = get_session() spec_ref = None - instance_type = instance_type_get_by_flavor_id(context, flavor_id) + instance_type = instance_type_get_by_flavor_id(context, flavor_id, + read_deleted='no') for key, value in specs.iteritems(): try: spec_ref = instance_type_extra_specs_get_item( diff --git a/nova/tests/api/openstack/compute/contrib/test_flavor_access.py b/nova/tests/api/openstack/compute/contrib/test_flavor_access.py index 0bf1f1b..075810b 100644 --- a/nova/tests/api/openstack/compute/contrib/test_flavor_access.py +++ b/nova/tests/api/openstack/compute/contrib/test_flavor_access.py @@ -68,7 +68,7 @@ def fake_get_instance_type_access_by_flavor_id(flavorid): return res -def fake_get_instance_type_by_flavor_id(flavorid): +def fake_get_instance_type_by_flavor_id(flavorid, ctxt=None): return INSTANCE_TYPES[flavorid] diff --git a/nova/tests/api/openstack/compute/contrib/test_flavor_disabled.py b/nova/tests/api/openstack/compute/contrib/test_flavor_disabled.py index 1225b56..933178a 100644 --- a/nova/tests/api/openstack/compute/contrib/test_flavor_disabled.py +++ b/nova/tests/api/openstack/compute/contrib/test_flavor_disabled.py @@ -44,7 +44,7 @@ FAKE_FLAVORS = { } -def fake_instance_type_get_by_flavor_id(flavorid): +def fake_instance_type_get_by_flavor_id(flavorid, ctxt=None): return FAKE_FLAVORS['flavor %s' % flavorid] diff --git a/nova/tests/api/openstack/compute/contrib/test_flavor_manage.py b/nova/tests/api/openstack/compute/contrib/test_flavor_manage.py index 70fd5e4..7174ed2 100644 --- a/nova/tests/api/openstack/compute/contrib/test_flavor_manage.py +++ b/nova/tests/api/openstack/compute/contrib/test_flavor_manage.py @@ -25,7 +25,8 @@ from nova import test from nova.tests.api.openstack import fakes -def fake_get_instance_type_by_flavor_id(flavorid, read_deleted='yes'): +def fake_get_instance_type_by_flavor_id(flavorid, ctxt=None, + read_deleted='yes'): if flavorid == 'failtest': raise exception.NotFound("Not found sucka!") elif not str(flavorid) == '1234': diff --git a/nova/tests/api/openstack/compute/contrib/test_flavor_rxtx.py b/nova/tests/api/openstack/compute/contrib/test_flavor_rxtx.py index 52163c7..afa2259 100644 --- a/nova/tests/api/openstack/compute/contrib/test_flavor_rxtx.py +++ b/nova/tests/api/openstack/compute/contrib/test_flavor_rxtx.py @@ -43,7 +43,7 @@ FAKE_FLAVORS = { } -def fake_instance_type_get_by_flavor_id(flavorid): +def fake_instance_type_get_by_flavor_id(flavorid, ctxt=None): return FAKE_FLAVORS['flavor %s' % flavorid] diff --git a/nova/tests/api/openstack/compute/contrib/test_flavor_swap.py b/nova/tests/api/openstack/compute/contrib/test_flavor_swap.py index 75e9cd7..3fd1ae9 100644 --- a/nova/tests/api/openstack/compute/contrib/test_flavor_swap.py +++ b/nova/tests/api/openstack/compute/contrib/test_flavor_swap.py @@ -43,7 +43,7 @@ FAKE_FLAVORS = { } -def fake_instance_type_get_by_flavor_id(flavorid): +def fake_instance_type_get_by_flavor_id(flavorid, ctxt=None): return FAKE_FLAVORS['flavor %s' % flavorid] diff --git a/nova/tests/api/openstack/compute/contrib/test_flavorextradata.py b/nova/tests/api/openstack/compute/contrib/test_flavorextradata.py index 8f5301a..9654605 100644 --- a/nova/tests/api/openstack/compute/contrib/test_flavorextradata.py +++ b/nova/tests/api/openstack/compute/contrib/test_flavorextradata.py @@ -23,7 +23,7 @@ from nova import test from nova.tests.api.openstack import fakes -def fake_get_instance_type_by_flavor_id(flavorid): +def fake_get_instance_type_by_flavor_id(flavorid, ctxt=None): return { 'id': flavorid, 'flavorid': str(flavorid), diff --git a/nova/tests/api/openstack/compute/test_flavors.py b/nova/tests/api/openstack/compute/test_flavors.py index 77d40df..cfa3429 100644 --- a/nova/tests/api/openstack/compute/test_flavors.py +++ b/nova/tests/api/openstack/compute/test_flavors.py @@ -54,7 +54,7 @@ FAKE_FLAVORS = { } -def fake_instance_type_get_by_flavor_id(flavorid): +def fake_instance_type_get_by_flavor_id(flavorid, ctxt=None): return FAKE_FLAVORS['flavor %s' % flavorid] @@ -80,7 +80,7 @@ def empty_instance_type_get_all(inactive=False, filters=None): return {} -def return_instance_type_not_found(flavor_id): +def return_instance_type_not_found(flavor_id, ctxt=None): raise exception.InstanceTypeNotFound(flavor_id=flavor_id) -- 1.8.1.5