Adds support for router rules to Big Switch plugin

Implements: blueprint bsn-router-rules

Adds bigswitch plugin extension which adds 'rules' dictionary to router objects.
Adds validation code and database components to store router rules
Adds configuration option to plugin to set default router rules and max router rules
Adds unit tests to test all router rule functionality
Adds database migration for router rules tables

The Big Switch controller's Virtual Router implementation supports "routing rules"
which are of the form:
<source, destination, next-hop, action>
This extension aims to expose this abstraction via the Big Switch Quantum plugin.

These rules are applied at the router level, allowing tenants to control
communication between networks at a high level without requiring security policies.
(e.g. prevent servers in a publicly accessible subnet from communicating with
database servers).

Change-Id: I37a2740dca93b0a8b5111764458d39f1c2b885ce

etc/quantum/plugins/bigswitch/restproxy.ini

@@ -50,3 +50,15 @@ servers=localhost:8080
 #    options: ivs or ovs
 #    default: ovs
 # vif_type = ovs
+
+[ROUTER]
+# Specify the default router rules installed in newly created tenant routers
+# Specify multiple times for multiple rules
+# Format is <tenant>:<source>:<destination>:<action>
+# Optionally, a comma-separated list of nexthops may be included after <action>
+# Use an * to specify default for all tenants
+# Default is any any allow for all tenants
+#tenant_default_router_rule=*:any:any:permit
+# Maximum number of rules that a single router may have
+# Default is 200
+#max_router_rules=200

quantum/db/migration/alembic_migrations/versions/5918cbddab04_add_tables_for_route.py

