diff --git a/distributedcloud/dcmanager/api/controllers/v1/subclouds.py b/distributedcloud/dcmanager/api/controllers/v1/subclouds.py index 1fac82ccd..2398773ec 100644 --- a/distributedcloud/dcmanager/api/controllers/v1/subclouds.py +++ b/distributedcloud/dcmanager/api/controllers/v1/subclouds.py @@ -1155,7 +1155,7 @@ class SubcloudsController(object): "request is from a peer site." ) else: - psd_common.validate_install_values(payload, subcloud) + psd_common.validate_install_values(payload, subcloud=subcloud) payload["data_install"] = json.dumps(payload[consts.INSTALL_VALUES]) try: diff --git a/distributedcloud/dcmanager/common/phased_subcloud_deploy.py b/distributedcloud/dcmanager/common/phased_subcloud_deploy.py index 4cb02ebfc..2e64f2016 100644 --- a/distributedcloud/dcmanager/common/phased_subcloud_deploy.py +++ b/distributedcloud/dcmanager/common/phased_subcloud_deploy.py @@ -479,20 +479,28 @@ def validate_group_id(context, group_id): pecan.abort(400, _("Invalid group_id")) +def get_sysinv_client(region_name=dccommon_consts.DEFAULT_REGION_NAME): + ks_client = get_ks_client(region_name) + endpoint = ks_client.endpoint_cache.get_endpoint('sysinv') + return SysinvClient(region_name, ks_client.session, endpoint=endpoint) + + def get_network_address_pool(network='management', region_name=dccommon_consts.DEFAULT_REGION_NAME): """Get the region network address pool""" - ks_client = get_ks_client(region_name) - endpoint = ks_client.endpoint_cache.get_endpoint('sysinv') - sysinv_client = SysinvClient(region_name, - ks_client.session, - endpoint=endpoint) + sysinv_client = get_sysinv_client(region_name) if network == 'admin': return sysinv_client.get_admin_address_pool() return sysinv_client.get_management_address_pool() -def validate_install_values(payload, subcloud=None): +def get_oam_addresses(region_name=dccommon_consts.DEFAULT_REGION_NAME): + """Get the region OAM addresses""" + sysinv_client = get_sysinv_client(region_name) + return sysinv_client.get_oam_addresses() + + +def validate_install_values(payload, ip_version=None, subcloud=None): """Validate install values if 'install_values' is present in payload. The image in payload install values is optional, and if not provided, @@ -604,12 +612,18 @@ def validate_install_values(payload, subcloud=None): install_values['install_type']) try: - ip_version = (netaddr.IPAddress(install_values['bootstrap_address']). - version) + bootstrap_address = netaddr.IPAddress(install_values['bootstrap_address']) except netaddr.AddrFormatError as e: LOG.exception(e) pecan.abort(400, _("bootstrap_address invalid: %s") % e) + if not ip_version: + ip_version = get_system_controller_oam_ip_version() + + if bootstrap_address.version != ip_version: + pecan.abort(400, _("bootstrap_address in install_values must be the same " + "IP version as the system controller OAM")) + try: bmc_address = netaddr.IPAddress(install_values['bmc_address']) except netaddr.AddrFormatError as e: @@ -754,6 +768,20 @@ def format_ip_address(payload): payload.update({k: address}) +def get_system_controller_oam_ip_version(): + oam_addresses = get_oam_addresses() + return netaddr.IPAddress(oam_addresses.oam_floating_ip).version + + +def check_bootstrap_ip_version(payload, ip_version): + address = payload.get('bootstrap-address', None) + if not address: + return + if netaddr.IPAddress(address).version != ip_version: + pecan.abort(400, _("bootstrap-address must be the same IP version " + "as the system controller OAM")) + + def upload_deploy_config_file(request, payload): file_item = request.POST.get(consts.DEPLOY_CONFIG) if file_item is None: @@ -1028,6 +1056,8 @@ def pre_deploy_create(payload: dict, context: RequestContext, validate_subcloud_config(context, payload) + ip_version = get_system_controller_oam_ip_version() + # install_values of secondary subclouds are validated on peer site if consts.DEPLOY_STATE_SECONDARY in payload and \ utils.is_req_from_another_dc(request): @@ -1035,11 +1065,12 @@ def pre_deploy_create(payload: dict, context: RequestContext, f"{payload['name']}. Subcloud is secondary and " "request is from a peer site.") else: - validate_install_values(payload) + validate_install_values(payload, ip_version) validate_k8s_version(payload) format_ip_address(payload) + check_bootstrap_ip_version(payload, ip_version) # Upload the deploy config files if it is included in the request # It has a dependency on the subcloud name, and it is called after @@ -1102,6 +1133,8 @@ def pre_deploy_bootstrap(context: RequestContext, payload: dict, validate_subcloud_config(context, payload, ignore_conflicts_with=subcloud) format_ip_address(payload) + ip_version = get_system_controller_oam_ip_version() + check_bootstrap_ip_version(payload, ip_version) # Patch status and fresh_install_k8s_version may have been changed # between deploy create and deploy bootstrap commands. Validate them diff --git a/distributedcloud/dcmanager/tests/base.py b/distributedcloud/dcmanager/tests/base.py index d41296af8..083c86e71 100644 --- a/distributedcloud/dcmanager/tests/base.py +++ b/distributedcloud/dcmanager/tests/base.py @@ -264,6 +264,13 @@ class DCManagerTestCase(base.BaseTestCase): self.mock_get_network_address_pool = mock_patch_object.start() self.addCleanup(mock_patch_object.stop) + def _mock_get_oam_addresses(self): + """Mock phased subcloud deploy's get_oam_addresses""" + + mock_patch_object = mock.patch.object(psd_common, 'get_oam_addresses') + self.mock_get_oam_addresses = mock_patch_object.start() + self.addCleanup(mock_patch_object.stop) + def _mock_get_ks_client(self): """Mock phased subcloud deploy's get_ks_client""" diff --git a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_phased_subcloud_deploy.py b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_phased_subcloud_deploy.py index d835fdd13..c316104f0 100644 --- a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_phased_subcloud_deploy.py +++ b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_phased_subcloud_deploy.py @@ -20,12 +20,11 @@ from dcmanager.common import phased_subcloud_deploy as psd_common from dcmanager.common import utils as dutils from dcmanager.db import api as db_api from dcmanager.tests.unit.api.test_root_controller import DCManagerApiTest -from dcmanager.tests.unit.api.v1.controllers.test_subclouds import \ - FakeAddressPool +from dcmanager.tests.unit.api.v1.controllers.test_subclouds import FAKE_IPV4_OAM_POOL +from dcmanager.tests.unit.api.v1.controllers.test_subclouds import FakeAddressPool from dcmanager.tests.unit.api.v1.controllers.test_subclouds import SubcloudAPIMixin from dcmanager.tests.unit.common import fake_subcloud -from dcmanager.tests.unit.manager.test_system_peer_manager import \ - TestSystemPeerManager +from dcmanager.tests.unit.manager.test_system_peer_manager import TestSystemPeerManager FAKE_URL = "/v1.0/phased-subcloud-deploy" FAKE_SOFTWARE_VERSION = "21.12" @@ -42,6 +41,8 @@ class BaseTestPhasedSubcloudDeployController(DCManagerApiTest): self._mock_rpc_client() self._mock_get_ks_client() self._mock_query() + self._mock_get_oam_addresses() + self.mock_get_oam_addresses.return_value = FAKE_IPV4_OAM_POOL def _mock_populate_payload(self): mock_patch_object = mock.patch.object( diff --git a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subclouds.py b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subclouds.py index 4db45babc..8428aa2bc 100644 --- a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subclouds.py +++ b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subclouds.py @@ -114,6 +114,16 @@ class FakeOAMAddressPool(object): self.oam_floating_ip = oam_floating_ip +FAKE_IPV4_OAM_POOL = FakeOAMAddressPool( + "10.10.10.0", "10.10.10.1", "10.10.10.254", "10.10.10.4", + "10.10.10.3", "10.10.10.1", "10.10.10.2") + + +FAKE_IPV6_OAM_POOL = FakeOAMAddressPool( + "fe00::", "fe00::1", "fe00::ffff", "fe00::4", + "fe00::3", "fe00::1", "fe00::2") + + class SubcloudAPIMixin(APIMixin): API_PREFIX = "/v1.0/subclouds" RESULT_KEY = "subclouds" @@ -231,6 +241,8 @@ class BaseTestSubcloudsController(DCManagerApiTest, SubcloudAPIMixin): self._mock_get_ks_client() self._mock_query() self._mock_valid_software_deploy_state() + self._mock_get_oam_addresses() + self.mock_get_oam_addresses.return_value = FAKE_IPV4_OAM_POOL def _update_subcloud(self, **kwargs): self.subcloud = sql_api.subcloud_update(self.ctx, self.subcloud.id, **kwargs) @@ -532,6 +544,24 @@ class TestSubcloudsPost(BaseTestSubcloudsPost, PostMixin): f"failed to detect a valid IP address from '{invalid_value}'", index ) + def _test_post_fails_bootstrap_address_with_wrong_ip_version(self, address): + """Test post fails with bootstrap address version different from OAM primary""" + + self.params["bootstrap-address"] = address + + response = self._send_request() + + self._assert_pecan_and_response( + response, http.client.BAD_REQUEST, "bootstrap-address must be the same IP " + "version as the system controller OAM") + + def test_post_fails_bootstrap_address_with_wrong_ip_version_ipv4(self): + self._test_post_fails_bootstrap_address_with_wrong_ip_version('fd01::77') + + def test_post_fails_bootstrap_address_with_wrong_ip_version_ipv6(self): + self.mock_get_oam_addresses.return_value = FAKE_IPV6_OAM_POOL + self._test_post_fails_bootstrap_address_with_wrong_ip_version('10.10.10.100') + def test_post_fails_with_invalid_systemcontroller_gateway_address(self): """Test post fails with invalid system controller gateway address @@ -958,6 +988,23 @@ class TestSubcloudsPostInstallData(BaseTestSubcloudsPost): "bootstrap_address", ["192.168.1.256", "192.168.206.wut", None] ) + def _test_post_fails_invalid_bootstrap_ip_version_in_install_values(self, address): + """Test post fails when bootstrap IP version differs from OAM's""" + + self._validate_invalid_property( + "bootstrap_address", [address], "bootstrap_address in install_values " + "must be the same IP version as the system controller OAM" + ) + + def test_post_fails_invalid_bootstrap_address_version_in_install_values_ipv4(self): + self._test_post_fails_invalid_bootstrap_ip_version_in_install_values( + "fd01:6::7") + + def test_post_fails_invalid_bootstrap_address_version_in_install_values_ipv6(self): + self.mock_get_oam_addresses.return_value = FAKE_IPV6_OAM_POOL + self._test_post_fails_invalid_bootstrap_ip_version_in_install_values( + "10.10.10.100") + def test_post_fails_with_invalid_bmc_address_in_install_values(self): """Test post fails with invalid bmc address in install values @@ -991,6 +1038,8 @@ class TestSubcloudsPostInstallData(BaseTestSubcloudsPost): defaults to IPv4. """ + self.mock_get_oam_addresses.return_value = FAKE_IPV6_OAM_POOL + kwargs = {"bootstrap_address": "fd01:6::7"} self._validate_invalid_property( @@ -999,6 +1048,8 @@ class TestSubcloudsPostInstallData(BaseTestSubcloudsPost): ) self.mock_pecan_abort.reset_mock() + self.mock_get_oam_addresses.return_value = FAKE_IPV4_OAM_POOL + self._validate_invalid_ip_address("bmc_address", ["fd01:6:-1", None]) def test_post_fails_with_invalid_persistent_size_in_install_values(self): @@ -1043,6 +1094,8 @@ class TestSubcloudsPostInstallData(BaseTestSubcloudsPost): All of the required IP addresses must be in the same IP version. """ + self.mock_get_oam_addresses.return_value = FAKE_IPV6_OAM_POOL + kwargs = {"bootstrap_address": "fd01:6::6", "bmc_address": "fd01:6::7"} self._validate_invalid_property( @@ -1051,6 +1104,8 @@ class TestSubcloudsPostInstallData(BaseTestSubcloudsPost): ) self.mock_pecan_abort.reset_mock() + self.mock_get_oam_addresses.return_value = FAKE_IPV4_OAM_POOL + self._validate_invalid_ip_address("nexthop_gateway", ["fd01:6:-1", None]) def test_post_fails_with_invalid_network_address_in_install_values(self):