From da476a99c62b3a2ab23eb036d528c531115bdb9e Mon Sep 17 00:00:00 2001 From: Zhenguo Niu Date: Thu, 10 Aug 2017 19:45:37 +0800 Subject: [PATCH] Make disabled flavors not available for common users The disabled field is intended to not allow new servers created from that flavor, so this makes it happen. Partially Implements: bp new-flavor Change-Id: I7a3509cef6f5dea5c7d6eac63fbb179a9850daae --- mogan/api/controllers/v1/servers.py | 2 ++ mogan/common/exception.py | 4 +++ mogan/db/sqlalchemy/api.py | 2 ++ mogan/tests/functional/api/v1/test_flavors.py | 8 ++++++ mogan/tests/unit/api/v1/test_server.py | 26 ++++++++++++++----- 5 files changed, 36 insertions(+), 6 deletions(-) diff --git a/mogan/api/controllers/v1/servers.py b/mogan/api/controllers/v1/servers.py index 0e92c820..66bffed7 100644 --- a/mogan/api/controllers/v1/servers.py +++ b/mogan/api/controllers/v1/servers.py @@ -697,6 +697,8 @@ class ServerController(ServerControllerBase): injected_files.append((item['path'], item['contents'])) flavor = objects.Flavor.get(pecan.request.context, flavor_uuid) + if flavor.disabled: + raise exception.FlavorDisabled(flavor_id=flavor.uuid) servers = pecan.request.engine_api.create( pecan.request.context, diff --git a/mogan/common/exception.py b/mogan/common/exception.py index ea5e4b31..8845d7c0 100644 --- a/mogan/common/exception.py +++ b/mogan/common/exception.py @@ -156,6 +156,10 @@ class FlavorNotFound(NotFound): _msg_fmt = _("Flavor %(flavor_id)s could not be found.") +class FlavorDisabled(Invalid): + _msg_fmt = _("Flavor %(flavor_id)s is disabled.") + + class ServerAlreadyExists(Conflict): _msg_fmt = _("Server with name %(name)s already exists.") diff --git a/mogan/db/sqlalchemy/api.py b/mogan/db/sqlalchemy/api.py index 4613f445..26a620bb 100644 --- a/mogan/db/sqlalchemy/api.py +++ b/mogan/db/sqlalchemy/api.py @@ -155,6 +155,7 @@ class Connection(api.Connection): uuid=flavor_uuid) if not context.is_admin: + query = query.filter_by(disabled=False) the_filter = [models.Flavors.is_public == true()] the_filter.extend([ models.Flavors.projects.has(project_id=context.project_id) @@ -184,6 +185,7 @@ class Connection(api.Connection): def flavor_get_all(self, context): query = model_query(context, models.Flavors) if not context.is_admin: + query = query.filter_by(disabled=False) the_filter = [models.Flavors.is_public == true()] the_filter.extend([ models.Flavors.projects.has(project_id=context.project_id) diff --git a/mogan/tests/functional/api/v1/test_flavors.py b/mogan/tests/functional/api/v1/test_flavors.py index 89dd94b3..a4c2f34e 100644 --- a/mogan/tests/functional/api/v1/test_flavors.py +++ b/mogan/tests/functional/api/v1/test_flavors.py @@ -57,8 +57,16 @@ class TestFlavor(v1_test.APITestV1): def test_flavor_get_all(self): self._prepare_flavors() + self.patch_json('/flavors/' + self.FLAVOR_UUIDS[0], + [{'path': '/disabled', 'value': 'true', + 'op': 'replace'}], + headers=self.headers, status=200) + # admin resp = self.get_json('/flavors', headers=self.headers) self.assertEqual(4, len(resp['flavors'])) + # non admin + resp = self.get_json('/flavors') + self.assertEqual(3, len(resp['flavors'])) def test_flavor_get_one(self): self._prepare_flavors() diff --git a/mogan/tests/unit/api/v1/test_server.py b/mogan/tests/unit/api/v1/test_server.py index e51e6728..234271a1 100644 --- a/mogan/tests/unit/api/v1/test_server.py +++ b/mogan/tests/unit/api/v1/test_server.py @@ -59,7 +59,9 @@ class TestServerAuthorization(v1_test.APITestV1): @mock.patch('mogan.engine.api.API.create') @mock.patch('mogan.objects.Flavor.get') def test_server_post(self, mock_get, mock_engine_create): - mock_get.side_effect = None + mock_flavor = mock.MagicMock() + mock_flavor.disabled = False + mock_get.return_value = mock_flavor mock_engine_create.side_effect = None mock_engine_create.return_value = [self.server1] body = gen_post_body() @@ -69,11 +71,20 @@ class TestServerAuthorization(v1_test.APITestV1): headers = self.gen_headers(self.context) self.post_json('/servers', body, headers=headers, status=201) + @mock.patch('mogan.objects.Flavor.get') + def test_server_post_with_disabled_flavor(self, mock_get): + mock_flavor = mock.MagicMock() + mock_flavor.disabled = True + mock_get.return_value = mock_flavor + body = gen_post_body() + self.post_json('/servers', body, status=204, expect_errors=True) + @mock.patch('mogan.engine.api.API.create') @mock.patch('mogan.objects.Flavor.get') def test_server_post_with_port_ids(self, mock_get, mock_engine_create): - flavor = mock.MagicMock() - mock_get.return_value = flavor + mock_flavor = mock.MagicMock() + mock_flavor.disabled = False + mock_get.return_value = mock_flavor mock_engine_create.side_effect = None mock_engine_create.return_value = [self.server1] fake_networks = [ @@ -95,8 +106,9 @@ class TestServerAuthorization(v1_test.APITestV1): @mock.patch('mogan.objects.Flavor.get') def test_server_post_with_port_ids_and_networks(self, mock_get, mock_engine_create): - flavor = mock.MagicMock() - mock_get.return_value = flavor + mock_flavor = mock.MagicMock() + mock_flavor.disabled = False + mock_get.return_value = mock_flavor mock_engine_create.side_effect = None mock_engine_create.return_value = [self.server1] fake_networks = [ @@ -161,7 +173,9 @@ class TestServerAuthorization(v1_test.APITestV1): @mock.patch('mogan.objects.Flavor.get') def test_server_post_with_port_limit_exceeded(self, mock_get, mock_engine_create): - mock_get.side_effect = None + mock_flavor = mock.Mock() + mock_flavor.disabled = False + mock_get.return_value = mock_flavor mock_engine_create.side_effect = exception.PortLimitExceeded() body = gen_post_body() self.context.roles = "no-admin"