From d9e590bdc615e0c23b790a65023d01a9e9611176 Mon Sep 17 00:00:00 2001 From: Kirsten G Date: Mon, 15 Jan 2018 22:04:49 -0800 Subject: [PATCH] Cache barbican certs for periodic tasks Added configuration parameter, temp_cache_dir, to magnum.conf with default value of "/var/lib/magnum/certificate-cache". This local directory will hold cached cluster TLS credentials that are generated during periodic tasks, to reduce load as the number of clusters increases. If the temp_cache_dir does not exist, the certificates will be created as tempfiles. Closes-Bug: #1659545 Change-Id: I8808c4098a7c8d22dbfc841142c9f9c8b976dde1 --- devstack/lib/magnum | 15 +- .../conductor/handlers/common/cert_manager.py | 80 +++++-- magnum/conf/cluster.py | 4 + magnum/drivers/heat/driver.py | 3 + .../handlers/common/test_cert_manager.py | 208 ++++++++++++++++++ 5 files changed, 291 insertions(+), 19 deletions(-) diff --git a/devstack/lib/magnum b/devstack/lib/magnum index 8f78dbe690..69f0d3181c 100644 --- a/devstack/lib/magnum +++ b/devstack/lib/magnum @@ -41,6 +41,7 @@ GITDIR["python-magnumclient"]=$DEST/python-magnumclient MAGNUM_STATE_PATH=${MAGNUM_STATE_PATH:=$DATA_DIR/magnum} MAGNUM_AUTH_CACHE_DIR=${MAGNUM_AUTH_CACHE_DIR:-/var/cache/magnum} +MAGNUM_CERTIFICATE_CACHE_DIR=${MAGNUM_CERTIFICATE_CACHE_DIR:-/var/lib/magnum/certificate-cache} MAGNUM_CONF_DIR=/etc/magnum MAGNUM_CONF=$MAGNUM_CONF_DIR/magnum.conf @@ -82,7 +83,7 @@ function is_magnum_enabled { # cleanup_magnum() - Remove residual data files, anything left over from previous # runs that a clean run would need to clean up function cleanup_magnum { - sudo rm -rf $MAGNUM_STATE_PATH $MAGNUM_AUTH_CACHE_DIR + sudo rm -rf $MAGNUM_STATE_PATH $MAGNUM_AUTH_CACHE_DIR $MAGNUM_CERTIFICATE_CACHE_DIR } # configure_magnum() - Set config files, create data dirs, etc @@ -139,6 +140,9 @@ function create_magnum_conf { iniset $MAGNUM_CONF api port "$MAGNUM_SERVICE_PORT" iniset $MAGNUM_CONF drivers verify_ca false fi + + iniset $MAGNUM_CONF cluster temp_cache_dir $MAGNUM_CERTIFICATE_CACHE_DIR + iniset $MAGNUM_CONF oslo_policy policy_file $MAGNUM_POLICY iniset $MAGNUM_CONF keystone_auth auth_type password @@ -237,9 +241,9 @@ function create_api_paste_conf { # create_magnum_cache_dir() - Part of the init_magnum() process function create_magnum_cache_dir { # Create cache dir - sudo mkdir -p $MAGNUM_AUTH_CACHE_DIR - sudo chown $STACK_USER $MAGNUM_AUTH_CACHE_DIR - rm -f $MAGNUM_AUTH_CACHE_DIR/* + sudo mkdir -p $1 + sudo chown $STACK_USER $1 + rm -f $1/* } @@ -253,7 +257,8 @@ function init_magnum { # Migrate magnum database $MAGNUM_BIN_DIR/magnum-db-manage upgrade fi - create_magnum_cache_dir + create_magnum_cache_dir $MAGNUM_AUTH_CACHE_DIR + create_magnum_cache_dir $MAGNUM_CERTIFICATE_CACHE_DIR } # magnum_register_image - Register heat image for magnum with property os_distro diff --git a/magnum/conductor/handlers/common/cert_manager.py b/magnum/conductor/handlers/common/cert_manager.py index 92b5684056..31fda33c1e 100755 --- a/magnum/conductor/handlers/common/cert_manager.py +++ b/magnum/conductor/handlers/common/cert_manager.py @@ -12,8 +12,6 @@ # License for the specific language governing permissions and limitations # under the License. -import tempfile - from oslo_log import log as logging import six @@ -22,9 +20,15 @@ from magnum.common import exception from magnum.common import short_id from magnum.common.x509 import operations as x509 +import magnum.conf +import os +import shutil +import tempfile + CONDUCTOR_CLIENT_NAME = six.u('Magnum-Conductor') LOG = logging.getLogger(__name__) +CONF = magnum.conf.CONF def _generate_ca_cert(issuer_name, context=None): @@ -139,22 +143,59 @@ def get_cluster_magnum_cert(cluster, context=None): def create_client_files(cluster, context=None): - ca_cert = get_cluster_ca_certificate(cluster, context) - magnum_cert = get_cluster_magnum_cert(cluster, context) + if not os.path.isdir(CONF.cluster.temp_cache_dir): + LOG.debug("Certificates will not be cached in the filesystem: they \ + will be created as tempfiles.") + ca_cert = get_cluster_ca_certificate(cluster, context) + magnum_cert = get_cluster_magnum_cert(cluster, context) - ca_cert_file = tempfile.NamedTemporaryFile() - ca_cert_file.write(ca_cert.get_certificate()) - ca_cert_file.flush() + ca_file = tempfile.NamedTemporaryFile(mode="w+") + ca_file.write(ca_cert.get_certificate()) + ca_file.flush() - magnum_key_file = tempfile.NamedTemporaryFile() - magnum_key_file.write(magnum_cert.get_decrypted_private_key()) - magnum_key_file.flush() + key_file = tempfile.NamedTemporaryFile(mode="w+") + key_file.write(magnum_cert.get_decrypted_private_key()) + key_file.flush() - magnum_cert_file = tempfile.NamedTemporaryFile() - magnum_cert_file.write(magnum_cert.get_certificate()) - magnum_cert_file.flush() + cert_file = tempfile.NamedTemporaryFile(mode="w+") + cert_file.write(magnum_cert.get_certificate()) + cert_file.flush() - return ca_cert_file, magnum_key_file, magnum_cert_file + else: + cached_cert_dir = os.path.join(CONF.cluster.temp_cache_dir, + cluster.uuid) + cached_ca_file = os.path.join(cached_cert_dir, 'ca.crt') + cached_key_file = os.path.join(cached_cert_dir, 'client.key') + cached_cert_file = os.path.join(cached_cert_dir, 'client.crt') + + if not os.path.isdir(cached_cert_dir): + os.mkdir(cached_cert_dir) + + ca_cert = get_cluster_ca_certificate(cluster, context) + magnum_cert = get_cluster_magnum_cert(cluster, context) + + ca_file = open(cached_ca_file, "w+") + ca_file.write(ca_cert.get_certificate()) + ca_file.flush() + + key_file = open(cached_key_file, "w+") + key_file.write(magnum_cert.get_decrypted_private_key()) + key_file.flush() + + cert_file = open(cached_cert_file, "w+") + cert_file.write(magnum_cert.get_certificate()) + cert_file.flush() + + os.chmod(cached_ca_file, 0o600) + os.chmod(cached_key_file, 0o600) + os.chmod(cached_cert_file, 0o600) + + else: + ca_file = open(cached_ca_file, "r") + key_file = open(cached_key_file, "r") + cert_file = open(cached_cert_file, "r") + + return ca_file, key_file, cert_file def sign_node_certificate(cluster, csr, context=None): @@ -185,3 +226,14 @@ def delete_certificates_from_cluster(cluster, context=None): except Exception: LOG.warning("Deleting certs is failed for Cluster %s", cluster.uuid) + + +def delete_client_files(cluster, context=None): + cached_cert_dir = os.path.join(CONF.cluster.temp_cache_dir, + cluster.uuid) + try: + if os.path.isdir(cached_cert_dir): + shutil.rmtree(cached_cert_dir) + except Exception: + LOG.warning("Deleting client files failed for Cluster %s", + cluster.uuid) diff --git a/magnum/conf/cluster.py b/magnum/conf/cluster.py index c7444c6eb5..015f258412 100644 --- a/magnum/conf/cluster.py +++ b/magnum/conf/cluster.py @@ -38,6 +38,10 @@ cluster_def_opts = [ 'Possible values include "affinity", "anti-affinity",' '"soft-affinity" and "soft-anti-affinity".') ), + cfg.StrOpt('temp_cache_dir', + default="/var/lib/magnum/certificate-cache", + help='Explicitly specify the temporary directory to hold ' + 'cached TLS certs.'), ] diff --git a/magnum/drivers/heat/driver.py b/magnum/drivers/heat/driver.py index 91059da225..7d1aff4f04 100755 --- a/magnum/drivers/heat/driver.py +++ b/magnum/drivers/heat/driver.py @@ -226,6 +226,9 @@ class HeatPoller(object): self.cluster) cert_manager.delete_certificates_from_cluster(self.cluster, context=self.context) + cert_manager.delete_client_files(self.cluster, + context=self.context) + except exception.ClusterNotFound: LOG.info('The cluster %s has been deleted by others.', self.cluster.uuid) diff --git a/magnum/tests/unit/conductor/handlers/common/test_cert_manager.py b/magnum/tests/unit/conductor/handlers/common/test_cert_manager.py index 1d46632549..da7c557066 100644 --- a/magnum/tests/unit/conductor/handlers/common/test_cert_manager.py +++ b/magnum/tests/unit/conductor/handlers/common/test_cert_manager.py @@ -17,6 +17,15 @@ import mock from magnum.common import exception from magnum.conductor.handlers.common import cert_manager from magnum.tests import base +from oslo_config import cfg + +import magnum.conf +import os +import stat +import tempfile + + +CONF = magnum.conf.CONF class CertManagerTestCase(base.BaseTestCase): @@ -229,6 +238,154 @@ class CertManagerTestCase(base.BaseTestCase): context=None) self.assertEqual(mock_ca_cert, cluster_ca_cert) + def test_create_client_files_notin_cache(self): + mock_cluster = mock.MagicMock() + mock_cluster.uuid = "mock_cluster_uuid" + mock_dir = tempfile.mkdtemp() + cert_dir = os.path.join(mock_dir, + mock_cluster.uuid) + cfg.CONF.set_override("temp_cache_dir", mock_dir, group='cluster') + + mock_ca_return = '%s/ca.crt' % cert_dir + mock_key_return = '%s/client.key' % cert_dir + mock_magnum_return = '%s/client.crt' % cert_dir + + mock_cert = mock.MagicMock() + mock_cert.get_certificate.return_value = "some_content" + mock_cert.get_decrypted_private_key.return_value = "some_key" + self.CertManager.get_cert.return_value = \ + mock_cert + + # Test that directory and files DNE + self.assertEqual(False, os.path.isdir(cert_dir)) + self.assertEqual(False, os.path.isfile(mock_ca_return)) + self.assertEqual(False, os.path.isfile(mock_key_return)) + self.assertEqual(False, os.path.isfile(mock_magnum_return)) + + (cluster_ca_cert, cluster_key, cluster_magnum_cert) = \ + cert_manager.create_client_files(mock_cluster) + + # Test the directory and files were created + self.assertEqual(True, os.path.isdir(cert_dir)) + self.assertEqual(True, os.path.isfile(mock_ca_return)) + self.assertEqual(True, os.path.isfile(mock_key_return)) + self.assertEqual(True, os.path.isfile(mock_magnum_return)) + + # Test that all functions were called in the if not conditional + self.assertEqual(self.CertManager.get_cert.call_count, 2) + self.assertEqual(mock_cert.get_certificate.call_count, 2) + self.assertEqual(mock_cert.get_decrypted_private_key.call_count, 1) + + # Test that contents were written to files & returned properly + cluster_ca_cert.seek(0) + cluster_key.seek(0) + cluster_magnum_cert.seek(0) + self.assertEqual(mock_cert.get_certificate.return_value, + cluster_ca_cert.read()) + self.assertEqual(mock_cert.get_decrypted_private_key.return_value, + cluster_key.read()) + self.assertEqual(mock_cert.get_certificate.return_value, + cluster_magnum_cert.read()) + + @mock.patch('magnum.conductor.handlers.common.cert_manager.LOG') + def test_create_client_files_temp_no_dir(self, mock_logging): + mock_cluster = mock.MagicMock() + mock_cluster.uuid = "mock_cluster_uuid" + + cfg.CONF.set_override("temp_cache_dir", "", group='cluster') + + mock_cert = mock.MagicMock() + mock_cert.get_certificate.return_value = "some_content" + mock_cert.get_decrypted_private_key.return_value = "some_key" + self.CertManager.get_cert.return_value = \ + mock_cert + + (cluster_ca_cert, cluster_key, cluster_magnum_cert) = \ + cert_manager.create_client_files(mock_cluster) + + mock_logging.debug.assert_called_once_with("Certificates will not be cached in the filesystem: they \ + will be created as tempfiles.") + self.assertEqual(self.CertManager.get_cert.call_count, 2) + self.assertEqual(mock_cert.get_certificate.call_count, 2) + self.assertEqual(mock_cert.get_decrypted_private_key.call_count, 1) + + # Test that contents were written to files & returned properly + cluster_ca_cert.seek(0) + cluster_key.seek(0) + cluster_magnum_cert.seek(0) + self.assertEqual(mock_cert.get_certificate.return_value, + cluster_ca_cert.read()) + self.assertEqual(mock_cert.get_decrypted_private_key.return_value, + cluster_key.read()) + self.assertEqual(mock_cert.get_certificate.return_value, + cluster_magnum_cert.read()) + + def test_create_client_files_in_cache(self): + mock_cluster = mock.MagicMock() + mock_cluster.uuid = "mock_cluster_uuid" + mock_dir = tempfile.mkdtemp() + cfg.CONF.set_override("temp_cache_dir", mock_dir, group='cluster') + + mock_cert = mock.MagicMock() + mock_cert.get_certificate.return_value = "some_content" + mock_cert.get_decrypted_private_key.return_value = "some_key" + self.CertManager.get_cert.return_value = \ + mock_cert + + # First call creates directory and writes files + (cluster_ca_cert, cluster_key, cluster_magnum_cert) = \ + cert_manager.create_client_files(mock_cluster) + + # Establish call count baseline + self.assertEqual(self.CertManager.get_cert.call_count, 2) + self.assertEqual(mock_cert.get_certificate.call_count, 2) + self.assertEqual(mock_cert.get_decrypted_private_key.call_count, 1) + + # Second call to create_client_files for same cluster should enter else + # conditional, open cached file and return file contents unchanged. + (cluster_ca_cert, cluster_key, cluster_magnum_cert) = \ + cert_manager.create_client_files(mock_cluster) + + # Test that function call count did not increase. + self.assertEqual(self.CertManager.get_cert.call_count, 2) + self.assertEqual(mock_cert.get_certificate.call_count, 2) + self.assertEqual(mock_cert.get_decrypted_private_key.call_count, 1) + + # Check that original file contents/return values have not changed + self.assertEqual(mock_cert.get_certificate.return_value, + cluster_ca_cert.read()) + self.assertEqual(mock_cert.get_decrypted_private_key.return_value, + cluster_key.read()) + self.assertEqual(mock_cert.get_certificate.return_value, + cluster_magnum_cert.read()) + + def test_create_client_files_set_file_permissions(self): + mock_cluster = mock.MagicMock() + mock_cluster.uuid = "mock_cluster_uuid" + mock_dir = tempfile.mkdtemp() + cert_dir = os.path.join(mock_dir, + mock_cluster.uuid) + cfg.CONF.set_override("temp_cache_dir", mock_dir, group='cluster') + + mock_ca_return = '%s/ca.crt' % cert_dir + mock_key_return = '%s/client.key' % cert_dir + mock_magnum_return = '%s/client.crt' % cert_dir + + mock_cert = mock.MagicMock() + mock_cert.get_certificate.return_value = "some_content" + mock_cert.get_decrypted_private_key.return_value = "some_key" + self.CertManager.get_cert.return_value = \ + mock_cert + + cert_manager.create_client_files(mock_cluster) + + ca_permission = stat.S_IMODE(os.lstat(mock_ca_return).st_mode) + self.assertEqual(ca_permission, 0o600) + key_permission = stat.S_IMODE(os.lstat(mock_key_return).st_mode) + self.assertEqual(key_permission, 0o600) + magnum_permission = stat.S_IMODE(os.lstat(mock_magnum_return).st_mode) + self.assertEqual(magnum_permission, 0o600) + def test_delete_certtificate(self): mock_delete_cert = self.CertManager.delete_cert expected_cert_ref = 'cert_ref' @@ -272,3 +429,54 @@ class CertManagerTestCase(base.BaseTestCase): cert_manager.delete_certificates_from_cluster(mock_cluster) self.assertFalse(mock_delete_cert.called) + + def test_delete_client_files(self): + mock_cluster = mock.MagicMock() + mock_cluster.uuid = "mock_cluster_uuid" + mock_dir = tempfile.mkdtemp() + cert_dir = os.path.join(mock_dir, + mock_cluster.uuid) + cfg.CONF.set_override("temp_cache_dir", mock_dir, group='cluster') + + mock_ca_return = '%s/ca.crt' % cert_dir + mock_key_return = '%s/client.key' % cert_dir + mock_magnum_return = '%s/client.crt' % cert_dir + + mock_cert = mock.MagicMock() + mock_cert.get_certificate.return_value = "some_content" + mock_cert.get_decrypted_private_key.return_value = "some_key" + self.CertManager.get_cert.return_value = \ + mock_cert + + (cluster_ca_cert, cluster_key, cluster_magnum_cert) = \ + cert_manager.create_client_files(mock_cluster) + + # Test the directory and files were created + self.assertEqual(True, os.path.isdir(cert_dir)) + self.assertEqual(True, os.path.isfile(mock_ca_return)) + self.assertEqual(True, os.path.isfile(mock_key_return)) + self.assertEqual(True, os.path.isfile(mock_magnum_return)) + + cert_manager.delete_client_files(mock_cluster) + + # Test that directory and files DNE + self.assertEqual(False, os.path.isdir(cert_dir)) + self.assertEqual(False, os.path.isfile(mock_ca_return)) + self.assertEqual(False, os.path.isfile(mock_key_return)) + self.assertEqual(False, os.path.isfile(mock_magnum_return)) + + def test_delete_client_files_none(self): + mock_cluster = mock.MagicMock() + mock_cluster.uuid = "mock_cluster_uuid" + mock_dir = tempfile.mkdtemp() + cfg.CONF.set_override("temp_cache_dir", mock_dir, group='cluster') + cert_dir = os.path.join(mock_dir, + mock_cluster.uuid) + + self.assertEqual(True, os.path.isdir(mock_dir)) + self.assertEqual(False, os.path.isdir(cert_dir)) + + cert_manager.delete_client_files(mock_cluster) + + self.assertEqual(True, os.path.isdir(mock_dir)) + self.assertEqual(False, os.path.isdir(cert_dir))