From 275a8f61c69a435763f893c60335616db546423e Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Thu, 21 Sep 2017 12:11:17 +0100 Subject: [PATCH] Ensure auth_encryption_key is identical for all units This patch ensures that if multiple heat units are deployer, that each one will have the same auth_encryption_key in the /etc/heat/heat.conf. This is automatically generated on the (juju) leader and then remains unchanged for the application's duration. It can be overriden by the config setting 'encryption-key'. Testing is via the amulet 500 test (added) which checkes that the two units deployed have the same key. Change-Id: I89a11efe772314acd58ab9be21773eee89a23980 Closes-Bug: #1714157 --- hooks/heat_context.py | 29 ++++------------ hooks/heat_relations.py | 35 +++++++++++++++++-- tests/basic_deployment.py | 58 ++++++++++++++++++++++++++++--- unit_tests/test_heat_relations.py | 30 +++++++++++++++- 4 files changed, 122 insertions(+), 30 deletions(-) diff --git a/hooks/heat_context.py b/hooks/heat_context.py index c04d39f..6b003da 100644 --- a/hooks/heat_context.py +++ b/hooks/heat_context.py @@ -12,11 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -import os - from charmhelpers.contrib.openstack import context from charmhelpers.core.hookenv import config, leader_get -from charmhelpers.core.host import pwgen from charmhelpers.contrib.hahelpers.cluster import ( determine_apache_port, determine_api_port, @@ -51,21 +48,10 @@ class HeatIdentityServiceContext(context.IdentityServiceContext): def get_encryption_key(): - encryption_path = os.path.join(HEAT_PATH, 'encryption-key') - if os.path.isfile(encryption_path): - with open(encryption_path, 'r') as enc: - encryption = enc.read() - else: - # create encryption key and store it - if not os.path.isdir(HEAT_PATH): - os.makedirs(HEAT_PATH) - encryption = config("encryption-key") - if not encryption: - # generate random key - encryption = pwgen(16) - with open(encryption_path, 'w') as enc: - enc.write(encryption) - return encryption + encryption_key = config("encryption-key") + if not encryption_key: + encryption_key = leader_get('heat-auth-encryption-key') + return encryption_key class HeatSecurityContext(context.OSContextGenerator): @@ -73,10 +59,9 @@ class HeatSecurityContext(context.OSContextGenerator): def __call__(self): ctxt = {} # check if we have stored encryption key - encryption = get_encryption_key() - ctxt['encryption_key'] = encryption - ctxt['heat_domain_admin_passwd'] = \ - leader_get('heat-domain-admin-passwd') + ctxt['encryption_key'] = get_encryption_key() + ctxt['heat_domain_admin_passwd'] = ( + leader_get('heat-domain-admin-passwd')) return ctxt diff --git a/hooks/heat_relations.py b/hooks/heat_relations.py index 513deb4..a8cdf86 100755 --- a/hooks/heat_relations.py +++ b/hooks/heat_relations.py @@ -95,6 +95,7 @@ from heat_utils import ( from heat_context import ( API_PORTS, + HEAT_PATH, ) from charmhelpers.contrib.openstack.context import ADDRESS_TYPES @@ -154,6 +155,25 @@ def config_changed(): @hooks.hook('upgrade-charm') @harden() def upgrade_charm(): + if is_leader(): + # if we are upgrading, then the old version might have used the + # HEAT_PATH/encryption-key. So we grab the key from that, and put it in + # leader settings to ensure that the key remains the same during an + # upgrade. + encryption_path = os.path.join(HEAT_PATH, 'encryption-key') + if os.path.isfile(encryption_path): + with open(encryption_path, 'r') as f: + encryption_key = f.read() + try: + leader_set({'heat-auth-encryption-key': encryption_key}) + except subprocess.CalledProcessError as e: + log("upgrade: leader_set: heat-auth-encryption-key failed," + " didn't delete the existing file: {}.\n" + "Error was: ".format(encryption_path, str(e)), + level=WARNING) + else: + # now we just delete the file + os.remove(encryption_path) leader_elected() @@ -283,8 +303,19 @@ def relation_broken(): @hooks.hook('leader-elected') def leader_elected(): - if is_leader() and not leader_get('heat-domain-admin-passwd'): - leader_set({'heat-domain-admin-passwd': pwgen(32)}) + if is_leader(): + if not leader_get('heat-domain-admin-passwd'): + try: + leader_set({'heat-domain-admin-passwd': pwgen(32)}) + except subprocess.CalledProcessError as e: + log('leader_set: heat-domain-admin-password failed: {}' + .format(str(e)), level=WARNING) + if not leader_get('heat-auth-encryption-key'): + try: + leader_set({'heat-auth-encryption-key': pwgen(32)}) + except subprocess.CalledProcessError as e: + log('leader_set: heat-domain-admin-password failed: {}' + .format(str(e)), level=WARNING) @hooks.hook('cluster-relation-joined') diff --git a/tests/basic_deployment.py b/tests/basic_deployment.py index 9074f3d..c2fbca3 100644 --- a/tests/basic_deployment.py +++ b/tests/basic_deployment.py @@ -17,6 +17,9 @@ """ Basic heat functional test. """ +import json +import subprocess + import amulet from heatclient.common import template_utils @@ -69,7 +72,9 @@ class HeatBasicDeployment(OpenStackAmuletDeployment): and the rest of the service are from lp branches that are compatible with the local charm (e.g. stable or next). """ - this_service = {'name': 'heat', 'constraints': {'mem': '2G'}} + this_service = {'name': 'heat', + 'constraints': {'mem': '2G'}, + 'units': 2} other_services = [ {'name': 'keystone'}, {'name': 'rabbitmq-server'}, @@ -417,10 +422,7 @@ class HeatBasicDeployment(OpenStackAmuletDeployment): expected = { 'private-address': u.valid_ip, 'db_host': u.valid_ip, - 'heat_allowed_units': '{}/{}'.format( - self.heat_sentry.info['service'], - self.heat_sentry.info['unit'] - ), + 'heat_allowed_units': u.not_null, 'heat_password': u.not_null } @@ -613,6 +615,52 @@ class HeatBasicDeployment(OpenStackAmuletDeployment): self._image_delete() self._keypair_delete() + def test_500_auth_encryption_key_same_on_units(self): + """Test that the auth_encryption_key in heat.conf is the same on all of + the units. + """ + u.log.debug("Checking the 'auth_encryption_key' is the same on " + "all units.") + output, ret = self._run_arbitrary( + "--application heat " + "--format json " + "grep auth_encryption_key /etc/heat/heat.conf") + if ret: + msg = "juju run returned error: ({}) -> {}".format(ret, output) + amulet.raise_status(amulet.FAIL, msg=msg) + output = json.loads(output) + keys = {} + for r in output: + k = r['Stdout'].split('=')[1].strip() + keys[r['UnitId']] = k + # see if keys are different. + ks = keys.values() + if any(((k != ks[0]) for k in ks[1:])): + msg = ("'auth_encryption_key' is not identical on every unit: {}" + .format("{}={}".format(k, v) for k, v in keys.items())) + amulet.raise_status(amulet.FAIL, msg=msg) + + @staticmethod + def _run_arbitrary(command, timeout=300): + """Run an arbitrary command (as root), but not necessarily on a unit. + + (Otherwise the self.run(...) command could have been used for the unit + + :param str command: The command to run. + :param int timeout: Seconds to wait before timing out. + :return: A 2-tuple containing the output of the command and the exit + code of the command. + """ + cmd = ['juju', 'run', '--timeout', "{}s".format(timeout), + ] + command.split() + p = subprocess.Popen( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + stdout, stderr = p.communicate() + output = stdout if p.returncode == 0 else stderr + return output.decode('utf8').strip(), p.returncode + def test_900_heat_restart_on_config_change(self): """Verify that the specified services are restarted when the config is changed.""" diff --git a/unit_tests/test_heat_relations.py b/unit_tests/test_heat_relations.py index 6f55a49..8631682 100644 --- a/unit_tests/test_heat_relations.py +++ b/unit_tests/test_heat_relations.py @@ -12,10 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os.path import sys from mock import call, patch, MagicMock -from test_utils import CharmTestCase +from test_utils import CharmTestCase, patch_open # python-apt is not installed as part of test-requirements but is imported by # some charmhelpers modules so create a fake import. @@ -44,6 +45,9 @@ TO_PATCH = [ # charmhelpers.core.hookenv 'Hooks', 'config', + 'is_leader', + 'leader_set', + 'log', 'open_port', 'relation_set', 'related_units', @@ -123,6 +127,30 @@ class HeatRelationTests(CharmTestCase): self.assertFalse(self.do_openstack_upgrade.called) + @patch('os.path.isfile') + @patch('os.remove') + @patch.object(relations, 'leader_elected') + def test_upgrade_charm(self, leader_elected, os_remove, os_path_isfile): + os_path_isfile.return_value = False + self.is_leader.return_value = False + relations.upgrade_charm() + leader_elected.assert_called_once_with() + os_path_isfile.assert_not_called() + # now say we are the leader + self.is_leader.return_value = True + os_path_isfile.return_value = False + relations.upgrade_charm() + self.leader_set.assert_not_called() + os_path_isfile.return_value = True + with patch_open() as (mock_open, mock_file): + mock_file.read.return_value = "abc" + relations.upgrade_charm() + file = os.path.join(relations.HEAT_PATH, 'encryption-key') + mock_open.assert_called_once_with(file, 'r') + self.leader_set.assert_called_once_with( + {'heat-auth-encryption-key': 'abc'}) + os_remove.assert_called_once_with(file) + def test_db_joined(self): self.get_relation_ip.return_value = '192.168.20.1' relations.db_joined()