From 2a7749333c6c25d97ed904fbf8207687e1f02955 Mon Sep 17 00:00:00 2001 From: James Page Date: Mon, 11 Jun 2018 15:25:18 +0100 Subject: [PATCH] Resolve issues with long interface names Resync charm helpers to pickup fixes to support veth wiring of OVS bridges directly to Linux bridges created with MAAS. Linux has a maximum interface name length of 15 charms; the change ensures that the generated interface length for the veth pair fits within this limitation. Previous behaviour (that worked) is preserved to avoid duplicate veth wiring or direct wiring of the Linux bridge to OVS. Change-Id: I294dc7c05b7c08503ef200707e0f9b1d1c199843 Closes-Bug: 1773353 --- .../charmhelpers/contrib/hahelpers/cluster.py | 5 + .../contrib/network/ovs/__init__.py | 57 ++++- .../contrib/openstack/amulet/utils.py | 12 +- .../contrib/openstack/cert_utils.py | 227 ++++++++++++++++++ .../charmhelpers/contrib/openstack/context.py | 51 ++-- hooks/charmhelpers/contrib/openstack/ip.py | 10 + .../contrib/openstack/templates/haproxy.cfg | 1 - hooks/charmhelpers/core/hookenv.py | 7 + .../charmhelpers/contrib/amulet/deployment.py | 6 +- .../contrib/openstack/amulet/utils.py | 12 +- tests/charmhelpers/core/hookenv.py | 7 + 11 files changed, 359 insertions(+), 36 deletions(-) create mode 100644 hooks/charmhelpers/contrib/openstack/cert_utils.py diff --git a/hooks/charmhelpers/contrib/hahelpers/cluster.py b/hooks/charmhelpers/contrib/hahelpers/cluster.py index 47facd91..4a737e24 100644 --- a/hooks/charmhelpers/contrib/hahelpers/cluster.py +++ b/hooks/charmhelpers/contrib/hahelpers/cluster.py @@ -223,6 +223,11 @@ def https(): return True if config_get('ssl_cert') and config_get('ssl_key'): return True + for r_id in relation_ids('certificates'): + for unit in relation_list(r_id): + ca = relation_get('ca', rid=r_id, unit=unit) + if ca: + return True for r_id in relation_ids('identity-service'): for unit in relation_list(r_id): # TODO - needs fixing for new helper as ssl_cert/key suffixes with CN diff --git a/hooks/charmhelpers/contrib/network/ovs/__init__.py b/hooks/charmhelpers/contrib/network/ovs/__init__.py index 9b3583f3..a8856e9c 100644 --- a/hooks/charmhelpers/contrib/network/ovs/__init__.py +++ b/hooks/charmhelpers/contrib/network/ovs/__init__.py @@ -13,6 +13,7 @@ # limitations under the License. ''' Helpers for interacting with OpenvSwitch ''' +import hashlib import subprocess import os import six @@ -39,6 +40,8 @@ iface {linuxbridge_port} inet manual down ip link del {linuxbridge_port} """ +MAX_KERNEL_INTERFACE_NAME_LEN = 15 + def add_bridge(name, datapath_type=None): ''' Add the named bridge to openvswitch ''' @@ -92,16 +95,39 @@ def add_ovsbridge_linuxbridge(name, bridge): apt_install('python3-netifaces', fatal=True) import netifaces + # NOTE(jamespage): + # Older code supported addition of a linuxbridge directly + # to an OVS bridge; ensure we don't break uses on upgrade + existing_ovs_bridge = port_to_br(bridge) + if existing_ovs_bridge is not None: + log('Linuxbridge {} is already directly in use' + ' by OVS bridge {}'.format(bridge, existing_ovs_bridge), + level=INFO) + return + + # NOTE(jamespage): + # preserve existing naming because interfaces may already exist. ovsbridge_port = "veth-" + name linuxbridge_port = "veth-" + bridge - log('Adding linuxbridge {} to ovsbridge {}'.format(bridge, name), - level=INFO) + if (len(ovsbridge_port) > MAX_KERNEL_INTERFACE_NAME_LEN or + len(linuxbridge_port) > MAX_KERNEL_INTERFACE_NAME_LEN): + # NOTE(jamespage): + # use parts of hashed bridgename (openstack style) when + # a bridge name exceeds 15 chars + hashed_bridge = hashlib.sha256(bridge.encode('UTF-8')).hexdigest() + base = '{}-{}'.format(hashed_bridge[:8], hashed_bridge[-2:]) + ovsbridge_port = "cvo{}".format(base) + linuxbridge_port = "cvb{}".format(base) + interfaces = netifaces.interfaces() for interface in interfaces: if interface == ovsbridge_port or interface == linuxbridge_port: log('Interface {} already exists'.format(interface), level=INFO) return + log('Adding linuxbridge {} to ovsbridge {}'.format(bridge, name), + level=INFO) + check_for_eni_source() with open('/etc/network/interfaces.d/{}.cfg'.format( @@ -134,6 +160,20 @@ def set_manager(manager): 'ssl:{}'.format(manager)]) +def set_Open_vSwitch_column_value(column_value): + """ + Calls ovs-vsctl and sets the 'column_value' in the Open_vSwitch table. + + :param column_value: + See http://www.openvswitch.org//ovs-vswitchd.conf.db.5.pdf for + details of the relevant values. + :type str + :raises CalledProcessException: possibly ovsdb-server is not running + """ + log('Setting {} in the Open_vSwitch table'.format(column_value)) + subprocess.check_call(['ovs-vsctl', 'set', 'Open_vSwitch', '.', column_value]) + + CERT_PATH = '/etc/openvswitch/ovsclient-cert.pem' @@ -194,3 +234,16 @@ def disable_ipfix(bridge): ''' cmd = ['ovs-vsctl', 'clear', 'Bridge', bridge, 'ipfix'] subprocess.check_call(cmd) + + +def port_to_br(port): + '''Determine the bridge that contains a port + :param port: Name of port to check for + :returns str: OVS bridge containing port or None if not found + ''' + try: + return subprocess.check_output( + ['ovs-vsctl', 'port-to-br', port] + ).decode('UTF-8').strip() + except subprocess.CalledProcessError: + return None diff --git a/hooks/charmhelpers/contrib/openstack/amulet/utils.py b/hooks/charmhelpers/contrib/openstack/amulet/utils.py index 84e87f5d..d43038b2 100644 --- a/hooks/charmhelpers/contrib/openstack/amulet/utils.py +++ b/hooks/charmhelpers/contrib/openstack/amulet/utils.py @@ -40,6 +40,7 @@ import novaclient import pika import swiftclient +from charmhelpers.core.decorators import retry_on_exception from charmhelpers.contrib.amulet.utils import ( AmuletUtils ) @@ -423,6 +424,7 @@ class OpenStackAmuletUtils(AmuletUtils): self.log.debug('Checking if tenant exists ({})...'.format(tenant)) return tenant in [t.name for t in keystone.tenants.list()] + @retry_on_exception(num_retries=5, base_delay=1) def keystone_wait_for_propagation(self, sentry_relation_pairs, api_version): """Iterate over list of sentry and relation tuples and verify that @@ -542,7 +544,7 @@ class OpenStackAmuletUtils(AmuletUtils): return ep def get_default_keystone_session(self, keystone_sentry, - openstack_release=None): + openstack_release=None, api_version=2): """Return a keystone session object and client object assuming standard default settings @@ -557,12 +559,12 @@ class OpenStackAmuletUtils(AmuletUtils): eyc """ self.log.debug('Authenticating keystone admin...') - api_version = 2 - client_class = keystone_client.Client # 11 => xenial_queens - if openstack_release and openstack_release >= 11: - api_version = 3 + if api_version == 3 or (openstack_release and openstack_release >= 11): client_class = keystone_client_v3.Client + api_version = 3 + else: + client_class = keystone_client.Client keystone_ip = keystone_sentry.info['public-address'] session, auth = self.get_keystone_session( keystone_ip, diff --git a/hooks/charmhelpers/contrib/openstack/cert_utils.py b/hooks/charmhelpers/contrib/openstack/cert_utils.py new file mode 100644 index 00000000..de853b53 --- /dev/null +++ b/hooks/charmhelpers/contrib/openstack/cert_utils.py @@ -0,0 +1,227 @@ +# Copyright 2014-2018 Canonical Limited. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Common python helper functions used for OpenStack charm certificats. + +import os +import json + +from charmhelpers.contrib.network.ip import ( + get_hostname, + resolve_network_cidr, +) +from charmhelpers.core.hookenv import ( + local_unit, + network_get_primary_address, + config, + relation_get, + unit_get, + NoNetworkBinding, + log, + WARNING, +) +from charmhelpers.contrib.openstack.ip import ( + ADMIN, + resolve_address, + get_vip_in_network, + INTERNAL, + PUBLIC, + ADDRESS_MAP) + +from charmhelpers.core.host import ( + mkdir, + write_file, +) + +from charmhelpers.contrib.hahelpers.apache import ( + install_ca_cert +) + + +class CertRequest(object): + + """Create a request for certificates to be generated + """ + + def __init__(self, json_encode=True): + self.entries = [] + self.hostname_entry = None + self.json_encode = json_encode + + def add_entry(self, net_type, cn, addresses): + """Add a request to the batch + + :param net_type: str netwrok space name request is for + :param cn: str Canonical Name for certificate + :param addresses: [] List of addresses to be used as SANs + """ + self.entries.append({ + 'cn': cn, + 'addresses': addresses}) + + def add_hostname_cn(self): + """Add a request for the hostname of the machine""" + ip = unit_get('private-address') + addresses = [ip] + # If a vip is being used without os-hostname config or + # network spaces then we need to ensure the local units + # cert has the approriate vip in the SAN list + vip = get_vip_in_network(resolve_network_cidr(ip)) + if vip: + addresses.append(vip) + self.hostname_entry = { + 'cn': get_hostname(ip), + 'addresses': addresses} + + def add_hostname_cn_ip(self, addresses): + """Add an address to the SAN list for the hostname request + + :param addr: [] List of address to be added + """ + for addr in addresses: + if addr not in self.hostname_entry['addresses']: + self.hostname_entry['addresses'].append(addr) + + def get_request(self): + """Generate request from the batched up entries + + """ + if self.hostname_entry: + self.entries.append(self.hostname_entry) + request = {} + for entry in self.entries: + sans = sorted(list(set(entry['addresses']))) + request[entry['cn']] = {'sans': sans} + if self.json_encode: + return {'cert_requests': json.dumps(request, sort_keys=True)} + else: + return {'cert_requests': request} + + +def get_certificate_request(json_encode=True): + """Generate a certificatee requests based on the network confioguration + + """ + req = CertRequest(json_encode=json_encode) + req.add_hostname_cn() + # Add os-hostname entries + for net_type in [INTERNAL, ADMIN, PUBLIC]: + net_config = config(ADDRESS_MAP[net_type]['override']) + try: + net_addr = resolve_address(endpoint_type=net_type) + ip = network_get_primary_address( + ADDRESS_MAP[net_type]['binding']) + addresses = [net_addr, ip] + vip = get_vip_in_network(resolve_network_cidr(ip)) + if vip: + addresses.append(vip) + if net_config: + req.add_entry( + net_type, + net_config, + addresses) + else: + # There is network address with no corresponding hostname. + # Add the ip to the hostname cert to allow for this. + req.add_hostname_cn_ip(addresses) + except NoNetworkBinding: + log("Skipping request for certificate for ip in {} space, no " + "local address found".format(net_type), WARNING) + return req.get_request() + + +def create_ip_cert_links(ssl_dir, custom_hostname_link=None): + """Create symlinks for SAN records + + :param ssl_dir: str Directory to create symlinks in + :param custom_hostname_link: str Additional link to be created + """ + hostname = get_hostname(unit_get('private-address')) + hostname_cert = os.path.join( + ssl_dir, + 'cert_{}'.format(hostname)) + hostname_key = os.path.join( + ssl_dir, + 'key_{}'.format(hostname)) + # Add links to hostname cert, used if os-hostname vars not set + for net_type in [INTERNAL, ADMIN, PUBLIC]: + try: + addr = resolve_address(endpoint_type=net_type) + cert = os.path.join(ssl_dir, 'cert_{}'.format(addr)) + key = os.path.join(ssl_dir, 'key_{}'.format(addr)) + if os.path.isfile(hostname_cert) and not os.path.isfile(cert): + os.symlink(hostname_cert, cert) + os.symlink(hostname_key, key) + except NoNetworkBinding: + log("Skipping creating cert symlink for ip in {} space, no " + "local address found".format(net_type), WARNING) + if custom_hostname_link: + custom_cert = os.path.join( + ssl_dir, + 'cert_{}'.format(custom_hostname_link)) + custom_key = os.path.join( + ssl_dir, + 'key_{}'.format(custom_hostname_link)) + if os.path.isfile(hostname_cert) and not os.path.isfile(custom_cert): + os.symlink(hostname_cert, custom_cert) + os.symlink(hostname_key, custom_key) + + +def install_certs(ssl_dir, certs, chain=None): + """Install the certs passed into the ssl dir and append the chain if + provided. + + :param ssl_dir: str Directory to create symlinks in + :param certs: {} {'cn': {'cert': 'CERT', 'key': 'KEY'}} + :param chain: str Chain to be appended to certs + """ + for cn, bundle in certs.items(): + cert_filename = 'cert_{}'.format(cn) + key_filename = 'key_{}'.format(cn) + cert_data = bundle['cert'] + if chain: + # Append chain file so that clients that trust the root CA will + # trust certs signed by an intermediate in the chain + cert_data = cert_data + chain + write_file( + path=os.path.join(ssl_dir, cert_filename), + content=cert_data, perms=0o640) + write_file( + path=os.path.join(ssl_dir, key_filename), + content=bundle['key'], perms=0o640) + + +def process_certificates(service_name, relation_id, unit, + custom_hostname_link=None): + """Process the certificates supplied down the relation + + :param service_name: str Name of service the certifcates are for. + :param relation_id: str Relation id providing the certs + :param unit: str Unit providing the certs + :param custom_hostname_link: str Name of custom link to create + """ + data = relation_get(rid=relation_id, unit=unit) + ssl_dir = os.path.join('/etc/apache2/ssl/', service_name) + mkdir(path=ssl_dir) + name = local_unit().replace('/', '_') + certs = data.get('{}.processed_requests'.format(name)) + chain = data.get('chain') + ca = data.get('ca') + if certs: + certs = json.loads(certs) + install_ca_cert(ca.encode()) + install_certs(ssl_dir, certs, chain) + create_ip_cert_links( + ssl_dir, + custom_hostname_link=custom_hostname_link) diff --git a/hooks/charmhelpers/contrib/openstack/context.py b/hooks/charmhelpers/contrib/openstack/context.py index 2d91f0a7..b196d63f 100644 --- a/hooks/charmhelpers/contrib/openstack/context.py +++ b/hooks/charmhelpers/contrib/openstack/context.py @@ -789,17 +789,18 @@ class ApacheSSLContext(OSContextGenerator): ssl_dir = os.path.join('/etc/apache2/ssl/', self.service_namespace) mkdir(path=ssl_dir) cert, key = get_cert(cn) - if cn: - cert_filename = 'cert_{}'.format(cn) - key_filename = 'key_{}'.format(cn) - else: - cert_filename = 'cert' - key_filename = 'key' + if cert and key: + if cn: + cert_filename = 'cert_{}'.format(cn) + key_filename = 'key_{}'.format(cn) + else: + cert_filename = 'cert' + key_filename = 'key' - write_file(path=os.path.join(ssl_dir, cert_filename), - content=b64decode(cert), perms=0o640) - write_file(path=os.path.join(ssl_dir, key_filename), - content=b64decode(key), perms=0o640) + write_file(path=os.path.join(ssl_dir, cert_filename), + content=b64decode(cert), perms=0o640) + write_file(path=os.path.join(ssl_dir, key_filename), + content=b64decode(key), perms=0o640) def configure_ca(self): ca_cert = get_ca_cert() @@ -871,23 +872,31 @@ class ApacheSSLContext(OSContextGenerator): if not self.external_ports or not https(): return {} - self.configure_ca() + use_keystone_ca = True + for rid in relation_ids('certificates'): + if related_units(rid): + use_keystone_ca = False + + if use_keystone_ca: + self.configure_ca() + self.enable_modules() ctxt = {'namespace': self.service_namespace, 'endpoints': [], 'ext_ports': []} - cns = self.canonical_names() - if cns: - for cn in cns: - self.configure_cert(cn) - else: - # Expect cert/key provided in config (currently assumed that ca - # uses ip for cn) - for net_type in (INTERNAL, ADMIN, PUBLIC): - cn = resolve_address(endpoint_type=net_type) - self.configure_cert(cn) + if use_keystone_ca: + cns = self.canonical_names() + if cns: + for cn in cns: + self.configure_cert(cn) + else: + # Expect cert/key provided in config (currently assumed that ca + # uses ip for cn) + for net_type in (INTERNAL, ADMIN, PUBLIC): + cn = resolve_address(endpoint_type=net_type) + self.configure_cert(cn) addresses = self.get_network_addresses() for address, endpoint in addresses: diff --git a/hooks/charmhelpers/contrib/openstack/ip.py b/hooks/charmhelpers/contrib/openstack/ip.py index d1476b1a..73102af7 100644 --- a/hooks/charmhelpers/contrib/openstack/ip.py +++ b/hooks/charmhelpers/contrib/openstack/ip.py @@ -184,3 +184,13 @@ def resolve_address(endpoint_type=PUBLIC, override=True): "clustered=%s)" % (net_type, clustered)) return resolved_address + + +def get_vip_in_network(network): + matching_vip = None + vips = config('vip') + if vips: + for vip in vips.split(): + if is_address_in_network(network, vip): + matching_vip = vip + return matching_vip diff --git a/hooks/charmhelpers/contrib/openstack/templates/haproxy.cfg b/hooks/charmhelpers/contrib/openstack/templates/haproxy.cfg index f99d99f1..d36af2aa 100644 --- a/hooks/charmhelpers/contrib/openstack/templates/haproxy.cfg +++ b/hooks/charmhelpers/contrib/openstack/templates/haproxy.cfg @@ -6,7 +6,6 @@ global group haproxy spread-checks 0 stats socket /var/run/haproxy/admin.sock mode 600 level admin - stats socket /var/run/haproxy/operator.sock mode 600 level operator stats timeout 2m defaults diff --git a/hooks/charmhelpers/core/hookenv.py b/hooks/charmhelpers/core/hookenv.py index 627d8f79..ed7af39e 100644 --- a/hooks/charmhelpers/core/hookenv.py +++ b/hooks/charmhelpers/core/hookenv.py @@ -972,6 +972,13 @@ def application_version_set(version): log("Application Version: {}".format(version)) +@translate_exc(from_exc=OSError, to_exc=NotImplementedError) +def goal_state(): + """Juju goal state values""" + cmd = ['goal-state', '--format=json'] + return json.loads(subprocess.check_output(cmd).decode('UTF-8')) + + @translate_exc(from_exc=OSError, to_exc=NotImplementedError) def is_leader(): """Does the current unit hold the juju leadership diff --git a/tests/charmhelpers/contrib/amulet/deployment.py b/tests/charmhelpers/contrib/amulet/deployment.py index 9c65518e..d21d01d8 100644 --- a/tests/charmhelpers/contrib/amulet/deployment.py +++ b/tests/charmhelpers/contrib/amulet/deployment.py @@ -50,7 +50,8 @@ class AmuletDeployment(object): this_service['units'] = 1 self.d.add(this_service['name'], units=this_service['units'], - constraints=this_service.get('constraints')) + constraints=this_service.get('constraints'), + storage=this_service.get('storage')) for svc in other_services: if 'location' in svc: @@ -64,7 +65,8 @@ class AmuletDeployment(object): svc['units'] = 1 self.d.add(svc['name'], charm=branch_location, units=svc['units'], - constraints=svc.get('constraints')) + constraints=svc.get('constraints'), + storage=svc.get('storage')) def _add_relations(self, relations): """Add all of the relations for the services.""" diff --git a/tests/charmhelpers/contrib/openstack/amulet/utils.py b/tests/charmhelpers/contrib/openstack/amulet/utils.py index 84e87f5d..d43038b2 100644 --- a/tests/charmhelpers/contrib/openstack/amulet/utils.py +++ b/tests/charmhelpers/contrib/openstack/amulet/utils.py @@ -40,6 +40,7 @@ import novaclient import pika import swiftclient +from charmhelpers.core.decorators import retry_on_exception from charmhelpers.contrib.amulet.utils import ( AmuletUtils ) @@ -423,6 +424,7 @@ class OpenStackAmuletUtils(AmuletUtils): self.log.debug('Checking if tenant exists ({})...'.format(tenant)) return tenant in [t.name for t in keystone.tenants.list()] + @retry_on_exception(num_retries=5, base_delay=1) def keystone_wait_for_propagation(self, sentry_relation_pairs, api_version): """Iterate over list of sentry and relation tuples and verify that @@ -542,7 +544,7 @@ class OpenStackAmuletUtils(AmuletUtils): return ep def get_default_keystone_session(self, keystone_sentry, - openstack_release=None): + openstack_release=None, api_version=2): """Return a keystone session object and client object assuming standard default settings @@ -557,12 +559,12 @@ class OpenStackAmuletUtils(AmuletUtils): eyc """ self.log.debug('Authenticating keystone admin...') - api_version = 2 - client_class = keystone_client.Client # 11 => xenial_queens - if openstack_release and openstack_release >= 11: - api_version = 3 + if api_version == 3 or (openstack_release and openstack_release >= 11): client_class = keystone_client_v3.Client + api_version = 3 + else: + client_class = keystone_client.Client keystone_ip = keystone_sentry.info['public-address'] session, auth = self.get_keystone_session( keystone_ip, diff --git a/tests/charmhelpers/core/hookenv.py b/tests/charmhelpers/core/hookenv.py index 627d8f79..ed7af39e 100644 --- a/tests/charmhelpers/core/hookenv.py +++ b/tests/charmhelpers/core/hookenv.py @@ -972,6 +972,13 @@ def application_version_set(version): log("Application Version: {}".format(version)) +@translate_exc(from_exc=OSError, to_exc=NotImplementedError) +def goal_state(): + """Juju goal state values""" + cmd = ['goal-state', '--format=json'] + return json.loads(subprocess.check_output(cmd).decode('UTF-8')) + + @translate_exc(from_exc=OSError, to_exc=NotImplementedError) def is_leader(): """Does the current unit hold the juju leadership