openstack/ovsdbapp
Code Issues Proposed changes

Add Port Group ACL commands

Browse Source 
After introducing the concept of Port Groups in OVN, ACLs can be
applied either to a Logical Switch or to a Port Group.
This patch is adding the commands to support adding ACLs to a Port
Group. Also it's removing the initial implementation of adding
standalone ACLs to a Port Group.

Change-Id: I7720cd7aa801fbc4be87e1c665e4549725576269
Signed-off-by: Daniel Alvarez <dalvarez@redhat.com>
This commit is contained in:
Daniel Alvarez 2018-06-01 09:45:57 +02:00
parent 68177608cd
commit 7e980f1ae5
5 changed files with 216 additions and 127 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

83
ovsdbapp/schema/ovn_northbound/api.py
View File

@ -140,6 +140,56 @@ class API(api.API):
:returns: :class:`Command` with RowView list result
"""
@abc.abstractmethod
def pg_acl_add(self, port_group, direction, priority, match, action,
log=False):
"""Add an ACL to 'port_group'
:param port_group: The name or uuid of the port group
:type port_group: string or uuid.UUID
:param direction: The traffic direction to match
:type direction: 'from-lport' or 'to-lport'
:param priority: The priority field of the ACL
:type priority: int
:param match: The match rule
:type match: string
:param action: The action to take upon match
:type action: 'allow', 'allow-related', 'drop', or 'reject'
:param log: If True, enable packet logging for the ACL
:type log: boolean
:returns: :class:`Command` with RowView result
"""
@abc.abstractmethod
def pg_acl_del(self, port_group, direction=None, priority=None,
match=None):
"""Remove ACLs from 'port_group'
If only port_group is supplied, all the ACLs from the logical switch
are deleted. If direction is also specified, then all the flows in
that direction will be deleted from the Port Group. If all the fields
are given, then only flows that match all fields will be deleted.
:param port_group: The name or uuid of the port group
:type port_group: string or uuid.UUID
:param direction: The traffic direction to match
:type direction: 'from-lport' or 'to-lport'
:param priority: The priority field of the ACL
:type priority: int
:param match: The match rule
:type match: string
:returns: :class:`Command` with no result
"""
@abc.abstractmethod
def pg_acl_list(self, port_group):
"""Get the ACLs for 'port group'
:param port_group: The name or uuid of the switch
:type port_group: string or uuid.UUID
:returns: :class:`Command` with RowView list result
"""
@abc.abstractmethod
def qos_add(self, switch, direction, priority, match, rate=None,
burst=None, dscp=None, may_exist=False, **columns):
@ -823,10 +873,10 @@ class API(api.API):
"""
@abc.abstractmethod
def pg_add(self, name, may_exist=False, **columns):
def pg_add(self, name=None, may_exist=False, **columns):
"""Create a port group
:param name: The name of the port group
:param name: The name of the port group (optional)
:type name: string
:param may_exist: If True, don't fail if the port group already
exists
@ -878,32 +928,3 @@ class API(api.API):
:type if_exists: boolean
:returns: :class:`Command` with no result
"""
@abc.abstractmethod
def pg_add_acls(self, pg_id, acl):
"""Add a list of ACL to a port group
:param pg_id: The name or uuid of the port group
:type pg_id: string or uuid.UUID
:param acl: The ACL instance or UUID
:type acl: A list of ACL instance or string or uuid.UUID
:param acl: A list of :class:`Command` with an ACL instance
result or UUID
:type acl: A :class:`Command` with an ACL or string
r uuid.UUID
:returns: :class:`Command` with no result
"""
@abc.abstractmethod
def pg_del_acls(self, pg_id, acl, if_exists=False):
"""Delete a list of ACL from a port group
:param pg_id: The name or uuid of the port group
:type pg_id: string or uuid.UUID
:type acl: A list of ACL instance or string or uuid.UUID
:param acl: A list of :class:`Command` with an ACL instance
result or UUID
:type if_exists: If True, don't fail if the ACL(s) doesn't exist
:type if_exists: boolean
:returns: :class:`Command` with no result
"""

