refactor: replace magic numbers with named constants

CONTRIBUTING.md

@@ -69,6 +69,15 @@ gateway, route, authorization)
 - `multicluster/` for multi-cluster scenarios
 - Group tests by feature area (authorino, limitador, gateway, etc.)
 
+**Using Constants:**
+
+- All shared magic numbers (timeouts, ports, retry configs) live in `testsuite/utils/constants.py`
+- Import constants instead of hardcoding values, e.g. `from testsuite.utils.constants import K8S_WAIT_UNTIL_TIMEOUT`
+- When adding a new timeout, port, or retry value, check `constants.py` first. A suitable constant may already exist
+- If no suitable constant exists, add a new one to `constants.py` rather than hardcoding the value locally
+- Name constants clearly by domain and purpose (e.g. `PROMETHEUS_VERIFY_NO_TARGETS_RETRIES`)
+- Follow the `*_MAX_RETRIES` pattern for retry counts and `*_TIMEOUT` for durations
+
 ### Reformat, Commit Acceptance & Cleanup
 
 Before committing your changes, make sure the code is properly formatted, and all commit checks pass. Otherwise, your pull request may fail the GitHub Actions **Code Static Analysis** checks.

testsuite/backend/__init__.py

@@ -6,6 +6,7 @@
 from testsuite.gateway import Referencable
 from testsuite.lifecycle import LifecycleObject
 from testsuite.kubernetes.client import KubernetesClient
+from testsuite.utils.constants import HTTP_API_PORT
 
 
 class Backend(LifecycleObject, Referencable):
@@ -21,7 +22,13 @@ def __init__(self, cluster: KubernetesClient, name: str, label: str):
 
     @property
     def reference(self):
-        return {"group": "", "kind": "Service", "port": 8080, "name": self.name, "namespace": self.cluster.project}
+        return {
+            "group": "",
+            "kind": "Service",
+            "port": HTTP_API_PORT,
+            "name": self.name,
+            "namespace": self.cluster.project,
+        }
 
     @property
     def url(self):

testsuite/backend/httpbin.py

@@ -5,6 +5,7 @@
 from testsuite.kubernetes.client import KubernetesClient
 from testsuite.kubernetes.deployment import Deployment
 from testsuite.kubernetes.service import Service, ServicePort
+from testsuite.utils.constants import HTTP_API_PORT
 
 
 class Httpbin(Backend):
@@ -22,7 +23,7 @@ def commit(self):
             self.name,
             container_name="httpbin",
             image=self.image,
-            ports={"api": 8080},
+            ports={"api": HTTP_API_PORT},
             selector=Selector(matchLabels=match_labels),
             labels={"app": self.label},
         )
@@ -33,7 +34,7 @@ def commit(self):
             self.cluster,
             self.name,
             selector=match_labels,
-            ports=[ServicePort(name="http", port=8080, targetPort="api")],
+            ports=[ServicePort(name="http", port=HTTP_API_PORT, targetPort="api")],
             labels={"app": self.label},
         )
         self.service.commit()

testsuite/backend/llm_sim.py

@@ -5,6 +5,7 @@
 from testsuite.kubernetes.client import KubernetesClient
 from testsuite.kubernetes.deployment import Deployment
 from testsuite.kubernetes.service import Service, ServicePort
+from testsuite.utils.constants import HTTP_API_PORT
 
 
 class LlmSim(Backend):
@@ -23,10 +24,10 @@ def commit(self):
             self.name,
             container_name="llm-sim",
             image=self.image,
-            ports={"api": 8080},
+            ports={"api": HTTP_API_PORT},
             selector=Selector(matchLabels=match_labels),
             labels={"app": self.label},
-            command_args=["--model", self.model, "--port", "8080"],
+            command_args=["--model", self.model, "--port", str(HTTP_API_PORT)],
         )
         self.deployment.commit()
         self.deployment.wait_for_ready()
@@ -35,6 +36,6 @@ def commit(self):
             self.cluster,
             self.name,
             selector=match_labels,
-            ports=[ServicePort(name="http", port=8080, targetPort="api")],
+            ports=[ServicePort(name="http", port=HTTP_API_PORT, targetPort="api")],
         )
         self.service.commit()

