Improve commit checks and code formatting (#26)

rickymoorhouse · web-flow · commit b5bdfaf01997 · 2023-02-13T17:55:40.000Z
- update github actions: 
     - Separate lint, build, test action
     - Add coverage report in text to PR

 - fix: reduce number of flake8 warnings
     - fix formating
     - remove unused code
- fix: enable warnings and improve logging
 - move to codeql v2

Signed-off-by: Ricky Moorhouse &lt;MOORH@uk.ibm.com&gt;
diff --git a/.coveragerc b/.coveragerc
@@ -0,0 +1,2 @@
+[run]
+relative_files = true
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
@@ -50,7 +50,7 @@ jobs:
     # Autobuild attempts to build any compiled languages  (C/C++, C#, or Java).
     # If this step fails, then you should remove it and run the build manually (see below)
     - name: Autobuild
-      uses: github/codeql-action/autobuild@v1
+      uses: github/codeql-action/autobuild@v2
 
     # ℹ️ Command-line programs to run using the OS shell.
     # 📚 https://git.io/JvXDl
diff --git a/.github/workflows/coverage.yaml b/.github/workflows/coverage.yaml
@@ -0,0 +1,25 @@
+# .github/workflows/coverage.yml
+name: Post coverage comment
+
+on:
+  workflow_run:
+    workflows: ["CI"]
+    types:
+      - completed
+
+jobs:
+  test:
+    name: Run tests & display coverage
+    runs-on: ubuntu-latest
+    if: github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion == 'success'
+    steps:
+      # DO NOT run actions/checkout@v2 here, for securitity reasons
+      # For details, refer to https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
+      - name: Post comment
+        uses: ewjoachim/python-coverage-comment-action@v2
+        with:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          GITHUB_PR_RUN_ID: ${{ github.event.workflow_run.id }}
+          # Update those if you changed the default values:
+          # COMMENT_ARTIFACT_NAME: python-coverage-comment-action
+          # COMMENT_FILENAME: python-coverage-comment-action.txt
diff --git a/.github/workflows/test-and-publish.yml b/.github/workflows/test-and-publish.yml
@@ -0,0 +1,80 @@
+# This workflow will install Python dependencies, run tests and lint with a single version of Python
+# For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions
+
+name: Build and publish
+
+on:
+  push:
+    # Publish `main` as Docker `latest` image.
+    branches:
+      - main    
+    # Publish `v1.2.3` tags as releases.
+    tags:
+      - v*
+  
+  pull_request:
+    branches: [ main ]
+
+env:
+  IMAGE_NAME: ghcr.io/ibm/apiconnect-trawler/trawler
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v2
+    - name: Set up Python 3.10
+      uses: actions/setup-python@v2
+      with:
+        python-version: "3.10"
+    - name: Install dependencies
+      run: |
+        python -m pip install --upgrade pip
+        pip install flake8 pytest
+        if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
+        if [ -f requirements-dev.txt ]; then pip install -r requirements-dev.txt; fi
+    - name: Lint with flake8
+      run: |
+        # stop the build if there are Python syntax errors or undefined names
+        flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
+        # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
+        flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
+    - name: Test with pytest
+      run: |
+        SECRETS=test-assets coverage run --source . -m py.test -Werror -p no:unraisableexception
+        coverage report
+
+  # Push image to GitHub Packages.
+  # See also https://docs.docker.com/docker-hub/builds/
+  publish:
+    runs-on: ubuntu-latest
+    if: github.event_name == 'push'
+
+    steps:
+      - name: Clone the repository
+        uses: actions/checkout@v2
+        
+      - name: Buildah Action
+        id: build-image
+        uses: redhat-actions/buildah-build@v2
+        with:
+          image: ${{ env.IMAGE_NAME }}
+          tags: latest ${{ github.ref_name }}
+          containerfiles: |
+            ./Containerfile
+
+      - name: Log in to the GitHub Container registry
+        uses: redhat-actions/podman-login@v1
+        with:
+          registry: "ghcr.io"
+          username: ${{ github.actor }}
+          password: ${{ secrets.CR_PAT }}
+
+      - name: Push to GitHub Container Repository
+        id: push-to-ghcr
+        uses: redhat-actions/push-to-registry@v2
+        with:
+          registry: "ghcr.io"
+          image: ${{ steps.build-image.outputs.image }}
+          tags: latest ${{ github.ref_name }}
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -0,0 +1,6 @@
+{
+    "python.linting.pylamaEnabled": false,
+    "python.linting.pylintEnabled": true,
+    "python.linting.enabled": true,
+    "git.alwaysSignOff": true
+}
diff --git a/analytics_net.py b/analytics_net.py
@@ -39,8 +39,6 @@ def __init__(self, config, trawler):
             self.find_hostname_and_certs()
         else:
             self.find_hostname_and_certs()
-        logger.trace("Hostname is %s", self.hostname)
-        logger.trace("Certificate file is %s", self.certificates.name)
 
     def load_certs_from_secret(self, v1, secret_name):
         # Get certificates to communicate with analytics
@@ -110,6 +108,7 @@ def find_hostname_and_certs(self):
                 if cert:
                     combined = key + "\n".encode() + cert
                     self.certificates = tempfile.NamedTemporaryFile('w', delete=False)
+                    logger.trace("Certificate file is %s", self.certificates.name)
                     with self.certificates as certfile:
                         certfile.write(combined.decode())
 
diff --git a/manager_net.py b/manager_net.py
@@ -98,8 +98,8 @@ def find_hostname(self):
             if self.use_kubeconfig:
                 logger.info("Using KUBECONFIG")
                 config.load_kube_config()
-                netv1 = client.NetworkingV1Api()
-                ingresslist = netv1.list_namespaced_ingress(namespace=self.namespace)
+                v1 = client.NetworkV1Api()
+                ingresslist = v1.list_namespaced_ingress(namespace=self.namespace)
                 for ing in ingresslist.items:
                     if ing.metadata.name.endswith('apiconnect-api') or ing.metadata.name.endswith('platform-api'):
                         logger.info("Identified ingress host: {}".format(ing.spec.rules[0].host))
diff --git a/trawler.py b/trawler.py
@@ -1,20 +1,20 @@
 #!/usr/bin/python3
+""" Main trawler application """
 
 import os
 import time
-import alog
 import threading
-import yaml
+import ssl
+import alog
 import click
+from prometheus_client import start_http_server, Gauge, Counter, make_wsgi_app
+import yaml
 from certs_net import CertsNet
 from apiconnect_net import APIConnectNet
 from datapower_net import DataPowerNet
 from manager_net import ManagerNet
 from analytics_net import AnalyticsNet
-from watch_pods import Watcher
-from prometheus_client import start_http_server
 import metrics_graphite
-from prometheus_client import Gauge, Counter
 
 
 logger = alog.use_channel("trawler")
@@ -67,24 +67,24 @@ def __init__(self, config_file=None):
             # Check for KUBERNETES_SERVICE_HOST to determine if running within kubernetes
             if os.getenv('KUBERNETES_SERVICE_HOST'):
                 self.use_kubeconfig = False
-        self.watcher = Watcher()
 
     def read_secret(self, key):
         """ Helper function read secrets from mounted k8s secrets """
         try:
-            with open("{}/{}".format(self.secrets_path, key), 'r') as secret:
+            with open("{}/{}".format(self.secrets_path, key), 'r', encoding='utf-8') as secret:
                 value = secret.read().rstrip()
             return value
-        except FileNotFoundError as e:
-            logger.exception(e)
+        except FileNotFoundError as not_found_exception:
+            logger.exception(not_found_exception)
             return None
 
     def load_config(self, config_file):
+        """ Load trawler config """
         try:
-            with open(config_file, 'r') as config_yaml:
+            with open(config_file, 'r', encoding='utf-8') as config_yaml:
                 self.config = yaml.safe_load(config_yaml)
-        except FileNotFoundError as e:
-            logger.exception(e)
+        except FileNotFoundError as not_found_exception:
+            logger.exception(not_found_exception)
             exit(2)
 
     def set_gauge(self, component, target_name, value, pod_name=None, labels=None):
@@ -95,14 +95,15 @@ def set_gauge(self, component, target_name, value, pod_name=None, labels=None):
             labels['pod'] = pod_name
         if 'labels' in self.config['prometheus']:
             labels = {**self.config['prometheus']['labels'], **labels}
-        logger.debug("Entering set_gauge - params: ({}, {}, {}, {})".format(component, target_name, value, pod_name))
+        logger.debug("Entering set_gauge - params: (%s, %s, %s, %s)",
+                     component, target_name, value, pod_name)
         logger.debug(labels)
-        if type(value) is float or type(value) is int:
+        if isinstance(value, (float, int)):
             target_name = target_name.replace('-', '_')
             if self.config['prometheus']['enabled']:
                 prometheus_target = "{}_{}".format(component, target_name.replace('.', '_'))
                 if prometheus_target not in self.gauges:
-                    logger.info("Creating gauge {}".format(prometheus_target))
+                    logger.info("Creating gauge %s", prometheus_target)
                     if labels:
                         self.gauges[prometheus_target] = Gauge(
                             prometheus_target,
@@ -136,14 +137,15 @@ def inc_counter(self, component, target_name, value, pod_name=None, labels=None)
             labels['pod'] = pod_name
         if 'labels' in self.config['prometheus']:
             labels = {**self.config['prometheus']['labels'], **labels}
-        logger.debug("Entering inc_counter - params: ({}, {}, {}, {})".format(component, target_name, value, pod_name))
+        logger.debug("Entering inc_counter - params: (%s, %s, %s, %s)",
+                     component, target_name, value, pod_name)
         logger.debug(labels)
-        if type(value) is float or type(value) is int:
+        if isinstance(value, (float, int)):
             target_name = target_name.replace('-', '_')
             if self.config['prometheus']['enabled']:
                 prometheus_target = "{}_{}".format(component, target_name.replace('.', '_'))
                 if prometheus_target not in self.gauges:
-                    logger.info("Creating counter {}".format(prometheus_target))
+                    logger.info("Creating counter %s", prometheus_target)
                     if labels:
                         self.gauges[prometheus_target] = Counter(
                             prometheus_target,
@@ -166,28 +168,30 @@ def inc_counter(self, component, target_name, value, pod_name=None, labels=None)
                     metric_name = "{}.{}".format(component, target_name)
                 self.graphite.stage(metric_name, value)
 
+    def is_enabled(self, net_name):
+        """ is net in config and enabled """
+        if net_name in self.config['nets']:
+            if self.config['nets']['certs'].get('enabled', True):
+                return True
+        return False
+
     @alog.timed_function(logger.trace)
     def trawl_metrics(self):
         """ Main loop to trawl for metrics """
         # Initialise
         logger.info("Laying nets...")
         nets = []
-        if 'certs' in self.config['nets'] and self.config['nets']['certs'].get('enabled', True):
+        if self.is_enabled('certs'):
             nets.append(CertsNet(self.config['nets']['certs'], self))
-        if 'apiconnect' in self.config['nets'] and self.config['nets']['apiconnect'].get('enabled', True):
+        if self.is_enabled('apiconnect'):
             nets.append(APIConnectNet(self.config['nets']['apiconnect'], self))
-        if 'datapower' in self.config['nets'] and self.config['nets']['datapower'].get('enabled', True):
+        if self.is_enabled('datapower'):
             nets.append(DataPowerNet(self.config['nets']['datapower'], self))
-        if 'manager' in self.config['nets'] and self.config['nets']['manager'].get('enabled', True):
+        if self.is_enabled('manager'):
             nets.append(ManagerNet(self.config['nets']['manager'], self))
-        if 'analytics' in self.config['nets'] and self.config['nets']['analytics'].get('enabled', True):
+        if self.is_enabled('analytics'):
             nets.append(AnalyticsNet(self.config['nets']['analytics'], self))
 
-        # Start thread to watch if needed (nets need to call watcher.register)
-        if self.watcher.enabled:
-            watchThread = threading.Thread(target=self.watcher.watch_pods, daemon=True)
-            watchThread.start()
-
         while True:
             logger.info("Trawling for metrics...")
             for net in nets:
