[aim-mapping] Fix policy-classifier direction impl
When mapping the GBP policy-classifier direction to AIM filters, the AIM filters were not being added to the correct filter directions in the AIM ContractSubjects. When the direction is "in" the forward filter should be added to the "in" bucket, the corresponding reverse filter should be added to the "out" bucket of the ContractSubject, and vice versa with the direction is "out". Both filters get added to both buckets when the direction is "bi". There was also an issue when the direction was updated in the policy-classifier. The updated ContractSubjects (as a result of the policy-classifier update) were not being written back, hence the direction update was not taking effect. This is also being fixed. Finally, when "any" protocol (i.e all protocols) classifier was created, the reverse filter entries were not being created. Reverse entries are now created for TCP, UDP, and ICMP. Change-Id: I39eef396893eec98b1da4ce42f29f0b39e8ea494
This commit is contained in:
Sumit Naiksatam
parent
2e4ff341e9
commit
23ff129cc6
|
|
@ -809,20 +809,28 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
|
|||
aim_reverse_filter.name]:
|
||||
if fname in flist:
|
||||
flist.remove(fname)
|
||||
to_add = []
|
||||
# Now add it to the relevant direction list(s)
|
||||
if c_dir == g_const.GP_DIRECTION_IN:
|
||||
to_add = [aim_contract_subject.in_filters]
|
||||
aim_contract_subject.in_filters.append(
|
||||
aim_filter.name)
|
||||
aim_contract_subject.out_filters.append(
|
||||
aim_reverse_filter.name)
|
||||
elif c_dir == g_const.GP_DIRECTION_OUT:
|
||||
to_add = [aim_contract_subject.out_filters]
|
||||
elif c_dir == g_const.GP_DIRECTION_BI:
|
||||
to_add = [aim_contract_subject.in_filters,
|
||||
aim_contract_subject.out_filters]
|
||||
for flist in to_add:
|
||||
for fname in filter(
|
||||
None, [aim_filter.name,
|
||||
aim_reverse_filter.name]):
|
||||
flist.append(fname)
|
||||
aim_contract_subject.in_filters.append(
|
||||
aim_reverse_filter.name)
|
||||
aim_contract_subject.out_filters.append(
|
||||
aim_filter.name)
|
||||
else:
|
||||
aim_contract_subject.in_filters.append(
|
||||
aim_filter.name)
|
||||
aim_contract_subject.out_filters.append(
|
||||
aim_reverse_filter.name)
|
||||
aim_contract_subject.in_filters.append(
|
||||
aim_reverse_filter.name)
|
||||
aim_contract_subject.out_filters.append(
|
||||
aim_filter.name)
|
||||
self.aim.create(aim_ctx, aim_contract_subject,
|
||||
overwrite=True)
|
||||
|
||||
@log.log_method_call
|
||||
def create_policy_rule_precommit(self, context):
|
||||
|
|
@ -1406,9 +1414,17 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
|
|||
classifier = context._plugin.get_policy_classifier(
|
||||
context._plugin_context, rule['policy_classifier_id'])
|
||||
if classifier['direction'] == g_const.GP_DIRECTION_IN:
|
||||
in_filters += aim_filters
|
||||
for fltr in aim_filters:
|
||||
if fltr.startswith(alib.REVERSE_PREFIX):
|
||||
out_filters.append(fltr)
|
||||
else:
|
||||
in_filters.append(fltr)
|
||||
elif classifier['direction'] == g_const.GP_DIRECTION_OUT:
|
||||
out_filters += aim_filters
|
||||
for fltr in aim_filters:
|
||||
if fltr.startswith(alib.REVERSE_PREFIX):
|
||||
in_filters.append(fltr)
|
||||
else:
|
||||
out_filters.append(fltr)
|
||||
else:
|
||||
in_filters += aim_filters
|
||||
out_filters += aim_filters
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ IMPLICIT_PREFIX = 'implicit-'
|
|||
PER_PROJECT = 'per-project'
|
||||
REVERSIBLE_PROTOCOLS = [n_constants.PROTO_NAME_TCP.lower(),
|
||||
n_constants.PROTO_NAME_UDP.lower(),
|
||||
n_constants.PROTO_NAME_ICMP.lower()]
|
||||
n_constants.PROTO_NAME_ICMP.lower(), None]
|
||||
ICMP_REPLY_TYPES = ['echo-rep', 'dst-unreach', 'src-quench', 'time-exceeded']
|
||||
CP_ENTRY = 'os-entry'
|
||||
|
||||
|
|
@ -78,24 +78,43 @@ def get_filter_entries_for_policy_classifier(classifier):
|
|||
f_attrs['dFromPort'] = port_min
|
||||
entries['forward_rules'] = {_get_filter_entry_name(x): f_attrs}
|
||||
# Also create reverse rule
|
||||
if f_attrs.get('prot') in REVERSIBLE_PROTOCOLS:
|
||||
if not f_attrs.get('prot') or (
|
||||
f_attrs.get('prot') in REVERSIBLE_PROTOCOLS):
|
||||
r_entries = {}
|
||||
r_attrs = copy.deepcopy(f_attrs)
|
||||
if r_attrs.get('dToPort') and r_attrs.get('dFromPort'):
|
||||
r_attrs.pop('dToPort')
|
||||
r_attrs.pop('dFromPort')
|
||||
r_attrs['sToPort'] = port_max
|
||||
r_attrs['sFromPort'] = port_min
|
||||
if r_attrs['prot'] == n_constants.PROTO_NAME_TCP.lower():
|
||||
# Only match on established sessions
|
||||
if f_attrs.get('prot') == n_constants.PROTO_NAME_TCP.lower() or (
|
||||
f_attrs.get('prot') == n_constants.PROTO_NAME_UDP.lower()):
|
||||
r_attrs = copy.deepcopy(f_attrs)
|
||||
if f_attrs.get('dToPort') and f_attrs.get('dFromPort'):
|
||||
r_attrs.pop('dToPort')
|
||||
r_attrs.pop('dFromPort')
|
||||
r_attrs['sToPort'] = port_max
|
||||
r_attrs['sFromPort'] = port_min
|
||||
if f_attrs.get('prot') == n_constants.PROTO_NAME_TCP.lower():
|
||||
# Only match on established sessions for tcp
|
||||
r_attrs['tcpRules'] = 'est'
|
||||
r_entries[_get_filter_entry_name(x)] = r_attrs
|
||||
if not f_attrs.get('prot'):
|
||||
# when no protocol is specified add reverse tcp rule
|
||||
# only for established sessions
|
||||
r_attrs = copy.deepcopy(f_attrs)
|
||||
r_attrs['etherT'] = 'ip'
|
||||
r_attrs['prot'] = n_constants.PROTO_NAME_TCP.lower()
|
||||
r_attrs['tcpRules'] = 'est'
|
||||
r_entries[_get_filter_entry_name(x)] = r_attrs
|
||||
if r_attrs['prot'] == n_constants.PROTO_NAME_ICMP.lower():
|
||||
r_entries = {}
|
||||
# create more entries:
|
||||
r_entries[_get_filter_entry_name(x)] = r_attrs
|
||||
# add another reverse rulw for UDP
|
||||
r_attrs = copy.deepcopy(f_attrs)
|
||||
r_attrs['etherT'] = 'ip'
|
||||
r_attrs['prot'] = n_constants.PROTO_NAME_UDP.lower()
|
||||
x += 1
|
||||
r_entries[_get_filter_entry_name(x)] = r_attrs
|
||||
if f_attrs.get('prot') == n_constants.PROTO_NAME_ICMP.lower() or (
|
||||
not f_attrs.get('prot')):
|
||||
# create more entries for icmp and no protocol cases
|
||||
for reply_type in ICMP_REPLY_TYPES:
|
||||
x += 1
|
||||
r_entry = copy.deepcopy(r_attrs)
|
||||
r_entry = copy.deepcopy(f_attrs)
|
||||
r_entry['etherT'] = 'ip'
|
||||
r_entry['prot'] = n_constants.PROTO_NAME_ICMP.lower()
|
||||
r_entry['icmpv4T'] = reply_type
|
||||
r_entries[_get_filter_entry_name(x)] = r_entry
|
||||
entries['reverse_rules'] = r_entries
|
||||
|
|
|
|||
|
|
@ -849,18 +849,20 @@ class AIMBaseTestCase(test_nr_base.CommonNeutronBaseTestCase,
|
|||
rev_filter = None
|
||||
|
||||
direction = pc['direction']
|
||||
expected_filters = []
|
||||
if direction == gp_const.GP_DIRECTION_IN:
|
||||
expected_filters = [expected_in_filters]
|
||||
elif direction == gp_const.GP_DIRECTION_OUT:
|
||||
expected_filters = [expected_out_filters]
|
||||
else:
|
||||
expected_filters = [expected_in_filters,
|
||||
expected_out_filters]
|
||||
for ef in expected_filters:
|
||||
ef.append(fwd_filter)
|
||||
expected_in_filters.append(fwd_filter)
|
||||
if rev_filter:
|
||||
ef.append(rev_filter)
|
||||
expected_out_filters.append(rev_filter)
|
||||
elif direction == gp_const.GP_DIRECTION_OUT:
|
||||
expected_out_filters.append(fwd_filter)
|
||||
if rev_filter:
|
||||
expected_in_filters.append(rev_filter)
|
||||
else:
|
||||
expected_in_filters.append(fwd_filter)
|
||||
expected_out_filters.append(fwd_filter)
|
||||
if rev_filter:
|
||||
expected_in_filters.append(rev_filter)
|
||||
expected_out_filters.append(rev_filter)
|
||||
|
||||
self.assertItemsEqual(expected_in_filters,
|
||||
contract_subject.in_filters)
|
||||
|
|
@ -3695,6 +3697,25 @@ class TestPolicyRuleBase(AIMBaseTestCase):
|
|||
dict((str(k), str(v)) for k, v in aim_object_to_dict(
|
||||
filter_entry).items()))
|
||||
|
||||
def _validate_1_to_many_reverse_filter_entries(
|
||||
self, policy_rule, afilter, filter_entries):
|
||||
pc = self.show_policy_classifier(
|
||||
policy_rule['policy_classifier_id'])['policy_classifier']
|
||||
expected_entries = alib.get_filter_entries_for_policy_classifier(pc)
|
||||
|
||||
for e_name, value in expected_entries['reverse_rules'].items():
|
||||
expected_filter_entry = self.driver._aim_filter_entry(
|
||||
self._neutron_context.session, afilter, e_name,
|
||||
alib.map_to_aim_filter_entry(value))
|
||||
filter_entry = (
|
||||
entry for entry in filter_entries if entry.name == e_name
|
||||
).next()
|
||||
self.assertItemsEqual(
|
||||
aim_object_to_dict(expected_filter_entry),
|
||||
# special processing to convert unicode to str
|
||||
dict((str(k), str(v)) for k, v in aim_object_to_dict(
|
||||
filter_entry).items()))
|
||||
|
||||
def _test_policy_rule_aim_mapping(self, policy_rule):
|
||||
aim_filter_name = str(self.name_mapper.policy_rule(
|
||||
self._neutron_context.session, policy_rule['id']))
|
||||
|
|
@ -3722,11 +3743,27 @@ class TestPolicyRuleBase(AIMBaseTestCase):
|
|||
self._aim_context, aim_resource.FilterEntry,
|
||||
tenant_name=aim_filters[0].tenant_name,
|
||||
filter_name=aim_filters[0].name)
|
||||
self.assertEqual(1, len(aim_filter_entries))
|
||||
self._validate_filter_entry(policy_rule, aim_filters[0],
|
||||
aim_filter_entries[0])
|
||||
if protocol == 'tcp' or protocol == 'udp' or not (
|
||||
filter_name.startswith('reverse')):
|
||||
# this will also check the forward entries for
|
||||
# icmp and no protocol case
|
||||
self.assertEqual(1, len(aim_filter_entries))
|
||||
self._validate_filter_entry(policy_rule, aim_filters[0],
|
||||
aim_filter_entries[0])
|
||||
elif protocol == 'icmp':
|
||||
# this is only for reverse entries with ICMP
|
||||
self.assertEqual(4, len(aim_filter_entries))
|
||||
self._validate_1_to_many_reverse_filter_entries(
|
||||
policy_rule, aim_filters[0], aim_filter_entries)
|
||||
else:
|
||||
# this is only for reverse entries when no protocol
|
||||
# is specified. One entry for TCP, one for UDP and
|
||||
# four entries for ICMP
|
||||
self.assertEqual(6, len(aim_filter_entries))
|
||||
self._validate_1_to_many_reverse_filter_entries(
|
||||
policy_rule, aim_filters[0], aim_filter_entries)
|
||||
filter_entries.append(aim_filter_entries[0])
|
||||
aim_obj_list.append(filter_entries)
|
||||
aim_obj_list.extend(filter_entries)
|
||||
self.assertEqual(
|
||||
filter_entries[0].dn,
|
||||
policy_rule[DN]['Forward-FilterEntries'][0])
|
||||
|
|
@ -3799,12 +3836,20 @@ class TestPolicyRule(TestPolicyRuleBase):
|
|||
|
||||
self.delete_policy_classifier(pc['id'], expected_res_status=204)
|
||||
|
||||
def test_policy_rule_lifecycle(self):
|
||||
def _test_policy_rule_lifecycle(self, protocol, direction):
|
||||
action1 = self.create_policy_action(
|
||||
action_type='redirect')['policy_action']
|
||||
classifier = self.create_policy_classifier(
|
||||
protocol='TCP', port_range="22",
|
||||
direction='bi')['policy_classifier']
|
||||
if protocol == 'ANY':
|
||||
classifier = self.create_policy_classifier(
|
||||
direction=direction)['policy_classifier']
|
||||
elif protocol in ['TCP', 'UDP']:
|
||||
classifier = self.create_policy_classifier(
|
||||
protocol=protocol, port_range="22",
|
||||
direction=direction)['policy_classifier']
|
||||
else:
|
||||
classifier = self.create_policy_classifier(
|
||||
protocol='ICMP',
|
||||
direction=direction)['policy_classifier']
|
||||
|
||||
pr = self.create_policy_rule(
|
||||
name="pr1", policy_classifier_id=classifier['id'],
|
||||
|
|
@ -3827,6 +3872,42 @@ class TestPolicyRule(TestPolicyRuleBase):
|
|||
|
||||
self._test_policy_rule_delete_aim_mapping(new_pr)
|
||||
|
||||
def test_policy_rule_lifecycle_tcp_in(self):
|
||||
self._test_policy_rule_lifecycle(protocol='TCP', direction='in')
|
||||
|
||||
def test_policy_rule_lifecycle_tcp_out(self):
|
||||
self._test_policy_rule_lifecycle(protocol='TCP', direction='out')
|
||||
|
||||
def test_policy_rule_lifecycle_tcp_bi(self):
|
||||
self._test_policy_rule_lifecycle(protocol='TCP', direction='bi')
|
||||
|
||||
def test_policy_rule_lifecycle_icmp_in(self):
|
||||
self._test_policy_rule_lifecycle(protocol='ICMP', direction='in')
|
||||
|
||||
def test_policy_rule_lifecycle_icmp_out(self):
|
||||
self._test_policy_rule_lifecycle(protocol='ICMP', direction='out')
|
||||
|
||||
def test_policy_rule_lifecycle_icmp_bi(self):
|
||||
self._test_policy_rule_lifecycle(protocol='ICMP', direction='bi')
|
||||
|
||||
def test_policy_rule_lifecycle_udp_in(self):
|
||||
self._test_policy_rule_lifecycle(protocol='UDP', direction='in')
|
||||
|
||||
def test_policy_rule_lifecycle_udp_out(self):
|
||||
self._test_policy_rule_lifecycle(protocol='UDP', direction='out')
|
||||
|
||||
def test_policy_rule_lifecycle_udp_bi(self):
|
||||
self._test_policy_rule_lifecycle(protocol='UDP', direction='bi')
|
||||
|
||||
def test_policy_rule_lifecycle_any_in(self):
|
||||
self._test_policy_rule_lifecycle(protocol='ANY', direction='in')
|
||||
|
||||
def test_policy_rule_lifecycle_any_out(self):
|
||||
self._test_policy_rule_lifecycle(protocol='ANY', direction='out')
|
||||
|
||||
def test_policy_rule_lifecycle_any_bi(self):
|
||||
self._test_policy_rule_lifecycle(protocol='ANY', direction='bi')
|
||||
|
||||
|
||||
class TestPolicyRuleRollback(TestPolicyRuleBase):
|
||||
|
||||
|
|
@ -3908,6 +3989,20 @@ class TestPolicyRuleSet(AIMBaseTestCase):
|
|||
'policy_rule_set']
|
||||
self._validate_policy_rule_set_aim_mapping(prs, rules)
|
||||
|
||||
# update directions of policy classifiers
|
||||
for r in rules:
|
||||
pc = self.show_policy_classifier(
|
||||
r['policy_classifier_id'])['policy_classifier']
|
||||
if pc['direction'] == 'in':
|
||||
new_direction = 'bi'
|
||||
elif pc['direction'] == 'out':
|
||||
new_direction = 'in'
|
||||
else:
|
||||
new_direction = 'out'
|
||||
self.update_policy_classifier(pc['id'],
|
||||
direction=new_direction)
|
||||
self._validate_policy_rule_set_aim_mapping(prs, rules)
|
||||
|
||||
new_rules = self._create_3_direction_rules()
|
||||
prs = self.update_policy_rule_set(
|
||||
prs['id'], policy_rules=[x['id'] for x in new_rules],
|
||||
|
|
|
|||
Loading…
Reference in New Issue