docker_image_availability: credentials to skopeo

Currently, docker_image_availability health_check
does not support authenticated registries.

This commit adds the '--creds=' option to skopeo
if needed to support authentication credentials.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1316341

Some other fixes to handle docker config better:

Should now account properly for blocked registries, insecure registries,
multiple additional registries, and oreg_url registry with or without credentials.
Output on failure should be clearer about what was tried.

Fixed a bug in the action_plugin_test exposed by these changes.

3 files changed, 76 insertions, 83 deletions

diff --git a/roles/openshift_health_checker/openshift_checks/docker_image_availability.py b/roles/openshift_health_checker/openshift_checks/docker_image_availability.py
index 63ccadcd1..7c8ac78fe 100644
--- a/roles/openshift_health_checker/openshift_checks/docker_image_availability.py
+++ b/roles/openshift_health_checker/openshift_checks/docker_image_availability.py
@@ -1,5 +1,6 @@
 """Check that required Docker images are available."""
 
+from pipes import quote
 from ansible.module_utils import six
 from openshift_checks import OpenShiftCheck
 from openshift_checks.mixins import DockerHostMixin
@@ -33,10 +34,39 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck):
     # we use python-docker-py to check local docker for images, and skopeo
     # to look for images available remotely without waiting to pull them.
     dependencies = ["python-docker-py", "skopeo"]
-    skopeo_img_check_command = "timeout 10 skopeo inspect --tls-verify=false docker://{registry}/{image}"
+    # command for checking if remote registries have an image, without docker pull
+    skopeo_command = "timeout 10 skopeo inspect --tls-verify={tls} {creds} docker://{registry}/{image}"
+    skopeo_example_command = "skopeo inspect [--tls-verify=false] [--creds=<user>:<pass>] docker://<registry>/<image>"
 
     def __init__(self, *args, **kwargs):
         super(DockerImageAvailability, self).__init__(*args, **kwargs)
+
+        self.registries = dict(
+            # set of registries that need to be checked insecurely (note: not accounting for CIDR entries)
+            insecure=set(self.ensure_list("openshift_docker_insecure_registries")),
+            # set of registries that should never be queried even if given in the image
+            blocked=set(self.ensure_list("openshift_docker_blocked_registries")),
+        )
+
+        # ordered list of registries (according to inventory vars) that docker will try for unscoped images
+        regs = self.ensure_list("openshift_docker_additional_registries")
+        # currently one of these registries is added whether the user wants it or not.
+        deployment_type = self.get_var("openshift_deployment_type")
+        if deployment_type == "origin" and "docker.io" not in regs:
+            regs.append("docker.io")
+        elif deployment_type == 'openshift-enterprise' and "registry.access.redhat.com" not in regs:
+            regs.append("registry.access.redhat.com")
+        self.registries["configured"] = regs
+
+        # for the oreg_url registry there may be credentials specified
+        components = self.get_var("oreg_url", default="").split('/')
+        self.registries["oreg"] = "" if len(components) < 3 else components[0]
+        self.skopeo_command_creds = ""
+        oreg_auth_user = self.get_var('oreg_auth_user', default='')
+        oreg_auth_password = self.get_var('oreg_auth_password', default='')
+        if oreg_auth_user != '' and oreg_auth_password != '':
+            self.skopeo_command_creds = "--creds={}:{}".format(quote(oreg_auth_user), quote(oreg_auth_password))
+
         # record whether we could reach a registry or not (and remember results)
         self.reachable_registries = {}
 
@@ -62,26 +92,25 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck):
         if not missing_images:
             return {}
 
-        registries = self.known_docker_registries()
-        if not registries:
-            return {"failed": True, "msg": "Unable to retrieve any docker registries."}
-
-        available_images = self.available_images(missing_images, registries)
+        available_images = self.available_images(missing_images)
         unavailable_images = set(missing_images) - set(available_images)
 
         if unavailable_images:
-            registries = [
-                reg if self.reachable_registries.get(reg, True) else reg + " (unreachable)"
-                for reg in registries
-            ]
+            unreachable = [reg for reg, reachable in self.reachable_registries.items() if not reachable]
+            unreachable_msg = "Failed connecting to: {}\n".format(", ".join(unreachable))
+            blocked_msg = "Blocked registries: {}\n".format(", ".join(self.registries["blocked"]))
             msg = (
-                "One or more required Docker images are not available:\n    {}\n"
-                "Configured registries: {}\n"
-                "Checked by: {}"
+                "One or more required container images are not available:\n    {missing}\n"
+                "Checked with: {cmd}\n"
+                "Default registries searched: {registries}\n"
+                "{blocked}"
+                "{unreachable}"
             ).format(
-                ",\n    ".join(sorted(unavailable_images)),
-                ", ".join(registries),
-                self.skopeo_img_check_command
+                missing=",\n    ".join(sorted(unavailable_images)),
+                cmd=self.skopeo_example_command,
+                registries=", ".join(self.registries["configured"]),
+                blocked=blocked_msg if self.registries["blocked"] else "",
+                unreachable=unreachable_msg if unreachable else "",
             )
 
             return dict(failed=True, msg=msg)
@@ -138,11 +167,10 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck):
 
     def local_images(self, images):
         """Filter a list of images and return those available locally."""
-        registries = self.known_docker_registries()
         found_images = []
         for image in images:
             # docker could have the image name as-is or prefixed with any registry
-            imglist = [image] + [reg + "/" + image for reg in registries]
+            imglist = [image] + [reg + "/" + image for reg in self.registries["configured"]]
             if self.is_image_local(imglist):
                 found_images.append(image)
         return found_images
@@ -152,37 +180,27 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck):
         result = self.execute_module("docker_image_facts", {"name": image})
         return bool(result.get("images")) and not result.get("failed")
 
-    def known_docker_registries(self):
-        """Build a list of docker registries available according to inventory vars."""
-        regs = self.get_var("openshift_docker_additional_registries", default=[])
+    def ensure_list(self, registry_param):
+        """Return the task var as a list."""
         # https://bugzilla.redhat.com/show_bug.cgi?id=1497274
-        # if the result was a string type, place it into a list. We must do this
+        # If the result was a string type, place it into a list. We must do this
         # as using list() on a string will split the string into its characters.
-        if isinstance(regs, six.string_types):
-            regs = [regs]
-        else:
-            # Otherwise cast to a list as was done previously
-            regs = list(regs)
+        # Otherwise cast to a list as was done previously.
+        registry = self.get_var(registry_param, default=[])
+        if not isinstance(registry, six.string_types):
+            return list(registry)
+        return self.normalize(registry)
 
-        deployment_type = self.get_var("openshift_deployment_type")
-        if deployment_type == "origin" and "docker.io" not in regs:
-            regs.append("docker.io")
-        elif deployment_type == 'openshift-enterprise' and "registry.access.redhat.com" not in regs:
-            regs.append("registry.access.redhat.com")
-
-        return regs
-
-    def available_images(self, images, default_registries):
+    def available_images(self, images):
         """Search remotely for images. Returns: list of images found."""
         return [
             image for image in images
-            if self.is_available_skopeo_image(image, default_registries)
+            if self.is_available_skopeo_image(image)
         ]
 
-    def is_available_skopeo_image(self, image, default_registries):
+    def is_available_skopeo_image(self, image):
         """Use Skopeo to determine if required image exists in known registry(s)."""
-        registries = default_registries
-
+        registries = self.registries["configured"]
         # If image already includes a registry, only use that.
         # NOTE: This logic would incorrectly identify images that do not use a namespace, e.g.
         # registry.access.redhat.com/rhel7 as if the registry were a namespace.
@@ -193,13 +211,18 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck):
             registries = [registry]
 
         for registry in registries:
+            if registry in self.registries["blocked"]:
+                continue  # blocked will never be consulted
             if registry not in self.reachable_registries:
                 self.reachable_registries[registry] = self.connect_to_registry(registry)
             if not self.reachable_registries[registry]:
-                continue
+                continue  # do not keep trying unreachable registries
+
+            args = dict(registry=registry, image=image)
+            args["tls"] = "false" if registry in self.registries["insecure"] else "true"
+            args["creds"] = self.skopeo_command_creds if registry == self.registries["oreg"] else ""
 
-            args = {"_raw_params": self.skopeo_img_check_command.format(registry=registry, image=image)}
-            result = self.execute_module_with_retries("command", args)
+            result = self.execute_module_with_retries("command", {"_raw_params": self.skopeo_command.format(**args)})
             if result.get("rc", 0) == 0 and not result.get("failed"):
                 return True
             if result.get("rc") == 124:  # RC 124 == timed out; mark unreachable