128
ovsdbapp/schema/ovn_northbound/commands.py
View File

@ -81,11 +81,12 @@ class LsGetCommand(cmd.BaseGetRowCommand):
table = 'Logical_Switch'
class AclAddCommand(cmd.AddCommand):
class _AclAddHelper(cmd.AddCommand):
table_name = 'ACL'
def __init__(self, api, switch, direction, priority, match, action,
log=False, may_exist=False, **external_ids):
def __init__(self, api, entity, direction, priority, match, action,
log=False, may_exist=False, severity=None, name=None,
**external_ids):
if direction not in ('from-lport', 'to-lport'):
raise TypeError("direction must be either from-lport or to-lport")
if not 0 <= priority <= const.ACL_PRIORITY_MAX:
@ -93,14 +94,16 @@ class AclAddCommand(cmd.AddCommand):
const.ACL_PRIORITY_MAX))
if action not in ('allow', 'allow-related', 'drop', 'reject'):
raise TypeError("action must be allow/allow-related/drop/reject")
super(AclAddCommand, self).__init__(api)
self.switch = switch
super(_AclAddHelper, self).__init__(api)
self.entity = entity
self.direction = direction
self.priority = priority
self.match = match
self.action = action
self.log = log
self.may_exist = may_exist
self.severity = severity
self.name = name
self.external_ids = external_ids
def acl_match(self, row):
@ -109,8 +112,8 @@ class AclAddCommand(cmd.AddCommand):
self.match == row.match)
def run_idl(self, txn):
ls = self.api.lookup('Logical_Switch', self.switch)
acls = [acl for acl in ls.acls if self.acl_match(acl)]
entity = self.api.lookup(self.lookup_table, self.entity)
acls = [acl for acl in entity.acls if self.acl_match(acl)]
if acls:
if self.may_exist:
self.result = rowview.RowView(acls[0])
@ -123,45 +126,87 @@ class AclAddCommand(cmd.AddCommand):
acl.match = self.match
acl.action = self.action
acl.log = self.log
ls.addvalue('acls', acl)
acl.severity = self.severity
acl.name = self.name
entity.addvalue('acls', acl)
for col, value in self.external_ids.items():
acl.setkey('external_ids', col, value)
self.result = acl.uuid
class AclDelCommand(cmd.BaseCommand):
def __init__(self, api, switch, direction=None,
class AclAddCommand(_AclAddHelper):
lookup_table = 'Logical_Switch'
def __init__(self, api, switch, direction, priority, match, action,
log=False, may_exist=False, severity=None, name=None,
**external_ids):
# NOTE: we're overriding the constructor here to not break any
# existing callers before we introduced Port Groups.
super(AclAddCommand, self).__init__(api, switch, direction, priority,
match, action, log, may_exist,
severity, name, **external_ids)
class PgAclAddCommand(_AclAddHelper):
lookup_table = 'Port_Group'
class _AclDelHelper(cmd.BaseCommand):
def __init__(self, api, entity, direction=None,
priority=None, match=None):
if (priority is None) != (match is None):
raise TypeError("Must specify priority and match together")
if priority is not None and not direction:
raise TypeError("Cannot specify priority/match without direction")
super(AclDelCommand, self).__init__(api)
self.switch = switch
super(_AclDelHelper, self).__init__(api)
self.entity = entity
self.conditions = []
if direction:
self.conditions.append(('direction', '=', direction))
# priority can be 0
if match: # and therefor prioroity due to the above check
if match: # and therefore priority due to the above check
self.conditions += [('priority', '=', priority),
('match', '=', match)]
def run_idl(self, txn):
ls = self.api.lookup('Logical_Switch', self.switch)
for acl in [a for a in ls.acls
entity = self.api.lookup(self.lookup_table, self.entity)
for acl in [a for a in entity.acls
if idlutils.row_match(a, self.conditions)]:
ls.delvalue('acls', acl)
entity.delvalue('acls', acl)
acl.delete()
class AclListCommand(cmd.BaseCommand):
def __init__(self, api, switch):
super(AclListCommand, self).__init__(api)
self.switch = switch
class AclDelCommand(_AclDelHelper):
lookup_table = 'Logical_Switch'
def __init__(self, api, switch, direction=None,
priority=None, match=None):
# NOTE: we're overriding the constructor here to not break any
# existing callers before we introduced Port Groups.
super(AclDelCommand, self).__init__(api, switch, direction, priority,
match)
class PgAclDelCommand(_AclDelHelper):
lookup_table = 'Port_Group'
class _AclListHelper(cmd.BaseCommand):
def __init__(self, api, entity):
super(_AclListHelper, self).__init__(api)
self.entity = entity
def run_idl(self, txn):
ls = self.api.lookup('Logical_Switch', self.switch)
self.result = [rowview.RowView(acl) for acl in ls.acls]
entity = self.api.lookup(self.lookup_table, self.entity)
self.result = [rowview.RowView(acl) for acl in entity.acls]
class AclListCommand(_AclListHelper):
lookup_table = 'Logical_Switch'
class PgAclListCommand(_AclListHelper):
lookup_table = 'Port_Group'
class QoSAddCommand(cmd.AddCommand):
@ -1173,7 +1218,7 @@ class PgAddCommand(cmd.AddCommand):
pass
pg = txn.insert(self.api._tables[self.table_name])
pg.name = self.name
pg.name = self.name or ""
self.set_columns(pg, **self.columns)
self.result = pg.uuid
@ -1196,40 +1241,34 @@ class PgDelCommand(cmd.BaseCommand):
raise RuntimeError('Port group %s does not exist' % self.name)
class _PgUpdateHelper(cmd.BaseCommand):
class _PgUpdatePortsHelper(cmd.BaseCommand):
method = None
def __init__(self, api, port_group, lsp=None, acl=None, if_exists=False):
super(_PgUpdateHelper, self).__init__(api)
def __init__(self, api, port_group, lsp=None, if_exists=False):
super(_PgUpdatePortsHelper, self).__init__(api)
self.port_group = port_group
self.lsp = [] if lsp is None else self._listify(lsp)
self.acl = [] if acl is None else self._listify(acl)
self.if_exists = if_exists
def _listify(self, res):
return res if isinstance(res, (list, tuple)) else [res]
def _fetch_resource(self, type_, uuid_):
table = 'Logical_Switch_Port' if type_ == 'ports' else 'ACL'
return self.api.lookup(table, uuid_)
def _run_method(self, pg, column, resource):
if not resource:
def _run_method(self, pg, port):
if not port:
return
if isinstance(resource, cmd.BaseCommand):
resource = resource.result
elif uuidutils.is_uuid_like(resource):
if isinstance(port, cmd.BaseCommand):
port = port.result
elif uuidutils.is_uuid_like(port):
try:
resource = self._fetch_resource(column, resource)
port = self.api.lookup('Logical_Switch_Port', port)
except idlutils.RowNotFound:
if self.if_exists:
return
raise RuntimeError(
'Resource %(res)s of type "%(type)s" does not exist' %
{'res': resource, 'type': column})
'Port %s does not exist' % port)
getattr(pg, self.method)(column, resource)
getattr(pg, self.method)('ports', port)
def run_idl(self, txn):
try:
@ -1239,15 +1278,12 @@ class _PgUpdateHelper(cmd.BaseCommand):
self.port_group)
for lsp in self.lsp:
self._run_method(pg, 'ports', lsp)
for acl in self.acl:
self._run_method(pg, 'acls', acl)
self._run_method(pg, lsp)
class PgAddDataCommand(_PgUpdateHelper):
class PgAddPortCommand(_PgUpdatePortsHelper):
method = 'addvalue'
class PgDelDataCommand(_PgUpdateHelper):
class PgDelPortCommand(_PgUpdatePortsHelper):
method = 'delvalue'

