Merge pull request #881 from abutcher/namedCertRefactor

Refactor named certficates

filter_plugins/oo_filters.py

@@ -346,27 +346,27 @@ class FilterModule(object):
 
     @staticmethod
     # pylint: disable=too-many-branches
-    def oo_parse_certificate_names(certificates, data_dir, internal_hostnames):
+    def oo_parse_named_certificates(certificates, named_certs_dir, internal_hostnames):
         ''' Parses names from list of certificate hashes.
 
-            Ex: certificates = [{ "certfile": "/etc/origin/master/custom1.crt",
-                                  "keyfile": "/etc/origin/master/custom1.key" },
+            Ex: certificates = [{ "certfile": "/root/custom1.crt",
+                                  "keyfile": "/root/custom1.key" },
                                 { "certfile": "custom2.crt",
                                   "keyfile": "custom2.key" }]
 
-                returns [{ "certfile": "/etc/origin/master/custom1.crt",
-                           "keyfile": "/etc/origin/master/custom1.key",
+                returns [{ "certfile": "/etc/origin/master/named_certificates/custom1.crt",
+                           "keyfile": "/etc/origin/master/named_certificates/custom1.key",
                            "names": [ "public-master-host.com",
                                       "other-master-host.com" ] },
-                         { "certfile": "/etc/origin/master/custom2.crt",
-                           "keyfile": "/etc/origin/master/custom2.key",
+                         { "certfile": "/etc/origin/master/named_certificates/custom2.crt",
+                           "keyfile": "/etc/origin/master/named_certificates/custom2.key",
                            "names": [ "some-hostname.com" ] }]
         '''
         if not issubclass(type(certificates), list):
             raise errors.AnsibleFilterError("|failed expects certificates is a list")
 
-        if not issubclass(type(data_dir), unicode):
-            raise errors.AnsibleFilterError("|failed expects data_dir is unicode")
+        if not issubclass(type(named_certs_dir), unicode):
+            raise errors.AnsibleFilterError("|failed expects named_certs_dir is unicode")
 
         if not issubclass(type(internal_hostnames), list):
             raise errors.AnsibleFilterError("|failed expects internal_hostnames is list")
@@ -399,6 +399,11 @@ class FilterModule(object):
                 raise errors.AnsibleFilterError(("|failed to parse certificate '%s' or " % certificate['certfile'] +
                                                  "detected a collision with internal hostname, please specify " +
                                                  "certificate names in host inventory"))
+
+        for certificate in certificates:
+            # Update paths for configuration
+            certificate['certfile'] = os.path.join(named_certs_dir, os.path.basename(certificate['certfile']))
+            certificate['keyfile'] = os.path.join(named_certs_dir, os.path.basename(certificate['keyfile']))
         return certificates
 
     @staticmethod
@@ -474,7 +479,7 @@ class FilterModule(object):
             "oo_split": self.oo_split,
             "oo_filter_list": self.oo_filter_list,
             "oo_parse_heat_stack_outputs": self.oo_parse_heat_stack_outputs,
-            "oo_parse_certificate_names": self.oo_parse_certificate_names,
+            "oo_parse_named_certificates": self.oo_parse_named_certificates,
             "oo_haproxy_backend_masters": self.oo_haproxy_backend_masters,
             "oo_pretty_print_cluster": self.oo_pretty_print_cluster
         }

inventory/byo/hosts.example

@@ -111,8 +111,17 @@ openshift_master_identity_providers=[{'name': 'htpasswd_auth', 'login': 'true',
 # set RPM version for debugging purposes
 #openshift_pkg_version=-3.0.0.0
 
-# Configure custom master certificates
+# Configure custom named certificates
+# NOTE: openshift_master_named_certificates is cached on masters and is an
+# additive fact, meaning that each run with a different set of certificates
+# will add the newly provided certificates to the cached set of certificates.
+# If you would like openshift_master_named_certificates to be overwritten with
+# the provided value, specify openshift_master_overwrite_named_certificates.
+#openshift_master_overwrite_named_certificates: true
+#
+# Provide local certificate paths which will be deployed to masters
 #openshift_master_named_certificates=[{"certfile": "/path/to/custom1.crt", "keyfile": "/path/to/custom1.key"}]
+#
 # Detected names may be overridden by specifying the "names" key
 #openshift_master_named_certificates=[{"certfile": "/path/to/custom1.crt", "keyfile": "/path/to/custom1.key", "names": ["public-master-host.com"]}]
 

playbooks/common/openshift-master/config.yml

@@ -204,14 +204,6 @@
       validate_checksum: yes
     with_items: masters_needing_certs
 
-- name: Inspect named certificates
-  hosts: oo_first_master
-  tasks:
-  - name: Collect certificate names
-    set_fact:
-      parsed_named_certificates: "{{ openshift_master_named_certificates | oo_parse_certificate_names(master_cert_config_dir, openshift.common.internal_hostnames) }}"
-    when: openshift_master_named_certificates is defined
-
 - name: Compute haproxy_backend_servers
   hosts: localhost
   connection: local
@@ -272,11 +264,55 @@
                                       | map(attribute='stdout')
                                       | list) }}"
 
