From 51d723a9ff0d7af47d1974a8e77fe7187e57357b Mon Sep 17 00:00:00 2001 From: Alex Schultz Date: Fri, 21 Jun 2019 14:07:34 -0600 Subject: [PATCH] Handle registries with incorrect certs correctly We actually end up allowing registries with incorrect certificates in the upload code so they get treated the same as inscure registries. This change updates the return code of the is_insecure_registry to match how we handle these registries in the image uploader. Additionally, this means that when the container image prepare code runs, we will correctly include these registries that have ssl enabled but incorrect certificates will correctly be included in the DockerInsecureRegistryAddress setting. Change-Id: I7fba881645ec2ea167c064be07ed6d4281b7ed3d Closes-Bug: #1833751 --- tripleo_common/image/image_uploader.py | 10 ++++++++-- tripleo_common/tests/image/test_image_uploader.py | 12 ++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/tripleo_common/image/image_uploader.py b/tripleo_common/image/image_uploader.py index 76a95f47d..8273b6171 100644 --- a/tripleo_common/image/image_uploader.py +++ b/tripleo_common/image/image_uploader.py @@ -668,7 +668,8 @@ class BaseImageUploader(object): def is_insecure_registry(self, registry_host): if registry_host in self.secure_registries: return False - if registry_host in self.insecure_registries: + if (registry_host in self.insecure_registries or + registry_host in self.no_verify_registries): return True try: requests.get('https://%s/v2' % registry_host, timeout=30) @@ -679,7 +680,12 @@ class BaseImageUploader(object): requests.get('https://%s/v2' % registry_host, timeout=30, verify=False) self.no_verify_registries.add(registry_host) - return False + # Techinically these type of registries are insecure when + # the container engine tries to do a pull. The python uploader + # ignores the certificate problem, but they are still inscure + # so we return True here while we'll still use https when we + # access the registry. LP#1833751 + return True except requests.exceptions.SSLError: # So nope, it's really not a certificate verification issue self.insecure_registries.add(registry_host) diff --git a/tripleo_common/tests/image/test_image_uploader.py b/tripleo_common/tests/image/test_image_uploader.py index d4f898de1..e3352f699 100644 --- a/tripleo_common/tests/image/test_image_uploader.py +++ b/tripleo_common/tests/image/test_image_uploader.py @@ -241,6 +241,18 @@ class TestBaseImageUploader(base.TestCase): self.requests.request_history[0].url ) + @mock.patch('requests.get') + def test_is_insecure_registry_bad_cert(self, mock_get): + mock_get.side_effect = [requests.exceptions.SSLError('ouch'), True] + self.assertTrue( + self.uploader.is_insecure_registry('bcert:8787')) + self.assertTrue( + self.uploader.is_insecure_registry('bcert:8787')) + calls = [mock.call('https://bcert:8787/v2', timeout=30), + mock.call('https://bcert:8787/v2', timeout=30, verify=False)] + mock_get.assert_has_calls(calls) + self.assertEqual(mock_get.call_count, 2) + def test_is_insecure_registry_timeout(self): self.requests.get( 'https://192.0.2.0:8787/',