Add github actions build and fix test and setup errors

Github actions includes the following tasks:
- runs linter
- runs pytest
- creates capirca package
- installs capirca package
- runs default `aclgen` command to get sample output
- creates the report with diff of output files from
  the main and current branches
- uploads the report as an artifact

Fix install/uninstall/lint errors:
- version string contains multiple lines
- during uninstall, the aclgen is not being removed
- various lint errors

PiperOrigin-RevId: 362635832

.github/workflows/main.yml

@@ -0,0 +1,102 @@
+---
+name: Python package
+
+on:
+  push:
+    branches: [ master ]
+  pull_request:
+    branches: [ master ]
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: true
+      matrix:
+        python-version: [3.7, 3.8, 3.9]
+
+    steps:
+    - name: Checkout branch with changes
+      uses: actions/checkout@v2
+      with:
+        path: current
+    - name: Checkout master branch
+      uses: actions/checkout@v2
+      with:
+        path: master
+        ref: master
+    - name: Set up Node.js
+      uses: actions/setup-node@v2
+      with:
+        node-version: '14'
+    - name: Set up Python ${{ matrix.python-version }}
+      uses: actions/setup-python@v2
+      with:
+        python-version: ${{ matrix.python-version }}
+    - name: Setup environment variables
+      run: |
+        mkdir -p artifacts
+        echo "wfdt=$(date +'%Y%m%d_%H%M%S')" >> $GITHUB_ENV
+    - name: Install dependencies
+      run: |
+        sudo apt update
+        sudo apt install unzip zip
+        python -m pip install --upgrade pip
+        python -m pip install setuptools wheel
+        python -m pip install flake8 pytest
+        cd current
+        if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
+        if [ -f test-requirements.txt ]; then pip install -r test-requirements.txt; fi
+    - name: Lint with flake8
+      run: |
+        cd current
+        flake8 . --count --select=W291,W293,W391 --statistic
+        flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
+        flake8 . --count --exit-zero --max-complexity=10 \
+          --max-line-length=127 --statistics
+    - name: Test with pytest
+      run: |
+        cd current
+        pytest
+    - name: Perform end-to-end testing with current branch or pull request
+      run: |
+        cd current
+        git status
+        python setup.py sdist bdist_wheel
+        python3 -m pip -v install dist/capirca*py3*.whl
+        aclgen --output_directory ./output --logtostderr
+        cd ./output/ && \
+          zip -r ../../artifacts/capirca_output_${{ env.wfdt }}.zip .
+        cd ..
+        python3 -m pip -v uninstall -y capirca
+    - name: Perform end-to-end testing with master branch
+      run: |
+        cd master
+        git status
+        python setup.py sdist bdist_wheel
+        python3 -m pip -v install dist/capirca*py3*.whl
+        aclgen --output_directory ./output --logtostderr
+        python3 -m pip -v uninstall -y capirca
+    - name: Compare output files between the branches
+      run: |
+        mkdir -p artifacts-diff
+        sudo npm install -g diff2html diff2html-cli
+        diff2html --version
+        diff -qr current/output master/output > \
+          ./artifacts-diff/policy_output.diff | true
+        cat ./artifacts-diff/policy_output.diff | grep Files | grep differ \
+          | cut -d" " -f2 | cut -d "/" -f3 > ./artifacts-diff/files.list
+        while read p; do diff -u master/output/$p current/output/$p | \
+          diff2html -i stdin --file ./artifacts-diff/$p.html | \
+          true; done < ./artifacts-diff/files.list
+        sed -i '/Diff to HTML by/d' ./artifacts-diff/*.html
+    - name: Upload generated policies
+      uses: actions/upload-artifact@v2
+      with:
+        name: capirca_output_${{ matrix.python-version }}_${{ env.wfdt }}
+        path: ./artifacts/capirca_output_${{ env.wfdt }}.zip
+    - name: Upload policy differences
+      uses: actions/upload-artifact@v2
+      with:
+        name: capirca_output_policy_diff
+        path: ./artifacts-diff

VERSION

@@ -1 +1 @@
-2.0.2
+2.0.3

capirca/lib/aclcheck.py

@@ -34,7 +34,7 @@ class AddressError(Error):
   """Incorrect IP address or format."""
 
 
-class BadPolicy(Error):
+class BadPolicyError(Error):
   """Item is not a valid policy object."""
 
 
@@ -45,19 +45,16 @@ class NoTargetError(Error):
 class AclCheck(object):
   """Check where hosts, ports and protocols match in a NAC policy.
 
-  Args:
-    pol:
-      policy.Policy object
-    src:
-      string, the source address
-    dst:
-      string: the destination address.
-    sport:
-      string, the source port.
-    dport:
-      string, the destination port.
-    proto:
-      string, the protocol.
+  Attributes:
+    pol_obj: policy.Policy object.
+    pol: policy.Policy object.
+    src: A string for the source address.
+    dst: A string for the destination address.
+    sport: A string for the source port.
+    dport: A string for the destination port.
+    proto: A string for the protocol.
+    matches: A list of term-related matches.
+    exact_matches: A list of exact matches.
 
   Returns:
     An AclCheck Object
@@ -112,8 +109,8 @@ def __init__(self,
       except ValueError:
         raise AddressError('bad destination address: %s\n' % dst)
 
-    if type(self.pol_obj) is not policy.Policy:
-      raise BadPolicy('Policy object is not valid.')
+    if not isinstance(self.pol_obj, (policy.Policy)):
+      raise BadPolicyError('Policy object is not valid.')
 
     self.matches = []
     self.exact_matches = []
@@ -184,7 +181,7 @@ def ActionMatch(self, action='any'):
     for match in self.matches:
       if match.action:
         if not match.possibles:
-          if action is 'any' or action in match.action:
+          if action == 'any' or action in match.action:
             match_list.append(match)
     return match_list
 
@@ -249,7 +246,7 @@ def _AddrInside(self, addr, addresses):
     Returns:
       bool: True of false
     """
-    if addr is 'any': return True   # always true if we match for any addr
+    if addr == 'any': return True   # always true if we match for any addr
     if not addresses: return True   # always true if term has nothing to match
     for ip in addresses:
       # ipaddr can incorrectly report ipv4 as contained with ipv6 addrs

capirca/lib/iptables.py

@@ -317,7 +317,7 @@ def __str__(self):
       ret_str.append(self._POSTJUMP_FORMAT.substitute(filter=self.filter,
                                                       term=self.term_name))
 
-    return '\n'.join(str(v) for v in ret_str if v is not '')
+    return '\n'.join(str(v) for v in ret_str if v)
 
   def _CalculateAddresses(self, term_saddr, exclude_saddr,
                           term_daddr, exclude_daddr):

capirca/lib/junipersrx.py

@@ -401,7 +401,7 @@ def _TranslatePolicy(self, pol, exp_info):
         address_book_type = set(
             [self._ZONE_ADDR_BOOK,
              self._GLOBAL_ADDR_BOOK]).intersection(extra_options)
-        if len(address_book_type) is 0:
+        if not address_book_type:
           address_book_type = {self._GLOBAL_ADDR_BOOK}
         self.addr_book_type.update(address_book_type)
         if len(self.addr_book_type) > 1:

capirca/lib/packetfilter.py

@@ -38,7 +38,7 @@ class DuplicateTermError(Error):
   """Raised when duplication of term names are detected."""
 
 
-class DuplicateShortenedTableName(Error):
+class DuplicateShortenedTableNameError(Error):
   """Raised when a duplicate shortened table name is found."""
 
 
@@ -256,7 +256,7 @@ def __str__(self):
         self.options,
         self.stateful,))
 
-    return '\n'.join(str(v) for v in ret_str if v is not '')
+    return '\n'.join(str(v) for v in ret_str if v)
 
   def _CheckAddressAf(self, addrs):
     """Verify that the requested address-family matches the address's family."""
@@ -481,7 +481,7 @@ def _TranslatePolicy(self, pol, exp_info):
 
           if (src_token in self.def_short_to_long and
               self.def_short_to_long[src_token] != source_addr.parent_token):
-            raise DuplicateShortenedTableName(
+            raise DuplicateShortenedTableNameError(
                 'There is a shortened name conflict between names %s and %s '
                 '(different named objects would conflict when shortened to %s)'
                 % (self.def_short_to_long[src_token],
@@ -500,7 +500,7 @@ def _TranslatePolicy(self, pol, exp_info):
 
           if (dst_token in self.def_short_to_long and
               self.def_short_to_long[dst_token] != dest_addr.parent_token):
-            raise DuplicateShortenedTableName(
+            raise DuplicateShortenedTableNameError(
                 'There is a shortened name conflict between names %s and %s '
                 '(different named objects would conflict when shortened to %s)'
                 %(self.def_short_to_long[dst_token],

capirca/lib/windows.py

@@ -148,7 +148,7 @@ def __str__(self):
                            dst_ports, ret_str)
     self._HandlePreRule(ret_str)
 
-    return '\n'.join(str(v) for v in ret_str if v is not '')
+    return '\n'.join(str(v) for v in ret_str if v)
 
   def _HandleIcmpTypes(self, icmp_types, protocols):
     """Perform implementation-specific icmp_type and protocol transforms.

setup.py

@@ -23,7 +23,7 @@
 root_dir = path.abspath(path.dirname(__file__))
 
 with open(path.join(root_dir, 'VERSION'), encoding='utf-8') as f:
-  version = f.read()
+  version = f.readline().strip()
 
 with open(path.join(root_dir, 'README.md'), encoding='utf-8') as f:
   long_description = f.read()

tests/lib/packetfilter_test.py

@@ -818,7 +818,7 @@ def testTableDuplicateShortNameError(self):
     self.naming.GetServiceByProto.return_value = ['25']
 
     self.assertRaises(
-        packetfilter.DuplicateShortenedTableName,
+        packetfilter.DuplicateShortenedTableNameError,
         packetfilter.PacketFilter.__init__,
         packetfilter.PacketFilter.__new__(packetfilter.PacketFilter),
         policy.ParsePolicy(
@@ -904,7 +904,7 @@ def testTableDiffObjectsShortenedAndNonShortened(self):
     self.naming.GetServiceByProto.return_value = ['53']
 
     self.assertRaises(
-        packetfilter.DuplicateShortenedTableName,
+        packetfilter.DuplicateShortenedTableNameError,
         packetfilter.PacketFilter.__init__,
         packetfilter.PacketFilter.__new__(packetfilter.PacketFilter),
         policy.ParsePolicy(
@@ -929,7 +929,7 @@ def testTableDuplicateShortNameErrorDiffFilter(self):
     self.naming.GetServiceByProto.return_value = ['53']
 
     self.assertRaises(
-        packetfilter.DuplicateShortenedTableName,
+        packetfilter.DuplicateShortenedTableNameError,
         packetfilter.PacketFilter.__init__,
         packetfilter.PacketFilter.__new__(packetfilter.PacketFilter),
         policy.ParsePolicy(