Update docker registry auth to idempotent

Currently, the primary method for placing registry
authentication credentials for use with oreg_url
is via a command call to 'docker login'

This cli call is not very idempotent.

This commit changes primary method to custom module
that creates / updates the json file directly in
an idempotent fashion and utilizes urllib to test
that the credentials are valid before proceeding.

roles/container_runtime/defaults/main.yml

@@ -2,12 +2,6 @@
 docker_cli_auth_config_path: '/root/.docker'
 openshift_docker_signature_verification: False
 
-openshift_docker_alternative_creds: False
-
-# oreg_url is defined by user input.
-oreg_host: "{{ oreg_url.split('/')[0] if (oreg_url is defined and '.' in oreg_url.split('/')[0]) else '' }}"
-oreg_auth_credentials_replace: False
-
 openshift_docker_selinux_enabled: True
 openshift_docker_service_name: "docker"
 

roles/container_runtime/tasks/package_crio.yml

@@ -85,5 +85,3 @@
 # If we are using crio only, docker.service might not be available for
 # 'docker login'
 - import_tasks: common/post.yml
-  vars:
-    openshift_docker_alternative_creds: "{{ openshift_use_crio_only | bool }}"

roles/container_runtime/tasks/registry_auth.yml

@@ -1,34 +1,18 @@
 ---
-- name: Check for credentials file for registry auth
-  stat:
-    path: "{{ docker_cli_auth_config_path }}/config.json"
-  when: oreg_auth_user is defined
-  register: docker_cli_auth_credentials_stat
-
-- name: Create credentials for docker cli registry auth
-  command: "docker --config={{ docker_cli_auth_config_path }} login -u {{ oreg_auth_user }} -p {{ oreg_auth_password }} {{ oreg_host }}"
-  register: openshift_docker_credentials_create_res
-  retries: 3
-  delay: 5
-  until: openshift_docker_credentials_create_res.rc == 0
-  when:
-  - not openshift_docker_alternative_creds | bool
-  - oreg_auth_user is defined
-  - (not docker_cli_auth_credentials_stat.stat.exists or oreg_auth_credentials_replace) | bool
-  no_log: True
-
 # docker_creds is a custom module from lib_utils
 # 'docker login' requires a docker.service running on the local host, this is an
-# alternative implementation for non-docker hosts.  This implementation does not
-# check the registry to determine whether or not the credentials will work.
+# alternative implementation that operates directly on config.json
 - name: Create credentials for docker cli registry auth (alternative)
   docker_creds:
     path: "{{ docker_cli_auth_config_path }}"
     registry: "{{ oreg_host }}"
     username: "{{ oreg_auth_user }}"
     password: "{{ oreg_auth_password }}"
+    # Test that we can actually connect with provided info
+    test_login: "{{ oreg_test_login | default(True) }}"
   when:
-  - openshift_docker_alternative_creds | bool
   - oreg_auth_user is defined
-  - (not docker_cli_auth_credentials_stat.stat.exists or oreg_auth_credentials_replace) | bool
-  no_log: True
+  register: crt_oreg_auth_credentials_create
+  retries: 3
+  delay: 5
+  until: crt_oreg_auth_credentials_create.rc == 0

roles/lib_utils/action_plugins/sanity_checks.py

@@ -63,8 +63,8 @@ IMAGE_POLICY_CONFIG_VAR = "openshift_master_image_policy_config"
 ALLOWED_REGISTRIES_VAR = "openshift_master_image_policy_allowed_registries_for_import"
 
 REMOVED_VARIABLES = (
-    # Leaving example as this code might be used again in future.
-    # ('old_var', 'new_var')
+    # TODO(michaelgugino): Remove in 3.12
+    ('oreg_auth_credentials_replace', 'Removed: Credentials are now always updated')
 )
 
 # JSON_FORMAT_VARIABLES does not intende to cover all json variables, but

roles/lib_utils/library/docker_creds.py

@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 # pylint: disable=missing-docstring
 #
-# Copyright 2017 Red Hat, Inc. and/or its affiliates
+# Copyright 2017, 2018 Red Hat, Inc. and/or its affiliates
 # and other contributors as indicated by the @author tags.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -21,7 +21,7 @@ import json
 import os
 
 from ansible.module_utils.basic import AnsibleModule
-
+from six.moves import urllib
 
 DOCUMENTATION = '''
 ---
@@ -53,6 +53,11 @@ options:
         description:
             - This is the password to authenticate to the registry with.
         required: true
+    test_login:
+        description:
+            - Attempt to connect to registry with username + password provided.
+        default: true
+        required: false
 
 author:
     - "Michael Gugino <mgugino@redhat.com>"
@@ -66,6 +71,7 @@ EXAMPLES = '''
     registry: registry.example.com:443
     username: myuser
     password: mypassword