diff --git a/roles/openshift_health_checker/test/action_plugin_test.py b/roles/openshift_health_checker/test/action_plugin_test.py
index f14887303..40ad27d5d 100644
--- a/roles/openshift_health_checker/test/action_plugin_test.py
+++ b/roles/openshift_health_checker/test/action_plugin_test.py
@@ -94,6 +94,7 @@ def skipped(result):
     {},
 ])
 def test_action_plugin_missing_openshift_facts(plugin, task_vars, monkeypatch):
+    monkeypatch.setattr(plugin, 'load_known_checks', lambda *_: {})
     monkeypatch.setattr('openshift_health_check.resolve_checks', lambda *args: ['fake_check'])
     result = plugin.run(tmp=None, task_vars=task_vars)
 
diff --git a/roles/openshift_health_checker/test/docker_image_availability_test.py b/roles/openshift_health_checker/test/docker_image_availability_test.py
index 43dcf0c9a..dec99e5db 100644
--- a/roles/openshift_health_checker/test/docker_image_availability_test.py
+++ b/roles/openshift_health_checker/test/docker_image_availability_test.py
@@ -98,40 +98,7 @@ def test_all_images_unavailable(task_vars):
     actual = check.run()
 
     assert actual['failed']
-    assert "required Docker images are not available" in actual['msg']
-
-
-def test_no_known_registries():
-    def execute_module(module_name=None, *_):
-        if module_name == "command":
-            return {
-                'failed': True,
-            }
-
-        return {
-            'changed': False,
-        }
-
-    def mock_known_docker_registries():
-        return []
-
-    dia = DockerImageAvailability(execute_module, task_vars=dict(
-        openshift=dict(
-            common=dict(
-                service_type='origin',
-                is_containerized=False,
-                is_atomic=False,
-            )
-        ),
-        openshift_docker_additional_registries=["docker.io"],
-        openshift_deployment_type="openshift-enterprise",
-        openshift_image_tag='latest',
-        group_names=['oo_nodes_to_config', 'oo_masters_to_config'],
-    ))
-    dia.known_docker_registries = mock_known_docker_registries
-    actual = dia.run()
-    assert actual['failed']
-    assert "Unable to retrieve any docker registries." in actual['msg']
+    assert "required container images are not available" in actual['msg']
 
 
 @pytest.mark.parametrize("message,extra_words", [
@@ -172,13 +139,13 @@ def test_skopeo_update_failure(task_vars, message, extra_words):
             "spam/eggs:v1", ["test.reg"],
             True, True,
             False,
-            {"test.reg": False},
+            {"test.reg": False, "docker.io": False},
         ),
         (
             "spam/eggs:v1", ["test.reg"],
             False, True,
             False,
-            {"test.reg": True},
+            {"test.reg": True, "docker.io": True},
         ),
         (
             "eggs.reg/spam/eggs:v1", ["test.reg"],
@@ -195,17 +162,19 @@ def test_registry_availability(image, registries, connection_test_failed, skopeo
         elif module_name == "command":
             return dict(msg="msg", failed=skopeo_failed)
 
-    check = DockerImageAvailability(execute_module, task_vars())
+    tv = task_vars()
+    tv.update({"openshift_docker_additional_registries": registries})
+    check = DockerImageAvailability(execute_module, tv)
     check._module_retry_interval = 0
 
-    available = check.is_available_skopeo_image(image, registries)
+    available = check.is_available_skopeo_image(image)
     assert available == expect_success
     assert expect_registries_reached == check.reachable_registries
 
 
 @pytest.mark.parametrize("deployment_type, is_containerized, groups, oreg_url, expected", [
     (  # standard set of stuff required on nodes
-        "origin", False, ['oo_nodes_to_config'], None,
+        "origin", False, ['oo_nodes_to_config'], "",
         set([
             'openshift/origin-pod:vtest',
             'openshift/origin-deployer:vtest',
@@ -225,7 +194,7 @@ def test_registry_availability(image, registries, connection_test_failed, skopeo
         ])
     ),
     (
-        "origin", True, ['oo_nodes_to_config', 'oo_masters_to_config', 'oo_etcd_to_config'], None,
+        "origin", True, ['oo_nodes_to_config', 'oo_masters_to_config', 'oo_etcd_to_config'], "",
         set([
             # images running on top of openshift
             'openshift/origin-pod:vtest',