From a828774c4868c6c74fc69e6b0f2a1e690a64d5d9 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Sat, 23 Jan 2021 16:17:28 +0000 Subject: [PATCH 1/8] Handle change of CA cert. Closes issue #487 --- zaza/openstack/charm_tests/keystone/setup.py | 4 +- zaza/openstack/charm_tests/keystone/tests.py | 4 +- zaza/openstack/charm_tests/policyd/tests.py | 3 +- zaza/openstack/charm_tests/vault/setup.py | 4 +- zaza/openstack/charm_tests/vault/tests.py | 4 +- zaza/openstack/utilities/openstack.py | 82 +++++++++++++++++--- 6 files changed, 85 insertions(+), 16 deletions(-) diff --git a/zaza/openstack/charm_tests/keystone/setup.py b/zaza/openstack/charm_tests/keystone/setup.py index 748439f..8d64993 100644 --- a/zaza/openstack/charm_tests/keystone/setup.py +++ b/zaza/openstack/charm_tests/keystone/setup.py @@ -41,9 +41,11 @@ def wait_for_cacert(model_name=None): :type model_name: str """ logging.info("Waiting for cacert") + cert_file = openstack_utils.get_cert_file_name( + 'keystone') zaza.model.block_until_file_has_contents( 'keystone', - openstack_utils.KEYSTONE_REMOTE_CACERT, + cert_file, 'CERTIFICATE', model_name=model_name) zaza.model.block_until_all_units_idle(model_name=model_name) diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py index b2830ef..477152d 100644 --- a/zaza/openstack/charm_tests/keystone/tests.py +++ b/zaza/openstack/charm_tests/keystone/tests.py @@ -229,7 +229,7 @@ class AuthenticationAuthorizationTest(BaseKeystoneTest): 'OS_DOMAIN_NAME': DEMO_DOMAIN, } if self.tls_rid: - openrc['OS_CACERT'] = openstack_utils.KEYSTONE_LOCAL_CACERT + openrc['OS_CACERT'] = openstack_utils.get_cacert() openrc['OS_AUTH_URL'] = ( openrc['OS_AUTH_URL'].replace('http', 'https')) logging.info('keystone IP {}'.format(ip)) @@ -259,7 +259,7 @@ class AuthenticationAuthorizationTest(BaseKeystoneTest): """ def _validate_token_data(openrc): if self.tls_rid: - openrc['OS_CACERT'] = openstack_utils.KEYSTONE_LOCAL_CACERT + openrc['OS_CACERT'] = openstack_utils.get_cacert() openrc['OS_AUTH_URL'] = ( openrc['OS_AUTH_URL'].replace('http', 'https')) logging.info('keystone IP {}'.format(ip)) diff --git a/zaza/openstack/charm_tests/policyd/tests.py b/zaza/openstack/charm_tests/policyd/tests.py index eca316d..4722755 100644 --- a/zaza/openstack/charm_tests/policyd/tests.py +++ b/zaza/openstack/charm_tests/policyd/tests.py @@ -337,8 +337,7 @@ class BasePolicydSpecialization(PolicydTest, logging.info('Authentication for {} on keystone IP {}' .format(openrc['OS_USERNAME'], ip)) if self.tls_rid: - openrc['OS_CACERT'] = \ - openstack_utils.KEYSTONE_LOCAL_CACERT + openrc['OS_CACERT'] = openstack_utils.get_cacert() openrc['OS_AUTH_URL'] = ( openrc['OS_AUTH_URL'].replace('http', 'https')) logging.info('keystone IP {}'.format(ip)) diff --git a/zaza/openstack/charm_tests/vault/setup.py b/zaza/openstack/charm_tests/vault/setup.py index 1e4e05e..d605063 100644 --- a/zaza/openstack/charm_tests/vault/setup.py +++ b/zaza/openstack/charm_tests/vault/setup.py @@ -222,9 +222,11 @@ def validate_ca(cacertificate, application="keystone", port=5000): :returns: None :rtype: None """ + cert_file = zaza.openstack.utilities.openstack.get_cert_file_name( + application) zaza.model.block_until_file_has_contents( application, - zaza.openstack.utilities.openstack.KEYSTONE_REMOTE_CACERT, + cert_file, cacertificate.decode().strip()) vip = (zaza.model.get_application_config(application) .get("vip").get("value")) diff --git a/zaza/openstack/charm_tests/vault/tests.py b/zaza/openstack/charm_tests/vault/tests.py index 40227fd..e96f36a 100644 --- a/zaza/openstack/charm_tests/vault/tests.py +++ b/zaza/openstack/charm_tests/vault/tests.py @@ -154,9 +154,11 @@ class VaultTest(BaseVaultTest): test_config = lifecycle_utils.get_charm_config() del test_config['target_deploy_status']['vault'] + cert_file = zaza.openstack.utilities.openstack.get_cert_file_name( + 'keystone') zaza.model.block_until_file_has_contents( 'keystone', - zaza.openstack.utilities.openstack.KEYSTONE_REMOTE_CACERT, + cert_file, cacert.decode().strip()) zaza.model.wait_for_application_states( states=test_config.get('target_deploy_status', {})) diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index 6e0fa20..b914c0f 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -69,6 +69,7 @@ from neutronclient.common import exceptions as neutronexceptions from octaviaclient.api.v2 import octavia as octaviaclient from swiftclient import client as swiftclient +from juju.errors import JujuError import zaza @@ -181,10 +182,68 @@ WORKLOAD_STATUS_EXCEPTIONS = { 'ceilometer and gnocchi')}} # For vault TLS certificates +LOCAL_CERT_DIR = "tests" KEYSTONE_CACERT = "keystone_juju_ca_cert.crt" KEYSTONE_REMOTE_CACERT = ( "/usr/local/share/ca-certificates/{}".format(KEYSTONE_CACERT)) -KEYSTONE_LOCAL_CACERT = ("tests/{}".format(KEYSTONE_CACERT)) +KEYSTONE_LOCAL_CACERT = ("{}/{}".format(LOCAL_CERT_DIR, KEYSTONE_CACERT)) + +VAULT_CACERT = "vault_juju_ca_cert.crt" +VAULT_REMOTE_CACERT = ( + "/usr/local/share/ca-certificates/{}".format(VAULT_CACERT)) +VAULT_LOCAL_CACERT = ("{}/{}".format(LOCAL_CERT_DIR, VAULT_CACERT)) + +REMOTE_CERTIFICATES = [VAULT_REMOTE_CACERT, KEYSTONE_REMOTE_CACERT] +LOCAL_CERTIFICATES = [VAULT_LOCAL_CACERT, KEYSTONE_LOCAL_CACERT] + + +async def async_get_cert_file_name(app, cert_files=None, block=True, + model_name=None, timeout=2700): + """Get the name of the CA cert file thats on all units of an application. + + :param app: Name of application + :type capp: str + :param cert_files: List of cert files to search for. + :type cert_files: List[str] + :param block: Whether to block until a consistent cert file is found. + :type block: bool + :param model_name: Name of model to run check in + :type model_name: str + :param timeout: Time to wait for consistent file + :type timeout: int + :returns: Credentials dictionary + :rtype: dict + """ + async def _check_for_file(model, cert_files): + units = model.applications[app].units + results = {u.entity_id: [] for u in units} + for unit in units: + try: + for cf in cert_files: + output = await unit.run('test -e "{}"; echo $?'.format(cf)) + contents = output.data.get('results')['Stdout'] + if "0" in contents: + results[unit.entity_id].append(cf) + except JujuError: + pass + for cert_file in cert_files: + # Check that the certificate file exists on all the units. + if all(cert_file in files for files in results.values()): + return cert_file + else: + return None + + if not cert_files: + cert_files = REMOTE_CERTIFICATES + cert_file = None + async with zaza.model.run_in_model(model_name) as model: + if block: + await zaza.model.async_block_until( + lambda: _check_for_file(model, cert_files), timeout=timeout) + cert_file = await _check_for_file(model, cert_files) + return cert_file + +get_cert_file_name = zaza.model.sync_wrapper(async_get_cert_file_name) def get_cacert(): @@ -193,8 +252,9 @@ def get_cacert(): :returns: Path to CA Certificate bundle or None. :rtype: Optional[str] """ - if os.path.exists(KEYSTONE_LOCAL_CACERT): - return KEYSTONE_LOCAL_CACERT + for _cert in LOCAL_CERTIFICATES: + if os.path.exists(_cert): + return _cert # OpenStack Client helpers @@ -1951,24 +2011,28 @@ def get_overcloud_auth(address=None, model_name=None): 'API_VERSION': 3, } if tls_rid: + cert_file = get_cert_file_name('keystone', model_name=model_name) unit = model.get_first_unit_name('keystone', model_name=model_name) # ensure that the path to put the local cacert in actually exists. The # assumption that 'tests/' exists for, say, mojo is false. # Needed due to: # commit: 537473ad3addeaa3d1e4e2d0fd556aeaa4018eb2 - _dir = os.path.dirname(KEYSTONE_LOCAL_CACERT) + _dir = os.path.dirname(cert_file) if not os.path.exists(_dir): os.makedirs(_dir) + _local_cert_file = "{}/{}".format( + LOCAL_CERT_DIR, + os.path.basename(cert_file)) model.scp_from_unit( unit, - KEYSTONE_REMOTE_CACERT, - KEYSTONE_LOCAL_CACERT) + cert_file, + _local_cert_file) - if os.path.exists(KEYSTONE_LOCAL_CACERT): - os.chmod(KEYSTONE_LOCAL_CACERT, 0o644) - auth_settings['OS_CACERT'] = KEYSTONE_LOCAL_CACERT + if os.path.exists(_local_cert_file): + os.chmod(_local_cert_file, 0o644) + auth_settings['OS_CACERT'] = _local_cert_file return auth_settings From a20733cd1460ada0987ebb2999086eb6dd77cee8 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Sun, 24 Jan 2021 14:31:29 +0000 Subject: [PATCH 2/8] Refactor ca functions --- zaza/openstack/charm_tests/keystone/setup.py | 4 +- zaza/openstack/charm_tests/vault/setup.py | 7 +- zaza/openstack/charm_tests/vault/tests.py | 4 +- zaza/openstack/utilities/openstack.py | 214 +++++++++++++------ 4 files changed, 153 insertions(+), 76 deletions(-) diff --git a/zaza/openstack/charm_tests/keystone/setup.py b/zaza/openstack/charm_tests/keystone/setup.py index 8d64993..bec43fe 100644 --- a/zaza/openstack/charm_tests/keystone/setup.py +++ b/zaza/openstack/charm_tests/keystone/setup.py @@ -41,9 +41,7 @@ def wait_for_cacert(model_name=None): :type model_name: str """ logging.info("Waiting for cacert") - cert_file = openstack_utils.get_cert_file_name( - 'keystone') - zaza.model.block_until_file_has_contents( + zaza.openstack.utilities.openstack.block_until_ca_exists( 'keystone', cert_file, 'CERTIFICATE', diff --git a/zaza/openstack/charm_tests/vault/setup.py b/zaza/openstack/charm_tests/vault/setup.py index d605063..ad1d535 100644 --- a/zaza/openstack/charm_tests/vault/setup.py +++ b/zaza/openstack/charm_tests/vault/setup.py @@ -222,9 +222,7 @@ def validate_ca(cacertificate, application="keystone", port=5000): :returns: None :rtype: None """ - cert_file = zaza.openstack.utilities.openstack.get_cert_file_name( - application) - zaza.model.block_until_file_has_contents( + zaza.openstack.utilities.openstack.block_until_ca_exists( application, cert_file, cacertificate.decode().strip()) @@ -238,3 +236,6 @@ def validate_ca(cacertificate, application="keystone", port=5000): fp.write(cacertificate.decode()) fp.flush() requests.get('https://{}:{}'.format(ip, str(port)), verify=fp.name) + +def get_cert(): + print(zaza.openstack.utilities.openstack.get_remote_ca_cert_file('masakari')) diff --git a/zaza/openstack/charm_tests/vault/tests.py b/zaza/openstack/charm_tests/vault/tests.py index e96f36a..141a1e6 100644 --- a/zaza/openstack/charm_tests/vault/tests.py +++ b/zaza/openstack/charm_tests/vault/tests.py @@ -154,9 +154,7 @@ class VaultTest(BaseVaultTest): test_config = lifecycle_utils.get_charm_config() del test_config['target_deploy_status']['vault'] - cert_file = zaza.openstack.utilities.openstack.get_cert_file_name( - 'keystone') - zaza.model.block_until_file_has_contents( + zaza.openstack.utilities.openstack.block_until_ca_exists( 'keystone', cert_file, cacert.decode().strip()) diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index b914c0f..7d2fc9d 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -27,6 +27,7 @@ import logging import os import paramiko import re +import shutil import six import subprocess import sys @@ -182,7 +183,10 @@ WORKLOAD_STATUS_EXCEPTIONS = { 'ceilometer and gnocchi')}} # For vault TLS certificates +CACERT_FILENAME_FORMAT = "{}_juju_ca_cert.crt" +CERT_PROVIDORS = ['vault'] LOCAL_CERT_DIR = "tests" +REMOTE_CERT_DIR = "/usr/local/share/ca-certificates" KEYSTONE_CACERT = "keystone_juju_ca_cert.crt" KEYSTONE_REMOTE_CACERT = ( "/usr/local/share/ca-certificates/{}".format(KEYSTONE_CACERT)) @@ -197,53 +201,91 @@ REMOTE_CERTIFICATES = [VAULT_REMOTE_CACERT, KEYSTONE_REMOTE_CACERT] LOCAL_CERTIFICATES = [VAULT_LOCAL_CACERT, KEYSTONE_LOCAL_CACERT] -async def async_get_cert_file_name(app, cert_files=None, block=True, - model_name=None, timeout=2700): - """Get the name of the CA cert file thats on all units of an application. - - :param app: Name of application - :type capp: str - :param cert_files: List of cert files to search for. - :type cert_files: List[str] - :param block: Whether to block until a consistent cert file is found. - :type block: bool - :param model_name: Name of model to run check in - :type model_name: str - :param timeout: Time to wait for consistent file - :type timeout: int - :returns: Credentials dictionary - :rtype: dict - """ - async def _check_for_file(model, cert_files): - units = model.applications[app].units - results = {u.entity_id: [] for u in units} - for unit in units: - try: - for cf in cert_files: - output = await unit.run('test -e "{}"; echo $?'.format(cf)) - contents = output.data.get('results')['Stdout'] - if "0" in contents: - results[unit.entity_id].append(cf) - except JujuError: - pass - for cert_file in cert_files: - # Check that the certificate file exists on all the units. - if all(cert_file in files for files in results.values()): - return cert_file +async def async_block_until_ca_exists(application_name, ca_cert, model_name=None, timeout=2700): + async def _check_ca_present(model, ca_files): + units = model.applications[application_name].units + print(ca_files) + for ca_file in ca_files: + for unit in units: + print(unit) + print(ca_file) + try: + output = await unit.run('cat {}'.format(ca_file)) + contents = output.data.get('results').get('Stdout', '') + if not ca_cert in contents: + print("It's not here!") + print(ca_cert) + print(contents) + break + if ca_cert in contents: + print("It's here!") + # libjuju throws a generic error for connection failure. So we + # cannot differentiate between a connectivity issue and a + # target file not existing error. For now just assume the + # latter. + except JujuError: + continue + else: + return True else: - return None - - if not cert_files: - cert_files = REMOTE_CERTIFICATES - cert_file = None + return False + ca_files = await _async_get_remote_ca_cert_file_candidates(application_name, model_name=model_name) + print(ca_files) async with zaza.model.run_in_model(model_name) as model: - if block: - await zaza.model.async_block_until( - lambda: _check_for_file(model, cert_files), timeout=timeout) - cert_file = await _check_for_file(model, cert_files) - return cert_file + await zaza.model.async_block_until( + lambda: _check_ca_present(model, ca_files), timeout=timeout) -get_cert_file_name = zaza.model.sync_wrapper(async_get_cert_file_name) +block_until_ca_exists = zaza.model.sync_wrapper(async_block_until_ca_exists) + + + +#async def async_get_cert_file_name(app, cert_files=None, block=True, +# model_name=None, timeout=2700): +# """Get the name of the CA cert file thats on all units of an application. +# +# :param app: Name of application +# :type capp: str +# :param cert_files: List of cert files to search for. +# :type cert_files: List[str] +# :param block: Whether to block until a consistent cert file is found. +# :type block: bool +# :param model_name: Name of model to run check in +# :type model_name: str +# :param timeout: Time to wait for consistent file +# :type timeout: int +# :returns: Credentials dictionary +# :rtype: dict +# """ +# async def _check_for_file(model, cert_files): +# units = model.applications[app].units +# results = {u.entity_id: [] for u in units} +# for unit in units: +# try: +# for cf in cert_files: +# output = await unit.run('test -e "{}"; echo $?'.format(cf)) +# contents = output.data.get('results')['Stdout'] +# if "0" in contents: +# results[unit.entity_id].append(cf) +# except JujuError: +# pass +# for cert_file in cert_files: +# # Check that the certificate file exists on all the units. +# if all(cert_file in files for files in results.values()): +# return cert_file +# else: +# return None +# +# if not cert_files: +# cert_files = REMOTE_CERTIFICATES +# cert_file = None +# async with zaza.model.run_in_model(model_name) as model: +# if block: +# await zaza.model.async_block_until( +# lambda: _check_for_file(model, cert_files), timeout=timeout) +# cert_file = await _check_for_file(model, cert_files) +# return cert_file +# +#get_cert_file_name = zaza.model.sync_wrapper(async_get_cert_file_name) def get_cacert(): @@ -2010,32 +2052,70 @@ def get_overcloud_auth(address=None, model_name=None): 'OS_PROJECT_DOMAIN_NAME': 'admin_domain', 'API_VERSION': 3, } - if tls_rid: - cert_file = get_cert_file_name('keystone', model_name=model_name) - unit = model.get_first_unit_name('keystone', model_name=model_name) - - # ensure that the path to put the local cacert in actually exists. The - # assumption that 'tests/' exists for, say, mojo is false. - # Needed due to: - # commit: 537473ad3addeaa3d1e4e2d0fd556aeaa4018eb2 - _dir = os.path.dirname(cert_file) - if not os.path.exists(_dir): - os.makedirs(_dir) - - _local_cert_file = "{}/{}".format( - LOCAL_CERT_DIR, - os.path.basename(cert_file)) - model.scp_from_unit( - unit, - cert_file, - _local_cert_file) - - if os.path.exists(_local_cert_file): - os.chmod(_local_cert_file, 0o644) - auth_settings['OS_CACERT'] = _local_cert_file + local_ca_cert = get_remote_ca_cert_file('keystone', model_name=model_name) + if ca_cert: + auth_settings['OS_CACERT'] = local_ca_cert return auth_settings +async def _async_get_remote_ca_cert_file_candidates(application, model_name=None): + cert_files = [] + # unit = model.get_first_unit_name(application, model_name=model_name) + units = await model.async_get_units(application, model_name=model_name) + unit = units[0].name + for _providor in CERT_PROVIDORS: + tls_rid = await model.async_get_relation_id( + application, + _providor, + model_name=model_name, + remote_interface_name='certificates') + if tls_rid: + cert_files.append(REMOTE_CERT_DIR + '/' + CACERT_FILENAME_FORMAT.format(_providor)) + cert_files.append(REMOTE_CERT_DIR + '/' + KEYSTONE_CACERT) + return cert_files + +_get_remote_ca_cert_file_candidates = zaza.model.sync_wrapper(_async_get_remote_ca_cert_file_candidates) + +def get_remote_ca_cert_file(application, model_name=None): +# CACERT_FILENAME = "{}_juju_ca_cert.crt" +# cert_files = [] +# unit = model.get_first_unit_name(application, model_name=model_name) +# for _providor in CERT_PROVIDORS: +# tls_rid = model.get_relation_id( +# application, +# _providor, +# model_name=model_name, +# remote_interface_name='certificates') +# if tls_rid: +# cert_files.append(CACERT_FILENAME.format(_providor)) +# cert_files.append(KEYSTONE_CACERT) + unit = model.get_first_unit_name(application, model_name=model_name) + local_cert_file = None + cert_files = _get_remote_ca_cert_file_candidates(application, model_name=model_name) + for cert_file in cert_files: + _local_cert_file = "{}/{}".format( + LOCAL_CERT_DIR, + os.path.basename(cert_file)) + with tempfile.NamedTemporaryFile(mode="w", delete=False) as _tmp_ca_file: + try: + model.scp_from_unit( + unit, + cert_file, + _tmp_ca_file.name) + except JujuError: + continue + # ensure that the path to put the local cacert in actually exists. The + # assumption that 'tests/' exists for, say, mojo is false. + # Needed due to: + # commit: 537473ad3addeaa3d1e4e2d0fd556aeaa4018eb2 + _dir = os.path.dirname(_local_cert_file) + if not os.path.exists(_dir): + os.makedirs(_dir) + shutil.move(_tmp_ca_file.name, _local_cert_file) + os.chmod(_local_cert_file, 0o644) + local_cert_file = _local_cert_file + break + return local_cert_file def get_urllib_opener(): """Create a urllib opener taking into account proxy settings. From 24fbc068c94f99a702f7b3f8a2a01b7de0bf855d Mon Sep 17 00:00:00 2001 From: Liam Young Date: Sun, 24 Jan 2021 15:27:06 +0000 Subject: [PATCH 3/8] Correct args to block_until_ca_exists --- zaza/openstack/charm_tests/keystone/setup.py | 1 - zaza/openstack/charm_tests/vault/setup.py | 1 - zaza/openstack/charm_tests/vault/tests.py | 1 - 3 files changed, 3 deletions(-) diff --git a/zaza/openstack/charm_tests/keystone/setup.py b/zaza/openstack/charm_tests/keystone/setup.py index bec43fe..73264cd 100644 --- a/zaza/openstack/charm_tests/keystone/setup.py +++ b/zaza/openstack/charm_tests/keystone/setup.py @@ -43,7 +43,6 @@ def wait_for_cacert(model_name=None): logging.info("Waiting for cacert") zaza.openstack.utilities.openstack.block_until_ca_exists( 'keystone', - cert_file, 'CERTIFICATE', model_name=model_name) zaza.model.block_until_all_units_idle(model_name=model_name) diff --git a/zaza/openstack/charm_tests/vault/setup.py b/zaza/openstack/charm_tests/vault/setup.py index ad1d535..25389d5 100644 --- a/zaza/openstack/charm_tests/vault/setup.py +++ b/zaza/openstack/charm_tests/vault/setup.py @@ -224,7 +224,6 @@ def validate_ca(cacertificate, application="keystone", port=5000): """ zaza.openstack.utilities.openstack.block_until_ca_exists( application, - cert_file, cacertificate.decode().strip()) vip = (zaza.model.get_application_config(application) .get("vip").get("value")) diff --git a/zaza/openstack/charm_tests/vault/tests.py b/zaza/openstack/charm_tests/vault/tests.py index 141a1e6..68130a0 100644 --- a/zaza/openstack/charm_tests/vault/tests.py +++ b/zaza/openstack/charm_tests/vault/tests.py @@ -156,7 +156,6 @@ class VaultTest(BaseVaultTest): del test_config['target_deploy_status']['vault'] zaza.openstack.utilities.openstack.block_until_ca_exists( 'keystone', - cert_file, cacert.decode().strip()) zaza.model.wait_for_application_states( states=test_config.get('target_deploy_status', {})) From d637646a9e139245029a4506afc2d0b71f2060bc Mon Sep 17 00:00:00 2001 From: Liam Young Date: Sun, 24 Jan 2021 16:19:45 +0000 Subject: [PATCH 4/8] Fix typo --- zaza/openstack/utilities/openstack.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index 7d2fc9d..5fb4980 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -2053,7 +2053,7 @@ def get_overcloud_auth(address=None, model_name=None): 'API_VERSION': 3, } local_ca_cert = get_remote_ca_cert_file('keystone', model_name=model_name) - if ca_cert: + if local_ca_cert: auth_settings['OS_CACERT'] = local_ca_cert return auth_settings From 401829f0a7b657f1eb9b3d129844476ce869839d Mon Sep 17 00:00:00 2001 From: Liam Young Date: Sun, 24 Jan 2021 17:24:20 +0000 Subject: [PATCH 5/8] Code tidy and docstrings --- zaza/openstack/charm_tests/vault/setup.py | 3 - zaza/openstack/utilities/openstack.py | 152 ++++++++-------------- 2 files changed, 53 insertions(+), 102 deletions(-) diff --git a/zaza/openstack/charm_tests/vault/setup.py b/zaza/openstack/charm_tests/vault/setup.py index 25389d5..c792508 100644 --- a/zaza/openstack/charm_tests/vault/setup.py +++ b/zaza/openstack/charm_tests/vault/setup.py @@ -235,6 +235,3 @@ def validate_ca(cacertificate, application="keystone", port=5000): fp.write(cacertificate.decode()) fp.flush() requests.get('https://{}:{}'.format(ip, str(port)), verify=fp.name) - -def get_cert(): - print(zaza.openstack.utilities.openstack.get_remote_ca_cert_file('masakari')) diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index 5fb4980..a975206 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -184,7 +184,7 @@ WORKLOAD_STATUS_EXCEPTIONS = { # For vault TLS certificates CACERT_FILENAME_FORMAT = "{}_juju_ca_cert.crt" -CERT_PROVIDORS = ['vault'] +CERT_PROVIDERS = ['vault'] LOCAL_CERT_DIR = "tests" REMOTE_CERT_DIR = "/usr/local/share/ca-certificates" KEYSTONE_CACERT = "keystone_juju_ca_cert.crt" @@ -192,33 +192,29 @@ KEYSTONE_REMOTE_CACERT = ( "/usr/local/share/ca-certificates/{}".format(KEYSTONE_CACERT)) KEYSTONE_LOCAL_CACERT = ("{}/{}".format(LOCAL_CERT_DIR, KEYSTONE_CACERT)) -VAULT_CACERT = "vault_juju_ca_cert.crt" -VAULT_REMOTE_CACERT = ( - "/usr/local/share/ca-certificates/{}".format(VAULT_CACERT)) -VAULT_LOCAL_CACERT = ("{}/{}".format(LOCAL_CERT_DIR, VAULT_CACERT)) -REMOTE_CERTIFICATES = [VAULT_REMOTE_CACERT, KEYSTONE_REMOTE_CACERT] -LOCAL_CERTIFICATES = [VAULT_LOCAL_CACERT, KEYSTONE_LOCAL_CACERT] +async def async_block_until_ca_exists(application_name, ca_cert, + model_name=None, timeout=2700): + """Block until a CA cert is on all units of application_name. - -async def async_block_until_ca_exists(application_name, ca_cert, model_name=None, timeout=2700): + :param application_name: Name of application to check + :type application_name: str + :param ca_cert: The certificate to look for. + :type ca_cert: str + :param model_name: Name of model to query. + :type model_name: str + :param timeout: How long in seconds to wait + :type timeout: int + """ async def _check_ca_present(model, ca_files): units = model.applications[application_name].units - print(ca_files) for ca_file in ca_files: for unit in units: - print(unit) - print(ca_file) try: output = await unit.run('cat {}'.format(ca_file)) contents = output.data.get('results').get('Stdout', '') - if not ca_cert in contents: - print("It's not here!") - print(ca_cert) - print(contents) + if ca_cert not in contents: break - if ca_cert in contents: - print("It's here!") # libjuju throws a generic error for connection failure. So we # cannot differentiate between a connectivity issue and a # target file not existing error. For now just assume the @@ -229,8 +225,9 @@ async def async_block_until_ca_exists(application_name, ca_cert, model_name=None return True else: return False - ca_files = await _async_get_remote_ca_cert_file_candidates(application_name, model_name=model_name) - print(ca_files) + ca_files = await _async_get_remote_ca_cert_file_candidates( + application_name, + model_name=model_name) async with zaza.model.run_in_model(model_name) as model: await zaza.model.async_block_until( lambda: _check_ca_present(model, ca_files), timeout=timeout) @@ -238,65 +235,19 @@ async def async_block_until_ca_exists(application_name, ca_cert, model_name=None block_until_ca_exists = zaza.model.sync_wrapper(async_block_until_ca_exists) - -#async def async_get_cert_file_name(app, cert_files=None, block=True, -# model_name=None, timeout=2700): -# """Get the name of the CA cert file thats on all units of an application. -# -# :param app: Name of application -# :type capp: str -# :param cert_files: List of cert files to search for. -# :type cert_files: List[str] -# :param block: Whether to block until a consistent cert file is found. -# :type block: bool -# :param model_name: Name of model to run check in -# :type model_name: str -# :param timeout: Time to wait for consistent file -# :type timeout: int -# :returns: Credentials dictionary -# :rtype: dict -# """ -# async def _check_for_file(model, cert_files): -# units = model.applications[app].units -# results = {u.entity_id: [] for u in units} -# for unit in units: -# try: -# for cf in cert_files: -# output = await unit.run('test -e "{}"; echo $?'.format(cf)) -# contents = output.data.get('results')['Stdout'] -# if "0" in contents: -# results[unit.entity_id].append(cf) -# except JujuError: -# pass -# for cert_file in cert_files: -# # Check that the certificate file exists on all the units. -# if all(cert_file in files for files in results.values()): -# return cert_file -# else: -# return None -# -# if not cert_files: -# cert_files = REMOTE_CERTIFICATES -# cert_file = None -# async with zaza.model.run_in_model(model_name) as model: -# if block: -# await zaza.model.async_block_until( -# lambda: _check_for_file(model, cert_files), timeout=timeout) -# cert_file = await _check_for_file(model, cert_files) -# return cert_file -# -#get_cert_file_name = zaza.model.sync_wrapper(async_get_cert_file_name) - - def get_cacert(): """Return path to CA Certificate bundle for verification during test. :returns: Path to CA Certificate bundle or None. - :rtype: Optional[str] + :rtype: Union[str, None] """ - for _cert in LOCAL_CERTIFICATES: + for _provider in CERT_PROVIDERS: + _cert = LOCAL_CERT_DIR + '/' + CACERT_FILENAME_FORMAT.format( + _provider) if os.path.exists(_cert): return _cert + if os.path.exists(KEYSTONE_LOCAL_CACERT): + return KEYSTONE_LOCAL_CACERT # OpenStack Client helpers @@ -2058,65 +2009,68 @@ def get_overcloud_auth(address=None, model_name=None): return auth_settings -async def _async_get_remote_ca_cert_file_candidates(application, model_name=None): + +async def _async_get_remote_ca_cert_file_candidates(application, + model_name=None): cert_files = [] - # unit = model.get_first_unit_name(application, model_name=model_name) - units = await model.async_get_units(application, model_name=model_name) - unit = units[0].name - for _providor in CERT_PROVIDORS: + for _provider in CERT_PROVIDERS: tls_rid = await model.async_get_relation_id( application, - _providor, + _provider, model_name=model_name, remote_interface_name='certificates') if tls_rid: - cert_files.append(REMOTE_CERT_DIR + '/' + CACERT_FILENAME_FORMAT.format(_providor)) - cert_files.append(REMOTE_CERT_DIR + '/' + KEYSTONE_CACERT) + cert_files.append( + REMOTE_CERT_DIR + '/' + CACERT_FILENAME_FORMAT.format( + _provider)) + cert_files.append(KEYSTONE_LOCAL_CACERT) return cert_files -_get_remote_ca_cert_file_candidates = zaza.model.sync_wrapper(_async_get_remote_ca_cert_file_candidates) +_get_remote_ca_cert_file_candidates = zaza.model.sync_wrapper( + _async_get_remote_ca_cert_file_candidates) + def get_remote_ca_cert_file(application, model_name=None): -# CACERT_FILENAME = "{}_juju_ca_cert.crt" -# cert_files = [] -# unit = model.get_first_unit_name(application, model_name=model_name) -# for _providor in CERT_PROVIDORS: -# tls_rid = model.get_relation_id( -# application, -# _providor, -# model_name=model_name, -# remote_interface_name='certificates') -# if tls_rid: -# cert_files.append(CACERT_FILENAME.format(_providor)) -# cert_files.append(KEYSTONE_CACERT) + """Collect CA certificate from application. + + :param application: Name of application to collect file from. + :type application: str + :param model_name: Name of model to query. + :type model_name: str + :returns: Path to cafile + :rtype: str + """ unit = model.get_first_unit_name(application, model_name=model_name) local_cert_file = None - cert_files = _get_remote_ca_cert_file_candidates(application, model_name=model_name) + cert_files = _get_remote_ca_cert_file_candidates( + application, + model_name=model_name) for cert_file in cert_files: _local_cert_file = "{}/{}".format( LOCAL_CERT_DIR, os.path.basename(cert_file)) - with tempfile.NamedTemporaryFile(mode="w", delete=False) as _tmp_ca_file: + with tempfile.NamedTemporaryFile(mode="w", delete=False) as _tmp_ca: try: model.scp_from_unit( unit, cert_file, - _tmp_ca_file.name) + _tmp_ca.name) except JujuError: continue - # ensure that the path to put the local cacert in actually exists. The - # assumption that 'tests/' exists for, say, mojo is false. + # ensure that the path to put the local cacert in actually exists. + # The assumption that 'tests/' exists for, say, mojo is false. # Needed due to: # commit: 537473ad3addeaa3d1e4e2d0fd556aeaa4018eb2 _dir = os.path.dirname(_local_cert_file) if not os.path.exists(_dir): os.makedirs(_dir) - shutil.move(_tmp_ca_file.name, _local_cert_file) + shutil.move(_tmp_ca.name, _local_cert_file) os.chmod(_local_cert_file, 0o644) local_cert_file = _local_cert_file break return local_cert_file + def get_urllib_opener(): """Create a urllib opener taking into account proxy settings. From e047150f5bae0b8a996cb57542491131348b92f1 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Mon, 25 Jan 2021 09:21:00 +0000 Subject: [PATCH 6/8] Add unit tests --- .../test_zaza_utilities_openstack.py | 157 +++++++++++++++++- ..._zaza_utilities_parallel_series_upgrade.py | 24 +-- unit_tests/utils.py | 22 +++ zaza/openstack/utilities/openstack.py | 12 +- 4 files changed, 190 insertions(+), 25 deletions(-) diff --git a/unit_tests/utilities/test_zaza_utilities_openstack.py b/unit_tests/utilities/test_zaza_utilities_openstack.py index 7e0e8f1..e4abfb1 100644 --- a/unit_tests/utilities/test_zaza_utilities_openstack.py +++ b/unit_tests/utilities/test_zaza_utilities_openstack.py @@ -17,6 +17,8 @@ import datetime import io import mock import subprocess +import sys +import unittest import tenacity import unit_tests.utils as ut_utils @@ -191,6 +193,7 @@ class TestOpenStackUtils(ut_utils.BaseTestCase): self.patch_object(openstack_utils, 'get_application_config_option') self.patch_object(openstack_utils, 'get_keystone_ip') self.patch_object(openstack_utils, "get_current_os_versions") + self.patch_object(openstack_utils, "get_remote_ca_cert_file") self.patch_object(openstack_utils.juju_utils, 'leader_get') if tls_relation: self.patch_object(openstack_utils.model, "scp_from_unit") @@ -204,6 +207,7 @@ class TestOpenStackUtils(ut_utils.BaseTestCase): self.get_relation_id.return_value = None self.get_application_config_option.return_value = None self.leader_get.return_value = 'openstack' + self.get_remote_ca_cert_file.return_value = None if tls_relation or ssl_cert: port = 35357 transport = 'https' @@ -245,7 +249,8 @@ class TestOpenStackUtils(ut_utils.BaseTestCase): 'API_VERSION': 3, } if tls_relation: - expect['OS_CACERT'] = openstack_utils.KEYSTONE_LOCAL_CACERT + self.get_remote_ca_cert_file.return_value = '/tmp/a.cert' + expect['OS_CACERT'] = '/tmp/a.cert' self.assertEqual(openstack_utils.get_overcloud_auth(), expect) @@ -1327,3 +1332,153 @@ class TestOpenStackUtils(ut_utils.BaseTestCase): mock.ANY, 'bridge-interface-mappings', {'ovn-bridge-mappings': 'physnet1:br-ex'})) + + def test_get_cacert(self): + self.patch_object(openstack_utils.os.path, 'exists') + results = { + 'tests/vault_juju_ca_cert.crt': True} + self.exists.side_effect = lambda x: results[x] + self.assertEqual( + openstack_utils.get_cacert(), + 'tests/vault_juju_ca_cert.crt') + + results = { + 'tests/vault_juju_ca_cert.crt': False, + 'tests/keystone_juju_ca_cert.crt': True} + self.assertEqual( + openstack_utils.get_cacert(), + 'tests/keystone_juju_ca_cert.crt') + + results = { + 'tests/vault_juju_ca_cert.crt': False, + 'tests/keystone_juju_ca_cert.crt': False} + self.assertIsNone(openstack_utils.get_cacert()) + + def test_get_remote_ca_cert_file(self): + self.patch_object(openstack_utils.model, 'get_first_unit_name') + self.patch_object( + openstack_utils, + '_get_remote_ca_cert_file_candidates') + self.patch_object(openstack_utils.model, 'scp_from_unit') + self.patch_object(openstack_utils.os.path, 'exists') + self.patch_object(openstack_utils.shutil, 'move') + self.patch_object(openstack_utils.os, 'chmod') + self.patch_object(openstack_utils.tempfile, 'NamedTemporaryFile') + enter_mock = mock.MagicMock() + enter_mock.__enter__.return_value.name = 'tempfilename' + self.NamedTemporaryFile.return_value = enter_mock + self.get_first_unit_name.return_value = 'neutron-api/0' + self._get_remote_ca_cert_file_candidates.return_value = [ + '/tmp/ca1.cert'] + self.exists.return_value = True + + openstack_utils.get_remote_ca_cert_file('neutron-api') + self.scp_from_unit.assert_called_once_with( + 'neutron-api/0', + '/tmp/ca1.cert', + 'tempfilename') + self.chmod.assert_called_once_with('tests/ca1.cert', 0o644) + self.move.assert_called_once_with('tempfilename', 'tests/ca1.cert') + + +class TestAsyncOpenstackUtils(ut_utils.AioTestCase): + + def setUp(self): + super(TestAsyncOpenstackUtils, self).setUp() + if sys.version_info < (3, 6, 0): + raise unittest.SkipTest("Can't AsyncMock in py35") + model_mock = mock.MagicMock() + test_mock = mock.MagicMock() + + class AsyncContextManagerMock(test_mock): + async def __aenter__(self): + yield model_mock + + async def __aexit__(self, *args): + pass + + self.model_mock = model_mock + self.patch_object(openstack_utils.zaza.model, "async_block_until") + + async def _block_until(f, timeout): + # Store the result of the call to _check_ca_present to validate + # tests + self.result = await f() + self.async_block_until.side_effect = _block_until + self.patch('zaza.model.run_in_model', name='_run_in_model') + self._run_in_model.return_value = AsyncContextManagerMock + self._run_in_model().__aenter__.return_value = self.model_mock + + async def test_async_block_until_ca_exists(self): + def _get_action_output(stdout, code, stderr=None): + stderr = stderr or '' + action = mock.MagicMock() + action.data = { + 'results': { + 'Code': code, + 'Stderr': stderr, + 'Stdout': stdout}} + return action + results = { + '/tmp/missing.cert': _get_action_output( + '', + '1', + 'cat: /tmp/missing.cert: No such file or directory'), + '/tmp/good.cert': _get_action_output('CERTIFICATE', '0')} + + async def _run(command, timeout=None): + return results[command.split()[-1]] + self.unit1 = mock.MagicMock() + self.unit2 = mock.MagicMock() + self.unit2.run.side_effect = _run + self.unit1.run.side_effect = _run + self.units = [self.unit1, self.unit2] + _units = mock.MagicMock() + _units.units = self.units + self.model_mock.applications = { + 'keystone': _units + } + self.patch_object( + openstack_utils, + "_async_get_remote_ca_cert_file_candidates") + + # Test a missing cert then a good cert. + self._async_get_remote_ca_cert_file_candidates.return_value = [ + '/tmp/missing.cert', + '/tmp/good.cert'] + await openstack_utils.async_block_until_ca_exists( + 'keystone', + 'CERTIFICATE') + self.assertTrue(self.result) + + # Test a single missing + self._async_get_remote_ca_cert_file_candidates.return_value = [ + '/tmp/missing.cert'] + await openstack_utils.async_block_until_ca_exists( + 'keystone', + 'CERTIFICATE') + self.assertFalse(self.result) + + async def test__async_get_remote_ca_cert_file_candidates(self): + self.patch_object(openstack_utils.zaza.model, "async_get_relation_id") + rel_id_out = { + } + + def _get_relation_id(app, cert_app, model_name, remote_interface_name): + return rel_id_out[cert_app] + self.async_get_relation_id.side_effect = _get_relation_id + + rel_id_out['vault'] = 'certs:1' + r = await openstack_utils._async_get_remote_ca_cert_file_candidates( + 'neutron-api', 'mymodel') + self.assertEqual( + r, + ['/usr/local/share/ca-certificates/vault_juju_ca_cert.crt', + '/usr/local/share/ca-certificates/keystone_juju_ca_cert.crt']) + + rel_id_out['vault'] = None + r = await openstack_utils._async_get_remote_ca_cert_file_candidates( + 'neutron-api', 'mymodel') + self.assertEqual( + r, + ['/usr/local/share/ca-certificates/keystone_juju_ca_cert.crt']) diff --git a/unit_tests/utilities/test_zaza_utilities_parallel_series_upgrade.py b/unit_tests/utilities/test_zaza_utilities_parallel_series_upgrade.py index 0231dbf..70ffd3b 100644 --- a/unit_tests/utilities/test_zaza_utilities_parallel_series_upgrade.py +++ b/unit_tests/utilities/test_zaza_utilities_parallel_series_upgrade.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import asyncio import mock import sys import unittest @@ -139,28 +138,7 @@ class Test_ParallelSeriesUpgradeSync(ut_utils.BaseTestCase): self.assertEqual(expected, config) -class AioTestCase(ut_utils.BaseTestCase): - def __init__(self, methodName='runTest', loop=None): - self.loop = loop or asyncio.get_event_loop() - self._function_cache = {} - super(AioTestCase, self).__init__(methodName=methodName) - - def coroutine_function_decorator(self, func): - def wrapper(*args, **kw): - return self.loop.run_until_complete(func(*args, **kw)) - return wrapper - - def __getattribute__(self, item): - attr = object.__getattribute__(self, item) - if asyncio.iscoroutinefunction(attr) and item.startswith('test_'): - if item not in self._function_cache: - self._function_cache[item] = ( - self.coroutine_function_decorator(attr)) - return self._function_cache[item] - return attr - - -class TestParallelSeriesUpgrade(AioTestCase): +class TestParallelSeriesUpgrade(ut_utils.AioTestCase): def setUp(self): super(TestParallelSeriesUpgrade, self).setUp() if sys.version_info < (3, 6, 0): diff --git a/unit_tests/utils.py b/unit_tests/utils.py index 4694d0d..8e31f45 100644 --- a/unit_tests/utils.py +++ b/unit_tests/utils.py @@ -19,6 +19,7 @@ """Module to provide helper for writing unit tests.""" +import asyncio import contextlib import io import mock @@ -96,3 +97,24 @@ class BaseTestCase(unittest.TestCase): started.return_value = return_value self._patches_start[name] = started setattr(self, name, started) + + +class AioTestCase(BaseTestCase): + def __init__(self, methodName='runTest', loop=None): + self.loop = loop or asyncio.get_event_loop() + self._function_cache = {} + super(AioTestCase, self).__init__(methodName=methodName) + + def coroutine_function_decorator(self, func): + def wrapper(*args, **kw): + return self.loop.run_until_complete(func(*args, **kw)) + return wrapper + + def __getattribute__(self, item): + attr = object.__getattribute__(self, item) + if asyncio.iscoroutinefunction(attr) and item.startswith('test_'): + if item not in self._function_cache: + self._function_cache[item] = ( + self.coroutine_function_decorator(attr)) + return self._function_cache[item] + return attr diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index a975206..d883e0a 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -222,6 +222,7 @@ async def async_block_until_ca_exists(application_name, ca_cert, except JujuError: continue else: + # The CA was found in `ca_file` on all units. return True else: return False @@ -2012,6 +2013,15 @@ def get_overcloud_auth(address=None, model_name=None): async def _async_get_remote_ca_cert_file_candidates(application, model_name=None): + """Return a list of possible remote CA file names. + + :param application: Name of application to examine. + :type application: str + :param model_name: Name of model to query. + :type model_name: str + :returns: List of paths to possible ca files. + :rtype: List[str] + """ cert_files = [] for _provider in CERT_PROVIDERS: tls_rid = await model.async_get_relation_id( @@ -2023,7 +2033,7 @@ async def _async_get_remote_ca_cert_file_candidates(application, cert_files.append( REMOTE_CERT_DIR + '/' + CACERT_FILENAME_FORMAT.format( _provider)) - cert_files.append(KEYSTONE_LOCAL_CACERT) + cert_files.append(KEYSTONE_REMOTE_CACERT) return cert_files _get_remote_ca_cert_file_candidates = zaza.model.sync_wrapper( From 5be8fc377179e8fce9de8e8e7d9744fc169d65a4 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Mon, 25 Jan 2021 11:41:39 +0000 Subject: [PATCH 7/8] Fix docstring and bug --- zaza/openstack/utilities/openstack.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zaza/openstack/utilities/openstack.py b/zaza/openstack/utilities/openstack.py index d883e0a..522df31 100644 --- a/zaza/openstack/utilities/openstack.py +++ b/zaza/openstack/utilities/openstack.py @@ -199,7 +199,7 @@ async def async_block_until_ca_exists(application_name, ca_cert, :param application_name: Name of application to check :type application_name: str - :param ca_cert: The certificate to look for. + :param ca_cert: The certificate content. :type ca_cert: str :param model_name: Name of model to query. :type model_name: str @@ -220,7 +220,7 @@ async def async_block_until_ca_exists(application_name, ca_cert, # target file not existing error. For now just assume the # latter. except JujuError: - continue + break else: # The CA was found in `ca_file` on all units. return True From 93a9aff92796ce626ae969fc0554f3411dc3adf2 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Mon, 25 Jan 2021 11:47:42 +0000 Subject: [PATCH 8/8] Fix context manager mock --- unit_tests/utilities/test_zaza_utilities_openstack.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unit_tests/utilities/test_zaza_utilities_openstack.py b/unit_tests/utilities/test_zaza_utilities_openstack.py index e4abfb1..67cd417 100644 --- a/unit_tests/utilities/test_zaza_utilities_openstack.py +++ b/unit_tests/utilities/test_zaza_utilities_openstack.py @@ -1392,7 +1392,7 @@ class TestAsyncOpenstackUtils(ut_utils.AioTestCase): class AsyncContextManagerMock(test_mock): async def __aenter__(self): - yield model_mock + return self async def __aexit__(self, *args): pass