Skip to content

Commit

Permalink
nokiasrl update description (#349)
Browse files Browse the repository at this point in the history 
* Based on user feedback:
* Put term name in the description (some policies have no comments)
* Put any comments in annotation

* Always set name field, not only when comments are present

* Update tests
  • Loading branch information
@jbemmel
jbemmel committed Sep 26, 2023
1 parent 312b448 commit 49bf474
Show file tree
Hide file tree
Showing 14 changed files with 63 additions and 24 deletions.
17 changes: 14 additions & 3 deletions aerleon/lib/nokiasrl.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,17 @@
Action = TypedDict("Action", {"accept": None, "drop": None})
ACLEntry = TypedDict(
"ACLEntry",
{"sequence-id": int, "action": Action, "match": Match},
{
"sequence-id": int,
"action": Action,
"match": Match,
"description": str,
"_annotate_description": str,
},
)
Entries = TypedDict(
"Entries", {"entry": List[ACLEntry], "description": str, "name": str, "_annotate": str}
"Entries",
{"entry": List[ACLEntry], "description": str, "name": str, "_annotate": str},
)
IPFilters = TypedDict("IPFilters", {"ipv4-filter": Entries, "ipv6-filter": Entries})

Expand Down Expand Up @@ -84,6 +91,10 @@ class SRLTerm(openconfig.Term):

ACTION_MAP = {'accept': 'accept', 'deny': 'drop', 'reject': 'drop'}

def SetName(self, name: str) -> None:
# Put name in description field
self.term_dict['description'] = name

def SetAction(self) -> None:
action = self.ACTION_MAP[self.term.action[0]]
log = {}
Expand All @@ -97,7 +108,7 @@ def SetAction(self) -> None:
self.term_dict['action'] = {action: log}

def SetComments(self, comments: List[str]) -> None:
self.term_dict['description'] = "_".join(comments)[:255]
self.term_dict['_annotate_description'] = "_".join(comments)[:255]

def SetOptions(self, family: str) -> None:
# Handle various options
Expand Down
5 changes: 5 additions & 0 deletions aerleon/lib/openconfig.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ def ConvertToDict(
term_af = self.AF_MAP.get(self.inet_version)
family = self.AF_RENAME[term_af]

self.SetName(self.term.name)

# Action
self.SetAction()

Expand Down Expand Up @@ -215,6 +217,9 @@ def ConvertToDict(

return rules

def SetName(self, name: str) -> None:
pass

def SetAction(self) -> None:
action = self.ACTION_MAP[self.term.action[0]]
self.term_dict['actions'] = {}
Expand Down
3 changes: 2 additions & 1 deletion tests/regression/nokiasrl/NokiaSRLTest.testDaddr.stdout.ref
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
"description": "The general policy comment.",
"entry": [
{
"_annotate_description": "Allow destination address.",
"action": {
"accept": {}
},
"description": "Allow destination address.",
"description": "good-term-1",
"match": {
"destination-ip": {
"prefix": "10.2.3.4/32"
Expand Down
3 changes: 2 additions & 1 deletion tests/regression/nokiasrl/NokiaSRLTest.testDport.stdout.ref
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
"description": "The general policy comment.",
"entry": [
{
"_annotate_description": "Allow TCP 53 dest.",
"action": {
"accept": {}
},
"description": "Allow TCP 53 dest.",
"description": "good-term-1",
"match": {
"destination-port": {
"value": 53
Expand Down
1 change: 1 addition & 0 deletions tests/regression/nokiasrl/NokiaSRLTest.testEstablished.stdout.ref
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"action": {
"accept": {}
},
"description": "established-term-1",
"match": {
"protocol": 6,
"source-port": {
Expand Down
6 changes: 4 additions & 2 deletions tests/regression/nokiasrl/NokiaSRLTest.testEverything.stdout.ref
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
"description": "The general policy comment.",
"entry": [
{
"_annotate_description": "Deny TCP & UDP 53 with saddr/daddr and logging.",
"action": {
"drop": {
"log": true
}
},
"description": "Deny TCP & UDP 53 with saddr/daddr and logging.",
"description": "good-term-1",
"match": {
"destination-ip": {
"prefix": "10.2.3.4/32"
Expand All @@ -26,12 +27,13 @@
"sequence-id": 5
},
{
"_annotate_description": "Deny TCP & UDP 53 with saddr/daddr and logging.",
"action": {
"drop": {
"log": true
}
},
"description": "Deny TCP & UDP 53 with saddr/daddr and logging.",
"description": "good-term-1",
"match": {
"destination-ip": {
"prefix": "10.2.3.4/32"
Expand Down
6 changes: 4 additions & 2 deletions tests/regression/nokiasrl/NokiaSRLTest.testMultiDport.stdout.ref
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
"description": "The general policy comment.",
"entry": [
{
"_annotate_description": "Allow TCP & UDP high.",
"action": {
"accept": {}
},
"description": "Allow TCP & UDP high.",
"description": "good-term-1",
"match": {
"destination-port": {
"range": {
Expand All @@ -27,10 +28,11 @@
"sequence-id": 5
},
{
"_annotate_description": "Allow TCP & UDP high.",
"action": {
"accept": {}
},
"description": "Allow TCP & UDP high.",
"description": "good-term-1",
"match": {
"destination-port": {
"range": {
Expand Down
3 changes: 2 additions & 1 deletion tests/regression/nokiasrl/NokiaSRLTest.testSaddr.stdout.ref
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
"description": "The general policy comment.",
"entry": [
{
"_annotate_description": "Allow source address.",
"action": {
"accept": {}
},
"description": "Allow source address.",
"description": "good-term-1",
"match": {
"source-ip": {
"prefix": "10.2.3.4/32"
Expand Down
3 changes: 2 additions & 1 deletion tests/regression/nokiasrl/NokiaSRLTest.testSport.stdout.ref
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
"description": "The general policy comment.",
"entry": [
{
"_annotate_description": "Allow TCP 53 source.",
"action": {
"accept": {}
},
"description": "Allow TCP 53 source.",
"description": "good-term-1",
"match": {
"protocol": 6,
"source-port": {
Expand Down
1 change: 1 addition & 0 deletions tests/regression/nokiasrl/NokiaSRLTest.testTcpEstablished.stdout.ref
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"action": {
"accept": {}
},
"description": "tcp-established-term-1",
"match": {
"protocol": 6,
"source-port": {
Expand Down
1 change: 1 addition & 0 deletions tests/regression/nokiasrl/NokiaSRLTest.testUdpEstablishedv6.stdout.ref
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"action": {
"accept": {}
},
"description": "udp-established-term-1",
"match": {
"destination-port": {
"range": {
Expand Down
3 changes: 2 additions & 1 deletion tests/regression/nokiasrl/NokiaSRLTest.testV6Daddr.stdout.ref
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
"description": "The general policy comment.",
"entry": [
{
"_annotate_description": "Allow destination address.",
"action": {
"accept": {}
},
"description": "Allow destination address.",
"description": "good-term-1",
"match": {
"destination-ip": {
"prefix": "2001:4860:8000::5/128"
Expand Down
3 changes: 2 additions & 1 deletion tests/regression/nokiasrl/NokiaSRLTest.testV6Saddr.stdout.ref
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
"description": "The general policy comment.",
"entry": [
{
"_annotate_description": "Allow source address.",
"action": {
"accept": {}
},
"description": "Allow source address.",
"description": "good-term-1",
"match": {
"source-ip": {
"prefix": "2001:4860:8000::5/128"
Expand Down
32 changes: 21 additions & 11 deletions tests/regression/nokiasrl/nokiasrl_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@
"action": {
"accept": {}
},
"description": "Allow source address.",
"description": "good-term-1",
"_annotate_description": "Allow source address.",
"match": {
"source-ip": {
"prefix": "10.2.3.4/32"
Expand All @@ -124,7 +125,8 @@
"action": {
"accept": {}
},
"description": "Allow source address.",
"description": "good-term-1",
"_annotate_description": "Allow source address.",
"match": {
"source-ip": {
"prefix": "2001:4860:8000::5/128"
Expand All @@ -150,7 +152,8 @@
"action": {
"accept": {}
},
"description": "Allow destination address.",
"description": "good-term-1",
"_annotate_description": "Allow destination address.",
"match": {
"destination-ip": {
"prefix": "10.2.3.4/32"
Expand All @@ -176,7 +179,8 @@
"action": {
"accept": {}
},
"description": "Allow destination address.",
"description": "good-term-1",
"_annotate_description": "Allow destination address.",
"match": {
"destination-ip": {
"prefix": "2001:4860:8000::5/128"
Expand All @@ -202,7 +206,8 @@
"action": {
"accept": {}
},
"description": "Allow TCP 53 source.",
"description": "good-term-1",
"_annotate_description": "Allow TCP 53 source.",
"match": {
"protocol": 6,
"source-port": { "value": 53 }
Expand All @@ -227,7 +232,8 @@
"action": {
"accept": {}
},
"description": "Allow TCP 53 dest.",
"description": "good-term-1",
"_annotate_description": "Allow TCP 53 dest.",
"match": {
"protocol": 6,
"destination-port": { "value": 53 }
Expand All @@ -252,7 +258,8 @@
"action": {
"accept": {}
},
"description": "Allow TCP & UDP high.",
"description": "good-term-1",
"_annotate_description": "Allow TCP & UDP high.",
"match": {
"protocol": 17,
"source-port": { "range": { "start": 1024, "end": 65535 } },
Expand All @@ -264,7 +271,8 @@
"action": {
"accept": {}
},
"description": "Allow TCP & UDP high.",
"description": "good-term-1",
"_annotate_description": "Allow TCP & UDP high.",
"match": {
"protocol": 6,
"source-port": { "range": { "start": 1024, "end": 65535 } },
Expand Down Expand Up @@ -292,7 +300,8 @@
"log": true
}
},
"description": "Deny TCP & UDP 53 with saddr/daddr and logging.",
"description": "good-term-1",
"_annotate_description": "Deny TCP & UDP 53 with saddr/daddr and logging.",
"match": {
"protocol": 17,
"destination-ip": { "prefix": "10.2.3.4/32" },
Expand All @@ -307,7 +316,8 @@
"log": true
}
},
"description": "Deny TCP & UDP 53 with saddr/daddr and logging.",
"description": "good-term-1",
"_annotate_description": "Deny TCP & UDP 53 with saddr/daddr and logging.",
"match": {
"protocol": 6,
"destination-ip": { "prefix": "10.2.3.4/32" },
Expand Down Expand Up @@ -352,7 +362,7 @@
"""

BAD_LOGGING = """
term good-term-1 {
term bad-term-5 {
comment:: "Allow TCP & UDP 53 with saddr/daddr and logging."
destination-address:: CORP_EXTERNAL
source-address:: CORP_EXTERNAL
Expand Down

0 comments on commit 49bf474

Please sign in to comment.