Port policy from oslo and refactor heat policy

This patch is to refactor Heat's usage to allow bringing the
policy code from oslo up to the current version to address
many fixed bugs which affect Heat.

    1. Refactor the heat wrapper policy.
    2. Add netaddr in requirement.txt.
    3. Add necessary unit test to cover the refactor part code.

Oslo commit: 3626b6db91ce1e897d1993fb4e13ac4237d04b7f
Fixes bug #1192551

Change-Id: Ia71d14b2cfde9110216ba38a515fee16d0f0c35e

6 files changed, 424 insertions, 191 deletions

diff --git a/heat/openstack/common/excutils.py b/heat/openstack/common/excutils.py
index 47791cbc0..1619e012c 100644
--- a/heat/openstack/common/excutils.py
+++ b/heat/openstack/common/excutils.py
@@ -19,16 +19,15 @@
 Exception related utilities.
 """
 
-import contextlib
 import logging
 import sys
+import time
 import traceback
 
-from heat.openstack.common.gettextutils import _
+from heat.openstack.common.gettextutils import _  # noqa
 
 
-@contextlib.contextmanager
-def save_and_reraise_exception():
+class save_and_reraise_exception(object):
     """Save current exception, run some code and then re-raise.
 
     In some cases the exception context can be cleared, resulting in None
@@ -40,12 +39,60 @@ def save_and_reraise_exception():
     To work around this, we save the exception state, run handler code, and
     then re-raise the original exception. If another exception occurs, the
     saved exception is logged and the new exception is re-raised.
