Add isort and Black (#2929)

.github/workflows/lint.yml

@@ -38,3 +38,10 @@ jobs:
         uses: golangci/golangci-lint-action@v3
         with:
           only-new-issues: true
+
+  lint-python:
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/checkout@v2
+      - uses: isort/[email protected]
+      - uses: psf/black@stable

Makefile

@@ -31,6 +31,13 @@ lint: ## Run linter
 	@git fetch
 	docker run --pull always --rm -v $(shell pwd):/kubernetes-ingress -w /kubernetes-ingress -v $(shell go env GOCACHE):/cache/go -e GOCACHE=/cache/go -e GOLANGCI_LINT_CACHE=/cache/go -v $(shell go env GOPATH)/pkg:/go/pkg golangci/golangci-lint:latest git diff -p origin/main > /tmp/diff.patch && golangci-lint --color always run -v --new-from-patch=/tmp/diff.patch
 
+.PHONY: lint-python
+lint-python: ## Run linter for python tests
+	@isort -V || (code=$$?; printf "\033[0;31mError\033[0m: there was a problem with isort, use 'brew install isort' to install it\n"; exit $$code)
+	@black --version || (code=$$?; printf "\033[0;31mError\033[0m: there was a problem with black, use 'brew install black' to install it\n"; exit $$code)
+	@isort .
+	@black .
+
 .PHONY: test
 test: ## Run tests
 	go test -tags=aws -shuffle=on -race ./...

perf-tests/conftest.py

@@ -1,18 +1,13 @@
 """Describe overall framework configuration."""
 
-from suite.resources_utils import get_first_pod_name
-from settings import (
-    DEFAULT_IMAGE,
-    DEFAULT_PULL_POLICY,
-    DEFAULT_IC_TYPE,
-    DEFAULT_SERVICE,
-    DEFAULT_DEPLOYMENT_TYPE,
-)
-from kubernetes.config.kube_config import KUBE_CONFIG_DEFAULT_LOCATION
 import os
-import pytest
 import sys
 
+import pytest
+from kubernetes.config.kube_config import KUBE_CONFIG_DEFAULT_LOCATION
+from settings import DEFAULT_DEPLOYMENT_TYPE, DEFAULT_IC_TYPE, DEFAULT_IMAGE, DEFAULT_PULL_POLICY, DEFAULT_SERVICE
+from suite.resources_utils import get_first_pod_name
+
 sys.path.insert(0, "../tests")
 
 
@@ -23,10 +18,16 @@ def pytest_addoption(parser) -> None:
     :return:
     """
     parser.addoption(
-        "--context", action="store", default="", help="The context to use in the kubeconfig file.",
+        "--context",
+        action="store",
+        default="",
+        help="The context to use in the kubeconfig file.",
     )
     parser.addoption(
-        "--image", action="store", default=DEFAULT_IMAGE, help="The Ingress Controller image.",
+        "--image",
+        action="store",
+        default=DEFAULT_IMAGE,
+        help="The Ingress Controller image.",
     )
     parser.addoption(
         "--image-pull-policy",
@@ -70,10 +71,16 @@ def pytest_addoption(parser) -> None:
         help="Show IC logs in stdout on test failure",
     )
     parser.addoption(
-        "--users", action="store", default="10", help="No. of users for response perf tests",
+        "--users",
+        action="store",
+        default="10",
+        help="No. of users for response perf tests",
     )
     parser.addoption(
-        "--hatch-rate", action="store", default="5", help="No. of users hatched per second",
+        "--hatch-rate",
+        action="store",
+        default="5",
+        help="No. of users hatched per second",
     )
     parser.addoption(
         "--time",
@@ -97,20 +104,17 @@ def pytest_collection_modifyitems(config, items) -> None:
     :return:
     """
     if config.getoption("--ic-type") == "nginx-ingress":
-        skip_for_nginx_oss = pytest.mark.skip(
-            reason="Skip a test for Nginx OSS")
+        skip_for_nginx_oss = pytest.mark.skip(reason="Skip a test for Nginx OSS")
         for item in items:
             if "skip_for_nginx_oss" in item.keywords:
                 item.add_marker(skip_for_nginx_oss)
     if config.getoption("--ic-type") == "nginx-plus-ingress":
-        skip_for_nginx_plus = pytest.mark.skip(
-            reason="Skip a test for Nginx Plus")
+        skip_for_nginx_plus = pytest.mark.skip(reason="Skip a test for Nginx Plus")
         for item in items:
             if "skip_for_nginx_plus" in item.keywords:
                 item.add_marker(skip_for_nginx_plus)
     if "-ap" not in config.getoption("--image"):
-        appprotect = pytest.mark.skip(
-            reason="Skip AppProtect test in non-AP image")
+        appprotect = pytest.mark.skip(reason="Skip AppProtect test in non-AP image")
         for item in items:
             if "appprotect" in item.keywords:
                 item.add_marker(appprotect)
@@ -134,9 +138,7 @@ def pytest_runtest_makereport(item) -> None:
     # we only look at actual failing test calls, not setup/teardown
     if rep.when == "call" and rep.failed and item.config.getoption("--show-ic-logs") == "yes":
         pod_namespace = item.funcargs["ingress_controller_prerequisites"].namespace
-        pod_name = get_first_pod_name(
-            item.funcargs["kube_apis"].v1, pod_namespace)
+        pod_name = get_first_pod_name(item.funcargs["kube_apis"].v1, pod_namespace)
         print("\n===================== IC Logs Start =====================")
-        print(item.funcargs["kube_apis"].v1.read_namespaced_pod_log(
-            pod_name, pod_namespace))
+        print(item.funcargs["kube_apis"].v1.read_namespaced_pod_log(pod_name, pod_namespace))
         print("\n===================== IC Logs End =====================")

perf-tests/suite/ap_request_perf.py

@@ -1,37 +1,33 @@
-import yaml, os
-from locust import TaskSet, HttpUser, task
+import os
+
+import yaml
+from locust import HttpUser, TaskSet, task
 
 host = ""
+
+
 class TestAPResponse(HttpUser):
-    # locust class to be invoked
+    # locust class to be invoked
     def on_start(self):
-        # get host from appprotect-ingress yaml before each test
+        # get host from appprotect-ingress yaml before each test
         ap_yaml = os.path.join(os.path.dirname(__file__), "../data/appprotect-ingress.yaml")
         with open(ap_yaml) as f:
             docs = yaml.safe_load_all(f)
             for dep in docs:
-                self.host = dep['spec']['rules'][0]['host']
+                self.host = dep["spec"]["rules"][0]["host"]
         print("Setup finished")
 
     @task
     def send_block_request(self):
-    # Send invalid request while dataguard alarm policy is active
-        response = self.client.get(
-            url="/<script>", 
-            headers={"host": self.host},
-            verify=False)
+        # Send invalid request while dataguard alarm policy is active
+        response = self.client.get(url="/<script>", headers={"host": self.host}, verify=False)
         print(response.text)
-    
+
     @task
     def send_allow_request(self):
-    # Send valid request while dataguard alarm policy is active
-        response = self.client.get(
-            url="", 
-            headers={"host": self.host},
-            verify=False)
+        # Send valid request while dataguard alarm policy is active
+        response = self.client.get(url="", headers={"host": self.host}, verify=False)
         print(response.text)
-    
+
     min_wait = 400
     max_wait = 1400
-
-

perf-tests/suite/test_ap_reload_perf.py

@@ -1,35 +1,40 @@
-import requests, logging
-import pytest, json
-import os, re, yaml, subprocess
+import json
+import logging
+import os
+import re
+import subprocess
 from datetime import datetime
-from settings import TEST_DATA, DEPLOYMENTS
+
+import pytest
+import requests
+import yaml
+from kubernetes.client import V1ContainerPort
+from settings import DEPLOYMENTS, TEST_DATA
 from suite.custom_resources_utils import (
     create_ap_logconf_from_yaml,
     create_ap_policy_from_yaml,
-    delete_ap_policy,
     delete_ap_logconf,
+    delete_ap_policy,
+    read_ap_crd,
 )
-from kubernetes.client import V1ContainerPort
 from suite.resources_utils import (
-    wait_before_test,
     create_example_app,
-    wait_until_all_pods_are_ready,
+    create_ingress,
+    create_ingress_with_ap_annotations,
     create_items_from_yaml,
-    delete_items_from_yaml,
     delete_common_app,
+    delete_items_from_yaml,
     ensure_connection_to_public_endpoint,
-    create_ingress,
-    create_ingress_with_ap_annotations,
-    replace_ingress_with_ap_annotations,
     ensure_response_from_backend,
-    wait_before_test,
     get_events,
-    get_ingress_nginx_template_conf,
+    get_file_contents,
     get_first_pod_name,
+    get_ingress_nginx_template_conf,
+    replace_ingress_with_ap_annotations,
+    wait_before_test,
     wait_for_event_increment,
-    get_file_contents,
+    wait_until_all_pods_are_ready,
 )
-from suite.custom_resources_utils import read_ap_crd
 from suite.yaml_utils import get_first_ingress_host_from_yaml
 
 ap_policy = "dataguard-alarm"
@@ -68,9 +73,7 @@ def enable_prometheus_port(
 
 
 @pytest.fixture(scope="class")
-def appprotect_setup(
-    request, kube_apis, ingress_controller_endpoint, test_namespace
-) -> AppProtectSetup:
+def appprotect_setup(request, kube_apis, ingress_controller_endpoint, test_namespace) -> AppProtectSetup:
     """
     Deploy simple application and all the AppProtect(dataguard-alarm) resources under test in one namespace.
 
@@ -169,7 +172,7 @@ def collect_prom_reload_metrics(self, metric_list, scenario, ip, port) -> None:
         reload_metric = ""
         for line in resp_decoded.splitlines():
             if "last_reload_milliseconds{class" in line:
-                reload_metric = re.findall("\d+", line)[0]
+                reload_metric = re.findall(r"\d+", line)[0]
                 metric_list.append(
                     {
                         f"Reload time ({scenario}) ": f"{reload_metric}ms",
@@ -200,9 +203,7 @@ def test_ap_perf_create_ingress(
         print("--------- Run test while AppProtect module is enabled with correct policy ---------")
         ensure_response_from_backend(appprotect_setup.req_url, ingress_host)
         wait_before_test(40)
-        response = requests.get(
-            appprotect_setup.req_url + "/<script>", headers={"host": ingress_host}, verify=False
-        )
+        response = requests.get(appprotect_setup.req_url + "/<script>", headers={"host": ingress_host}, verify=False)
         print(response.text)
         self.collect_prom_reload_metrics(
             reload_ap,
@@ -252,9 +253,7 @@ def test_ap_perf_ingress_path_change(
         ensure_response_from_backend(appprotect_setup.req_url, ingress_host)
         wait_before_test(30)
         response = ""
-        response = requests.get(
-            appprotect_setup.req_url + "/v1/<script>", headers={"host": ingress_host}, verify=False
-        )
+        response = requests.get(appprotect_setup.req_url + "/v1/<script>", headers={"host": ingress_host}, verify=False)
         print(response.text)
         self.collect_prom_reload_metrics(
             reload_ap_path,
@@ -298,9 +297,7 @@ def test_ap_perf_multiple_ingress(
         print("--------- Run test while AppProtect module is enabled with correct policy ---------")
         ensure_response_from_backend(appprotect_setup.req_url, ingress_host)
         wait_before_test(30)
-        response = requests.get(
-            appprotect_setup.req_url + "/<script>", headers={"host": ingress_host}, verify=False
-        )
+        response = requests.get(appprotect_setup.req_url + "/<script>", headers={"host": ingress_host}, verify=False)
         print(response.text)
         self.collect_prom_reload_metrics(
             reload_ap_with_ingress,
@@ -343,9 +340,7 @@ def test_ap_perf_response(
         ensure_response_from_backend(appprotect_setup.req_url, ingress_host)
         wait_before_test(30)
         response = ""
-        response = requests.get(
-            appprotect_setup.req_url + "/<script>", headers={"host": ingress_host}, verify=False
-        )
+        response = requests.get(appprotect_setup.req_url + "/<script>", headers={"host": ingress_host}, verify=False)
         print(appprotect_setup.req_url + "/<script>")
         print(ingress_host)
         print(response.text)

pyproject.toml

@@ -0,0 +1,13 @@
+[tool.black]
+line-length = 120
+target-version = ['py310']
+extend-exclude = '.*pb2.*'
+
+[tool.isort]
+atomic = true
+profile = "black"
+line_length = 120
+skip_gitignore = true
+balanced_wrapping = true
+filter_files = true
+skip_glob = ['*pb2*']

tests/conftest.py

@@ -4,9 +4,16 @@
 
 import pytest
 from kubernetes.config.kube_config import KUBE_CONFIG_DEFAULT_LOCATION
-from settings import (BATCH_RESOURCES, BATCH_START, DEFAULT_DEPLOYMENT_TYPE,
-                      DEFAULT_IC_TYPE, DEFAULT_IMAGE, DEFAULT_PULL_POLICY,
-                      DEFAULT_SERVICE, NUM_REPLICAS)
+from settings import (
+    BATCH_RESOURCES,
+    BATCH_START,
+    DEFAULT_DEPLOYMENT_TYPE,
+    DEFAULT_IC_TYPE,
+    DEFAULT_IMAGE,
+    DEFAULT_PULL_POLICY,
+    DEFAULT_SERVICE,
+    NUM_REPLICAS,
+)
 from suite.resources_utils import get_first_pod_name
 
 
@@ -127,7 +134,7 @@ def pytest_collection_modifyitems(config, items) -> None:
         for item in items:
             if "dos" in item.keywords:
                 item.add_marker(dos)
-    if  str(config.getoption("--batch-start")) != "True":
+    if str(config.getoption("--batch-start")) != "True":
         batch_start = pytest.mark.skip(reason="Skipping pod restart test with multiple resources")
         for item in items:
             if "batch_start" in item.keywords:
@@ -150,17 +157,9 @@ def pytest_runtest_makereport(item) -> None:
     rep = outcome.get_result()
 
     # we only look at actual failing test calls, not setup/teardown
-    if (
-        rep.when == "call"
-        and rep.failed
-        and item.config.getoption("--show-ic-logs") == "yes"
-    ):
+    if rep.when == "call" and rep.failed and item.config.getoption("--show-ic-logs") == "yes":
         pod_namespace = item.funcargs["ingress_controller_prerequisites"].namespace
         pod_name = get_first_pod_name(item.funcargs["kube_apis"].v1, pod_namespace)
         print("\n===================== IC Logs Start =====================")
-        print(
-            item.funcargs["kube_apis"].v1.read_namespaced_pod_log(
-                pod_name, pod_namespace
-            )
-        )
+        print(item.funcargs["kube_apis"].v1.read_namespaced_pod_log(pod_name, pod_namespace))
         print("\n===================== IC Logs End =====================")