From 0bd38dbadcaeef59213e5471018649f7a9d2d9bf Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Mon, 24 Sep 2018 13:36:39 -0700 Subject: [PATCH] Enforce case-sensitive hostnames in aggregate host add If we are using a case-insensitive backend database like mysql, a user can request an aggregate host add with a non-matching hostname and we will not signal failure. We will create a mapping which will not actually include that host in the aggregate, which will be confusing later. This change makes us fail if the host name does not match exactly, which is the same behavior as if we are using a case-sensitive backend (like postgres). Change-Id: I597dee74d33de337913eddda74ab056fbf81a23c Closes-Bug: #1709260 (cherry picked from commit c8e2de668434861b87495e0a0f715b2f90d8ec05) (cherry picked from commit c72bb2c747016b22e71109b9750d5899b776e67a) (cherry picked from commit 22197f3d0156b9fb87a132e77ab44dbdd63bc826) --- nova/compute/api.py | 13 +++++++++++-- nova/tests/unit/compute/test_compute.py | 24 +++++++++++++++++++++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 8c38a3b7391b..d069db4057b7 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -4896,14 +4896,23 @@ class AggregateAPI(base.Base): try: mapping = objects.HostMapping.get_by_host(context, host_name) nova_context.set_target_cell(context, mapping.cell_mapping) - objects.Service.get_by_compute_host(context, host_name) + service = objects.Service.get_by_compute_host(context, host_name) except exception.HostMappingNotFound: try: # NOTE(danms): This targets our cell - _find_service_in_cell(context, service_host=host_name) + service = _find_service_in_cell(context, + service_host=host_name) except exception.NotFound: raise exception.ComputeHostNotFound(host=host_name) + if service.host != host_name: + # NOTE(danms): If we found a service but it is not an + # exact match, we may have a case-insensitive backend + # database (like mysql) which will end up with us + # adding the host-aggregate mapping with a + # non-matching hostname. + raise exception.ComputeHostNotFound(host=host_name) + aggregate = objects.Aggregate.get_by_id(context, aggregate_id) self.is_safe_to_update_az(context, aggregate.metadata, hosts=[host_name], aggregate=aggregate) diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 7e15f5c6aae6..7559e8d65cf5 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -11507,6 +11507,26 @@ class ComputeAPIAggrTestCase(BaseTestCase): self.assertEqual(fake_notifier.NOTIFICATIONS[1].publisher_id, 'compute.fake-mini') + @mock.patch('nova.objects.Service.get_by_compute_host') + def test_add_host_to_aggregate_raise_not_found_case(self, + mock_get_service): + # Ensure ComputeHostNotFound is raised when adding a host with a + # hostname that doesn't exactly map what we have stored. + + def return_anyway(context, host_name): + return objects.Service(host=host_name.upper()) + + mock_get_service.side_effect = return_anyway + + aggr = self.api.create_aggregate(self.context, 'fake_aggregate', + 'fake_zone') + fake_notifier.NOTIFICATIONS = [] + values = _create_service_entries(self.context) + fake_host = values[0][1][0] + self.assertRaises(exception.ComputeHostNotFound, + self.api.add_host_to_aggregate, + self.context, aggr.id, fake_host) + @mock.patch('nova.objects.HostMapping.get_by_host') @mock.patch('nova.context.set_target_cell') def test_add_host_to_aggregate_raise_cn_not_found(self, mock_st, @@ -11674,7 +11694,9 @@ class ComputeAPIAggrCallsSchedulerTestCase(test.NoDBTestCase): agg = objects.Aggregate(name='fake', metadata={}) agg.add_host = mock.Mock() with test.nested( - mock.patch.object(objects.Service, 'get_by_compute_host'), + mock.patch.object(objects.Service, 'get_by_compute_host', + return_value=objects.Service( + host='fakehost')), mock.patch.object(objects.Aggregate, 'get_by_id', return_value=agg)): self.api.add_host_to_aggregate(self.context, 1, 'fakehost')