+- name: Parse named certificates
+  hosts: localhost
+  vars:
+    internal_hostnames: "{{ hostvars[groups.oo_first_master.0].openshift.common.internal_hostnames }}"
+    named_certificates: "{{ hostvars[groups.oo_first_master.0].openshift_master_named_certificates | default([]) }}"
+    named_certificates_dir: "{{ hostvars[groups.oo_first_master.0].master_cert_config_dir }}/named_certificates/"
+  tasks:
+  - set_fact:
+      parsed_named_certificates: "{{ named_certificates | oo_parse_named_certificates(named_certificates_dir, internal_hostnames) }}"
+    when: named_certificates | length > 0
+
+- name: Deploy named certificates
+  hosts: oo_masters_to_config
+  vars:
+    named_certs_dir: "{{ master_cert_config_dir }}/named_certificates/"
+    named_certs_specified: "{{ openshift_master_named_certificates is defined }}"
+    overwrite_named_certs: "{{ openshift_master_overwrite_named_certificates | default(false) }}"
+  roles:
+  - role: openshift_facts
+  post_tasks:
+  - openshift_facts:
+      role: master
+      local_facts:
+        named_certificates: "{{ hostvars.localhost.parsed_named_certificates | default([]) }}"
+      additive_facts_to_overwrite:
+      - "{{ 'master.named_certificates' if overwrite_named_certs | bool else omit }}"
+  - name: Clear named certificates
+    file:
+      path: "{{ named_certs_dir }}"
+      state: absent
+    when: overwrite_named_certs | bool
+  - name: Ensure named certificate directory exists
+    file:
+      path: "{{ named_certs_dir }}"
+      state: directory
+    when: named_certs_specified | bool
+  - name: Land named certificates
+    copy: src="{{ item.certfile }}" dest="{{ named_certs_dir }}"
+    with_items: openshift_master_named_certificates
+    when: named_certs_specified | bool
+  - name: Land named certificate keys
+    copy: src="{{ item.keyfile }}" dest="{{ named_certs_dir }}"
+    with_items: openshift_master_named_certificates
+    when: named_certs_specified | bool
+
 - name: Configure master instances
   hosts: oo_masters_to_config
   serial: 1
   vars:
-    named_certificates: "{{ hostvars[groups['oo_first_master'][0]]['parsed_named_certificates'] | default([])}}"
     sync_tmpdir: "{{ hostvars.localhost.g_master_mktemp.stdout }}"
     openshift_master_ha: "{{ groups.oo_masters_to_config | length > 1 }}"
     openshift_master_count: "{{ groups.oo_masters_to_config | length }}"

roles/openshift_facts/library/openshift_facts.py

@@ -864,20 +864,38 @@ def apply_provider_facts(facts, provider_facts):
     return facts
 
 
-def merge_facts(orig, new):
+def merge_facts(orig, new, additive_facts_to_overwrite):
     """ Recursively merge facts dicts
 
         Args:
             orig (dict): existing facts
             new (dict): facts to update
+
+            additive_facts_to_overwrite (list): additive facts to overwrite in jinja
+                                                '.' notation ex: ['master.named_certificates']
+
         Returns:
             dict: the merged facts
     """
+    additive_facts = ['named_certificates']
     facts = dict()
     for key, value in orig.iteritems():
         if key in new:
             if isinstance(value, dict) and isinstance(new[key], dict):
-                facts[key] = merge_facts(value, new[key])
+                relevant_additive_facts = []
+                # Keep additive_facts_to_overwrite if key matches
+                for item in additive_facts_to_overwrite:
+                    if '.' in item and item.startswith(key + '.'):
+                        relevant_additive_facts.append(item)
+                facts[key] = merge_facts(value, new[key], relevant_additive_facts)
+            elif key in additive_facts and key not in [x.split('.')[-1] for x in additive_facts_to_overwrite]:
+                # Fact is additive so we'll combine orig and new.
+                if isinstance(value, list) and isinstance(new[key], list):
+                    new_fact = []
+                    for item in copy.deepcopy(value) + copy.copy(new[key]):
+                        if item not in new_fact:
+                            new_fact.append(item)
+                    facts[key] = new_fact
             else:
                 facts[key] = copy.copy(new[key])
         else:
@@ -961,13 +979,15 @@ class OpenShiftFacts(object):
             role (str): role for setting local facts
             filename (str): local facts file to use
             local_facts (dict): local facts to set