testsuite/backend/mockserver.py

@@ -6,6 +6,7 @@
 from testsuite.kubernetes.config_map import ConfigMap
 from testsuite.kubernetes.deployment import Deployment, ContainerResources, ConfigMapVolume, VolumeMount
 from testsuite.kubernetes.service import Service, ServicePort
+from testsuite.utils.constants import MOCKSERVER_INTERNAL_PORT, HTTP_API_PORT, SERVICE_READY_TIMEOUT
 
 INIT_JSON_MOUNT = "/config/mockserver"
 
@@ -74,7 +75,7 @@ def commit(self):
             self.name,
             container_name="mockserver",
             image=settings["mockserver"]["image"],
-            ports={"api": 1080},
+            ports={"api": MOCKSERVER_INTERNAL_PORT},
             selector=Selector(matchLabels=match_labels),
             labels={"app": self.label},
             resources=ContainerResources(limits_memory="2G"),
@@ -89,7 +90,7 @@ def commit(self):
             self.cluster,
             self.name,
             selector=match_labels,
-            ports=[ServicePort(name="http", port=8080, targetPort="api")],
+            ports=[ServicePort(name="http", port=HTTP_API_PORT, targetPort="api")],
             labels={"app": self.label},
             service_type=self.service_type,
         )
@@ -103,7 +104,7 @@ def delete(self):
         finally:
             super().delete()
 
-    def wait_for_ready(self, timeout=60 * 5):
+    def wait_for_ready(self, timeout=SERVICE_READY_TIMEOUT):
         """Waits until Deployment and Service is marked as ready"""
         self.deployment.wait_for_ready(timeout)
         self.service.wait_for_ready(timeout, settings["control_plane"]["slow_loadbalancers"])

testsuite/config/__init__.py

@@ -4,6 +4,13 @@
 
 from testsuite.utils import hostname_to_ip
 from testsuite.config.tools import fetch_route, fetch_service, fetch_secret, fetch_service_ip, fetch_prometheus_url
+from testsuite.utils.constants import (
+    OTEL_COLLECTOR_PORT,
+    HTTP_API_PORT,
+    MOCKSERVER_INTERNAL_PORT,
+    VAULT_PORT,
+    REDIS_PORT,
+)
 
 
 # pylint: disable=too-few-public-methods
@@ -41,7 +48,7 @@ def __init__(self, name, default, **kwargs) -> None:
         ),
         DefaultValueValidator("tracing.backend", default="jaeger", is_in=["jaeger", "tempo"]),
         DefaultValueValidator(
-            "tracing.collector_url", default=fetch_service("jaeger-collector", protocol="rpc", port=4317)
+            "tracing.collector_url", default=fetch_service("jaeger-collector", protocol="rpc", port=OTEL_COLLECTOR_PORT)
         ),
         DefaultValueValidator("tracing.query_url", default=fetch_service_ip("jaeger-query", protocol="http", port=80)),
         Validator(
@@ -68,20 +75,25 @@ def __init__(self, name, default, **kwargs) -> None:
             & Validator("dns.dns_server2.geo_code", must_exist=True, ne=None)
         ),
         Validator("dns.default_geo_server", must_exist=True, ne=None, cast=hostname_to_ip),
-        DefaultValueValidator("keycloak.url", default=fetch_service_ip("keycloak", protocol="http", port=8080)),
+        DefaultValueValidator(
+            "keycloak.url", default=fetch_service_ip("keycloak", protocol="http", port=HTTP_API_PORT)
+        ),
         DefaultValueValidator("keycloak.password", default=fetch_secret("credential-sso", "ADMIN_PASSWORD")),
-        DefaultValueValidator("mockserver.url", default=fetch_service_ip("mockserver", protocol="http", port=1080)),
         DefaultValueValidator(
-            "vault.url", default=fetch_service_ip("vault", protocol="http", port=8200, namespace="tools-vault")
+            "mockserver.url", default=fetch_service_ip("mockserver", protocol="http", port=MOCKSERVER_INTERNAL_PORT)
+        ),
+        DefaultValueValidator(
+            "vault.url", default=fetch_service_ip("vault", protocol="http", port=VAULT_PORT, namespace="tools-vault")
         ),
         DefaultValueValidator("vault.token", default="root"),
