From 4141cc8b160d1208d6aa3a6717963f4a588a92d1 Mon Sep 17 00:00:00 2001 From: rlima Date: Mon, 27 Nov 2023 09:07:23 -0300 Subject: [PATCH] Missing validation for bootstrap_address in restore_values When providing the bootstrap_address in restore_values, there wasn't a validation to ensure the provided value had a valid format. Right now, the address is added in inventory exactly as provided and will fail when running the Ansible playbook. This review includes a validation step that evaluates the IP address value and raises an exception in case it isn't valid, aborting the process for the invalid subcloud. Test plan: All test cases were performed after a subcloud was deployed and had its backup taken. Success cases: - PASS: Run 'dcmanager subcloud-backup restore' for a subcloud using the restore_values.yml file with a valid IP address. The subcloud was successfully restored. - PASS: Run 'dcmanager subcloud-backup restore' for a subcloud group using the restore_values.yml file with a valid IP address for a subcloud and an invalid one for another. The subcloud with the valid IP address was successfully restored. Failure cases: - PASS: Run 'dcmanager subcloud-backup restore' for a subcloud using the restore_values.yml file with an invalid IP address. An error appears in the console stating that the IP address is invalid for the specified subcloud. - PASS: Run 'dcmanager subcloud-backup restore' for a subcloud group using the restore_values.yml file with a valid IP address for a subcloud and an invalid one for another. The subcloud with the invalid IP address wasn't restored. Closes-Bug: 2044551 Change-Id: Ie9f937db1185aa242112efba17741d97707181f3 Signed-off-by: rlima --- distributedcloud/dcmanager/common/utils.py | 6 +++ .../v1/controllers/test_subcloud_backup.py | 52 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/distributedcloud/dcmanager/common/utils.py b/distributedcloud/dcmanager/common/utils.py index 8e8183ebf..28c14edac 100644 --- a/distributedcloud/dcmanager/common/utils.py +++ b/distributedcloud/dcmanager/common/utils.py @@ -969,6 +969,12 @@ def _is_valid_for_backup_restore(subcloud, bootstrap_address_dict=None): msg = ('Unable to obtain the subcloud %s bootstrap_address from either ' 'restore or install values. Please ensure bootstrap_address is ' 'specified in the restore-values.yml and try again.' % subcloud.name) + elif has_bootstrap_address: + try: + netaddr.IPAddress(bootstrap_address_dict[subcloud.name]) + except netaddr.AddrFormatError: + msg = (f'Subcloud {subcloud.name} must have a valid bootstrap address: ' + f'{bootstrap_address_dict[subcloud.name]}') if msg: raise exceptions.ValidateFail(msg) diff --git a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_backup.py b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_backup.py index 5900d5595..72fc76d8e 100644 --- a/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_backup.py +++ b/distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subcloud_backup.py @@ -81,6 +81,16 @@ FAKE_SYSTEM_HEALTH_K8S_FAIL = \ "[2] alarms found, [2] of which are management affecting\n" "All kubernetes nodes are ready: [OK]\n" "All kubernetes control plane pods are ready: [OK]\n") +FAKE_RESTORE_VALUES_INVALID_IP = { + 'bootstrap_address': { + 'subcloud1': '10.10.20.12.22' + } +} +FAKE_RESTORE_VALUES_VALID_IP = { + 'bootstrap_address': { + 'subcloud1': '10.10.20.12' + } +} class TestSubcloudCreate(testroot.DCManagerApiTest): @@ -1021,6 +1031,48 @@ class TestSubcloudRestore(testroot.DCManagerApiTest): self.assertEqual(response.status_int, 200) + @mock.patch.object(rpc_client, 'ManagerClient') + def test_backup_restore_subcloud_valid_restore_values(self, mock_rpc_client): + + subcloud = fake_subcloud.create_fake_subcloud(self.ctx) + db_api.subcloud_update(self.ctx, + subcloud.id, + availability_status=dccommon_consts.AVAILABILITY_ONLINE, + management_state=dccommon_consts.MANAGEMENT_UNMANAGED, + backup_datetime=None, + backup_status=consts.BACKUP_STATE_UNKNOWN) + + fake_password = (base64.b64encode('testpass'.encode("utf-8"))).decode('ascii') + data = {'sysadmin_password': fake_password, 'subcloud': '1', + 'restore_values': FAKE_RESTORE_VALUES_VALID_IP} + + mock_rpc_client().restore_subcloud_backups.return_value = True + response = self.app.patch_json(FAKE_URL_RESTORE, + headers=FAKE_HEADERS, + params=data) + + self.assertEqual(response.status_int, 200) + + @mock.patch.object(rpc_client, 'ManagerClient') + def test_backup_restore_subcloud_invalid_restore_values(self, mock_rpc_client): + + subcloud = fake_subcloud.create_fake_subcloud(self.ctx) + db_api.subcloud_update(self.ctx, + subcloud.id, + availability_status=dccommon_consts.AVAILABILITY_ONLINE, + management_state=dccommon_consts.MANAGEMENT_UNMANAGED, + backup_datetime=None, + backup_status=consts.BACKUP_STATE_UNKNOWN) + + fake_password = (base64.b64encode('testpass'.encode("utf-8"))).decode('ascii') + data = {'sysadmin_password': fake_password, 'subcloud': '1', + 'restore_values': FAKE_RESTORE_VALUES_INVALID_IP} + + mock_rpc_client().restore_subcloud_backups.return_value = True + six.assertRaisesRegex(self, webtest.app.AppError, "400 *", + self.app.patch_json, FAKE_URL_RESTORE, + headers=FAKE_HEADERS, params=data) + @mock.patch.object(rpc_client, 'ManagerClient') def test_backup_restore_unknown_subcloud(self, mock_rpc_client):