Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip NO_AF_LOG_ADDR when target=mixed #329

Merged
merged 3 commits into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions aerleon/lib/arista_tp.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,14 @@
},
}

def __init__(self, term: policy.Term, term_type: str, noverbose: bool) -> None:
def __init__(
self, term: policy.Term, term_type: str, noverbose: bool, filter_type: str = None
) -> None:
super().__init__(term)
self.term = term
self.term_type = term_type # drives the address-family
self.noverbose = noverbose
self.filter_type = filter_type

if term_type not in self._TERM_TYPE:
raise ValueError("unknown filter type: %s" % term_type)
Expand Down Expand Up @@ -260,11 +263,12 @@

term_block.append([MATCH_INDENT, src_str, False])
elif self.term.source_address:
logging.warning(
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction="source", af=self.term_type
if self.filter_type != 'mixed':
logging.warning(

Check warning on line 267 in aerleon/lib/arista_tp.py

View check run for this annotation

Codecov / codecov/patch

aerleon/lib/arista_tp.py#L267

Added line #L267 was not covered by tests
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction="source", af=self.term_type
)
)
)
return ""

# destination address
Expand All @@ -283,11 +287,12 @@
term_block.append([MATCH_INDENT, dst_str, False])

elif self.term.destination_address:
logging.warning(
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction="destination", af=self.term_type
if self.filter_type != 'mixed':
logging.warning(
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction="destination", af=self.term_type
)
)
)
return ""

if self.term.source_prefix:
Expand Down Expand Up @@ -905,7 +910,7 @@
term.counter = re.sub(r"\.", "-", str(term.counter))
policy_counters.add(term.counter)

new_terms.append(self._TERM(term, ft, noverbose))
new_terms.append(self._TERM(term, ft, noverbose, filter_type=filter_type))

self.arista_traffic_policies.append(
(
Expand Down
22 changes: 14 additions & 8 deletions aerleon/lib/cisco.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,7 @@
term_remark: bool = True,
platform: str = 'cisco',
verbose: bool = True,
filter_type: str = None,
) -> None:
self.term = term
self.proto_int = proto_int
Expand All @@ -434,6 +435,7 @@
self.term_remark = term_remark
self.platform = platform
self.verbose = verbose
self.filter_type = filter_type
# Our caller should have already verified the address family.
assert af in (4, 6)
self.af = af
Expand Down Expand Up @@ -518,11 +520,12 @@
if source_address_exclude:
source_address = nacaddr.ExcludeAddrs(source_address, source_address_exclude)
if not source_address:
logging.warning(
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction='source', af=self.text_af
if self.filter_type != 'mixed':
logging.warning(

Check warning on line 524 in aerleon/lib/cisco.py

View check run for this annotation

Codecov / codecov/patch

aerleon/lib/cisco.py#L524

Added line #L524 was not covered by tests
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction='source', af=self.text_af
)
)
)
return ''
if self.enable_dsmo:
source_address = summarizer.Summarize(source_address)
Expand All @@ -542,11 +545,12 @@
destination_address, destination_address_exclude
)
if not destination_address:
logging.warning(
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction='destination', af=self.text_af
if self.filter_type != 'mixed':
logging.warning(
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction='destination', af=self.text_af
)
)
)
return ''
if self.enable_dsmo:
destination_address = summarizer.Summarize(destination_address)
Expand Down Expand Up @@ -971,6 +975,7 @@
term_remark=self._TERM_REMARK,
platform=self._PLATFORM,
verbose=self.verbose,
filter_type=filter_type,
)
)
elif next_filter == 'object-group':
Expand All @@ -984,6 +989,7 @@
proto_int=self._PROTO_INT,
platform=self._PLATFORM,
verbose=self.verbose,
filter_type=filter_type,
)
)