+    test_login: True
 '''
 
 
@@ -125,7 +131,23 @@ def load_config_file(module, dest):
         return {}
 
 
-def update_config(docker_config, registry, username, password):
+def validate_registry_login(module, registry, encoded_auth):
+    '''Attempt to use credentials to log into registry'''
+    url = 'https://' + registry + '/v2/'
+    auth_header = 'Basic {}'.format(encoded_auth)
+    headers = {'Authorization': auth_header}
+    req = urllib.request.Request(url=url, headers=headers)
+    try:
+        urllib.request.urlopen(req)
+    except urllib.error.HTTPError as err:
+        result = {'failed': True,
+                  'changed': False,
+                  'msg': str(err),
+                  'state': 'unknown'}
+        module.fail_json(**result)
+
+
+def update_config(docker_config, registry, encoded_auth):
     '''Add our registry auth credentials into docker_config dict'''
 
     # Add anything that might be missing in our dictionary
@@ -134,15 +156,12 @@ def update_config(docker_config, registry, username, password):
     if registry not in docker_config['auths']:
         docker_config['auths'][registry] = {}
 
-    # base64 encode our username:password string
-    encoded_data = base64.b64encode('{}:{}'.format(username, password).encode())
-
     # check if the same value is already present for idempotency.
     if 'auth' in docker_config['auths'][registry]:
-        if docker_config['auths'][registry]['auth'] == encoded_data:
+        if docker_config['auths'][registry]['auth'] == encoded_auth:
             # No need to go further, everything is already set in file.
             return False
-    docker_config['auths'][registry]['auth'] = encoded_data
+    docker_config['auths'][registry]['auth'] = encoded_auth
     return True
 
 
@@ -168,7 +187,8 @@ def run_module():
         path=dict(aliases=['dest', 'name'], required=True, type='path'),
         registry=dict(type='str', required=True),
         username=dict(type='str', required=True),
-        password=dict(type='str', required=True, no_log=True)
+        password=dict(type='str', required=True, no_log=True),
+        test_login=dict(type='str', required=False, default=True),
     )
 
     module = AnsibleModule(
@@ -181,6 +201,7 @@ def run_module():
     registry = module.params['registry']
     username = module.params['username']
     password = module.params['password']
+    test_login = module.params['test_login']
 
     if not check_dest_dir_exists(module, dest):
         create_dest_dir(module, dest)
@@ -190,8 +211,15 @@ def run_module():
         # in case there are other registries/settings already present.
         docker_config = load_config_file(module, dest)
 
+    # base64 encode our username:password string
+    encoded_auth = base64.b64encode('{}:{}'.format(username, password).encode())
+
+    # Test the credentials
+    if test_login:
+        validate_registry_login(module, registry, encoded_auth)
+
     # Put the registry auth info into the config dict.
-    changed = update_config(docker_config, registry, username, password)
+    changed = update_config(docker_config, registry, encoded_auth)
 
     if changed:
         write_config(module, docker_config, dest)

roles/openshift_control_plane/defaults/main.yml

@@ -52,11 +52,7 @@ default_r_openshift_master_os_firewall_allow:
   port: "{{ openshift_master_dns_port }}/udp"
 r_openshift_master_os_firewall_allow: "{{ default_r_openshift_master_os_firewall_allow | union(openshift_master_open_ports | default([])) }}"
 
-# oreg_url is defined by user input
-oreg_host: "{{ oreg_url.split('/')[0] if (oreg_url is defined and '.' in oreg_url.split('/')[0]) else '' }}"
 oreg_auth_credentials_path: "{{ r_openshift_master_data_dir }}/.docker"
-oreg_auth_credentials_replace: False
-openshift_docker_alternative_creds: "{{ openshift_use_crio_only | bool }}"
 
 containerized_svc_dir: "/usr/lib/systemd/system"
 ha_svc_template_path: "native-cluster"

roles/openshift_control_plane/tasks/registry_auth.yml

@@ -1,35 +1,19 @@
 ---
-- name: Check for credentials file for registry auth
-  stat:
-    path: "{{ oreg_auth_credentials_path }}"
-  when: oreg_auth_user is defined
-  register: master_oreg_auth_credentials_stat
-
-- name: Create credentials for registry auth
-  command: "docker --config={{ oreg_auth_credentials_path }} login -u {{ oreg_auth_user }} -p {{ oreg_auth_password }} {{ oreg_host }}"
-  when:
-  - not (openshift_docker_alternative_creds | default(False))
-  - oreg_auth_user is defined
-  - (not master_oreg_auth_credentials_stat.stat.exists or oreg_auth_credentials_replace) | bool
-  register: master_oreg_auth_credentials_create
-  retries: 3
-  delay: 5
-  until: master_oreg_auth_credentials_create.rc == 0
-  notify: restart master
-
 # docker_creds is a custom module from lib_utils
 # 'docker login' requires a docker.service running on the local host, this is an
-# alternative implementation for non-docker hosts.  This implementation does not
-# check the registry to determine whether or not the credentials will work.
+# alternative implementation that operates directly on config.json
 - name: Create credentials for registry auth (alternative)
   docker_creds:
     path: "{{ oreg_auth_credentials_path }}"
     registry: "{{ oreg_host }}"
     username: "{{ oreg_auth_user }}"
     password: "{{ oreg_auth_password }}"
+    # Test that we can actually connect with provided info
+    test_login: "{{ oreg_test_login | default(True) }}"
   when:
-  - openshift_docker_alternative_creds | default(False) | bool
   - oreg_auth_user is defined
-  - (not master_oreg_auth_credentials_stat.stat.exists or oreg_auth_credentials_replace) | bool
-  register: master_oreg_auth_credentials_create_alt
+  register: master_oreg_auth_credentials_create
   notify: restart master
+  retries: 3
+  delay: 5
+  until: master_oreg_auth_credentials_create.rc == 0

roles/openshift_facts/defaults/main.yml

@@ -10,6 +10,9 @@ l_openshift_images_dict:
   origin: 'docker.io/openshift/origin-${component}:${version}'
   openshift-enterprise: 'registry.access.redhat.com/openshift3/ose-${component}:${version}'
 
+# oreg_url is defined by user input.
+oreg_host: "{{ oreg_url.split('/')[0] if (oreg_url is defined and '.' in oreg_url.split('/')[0]) else '' }}"
+
 l_osm_registry_url_default: "{{ l_openshift_images_dict[openshift_deployment_type] }}"
 l_osm_registry_url: "{{ oreg_url_master | default(oreg_url) | default(l_osm_registry_url_default) | regex_replace('${version}' | regex_escape, openshift_image_tag | default('${version}')) }}"
 l_os_registry_url: "{{ oreg_url | default(l_osm_registry_url_default) | regex_replace('${version}' | regex_escape, openshift_image_tag | default('${version}')) }}"

roles/openshift_node/defaults/main.yml

@@ -148,13 +148,10 @@ default_r_openshift_node_os_firewall_allow:
 r_openshift_node_os_firewall_allow: "{{ default_r_openshift_node_os_firewall_allow | union(openshift_node_open_ports | default([])) }}"
 
 # oreg_url is defined by user input
-oreg_host: "{{ oreg_url.split('/')[0] if (oreg_url is defined and '.' in oreg_url.split('/')[0]) else '' }}"
 oreg_auth_credentials_path: "{{ openshift_node_data_dir }}/.docker"
-oreg_auth_credentials_replace: False
 l_bind_docker_reg_auth: False
 openshift_use_crio: False
 l_crio_var_sock: "/var/run/crio/crio.sock"
-openshift_docker_alternative_creds: "{{ openshift_use_crio_only | bool }}"
 
 openshift_docker_service_name: "docker"
 

roles/openshift_node/tasks/registry_auth.yml

@@ -1,36 +1,29 @@
 ---
+# There might be other settings in this file besides auth; we want to ensure it
+# will always be bind-mounted into the node for system containers (atomic).
 - name: Check for credentials file for registry auth
   stat:
     path: "{{ oreg_auth_credentials_path }}"
   when: oreg_auth_user is defined
   register: node_oreg_auth_credentials_stat
 
-- name: Create credentials for registry auth
-  command: "docker --config={{ oreg_auth_credentials_path }} login -u {{ oreg_auth_user }} -p {{ oreg_auth_password }} {{ oreg_host }}"
-  when:
-    - not (openshift_docker_alternative_creds | default(False))
-    - oreg_auth_user is defined
-    - (not node_oreg_auth_credentials_stat.stat.exists or oreg_auth_credentials_replace) | bool
-  register: node_oreg_auth_credentials_create
-  retries: 3
-  delay: 5
-  until: node_oreg_auth_credentials_create.rc == 0
-
 # docker_creds is a custom module from lib_utils
 # 'docker login' requires a docker.service running on the local host, this is an
-# alternative implementation for non-docker hosts.  This implementation does not
-# check the registry to determine whether or not the credentials will work.
-- name: Create credentials for registry auth (alternative)
+# alternative implementation that operates directly on config.json
+- name: Create credentials for registry auth
   docker_creds:
     path: "{{ oreg_auth_credentials_path }}"
     registry: "{{ oreg_host }}"
     username: "{{ oreg_auth_user }}"
     password: "{{ oreg_auth_password }}"
+    # Test that we can actually connect with provided info
+    test_login: "{{ oreg_test_login | default(True) }}"
   when:
-    - openshift_docker_alternative_creds | bool
     - oreg_auth_user is defined
-    - (not node_oreg_auth_credentials_stat.stat.exists or oreg_auth_credentials_replace) | bool
-  register: node_oreg_auth_credentials_create_alt
+  register: node_oreg_auth_credentials_create
+  retries: 3
+  delay: 5
+  until: node_oreg_auth_credentials_create.rc == 0
 
 # Container images may need the registry credentials
 - name: Setup ro mount of /root/.docker for containerized hosts
@@ -41,6 +34,4 @@
     - oreg_auth_user is defined
     - >
         (node_oreg_auth_credentials_stat.stat.exists
-        or oreg_auth_credentials_replace
-        or node_oreg_auth_credentials_create.changed
-        or node_oreg_auth_credentials_create_alt.changed) | bool
+        or node_oreg_auth_credentials_create.changed) | bool