+            additive_facts_to_overwrite (list): additive facts to overwrite in jinja
+                                                '.' notation ex: ['master.named_certificates']
 
         Raises:
             OpenShiftFactsUnsupportedRoleError:
     """
     known_roles = ['common', 'master', 'node', 'master_sdn', 'node_sdn', 'dns', 'etcd']
 
-    def __init__(self, role, filename, local_facts):
+    def __init__(self, role, filename, local_facts, additive_facts_to_overwrite=False):
         self.changed = False
         self.filename = filename
         if role not in self.known_roles:
@@ -976,25 +996,27 @@ class OpenShiftFacts(object):
             )
         self.role = role
         self.system_facts = ansible_facts(module)
-        self.facts = self.generate_facts(local_facts)
+        self.facts = self.generate_facts(local_facts, additive_facts_to_overwrite)
 
-    def generate_facts(self, local_facts):
+    def generate_facts(self, local_facts, additive_facts_to_overwrite):
         """ Generate facts
 
             Args:
                 local_facts (dict): local_facts for overriding generated
                                     defaults
+                additive_facts_to_overwrite (list): additive facts to overwrite in jinja
+                                                    '.' notation ex: ['master.named_certificates']
 
             Returns:
                 dict: The generated facts
         """
-        local_facts = self.init_local_facts(local_facts)
+        local_facts = self.init_local_facts(local_facts, additive_facts_to_overwrite)
         roles = local_facts.keys()
 
         defaults = self.get_defaults(roles)
         provider_facts = self.init_provider_facts()
         facts = apply_provider_facts(defaults, provider_facts)
-        facts = merge_facts(facts, local_facts)
+        facts = merge_facts(facts, local_facts, additive_facts_to_overwrite)
         facts['current_config'] = get_current_config(facts)
         facts = set_url_facts_if_unset(facts)
         facts = set_project_cfg_facts_if_unset(facts)
@@ -1132,11 +1154,13 @@ class OpenShiftFacts(object):
         )
         return provider_facts
 
-    def init_local_facts(self, facts=None):
+    def init_local_facts(self, facts=None, additive_facts_to_overwrite=False):
         """ Initialize the provider facts
 
             Args:
                 facts (dict): local facts to set
+                additive_facts_to_overwrite (list): additive facts to overwrite in jinja
+                                                    '.' notation ex: ['master.named_certificates']
 
             Returns:
                 dict: The result of merging the provided facts with existing
@@ -1154,7 +1178,7 @@ class OpenShiftFacts(object):
                                                   basestring):
                 facts_to_set[arg] = module.from_json(facts_to_set[arg])
 
-        new_local_facts = merge_facts(local_facts, facts_to_set)
+        new_local_facts = merge_facts(local_facts, facts_to_set, additive_facts_to_overwrite)
         for facts in new_local_facts.values():
             keys_to_delete = []
             for fact, value in facts.iteritems():
@@ -1184,6 +1208,7 @@ def main():
             role=dict(default='common', required=False,
                       choices=OpenShiftFacts.known_roles),
             local_facts=dict(default=None, type='dict', required=False),
+            additive_facts_to_overwrite=dict(default=[], type='list', required=False),
         ),
         supports_check_mode=True,
         add_file_common_args=True,
@@ -1191,9 +1216,10 @@ def main():
 
     role = module.params['role']
     local_facts = module.params['local_facts']
+    additive_facts_to_overwrite = module.params['additive_facts_to_overwrite']
     fact_file = '/etc/ansible/facts.d/openshift.fact'
 
-    openshift_facts = OpenShiftFacts(role, fact_file, local_facts)
+    openshift_facts = OpenShiftFacts(role, fact_file, local_facts, additive_facts_to_overwrite)
 
     file_params = module.params.copy()
     file_params['path'] = fact_file

roles/openshift_master/templates/master.yaml.v1.j2

@@ -27,9 +27,6 @@ corsAllowedOrigins:
 {% for custom_origin in openshift.master.custom_cors_origins | default("") %}
   - {{ custom_origin }}
 {% endfor %}
-{% for name in (named_certificates | map(attribute='names')) | list | oo_flatten %}
-  - {{ name }}
-{% endfor %}
 {% if 'disabled_features' in openshift.master %}
 disabledFeatures: {{ openshift.master.disabled_features | to_json }}
 {% endif %}
@@ -144,9 +141,9 @@ servingInfo:
   keyFile: master.server.key
   maxRequestsInFlight: 500
   requestTimeoutSeconds: 3600
-{% if named_certificates %}
+{% if openshift.master.named_certificates %}
   namedCertificates:
-{% for named_certificate in named_certificates %}
+{% for named_certificate in openshift.master.named_certificates %}
   - certFile: {{ named_certificate['certfile'] }}
     keyFile: {{ named_certificate['keyfile'] }}
     names: