Add master config filepath checking

When upgrading a cluster to 3.10, it's important that
all config files need to be present in /etc/origin/master
otherwise the pod will not be able to access those files
due to only a few directories are mounted into the pod.

This commit adds checking of the master's config files
to warn the user of any files we can't migrate for them
and instructs them to move their files.

playbooks/init/basic_facts.yml

@@ -70,6 +70,15 @@
         - l_atomic_docker_version.stdout | replace('"', '') is version_compare('1.12','>=')
         msg: Installation on Atomic Host requires Docker 1.12 or later. Please upgrade and restart the Atomic Host.
 
+- name: Retrieve existing master configs and validate
+  hosts: oo_masters_to_config
+  gather_facts: no
+  any_errors_fatal: true
+  tasks:
+  - import_role:
+      name: openshift_control_plane
+      tasks_from: check_existing_config.yml
+
 - name: Initialize special first-master variables
   hosts: oo_first_master
   roles:
@@ -77,36 +86,15 @@
   tasks:
   - when: not (osm_default_node_selector is defined)
     block:
-    - stat:
-        path: "/etc/origin/master/master-config.yaml"
-      register: master_config_path_check
-
-    - slurp:
-        src: "/etc/origin/master/master-config.yaml"
-      register: openshift_master_config_encoded_contents
-      when: master_config_path_check.stat.exists
-
     - set_fact:
+        # l_existing_config_master_config is set in openshift_control_plane/tasks/check_existing_config.yml
         openshift_master_config_node_selector: "{{ l_existing_config_master_config.projectConfig.defaultNodeSelector }}"
-      vars:
-        l_existing_config_master_config: "{{ (openshift_master_config_encoded_contents.content | b64decode | from_yaml) }}"
       when:
-      - master_config_path_check.stat.exists
+      - l_existing_config_master_config is defined
       - l_existing_config_master_config.projectConfig is defined
       - l_existing_config_master_config.projectConfig.defaultNodeSelector is defined
       - l_existing_config_master_config.projectConfig.defaultNodeSelector != ''
 
-    - set_fact:
-        # Take list of existing IDProviders so we can use/modify them later,
-        # especially for migrating htpasswd file.
-        openshift_master_existing_idproviders: "{{ l_existing_config_master_config.oauthConfig.identityProviders }}"
-      vars:
-        l_existing_config_master_config: "{{ (openshift_master_config_encoded_contents.content | b64decode | from_yaml) }}"
-      when:
-      - master_config_path_check.stat.exists
-      - l_existing_config_master_config.oauthConfig is defined
-      - l_existing_config_master_config.oauthConfig.identityProviders is defined
-
   - set_fact:
       # We need to setup openshift_client_binary here for special uses of delegate_to in
       # later roles and plays.

playbooks/openshift-master/private/config.yml

@@ -76,14 +76,14 @@
       local_facts:
         bootstrapped: true
 
-- name: Inspect state of first master config settings
+- name: Generate or retrieve existing session secrets
   hosts: oo_first_master
   roles:
   - role: openshift_facts
   tasks:
   - import_role:
       name: openshift_control_plane
-      tasks_from: check_existing_config
+      tasks_from: generate_session_secrets.yml
 
 - name: Configure masters
   hosts: oo_masters_to_config

roles/lib_utils/action_plugins/master_check_paths_in_config.py