-    """
-    type_, value, tb = sys.exc_info()
-    try:
-        yield
+
+    In some cases the caller may not want to re-raise the exception, and
+    for those circumstances this context provides a reraise flag that
+    can be used to suppress the exception.  For example:
+
     except Exception:
-        logging.error(_('Original exception being dropped: %s'),
-                      traceback.format_exception(type_, value, tb))
-        raise
-    raise type_, value, tb
+        with save_and_reraise_exception() as ctxt:
+            decide_if_need_reraise()
+            if not should_be_reraised:
+                ctxt.reraise = False
+    """
+    def __init__(self):
+        self.reraise = True
+
+    def __enter__(self):
+        self.type_, self.value, self.tb, = sys.exc_info()
+        return self
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        if exc_type is not None:
+            logging.error(_('Original exception being dropped: %s'),
+                          traceback.format_exception(self.type_,
+                                                     self.value,
+                                                     self.tb))
+            return False
+        if self.reraise:
+            raise self.type_, self.value, self.tb
+
+
+def forever_retry_uncaught_exceptions(infunc):
+    def inner_func(*args, **kwargs):
+        last_log_time = 0
+        last_exc_message = None
+        exc_count = 0
+        while True:
+            try:
+                return infunc(*args, **kwargs)
+            except Exception as exc:
+                if exc.message == last_exc_message:
+                    exc_count += 1
+                else:
+                    exc_count = 1
+                # Do not log any more frequently than once a minute unless
+                # the exception message changes
+                cur_time = int(time.time())
+                if (cur_time - last_log_time > 60 or
+                        exc.message != last_exc_message):
+                    logging.exception(
+                        _('Unexpected exception occurred %d time(s)... '
+                          'retrying.') % exc_count)
+                    last_log_time = cur_time
+                    last_exc_message = exc.message
+                    exc_count = 0
+                # This should be a very rare event. In case it isn't, do
+                # a sleep.
+                time.sleep(1)
+    return inner_func
diff --git a/heat/openstack/common/fileutils.py b/heat/openstack/common/fileutils.py
new file mode 100644
index 000000000..542801d55
--- /dev/null
+++ b/heat/openstack/common/fileutils.py
@@ -0,0 +1,110 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack Foundation.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+
+import contextlib
+import errno
+import os
+
+from heat.openstack.common import excutils
+from heat.openstack.common.gettextutils import _  # noqa
+from heat.openstack.common import log as logging
+
+LOG = logging.getLogger(__name__)
+
+_FILE_CACHE = {}
+
+
+def ensure_tree(path):
+    """Create a directory (and any ancestor directories required)
+
+    :param path: Directory to create
+    """
+    try:
+        os.makedirs(path)
+    except OSError as exc:
+        if exc.errno == errno.EEXIST:
+            if not os.path.isdir(path):
+                raise
+        else:
+            raise
+
+
+def read_cached_file(filename, force_reload=False):
+    """Read from a file if it has been modified.
+
+    :param force_reload: Whether to reload the file.
+    :returns: A tuple with a boolean specifying if the data is fresh
+              or not.
+    """
+    global _FILE_CACHE
+
+    if force_reload and filename in _FILE_CACHE:
+        del _FILE_CACHE[filename]
+
+    reloaded = False
+    mtime = os.path.getmtime(filename)
+    cache_info = _FILE_CACHE.setdefault(filename, {})
+
+    if not cache_info or mtime > cache_info.get('mtime', 0):
+        LOG.debug(_("Reloading cached file %s") % filename)
+        with open(filename) as fap:
+            cache_info['data'] = fap.read()
+        cache_info['mtime'] = mtime
+        reloaded = True
+    return (reloaded, cache_info['data'])
+
+
+def delete_if_exists(path):
+    """Delete a file, but ignore file not found error.
+
+    :param path: File to delete
+    """
+
+    try:
+        os.unlink(path)
+    except OSError as e:
+        if e.errno == errno.ENOENT:
+            return
+        else:
+            raise
+
+
+@contextlib.contextmanager
+def remove_path_on_error(path):
+    """Protect code that wants to operate on PATH atomically.
+    Any exception will cause PATH to be removed.
+
+    :param path: File to work with
+    """
+    try:
+        yield
+    except Exception:
+        with excutils.save_and_reraise_exception():
+            delete_if_exists(path)
+
+
+def file_open(*args, **kwargs):
+    """Open file
+
+    see built-in file() documentation for more details
+
+    Note: The reason this is kept in a separate module is to easily
+    be able to provide a stub module that doesn't alter system
+    state at all (for unit tests)
+    """
+    return file(*args, **kwargs)
diff --git a/heat/openstack/common/jsonutils.py b/heat/openstack/common/jsonutils.py
index 2592f6fa9..94fbecff9 100644
--- a/heat/openstack/common/jsonutils.py
+++ b/heat/openstack/common/jsonutils.py
@@ -41,6 +41,7 @@ import json
 import types
 import xmlrpclib
 
+import netaddr
 import six
 
 from heat.openstack.common import timeutils