@@ -0,0 +1,71 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+#
+# Copyright 2013 OpenStack Foundation
+#
+#    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.
+#
+
+"""add tables for router rules support
+
+Revision ID: 5918cbddab04
+Revises: 3cbf70257c28
+Create Date: 2013-06-16 02:20:07.024752
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '5918cbddab04'
+down_revision = '3cbf70257c28'
+
+# Change to ['*'] if this migration applies to all plugins
+
+migration_for_plugins = [
+    'quantum.plugins.bigswitch.plugin.QuantumRestProxyV2'
+]
+
+from alembic import op
+import sqlalchemy as sa
+
+
+from quantum.db import migration
+
+
+def upgrade(active_plugin=None, options=None):
+    if not migration.should_run(active_plugin, migration_for_plugins):
+        return
+
+    op.create_table('routerrules',
+                    sa.Column('id', sa.Integer(), nullable=False),
+                    sa.Column('source', sa.String(length=64), nullable=False),
+                    sa.Column('destination', sa.String(length=64),
+                              nullable=False),
+                    sa.Column('action', sa.String(length=10), nullable=False),
+                    sa.Column('router_id', sa.String(length=36),
+                              nullable=True),
+                    sa.ForeignKeyConstraint(['router_id'], ['routers.id'],
+                                            ondelete='CASCADE'),
+                    sa.PrimaryKeyConstraint('id'))
+    op.create_table('nexthops',
+                    sa.Column('rule_id', sa.Integer(), nullable=False),
+                    sa.Column('nexthop', sa.String(length=64), nullable=False),
+                    sa.ForeignKeyConstraint(['rule_id'], ['routerrules.id'],
+                                            ondelete='CASCADE'),
+                    sa.PrimaryKeyConstraint('rule_id', 'nexthop'))
+
+
+def downgrade(active_plugin=None, options=None):
+    if not migration.should_run(active_plugin, migration_for_plugins):
+        return
+
+    op.drop_table('nexthops')
+    op.drop_table('routerrules')

quantum/plugins/bigswitch/extensions/__init__.py

@@ -0,0 +1,18 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2013 Big Switch Networks, Inc.
+# 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.
+#
+# @author: Kevin Benton, Big Switch Networks, Inc.

quantum/plugins/bigswitch/extensions/routerrule.py

@@ -0,0 +1,143 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2013 Big Switch Networks, Inc.
+# 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.
+#
+# @author: Kevin Benton, Big Switch Networks, Inc.
+
+from quantum.api.v2 import attributes as attr
+from quantum.common import exceptions as qexception
+from quantum.openstack.common import log as logging
+
+
+LOG = logging.getLogger(__name__)
+
+
+# Router Rules Exceptions
+class InvalidRouterRules(qexception.InvalidInput):
+    message = _("Invalid format for router rules: %(rule)s, %(reason)s")
+
+
+class RulesExhausted(qexception.BadRequest):
+    message = _("Unable to complete rules update for %(router_id)s. "
+                "The number of rules exceeds the maximum %(quota)s.")
+
+
+def convert_to_valid_router_rules(data):
+    """
+    Validates and converts router rules to the appropriate data structure
+    Example argument = [{'source': 'any', 'destination': 'any',
+                         'action':'deny'},
+                        {'source': '1.1.1.1/32', 'destination': 'any',
+                         'action':'permit',
+                         'nexthops': ['1.1.1.254', '1.1.1.253']}
+                       ]
+    """
+    V4ANY = '0.0.0.0/0'
+    if not isinstance(data, list):
+        emsg = _("Invalid data format for router rule: '%s'") % data
+        LOG.debug(emsg)
+        raise qexception.InvalidInput(error_message=emsg)
+    _validate_uniquerules(data)
+    rules = []
+    expected_keys = ['source', 'destination', 'action']
+    for rule in data:
+        rule['nexthops'] = rule.get('nexthops', [])
+        if not isinstance(rule['nexthops'], list):
+            rule['nexthops'] = rule['nexthops'].split('+')
+
+        src = V4ANY if rule['source'] == 'any' else rule['source']
+        dst = V4ANY if rule['destination'] == 'any' else rule['destination']
+
+        errors = [attr._verify_dict_keys(expected_keys, rule, False),
+                  attr._validate_subnet(dst),
+                  attr._validate_subnet(src),
+                  _validate_nexthops(rule['nexthops']),
+                  _validate_action(rule['action'])]
+        errors = [m for m in errors if m]
+        if errors:
+            LOG.debug(errors)
+            raise qexception.InvalidInput(error_message=errors)
+        rules.append(rule)
+    return rules
+
+
+def _validate_nexthops(nexthops):
+    seen = []
+    for ip in nexthops:
+        msg = attr._validate_ip_address(ip)
+        if ip in seen:
+            msg = _("Duplicate nexthop in rule '%s'") % ip
+        seen.append(ip)
+        if msg:
+            return msg
+
+
+def _validate_action(action):
+    if action not in ['permit', 'deny']:
+        return _("Action must be either permit or deny."
+                 " '%s' was provided") % action
+
+
+def _validate_uniquerules(rules):
+    pairs = []
+    for r in rules:
+        if 'source' not in r or 'destination' not in r:
+            continue
+        pairs.append((r['source'], r['destination']))
+
+    if len(set(pairs)) != len(pairs):
+        error = _("Duplicate router rules (src,dst)  found '%s'") % pairs
+        LOG.debug(error)
+        raise qexception.InvalidInput(error_message=error)
+
+
+class Routerrule(object):
+
+    @classmethod
+    def get_name(cls):
+        return "Quantum Router Rule"
+
+    @classmethod
+    def get_alias(cls):
+        return "router_rules"
+
+    @classmethod
+    def get_description(cls):
+        return "Router rule configuration for L3 router"
+
+    @classmethod
+    def get_namespace(cls):
+        return "http://docs.openstack.org/ext/quantum/routerrules/api/v1.0"
+
+    @classmethod
+    def get_updated(cls):
+        return "2013-05-23T10:00:00-00:00"
+
+    def get_extended_resources(self, version):
+        if version == "2.0":
+            return EXTENDED_ATTRIBUTES_2_0
+        else:
+            return {}
+
+# Attribute Map
+EXTENDED_ATTRIBUTES_2_0 = {
+    'routers': {
+        'router_rules': {'allow_post': False, 'allow_put': True,
+                         'convert_to': convert_to_valid_router_rules,
+                         'is_visible': True,
+                         'default': attr.ATTR_NOT_SPECIFIED},
+    }
+}

quantum/plugins/bigswitch/plugin.py

@@ -48,6 +48,7 @@ import base64
 import copy
 import httplib
 import json
+import os
 import socket
 
 from oslo.config import cfg
@@ -67,11 +68,16 @@ from quantum.extensions import l3
 from quantum.extensions import portbindings
 from quantum.openstack.common import log as logging
 from quantum.openstack.common import rpc
+from quantum.plugins.bigswitch import routerrule_db
 from quantum.plugins.bigswitch.version import version_string_with_vcs
 
-
 LOG = logging.getLogger(__name__)
 
+# Include the BigSwitch Extensions path in the api_extensions
+EXTENSIONS_PATH = os.path.join(os.path.dirname(__file__), 'extensions')
+if not cfg.CONF.api_extensions_path:
+    cfg.CONF.set_override('api_extensions_path',
+                          EXTENSIONS_PATH)
 
 restproxy_opts = [
     cfg.StrOpt('servers', default='localhost:8800',
@@ -102,6 +108,17 @@ restproxy_opts = [
 
 cfg.CONF.register_opts(restproxy_opts, "RESTPROXY")
 
+router_opts = [
+    cfg.MultiStrOpt('tenant_default_router_rule', default=['*:any:any:permit'],
+                    help=_("The default router rules installed in new tenant "
+                           "routers. Repeat the config option for each rule. "
+                           "Format is <tenant>:<source>:<destination>:<action>"
+                           " Use an * to specify default for all tenants.")),
+    cfg.IntOpt('max_router_rules', default=200,
+               help=_("Maximum number of router rules")),
+]
+
+cfg.CONF.register_opts(router_opts, "ROUTER")
 
 nova_opts = [
     cfg.StrOpt('vif_type', default='ovs',
@@ -302,9 +319,9 @@ class RpcProxy(dhcp_rpc_base.DhcpRpcCallbackMixin):
 
 
 class QuantumRestProxyV2(db_base_plugin_v2.QuantumDbPluginV2,
-                         l3_db.L3_NAT_db_mixin):
+                         routerrule_db.RouterRule_db_mixin):
 
-    supported_extension_aliases = ["router", "binding"]
+    supported_extension_aliases = ["router", "binding", "router_rules"]
 
     def __init__(self):
         LOG.info(_('QuantumRestProxy: Starting plugin. Version=%s'),
@@ -842,6 +859,32 @@ class QuantumRestProxyV2(db_base_plugin_v2.QuantumDbPluginV2,
             # TODO(Sumit): rollback deletion of subnet
             raise
 
+    def _get_tenant_default_router_rules(self, tenant):
+        rules = cfg.CONF.ROUTER.tenant_default_router_rule
+        defaultset = []
+        tenantset = []
+        for rule in rules:
+            items = rule.split(':')
+            if len(items) == 5:
+                (tenantid, source, destination, action, nexthops) = items
+            elif len(items) == 4:
+                (tenantid, source, destination, action) = items
+                nexthops = ''
+            else:
+                continue
+            parsedrule = {'source': source,
+                          'destination': destination, 'action': action,
+                          'nexthops': nexthops.split(',')}
+            if parsedrule['nexthops'][0] == '':
+                parsedrule['nexthops'] = []
+            if tenantid == '*':
+                defaultset.append(parsedrule)
+            if tenantid == tenant:
+                tenantset.append(parsedrule)
+        if tenantset:
+            return tenantset
+        return defaultset
+
     def create_router(self, context, router):
         LOG.debug(_("QuantumRestProxyV2: create_router() called"))
 
@@ -849,6 +892,10 @@ class QuantumRestProxyV2(db_base_plugin_v2.QuantumDbPluginV2,
 
         tenant_id = self._get_tenant_id_for_create(context, router["router"])
 
+        # set default router rules
+        rules = self._get_tenant_default_router_rules(tenant_id)
+        router['router']['router_rules'] = rules
+
         # create router in DB
         new_router = super(QuantumRestProxyV2, self).create_router(context,
                                                                    router)

quantum/plugins/bigswitch/routerrule_db.py

@@ -0,0 +1,148 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+#
+# Copyright 2013, Big Switch Networks
+# 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.
+
+from oslo.config import cfg
+import sqlalchemy as sa
+from sqlalchemy import orm
+
+from quantum.db import l3_db
+from quantum.db import model_base
+from quantum.openstack.common import log as logging
+from quantum.plugins.bigswitch.extensions import routerrule
+
+
+LOG = logging.getLogger(__name__)
+
+
+class RouterRule(model_base.BASEV2):
+    id = sa.Column(sa.Integer, primary_key=True)
+    source = sa.Column(sa.String(64), nullable=False)
+    destination = sa.Column(sa.String(64), nullable=False)
+    nexthops = orm.relationship('NextHop', cascade='all,delete')
+    action = sa.Column(sa.String(10), nullable=False)
+    router_id = sa.Column(sa.String(36),
+                          sa.ForeignKey('routers.id',
+                                        ondelete="CASCADE"))
+
+
+class NextHop(model_base.BASEV2):
+    rule_id = sa.Column(sa.Integer,
+                        sa.ForeignKey('routerrules.id',
+                                      ondelete="CASCADE"),
+                        primary_key=True)
+    nexthop = sa.Column(sa.String(64), nullable=False, primary_key=True)
+
+
+class RouterRule_db_mixin(l3_db.L3_NAT_db_mixin):
+    """ Mixin class to support route rule configuration on a router"""
+    def update_router(self, context, id, router):
+        r = router['router']
+        with context.session.begin(subtransactions=True):
+            router_db = self._get_router(context, id)
+            if 'router_rules' in r:
+                self._update_router_rules(context,
+                                          router_db,
+                                          r['router_rules'])
+            updated = super(RouterRule_db_mixin, self).update_router(
+                context, id, router)
+            updated['router_rules'] = self._get_router_rules_by_router_id(
+                context, id)
+
+        return updated
+
+    def create_router(self, context, router):
+        r = router['router']
+        with context.session.begin(subtransactions=True):
+            router_db = super(RouterRule_db_mixin, self).create_router(
+                context, router)
+            if 'router_rules' in r:
+                self._update_router_rules(context,
+                                          router_db,
+                                          r['router_rules'])
+            else:
+                LOG.debug('No rules in router')
+            router_db['router_rules'] = self._get_router_rules_by_router_id(
+                context, router_db['id'])
+
+        return router_db
+
+    def _update_router_rules(self, context, router, rules):
+        if len(rules) > cfg.CONF.ROUTER.max_router_rules:
+            raise routerrule.RulesExhausted(
+                router_id=router['id'],
+                quota=cfg.CONF.ROUTER.max_router_rules)
+        del_context = context.session.query(RouterRule)
+        del_context.filter_by(router_id=router['id']).delete()
+        context.session.expunge_all()
+        LOG.debug('Updating router rules to %s' % rules)
+        for rule in rules:
+            router_rule = RouterRule(
+                router_id=router['id'],
+                destination=rule['destination'],
+                source=rule['source'],
+                action=rule['action'])
+            router_rule.nexthops = [NextHop(nexthop=hop)
+                                    for hop in rule['nexthops']]
+            context.session.add(router_rule)
+        context.session.flush()
+
+    def _make_router_rule_list(self, router_rules):
+        ruleslist = []
+        for rule in router_rules:
+            hops = [hop['nexthop'] for hop in rule['nexthops']]
+            ruleslist.append({'id': rule['id'],
+                              'destination': rule['destination'],
+                              'source': rule['source'],
+                              'action': rule['action'],
+                              'nexthops': hops})
+        return ruleslist
+
+    def _get_router_rules_by_router_id(self, context, id):
+        query = context.session.query(RouterRule)
+        router_rules = query.filter_by(router_id=id).all()
+        return self._make_router_rule_list(router_rules)
+
+    def get_router(self, context, id, fields=None):
+        with context.session.begin(subtransactions=True):
+            router = super(RouterRule_db_mixin, self).get_router(
+                context, id, fields)
+            router['router_rules'] = self._get_router_rules_by_router_id(
+                context, id)
+            return router
+
+    def get_routers(self, context, filters=None, fields=None,
+                    sorts=None, limit=None, marker=None,
+                    page_reverse=False):
+        with context.session.begin(subtransactions=True):
+            routers = super(RouterRule_db_mixin, self).get_routers(
+                context, filters, fields, sorts=sorts, limit=limit,
+                marker=marker, page_reverse=page_reverse)
+            for router in routers:
+                router['router_rules'] = self._get_router_rules_by_router_id(
+                    context, router['id'])
+            return routers
+
+    def get_sync_data(self, context, router_ids=None, active=None):
+        """Query routers and their related floating_ips, interfaces."""
+        with context.session.begin(subtransactions=True):
+            routers = super(RouterRule_db_mixin,
+                            self).get_sync_data(context, router_ids,
+                                                active=active)
+            for router in routers:
+                router['router_rules'] = self._get_router_rules_by_router_id(
+                    context, router['id'])
+        return routers

quantum/tests/unit/bigswitch/etc/restproxy.ini.test

@@ -31,3 +31,11 @@ serverssl=False
 #   default: ovs
 vif_type = ovs
 
+[ROUTER]
+# Specify the default router rules installed in newly created tenant routers
+# Specify multiple times for multiple rules
+# Use an * to specify default for all tenants
+# Default is any any allow for all tenants
+#tenant_default_router_rule=*:any:any:permit
+# Maximum number of rules that a single router may have
+max_router_rules=200

quantum/tests/unit/bigswitch/test_router_db.py

@@ -18,6 +18,7 @@
 # @author: Sumit Naiksatam, sumitnaiksatam@gmail.com
 #
 
+import copy
 import os
 
 from mock import patch
@@ -29,6 +30,7 @@ from quantum.extensions import l3
 from quantum.manager import QuantumManager
 from quantum.openstack.common.notifier import api as notifier_api
 from quantum.openstack.common.notifier import test_notifier
+from quantum.plugins.bigswitch.extensions import routerrule
 from quantum.tests.unit import test_l3_plugin
 
 
@@ -39,7 +41,7 @@ def new_L3_setUp(self):
     rp_conf_file = os.path.join(etc_path, 'restproxy.ini.test')
     test_config['config_files'] = [rp_conf_file]
     cfg.CONF.set_default('allow_overlapping_ips', False)
-    ext_mgr = L3TestExtensionManager()
+    ext_mgr = RouterRulesTestExtensionManager()
     test_config['extension_manager'] = ext_mgr
     super(test_l3_plugin.L3NatTestCaseBase, self).setUp()
 
@@ -78,9 +80,11 @@ class HTTPConnectionMock():
         pass
 
 
-class L3TestExtensionManager(object):
+class RouterRulesTestExtensionManager(object):
 
     def get_resources(self):
+        l3.RESOURCE_ATTRIBUTE_MAP['routers'].update(
+            routerrule.EXTENDED_ATTRIBUTES_2_0['routers'])
         return l3.L3.get_resources()
 
     def get_actions(self):
@@ -319,3 +323,155 @@ class RouterDBTestCase(test_l3_plugin.L3NatDBTestCase):
                                                       None)
                         self._show('ports', r1_port_id,
                                    expected_code=exc.HTTPNotFound.code)
+
+    def test_router_rules_update(self):
+        with self.router() as r:
+            r_id = r['router']['id']
+            router_rules = [{'destination': '1.2.3.4/32',
+                             'source': '4.3.2.1/32',
+                             'action': 'permit',
+                             'nexthops': ['4.4.4.4', '4.4.4.5']}]
+            body = self._update('routers', r_id,
+                                {'router': {'router_rules': router_rules}})
+
+            body = self._show('routers', r['router']['id'])
+            self.assertIn('router_rules', body['router'])
+            rules = body['router']['router_rules']
+            self.assertEqual(_strip_rule_ids(rules), router_rules)
+            # Try after adding another rule
+            router_rules.append({'source': 'any',
+                                 'destination': '8.8.8.8/32',
+                                 'action': 'permit', 'nexthops': []})
+            body = self._update('routers', r['router']['id'],
+                                {'router': {'router_rules': router_rules}})
+
+            body = self._show('routers', r['router']['id'])
+            self.assertIn('router_rules', body['router'])
+            rules = body['router']['router_rules']
+            self.assertEqual(_strip_rule_ids(rules), router_rules)
+
+    def test_router_rules_separation(self):
+        with self.router() as r1:
+            with self.router() as r2:
+                r1_id = r1['router']['id']
+                r2_id = r2['router']['id']
+                router1_rules = [{'destination': '5.6.7.8/32',
+                                 'source': '8.7.6.5/32',
+                                 'action': 'permit',
+                                 'nexthops': ['8.8.8.8', '9.9.9.9']}]
+                router2_rules = [{'destination': '1.2.3.4/32',
+                                 'source': '4.3.2.1/32',
+                                 'action': 'permit',
+                                 'nexthops': ['4.4.4.4', '4.4.4.5']}]
+                body1 = self._update('routers', r1_id,
+                                     {'router':
+                                     {'router_rules': router1_rules}})
+                body2 = self._update('routers', r2_id,
+                                     {'router':
+                                     {'router_rules': router2_rules}})
+
+                body1 = self._show('routers', r1_id)
+                body2 = self._show('routers', r2_id)
+                rules1 = body1['router']['router_rules']
+                rules2 = body2['router']['router_rules']
+                self.assertEqual(_strip_rule_ids(rules1), router1_rules)
+                self.assertEqual(_strip_rule_ids(rules2), router2_rules)
+
+    def test_router_rules_validation(self):
+        with self.router() as r:
+            r_id = r['router']['id']
+            good_rules = [{'destination': '1.2.3.4/32',
+                           'source': '4.3.2.1/32',
+                           'action': 'permit',
+                           'nexthops': ['4.4.4.4', '4.4.4.5']}]
+
+            body = self._update('routers', r_id,
+                                {'router': {'router_rules': good_rules}})
+            body = self._show('routers', r_id)
+            self.assertIn('router_rules', body['router'])
+            self.assertEqual(good_rules,
+                             _strip_rule_ids(body['router']['router_rules']))
+
+            # Missing nexthops should be populated with an empty list
+            light_rules = copy.deepcopy(good_rules)
+            del light_rules[0]['nexthops']
+            body = self._update('routers', r_id,
+                                {'router': {'router_rules': light_rules}})
+            body = self._show('routers', r_id)
+            self.assertIn('router_rules', body['router'])
+            light_rules[0]['nexthops'] = []
+            self.assertEqual(light_rules,
+                             _strip_rule_ids(body['router']['router_rules']))
+            # bad CIDR
+            bad_rules = copy.deepcopy(good_rules)
+            bad_rules[0]['destination'] = '1.1.1.1'
+            body = self._update('routers', r_id,
+                                {'router': {'router_rules': bad_rules}},
+                                expected_code=exc.HTTPBadRequest.code)
+            # bad next hop
+            bad_rules = copy.deepcopy(good_rules)
+            bad_rules[0]['nexthops'] = ['1.1.1.1', 'f2']
+            body = self._update('routers', r_id,
+                                {'router': {'router_rules': bad_rules}},
+                                expected_code=exc.HTTPBadRequest.code)
+            # bad action
+            bad_rules = copy.deepcopy(good_rules)
+            bad_rules[0]['action'] = 'dance'
+            body = self._update('routers', r_id,
+                                {'router': {'router_rules': bad_rules}},
+                                expected_code=exc.HTTPBadRequest.code)
+            # duplicate rule with opposite action
+            bad_rules = copy.deepcopy(good_rules)
+            bad_rules.append(copy.deepcopy(bad_rules[0]))
+            bad_rules.append(copy.deepcopy(bad_rules[0]))
+            bad_rules[1]['source'] = 'any'
+            bad_rules[2]['action'] = 'deny'
+            body = self._update('routers', r_id,
+                                {'router': {'router_rules': bad_rules}},
+                                expected_code=exc.HTTPBadRequest.code)
+            # duplicate nexthop
+            bad_rules = copy.deepcopy(good_rules)
+            bad_rules[0]['nexthops'] = ['1.1.1.1', '1.1.1.1']
+            body = self._update('routers', r_id,
+                                {'router': {'router_rules': bad_rules}},
+                                expected_code=exc.HTTPBadRequest.code)
+            # make sure light rules persisted during bad updates
+            body = self._show('routers', r_id)
+            self.assertIn('router_rules', body['router'])
+            self.assertEqual(light_rules,
+                             _strip_rule_ids(body['router']['router_rules']))
+
+    def test_router_rules_config_change(self):
+        cfg.CONF.set_override('tenant_default_router_rule',
+                              ['*:any:any:deny',
+                               '*:8.8.8.8/32:any:permit:1.2.3.4'],
+                              'ROUTER')
+        with self.router() as r:
+            body = self._show('routers', r['router']['id'])
+            expected_rules = [{'source': 'any', 'destination': 'any',
+                               'nexthops': [], 'action': 'deny'},
+                              {'source': '8.8.8.8/32', 'destination': 'any',
+                               'nexthops': ['1.2.3.4'], 'action': 'permit'}]
+            self.assertEqual(expected_rules,
+                             _strip_rule_ids(body['router']['router_rules']))
+
+    def test_rule_exhaustion(self):
+        cfg.CONF.set_override('max_router_rules', 10, 'ROUTER')
+        with self.router() as r:
+            rules = []
+            for i in xrange(1, 12):
+                rule = {'source': 'any', 'nexthops': [],
+                        'destination': '1.1.1.' + str(i) + '/32',
+                        'action': 'permit'}
+                rules.append(rule)
+            self._update('routers', r['router']['id'],
+                         {'router': {'router_rules': rules}},
+                         expected_code=exc.HTTPBadRequest.code)
+
+
+def _strip_rule_ids(rules):
+    cleaned = []
+    for rule in rules:
+        del rule['id']
+        cleaned.append(rule)
+    return cleaned