From 6249e776eb6330003e13f8bac2df2885da477164 Mon Sep 17 00:00:00 2001 From: Bharath Thiruveedula Date: Sat, 5 Sep 2015 23:45:07 +0530 Subject: [PATCH] "keypair_id" should be existent when creating a baymodel While creating the baymodel we must check existence of keypair-id, So that we can avoid keypair NotFound error while creating the bay. Change-Id: I3796503bac05a9aa1c9d155e93447a95fb567daf Closes-Bug: #1476500 --- etc/magnum/magnum.conf.sample | 40 +++++++-- magnum/api/controllers/v1/baymodel.py | 10 +++ magnum/common/clients.py | 27 ++++++ magnum/common/exception.py | 4 + magnum/opts.py | 1 + .../unit/api/controllers/v1/test_baymodel.py | 85 ++++++++++++++++--- requirements.txt | 1 + 7 files changed, 149 insertions(+), 19 deletions(-) diff --git a/etc/magnum/magnum.conf.sample b/etc/magnum/magnum.conf.sample index 6731fa4508..f406831922 100644 --- a/etc/magnum/magnum.conf.sample +++ b/etc/magnum/magnum.conf.sample @@ -35,7 +35,7 @@ # (integer value) #periodic_interval_max = 60 -# Name of this node. This can be an opaque identifier. It is not +# Name of this node. This can be an opaque identifier. It is not # necessarily a hostname, FQDN, or IP address. However, the node name # must be valid within an AMQP key, and if using ZeroMQ, a valid # hostname, FQDN, or IP address. (string value) @@ -46,7 +46,7 @@ # # Print debugging output (set logging level to DEBUG instead of -# default WARNING level). (boolean value) +# default INFO level). (boolean value) #debug = false # If set to false, will disable INFO logging level, making WARNING the @@ -88,8 +88,9 @@ # (Optional) Enables or disables syslog rfc5424 format for logging. If # enabled, prefixes the MSG part of the syslog message with APP-NAME -# (RFC5424). The format without the APP-NAME is deprecated in K, and -# will be removed in M, along with this option. (boolean value) +# (RFC5424). The format without the APP-NAME is deprecated in Kilo, +# and will be removed in Mitaka, along with this option. (boolean +# value) # This option is deprecated for removal. # Its value may be silently ignored in the future. #use_syslog_rfc_format = true @@ -115,7 +116,7 @@ #logging_exception_prefix = %(asctime)s.%(msecs)03d %(process)d ERROR %(name)s %(instance)s # List of logger=LEVEL pairs. (list value) -#default_log_levels = amqp=WARN,amqplib=WARN,boto=WARN,qpid=WARN,sqlalchemy=WARN,suds=INFO,oslo.messaging=INFO,iso8601=WARN,requests.packages.urllib3.connectionpool=WARN,urllib3.connectionpool=WARN,websocket=WARN,requests.packages.urllib3.util.retry=WARN,urllib3.util.retry=WARN,keystonemiddleware=WARN,routes.middleware=WARN,stevedore=WARN +#default_log_levels = amqp=WARN,amqplib=WARN,boto=WARN,qpid=WARN,sqlalchemy=WARN,suds=INFO,oslo.messaging=INFO,iso8601=WARN,requests.packages.urllib3.connectionpool=WARN,urllib3.connectionpool=WARN,websocket=WARN,requests.packages.urllib3.util.retry=WARN,urllib3.util.retry=WARN,keystonemiddleware=WARN,routes.middleware=WARN,stevedore=WARN,taskflow=WARN # Enables or disables publication of error events. (boolean value) #publish_errors = false @@ -174,8 +175,9 @@ # Heartbeat time-to-live. (integer value) #matchmaker_heartbeat_ttl = 600 -# Size of RPC thread pool. (integer value) -#rpc_thread_pool_size = 64 +# Size of executor thread pool. (integer value) +# Deprecated group/name - [DEFAULT]/rpc_thread_pool_size +#executor_thread_pool_size = 64 # The Drivers(s) to handle sending notifications. Possible values are # messaging, messagingv2, routing, log, test, noop (multi valued) @@ -588,6 +590,9 @@ # Verify HTTPS connections. (boolean value) #insecure = false +# The region in which the identity server can be found. (string value) +#region_name = + # Directory used to cache files related to PKI tokens. (string value) #signing_dir = @@ -772,6 +777,21 @@ #ringfile = /etc/oslo/matchmaker_ring.json +[nova_client] + +# +# From magnum +# + +# Region in Identity service catalog to use for communication with the +# OpenStack service. (string value) +#region_name = + +# Type of endpoint in Identity service catalog to use for +# communication with the OpenStack service. (string value) +#endpoint_type = publicURL + + [oslo_concurrency] # @@ -865,7 +885,8 @@ # keep backward compatible at the same time. This option provides such # compatibility - it defaults to False in Liberty and can be turned on # for early adopters with a new installations or for testing. Please -# note, that this option will be removed in M release. (boolean value) +# note, that this option will be removed in the Mitaka release. +# (boolean value) #send_single_reply = false # Qpid broker hostname. (string value) @@ -940,7 +961,8 @@ # keep backward compatible at the same time. This option provides such # compatibility - it defaults to False in Liberty and can be turned on # for early adopters with a new installations or for testing. Please -# note, that this option will be removed in M release. (boolean value) +# note, that this option will be removed in the Mitaka release. +# (boolean value) #send_single_reply = false # SSL version to use (valid only if SSL enabled). Valid values are diff --git a/magnum/api/controllers/v1/baymodel.py b/magnum/api/controllers/v1/baymodel.py index 865c7bd642..58d1b474f0 100644 --- a/magnum/api/controllers/v1/baymodel.py +++ b/magnum/api/controllers/v1/baymodel.py @@ -15,6 +15,7 @@ import datetime import glanceclient.exc +import novaclient.exceptions as nova_exc import pecan from pecan import rest import wsme @@ -282,6 +283,14 @@ class BayModelsController(rest.RestController): rpc_baymodel = api_utils.get_rpc_resource('BayModel', baymodel_ident) return BayModel.convert_with_links(rpc_baymodel) + def check_keypair_exists(self, context, keypair): + """Checks the existence of the keypair""" + cli = clients.OpenStackClients(context) + try: + cli.nova().keypairs.get(keypair) + except nova_exc.NotFound: + raise exception.KeyPairNotFound(keypair=keypair) + @policy.enforce_wsgi("baymodel", "create") @expose.expose(BayModel, body=BayModel, status_code=201) def post(self, baymodel): @@ -291,6 +300,7 @@ class BayModelsController(rest.RestController): """ baymodel_dict = baymodel.as_dict() context = pecan.request.context + self.check_keypair_exists(context, baymodel_dict['keypair_id']) baymodel_dict['project_id'] = context.project_id baymodel_dict['user_id'] = context.user_id image_data = self._get_image_data(context, baymodel_dict['image_id']) diff --git a/magnum/common/clients.py b/magnum/common/clients.py index 597a3f68c1..aa0439cf3f 100644 --- a/magnum/common/clients.py +++ b/magnum/common/clients.py @@ -15,6 +15,7 @@ from barbicanclient import client as barbicanclient from glanceclient.v2 import client as glanceclient from heatclient.v1 import client as heatclient +from novaclient.v2 import client as novaclient from oslo_config import cfg from oslo_log import log as logging @@ -81,10 +82,22 @@ barbican_client_opts = [ 'Type of endpoint in Identity service catalog to use ' 'for communication with the OpenStack service.'))] +nova_client_opts = [ + cfg.StrOpt('region_name', + default=None, + help=_('Region in Identity service catalog to use for ' + 'communication with the OpenStack service.')), + cfg.StrOpt('endpoint_type', + default='publicURL', + help=_( + 'Type of endpoint in Identity service catalog to use ' + 'for communication with the OpenStack service.'))] + cfg.CONF.register_opts(magnum_client_opts, group='magnum_client') cfg.CONF.register_opts(heat_client_opts, group='heat_client') cfg.CONF.register_opts(glance_client_opts, group='glance_client') cfg.CONF.register_opts(barbican_client_opts, group='barbican_client') +cfg.CONF.register_opts(nova_client_opts, group='nova_client') class OpenStackClients(object): @@ -96,6 +109,7 @@ class OpenStackClients(object): self._heat = None self._glance = None self._barbican = None + self._nova = None def url_for(self, **kwargs): return self.keystone().client.service_catalog.url_for(**kwargs) @@ -187,3 +201,16 @@ class OpenStackClients(object): endpoint=endpoint) return self._barbican + + @exception.wrap_keystone_exception + def nova(self): + if self._nova: + return self._nova + endpoint_type = self._get_client_option('nova', 'endpoint_type') + region_name = self._get_client_option('nova', 'region_name') + endpoint = self.url_for(service_type='compute', + endpoint_type=endpoint_type, + region_name=region_name) + self._nova = novaclient.Client(auth_token=self.auth_token) + self._nova.client.management_url = endpoint + return self._nova diff --git a/magnum/common/exception.py b/magnum/common/exception.py index 0c7f791ba5..7894feb1a9 100644 --- a/magnum/common/exception.py +++ b/magnum/common/exception.py @@ -483,3 +483,7 @@ class CertificateStorageException(MagnumException): class CertificateValidationError(Invalid): message = _("Extension '%(extension)s' not allowed") + + +class KeyPairNotFound(ResourceNotFound): + message = _("Unable to find keypair %(keypair)s.") diff --git a/magnum/opts.py b/magnum/opts.py index d45da325f5..0b2f5feabe 100644 --- a/magnum/opts.py +++ b/magnum/opts.py @@ -51,6 +51,7 @@ def list_opts(): ('heat_client', magnum.common.clients.heat_client_opts), ('glance_client', magnum.common.clients.glance_client_opts), ('barbican_client', magnum.common.clients.barbican_client_opts), + ('nova_client', magnum.common.clients.nova_client_opts), ('x509', magnum.common.x509.config.x509_opts), ('bay_heat', magnum.conductor.handlers.bay_conductor.bay_heat_opts), ('certificates', diff --git a/magnum/tests/unit/api/controllers/v1/test_baymodel.py b/magnum/tests/unit/api/controllers/v1/test_baymodel.py index 17867b6f58..51b3347a0e 100644 --- a/magnum/tests/unit/api/controllers/v1/test_baymodel.py +++ b/magnum/tests/unit/api/controllers/v1/test_baymodel.py @@ -27,6 +27,7 @@ from magnum.tests import base from magnum.tests.unit.api import base as api_base from magnum.tests.unit.api import utils as apiutils from magnum.tests.unit.objects import utils as obj_utils +from novaclient import exceptions as nova_exc class TestBayModelObject(base.TestCase): @@ -407,9 +408,13 @@ class TestPatch(api_base.FunctionalTest): class TestPost(api_base.FunctionalTest): @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data') + @mock.patch.object(api_baymodel.BayModelsController, + 'check_keypair_exists') @mock.patch('oslo_utils.timeutils.utcnow') - def test_create_baymodel(self, mock_utcnow, mock_image_data): + def test_create_baymodel(self, mock_utcnow, + mock_keypair_exists, mock_image_data): bdict = apiutils.baymodel_post_data() + mock_keypair_exists.return_value = None test_time = datetime.datetime(2000, 1, 1, 0, 0) mock_utcnow.return_value = test_time mock_image_data.return_value = {'name': 'mock_name', @@ -429,10 +434,14 @@ class TestPost(api_base.FunctionalTest): self.assertEqual(test_time, return_created_at) @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data') - def test_create_baymodel_set_project_id_and_user_id(self, mock_image_data): - + @mock.patch.object(api_baymodel.BayModelsController, + 'check_keypair_exists') + def test_create_baymodel_set_project_id_and_user_id(self, + mock_keypair_exists, + mock_image_data): with mock.patch.object(self.dbapi, 'create_baymodel', wraps=self.dbapi.create_baymodel) as cc_mock: + mock_keypair_exists.return_value = None mock_image_data.return_value = {'name': 'mock_name', 'os_distro': 'fedora-atomic'} bdict = apiutils.baymodel_post_data() @@ -444,9 +453,14 @@ class TestPost(api_base.FunctionalTest): self.context.user_id) @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data') - def test_create_baymodel_doesnt_contain_id(self, mock_image_data): + @mock.patch.object(api_baymodel.BayModelsController, + 'check_keypair_exists') + def test_create_baymodel_doesnt_contain_id(self, + mock_keypair_exists, + mock_image_data): with mock.patch.object(self.dbapi, 'create_baymodel', wraps=self.dbapi.create_baymodel) as cc_mock: + mock_keypair_exists.return_value = None mock_image_data.return_value = {'name': 'mock_name', 'os_distro': 'fedora-atomic'} bdict = apiutils.baymodel_post_data(image_id='my-image') @@ -504,10 +518,14 @@ class TestPost(api_base.FunctionalTest): self._create_baymodel_raises_app_error(apiserver_port='not an int') @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data') + @mock.patch.object(api_baymodel.BayModelsController, + 'check_keypair_exists') def test_create_baymodel_with_docker_volume_size(self, + mock_keypair_exists, mock_image_data): with mock.patch.object(self.dbapi, 'create_baymodel', wraps=self.dbapi.create_baymodel) as cc_mock: + mock_keypair_exists.return_value = None mock_image_data.return_value = {'name': 'mock_name', 'os_distro': 'fedora-atomic'} bdict = apiutils.baymodel_post_data(docker_volume_size=99) @@ -518,7 +536,12 @@ class TestPost(api_base.FunctionalTest): self.assertNotIn('id', cc_mock.call_args[0][0]) @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data') - def test_create_baymodel_generate_uuid(self, mock_image_data): + @mock.patch.object(api_baymodel.BayModelsController, + 'check_keypair_exists') + def test_create_baymodel_generate_uuid(self, + mock_keypair_exists, + mock_image_data): + mock_keypair_exists.return_value = None mock_image_data.return_value = {'name': 'mock_name', 'os_distro': 'fedora-atomic'} bdict = apiutils.baymodel_post_data() @@ -529,7 +552,12 @@ class TestPost(api_base.FunctionalTest): self.assertTrue(utils.is_uuid_like(response.json['uuid'])) @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data') - def test_create_baymodel_with_no_os_distro_image(self, mock_image_data): + @mock.patch.object(api_baymodel.BayModelsController, + 'check_keypair_exists') + def test_create_baymodel_with_no_os_distro_image(self, + mock_keypair_exists, + mock_image_data): + mock_keypair_exists.return_value = None mock_image_data.return_value = {'name': 'mock_name'} bdict = apiutils.baymodel_post_data() del bdict['uuid'] @@ -537,7 +565,12 @@ class TestPost(api_base.FunctionalTest): self.assertEqual(404, response.status_int) @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data') - def test_create_baymodel_with_os_distro_image(self, mock_image_data): + @mock.patch.object(api_baymodel.BayModelsController, + 'check_keypair_exists') + def test_create_baymodel_with_os_distro_image(self, + mock_keypair_exists, + mock_image_data): + mock_keypair_exists.return_value = None mock_image_data.return_value = {'name': 'mock_name', 'os_distro': 'fedora-atomic'} bdict = apiutils.baymodel_post_data() @@ -546,7 +579,12 @@ class TestPost(api_base.FunctionalTest): self.assertEqual(201, response.status_int) @mock.patch.object(openstack_client, 'glance') - def test_create_baymodel_with_image_name(self, mock_glance_client): + @mock.patch.object(api_baymodel.BayModelsController, + 'check_keypair_exists') + def test_create_baymodel_with_image_name(self, + mock_keypair_exists, + mock_glance_client): + mock_keypair_exists.return_value = None mock_images = [{'name': 'mock_name', 'os_distro': 'fedora-atomic'}] mock_glance = mock.MagicMock() @@ -558,9 +596,13 @@ class TestPost(api_base.FunctionalTest): self.assertEqual(201, response.status_int) @mock.patch.object(openstack_client, 'glance') + @mock.patch.object(api_baymodel.BayModelsController, + 'check_keypair_exists') def test_create_baymodel_with_no_exist_image_name(self, + mock_keypair_exists, mock_glance_client): mock_images = [] + mock_keypair_exists.return_value = None mock_glance = mock.MagicMock() mock_glance.images.list.return_value = mock_images mock_glance_client.return_value = mock_glance @@ -570,7 +612,12 @@ class TestPost(api_base.FunctionalTest): self.assertEqual(404, response.status_int) @mock.patch.object(openstack_client, 'glance') - def test_create_baymodel_with_multi_image_name(self, mock_glance_client): + @mock.patch.object(api_baymodel.BayModelsController, + 'check_keypair_exists') + def test_create_baymodel_with_multi_image_name(self, + mock_keypair_exists, + mock_glance_client): + mock_keypair_exists.return_value = None mock_images = [{'name': 'mock_name', 'os_distro': 'fedora-atomic'}, {'name': 'mock_name', @@ -596,7 +643,11 @@ class TestPost(api_base.FunctionalTest): self.assertEqual(400, response.status_int) @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data') - def test_create_baymodel_with_dns(self, mock_image_data): + @mock.patch.object(api_baymodel.BayModelsController, + 'check_keypair_exists') + def test_create_baymodel_with_dns(self, mock_keypair_exists, + mock_image_data): + mock_keypair_exists.return_value = None mock_image_data.return_value = {'name': 'mock_name', 'os_distro': 'fedora-atomic'} bdict = apiutils.baymodel_post_data() @@ -605,6 +656,20 @@ class TestPost(api_base.FunctionalTest): self.assertEqual(bdict['dns_nameserver'], response.json['dns_nameserver']) + @mock.patch.object(openstack_client, 'nova') + @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data') + def test_create_baymodel_with_no_exist_keypair(self, + mock_image_data, + mock_nova_client): + mock_nova = mock.MagicMock() + mock_nova.keypairs.get.side_effect = nova_exc.NotFound("Test") + mock_nova_client.return_value = mock_nova + mock_image_data.return_value = {'name': 'mock_name', + 'os_distro': 'fedora-atomic'} + bdict = apiutils.baymodel_post_data() + response = self.post_json('/baymodels', bdict, expect_errors=True) + self.assertEqual(404, response.status_int) + class TestDelete(api_base.FunctionalTest): diff --git a/requirements.txt b/requirements.txt index 2324bc1d0c..4935651fee 100644 --- a/requirements.txt +++ b/requirements.txt @@ -39,6 +39,7 @@ pecan>=1.0.0 python-barbicanclient>=3.3.0 python-glanceclient>=0.18.0 python-heatclient>=0.3.0 +python-novaclient>=2.28.1 python-keystoneclient>=1.6.0 requests>=2.5.2 six>=1.9.0