26
ovsdbapp/schema/ovn_northbound/impl_idl.py
View File

@ -64,6 +64,20 @@ class OvnNbApiIdlImpl(ovs_idl.Backend, api.API):
def acl_list(self, switch):
return cmd.AclListCommand(self, switch)
def pg_acl_add(self, port_group, direction, priority, match, action,
log=False, may_exist=False, **external_ids):
return cmd.PgAclAddCommand(self, port_group, direction, priority,
match, action, log, may_exist,
**external_ids)
def pg_acl_del(self, port_group, direction=None, priority=None,
match=None):
return cmd.PgAclDelCommand(self, port_group, direction, priority,
match)
def pg_acl_list(self, port_group):
return cmd.PgAclListCommand(self, port_group)
def qos_add(self, switch, direction, priority, match, rate=None,
burst=None, dscp=None, may_exist=False, **columns):
return cmd.QoSAddCommand(self, switch, direction, priority, match,
@ -261,20 +275,14 @@ class OvnNbApiIdlImpl(ovs_idl.Backend, api.API):
def dns_set_external_ids(self, uuid, **external_ids):
return cmd.DnsSetExternalIdsCommand(self, uuid, **external_ids)
def pg_add(self, name, may_exist=False, **columns):
def pg_add(self, name=None, may_exist=False, **columns):
return cmd.PgAddCommand(self, name, may_exist=may_exist, **columns)
def pg_del(self, name, if_exists=False):
return cmd.PgDelCommand(self, name, if_exists=if_exists)
def pg_add_ports(self, pg_id, lsp):
return cmd.PgAddDataCommand(self, pg_id, lsp=lsp)
return cmd.PgAddPortCommand(self, pg_id, lsp=lsp)
def pg_del_ports(self, pg_id, lsp, if_exists=False):
return cmd.PgDelDataCommand(self, pg_id, lsp=lsp, if_exists=if_exists)
def pg_add_acls(self, pg_id, acl):
return cmd.PgAddDataCommand(self, pg_id, acl=acl)
def pg_del_acls(self, pg_id, acl, if_exists=False):
return cmd.PgDelDataCommand(self, pg_id, acl=acl, if_exists=if_exists)
return cmd.PgDelPortCommand(self, pg_id, lsp=lsp, if_exists=if_exists)

6
ovsdbapp/tests/functional/schema/ovn_northbound/fixtures.py
View File

@ -44,3 +44,9 @@ class DnsFixture(fixtures.ImplIdlFixture):
create = 'dns_add'
delete = 'dns_del'
delete_args = {}
class PortGroupFixture(fixtures.ImplIdlFixture):
api = impl_idl.OvnNbApiIdlImpl
create = 'pg_add'
delete = 'pg_del'

100
ovsdbapp/tests/functional/schema/ovn_northbound/test_impl_idl.py
View File

@ -108,11 +108,19 @@ class TestAclOps(OvnNorthboundTest):
def setUp(self):
super(TestAclOps, self).setUp()
self.switch = self.useFixture(fixtures.LogicalSwitchFixture()).obj
self.port_group = self.useFixture(fixtures.PortGroupFixture()).obj
def _acl_add(self, entity, *args, **kwargs):
self.assertIn(entity, ['lswitch', 'port_group'])
if entity == 'lswitch':
cmd = self.api.acl_add(self.switch.uuid, *args, **kwargs)
resource = self.switch
else:
cmd = self.api.pg_acl_add(self.port_group.uuid, *args, **kwargs)
resource = self.port_group
def _acl_add(self, *args, **kwargs):
cmd = self.api.acl_add(self.switch.uuid, *args, **kwargs)
aclrow = cmd.execute(check_error=True)
self.assertIn(aclrow._row, self.switch.acls)
self.assertIn(aclrow._row, resource.acls)
self.assertEqual(cmd.direction, aclrow.direction)
self.assertEqual(cmd.priority, aclrow.priority)
self.assertEqual(cmd.match, aclrow.match)
@ -120,43 +128,50 @@ class TestAclOps(OvnNorthboundTest):
return aclrow
def test_acl_add(self):
self._acl_add('from-lport', 0, 'output == "fake_port" && ip',
'drop')
self._acl_add('lswitch', 'from-lport', 0,
'output == "fake_port" && ip', 'drop')
def test_acl_add_exists(self):
args = ('from-lport', 0, 'output == "fake_port" && ip', 'drop')
args = ('lswitch', 'from-lport', 0, 'output == "fake_port" && ip',
'drop')
self._acl_add(*args)
self.assertRaises(RuntimeError, self._acl_add, *args)
def test_acl_add_may_exist(self):
args = ('from-lport', 0, 'output == "fake_port" && ip', 'drop')
row = self._acl_add(*args)
row2 = self._acl_add(*args, may_exist=True)
row = self._acl_add('lswitch', *args)
row2 = self._acl_add('lswitch', *args, may_exist=True)
self.assertEqual(row, row2)
def test_acl_add_extids(self):
external_ids = {'mykey': 'myvalue', 'yourkey': 'yourvalue'}
acl = self._acl_add('from-lport', 0, 'output == "fake_port" && ip',
acl = self._acl_add('lswitch',
'from-lport', 0, 'output == "fake_port" && ip',
'drop', **external_ids)
self.assertEqual(external_ids, acl.external_ids)
def test_acl_del_all(self):
r1 = self._acl_add('from-lport', 0, 'output == "fake_port"', 'drop')
r1 = self._acl_add('lswitch', 'from-lport', 0, 'output == "fake_port"',
'drop')
self.api.acl_del(self.switch.uuid).execute(check_error=True)
self.assertNotIn(r1.uuid, self.api.tables['ACL'].rows)
self.assertEqual([], self.switch.acls)
def test_acl_del_direction(self):
r1 = self._acl_add('from-lport', 0, 'output == "fake_port"', 'drop')
r2 = self._acl_add('to-lport', 0, 'output == "fake_port"', 'allow')
r1 = self._acl_add('lswitch', 'from-lport', 0,
'output == "fake_port"', 'drop')
r2 = self._acl_add('lswitch', 'to-lport', 0,
'output == "fake_port"', 'allow')
self.api.acl_del(self.switch.uuid, 'from-lport').execute(
check_error=True)
self.assertNotIn(r1, self.switch.acls)
self.assertIn(r2, self.switch.acls)
def test_acl_del_direction_priority_match(self):
r1 = self._acl_add('from-lport', 0, 'output == "fake_port"', 'drop')
r2 = self._acl_add('from-lport', 1, 'output == "fake_port"', 'allow')
r1 = self._acl_add('lswitch', 'from-lport', 0,
'output == "fake_port"', 'drop')
r2 = self._acl_add('lswitch', 'from-lport', 1,
'output == "fake_port"', 'allow')
cmd = self.api.acl_del(self.switch.uuid,
'from-lport', 0, 'output == "fake_port"')
cmd.execute(check_error=True)
@ -172,12 +187,35 @@ class TestAclOps(OvnNorthboundTest):
priority=0)
def test_acl_list(self):
r1 = self._acl_add('from-lport', 0, 'output == "fake_port"', 'drop')
r2 = self._acl_add('from-lport', 1, 'output == "fake_port2"', 'allow')
r1 = self._acl_add('lswitch', 'from-lport', 0,
'output == "fake_port"', 'drop')
r2 = self._acl_add('lswitch', 'from-lport', 1,
'output == "fake_port2"', 'allow')
acls = self.api.acl_list(self.switch.uuid).execute(check_error=True)
self.assertIn(r1, acls)
self.assertIn(r2, acls)
def test_pg_acl_add(self):
self._acl_add('port_group', 'from-lport', 0,
'output == "fake_port" && ip', 'drop')
def test_pg_acl_del_all(self):
r1 = self._acl_add('port_group', 'from-lport', 0,
'output == "fake_port"', 'drop')
self.api.pg_acl_del(self.port_group.uuid).execute(check_error=True)
self.assertNotIn(r1.uuid, self.api.tables['ACL'].rows)
self.assertEqual([], self.port_group.acls)
def test_pg_acl_list(self):
r1 = self._acl_add('port_group', 'from-lport', 0,
'output == "fake_port"', 'drop')
r2 = self._acl_add('port_group', 'from-lport', 1,
'output == "fake_port2"', 'allow')
acls = self.api.pg_acl_list(self.port_group.uuid).execute(
check_error=True)
self.assertIn(r1, acls)
self.assertIn(r2, acls)
class TestQoSOps(OvnNorthboundTest):
def setUp(self):
@ -1325,25 +1363,13 @@ class TestPortGroup(OvnNorthboundTest):
('name', '=', self.pg_name)).execute(check_error=True)
self.assertEqual([], row)
def test_port_group_ports_and_acls(self):
def test_port_group_ports(self):
lsp_add_cmd = self.api.lsp_add(self.switch.uuid, 'testport')
acl_add_cmd_1 = self.api.acl_add(
self.switch.uuid, 'from-lport', 0, 'output == "fake_port"',
'drop')
acl_add_cmd_2 = self.api.acl_add(
self.switch.uuid, 'from-lport', 0, 'output == "fake_port" && ip',
'drop')
with self.api.transaction(check_error=True) as txn:
txn.add(lsp_add_cmd)
txn.add(acl_add_cmd_1)
txn.add(acl_add_cmd_2)
txn.add(self.api.pg_add(self.pg_name))
txn.add(self.api.pg_add_acls(
self.pg_name, [acl_add_cmd_1, acl_add_cmd_2]))
port_uuid = lsp_add_cmd.result.uuid
acl_uuid_1 = acl_add_cmd_1.result.uuid
acl_uuid_2 = acl_add_cmd_2.result.uuid
# Lets add the port using the UUID instead of a `Command` to
# exercise the API
@ -1354,37 +1380,29 @@ class TestPortGroup(OvnNorthboundTest):
('name', '=', self.pg_name)).execute(check_error=True)
self.assertIsNotNone(row)
self.assertEqual(self.pg_name, row[0]['name'])
# Assert the port and ACLs were added from the Port Group
# Assert the port was added from the Port Group
self.assertEqual([port_uuid], row[0]['ports'])
self.assertEqual(sorted([acl_uuid_1, acl_uuid_2]),
sorted(row[0]['acls']))
# Delete an ACL and the Port from the Port Group
# Delete the Port from the Port Group
with self.api.transaction(check_error=True) as txn:
txn.add(self.api.pg_del_ports(self.pg_name, port_uuid))
txn.add(self.api.pg_del_acls(self.pg_name, acl_uuid_1))
row = self.api.db_find(
'Port_Group',
('name', '=', self.pg_name)).execute(check_error=True)
self.assertIsNotNone(row)
self.assertEqual(self.pg_name, row[0]['name'])
# Assert the port and ACL were removed from the Port Group
# Assert the port was removed from the Port Group
self.assertEqual([], row[0]['ports'])
self.assertEqual([acl_uuid_2], row[0]['acls'])
def test_pg_del_ports_and_acls_if_exists(self):
def test_pg_del_ports_if_exists(self):
self.api.pg_add(self.pg_name).execute(check_error=True)
non_existent_res = uuidutils.generate_uuid()
# Assert that if if_exists is False (default) it will raise an error
self.assertRaises(RuntimeError, self.api.pg_del_ports(self.pg_name,
non_existent_res).execute, True)
self.assertRaises(RuntimeError, self.api.pg_del_acls(self.pg_name,
non_existent_res).execute, True)
# Assert that if if_exists is True it won't raise an error
self.api.pg_del_ports(self.pg_name, non_existent_res,
if_exists=True).execute(check_error=True)
self.api.pg_del_acls(self.pg_name, non_existent_res,
if_exists=True).execute(check_error=True)