Remove unneeded call to nova API, defer other API calls

We get an instance ID directly from nova, which calls our API,
consequently we don't need to call back to nova to double check
if the instance ID realy exists.

Additionally, defer calling keystone and glance APIs to the moment
that the retrieved objects are realy needed.

Change-Id: I64a20c88229490690798aaf75ca0d96d98032467
This commit is contained in:
Grzegorz Grasza 2019-01-03 13:20:22 +01:00
parent 5633d348e3
commit db868ea7c1
3 changed files with 31 additions and 114 deletions

View File

@ -24,7 +24,6 @@ from novajoin import exception
from novajoin.glance import get_default_image_service
from novajoin.ipa import IPAClient
from novajoin import keystone_client
from novajoin.nova import get_instance
from novajoin import policy
from novajoin import util
@ -119,48 +118,36 @@ class JoinController(Controller):
except exception.PolicyNotAuthorized:
raise base.Fault(webob.exc.HTTPForbidden())
instance_id = body.get('instance-id')
image_id = body.get('image-id')
project_id = body.get('project-id')
hostname_short = body.get('hostname')
metadata = body.get('metadata', {})
if not instance_id:
LOG.error('No instance-id in request')
raise base.Fault(webob.exc.HTTPBadRequest())
if not hostname_short:
LOG.error('No hostname in request')
raise base.Fault(webob.exc.HTTPBadRequest())
if not image_id:
LOG.error('No image-id in request')
raise base.Fault(webob.exc.HTTPBadRequest())
metadata = body.get('metadata', {})
enroll = metadata.get('ipa_enroll', '').lower() == 'true'
if not project_id:
LOG.error('No project-id in request')
raise base.Fault(webob.exc.HTTPBadRequest())
enroll = metadata.get('ipa_enroll', '')
if enroll.lower() != 'true':
LOG.debug('IPA enrollment not requested in instance creation')
image_service = get_default_image_service()
image_metadata = {}
try:
image = image_service.show(context, image_id)
except (exception.ImageNotFound, exception.ImageNotAuthorized) as e:
msg = 'Failed to get image: %s' % e
LOG.error(msg)
raise base.Fault(webob.exc.HTTPBadRequest(explanation=msg))
else:
image_metadata = image.get('properties', {})
if not enroll:
LOG.debug('IPA enrollment not requested in instance creation')
# Check the image metadata to see if enrollment was requested
# Check the image metadata to see if enrollment was requested
if enroll.lower() != 'true':
enroll = image_metadata.get('ipa_enroll', '')
if enroll.lower() != 'true':
image_id = body.get('image-id')
if not image_id:
LOG.error('No image-id in request')
raise base.Fault(webob.exc.HTTPBadRequest())
image_service = get_default_image_service()
try:
image = image_service.show(context, image_id)
except (exception.ImageNotFound,
exception.ImageNotAuthorized) as e:
msg = 'Failed to get image: %s' % e
LOG.error(msg)
raise base.Fault(webob.exc.HTTPBadRequest(explanation=msg))
else:
image_metadata = image.get('properties', {})
enroll = image_metadata.get('ipa_enroll', '').lower() == 'true'
if not enroll:
LOG.debug('IPA enrollment not requested in image')
return {}
else:
@ -168,22 +155,15 @@ class JoinController(Controller):
else:
LOG.debug('IPA enrollment requested as property')
# Ensure this instance exists in nova and retrieve the
# name of the user that requested it.
instance = get_instance(instance_id)
if instance is None:
msg = 'No such instance-id, %s' % instance_id
LOG.error(msg)
raise base.Fault(webob.exc.HTTPBadRequest(explanation=msg))
# TODO(rcritten): Eventually may check the user for permission
# as well using:
# user = keystone_client.get_user_name(instance.user_id)
hostclass = metadata.get('ipa_hostclass')
if hostclass:
# Only look up project_name when hostclass is requested to
# save a round-trip with Keystone.
project_id = body.get('project-id')
if not project_id:
LOG.error('No project-id in request')
raise base.Fault(webob.exc.HTTPBadRequest())
project_name = keystone_client.get_project_name(project_id)
if project_name is None:
msg = 'No such project-id, %s' % project_id

View File

