Refix: Validate acl.json after parsing, add unit test (sonic-net#322)

* Refix: Validate acl.json after parsing, add unit test * Include test input files in package
dawnbeauty · Sep 19, 2018 · 2ce2b78 · 2ce2b78
1 parent ac43b20
commit 2ce2b78
Show file tree

Hide file tree

Showing 9 changed files with 357 additions and 5 deletions.
diff --git a/README.md b/README.md
@@ -4,8 +4,12 @@
 
 Command-line utilities for SONiC
 
+## How to run unit test
+```python
+python2 -m py.test -v
+```
 
-# Contribution guide
+## Contribution guide
 
 All contributors must sign a contribution license agreement (CLA) before contributions can be accepted. This process is now automated via a GitHub bot when submitting new pull request. If the contributor has not yet signed a CLA, the bot will create a comment on the pull request containing a link to electronically sign the CLA.
 

diff --git a/acl_loader/main.py b/acl_loader/main.py
@@ -193,16 +193,25 @@ def is_table_control_plane(self, tname):
         """
         return self.tables_db_info[tname]['type'].upper() == self.ACL_TABLE_TYPE_CTRLPLANE
 
+    @staticmethod
+    def parse_acl_json(filename):
+        yang_acl = pybindJSON.load(filename, openconfig_acl, "openconfig_acl")
+        # Check pybindJSON parsing
+        # pybindJSON.load will silently return an empty json object if input invalid
+        with open(filename, 'r') as f:
+            plain_json = json.load(f)
+            if len(plain_json['acl']['acl-sets']['acl-set']) != len(yang_acl.acl.acl_sets.acl_set):
+                raise AclLoaderException("Invalid input file %s" % filename)
+        return yang_acl
+
     def load_rules_from_file(self, filename):
         """
         Load file with ACL rules configuration in openconfig ACL format. Convert rules
         to Config DB schema.
         :param filename: File in openconfig ACL format
         :return:
         """
-        self.yang_acl = pybindJSON.load(filename, openconfig_acl, "openconfig_acl")
-        if pybindJSON.dumps(self.yang_acl) == '{}':
-            raise AclLoaderException("Invalid input file %s" % filename)
+        self.yang_acl = AclLoader.parse_acl_json(filename)
         self.convert_rules()
 
     def convert_action(self, table_name, rule_idx, rule):

diff --git a/pytest.ini b/pytest.ini
@@ -0,0 +1,3 @@
+[pytest]
+filterwarnings =
+    ignore::DeprecationWarning
diff --git a/setup.py b/setup.py
@@ -36,7 +36,8 @@ def get_test_suite():
         'undebug',
     ],
     package_data={
-        'show': ['aliases.ini']
+        'show': ['aliases.ini'],
+        'sonic-utilities-tests': ['acl_input/*'],
     },
     scripts=[
         'scripts/aclshow',

diff --git a/sonic-utilities-tests/acl_input/acl1.json b/sonic-utilities-tests/acl_input/acl1.json
@@ -0,0 +1,148 @@
+{
+	"acl": {
+		"acl-sets": {
+			"acl-set": {
+				"sonic-ssh-only": {
+					"acl-entries": {
+						"acl-entry": {
+							"1": {
+								"config": {
+									"sequence-id": 1
+								},
+								"actions": {
+									"config": {
+										"forwarding-action": "ACCEPT"
+									}
+								},
+								"ip": {
+									"config": {
+										"protocol": "IP_TCP",
+										"source-ip-address": "192.168.0.0/18"
+									}
+								},
+								"transport": {
+									"config": {
+										"destination-port": "22"
+									}
+								}
+							},
+							"2": {
+								"config": {
+									"sequence-id": 2
+								},
+								"actions": {
+									"config": {
+										"forwarding-action": "ACCEPT"
+									}
+								},
+								"ip": {
+									"config": {
+										"protocol": "IP_TCP",
+										"source-ip-address": "192.168.192.0/18"
+									}
+								},
+								"transport": {
+									"config": {
+										"destination-port": "22"
+									}
+								}
+							}
+						}
+					},
+					"config": {
+						"name": "sonic-ssh-only"
+					}
+				},
+				"Sonic-SNMP_ACL": {
+					"acl-entries": {
+						"acl-entry": {
+							"1": {
+								"config": {
+									"sequence-id": 1
+								},
+								"actions": {
+									"config": {
+										"forwarding-action": "ACCEPT"
+									}
+								},
+								"ip": {
+									"config": {
+										"protocol": "IP_UDP",
+										"source-ip-address": "192.168.0.0/18"
+									}
+								}
+							}
+						}
+					},
+					"config": {
+						"name": "Sonic-SNMP_ACL"
+					}
+				},
+				"sonic-everflow": {
+					"acl-entries": {
+						"acl-entry": {
+							"1": {
+								"config": {
+									"sequence-id": 1
+								},
+								"actions": {
+									"config": {
+										"forwarding-action": "ACCEPT"
+									}
+								},
+								"ip": {
+									"config": {
+										"protocol": "IP_TCP",
+										"source-ip-address": "127.0.0.1/32",
+										"destination-ip-address": "127.0.0.1/32"
+									}
+								},
+								"transport": {
+									"config": {
+										"source-port": "0",
+										"destination-port": "0"
+									}
+								}
+							}
+						}
+					},
+					"config": {
+						"name": "sonic-everflow"
+					}
+				},
+				"everflowV6": {
+					"acl-entries": {
+						"acl-entry": {
+							"1": {
+								"config": {
+									"sequence-id": 1
+								},
+								"actions": {
+									"config": {
+										"forwarding-action": "ACCEPT"
+									}
+								},
+								"ip": {
+									"config": {
+										"protocol": "IP_TCP",
+										"source-ip-address": "::1/128",
+										"destination-ip-address": "::1/128"
+									}
+								},
+								"transport": {
+									"config": {
+										"source-port": "0",
+										"destination-port": "0"
+									}
+								}
+							}
+						}
+					},
+					"config": {
+						"name": "everflowV6"
+					}
+				}
+			}
+		}
+	}
+}
diff --git a/sonic-utilities-tests/acl_input/acl2.json b/sonic-utilities-tests/acl_input/acl2.json
@@ -0,0 +1,149 @@
+{
+	"acl": {
+		"acl-sets": {
+			"acl-set": {
+				"sonic-ssh-only": {
+					"acl-entries": {
+						"acl-entry": {
+							"1": {
+								"config": {
+									"sequence-id": 1
+								},
+								"actions": {
+									"config": {
+										"forwarding-action": "ACCEPT"
+									}
+								},
+								"ip": {
+									"config": {
+										"protocol": "IP_TCP",
+										"source-ip-address": "192.168.0.0/18"
+									}
+								},
+								"transport": {
+									"config": {
+										"destination-port": "22"
+									}
+								}
+							},
+							"2": {
+								"config": {
+									"sequence-id": 2
+								},
+								"actions": {
+									"config": {
+										"forwarding-action": "ACCEPT"
+									}
+								},
+								"ip": {
+									"config": {
+										"protocol": "IP_TCP",
+										"source-ip-address": "192.168.192.0/18"
+									}
+								},
+								"transport": {
+									"config": {
+										"destination-port": "22"
+									}
+								}
+							}
+						}
+					},
+					"config": {
+						"name": "sonic-ssh-only"
+					}
+				},
+				"Sonic-SNMP_ACL": {
+					"acl-entries": {
+						"acl-entry": {
+							"1": {
+								"config": {
+									"sequence-id": 1
+								},
+								"actions": {
+									"config": {
+										"forwarding-action": "ACCEPT"
+									}
+								},
+								"ip": {
+									"config": {
+										"protocol": "IP_UDP",
+										"source-ip-address": "192.168.0.0/18"
+									}
+								}
+							}
+						}
+					},
+					"config": {
+						"name": "Sonic-SNMP_ACL"
+					}
+				},
+				"sonic-everflow": {
+					"acl-entries": {
+						"acl-entry": {
+							"1": {
+								"config": {
+									"sequence-id": 1
+								},
+								"actions": {
+									"config": {
+										"forwarding-action": "ACCEPT"
+									}
+								},
+								"ip": {
+									"config": {
+										"protocol": "IP_TCP",
+										"source-ip-address": "127.0.0.1/32",
+										"destination-ip-address": "127.0.0.1/32"
+									}
+								},
+								"transport": {
+									"config": {
+										"source-port": "0",
+										"destination-port": "0"
+									}
+								}
+							}
+						}
+					},
+					"config": {
+						"name": "sonic-everflow"
+					}
+				},
+				"everflowV6": {
+					"acl-entries": {
+						"acl-entry": {
+							"1": {
+								"config": {
+									"sequence-id": 1
+								},
+								"actions": {
+									"config": {
+										"forwarding-action": "ACCEPT"
+									}
+								},
+								"ip": {
+									"config": {
+										"protocol": "IP_TCP",
+										"source-ip-address": "::1/128",
+										"destination-ip-address": "::1/128"
+									}
+								},
+								"transport": {
+									"config": {
+										"source-port": "0",
+										"destination-port": "0"
+									}
+								}
+							}
+						}
+					},
+					"config": {
+						"name": "everflowV6"
+					}
+				},
+				"TimeStampHeader": "Configuration last updated on: [9/13/2018 11:00:16 AM]"
+			}
+		}
+	}
+}
diff --git a/sonic-utilities-tests/acl_input/empty_acl.json b/sonic-utilities-tests/acl_input/empty_acl.json
@@ -0,0 +1,8 @@
+{
+  "acl": {
+    "acl-sets": {
+      "acl-set": {}
+    }
+  }
+}
+
diff --git a/sonic-utilities-tests/acl_loader_test.py b/sonic-utilities-tests/acl_loader_test.py
@@ -0,0 +1,27 @@
+import sys
+import os
+import pytest
+from unittest import TestCase
+
+test_path = os.path.dirname(os.path.abspath(__file__))
+modules_path = os.path.dirname(test_path)
+sys.path.insert(0, modules_path)
+
+from acl_loader import *
+from acl_loader.main import *
+
+class TestAclLoader(TestCase):
+    def setUp(self):
+        pass
+
+    def test_acl_empty(self):
+        yang_acl = AclLoader.parse_acl_json(os.path.join(test_path, 'acl_input/empty_acl.json'))
+        assert len(yang_acl.acl.acl_sets.acl_set) == 0
+
+    def test_valid(self):
+        yang_acl = AclLoader.parse_acl_json(os.path.join(test_path, 'acl_input/acl1.json'))
+        assert len(yang_acl.acl.acl_sets.acl_set) == 4
+
+    def test_invalid(self):
+        with pytest.raises(AclLoaderException):
+            yang_acl = AclLoader.parse_acl_json(os.path.join(test_path, 'acl_input/acl2.json'))
diff --git a/sonic-utilities-tests/pytest.ini b/sonic-utilities-tests/pytest.ini
@@ -0,0 +1,3 @@
+[pytest]
+filterwarnings =
+    ignore::DeprecationWarning