From 03e6ae850ce718c008636bd8db093f453e62ccf3 Mon Sep 17 00:00:00 2001 From: Andrew Butcher Date: Thu, 12 Nov 2015 10:46:25 -0500 Subject: Refactor named certificates. --- filter_plugins/oo_filters.py | 24 ++++++---- inventory/byo/hosts.example | 11 ++++- playbooks/common/openshift-master/config.yml | 53 ++++++++++++++++++---- roles/openshift_facts/library/openshift_facts.py | 34 ++++++++++---- roles/openshift_master/templates/master.yaml.v1.j2 | 7 +-- 5 files changed, 94 insertions(+), 35 deletions(-) diff --git a/filter_plugins/oo_filters.py b/filter_plugins/oo_filters.py index 2386b5878..dc59d3c68 100644 --- a/filter_plugins/oo_filters.py +++ b/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,10 @@ 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")) + + # 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 +478,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 } diff --git a/inventory/byo/hosts.example b/inventory/byo/hosts.example index 56bbb9612..423581281 100644 --- a/inventory/byo/hosts.example +++ b/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"]}] diff --git a/playbooks/common/openshift-master/config.yml b/playbooks/common/openshift-master/config.yml index b1da85d5d..8719d080d 100644 --- a/playbooks/common/openshift-master/config.yml +++ b/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,54 @@ | 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([]) }}" + overwrite_additive_facts: "{{ overwrite_named_certs }}" + - 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 }}" diff --git a/roles/openshift_facts/library/openshift_facts.py b/roles/openshift_facts/library/openshift_facts.py index 091ba4e2b..995d2c5fb 100755 --- a/roles/openshift_facts/library/openshift_facts.py +++ b/roles/openshift_facts/library/openshift_facts.py @@ -864,20 +864,29 @@ def apply_provider_facts(facts, provider_facts): return facts -def merge_facts(orig, new): +def merge_facts(orig, new, overwrite_additive_facts): """ Recursively merge facts dicts Args: orig (dict): existing facts new (dict): facts to update + overwrite_additive_facts (bool): overwrite additive facts 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]) + facts[key] = merge_facts(value, new[key], overwrite_additive_facts) + elif key in additive_facts and not overwrite_additive_facts: + 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 +970,14 @@ class OpenShiftFacts(object): role (str): role for setting local facts filename (str): local facts file to use local_facts (dict): local facts to set + overwrite_additive_facts (bool): overwrite additive facts 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, overwrite_additive_facts=False): self.changed = False self.filename = filename if role not in self.known_roles: @@ -976,25 +986,26 @@ 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, overwrite_additive_facts) - def generate_facts(self, local_facts): + def generate_facts(self, local_facts, overwrite_additive_facts): """ Generate facts Args: local_facts (dict): local_facts for overriding generated defaults + overwrite_additive_facts (dict): overwrite additive facts Returns: dict: The generated facts """ - local_facts = self.init_local_facts(local_facts) + local_facts = self.init_local_facts(local_facts, overwrite_additive_facts) 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, overwrite_additive_facts) facts['current_config'] = get_current_config(facts) facts = set_url_facts_if_unset(facts) facts = set_project_cfg_facts_if_unset(facts) @@ -1132,11 +1143,12 @@ class OpenShiftFacts(object): ) return provider_facts - def init_local_facts(self, facts=None): + def init_local_facts(self, facts=None, overwrite_additive_facts=False): """ Initialize the provider facts Args: facts (dict): local facts to set + overwrite_additive_facts (bool): overwrite additive facts Returns: dict: The result of merging the provided facts with existing @@ -1154,7 +1166,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, overwrite_additive_facts) for facts in new_local_facts.values(): keys_to_delete = [] for fact, value in facts.iteritems(): @@ -1184,6 +1196,7 @@ def main(): role=dict(default='common', required=False, choices=OpenShiftFacts.known_roles), local_facts=dict(default=None, type='dict', required=False), + overwrite_additive_facts=dict(default=False, type='bool', required=False), ), supports_check_mode=True, add_file_common_args=True, @@ -1191,9 +1204,10 @@ def main(): role = module.params['role'] local_facts = module.params['local_facts'] + overwrite_additive_facts = module.params['overwrite_additive_facts'] fact_file = '/etc/ansible/facts.d/openshift.fact' - openshift_facts = OpenShiftFacts(role, fact_file, local_facts) + openshift_facts = OpenShiftFacts(role, fact_file, local_facts, overwrite_additive_facts) file_params = module.params.copy() file_params['path'] = fact_file diff --git a/roles/openshift_master/templates/master.yaml.v1.j2 b/roles/openshift_master/templates/master.yaml.v1.j2 index bb12a0a0f..2a37c06d9 100644 --- a/roles/openshift_master/templates/master.yaml.v1.j2 +++ b/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: -- cgit v1.2.3 From 927e585bbeb049523313bacedc57efee2eacf232 Mon Sep 17 00:00:00 2001 From: Andrew Butcher Date: Mon, 16 Nov 2015 16:01:35 -0500 Subject: Add additive_facts_to_overwrite instead of overwriting all additive_facts --- playbooks/common/openshift-master/config.yml | 3 +- roles/openshift_facts/library/openshift_facts.py | 46 +++++++++++++++--------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/playbooks/common/openshift-master/config.yml b/playbooks/common/openshift-master/config.yml index 8719d080d..ff1579218 100644 --- a/playbooks/common/openshift-master/config.yml +++ b/playbooks/common/openshift-master/config.yml @@ -288,7 +288,8 @@ role: master local_facts: named_certificates: "{{ hostvars.localhost.parsed_named_certificates | default([]) }}" - overwrite_additive_facts: "{{ overwrite_named_certs }}" + additive_facts_to_overwrite: + - "{{ 'master.named_certificates' if overwrite_named_certs | bool else omit }}" - name: Clear named certificates file: path: "{{ named_certs_dir }}" diff --git a/roles/openshift_facts/library/openshift_facts.py b/roles/openshift_facts/library/openshift_facts.py index 995d2c5fb..fc701d42b 100755 --- a/roles/openshift_facts/library/openshift_facts.py +++ b/roles/openshift_facts/library/openshift_facts.py @@ -864,13 +864,16 @@ def apply_provider_facts(facts, provider_facts): return facts -def merge_facts(orig, new, overwrite_additive_facts): +def merge_facts(orig, new, additive_facts_to_overwrite): """ Recursively merge facts dicts Args: orig (dict): existing facts new (dict): facts to update - overwrite_additive_facts (bool): overwrite additive facts + + additive_facts_to_overwrite (list): additive facts to overwrite in jinja + '.' notation ex: ['master.named_certificates'] + Returns: dict: the merged facts """ @@ -879,8 +882,14 @@ def merge_facts(orig, new, overwrite_additive_facts): 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], overwrite_additive_facts) - elif key in additive_facts and not overwrite_additive_facts: + 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]): @@ -970,14 +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 - overwrite_additive_facts (bool): overwrite additive facts + 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, overwrite_additive_facts=False): + 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: @@ -986,26 +996,27 @@ class OpenShiftFacts(object): ) self.role = role self.system_facts = ansible_facts(module) - self.facts = self.generate_facts(local_facts, overwrite_additive_facts) + self.facts = self.generate_facts(local_facts, additive_facts_to_overwrite) - def generate_facts(self, local_facts, overwrite_additive_facts): + def generate_facts(self, local_facts, additive_facts_to_overwrite): """ Generate facts Args: local_facts (dict): local_facts for overriding generated defaults - overwrite_additive_facts (dict): overwrite additive facts + 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, overwrite_additive_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, overwrite_additive_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) @@ -1143,12 +1154,13 @@ class OpenShiftFacts(object): ) return provider_facts - def init_local_facts(self, facts=None, overwrite_additive_facts=False): + def init_local_facts(self, facts=None, additive_facts_to_overwrite=False): """ Initialize the provider facts Args: facts (dict): local facts to set - overwrite_additive_facts (bool): overwrite additive facts + 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 @@ -1166,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, overwrite_additive_facts) + 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(): @@ -1196,7 +1208,7 @@ def main(): role=dict(default='common', required=False, choices=OpenShiftFacts.known_roles), local_facts=dict(default=None, type='dict', required=False), - overwrite_additive_facts=dict(default=False, type='bool', required=False), + additive_facts_to_overwrite=dict(default=[], type='list', required=False), ), supports_check_mode=True, add_file_common_args=True, @@ -1204,10 +1216,10 @@ def main(): role = module.params['role'] local_facts = module.params['local_facts'] - overwrite_additive_facts = module.params['overwrite_additive_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, overwrite_additive_facts) + openshift_facts = OpenShiftFacts(role, fact_file, local_facts, additive_facts_to_overwrite) file_params = module.params.copy() file_params['path'] = fact_file -- cgit v1.2.3 From dd5508bdb13af0c67aae49131b314d2c9443282c Mon Sep 17 00:00:00 2001 From: Andrew Butcher Date: Wed, 18 Nov 2015 08:58:14 -0500 Subject: Update certificate paths when 'names' key is provided. --- filter_plugins/oo_filters.py | 1 + 1 file changed, 1 insertion(+) diff --git a/filter_plugins/oo_filters.py b/filter_plugins/oo_filters.py index dc59d3c68..83f5cdcef 100644 --- a/filter_plugins/oo_filters.py +++ b/filter_plugins/oo_filters.py @@ -400,6 +400,7 @@ class FilterModule(object): "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'])) -- cgit v1.2.3