diff --git a/charmhelpers/contrib/openstack/utils.py b/charmhelpers/contrib/openstack/utils.py index 9e5af342..e1d852db 100644 --- a/charmhelpers/contrib/openstack/utils.py +++ b/charmhelpers/contrib/openstack/utils.py @@ -2045,14 +2045,25 @@ def token_cache_pkgs(source=None, release=None): def update_json_file(filename, items): """Updates the json `filename` with a given dict. - :param filename: json filename (i.e.: /etc/glance/policy.json) + :param filename: path to json file (e.g. /etc/glance/policy.json) :param items: dict of items to update """ + if not items: + return + with open(filename) as fd: policy = json.load(fd) + + # Compare before and after and if nothing has changed don't write the file + # since that could cause unnecessary service restarts. + before = json.dumps(policy, indent=4, sort_keys=True) policy.update(items) + after = json.dumps(policy, indent=4, sort_keys=True) + if before == after: + return + with open(filename, "w") as fd: - fd.write(json.dumps(policy, indent=4)) + fd.write(after) @cached diff --git a/hooks/glance_relations.py b/hooks/glance_relations.py index a770b4c7..b74afa50 100755 --- a/hooks/glance_relations.py +++ b/hooks/glance_relations.py @@ -41,6 +41,7 @@ from glance_utils import ( assess_status, reinstall_paste_ini, is_api_ready, + update_image_location_policy, ) from charmhelpers.core.hookenv import ( config, @@ -118,7 +119,6 @@ from charmhelpers.contrib.openstack.context import ( from charmhelpers.contrib.charmsupport import nrpe from charmhelpers.contrib.hardening.harden import harden - hooks = Hooks() CONFIGS = register_configs() @@ -290,7 +290,7 @@ def object_store_joined(): return [image_service_joined(rid) for rid in relation_ids('image-service')] - + update_image_location_policy() CONFIGS.write(GLANCE_API_CONF) @@ -421,6 +421,7 @@ def config_changed(): # NOTE(jamespage): trigger any configuration related changes # for cephx permissions restrictions ceph_changed() + update_image_location_policy() @hooks.hook('cluster-relation-joined') @@ -456,6 +457,7 @@ def upgrade_charm(): reinstall_paste_ini() configure_https() update_nrpe_config() + update_image_location_policy() CONFIGS.write_all() diff --git a/hooks/glance_utils.py b/hooks/glance_utils.py index 7506c4ce..837f2b41 100644 --- a/hooks/glance_utils.py +++ b/hooks/glance_utils.py @@ -14,6 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json import os import shutil import subprocess @@ -37,6 +38,7 @@ from charmhelpers.core.hookenv import ( charm_dir, config, log, + INFO, relation_ids, service_name, ) @@ -87,6 +89,7 @@ from charmhelpers.contrib.openstack.utils import ( pause_unit, resume_unit, token_cache_pkgs, + update_json_file, ) from charmhelpers.core.templating import render @@ -142,6 +145,7 @@ GLANCE_REGISTRY_PASTE = os.path.join(GLANCE_CONF_DIR, 'glance-registry-paste.ini') GLANCE_API_PASTE = os.path.join(GLANCE_CONF_DIR, 'glance-api-paste.ini') +GLANCE_POLICY_FILE = os.path.join(GLANCE_CONF_DIR, "policy.json") CEPH_CONF = "/etc/ceph/ceph.conf" CHARM_CEPH_CONF = '/var/lib/charm/{}/ceph.conf' @@ -345,6 +349,8 @@ def restart_map(): if enable_memcache(source=config('openstack-origin')): _map.append((MEMCACHED_CONF, ['memcached'])) + _map.append((GLANCE_POLICY_FILE, ['glance-api', 'glance-registry'])) + return OrderedDict(_map) @@ -684,3 +690,36 @@ def reinstall_paste_ini(): def is_api_ready(configs): return (not incomplete_relation_data(configs, REQUIRED_INTERFACES)) + + +def update_image_location_policy(): + """Update *_image_location policy to restrict to admin role. + + We do this unconditonally and keep a record of the original as installed by + the package. + """ + if CompareOpenStackReleases(os_release('glance-common')) < 'kilo': + # NOTE(hopem): at the time of writing we are unable to do this for + # earlier than Kilo due to LP: #1502136 + return + + db = kv() + policies = ["get_image_location", "set_image_location", + "delete_image_location"] + for policy_key in policies: + # Save original value at time of first install in case we ever need to + # revert. + db_key = "policy_{}".format(policy_key) + if db.get(db_key) is None: + p = json.loads(open(GLANCE_POLICY_FILE).read()) + if policy_key in p: + db.set(db_key, p[policy_key]) + db.flush() + else: + log("key '{}' not found in policy file".format(policy_key), + level=INFO) + + policy_value = 'role:admin' + log("Updating Glance policy file setting policy " + "'{}':'{}'".format(policy_key, policy_value), level=INFO) + update_json_file(GLANCE_POLICY_FILE, {policy_key: policy_value}) diff --git a/unit_tests/test_glance_relations.py b/unit_tests/test_glance_relations.py index 81952f58..1527f74f 100644 --- a/unit_tests/test_glance_relations.py +++ b/unit_tests/test_glance_relations.py @@ -397,9 +397,10 @@ class GlanceRelationTests(CharmTestCase): 'swift relation incomplete' ) + @patch.object(relations, 'update_image_location_policy') @patch.object(relations, 'CONFIGS') def test_object_store_joined_with_identity_service_with_object_store( - self, configs): + self, configs, mock_update_image_location_policy): configs.complete_contexts = MagicMock() configs.complete_contexts.return_value = ['identity-service', 'object-store'] @@ -407,6 +408,18 @@ class GlanceRelationTests(CharmTestCase): relations.object_store_joined() self.assertEqual([call('/etc/glance/glance-api.conf')], configs.write.call_args_list) + self.assertTrue(mock_update_image_location_policy.called) + + @patch.object(relations, 'update_image_location_policy') + @patch.object(relations, 'CONFIGS') + def test_object_store_joined_with_expose_image_locations_false( + self, configs, mock_update_image_location_policy): + configs.complete_contexts = MagicMock() + configs.complete_contexts.return_value = ['identity-service', + 'object-store'] + configs.write = MagicMock() + relations.object_store_joined() + self.assertTrue(mock_update_image_location_policy.called) def test_ceph_joined(self): relations.ceph_joined() @@ -616,22 +629,27 @@ class GlanceRelationTests(CharmTestCase): relations.keystone_changed() [self.assertIn(call(r), imgsj.call_args_list) for r in rids] + @patch.object(relations, 'update_image_location_policy') @patch.object(relations, 'configure_https') @patch.object(relations, 'git_install_requested') def test_config_changed_no_openstack_upgrade(self, git_requested, - configure_https): + configure_https, + mock_update_policy): git_requested.return_value = False self.openstack_upgrade_available.return_value = False relations.config_changed() self.open_port.assert_called_with(9292) self.assertTrue(configure_https.called) + self.assertTrue(mock_update_policy.called) + @patch.object(relations, 'update_image_location_policy') @patch.object(relations, 'status_set') @patch.object(relations, 'configure_https') @patch.object(relations, 'git_install_requested') def test_config_changed_with_openstack_upgrade(self, git_requested, configure_https, - status): + status, + mock_update_policy): git_requested.return_value = False self.openstack_upgrade_available.return_value = True relations.config_changed() @@ -641,21 +659,27 @@ class GlanceRelationTests(CharmTestCase): ) self.assertTrue(self.do_openstack_upgrade.called) self.assertTrue(configure_https.called) + self.assertTrue(mock_update_policy.called) + @patch.object(relations, 'update_image_location_policy') @patch.object(relations, 'git_install_requested') - def test_config_changed_with_openstack_upgrade_action(self, git_requested): + def test_config_changed_with_openstack_upgrade_action(self, git_requested, + mock_update_policy): git_requested.return_value = False self.openstack_upgrade_available.return_value = True self.test_config.set('action-managed-upgrade', True) relations.config_changed() self.assertFalse(self.do_openstack_upgrade.called) + self.assertTrue(mock_update_policy.called) + @patch.object(relations, 'update_image_location_policy') @patch.object(relations, 'configure_https') @patch.object(relations, 'git_install_requested') @patch.object(relations, 'config_value_changed') def test_config_changed_git_updated(self, config_val_changed, - git_requested, configure_https): + git_requested, configure_https, + mock_update_image_location_policy): git_requested.return_value = True repo = 'cloud:trusty-juno' openstack_origin_git = { @@ -676,6 +700,7 @@ class GlanceRelationTests(CharmTestCase): self.git_install.assert_called_with(projects_yaml) self.assertFalse(self.do_openstack_upgrade.called) self.assertTrue(configure_https.called) + self.assertTrue(mock_update_image_location_policy.called) @patch.object(relations, 'CONFIGS') def test_cluster_changed(self, configs): @@ -704,18 +729,20 @@ class GlanceRelationTests(CharmTestCase): call('/etc/haproxy/haproxy.cfg')], configs.write.call_args_list) + @patch.object(relations, 'update_image_location_policy') @patch.object(utils, 'config') @patch.object(utils, 'token_cache_pkgs') @patch.object(relations, 'CONFIGS') @patch.object(utils, 'git_install_requested') def test_upgrade_charm(self, git_requested, configs, token_cache_pkgs, - util_config): + util_config, mock_update_image_location_policy): git_requested.return_value = False self.filter_installed_packages.return_value = ['test'] relations.upgrade_charm() self.apt_install.assert_called_with(['test'], fatal=True) self.assertTrue(configs.write_all.called) self.assertTrue(self.reinstall_paste_ini.called) + self.assertTrue(mock_update_image_location_policy.called) def test_ha_relation_joined(self): self.get_hacluster_config.return_value = { diff --git a/unit_tests/test_glance_utils.py b/unit_tests/test_glance_utils.py index df6665e3..2640e773 100644 --- a/unit_tests/test_glance_utils.py +++ b/unit_tests/test_glance_utils.py @@ -15,7 +15,7 @@ import os from collections import OrderedDict -from mock import patch, call, MagicMock +from mock import patch, call, MagicMock, mock_open os.environ['JUJU_UNIT_NAME'] = 'glance' import hooks.glance_utils as utils @@ -145,7 +145,8 @@ class TestGlanceUtils(CharmTestCase): (utils.HAPROXY_CONF, ['haproxy']), (utils.HTTPS_APACHE_CONF, ['apache2']), (utils.HTTPS_APACHE_24_CONF, ['apache2']), - (utils.MEMCACHED_CONF, ['memcached']) + (utils.MEMCACHED_CONF, ['memcached']), + (utils.GLANCE_POLICY_FILE, ['glance-api', 'glance-registry']) ]) self.assertEqual(ex_map, utils.restart_map()) self.enable_memcache.return_value = False @@ -442,3 +443,40 @@ class TestGlanceUtils(CharmTestCase): def test_is_api_ready_false(self): self._test_is_api_ready(False) + + @patch.object(utils, 'json') + @patch.object(utils, 'update_json_file') + @patch.object(utils, 'kv') + @patch.object(utils, 'os_release') + def test_update_image_location_policy(self, mock_os_release, mock_kv, + mock_update_json_file, mock_json): + db_vals = {} + + def fake_db_get(key): + return db_vals.get(key) + + db_obj = mock_kv.return_value + db_obj.get = MagicMock() + db_obj.get.side_effect = fake_db_get + db_obj.set = MagicMock() + + fake_open = mock_open() + with patch.object(utils, 'open', fake_open, create=True): + mock_json.loads.return_value = {'get_image_location': '', + 'set_image_location': '', + 'delete_image_location': ''} + + mock_os_release.return_value = 'icehouse' + utils.update_image_location_policy() + self.assertFalse(mock_kv.called) + + mock_os_release.return_value = 'kilo' + utils.update_image_location_policy() + self.assertTrue(mock_kv.called) + db_obj.get.assert_has_calls([call('policy_get_image_location'), + call('policy_set_image_location'), + call('policy_delete_image_location')]) + db_obj.set.assert_has_calls([call('policy_get_image_location', ''), + call('policy_set_image_location', ''), + call('policy_delete_image_location', + '')])