@ -70,27 +70,8 @@ class JoinTest(test.TestCase):
else:
assert(False)
def test_no_instanceid(self):
body = {"metadata": {"ipa_enroll": "True"},
"image-id": fake.IMAGE_ID,
"project-id": fake.PROJECT_ID,
"hostname": "test"}
req = fakes.HTTPRequest.blank('/v1/')
req.method = 'POST'
req.content_type = "application/json"
# Not using assertRaises because the exception is wrapped as
# a Fault
try:
self.join_controller.create(req, body)
except Fault as fault:
assert fault.status_int == 400
else:
assert(False)
def test_no_imageid(self):
body = {"metadata": {"ipa_enroll": "True"},
"instance-id": fake.INSTANCE_ID,
body = {"metadata": {"ipa_enroll": "False"},
"project-id": fake.PROJECT_ID,
"hostname": "test"}
req = fakes.HTTPRequest.blank('/v1/')
@ -108,7 +89,6 @@ class JoinTest(test.TestCase):
def test_no_hostname(self):
body = {"metadata": {"ipa_enroll": "True"},
"instance-id": fake.INSTANCE_ID,
"project-id": fake.PROJECT_ID,
"image-id": fake.IMAGE_ID}
req = fakes.HTTPRequest.blank('/v1/')
@ -125,8 +105,7 @@ class JoinTest(test.TestCase):
assert(False)
def test_no_project_id(self):
body = {"metadata": {"ipa_enroll": "True"},
"instance-id": fake.INSTANCE_ID,
body = {"metadata": {"ipa_enroll": "True", "ipa_hostclass": "foo"},
"image-id": fake.IMAGE_ID,
"hostname": "test"}
req = fakes.HTTPRequest.blank('/v1/')
@ -146,7 +125,6 @@ class JoinTest(test.TestCase):
def test_request_no_enrollment(self, mock_get_image):
mock_get_image.return_value = FakeImageService()
body = {"metadata": {"ipa_enroll": "False"},
"instance-id": fake.INSTANCE_ID,
"project-id": fake.PROJECT_ID,
"image-id": fake.IMAGE_ID,
"hostname": "test"}
@ -162,7 +140,6 @@ class JoinTest(test.TestCase):
def test_request_invalid_image(self, mock_get_image):
mock_get_image.side_effect = Fault(webob.exc.HTTPBadRequest())
body = {"metadata": {"ipa_enroll": "False"},
"instance-id": fake.INSTANCE_ID,
"project-id": fake.PROJECT_ID,
"image-id": "invalid",
"hostname": "test"}
@ -181,13 +158,11 @@ class JoinTest(test.TestCase):
assert(False)
@mock.patch('novajoin.ipa.SafeConfigParser')
@mock.patch('novajoin.join.get_instance')
@mock.patch('novajoin.join.get_default_image_service')
@mock.patch('novajoin.util.get_domain')
def test_valid_request(self, mock_get_domain, mock_get_image,
mock_get_instance, mock_conf_parser):
mock_conf_parser):
mock_get_image.return_value = FakeImageService()
mock_get_instance.return_value = fake.fake_instance
mock_get_domain.return_value = "test"
mock_conf_parser_instance = mock.MagicMock()
@ -196,7 +171,6 @@ class JoinTest(test.TestCase):
mock_conf_parser.return_value = mock_conf_parser_instance
body = {"metadata": {"ipa_enroll": "True"},
"instance-id": fake.INSTANCE_ID,
"project-id": fake.PROJECT_ID,
"image-id": fake.IMAGE_ID,
"hostname": "test"}
@ -221,15 +195,12 @@ class JoinTest(test.TestCase):
@mock.patch('novajoin.ipa.SafeConfigParser')
@mock.patch('novajoin.keystone_client.get_project_name')
@mock.patch('novajoin.join.get_instance')
@mock.patch('novajoin.join.get_default_image_service')
@mock.patch('novajoin.util.get_domain')
def test_valid_hostclass_request(self, mock_get_domain, mock_get_image,
mock_get_instance,
mock_get_project_name,
mock_conf_parser):
mock_get_image.return_value = FakeImageService()
mock_get_instance.return_value = fake.fake_instance
mock_get_domain.return_value = "test"
mock_get_project_name.return_value = "test"
@ -239,7 +210,6 @@ class JoinTest(test.TestCase):
mock_conf_parser.return_value = mock_conf_parser_instance
body = {"metadata": {"ipa_enroll": "True"},
"instance-id": fake.INSTANCE_ID,
"project-id": fake.PROJECT_ID,
"image-id": fake.IMAGE_ID,
"hostname": "test"}
@ -262,45 +232,16 @@ class JoinTest(test.TestCase):
# because in all likelihood the keytab cannot be read (and
# probably doesn't exist. This can be ignored.
@mock.patch('novajoin.join.get_instance')
@mock.patch('novajoin.join.get_default_image_service')
def test_invalid_instance_id(self, mock_get_image, mock_get_instance):
"""Mock the instance to not exist so there is nothing to enroll."""
mock_get_image.return_value = FakeImageService()
mock_get_instance.return_value = None
body = {"metadata": {"ipa_enroll": "True"},
"instance-id": "invalid",
"project-id": fake.PROJECT_ID,
"image-id": fake.IMAGE_ID,
"hostname": "test"}
req = fakes.HTTPRequest.blank('/v1')
req.method = 'POST'
req.content_type = "application/json"
req.body = jsonutils.dump_as_bytes(body)
# Not using assertRaises because the exception is wrapped as
# a Fault
try:
self.join_controller.create(req, body)
except Fault as fault:
assert fault.status_int == 400
else:
assert(False)
@mock.patch('novajoin.join.get_instance')
@mock.patch('novajoin.join.get_default_image_service')
@mock.patch('novajoin.keystone_client.get_project_name')
@mock.patch('novajoin.util.get_domain')
def test_invalid_project_id(self, mock_get_domain, mock_get_project_name,
mock_get_image, mock_get_instance):
mock_get_image):
mock_get_image.return_value = FakeImageService()
mock_get_instance.return_value = None
mock_get_project_name.return_value = None
mock_get_domain.return_value = "test"
body = {"metadata": {"ipa_enroll": "True", "ipa_hostclass": "foo"},
"instance-id": fake.INSTANCE_ID,
"project-id": "invalid",
"image-id": fake.IMAGE_ID,
"hostname": "test"}

View File

@ -14,8 +14,4 @@
PROJECT_ID = '89afd400-b646-4bbc-b12b-c0a4d63e5bd3'
USER_ID = 'c853ca26-e8ea-4797-8a52-ee124a013d0e'
INSTANCE_ID = 'e4274dc8-325a-409b-92fd-cfdfdd65ae8b'
IMAGE_ID = 'b8c88e01-c820-40f6-b026-00926706e374'
# In reality this should be a Server object.
fake_instance = {'instance_name': 'test', 'id': INSTANCE_ID}