@@ -0,0 +1,129 @@
+"""
+Ansible action plugin to ensure inventory variables are set
+appropriately and no conflicting options have been provided.
+"""
+import collections
+import six
+
+from ansible.plugins.action import ActionBase
+from ansible import errors
+
+
+FAIL_MSG = """A string value that appears to be a file path located outside of
+{} has been found in /etc/origin/master/master-config.yaml.
+In 3.10 and newer, all files needed by the master must reside inside of
+those directories or a subdirectory or it will not be readable by the
+master process. Please migrate all files needed by the master into
+one of {} or a subdirectory and update your master configs before
+proceeding. The string found was: {}
+***********************
+NOTE: the following items do not need to be migrated, they will be migrated
+for you: {}"""
+
+
+ITEMS_TO_POP = (
+    ('oauthConfig', 'identityProviders'),
+)
+# Create csv string of dot-separated dictionary keys:
+# eg: 'oathConfig.identityProviders, something.else.here'
+MIGRATED_ITEMS = ", ".join([".".join(x) for x in ITEMS_TO_POP])
+
+ALLOWED_DIRS = (
+    '/etc/origin/master/',
+    '/var/lib/origin',
+    '/etc/origin/cloudprovider',
+)
+
+ALLOWED_DIRS_STRING = ', '.join(ALLOWED_DIRS)
+
+
+def pop_migrated_fields(mastercfg):
+    """Some fields do not need to be searched because they will be migrated
+    for users automatically"""
+    # Walk down the tree and pop the specific item we migrate / don't care about
+    for item in ITEMS_TO_POP:
+        field = mastercfg
+        for sub_field in item:
+            parent_field = field
+            field = field[sub_field]
+        parent_field.pop(item[len(item) - 1])
+
+
+def do_item_check(val, strings_to_check):
+    """Check type of val, append to strings_to_check if string, otherwise if
+    it's a dictionary-like object call walk_mapping, if it's a list-like
+    object call walk_sequence, else ignore."""
+    if isinstance(val, six.string_types):
+        strings_to_check.append(val)
+    elif isinstance(val, collections.Sequence):
+        # A list-like object
+        walk_sequence(val, strings_to_check)
+    elif isinstance(val, collections.Mapping):
+        # A dictionary-like object
+        walk_mapping(val, strings_to_check)
+    # If it's not a string, list, or dictionary, we're not interested.
+
+
+def walk_sequence(items, strings_to_check):
+    """Walk recursively through a list, items"""
+    for item in items:
+        do_item_check(item, strings_to_check)
+
+
+def walk_mapping(map_to_walk, strings_to_check):
+    """Walk recursively through map_to_walk dictionary and add strings to
+    strings_to_check"""
+    for _, val in map_to_walk.items():
+        do_item_check(val, strings_to_check)
+
+
+def check_strings(strings_to_check):
+    """Check the strings we found to see if they look like file paths and if
+    they are, fail if not start with /etc/origin/master"""
+    for item in strings_to_check:
+        if item.startswith('/') or item.startswith('../'):
+            matches = 0
+            for allowed in ALLOWED_DIRS:
+                if item.startswith(allowed):
+                    matches += 1
+            if matches == 0:
+                raise errors.AnsibleModuleError(
+                    FAIL_MSG.format(ALLOWED_DIRS_STRING,
+                                    ALLOWED_DIRS_STRING,
+                                    item, MIGRATED_ITEMS))
+
+
+# pylint: disable=R0903
+class ActionModule(ActionBase):
+    """Action plugin to validate no files are needed by master that reside
+    outside of /etc/origin/master as masters will now run as pods and cannot
+    utilize files outside of that path as they will not be mounted inside the
+    containers."""
+    def run(self, tmp=None, task_vars=None):
+        """Run this action module"""
+        result = super(ActionModule, self).run(tmp, task_vars)
+
+        # self.task_vars holds all in-scope variables.
+        # Ignore settting self.task_vars outside of init.
+        # pylint: disable=W0201
+        self.task_vars = task_vars or {}
+
+        # mastercfg should be a dictionary from scraping an existing master's
+        # config yaml file.
+        mastercfg = self._task.args.get('mastercfg')
+
+        # We migrate some paths for users automatically, so we pop those.
+        pop_migrated_fields(mastercfg)
+
+        # Create an empty list to append strings from our config file to to check
+        # later.
+        strings_to_check = []
+
+        walk_mapping(mastercfg, strings_to_check)
+
+        check_strings(strings_to_check)
+
+        result["changed"] = False
+        result["failed"] = False
+        result["msg"] = "Aight, configs looking good"
+        return result

roles/lib_utils/test/test_master_check_paths_in_config.py

@@ -0,0 +1,82 @@
+'''
+ Unit tests for the master_check_paths_in_config action plugin
+'''
+import os
+import sys
+
+from ansible import errors
+import pytest
+
+
+MODULE_PATH = os.path.realpath(os.path.join(__file__, os.pardir, os.pardir, 'action_plugins'))
+sys.path.insert(1, MODULE_PATH)
+
+# pylint: disable=import-error,wrong-import-position,missing-docstring
+# pylint: disable=invalid-name,redefined-outer-name
+import master_check_paths_in_config  # noqa: E402
+
+
+@pytest.fixture()
+def loaded_config():
+    """return testing master config"""
+    data = {
+        'apiVersion': 'v1',
+        'oauthConfig':
+        {'identityProviders':
+            ['1', '2', '/this/will/fail']},
+        'fake_top_item':
+        {'fake_item':
+            {'fake_item2':
+                ["some string",
+                    {"fake_item3":
+                        ["some string 2",
+                            {"fake_item4":
+                                {"some_key": "deeply_nested_string"}}]}]}}
+    }
+    return data
+
+
+def test_pop_migrated(loaded_config):
+    """Params:
+
+    * `loaded_config` comes from the `loaded_config` fixture in this file
+    """
+    # Ensure we actually loaded a valid config
+    assert loaded_config['apiVersion'] == 'v1'
+
+    # Test that migrated key is removed
+    master_check_paths_in_config.pop_migrated_fields(loaded_config)
+    assert loaded_config['oauthConfig'] is not None
+    assert loaded_config['oauthConfig'].get('identityProviders') is None
+
+
+def test_walk_mapping(loaded_config):
+    """Params:
+    * `loaded_config` comes from the `loaded_config` fixture in this file
+    """
+    # Ensure we actually loaded a valid config
+    fake_top_item = loaded_config['fake_top_item']
+    stc = []
+    expected_keys = ("some string", "some string 2", "deeply_nested_string")
+
+    # Test that we actually extract all the strings from complicated nested
+    # structures
+    master_check_paths_in_config.walk_mapping(fake_top_item, stc)
+    assert len(stc) == 3
+    for item in expected_keys:
+        assert item in stc
+
+
+def test_check_strings():
+    stc_good = ('/etc/origin/master/good', 'some/child/dir')
+    # This should not raise
+    master_check_paths_in_config.check_strings(stc_good)
+
+    # This is a string we should alert on
+    stc_bad = ('goodfile.txt', '/root/somefile')
+    with pytest.raises(errors.AnsibleModuleError):
+        master_check_paths_in_config.check_strings(stc_bad)
+
+    stc_bad_relative = ('goodfile.txt', '../node/otherfile')
+    with pytest.raises(errors.AnsibleModuleError):
+        master_check_paths_in_config.check_strings(stc_bad_relative)