Expand Down
45 changes: 26 additions & 19 deletions aerleon/lib/juniper.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@
interface e.g. INGRESS.
interface_type: Enum indicating the type of interface filter will be applied
e.g. LOOPBACK.
filter_type: String indicating the type of the filter, which may be mixed.
"""

_PLATFORM = 'juniper'
Expand Down Expand Up @@ -199,6 +200,7 @@
noverbose: bool,
filter_direction: str = None,
interface_type: str = None,
filter_type: str = None,
):
super().__init__(term)
self.term = term
Expand All @@ -208,6 +210,7 @@
# Filter direction and interface type are needed in juniperevo sub-class for IPv6 filters.
self.filter_direction = filter_direction
self.interface_type = interface_type
self.filter_type = filter_type

if self._PLATFORM != 'msmpc':
if term_type not in self._TERM_TYPE:
Expand Down Expand Up @@ -369,9 +372,10 @@
config.Append('%s;' % addr)
config.Append('}')
elif self.term.address:
logging.warning(
self.NO_AF_LOG_ADDR.substitute(term=self.term.name, af=self.term_type)
)
if self.filter_type != 'mixed':
logging.warning(

Check warning on line 376 in aerleon/lib/juniper.py

View check run for this annotation

Codecov / codecov/patch

aerleon/lib/juniper.py#L376

Added line #L376 was not covered by tests
self.NO_AF_LOG_ADDR.substitute(term=self.term.name, af=self.term_type)
)
return ''

# source address
Expand Down Expand Up @@ -402,11 +406,12 @@
config.Append('%s except;' % addr)
config.Append('}')
elif self.term.source_address:
logging.warning(
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction='source', af=self.term_type
if self.filter_type != 'mixed':
logging.warning(
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction='source', af=self.term_type
)
)
)
return ''

# destination address
Expand Down Expand Up @@ -436,11 +441,12 @@
config.Append('%s except;' % addr)
config.Append('}')
elif self.term.destination_address:
logging.warning(
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction='destination', af=self.term_type
if self.filter_type != 'mixed':
logging.warning(

Check warning on line 445 in aerleon/lib/juniper.py

View check run for this annotation

Codecov / codecov/patch

aerleon/lib/juniper.py#L445

Added line #L445 was not covered by tests
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction='destination', af=self.term_type
)
)
)
return ''

# forwarding-class
Expand Down Expand Up @@ -1035,14 +1041,14 @@
else:
filter_types_to_process = [filter_type]

for filter_type in filter_types_to_process:
for term_filter_type in filter_types_to_process:

filter_name_suffix = ''
# If mixed filter_type, will append 4 or 6 to the filter name
if len(filter_types_to_process) > 1:
if filter_type == 'inet':
if term_filter_type == 'inet':
filter_name_suffix = '4'
if filter_type == 'inet6':
if term_filter_type == 'inet6':
filter_name_suffix = '6'

term_names = set()
Expand All @@ -1052,7 +1058,7 @@
# Ignore if the term is for a different AF
if (
term.restrict_address_family
and term.restrict_address_family != filter_type
and term.restrict_address_family != term_filter_type
):
continue

Expand All @@ -1069,31 +1075,32 @@
)
term_names.add(term.name)

term = self.FixHighPorts(term, af=filter_type)
term = self.FixHighPorts(term, af=term_filter_type)
if not term:
continue

if 'is-fragment' in term.option and filter_type == 'inet6':
if 'is-fragment' in term.option and term_filter_type == 'inet6':
raise JuniperFragmentInV6Error(
'The term %s uses "is-fragment" but ' 'is a v6 policy.' % term.name
)

new_terms.append(
self._TERM(
term,
filter_type,
term_filter_type,
enable_dsmo,
noverbose,
filter_direction,
interface_type,
filter_type=filter_type,
)
)

self.juniper_policies.append(
(
header,
filter_name + filter_name_suffix,
filter_type,
term_filter_type,
interface_specific,
filter_enhanced_mode,
new_terms,
Expand Down
18 changes: 11 additions & 7 deletions aerleon/lib/packetfilter.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,20 +155,24 @@
# source address
term_saddrs = self._CheckAddressAf(self.term.source_address)
if not term_saddrs:
logging.warning(
self.NO_AF_LOG_ADDR.substitute(term=self.term.name, direction='source', af=self.af)
)
if self.af != 'mixed':
logging.warning(

Check warning on line 159 in aerleon/lib/packetfilter.py

View check run for this annotation

Codecov / codecov/patch

aerleon/lib/packetfilter.py#L159

Added line #L159 was not covered by tests
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction='source', af=self.af
)
)
return ''
term_saddr = self._GenerateAddrStatement(term_saddrs, self.term.source_address_exclude)

# destination address
term_daddrs = self._CheckAddressAf(self.term.destination_address)
if not term_daddrs:
logging.warning(
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction='destination', af=self.af
if self.af != 'mixed':
logging.warning(

Check warning on line 171 in aerleon/lib/packetfilter.py

View check run for this annotation

Codecov / codecov/patch

aerleon/lib/packetfilter.py#L171

Added line #L171 was not covered by tests
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction='destination', af=self.af
)
)
)
return ''
term_daddr = self._GenerateAddrStatement(
term_daddrs, self.term.destination_address_exclude
Expand Down
18 changes: 11 additions & 7 deletions aerleon/lib/pcap.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,12 @@
# source address
term_saddrs = self._CheckAddressAf(self.term.source_address)
if not term_saddrs:
logging.warning(
self.NO_AF_LOG_ADDR.substitute(term=self.term.name, direction='source', af=self.af)
)
if self.af != 'mixed':
logging.warning(

Check warning on line 132 in aerleon/lib/pcap.py

View check run for this annotation

Codecov / codecov/patch

aerleon/lib/pcap.py#L132

Added line #L132 was not covered by tests
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction='source', af=self.af
)
)
return ''

conditions.append(
Expand All @@ -140,11 +143,12 @@
# destination address
term_daddrs = self._CheckAddressAf(self.term.destination_address)
if not term_daddrs:
logging.warning(
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction='destination', af=self.af
if self.af != 'mixed':
logging.warning(

Check warning on line 147 in aerleon/lib/pcap.py

View check run for this annotation

Codecov / codecov/patch

aerleon/lib/pcap.py#L147

Added line #L147 was not covered by tests
self.NO_AF_LOG_ADDR.substitute(
term=self.term.name, direction='destination', af=self.af
)
)
)
return ''

conditions.append(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
traffic-policies
no traffic-policy test-filter
traffic-policy test-filter
match good-term-1 ipv6
protocol icmpv6
!

23 changes: 23 additions & 0 deletions tests/regression/arista_tp/arista_tp_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@
target:: arista_tp test-filter inet6
}
"""
GOOD_MIXED_HEADER = """
header {
target:: arista_tp test-filter mixed
}
"""
GOOD_NOVERBOSE_MIXED_HEADER = """
header {
target:: arista_tp test-filter mixed noverbose
Expand Down Expand Up @@ -902,6 +907,24 @@ def testLoggingOption(self):
self.assertIn(" log\n", output)
print(atp)

@mock.patch.object(arista_tp.logging, "warning")
@capture.stdout
def testSkipTermAF(self, mock_warning):
self.naming.GetNetAddr.return_value = [nacaddr.IP("10.0.0.0/8")]
self.naming.GetServiceByProto.return_value = ["25"]

atp = arista_tp.AristaTrafficPolicy(
policy.ParsePolicy(GOOD_HEADER_INET6 + GOOD_TERM_1_V6, self.naming), EXP_INFO
)
str(atp)

mock_warning.assert_called_once_with(
"Term good-term-2 will not be rendered,"
" as it has destination address match specified"
" but no destination addresses of inet6 address family are present."
)
print(atp)

@mock.patch.object(arista_tp.logging, "warning")
@capture.stdout
def testIcmpv6InetMismatch(self, mock_warning):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
! $Id:$
! $Date:$
! $Revision:$
no ip access-list extended mixed_acl
ip access-list extended mixed_acl
remark $Id:$
remark mixed inet/inet6 header test


remark good-term
permit tcp any 10.0.0.0 0.255.255.255

exit

no ipv6 access-list ipv6-mixed_acl
ipv6 access-list ipv6-mixed_acl
remark $Id:$
remark mixed inet/inet6 header test

exit

Loading