diff --git a/hooks/glance_contexts.py b/hooks/glance_contexts.py index 04c9ff98..eb547475 100644 --- a/hooks/glance_contexts.py +++ b/hooks/glance_contexts.py @@ -71,6 +71,35 @@ class GlanceContext(OSContextGenerator): return ctxt +class GlancePolicyContext(OSContextGenerator): + """This Context is only used from Ussuri onwards. At Ussuri, Glance + implemented policy-in-code, and thus didn't ship with a policy.json. + Therefore, the charm introduces a 'policy.yaml' file that is used to + provide the override here. + + Note that this is separate from policy overrides as it's a charm config + option that has existed prior to its introduction. + + 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. + """ + + def __call__(self): + if config('restrict-image-location-operations'): + policy_value = 'role:admin' + else: + policy_value = '' + + ctxt = { + "get_image_location": policy_value, + "set_image_location": policy_value, + "delete_image_location": policy_value, + } + return ctxt + + class CephGlanceContext(OSContextGenerator): interfaces = ['ceph-glance'] diff --git a/hooks/glance_relations.py b/hooks/glance_relations.py index 8c234908..ac24b180 100755 --- a/hooks/glance_relations.py +++ b/hooks/glance_relations.py @@ -272,7 +272,7 @@ def object_store_joined(): return [image_service_joined(rid) for rid in relation_ids('image-service')] - update_image_location_policy() + update_image_location_policy(CONFIGS) CONFIGS.write(GLANCE_API_CONF) @@ -400,7 +400,7 @@ def config_changed(): # NOTE(jamespage): trigger any configuration related changes # for cephx permissions restrictions ceph_changed() - update_image_location_policy() + update_image_location_policy(CONFIGS) # call the policy overrides handler which will install any policy overrides maybe_do_policyd_overrides_on_config_changed( @@ -443,6 +443,9 @@ def upgrade_charm(): reinstall_paste_ini(force_reinstall=packages_removed) configure_https() update_nrpe_config() + # NOTE(ajkavanagh) the update_image_location_policy() call below isn't + # called with CONFIGS as the config files all get re-written after the + # call. update_image_location_policy() CONFIGS.write_all() if packages_removed: diff --git a/hooks/glance_utils.py b/hooks/glance_utils.py index 23cbaf4e..8e3e8168 100644 --- a/hooks/glance_utils.py +++ b/hooks/glance_utils.py @@ -36,6 +36,7 @@ from charmhelpers.core.hookenv import ( config, log, INFO, + WARNING, relation_ids, service_name, ) @@ -119,6 +120,11 @@ GLANCE_REGISTRY_PASTE = os.path.join(GLANCE_CONF_DIR, GLANCE_API_PASTE = os.path.join(GLANCE_CONF_DIR, 'glance-api-paste.ini') GLANCE_POLICY_FILE = os.path.join(GLANCE_CONF_DIR, "policy.json") +# NOTE(ajkavanagh): from Ussuri, glance switched to policy-in-code; this is the +# policy.yaml file (as there is not packaged policy.json or .yaml) that is used +# to provide the image_location override config value: +# 'restrict-image-location-operations' +GLANCE_POLICY_YAML = os.path.join(GLANCE_CONF_DIR, "policy.yaml") CEPH_CONF = "/etc/ceph/ceph.conf" CHARM_CEPH_CONF = '/var/lib/charm/{}/ceph.conf' @@ -220,6 +226,10 @@ CONFIG_FILES = OrderedDict([ 'contexts': [], 'services': ['apache2'], }), + (GLANCE_POLICY_YAML, { + 'hook_contexts': [glance_contexts.GlancePolicyContext()], + 'services': [], + }), ]) @@ -270,8 +280,8 @@ def register_configs(): CONFIG_FILES[GLANCE_SWIFT_CONF]['hook_contexts']) if cmp_release >= 'ussuri': - configs.register(GLANCE_POLICY_FILE, - CONFIG_FILES[GLANCE_POLICY_FILE]['hook_contexts']) + configs.register(GLANCE_POLICY_YAML, + CONFIG_FILES[GLANCE_POLICY_YAML]['hook_contexts']) return configs @@ -599,38 +609,79 @@ def is_api_ready(configs): return (not incomplete_relation_data(configs, REQUIRED_INTERFACES)) -def update_image_location_policy(): +def update_image_location_policy(configs=None): """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. + + For ussuri, the charm updates/writes the policy.yaml file. The configs + param is optional as the caller may already be writing all the configs. + From ussuri onwards glance is policy-in-code (rather than using a + policy.json) and, therefore, policy files are essentially all overrides. + + From ussuri, this function deletes the policy.json file and alternatively + writes the GLANCE_POLICY_YAML file via the configs object. + + :param configs: The configs for the charm + :type configs: Optional[:class:templating.OSConfigRenderer()] """ - if CompareOpenStackReleases(os_release('glance-common')) < 'kilo': + _res = os_release('glance-common') + cmp = CompareOpenStackReleases(_res) + if cmp < 'kilo': # NOTE(hopem): at the time of writing we are unable to do this for # earlier than Kilo due to LP: #1502136 return + if cmp >= 'ussuri': + # If the policy.json exists, then remove it as it's the packaged + # version from a previous version of OpenStack, and thus not used. + if os.path.isfile(GLANCE_POLICY_FILE): + try: + os.remove(GLANCE_POLICY_FILE) + except Exception as e: + log("Problem removing file: {}: {}" + .format(GLANCE_POLICY_FILE, str(e))) + # if the caller supplied a configs param then update the + # GLANCE_POLICY_FILE using its context. + if configs is not None: + configs.write(GLANCE_POLICY_YAML) + return + # otherwise the OpenStack release after kilo and before ussuri, so continue + # modifying the existing policy.json file. db = kv() policies = ["get_image_location", "set_image_location", "delete_image_location"] + + try: + with open(GLANCE_POLICY_FILE) as f: + pmap = json.load(f) + except IOError as e: + log("Problem opening glance policy file: {}. Error was:{}" + .format(GLANCE_POLICY_FILE, str(e)), + level=WARNING) + return + 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]) + if policy_key in pmap: + db.set(db_key, pmap[policy_key]) db.flush() else: log("key '{}' not found in policy file".format(policy_key), level=INFO) - if config('restrict-image-location-operations'): - policy_value = 'role:admin' - else: - policy_value = '' + if config('restrict-image-location-operations'): + policy_value = 'role:admin' + else: + policy_value = '' + new_policies = {k: policy_value for k in policies} + for policy_key, policy_value in new_policies.items(): log("Updating Glance policy file setting policy " - "'{}':'{}'".format(policy_key, policy_value), level=INFO) - update_json_file(GLANCE_POLICY_FILE, {policy_key: policy_value}) + "'{}': '{}'".format(policy_key, policy_value), level=INFO) + + update_json_file(GLANCE_POLICY_FILE, new_policies) diff --git a/templates/ussuri/policy.json b/templates/ussuri/policy.json deleted file mode 100644 index 9ccdb5c9..00000000 --- a/templates/ussuri/policy.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "delete_image_location": "rule:default", - "get_image_location": "rule:default", - "set_image_location": "rule:default" -} diff --git a/templates/ussuri/policy.yaml b/templates/ussuri/policy.yaml new file mode 100644 index 00000000..f15efec5 --- /dev/null +++ b/templates/ussuri/policy.yaml @@ -0,0 +1,5 @@ +# The file is controlled by Juju -- do not edit by hand as it will be +# overwritten +"get_image_location": "{{ get_image_location }}" +"set_image_location": "{{ set_image_location }}" +"delete_image_location": "{{ delete_image_location }}" diff --git a/tests/bundles/focal-ussuri.yaml b/tests/bundles/focal-ussuri.yaml index 54c9bcac..1f4d9d9e 100644 --- a/tests/bundles/focal-ussuri.yaml +++ b/tests/bundles/focal-ussuri.yaml @@ -1,3 +1,6 @@ +variables: + openstack-origin: &openstack-origin distro + series: focal comment: @@ -13,32 +16,48 @@ machines: '3': '4': -relations: - - ["keystone:shared-db", "keystone-mysql-router:shared-db"] - - ["glance:shared-db", "glance-mysql-router:shared-db"] - - ["glance:identity-service", "keystone:identity-service"] - - ["glance-mysql-router:db-router", "mysql-innodb-cluster:db-router"] - - ["keystone-mysql-router:db-router", "mysql-innodb-cluster:db-router"] - applications: + glance-mysql-router: charm: cs:~openstack-charmers-next/mysql-router keystone-mysql-router: charm: cs:~openstack-charmers-next/mysql-router + mysql-innodb-cluster: charm: cs:~openstack-charmers-next/mysql-innodb-cluster num_units: 3 + options: + source: *openstack-origin to: - '0' - '1' - '2' + keystone: charm: cs:~openstack-charmers-next/keystone num_units: 1 + options: + openstack-origin: *openstack-origin to: - '3' + glance: charm: ../../../glance num_units: 1 + options: + openstack-origin: *openstack-origin to: - '4' + +relations: + + - - "keystone:shared-db" + - "keystone-mysql-router:shared-db" + - - "glance:shared-db" + - "glance-mysql-router:shared-db" + - - "glance:identity-service" + - "keystone:identity-service" + - - "glance-mysql-router:db-router" + - "mysql-innodb-cluster:db-router" + - - "keystone-mysql-router:db-router" + - "mysql-innodb-cluster:db-router" diff --git a/unit_tests/test_glance_utils.py b/unit_tests/test_glance_utils.py index 78346fa5..f5d016a4 100644 --- a/unit_tests/test_glance_utils.py +++ b/unit_tests/test_glance_utils.py @@ -471,32 +471,30 @@ class TestGlanceUtils(CharmTestCase): utils.update_image_location_policy() self.assertFalse(mock_kv.called) + # Function has no effect in ussuri onwards + mock_os_release.return_value = 'ussuri' + 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) + mock_update_json_file.assert_has_calls([ call('/etc/glance/policy.json', - {'get_image_location': ''}), - call('/etc/glance/policy.json', - {'set_image_location': ''}), - call('/etc/glance/policy.json', - {'delete_image_location': ''})]) + {'get_image_location': '', + 'set_image_location': '', + 'delete_image_location': ''})]) mock_update_json_file.reset_mock() config['restrict-image-location-operations'] = True utils.update_image_location_policy() mock_update_json_file.assert_has_calls([ call('/etc/glance/policy.json', - {'get_image_location': 'role:admin'}), - call('/etc/glance/policy.json', - {'set_image_location': 'role:admin'}), - call('/etc/glance/policy.json', - {'delete_image_location': 'role:admin'})]) + {'get_image_location': 'role:admin', + 'set_image_location': 'role:admin', + 'delete_image_location': 'role:admin'})]) 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', - '')])