-        DefaultValueValidator("redis.url", default=fetch_service_ip("redis", protocol="redis", port=6379)),
-        DefaultValueValidator("dragonfly.url", default=fetch_service_ip("dragonfly", protocol="redis", port=6379)),
-        DefaultValueValidator("valkey.url", default=fetch_service_ip("valkey", protocol="redis", port=6379)),
-        DefaultValueValidator("mockserver.url", default=fetch_service_ip("mockserver", protocol="http", port=1080)),
+        DefaultValueValidator("redis.url", default=fetch_service_ip("redis", protocol="redis", port=REDIS_PORT)),
+        DefaultValueValidator(
+            "dragonfly.url", default=fetch_service_ip("dragonfly", protocol="redis", port=REDIS_PORT)
+        ),
+        DefaultValueValidator("valkey.url", default=fetch_service_ip("valkey", protocol="redis", port=REDIS_PORT)),
         DefaultValueValidator(
             "custom_metrics_apiserver.url",
-            default=fetch_service_ip("custom-metrics-apiserver", protocol="http", port=8080),
+            default=fetch_service_ip("custom-metrics-apiserver", protocol="http", port=HTTP_API_PORT),
         ),
         DefaultValueValidator(
             "prometheus.url",

testsuite/gateway/envoy/__init__.py

@@ -12,6 +12,14 @@
 from testsuite.gateway.envoy.config import EnvoyConfig
 from testsuite.kubernetes.deployment import Deployment, VolumeMount, ConfigMapVolume
 from testsuite.kubernetes.service import Service, ServicePort
+from testsuite.utils.constants import (
+    ENVOY_STARTUP_SETTLE,
+    GATEWAY_READY_TIMEOUT,
+    HTTP_API_PORT,
+    ENVOY_ADMIN_PORT,
+    ENVOY_READINESS_INITIAL_DELAY,
+    ENVOY_READINESS_PERIOD,
+)
 
 
 class Envoy(Gateway):  # pylint: disable=too-many-instance-attributes
@@ -54,9 +62,9 @@ def rollout(self):
         """Restarts Envoy to apply newest config changes"""
         self.cluster.do_action("rollout", ["restart", f"deployment/{self.name}"])
         self.wait_for_ready()
-        time.sleep(3)  # or some reason wait_for_ready is not enough, needs more investigation
+        time.sleep(ENVOY_STARTUP_SETTLE)  # or some reason wait_for_ready is not enough, needs more investigation
 
-    def wait_for_ready(self, timeout: int = 10 * 60):
+    def wait_for_ready(self, timeout: int = GATEWAY_READY_TIMEOUT):
         with oc.timeout(timeout):
             assert self.cluster.do_action(
                 "rollout", ["status", f"deployment/{self.name}"]
@@ -69,7 +77,7 @@ def create_deployment(self) -> Deployment:
             self.name,
             container_name="envoy",
             image=self.image,
-            ports={"api": 8080, "admin": 8001},
+            ports={"api": HTTP_API_PORT, "admin": ENVOY_ADMIN_PORT},
             selector=Selector(matchLabels={"deployment": self.name, **self.labels}),
             labels=self.labels,
             command_args=[
@@ -79,7 +87,11 @@ def create_deployment(self) -> Deployment:
             ],
             volumes=[ConfigMapVolume(config_map_name=self.name, name="config", items={"envoy.yaml": "envoy.yaml"})],
             volume_mounts=[VolumeMount(mountPath="/usr/local/etc/envoy", name="config", readOnly=True)],
-            readiness_probe={"httpGet": {"path": "/ready", "port": 8001}, "initialDelaySeconds": 3, "periodSeconds": 4},
+            readiness_probe={
+                "httpGet": {"path": "/ready", "port": ENVOY_ADMIN_PORT},
+                "initialDelaySeconds": ENVOY_READINESS_INITIAL_DELAY,
+                "periodSeconds": ENVOY_READINESS_PERIOD,
+            },
         )
 
     def commit(self):
@@ -94,7 +106,7 @@ def commit(self):
             self.cluster,
             self.name,
             selector={"deployment": self.name, **self.labels},
-            ports=[ServicePort(name="api", port=8080, targetPort="api")],
+            ports=[ServicePort(name="api", port=HTTP_API_PORT, targetPort="api")],
             service_type="LoadBalancer",
         )
         self.service.commit()