roles/openshift_control_plane/tasks/check_existing_config.yml

@@ -1,57 +1,32 @@
 ---
-- name: Check for existing configuration
-  stat:
-    path: /etc/origin/master/master-config.yaml
-  register: master_config_stat
-
-- name: Set clean install fact
-  set_fact:
-    l_clean_install: "{{ not master_config_stat.stat.exists | bool }}"
-
-- name: Determine if etcd3 storage is in use
-  command: grep  -Pzo  "storage-backend:\n.*etcd3" /etc/origin/master/master-config.yaml -q
-  register: etcd3_grep
-  failed_when: false
-  changed_when: false
-
-- name: Set etcd3 fact
-  set_fact:
-    l_etcd3_enabled: "{{ etcd3_grep.rc == 0 | bool }}"
-
-- name: Check if atomic-openshift-master sysconfig exists yet
-  stat:
-    path: /etc/sysconfig/atomic-openshift-master
-  register: l_aom_exists
-
-- name: Preserve OPENSHIFT_DEFAULT_REGISTRY master parameter if present
-  command: awk '/^OPENSHIFT_DEFAULT_REGISTRY/' /etc/sysconfig/atomic-openshift-master
-  register: l_default_registry_defined
-  when: l_aom_exists.stat.exists | bool
-
-- name: Check if atomic-openshift-master-api sysconfig exists yet
-  stat:
-    path: /etc/sysconfig/atomic-openshift-master-api
-  register: l_aom_api_exists
-
-- name: Preserve OPENSHIFT_DEFAULT_REGISTRY master-api parameter if present
-  command: awk '/^OPENSHIFT_DEFAULT_REGISTRY/' /etc/sysconfig/atomic-openshift-master-api
-  register: l_default_registry_defined_api
-  when: l_aom_api_exists.stat.exists | bool
-
-- name: Check if atomic-openshift-master-controllers sysconfig exists yet
-  stat:
-    path: /etc/sysconfig/atomic-openshift-master-controllers
-  register: l_aom_controllers_exists
-
-- name: Preserve OPENSHIFT_DEFAULT_REGISTRY master-controllers parameter if present
-  command: awk '/^OPENSHIFT_DEFAULT_REGISTRY/' /etc/sysconfig/atomic-openshift-master-controllers
-  register: l_default_registry_defined_controllers
-  when: l_aom_controllers_exists.stat.exists | bool
-
-- name: Update facts with OPENSHIFT_DEFAULT_REGISTRY value
-  set_fact:
-    l_default_registry_value: "{{ l_default_registry_defined.stdout | default('') }}"
-    l_default_registry_value_api: "{{ l_default_registry_defined_api.stdout | default('') }}"
-    l_default_registry_value_controllers: "{{ l_default_registry_defined_controllers.stdout | default('') }}"
-
-- import_tasks: generate_session_secrets.yml
+# We need to scrape existing config and check some items.
+- stat:
+    path: "/etc/origin/master/master-config.yaml"
+  register: master_config_path_check
+
+- slurp:
+    src: "/etc/origin/master/master-config.yaml"
+  register: openshift_master_config_encoded_contents
+  when: master_config_path_check.stat.exists
+
+# The existing config will also be used to determine first node selector on
+# first master.
+- set_fact:
+    l_existing_config_master_config: "{{ (openshift_master_config_encoded_contents.content | b64decode | from_yaml) }}"
+  when:
+  - openshift_master_config_encoded_contents is defined
+  - openshift_master_config_encoded_contents.content is defined
+
+- name: Check for file paths outside of /etc/origin/master in master's config
+  master_check_paths_in_config:
+    mastercfg: "{{ l_existing_config_master_config }}"
+  when: l_existing_config_master_config is defined
+
+- set_fact:
+    # Take list of existing IDProviders so we can use/modify them later,
+    # especially for migrating htpasswd file.
+    openshift_master_existing_idproviders: "{{ l_existing_config_master_config.oauthConfig.identityProviders }}"
+  when:
+  - l_existing_config_master_config is defined
+  - l_existing_config_master_config.oauthConfig is defined
+  - l_existing_config_master_config.oauthConfig.identityProviders is defined