Policyd override implementation

This patchset implements policy overrides for neutron-gateway.

This change includes a charm-helpers sync to bring in the policyd helper
code.

Change-Id: I89f1f4b5d58843017e428a8d2cfada840dde14de
Closes-Bug: #1741723

README.md

@@ -135,7 +135,7 @@ To use this feature, use the --bind option when deploying the charm:
 
     juju deploy neutron-api --bind "public=public-space internal=internal-space admin=admin-space shared-db=internal-space"
 
-alternatively these can also be provided as part of a juju native
+Alternatively these can also be provided as part of a juju native
 bundle configuration:
 
     neutron-api:
@@ -187,3 +187,46 @@ middleware types - these are the prefixes the charm code validates
 passed data against:
 
 https://bitbucket.org/ianb/pastedeploy/src/4b27133a2a7db58b213ae55b580039c11d2055c0/paste/deploy/loadwsgi.py?at=default&fileviewer=file-view-default
+
+# 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/keystone/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 neutron-api policyd-override=overrides.zip
+
+The policy override is enabled in the charm using:
+
+    juju config neutron-api 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.

config.yaml

@@ -747,3 +747,11 @@ options:
     description: |
       Sets the resource type used in weight calculations during
       AZ-aware scheduling (networks, subnets or ports).
+  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.

hooks/charmhelpers/contrib/openstack/policyd.py

@@ -299,10 +299,17 @@ def maybe_do_policyd_overrides(openstack_release,
     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:
+        print("Exception is: ", str(e))
+        import traceback
+        traceback.print_exc()
         return
     if not is_policyd_override_valid_on_this_release(openstack_release):
         return
@@ -348,8 +355,12 @@ def maybe_do_policyd_overrides_on_config_changed(openstack_release,
     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
@@ -430,8 +441,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()

hooks/neutron_api_hooks.py

@@ -140,6 +140,11 @@ from charmhelpers.contrib.openstack.cert_utils import (
     process_certificates,
 )
 
+from charmhelpers.contrib.openstack.policyd import (
+    maybe_do_policyd_overrides,
+    maybe_do_policyd_overrides_on_config_changed,
+)
+
 from charmhelpers.contrib.openstack.context import ADDRESS_TYPES
 
 from charmhelpers.contrib.charmsupport import nrpe
@@ -215,11 +220,17 @@ def install():
     packages = determine_packages(openstack_origin)
     apt_install(packages, fatal=True)
 
-    [open_port(port) for port in determine_ports()]
+    for port in determine_ports():
+        open_port(port)
 
     if neutron_plugin == 'midonet':
         mkdir('/etc/neutron/plugins/midonet', owner='neutron', group='neutron',
               perms=0o755, force=False)
+    # call the policy overrides handler which will install any policy overrides
+    maybe_do_policyd_overrides(
+        os_release('neutron-server'),
+        'neutron',
+        restart_handler=lambda: service_restart('neutron-server'))
 
 
 @hooks.hook('vsd-rest-api-relation-joined')
@@ -258,10 +269,31 @@ def vsd_changed(relation_id=None, remote_unit=None):
 
 
 @hooks.hook('upgrade-charm')
+@restart_on_change(restart_map(), stopstart=True)
+@harden()
+def upgrade_charm():
+    common_upgrade_charm_and_config_changed()
+    # call the policy overrides handler which will install any policy overrides
+    maybe_do_policyd_overrides(
+        os_release('neutron-server'),
+        'neutron',
+        restart_handler=lambda: service_restart('neutron-server'))
+
+
 @hooks.hook('config-changed')
 @restart_on_change(restart_map(), stopstart=True)
 @harden()
 def config_changed():
+    common_upgrade_charm_and_config_changed()
+    # call the policy overrides handler which will install any policy overrides
+    maybe_do_policyd_overrides_on_config_changed(
+        os_release('neutron-server'),
+        'neutron',
+        restart_handler=lambda: service_restart('neutron-server'))
+
+
+def common_upgrade_charm_and_config_changed():
+    """Common code between upgrade-charm and config-changed hooks"""
     # if we are paused, delay doing any config changed hooks.
     # It is forced on the resume.
     if is_unit_paused_set():
@@ -328,7 +360,8 @@ def config_changed():
         ha_joined(relation_id=r_id)
     for r_id in relation_ids('neutron-plugin-api-subordinate'):
         neutron_plugin_api_subordinate_relation_joined(relid=r_id)
-    [cluster_joined(rid) for rid in relation_ids('cluster')]
+    for rid in relation_ids('cluster'):
+        cluster_joined(rid)
 
 
 @hooks.hook('amqp-relation-joined')

metadata.yaml

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

tests/tests.yaml

@@ -22,8 +22,13 @@ tests:
   - zaza.openstack.charm_tests.neutron.tests.NeutronApiTest
   - zaza.openstack.charm_tests.neutron.tests.SecurityTest
   - zaza.openstack.charm_tests.neutron.tests.NeutronNetworkingTest
+  - zaza.openstack.charm_tests.policyd.tests.NeutronApiTests
 configure:
   - zaza.openstack.charm_tests.glance.setup.add_lts_image
   - zaza.openstack.charm_tests.neutron.setup.basic_overcloud_network
   - zaza.openstack.charm_tests.nova.setup.create_flavors
   - zaza.openstack.charm_tests.nova.setup.manage_ssh_key
+  - zaza.openstack.charm_tests.keystone.setup.add_demo_user
+tests_options:
+  policyd:
+    service: neutron

unit_tests/test_neutron_api_hooks.py

@@ -94,6 +94,8 @@ TO_PATCH = [
     'services',
     'service_restart',
     'is_db_initialised',
+    'maybe_do_policyd_overrides',
+    'maybe_do_policyd_overrides_on_config_changed',
 ]
 NEUTRON_CONF_DIR = "/etc/neutron"