testsuite/gateway/gateway_api/gateway.py

@@ -13,6 +13,7 @@
 from testsuite.kuadrant.policy import Policy
 from testsuite.kubernetes.deployment import Deployment
 from testsuite.utils import check_condition, asdict, domain_match
+from testsuite.utils.constants import GATEWAY_READY_TIMEOUT, SLOW_LOADBALANCER_WAIT
 
 
 class KuadrantGateway(KubernetesObject, Gateway):
@@ -92,12 +93,12 @@ def is_ready(self):
                 return True
         return False
 
-    def wait_for_ready(self, timeout: int = 10 * 60):
+    def wait_for_ready(self, timeout: int = GATEWAY_READY_TIMEOUT):
         """Waits for the gateway to be ready in the sense of is_ready(self)"""
         success = self.wait_until(lambda obj: self.__class__(obj.model).is_ready(), timelimit=timeout)
         assert success, f"Gateway didn't reach required state, instead it was: {self.model.status.conditions}"
         if settings["control_plane"]["slow_loadbalancers"]:
-            sleep(60)
+            sleep(SLOW_LOADBALANCER_WAIT)
 
     def is_affected_by(self, policy: Policy) -> bool:
         """Returns True, if affected by status is found within the object for the specific policy"""

testsuite/gateway/gateway_api/grpc_route.py

@@ -7,6 +7,7 @@
 from testsuite.kubernetes import KubernetesObject, modify
 from testsuite.kuadrant.policy import Policy
 from testsuite.utils import asdict, check_condition
+from testsuite.utils.constants import ROUTE_READY_TIMEOUT
 
 if typing.TYPE_CHECKING:
     from testsuite.backend import Backend
@@ -125,5 +126,5 @@ def _ready(obj):
                     return all(x.status == "True" for x in condition_set.conditions)
             return False
 
-        success = self.wait_until(_ready, timelimit=10)
+        success = self.wait_until(_ready, timelimit=ROUTE_READY_TIMEOUT)
         assert success, f"{self.kind()} did not get ready in time"

testsuite/gateway/gateway_api/route.py

@@ -8,6 +8,7 @@
 from testsuite.gateway import Gateway, GatewayRoute, PathMatch, MatchType, RouteMatch, URLRewriteFilter
 from testsuite.kubernetes.client import KubernetesClient
 from testsuite.kubernetes import KubernetesObject, modify
+from testsuite.utils.constants import ROUTE_READY_TIMEOUT
 from testsuite.kuadrant.policy import Policy
 from testsuite.utils import asdict, check_condition
 
@@ -134,5 +135,5 @@ def _ready(obj):
                     return (all(x.status == "True" for x in condition_set.conditions),)
             return False
 
-        success = self.wait_until(_ready, timelimit=10)
+        success = self.wait_until(_ready, timelimit=ROUTE_READY_TIMEOUT)
         assert success, f"{self.kind()} did not get ready in time"

testsuite/grpc/__init__.py

@@ -7,6 +7,7 @@
 
 from testsuite.httpx import ResultList
 from testsuite.backend.grpc import grpcbin_pb2
+from testsuite.utils.constants import GRPC_CALL_TIMEOUT
 
 SERVICE_DESCRIPTOR = grpcbin_pb2.DESCRIPTOR.services_by_name["GRPCBin"]
 
@@ -59,7 +60,7 @@ def call(self, method, *, service="/grpcbin.GRPCBin", auth=None, headers=None):
             response_deserializer=GetMessageClass(SERVICE_DESCRIPTOR.methods_by_name[method].output_type).FromString,
         )
         try:
-            response = make_call(grpcbin_pb2.EmptyMessage(), metadata=metadata or None, timeout=10)
+            response = make_call(grpcbin_pb2.EmptyMessage(), metadata=metadata or None, timeout=GRPC_CALL_TIMEOUT)
             return GRPCResult(StatusCode.OK, response=response)
         except grpc.RpcError as e:
             return GRPCResult(e.code(), error=e)  # pylint: disable=no-member