@@ -137,6 +138,8 @@ def to_primitive(value, convert_instances=False, convert_datetime=True,
             # Likely an instance of something. Watch for cycles.
             # Ignore class member vars.
             return recursive(value.__dict__, level=level + 1)
+        elif isinstance(value, netaddr.IPAddress):
+            return six.text_type(value)
         else:
             if any(test(value) for test in _nasty_type_tests):
                 return six.text_type(value)
diff --git a/heat/openstack/common/log.py b/heat/openstack/common/log.py
index 84113dc97..94c85147e 100644
--- a/heat/openstack/common/log.py
+++ b/heat/openstack/common/log.py
@@ -29,8 +29,6 @@ It also allows setting of formatting information through conf.
 
 """
 
-import ConfigParser
-import cStringIO
 import inspect
 import itertools
 import logging
@@ -41,8 +39,9 @@ import sys
 import traceback
 
 from oslo.config import cfg
+from six import moves
 
-from heat.openstack.common.gettextutils import _
+from heat.openstack.common.gettextutils import _  # noqa
 from heat.openstack.common import importutils
 from heat.openstack.common import jsonutils
 from heat.openstack.common import local
@@ -74,7 +73,8 @@ logging_cli_opts = [
     cfg.StrOpt('log-format',
                default=None,
                metavar='FORMAT',
-               help='A logging.Formatter log message format string which may '
+               help='DEPRECATED. '
+                    'A logging.Formatter log message format string which may '
                     'use any of the available logging.LogRecord attributes. '
                     'This option is deprecated.  Please use '
                     'logging_context_format_string and '
@@ -347,7 +347,7 @@ class LogConfigError(Exception):
 def _load_log_config(log_config):
     try:
         logging.config.fileConfig(log_config)
-    except ConfigParser.Error as exc:
+    except moves.configparser.Error as exc:
         raise LogConfigError(log_config, str(exc))
 
 
@@ -520,7 +520,7 @@ class ContextFormatter(logging.Formatter):
         if not record:
             return logging.Formatter.formatException(self, exc_info)
 
-        stringbuffer = cStringIO.StringIO()
+        stringbuffer = moves.StringIO()
         traceback.print_exception(exc_info[0], exc_info[1], exc_info[2],
                                   None, stringbuffer)
         lines = stringbuffer.getvalue().split('\n')
diff --git a/heat/openstack/common/policy.py b/heat/openstack/common/policy.py
index 4c483d6c4..64455444b 100644
--- a/heat/openstack/common/policy.py
+++ b/heat/openstack/common/policy.py
@@ -59,32 +59,46 @@ as it allows particular rules to be explicitly disabled.
 import abc
 import re
 import urllib
+import urllib2
 
+from oslo.config import cfg
 import six
-import urllib2
 
-from heat.openstack.common.gettextutils import _
+from heat.openstack.common import fileutils
+from heat.openstack.common.gettextutils import _  # noqa
 from heat.openstack.common import jsonutils
 from heat.openstack.common import log as logging
 
+policy_opts = [
+    cfg.StrOpt('policy_file',
+               default='policy.json',
+               help=_('JSON file containing policy')),
+    cfg.StrOpt('policy_default_rule',
+               default='default',
+               help=_('Rule enforced when requested rule is not found')),
+]
 
-LOG = logging.getLogger(__name__)
+CONF = cfg.CONF
+CONF.register_opts(policy_opts)
 
+LOG = logging.getLogger(__name__)
 
-_rules = None
 _checks = {}
 
 
+class PolicyNotAuthorized(Exception):
+
+    def __init__(self, rule):
+        msg = _("Policy doesn't allow %s to be performed.") % rule
+        super(PolicyNotAuthorized, self).__init__(msg)
+
+
 class Rules(dict):
-    """
-    A store for rules.  Handles the default_rule setting directly.
-    """
+    """A store for rules. Handles the default_rule setting directly."""
 
     @classmethod
     def load_json(cls, data, default_rule=None):
-        """
-        Allow loading of JSON rule data.
-        """
+        """Allow loading of JSON rule data."""
 
         # Suck in the JSON data and parse the rules
         rules = dict((k, parse_rule(v)) for k, v in
@@ -101,12 +115,18 @@ class Rules(dict):
     def __missing__(self, key):
         """Implements the default rule handling."""
 
+        if isinstance(self.default_rule, dict):
+            raise KeyError(key)
+
         # If the default rule isn't actually defined, do something
         # reasonably intelligent
         if not self.default_rule or self.default_rule not in self:
             raise KeyError(key)
 
-        return self[self.default_rule]
+        if isinstance(self.default_rule, BaseCheck):
+            return self.default_rule
+        elif isinstance(self.default_rule, six.string_types):
+            return self[self.default_rule]
 
     def __str__(self):
         """Dumps a string representation of the rules."""
@@ -124,87 +144,157 @@ class Rules(dict):
         return jsonutils.dumps(out_rules, indent=4)
 
 
-# Really have to figure out a way to deprecate this
-def set_rules(rules):
-    """Set the rules in use for policy checks."""
+class Enforcer(object):
+    """Responsible for loading and enforcing rules.
 
-    global _rules
+    :param policy_file: Custom policy file to use, if none is
+                        specified, `CONF.policy_file` will be
+                        used.
+    :param rules: Default dictionary / Rules to use. It will be
+                  considered just in the first instantiation. If
+                  `load_rules(True)`, `clear()` or `set_rules(True)`
+                  is called this will be overwritten.
+    :param default_rule: Default rule to use, CONF.default_rule will
+                         be used if none is specified.
+    """
 
-    _rules = rules
+    def __init__(self, policy_file=None, rules=None, default_rule=None):
+        self.rules = Rules(rules, default_rule)
+        self.default_rule = default_rule or CONF.policy_default_rule
 
+        self.policy_path = None
+        self.policy_file = policy_file or CONF.policy_file
 
-# Ditto
-def reset():
-    """Clear the rules used for policy checks."""
+    def set_rules(self, rules, overwrite=True):
+        """Create a new Rules object based on the provided dict of rules.
 
-    global _rules
+        :param rules: New rules to use. It should be an instance of dict.
+        :param overwrite: Whether to overwrite current rules or update them
+                          with the new rules.
+        """
 
-    _rules = None
+        if not isinstance(rules, dict):
+            raise TypeError(_("Rules must be an instance of dict or Rules, "
+                            "got %s instead") % type(rules))
 
+        if overwrite:
+            self.rules = Rules(rules, self.default_rule)
+        else:
+            self.rules.update(rules)
 
-def check(rule, target, creds, exc=None, *args, **kwargs):
-    """
-    Checks authorization of a rule against the target and credentials.
-
-    :param rule: The rule to evaluate.
-    :param target: As much information about the object being operated
-                   on as possible, as a dictionary.
-    :param creds: As much information about the user performing the
-                  action as possible, as a dictionary.
-    :param exc: Class of the exception to raise if the check fails.
-                Any remaining arguments passed to check() (both
-                positional and keyword arguments) will be passed to
-                the exception class.  If exc is not provided, returns
-                False.
-
-    :return: Returns False if the policy does not allow the action and
-             exc is not provided; otherwise, returns a value that
-             evaluates to True.  Note: for rules using the "case"
-             expression, this True value will be the specified string
-             from the expression.
-    """
+    def clear(self):
+        """Clears Enforcer rules, policy's cache and policy's path."""
+        self.set_rules({})
+        self.default_rule = None
+        self.policy_path = None
 
-    # Allow the rule to be a Check tree
-    if isinstance(rule, BaseCheck):
-        result = rule(target, creds)
-    elif not _rules:
-        # No rules to reference means we're going to fail closed
-        result = False
-    else:
-        try:
-            # Evaluate the rule
-            result = _rules[rule](target, creds)
-        except KeyError:
-            # If the rule doesn't exist, fail closed
+    def load_rules(self, force_reload=False):
+        """Loads policy_path's rules.
+
+        Policy file is cached and will be reloaded if modified.
+
+        :param force_reload: Whether to overwrite current rules.
+        """
+
+        if not self.policy_path:
+            self.policy_path = self._get_policy_path()
+
+        reloaded, data = fileutils.read_cached_file(self.policy_path,
+                                                    force_reload=force_reload)
+        if reloaded or not self.rules:
+            rules = Rules.load_json(data, self.default_rule)
+            self.set_rules(rules)
+            LOG.debug(_("Rules successfully reloaded"))
+
+    def _get_policy_path(self):
+        """Locate the policy json data file.
+
+        :param policy_file: Custom policy file to locate.
+
+        :returns: The policy path
+
+        :raises: ConfigFilesNotFoundError if the file couldn't
+                 be located.
+        """
+        policy_file = CONF.find_file(self.policy_file)
+
+        if policy_file:
+            return policy_file
+
+        raise cfg.ConfigFilesNotFoundError(path=CONF.policy_file)
+
+    def enforce(self, rule, target, creds, do_raise=False,
+                exc=None, *args, **kwargs):
+        """Checks authorization of a rule against the target and credentials.
+
+        :param rule: A string or BaseCheck instance specifying the rule
+                    to evaluate.
+        :param target: As much information about the object being operated
+                    on as possible, as a dictionary.
+        :param creds: As much information about the user performing the
+                    action as possible, as a dictionary.
+        :param do_raise: Whether to raise an exception or not if check
+                        fails.
+        :param exc: Class of the exception to raise if the check fails.
+                    Any remaining arguments passed to check() (both
+                    positional and keyword arguments) will be passed to
+                    the exception class. If not specified, PolicyNotAuthorized
+                    will be used.
+
+        :return: Returns False if the policy does not allow the action and
+                exc is not provided; otherwise, returns a value that
+                evaluates to True.  Note: for rules using the "case"
+                expression, this True value will be the specified string
+                from the expression.
+        """
+
+        # NOTE(flaper87): Not logging target or creds to avoid
+        # potential security issues.
+        LOG.debug(_("Rule %s will be now enforced") % rule)
+
+        self.load_rules()
+
+        # Allow the rule to be a Check tree
+        if isinstance(rule, BaseCheck):
+            result = rule(target, creds, self)
+        elif not self.rules:
+            # No rules to reference means we're going to fail closed
             result = False
+        else:
+            try:
+                # Evaluate the rule
+                result = self.rules[rule](target, creds, self)
+            except KeyError:
+                LOG.debug(_("Rule [%s] doesn't exist") % rule)
+                # If the rule doesn't exist, fail closed
+                result = False
 
-    # If it is False, raise the exception if requested
-    if exc and result is False:
-        raise exc(*args, **kwargs)
+        # If it is False, raise the exception if requested
+        if do_raise and not result:
+            if exc:
+                raise exc(*args, **kwargs)
 
-    return result
+            raise PolicyNotAuthorized(rule)
+
+        return result
 
 
 class BaseCheck(object):
-    """
-    Abstract base class for Check classes.
-    """
+    """Abstract base class for Check classes."""
 
     __metaclass__ = abc.ABCMeta
 
     @abc.abstractmethod
     def __str__(self):
-        """
-        Retrieve a string representation of the Check tree rooted at
-        this node.
-        """
+        """String representation of the Check tree rooted at this node."""
 
         pass
 
     @abc.abstractmethod
-    def __call__(self, target, cred):
-        """
-        Perform the check.  Returns False to reject the access or a
+    def __call__(self, target, cred, enforcer):
+        """Triggers if instance of the class is called.
+
+        Performs the check. Returns False to reject the access or a
         true value (not necessary True) to accept the access.
         """
 
@@ -212,44 +302,39 @@ class BaseCheck(object):
 
 
 class FalseCheck(BaseCheck):
-    """
-    A policy check that always returns False (disallow).
-    """
+    """A policy check that always returns False (disallow)."""
 
     def __str__(self):
         """Return a string representation of this check."""
 
         return "!"
 
-    def __call__(self, target, cred):
+    def __call__(self, target, cred, enforcer):
         """Check the policy."""
 
         return False
 
 
 class TrueCheck(BaseCheck):
-    """
-    A policy check that always returns True (allow).
-    """
+    """A policy check that always returns True (allow)."""
 
     def __str__(self):
         """Return a string representation of this check."""
 
         return "@"
 
-    def __call__(self, target, cred):
+    def __call__(self, target, cred, enforcer):
         """Check the policy."""
 
         return True
 
 
 class Check(BaseCheck):
-    """
-    A base class to allow for user-defined policy checks.
-    """
+    """A base class to allow for user-defined policy checks."""
 
     def __init__(self, kind, match):
-        """
+        """Initiates Check instance.
+
         :param kind: The kind of the check, i.e., the field before the
                      ':'.
         :param match: The match of the check, i.e., the field after
@@ -266,14 +351,13 @@ class Check(BaseCheck):
 
 
 class NotCheck(BaseCheck):
-    """
+    """Implements the "not" logical operator.
+
     A policy check that inverts the result of another policy check.
-    Implements the "not" operator.
     """
 
     def __init__(self, rule):
-        """
-        Initialize the 'not' check.
+        """Initialize the 'not' check.
 
         :param rule: The rule to negate.  Must be a Check.
         """
@@ -285,24 +369,23 @@ class NotCheck(BaseCheck):
 
         return "not %s" % self.rule
 
-    def __call__(self, target, cred):
-        """
-        Check the policy.  Returns the logical inverse of the wrapped
-        check.
+    def __call__(self, target, cred, enforcer):
+        """Check the policy.
+
+        Returns the logical inverse of the wrapped check.
         """
 
-        return not self.rule(target, cred)
+        return not self.rule(target, cred, enforcer)
 
 
 class AndCheck(BaseCheck):
-    """
-    A policy check that requires that a list of other checks all
-    return True.  Implements the "and" operator.
+    """Implements the "and" logical operator.
+
+    A policy check that requires that a list of other checks all return True.
     """
 
     def __init__(self, rules):
-        """
-        Initialize the 'and' check.
+        """Initialize the 'and' check.
 
         :param rules: A list of rules that will be tested.
         """
@@ -314,10 +397,10 @@ class AndCheck(BaseCheck):
 
         return "(%s)" % ' and '.join(str(r) for r in self.rules)
 
-    def __call__(self, target, cred):
-        """
-        Check the policy.  Requires that all rules accept in order to
-        return True.
+    def __call__(self, target, cred, enforcer):
+        """Check the policy.
+
+        Requires that all rules accept in order to return True.
         """
 
         for rule in self.rules:
@@ -327,7 +410,8 @@ class AndCheck(BaseCheck):
         return True
 
     def add_check(self, rule):
-        """
+        """Adds rule to be tested.
+
         Allows addition of another rule to the list of rules that will
         be tested.  Returns the AndCheck object for convenience.
         """
@@ -337,14 +421,14 @@ class AndCheck(BaseCheck):
 
 
 class OrCheck(BaseCheck):
-    """
+    """Implements the "or" operator.
+
     A policy check that requires that at least one of a list of other
-    checks returns True.  Implements the "or" operator.
+    checks returns True.
     """
 
     def __init__(self, rules):
-        """
-        Initialize the 'or' check.
+        """Initialize the 'or' check.
 
         :param rules: A list of rules that will be tested.
         """
@@ -356,10 +440,10 @@ class OrCheck(BaseCheck):
 
         return "(%s)" % ' or '.join(str(r) for r in self.rules)
 
-    def __call__(self, target, cred):
-        """
-        Check the policy.  Requires that at least one rule accept in
-        order to return True.
+    def __call__(self, target, cred, enforcer):
+        """Check the policy.
+
+        Requires that at least one rule accept in order to return True.
         """
 
         for rule in self.rules:
@@ -369,7 +453,8 @@ class OrCheck(BaseCheck):
         return False
 
     def add_check(self, rule):
-        """
+        """Adds rule to be tested.
+
         Allows addition of another rule to the list of rules that will
         be tested.  Returns the OrCheck object for convenience.
         """
@@ -379,9 +464,7 @@ class OrCheck(BaseCheck):
 
 
 def _parse_check(rule):
-    """
-    Parse a single base check rule into an appropriate Check object.
-    """
+    """Parse a single base check rule into an appropriate Check object."""
 
     # Handle the special checks
     if rule == '!':
@@ -392,7 +475,7 @@ def _parse_check(rule):
     try:
         kind, match = rule.split(':', 1)
     except Exception:
-        LOG.exception(_("Failed to understand rule %(rule)s") % locals())
+        LOG.exception(_("Failed to understand rule %s") % rule)
         # If the rule is invalid, we'll fail closed
         return FalseCheck()
 
@@ -407,9 +490,9 @@ def _parse_check(rule):
 
 
 def _parse_list_rule(rule):
-    """
-    Provided for backwards compatibility.  Translates the old
-    list-of-lists syntax into a tree of Check objects.
+    """Translates the old list-of-lists syntax into a tree of Check objects.
+
+    Provided for backwards compatibility.
     """
 
     # Empty rule defaults to True
@@ -450,8 +533,7 @@ _tokenize_re = re.compile(r'\s+')
 
 
 def _parse_tokenize(rule):
-    """
-    Tokenizer for the policy language.
+    """Tokenizer for the policy language.
 
     Most of the single-character tokens are specified in the
     _tokenize_re; however, parentheses need to be handled specially,
@@ -500,16 +582,16 @@ def _parse_tokenize(rule):
 
 
 class ParseStateMeta(type):
-    """
-    Metaclass for the ParseState class.  Facilitates identifying
-    reduction methods.
+    """Metaclass for the ParseState class.
+
+    Facilitates identifying reduction methods.
     """
 
     def __new__(mcs, name, bases, cls_dict):
-        """
-        Create the class.  Injects the 'reducers' list, a list of
-        tuples matching token sequences to the names of the
-        corresponding reduction methods.
+        """Create the class.
+
+        Injects the 'reducers' list, a list of tuples matching token sequences
+        to the names of the corresponding reduction methods.
         """
 
         reducers = []
@@ -526,10 +608,10 @@ class ParseStateMeta(type):
 
 
 def reducer(*tokens):
-    """
-    Decorator for reduction methods.  Arguments are a sequence of
-    tokens, in order, which should trigger running this reduction
-    method.
+    """Decorator for reduction methods.
+
+    Arguments are a sequence of tokens, in order, which should trigger running
+    this reduction method.
     """
 
     def decorator(func):
@@ -546,10 +628,10 @@ def reducer(*tokens):
 
 
 class ParseState(object):
-    """
-    Implement the core of parsing the policy language.  Uses a greedy
-    reduction algorithm to reduce a sequence of tokens into a single
-    terminal, the value of which will be the root of the Check tree.
+    """Implement the core of parsing the policy language.
+
+    Uses a greedy reduction algorithm to reduce a sequence of tokens into
+    a single terminal, the value of which will be the root of the Check tree.
 
     Note: error reporting is rather lacking.  The best we can get with
     this parser formulation is an overall "parse failed" error.
@@ -566,11 +648,11 @@ class ParseState(object):
         self.values = []
 
     def reduce(self):
-        """
-        Perform a greedy reduction of the token stream.  If a reducer
-        method matches, it will be executed, then the reduce() method
-        will be called recursively to search for any more possible
-        reductions.
+        """Perform a greedy reduction of the token stream.
+
+        If a reducer method matches, it will be executed, then the
+        reduce() method will be called recursively to search for any more
+        possible reductions.
         """
 
         for reduction, methname in self.reducers:
@@ -600,9 +682,9 @@ class ParseState(object):
 
     @property
     def result(self):
-        """
-        Obtain the final result of the parse.  Raises ValueError if
-        the parse failed to reduce to a single result.
+        """Obtain the final result of the parse.
+
+        Raises ValueError if the parse failed to reduce to a single result.
         """
 
         if len(self.values) != 1:
@@ -619,35 +701,31 @@ class ParseState(object):
 
     @reducer('check', 'and', 'check')
     def _make_and_expr(self, check1, _and, check2):
-        """
-        Create an 'and_expr' from two checks joined by the 'and'
-        operator.
+        """Create an 'and_expr'.
+
+        Join two checks by the 'and' operator.
         """
 
         return [('and_expr', AndCheck([check1, check2]))]
 
     @reducer('and_expr', 'and', 'check')
     def _extend_and_expr(self, and_expr, _and, check):
-        """
-        Extend an 'and_expr' by adding one more check.
-        """
+        """Extend an 'and_expr' by adding one more check."""
 
         return [('and_expr', and_expr.add_check(check))]
 
     @reducer('check', 'or', 'check')
     def _make_or_expr(self, check1, _or, check2):
-        """
-        Create an 'or_expr' from two checks joined by the 'or'
-        operator.
+        """Create an 'or_expr'.
+
+        Join two checks by the 'or' operator.
         """
 
         return [('or_expr', OrCheck([check1, check2]))]
 
     @reducer('or_expr', 'or', 'check')
     def _extend_or_expr(self, or_expr, _or, check):
-        """
-        Extend an 'or_expr' by adding one more check.
-        """
+        """Extend an 'or_expr' by adding one more check."""
 
         return [('or_expr', or_expr.add_check(check))]
 
@@ -659,7 +737,8 @@ class ParseState(object):
 
 
 def _parse_text_rule(rule):
-    """
+    """Parses policy to the tree.
+
     Translates a policy written in the policy language into a tree of
     Check objects.
     """
@@ -677,16 +756,14 @@ def _parse_text_rule(rule):
         return state.result
     except ValueError:
         # Couldn't parse the rule
-        LOG.exception(_("Failed to understand rule %(rule)r") % locals())
+        LOG.exception(_("Failed to understand rule %r") % rule)
 
         # Fail closed
         return FalseCheck()
 
 
 def parse_rule(rule):
-    """
-    Parses a policy rule into a tree of Check objects.
-    """
+    """Parses a policy rule into a tree of Check objects."""
 
     # If the rule is a string, it's in the policy language
     if isinstance(rule, basestring):
@@ -695,8 +772,7 @@ def parse_rule(rule):
 
 
 def register(name, func=None):
-    """
-    Register a function or Check class as a policy check.
+    """Register a function or Check class as a policy check.
 
     :param name: Gives the name of the check type, e.g., 'rule',
                  'role', etc.  If name is None, a default check type
@@ -723,13 +799,11 @@ def register(name, func=None):
 
 @register("rule")
 class RuleCheck(Check):
-    def __call__(self, target, creds):
-        """
-        Recursively checks credentials based on the defined rules.
-        """
+    def __call__(self, target, creds, enforcer):
+        """Recursively checks credentials based on the defined rules."""
 
         try:
-            return _rules[self.match](target, creds)
+            return enforcer.rules[self.match](target, creds, enforcer)
         except KeyError:
             # We don't have any matching rule; fail closed
             return False
@@ -737,7 +811,7 @@ class RuleCheck(Check):
 
 @register("role")
 class RoleCheck(Check):
-    def __call__(self, target, creds):
+    def __call__(self, target, creds, enforcer):
         """Check that there is a matching role in the cred dict."""
 
         return self.match.lower() in [x.lower() for x in creds['roles']]
@@ -745,9 +819,8 @@ class RoleCheck(Check):
 
 @register('http')
 class HttpCheck(Check):
-    def __call__(self, target, creds):
-        """
-        Check http: rules by calling to a remote server.
+    def __call__(self, target, creds, enforcer):
+        """Check http: rules by calling to a remote server.
 
         This example implementation simply verifies that the response
         is exactly 'True'.
@@ -763,9 +836,8 @@ class HttpCheck(Check):
 
 @register(None)
 class GenericCheck(Check):
-    def __call__(self, target, creds):
-        """
-        Check an individual match.
+    def __call__(self, target, creds, enforcer):
+        """Check an individual match.
 
         Matches look like:
 
diff --git a/heat/openstack/common/timeutils.py b/heat/openstack/common/timeutils.py
index ac2441bcb..bd60489e5 100644
--- a/heat/openstack/common/timeutils.py
+++ b/heat/openstack/common/timeutils.py
@@ -23,6 +23,7 @@ import calendar
 import datetime
 
 import iso8601
+import six
 
 
 # ISO 8601 extended time format with microseconds
@@ -75,14 +76,14 @@ def normalize_time(timestamp):
 
 def is_older_than(before, seconds):
     """Return True if before is older than seconds."""
-    if isinstance(before, basestring):
+    if isinstance(before, six.string_types):
         before = parse_strtime(before).replace(tzinfo=None)
     return utcnow() - before > datetime.timedelta(seconds=seconds)
 
 
 def is_newer_than(after, seconds):
     """Return True if after is newer than seconds."""
-    if isinstance(after, basestring):
+    if isinstance(after, six.string_types):
         after = parse_strtime(after).replace(tzinfo=None)
     return after - utcnow() > datetime.timedelta(seconds=seconds)