x
/
novajoin
Code Issues Proposed changes

Merge "Remove unneeded call to nova API, defer other API calls"

This commit is contained in:
Zuul 2019-01-31 19:17:43 +00:00 committed by Gerrit Code Review
parent fe512714e2 db868ea7c1
commit b0d81f8c86
3 changed files with 31 additions and 114 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
novajoin/join.py
View File

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

67
novajoin/tests/unit/api/v1/test_api.py
View File

@ -70,27 +70,8 @@ class JoinTest(test.TestCase):
else: else:
assert(False) 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): def test_no_imageid(self):
body = {"metadata": {"ipa_enroll": "True"}, body = {"metadata": {"ipa_enroll": "False"},
"instance-id": fake.INSTANCE_ID,
"project-id": fake.PROJECT_ID, "project-id": fake.PROJECT_ID,
"hostname": "test"} "hostname": "test"}
req = fakes.HTTPRequest.blank('/v1/') req = fakes.HTTPRequest.blank('/v1/')
@ -108,7 +89,6 @@ class JoinTest(test.TestCase):
def test_no_hostname(self): def test_no_hostname(self):
body = {"metadata": {"ipa_enroll": "True"}, body = {"metadata": {"ipa_enroll": "True"},
"instance-id": fake.INSTANCE_ID,
"project-id": fake.PROJECT_ID, "project-id": fake.PROJECT_ID,
"image-id": fake.IMAGE_ID} "image-id": fake.IMAGE_ID}
req = fakes.HTTPRequest.blank('/v1/') req = fakes.HTTPRequest.blank('/v1/')
@ -125,8 +105,7 @@ class JoinTest(test.TestCase):
assert(False) assert(False)
def test_no_project_id(self): def test_no_project_id(self):
body = {"metadata": {"ipa_enroll": "True"}, body = {"metadata": {"ipa_enroll": "True", "ipa_hostclass": "foo"},
"instance-id": fake.INSTANCE_ID,
"image-id": fake.IMAGE_ID, "image-id": fake.IMAGE_ID,
"hostname": "test"} "hostname": "test"}
req = fakes.HTTPRequest.blank('/v1/') req = fakes.HTTPRequest.blank('/v1/')
@ -146,7 +125,6 @@ class JoinTest(test.TestCase):
def test_request_no_enrollment(self, mock_get_image): def test_request_no_enrollment(self, mock_get_image):
mock_get_image.return_value = FakeImageService() mock_get_image.return_value = FakeImageService()
body = {"metadata": {"ipa_enroll": "False"}, body = {"metadata": {"ipa_enroll": "False"},
"instance-id": fake.INSTANCE_ID,
"project-id": fake.PROJECT_ID, "project-id": fake.PROJECT_ID,
"image-id": fake.IMAGE_ID, "image-id": fake.IMAGE_ID,
"hostname": "test"} "hostname": "test"}
@ -162,7 +140,6 @@ class JoinTest(test.TestCase):
def test_request_invalid_image(self, mock_get_image): def test_request_invalid_image(self, mock_get_image):
mock_get_image.side_effect = Fault(webob.exc.HTTPBadRequest()) mock_get_image.side_effect = Fault(webob.exc.HTTPBadRequest())
body = {"metadata": {"ipa_enroll": "False"}, body = {"metadata": {"ipa_enroll": "False"},
"instance-id": fake.INSTANCE_ID,
"project-id": fake.PROJECT_ID, "project-id": fake.PROJECT_ID,
"image-id": "invalid", "image-id": "invalid",
"hostname": "test"} "hostname": "test"}
@ -181,13 +158,11 @@ class JoinTest(test.TestCase):
assert(False) assert(False)
@mock.patch('novajoin.ipa.SafeConfigParser') @mock.patch('novajoin.ipa.SafeConfigParser')
@mock.patch('novajoin.join.get_instance')
@mock.patch('novajoin.join.get_default_image_service') @mock.patch('novajoin.join.get_default_image_service')
@mock.patch('novajoin.util.get_domain') @mock.patch('novajoin.util.get_domain')
def test_valid_request(self, mock_get_domain, mock_get_image, 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_image.return_value = FakeImageService()
mock_get_instance.return_value = fake.fake_instance
mock_get_domain.return_value = "test" mock_get_domain.return_value = "test"
mock_conf_parser_instance = mock.MagicMock() mock_conf_parser_instance = mock.MagicMock()
@ -196,7 +171,6 @@ class JoinTest(test.TestCase):
mock_conf_parser.return_value = mock_conf_parser_instance mock_conf_parser.return_value = mock_conf_parser_instance
body = {"metadata": {"ipa_enroll": "True"}, body = {"metadata": {"ipa_enroll": "True"},
"instance-id": fake.INSTANCE_ID,
"project-id": fake.PROJECT_ID, "project-id": fake.PROJECT_ID,
"image-id": fake.IMAGE_ID, "image-id": fake.IMAGE_ID,
"hostname": "test"} "hostname": "test"}
@ -221,15 +195,12 @@ class JoinTest(test.TestCase):
@mock.patch('novajoin.ipa.SafeConfigParser') @mock.patch('novajoin.ipa.SafeConfigParser')
@mock.patch('novajoin.keystone_client.get_project_name') @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.join.get_default_image_service')
@mock.patch('novajoin.util.get_domain') @mock.patch('novajoin.util.get_domain')
def test_valid_hostclass_request(self, mock_get_domain, mock_get_image, def test_valid_hostclass_request(self, mock_get_domain, mock_get_image,
mock_get_instance,
mock_get_project_name, mock_get_project_name,
mock_conf_parser): mock_conf_parser):
mock_get_image.return_value = FakeImageService() mock_get_image.return_value = FakeImageService()
mock_get_instance.return_value = fake.fake_instance
mock_get_domain.return_value = "test" mock_get_domain.return_value = "test"
mock_get_project_name.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 mock_conf_parser.return_value = mock_conf_parser_instance
body = {"metadata": {"ipa_enroll": "True"}, body = {"metadata": {"ipa_enroll": "True"},
"instance-id": fake.INSTANCE_ID,
"project-id": fake.PROJECT_ID, "project-id": fake.PROJECT_ID,
"image-id": fake.IMAGE_ID, "image-id": fake.IMAGE_ID,
"hostname": "test"} "hostname": "test"}
@ -262,45 +232,16 @@ class JoinTest(test.TestCase):
# because in all likelihood the keytab cannot be read (and # because in all likelihood the keytab cannot be read (and
# probably doesn't exist. This can be ignored. # 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.join.get_default_image_service')
@mock.patch('novajoin.keystone_client.get_project_name') @mock.patch('novajoin.keystone_client.get_project_name')
@mock.patch('novajoin.util.get_domain') @mock.patch('novajoin.util.get_domain')
def test_invalid_project_id(self, mock_get_domain, mock_get_project_name, 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_image.return_value = FakeImageService()
mock_get_instance.return_value = None
mock_get_project_name.return_value = None mock_get_project_name.return_value = None
mock_get_domain.return_value = "test" mock_get_domain.return_value = "test"
body = {"metadata": {"ipa_enroll": "True", "ipa_hostclass": "foo"}, body = {"metadata": {"ipa_enroll": "True", "ipa_hostclass": "foo"},
"instance-id": fake.INSTANCE_ID,
"project-id": "invalid", "project-id": "invalid",
"image-id": fake.IMAGE_ID, "image-id": fake.IMAGE_ID,
"hostname": "test"} "hostname": "test"}

4
novajoin/tests/unit/fake_constants.py
View File

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