From e56b55452549cb037da0a4165154ffa494e9678a Mon Sep 17 00:00:00 2001 From: Thomas Stringer Date: Thu, 10 Sep 2020 14:29:54 -0400 Subject: Retrieve SSH keys from IMDS first with OVF as a fallback (#509) * pull ssh keys from imds first and fall back to ovf if unavailable * refactor log and diagnostic messages * refactor the OpenSSLManager instantiation and certificate usage * fix unit test where exception was being silenced for generate cert * fix tests now that certificate is not always generated * add documentation for ssh key retrieval * add ability to check if http client has security enabled * refactor certificate logic to GoalState --- tests/unittests/test_datasource/test_azure.py | 64 ++++++++++++++++++---- .../unittests/test_datasource/test_azure_helper.py | 13 +++-- 2 files changed, 62 insertions(+), 15 deletions(-) (limited to 'tests/unittests/test_datasource') diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 47e03bd1..2dda9925 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -102,7 +102,13 @@ NETWORK_METADATA = { "vmId": "ff702a6b-cb6a-4fcd-ad68-b4ce38227642", "vmScaleSetName": "", "vmSize": "Standard_DS1_v2", - "zone": "" + "zone": "", + "publicKeys": [ + { + "keyData": "key1", + "path": "path1" + } + ] }, "network": { "interface": [ @@ -302,7 +308,7 @@ class TestGetMetadataFromIMDS(HttprettyTestCase): def setUp(self): super(TestGetMetadataFromIMDS, self).setUp() - self.network_md_url = dsaz.IMDS_URL + "instance?api-version=2017-12-01" + self.network_md_url = dsaz.IMDS_URL + "instance?api-version=2019-06-01" @mock.patch(MOCKPATH + 'readurl') @mock.patch(MOCKPATH + 'EphemeralDHCPv4') @@ -1304,6 +1310,40 @@ scbus-1 on xpt0 bus 0 dsaz.get_hostname(hostname_command=("hostname",)) m_subp.assert_called_once_with(("hostname",), capture=True) + @mock.patch( + 'cloudinit.sources.helpers.azure.OpenSSLManager.parse_certificates') + def test_get_public_ssh_keys_with_imds(self, m_parse_certificates): + sys_cfg = {'datasource': {'Azure': {'apply_network_config': True}}} + odata = {'HostName': "myhost", 'UserName': "myuser"} + data = { + 'ovfcontent': construct_valid_ovf_env(data=odata), + 'sys_cfg': sys_cfg + } + dsrc = self._get_ds(data) + dsrc.get_data() + dsrc.setup(True) + ssh_keys = dsrc.get_public_ssh_keys() + self.assertEqual(ssh_keys, ['key1']) + self.assertEqual(m_parse_certificates.call_count, 0) + + @mock.patch(MOCKPATH + 'get_metadata_from_imds') + def test_get_public_ssh_keys_without_imds( + self, + m_get_metadata_from_imds): + m_get_metadata_from_imds.return_value = dict() + sys_cfg = {'datasource': {'Azure': {'apply_network_config': True}}} + odata = {'HostName': "myhost", 'UserName': "myuser"} + data = { + 'ovfcontent': construct_valid_ovf_env(data=odata), + 'sys_cfg': sys_cfg + } + dsrc = self._get_ds(data) + dsaz.get_metadata_from_fabric.return_value = {'public-keys': ['key2']} + dsrc.get_data() + dsrc.setup(True) + ssh_keys = dsrc.get_public_ssh_keys() + self.assertEqual(ssh_keys, ['key2']) + class TestAzureBounce(CiTestCase): @@ -2094,14 +2134,18 @@ class TestAzureDataSourcePreprovisioning(CiTestCase): md, _ud, cfg, _d = dsa._reprovision() self.assertEqual(md['local-hostname'], hostname) self.assertEqual(cfg['system_info']['default_user']['name'], username) - self.assertEqual(fake_resp.call_args_list, - [mock.call(allow_redirects=True, - headers={'Metadata': 'true', - 'User-Agent': - 'Cloud-Init/%s' % vs()}, - method='GET', - timeout=dsaz.IMDS_TIMEOUT_IN_SECONDS, - url=full_url)]) + self.assertIn( + mock.call( + allow_redirects=True, + headers={ + 'Metadata': 'true', + 'User-Agent': 'Cloud-Init/%s' % vs() + }, + method='GET', + timeout=dsaz.IMDS_TIMEOUT_IN_SECONDS, + url=full_url + ), + fake_resp.call_args_list) self.assertEqual(m_dhcp.call_count, 2) m_net.assert_any_call( broadcast='192.168.2.255', interface='eth9', ip='192.168.2.9', diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py index 5e6d3d2d..5c31b8be 100644 --- a/tests/unittests/test_datasource/test_azure_helper.py +++ b/tests/unittests/test_datasource/test_azure_helper.py @@ -609,11 +609,11 @@ class TestWALinuxAgentShim(CiTestCase): self.GoalState.return_value.container_id = self.test_container_id self.GoalState.return_value.instance_id = self.test_instance_id - def test_azure_endpoint_client_uses_certificate_during_report_ready(self): + def test_http_client_does_not_use_certificate(self): shim = wa_shim() shim.register_with_azure_and_fetch_data() self.assertEqual( - [mock.call(self.OpenSSLManager.return_value.certificate)], + [mock.call(None)], self.AzureEndpointHttpClient.call_args_list) def test_correct_url_used_for_goalstate_during_report_ready(self): @@ -625,8 +625,11 @@ class TestWALinuxAgentShim(CiTestCase): [mock.call('http://test_endpoint/machine/?comp=goalstate')], get.call_args_list) self.assertEqual( - [mock.call(get.return_value.contents, - self.AzureEndpointHttpClient.return_value)], + [mock.call( + get.return_value.contents, + self.AzureEndpointHttpClient.return_value, + False + )], self.GoalState.call_args_list) def test_certificates_used_to_determine_public_keys(self): @@ -701,7 +704,7 @@ class TestWALinuxAgentShim(CiTestCase): shim.register_with_azure_and_fetch_data() shim.clean_up() self.assertEqual( - 1, self.OpenSSLManager.return_value.clean_up.call_count) + 0, self.OpenSSLManager.return_value.clean_up.call_count) def test_fetch_goalstate_during_report_ready_raises_exc_on_get_exc(self): self.AzureEndpointHttpClient.return_value.get \ -- cgit v1.2.3