testsuite/httpx/__init__.py

@@ -23,6 +23,7 @@
 )
 
 from testsuite.certificates import Certificate
+from testsuite.utils.constants import HTTP_BACKOFF_MAX_RETRIES
 
 
 def create_tmp_file(content: str):
@@ -151,7 +152,9 @@ def add_retry_code(self, code):
         self.retry_codes.add(code)
 
     # pylint: disable=too-many-locals
-    @backoff.on_predicate(backoff.fibo, lambda result: result.should_backoff(), max_tries=8, jitter=None)
+    @backoff.on_predicate(
+        backoff.fibo, lambda result: result.should_backoff(), max_tries=HTTP_BACKOFF_MAX_RETRIES, jitter=None
+    )
     def request(
         self,
         method: str,

testsuite/kuadrant/extensions/oidc_policy.py

@@ -7,6 +7,7 @@
 from testsuite.kubernetes.client import KubernetesClient
 from testsuite.kuadrant.policy import Policy
 from testsuite.utils import asdict
+from testsuite.utils.constants import OIDC_POST_ENFORCEMENT_WAIT
 
 
 @dataclass
@@ -63,4 +64,4 @@ def wait_for_ready(self):
         """Wait for OIDCPolicy to be enforced"""
         super().wait_for_ready()
         # Even after enforced condition OIDCPolicy requires a short sleep
-        time.sleep(10)  # https://github.com/Kuadrant/testsuite/issues/884
+        time.sleep(OIDC_POST_ENFORCEMENT_WAIT)  # Workaround for issue #884

testsuite/kuadrant/policy/__init__.py

@@ -5,6 +5,7 @@
 
 from testsuite.kubernetes import KubernetesObject
 from testsuite.utils import check_condition
+from testsuite.utils.constants import POLICY_ENFORCEMENT_TIMEOUT
 
 
 class Strategy(Enum):
@@ -90,7 +91,7 @@ def wait_for_partial_enforced(self):
         )
         assert success, f"{self.kind(False)} did not get partially enforced in time"
 
-    def wait_for_full_enforced(self, timelimit=60):
+    def wait_for_full_enforced(self, timelimit=POLICY_ENFORCEMENT_TIMEOUT):
         """Wait for a Policy to be fully Enforced"""
         success = self.wait_until(
             has_condition("Enforced", "True", "Enforced", f"{self.kind(False)} has been successfully enforced"),

testsuite/kuadrant/policy/dns.py

@@ -11,6 +11,7 @@
 from testsuite.kubernetes.client import KubernetesClient
 from testsuite.kuadrant.policy import Policy
 from testsuite.utils import asdict, check_condition
+from testsuite.utils.constants import DNS_POLICY_ENFORCEMENT_TIMEOUT
 
 
 def has_record_condition(condition_type, status="True", reason=None, message=None):
@@ -199,6 +200,6 @@ def get_dns_health_probe(self) -> DNSHealthCheckProbe:
             dns_probe = dns_record.get_owned("DNSHealthCheckProbe")[0]
         return DNSHealthCheckProbe(dns_probe.model, context=self.context)
 
-    def wait_for_full_enforced(self, timelimit=300):
+    def wait_for_full_enforced(self, timelimit=DNS_POLICY_ENFORCEMENT_TIMEOUT):
         """Wait for a Policy to be fully Enforced with increased timelimit for DNSPolicy"""
         super().wait_for_full_enforced(timelimit=timelimit)

testsuite/kuadrant/policy/rate_limit.py

@@ -9,6 +9,7 @@
 from testsuite.kubernetes.client import KubernetesClient
 from testsuite.kuadrant.policy import Policy, CelPredicate, CelExpression, Strategy
 from testsuite.utils import asdict
+from testsuite.utils.constants import RLP_POST_ENFORCEMENT_WAIT
 
 
 @dataclass
@@ -114,4 +115,4 @@ def wait_for_ready(self):
         """Wait for RLP to be enforced"""
         super().wait_for_ready()
         # Even after enforced condition RLP requires a short sleep
-        time.sleep(5)
+        time.sleep(RLP_POST_ENFORCEMENT_WAIT)