Policyd override implementation

This patchset implements policy overrides for heat.  It uses the
code in charmhelpers.

It also fixes a bug in the actions/domain-setup where it assumes that
the python2 version of openstackclient should be installed, and corrects
this via code in hooks/install and hooks/upgrade-charm.

A sync of charm-helpers is included to bring the latest policyd changes
through to the charm.

func-test-pr: https://github.com/openstack-charmers/zaza-openstack-tests/pull/111

Change-Id: Ia607dc9120cfb03902efb019041b43cf12ade2d3
Closed-Bug: #1741723
This commit is contained in:
Alex Kavanagh 2019-11-06 11:44:06 +00:00
parent a963596ed5
commit 98de623820
17 changed files with 296 additions and 85 deletions

View File

@ -89,3 +89,47 @@ alternatively these can also be provided as part of a juju native bundle configu
NOTE: Spaces must be configured in the underlying provider prior to attempting to use them.
NOTE: Existing deployments using os-*-network configuration options will continue to function; these options are preferred over any network space binding provided if set.
Policy Overrides
----------------
This feature allows for policy overrides using the `policy.d` directory. This
is an **advanced** feature and the policies that the OpenStack service supports
should be clearly and unambiguously understood before trying to override, or
add to, the default policies that the service uses. The charm also has some
policy defaults. They should also be understood before being overridden.
> **Caution**: It is possible to break the system (for tenants and other
services) if policies are incorrectly applied to the service.
Policy overrides are YAML files that contain rules that will add to, or
override, existing policy rules in the service. The `policy.d` directory is
a place to put the YAML override files. This charm owns the
`/etc/heat/policy.d` directory, and as such, any manual changes to it will
be overwritten on charm upgrades.
Overrides are provided to the charm using a Juju resource called
`policyd-override`. The resource is a ZIP file. This file, say
`overrides.zip`, is attached to the charm by:
juju attach-resource heat policyd-override=overrides.zip
The policy override is enabled in the charm using:
juju config heat use-policyd-override=true
When `use-policyd-override` is `True` the status line of the charm will be
prefixed with `PO:` indicating that policies have been overridden. If the
installation of the policy override YAML files failed for any reason then the
status line will be prefixed with `PO (broken):`. The log file for the charm
will indicate the reason. No policy override files are installed if the `PO
(broken):` is shown. The status line indicates that the overrides are broken,
not that the policy for the service has failed. The policy will be the defaults
for the charm and service.
Policy overrides on one service may affect the functionality of another
service. Therefore, it may be necessary to provide policy overrides for
multiple service charms to achieve a consistent set of policies across the
OpenStack system. The charms for the other services that may need overrides
should be checked to ensure that they support overrides before proceeding.

View File

@ -4,8 +4,6 @@ set -e
. /root/admin-openrc-v3
dpkg -l | grep python-openstackclient || apt-get install -y python-openstackclient
openstack domain show heat || {
openstack domain create --description "Stack projects and users" heat
}

View File

@ -254,3 +254,11 @@ options:
description: |
Connect timeout configuration in ms for haproxy, used in HA
configurations. If not provided, default value of 9000ms is used.
use-policyd-override:
type: boolean
default: False
description: |
If True then use the resource file named 'policyd-override' to install
override YAML files in the service's policy.d directory. The resource
file should be a ZIP file containing at least one yaml file with a .yaml
or .yml extension. If False then remove the overrides.

View File

@ -1940,7 +1940,7 @@ class VolumeAPIContext(InternalEndpointContext):
as well as the catalog_info string that would be supplied. Returns
a dict containing the volume_api_version and the volume_catalog_info.
"""
rel = os_release(self.pkg, base='icehouse')
rel = os_release(self.pkg)
version = '2'
if CompareOpenStackReleases(rel) >= 'pike':
version = '3'
@ -2140,7 +2140,7 @@ class VersionsContext(OSContextGenerator):
self.pkg = pkg
def __call__(self):
ostack = os_release(self.pkg, base='icehouse')
ostack = os_release(self.pkg)
osystem = lsb_release()['DISTRIB_CODENAME'].lower()
return {
'openstack_release': ostack,

View File

@ -17,6 +17,7 @@ import contextlib
import os
import six
import shutil
import sys
import yaml
import zipfile
@ -115,8 +116,8 @@ library for further details).
default: False
description: |
If True then use the resource file named 'policyd-override' to install
override yaml files in the service's policy.d directory. The resource
file should be a zip file containing at least one yaml file with a .yaml
override YAML files in the service's policy.d directory. The resource
file should be a ZIP file containing at least one yaml file with a .yaml
or .yml extension. If False then remove the overrides.
"""
@ -134,14 +135,14 @@ resources:
Policy Overrides
----------------
This service allows for policy overrides using the `policy.d` directory. This
is an **advanced** feature and the policies that the service supports should be
clearly and unambiguously understood before trying to override, or add to, the
default policies that the service uses.
This feature allows for policy overrides using the `policy.d` directory. This
is an **advanced** feature and the policies that the OpenStack service supports
should be clearly and unambiguously understood before trying to override, or
add to, the default policies that the service uses. The charm also has some
policy defaults. They should also be understood before being overridden.
The charm also has some policy defaults. They should also be understood before
being overridden. It is possible to break the system (for tenants and other
services) if policies are incorrectly applied to the service.
> **Caution**: It is possible to break the system (for tenants and other
services) if policies are incorrectly applied to the service.
Policy overrides are YAML files that contain rules that will add to, or
override, existing policy rules in the service. The `policy.d` directory is
@ -149,30 +150,16 @@ a place to put the YAML override files. This charm owns the
`/etc/keystone/policy.d` directory, and as such, any manual changes to it will
be overwritten on charm upgrades.
Policy overrides are provided to the charm using a resource file called
`policyd-override`. This is attached to the charm using (for example):
Overrides are provided to the charm using a Juju resource called
`policyd-override`. The resource is a ZIP file. This file, say
`overrides.zip`, is attached to the charm by:
juju attach-resource <charm-name> policyd-override=<some-file>
The `<charm-name>` is the name that this charm is deployed as, with
`<some-file>` being the resource file containing the policy overrides.
juju attach-resource <charm-name> policyd-override=overrides.zip
The format of the resource file is a ZIP file (.zip extension) containing at
least one YAML file with an extension of `.yaml` or `.yml`. Note that any
directories in the ZIP file are ignored; all of the files are flattened into a
single directory. There must not be any duplicated filenames; this will cause
an error and nothing in the resource file will be applied.
The policy override is enabled in the charm using:
(ed. next part is optional is the charm supports some form of
template/substitution on a read file)
If a (ed. "one or more of") [`.j2`, `.tmpl`, `.tpl`] file is found in the
resource file then the charm will perform a substitution with charm variables
taken from the config or relations. (ed. edit as appropriate to include the
variable).
To enable the policy overrides the config option `use-policyd-override` must be
set to `True`.
juju config <charm-name> use-policyd-override=true
When `use-policyd-override` is `True` the status line of the charm will be
prefixed with `PO:` indicating that policies have been overridden. If the
@ -180,12 +167,8 @@ installation of the policy override YAML files failed for any reason then the
status line will be prefixed with `PO (broken):`. The log file for the charm
will indicate the reason. No policy override files are installed if the `PO
(broken):` is shown. The status line indicates that the overrides are broken,
not that the policy for the service has failed - they will be the defaults for
the charm and service.
If the policy overrides did not install then *either* attach a new, corrected,
resource file *or* disable the policy overrides by setting
`use-policyd-override` to False.
not that the policy for the service has failed. The policy will be the defaults
for the charm and service.
Policy overrides on one service may affect the functionality of another
service. Therefore, it may be necessary to provide policy overrides for
@ -296,15 +279,28 @@ def maybe_do_policyd_overrides(openstack_release,
restarted.
:type restart_handler: Union[None, Callable[]]
"""
hookenv.log("Running maybe_do_policyd_overrides",
level=POLICYD_LOG_LEVEL_DEFAULT)
if not is_policyd_override_valid_on_this_release(openstack_release):
hookenv.log("... policy overrides not valid on this release: {}"
.format(openstack_release),
level=POLICYD_LOG_LEVEL_DEFAULT)
return
config = hookenv.config()
try:
if not config.get(POLICYD_CONFIG_NAME, False):
remove_policy_success_file()
clean_policyd_dir_for(service, blacklist_paths)
if (os.path.isfile(_policy_success_file()) and
restart_handler is not None and
callable(restart_handler)):
restart_handler()
remove_policy_success_file()
return
except Exception:
return
if not is_policyd_override_valid_on_this_release(openstack_release):
except Exception as e:
hookenv.log("... ERROR: Exception is: {}".format(str(e)),
level=POLICYD_CONFIG_NAME)
import traceback
hookenv.log(traceback.format_exc(), level=POLICYD_LOG_LEVEL_DEFAULT)
return
# from now on it should succeed; if it doesn't then status line will show
# broken.
@ -345,16 +341,30 @@ def maybe_do_policyd_overrides_on_config_changed(openstack_release,
restarted.
:type restart_handler: Union[None, Callable[]]
"""
if not is_policyd_override_valid_on_this_release(openstack_release):
return
hookenv.log("Running maybe_do_policyd_overrides_on_config_changed",
level=POLICYD_LOG_LEVEL_DEFAULT)
config = hookenv.config()
try:
if not config.get(POLICYD_CONFIG_NAME, False):
remove_policy_success_file()
clean_policyd_dir_for(service, blacklist_paths)
if (os.path.isfile(_policy_success_file()) and
restart_handler is not None and
callable(restart_handler)):
restart_handler()
remove_policy_success_file()
return
except Exception:
except Exception as e:
hookenv.log("... ERROR: Exception is: {}".format(str(e)),
level=POLICYD_CONFIG_NAME)
import traceback
hookenv.log(traceback.format_exc(), level=POLICYD_LOG_LEVEL_DEFAULT)
return
# if the policyd overrides have been performed just return
if os.path.isfile(_policy_success_file()):
hookenv.log("... already setup, so skipping.",
level=POLICYD_LOG_LEVEL_DEFAULT)
return
maybe_do_policyd_overrides(
openstack_release, service, blacklist_paths, blacklist_keys,
@ -430,8 +440,13 @@ def _yamlfiles(zipfile):
"""
l = []
for infolist_item in zipfile.infolist():
if infolist_item.is_dir():
continue
try:
if infolist_item.is_dir():
continue
except AttributeError:
# fallback to "old" way to determine dir entry for pre-py36
if infolist_item.filename.endswith('/'):
continue
_, name_ext = os.path.split(infolist_item.filename)
name, ext = os.path.splitext(name_ext)
ext = ext.lower()
@ -511,7 +526,7 @@ def clean_policyd_dir_for(service, keep_paths=None):
path = policyd_dir_for(service)
if not os.path.exists(path):
ch_host.mkdir(path, owner=service, group=service, perms=0o775)
_scanner = os.scandir if six.PY3 else _py2_scandir
_scanner = os.scandir if sys.version_info > (3, 4) else _py2_scandir
for direntry in _scanner(path):
# see if the path should be kept.
if direntry.path in keep_paths:
@ -641,6 +656,7 @@ def process_policy_resource_file(resource_file,
:returns: True if the processing was successful, False if not.
:rtype: boolean
"""
hookenv.log("Running process_policy_resource_file", level=hookenv.DEBUG)
blacklist_paths = blacklist_paths or []
completed = False
try:

View File

@ -0,0 +1,19 @@
[placement]
{% if auth_host -%}
auth_url = {{ auth_protocol }}://{{ auth_host }}:{{ auth_port }}
auth_type = password
{% if api_version == "3" -%}
project_domain_name = {{ admin_domain_name }}
user_domain_name = {{ admin_domain_name }}
{% else -%}
project_domain_name = default
user_domain_name = default
{% endif -%}
project_name = {{ admin_tenant_name }}
username = {{ admin_user }}
password = {{ admin_password }}
{% endif -%}
{% if region -%}
os_region_name = {{ region }}
{% endif -%}
randomize_allocation_candidates = true

View File

@ -204,7 +204,7 @@ SWIFT_CODENAMES = OrderedDict([
('stein',
['2.20.0', '2.21.0']),
('train',
['2.22.0']),
['2.22.0', '2.23.0']),
])
# >= Liberty version->codename mapping
@ -531,7 +531,7 @@ def reset_os_release():
_os_rel = None
def os_release(package, base='essex', reset_cache=False):
def os_release(package, base=None, reset_cache=False):
'''
Returns OpenStack release codename from a cached global.
@ -542,6 +542,8 @@ def os_release(package, base='essex', reset_cache=False):
the installation source, the earliest release supported by the charm should
be returned.
'''
if not base:
base = UBUNTU_OPENSTACK_RELEASE[lsb_release()['DISTRIB_CODENAME']]
global _os_rel
if reset_cache:
reset_os_release()
@ -670,7 +672,10 @@ def openstack_upgrade_available(package):
codename = get_os_codename_install_source(src)
avail_vers = get_os_version_codename_swift(codename)
else:
avail_vers = get_os_version_install_source(src)
try:
avail_vers = get_os_version_install_source(src)
except:
avail_vers = cur_vers
apt.init()
return apt.version_compare(avail_vers, cur_vers) >= 1
@ -1693,7 +1698,7 @@ def enable_memcache(source=None, release=None, package=None):
if release:
_release = release
else:
_release = os_release(package, base='icehouse')
_release = os_release(package)
if not _release:
_release = get_os_codename_install_source(source)

View File

@ -422,6 +422,8 @@ def enabled_manager_modules():
cmd = ['ceph', 'mgr', 'module', 'ls']
try:
modules = check_output(cmd)
if six.PY3:
modules = modules.decode('UTF-8')
except CalledProcessError as e:
log("Failed to list ceph modules: {}".format(e), WARNING)
return []
@ -1185,6 +1187,15 @@ class CephBrokerRq(object):
self.request_id = str(uuid.uuid1())
self.ops = []
def add_op(self, op):
"""Add an op if it is not already in the list.
:param op: Operation to add.
:type op: dict
"""
if op not in self.ops:
self.ops.append(op)
def add_op_request_access_to_group(self, name, namespace=None,
permission=None, key_name=None,
object_prefix_permissions=None):
@ -1198,7 +1209,7 @@ class CephBrokerRq(object):
'rwx': ['prefix1', 'prefix2'],
'class-read': ['prefix3']}
"""
self.ops.append({
self.add_op({
'op': 'add-permissions-to-key', 'group': name,
'namespace': namespace,
'name': key_name or service_name(),
@ -1251,11 +1262,11 @@ class CephBrokerRq(object):
if pg_num and weight:
raise ValueError('pg_num and weight are mutually exclusive')
self.ops.append({'op': 'create-pool', 'name': name,
'replicas': replica_count, 'pg_num': pg_num,
'weight': weight, 'group': group,
'group-namespace': namespace, 'app-name': app_name,
'max-bytes': max_bytes, 'max-objects': max_objects})
self.add_op({'op': 'create-pool', 'name': name,
'replicas': replica_count, 'pg_num': pg_num,
'weight': weight, 'group': group,
'group-namespace': namespace, 'app-name': app_name,
'max-bytes': max_bytes, 'max-objects': max_objects})
def add_op_create_erasure_pool(self, name, erasure_profile=None,
weight=None, group=None, app_name=None,
@ -1283,12 +1294,12 @@ class CephBrokerRq(object):
:param max_objects: Maximum objects quota to apply
:type max_objects: int
"""
self.ops.append({'op': 'create-pool', 'name': name,
'pool-type': 'erasure',
'erasure-profile': erasure_profile,
'weight': weight,
'group': group, 'app-name': app_name,
'max-bytes': max_bytes, 'max-objects': max_objects})
self.add_op({'op': 'create-pool', 'name': name,
'pool-type': 'erasure',
'erasure-profile': erasure_profile,
'weight': weight,
'group': group, 'app-name': app_name,
'max-bytes': max_bytes, 'max-objects': max_objects})
def set_ops(self, ops):
"""Set request ops to provided value.

View File

@ -119,6 +119,24 @@ def log(message, level=None):
raise
def action_log(message):
"""Write an action progress message"""
command = ['action-log']
if not isinstance(message, six.string_types):
message = repr(message)
command += [message[:SH_MAX_ARG]]
# Missing action-log should not cause failures in unit tests
# Send action_log output to stderr
try:
subprocess.call(command)
except OSError as e:
if e.errno == errno.ENOENT:
message = "action-log: {}".format(message)
print(message, file=sys.stderr)
else:
raise
class Serializable(UserDict):
"""Wrapper, an object that can be serialized to yaml or json"""

View File

@ -61,9 +61,10 @@ from charmhelpers.contrib.network.ip import (
from charmhelpers.contrib.openstack.utils import (
configure_installation_source,
openstack_upgrade_available,
sync_db_with_multi_ipv6_addresses,
series_upgrade_prepare,
os_release,
series_upgrade_complete,
series_upgrade_prepare,
sync_db_with_multi_ipv6_addresses,
)
from charmhelpers.contrib.openstack.ha.utils import (
@ -107,6 +108,8 @@ from charmhelpers.contrib.openstack.cert_utils import (
process_certificates,
)
import charmhelpers.contrib.openstack.policyd as policyd
hooks = Hooks()
CONFIGS = register_configs()
@ -130,6 +133,16 @@ def install():
for port in API_PORTS.values():
open_port(port)
# call the policy overrides handler which will install any policy overrides
policyd.maybe_do_policyd_overrides(
os_release('heat-common'),
'heat',
restart_handler=restart_heat_api,
)
def restart_heat_api():
service_restart('heat-api')
@hooks.hook('config-changed')
@ -156,8 +169,15 @@ def config_changed():
for r_id in relation_ids('ha'):
ha_joined(relation_id=r_id)
# call the policy overrides handler which will install any policy overrides
policyd.maybe_do_policyd_overrides_on_config_changed(
os_release('heat-common'),
'heat',
restart_handler=restart_heat_api,
)
@hooks.hook('upgrade-charm')
@hooks.hook('upgrade-charm.real')
@harden()
def upgrade_charm():
apt_install(determine_packages(), fatal=True)
@ -185,6 +205,12 @@ def upgrade_charm():
# now we just delete the file
os.remove(encryption_path)
leader_elected()
# call the policy overrides handler which will install any policy overrides
policyd.maybe_do_policyd_overrides(
os_release('heat-common'),
'heat',
restart_handler=restart_heat_api,
)
@hooks.hook('amqp-relation-joined')

View File

@ -2,7 +2,15 @@
# Wrapper to deal with newer Ubuntu versions that don't have py2 installed
# by default.
declare -a DEPS=('apt' 'netaddr' 'netifaces' 'pip' 'yaml' 'dnspython')
declare -a DEPS=('apt' 'netaddr' 'netifaces' 'pip' 'yaml' 'dnspython' 'openstackclient')
# drop this when trusty support is ended. Only need python3 at that point
release=$(lsb_release -c -s)
if [ "$release" == "trusty" ]; then
PYTHON="python"
else
PYTHON="python3"
fi
check_and_install() {
pkg="${1}-${2}"
@ -11,8 +19,6 @@ check_and_install() {
fi
}
PYTHON="python"
for dep in ${DEPS[@]}; do
check_and_install ${PYTHON} ${dep}
done

View File

@ -1 +0,0 @@
heat_relations.py

30
hooks/upgrade-charm Executable file
View File

@ -0,0 +1,30 @@
#!/bin/bash -e
# Wrapper to deal with newer Ubuntu versions that don't have py2 installed
# by default.
declare -a DEPS=('apt' 'netaddr' 'netifaces' 'pip' 'yaml' 'dnspython' 'openstackclient')
# drop this when trusty support is ended. Only need python3 at that point
release=$(lsb_release -c -s)
if [ "$release" == "trusty" ]; then
PYTHON="python"
else
PYTHON="python3"
# if python-openstackclient is installed then remove it
if dpkg -s python-openstackclient 2>&1 > /dev/null; then
apt purge -y python-openstackclient
fi
fi
check_and_install() {
pkg="${1}-${2}"
if ! dpkg -s ${pkg} 2>&1 > /dev/null; then
apt-get -y install ${pkg}
fi
}
for dep in ${DEPS[@]}; do
check_and_install ${PYTHON} ${dep}
done
exec ./hooks/upgrade-charm.real

1
hooks/upgrade-charm.real Symbolic link
View File

@ -0,0 +1 @@
heat_relations.py

View File

@ -35,3 +35,8 @@ requires:
peers:
cluster:
interface: heat-ha
resources:
policyd-override:
type: file
filename: policyd-override.zip
description: The policy.d overrides file

View File

@ -58,7 +58,7 @@ applications:
charm: cs:~openstack-charmers-next/neutron-openvswitch
heat:
charm: ../../../heat
num_units: 2
num_units: 1
series: bionic
constraints: mem=2048
options:

View File

@ -1,8 +1,4 @@
charm_name: heat
configure:
- zaza.openstack.charm_tests.glance.setup.add_cirros_image
- zaza.openstack.charm_tests.nova.setup.manage_ssh_key
- zaza.openstack.charm_tests.nova.setup.create_flavors
smoke_bundles:
- bionic-train
gate_bundles:
@ -18,5 +14,13 @@ gate_bundles:
- disco-stein
dev_bundles:
- bionic-train
configure:
- zaza.openstack.charm_tests.glance.setup.add_cirros_image
- zaza.openstack.charm_tests.nova.setup.manage_ssh_key
- zaza.openstack.charm_tests.nova.setup.create_flavors
tests:
- zaza.openstack.charm_tests.heat.tests.HeatBasicDeployment
- zaza.openstack.charm_tests.policyd.tests.HeatTests
tests_options:
policyd:
service: heat

View File

@ -57,10 +57,11 @@ TO_PATCH = [
'restart_on_change',
'service_restart',
# charmhelpers.contrib.openstack.utils
'configure_installation_source',
'openstack_upgrade_available',
'determine_packages',
'charm_dir',
'configure_installation_source',
'determine_packages',
'openstack_upgrade_available',
'os_release',
'sync_db_with_multi_ipv6_addresses',
# charmhelpers.contrib.openstack.ha.utils
'generate_ha_relation_data',
@ -90,7 +91,8 @@ class HeatRelationTests(CharmTestCase):
self.config.side_effect = self.test_config.get
self.charm_dir.return_value = '/var/lib/juju/charms/heat/charm'
def test_install_hook(self):
@patch.object(relations.policyd, 'maybe_do_policyd_overrides')
def test_install_hook(self, mock_maybe_do_policyd_overrides):
repo = 'cloud:precise-havana'
self.determine_packages.return_value = [
'python-keystoneclient', 'uuid', 'heat-api',
@ -106,20 +108,36 @@ class HeatRelationTests(CharmTestCase):
self.assertTrue(self.execd_preinstall.called)
@patch.object(relations, 'configure_https')
def test_config_changed_no_upgrade(self, mock_configure_https):
@patch.object(relations.policyd,
'maybe_do_policyd_overrides_on_config_changed')
def test_config_changed_no_upgrade(
self,
maybe_do_policyd_overrides_on_config_changed,
mock_configure_https
):
self.openstack_upgrade_available.return_value = False
relations.config_changed()
@patch.object(relations, 'configure_https')
def test_config_changed_with_upgrade(self, mock_configure_https):
@patch.object(relations.policyd,
'maybe_do_policyd_overrides_on_config_changed')
def test_config_changed_with_upgrade(
self,
maybe_do_policyd_overrides_on_config_changed,
mock_configure_https
):
self.openstack_upgrade_available.return_value = True
relations.config_changed()
self.assertTrue(self.do_openstack_upgrade.called)
@patch.object(relations, 'configure_https')
@patch.object(relations.policyd,
'maybe_do_policyd_overrides_on_config_changed')
def test_config_changed_with_openstack_upgrade_action(
self,
mock_configure_https):
self,
maybe_do_policyd_overrides_on_config_changed,
mock_configure_https
):
self.openstack_upgrade_available.return_value = True
self.test_config.set('action-managed-upgrade', True)
@ -130,7 +148,10 @@ class HeatRelationTests(CharmTestCase):
@patch('os.path.isfile')
@patch('os.remove')
@patch.object(relations, 'leader_elected')
def test_upgrade_charm(self, leader_elected, os_remove, os_path_isfile):
@patch.object(relations.policyd, 'maybe_do_policyd_overrides')
def test_upgrade_charm(self,
mock_maybe_do_policyd_overrides,
leader_elected, os_remove, os_path_isfile):
os_path_isfile.return_value = False
self.is_leader.return_value = False
self.remove_old_packages.return_value = True