From 44db6b7f4f37b56c5709da3a9b171cc075a0875b Mon Sep 17 00:00:00 2001 From: Alex Zgabur Date: Thu, 12 Sep 2024 12:21:06 +0200 Subject: [PATCH] Add gateway listener editing support Signed-off-by: Alex Zgabur --- testsuite/gateway/__init__.py | 40 ++++++++++ testsuite/gateway/gateway_api/gateway.py | 54 ++++++------- testsuite/tests/multicluster/conftest.py | 11 ++- testsuite/tests/singlecluster/conftest.py | 4 +- .../tests/singlecluster/gateway/conftest.py | 10 ++- .../dnspolicy/test_invalid_credentials.py | 4 +- .../test_gateway_listeners_dns.py | 76 ------------------- 7 files changed, 84 insertions(+), 115 deletions(-) delete mode 100644 testsuite/tests/singlecluster/gateway/reconciliation/test_gateway_listeners_dns.py diff --git a/testsuite/gateway/__init__.py b/testsuite/gateway/__init__.py index 452f803a..5901758e 100644 --- a/testsuite/gateway/__init__.py +++ b/testsuite/gateway/__init__.py @@ -177,6 +177,46 @@ def remove_all_backend(self): """Sets match for a specific backend""" +@dataclass +class GatewayListener: + """ + Dataclass of Gateway listener object + If used in Gateway creation `.create_instance()` function the `name` parameter is mandatory. + If used in `.add_listener()` function, you can omit the `name` parameter and a unique parameter will be generated. + """ + + hostname: str + name: Optional[str] = None + port: int = 80 + protocol: str = "HTTP" + allowedRoutes = {"namespaces": {"from": "All"}} + + +@dataclass(kw_only=True) +class GatewayListenerTls(GatewayListener): + """ + Dataclass for Gateway listener supporting TLS. + If used in Gateway creation `.create_instance()` function the `name` parameter is mandatory. + If used in `.add_listener()` function, you can omit the `name` parameter and a unique parameter will be generated. + """ + + gateway_name: str + mode: str = "Terminate" + port: int = 443 + protocol: str = "HTTPS" + + def asdict(self): + """Custom asdict to easily add tls certificateRefs""" + return { + "name": self.name, + "hostname": self.hostname, + "port": self.port, + "protocol": self.protocol, + "allowedRoutes": self.allowedRoutes, + "tls": {"mode": self.mode, "certificateRefs": [{"name": f"{self.gateway_name}-tls", "kind": "Secret"}]}, + } + + class Hostname(ABC): """ Abstraction layer on top of externally exposed hostname diff --git a/testsuite/gateway/gateway_api/gateway.py b/testsuite/gateway/gateway_api/gateway.py index 9419f792..2e77591c 100644 --- a/testsuite/gateway/gateway_api/gateway.py +++ b/testsuite/gateway/gateway_api/gateway.py @@ -5,55 +5,47 @@ import openshift_client as oc from testsuite.certificates import Certificate -from testsuite.gateway import Gateway +from testsuite.gateway import Gateway, GatewayListener from testsuite.kubernetes.client import KubernetesClient -from testsuite.kubernetes import KubernetesObject +from testsuite.kubernetes import KubernetesObject, modify from testsuite.kuadrant.policy import Policy -from testsuite.utils import check_condition +from testsuite.utils import check_condition, asdict class KuadrantGateway(KubernetesObject, Gateway): """Gateway object for Kuadrant""" + # Used to make unique listener names + listener_counter = 1 + @classmethod - def create_instance(cls, cluster: KubernetesClient, name, hostname, labels, tls=False): + def create_instance(cls, cluster: KubernetesClient, name, listener: GatewayListener, labels): """Creates new instance of Gateway""" model: dict[Any, Any] = { "apiVersion": "gateway.networking.k8s.io/v1beta1", "kind": "Gateway", "metadata": {"name": name, "labels": labels}, - "spec": { - "gatewayClassName": "istio", - "listeners": [ - { - "name": "api", - "port": 80, - "protocol": "HTTP", - "hostname": hostname, - "allowedRoutes": {"namespaces": {"from": "All"}}, - } - ], - }, + "spec": {"gatewayClassName": "istio", "listeners": [asdict(listener)]}, } - if tls: - model["spec"]["listeners"] = [ - { - "name": "api", - "port": 443, - "protocol": "HTTPS", - "hostname": hostname, - "allowedRoutes": {"namespaces": {"from": "All"}}, - "tls": { - "mode": "Terminate", - "certificateRefs": [{"name": f"{name}-tls", "kind": "Secret"}], - }, - } - ] - return cls(model, context=cluster.context) + @modify + def add_listener(self, listener: GatewayListener): + """Adds a listener to Gateway. If the argument doesn't contain name, generate new sequential one.""" + if listener.name is None: + listener.name = f"api{self.listener_counter}" + self.listener_counter += 1 + self.model.spec.listeners.append(asdict(listener)) + + @modify + def remove_listener(self, hostname: GatewayListener | str): + """Removes a listener based on hostname""" + if isinstance(hostname, GatewayListener): + hostname = hostname.hostname + self.model.spec.listeners = list(filter(lambda i: i["hostname"] != hostname, self.model.spec.listeners)) + @property def service_name(self) -> str: return f"{self.name()}-istio" diff --git a/testsuite/tests/multicluster/conftest.py b/testsuite/tests/multicluster/conftest.py index 0df5fe24..a26bf623 100644 --- a/testsuite/tests/multicluster/conftest.py +++ b/testsuite/tests/multicluster/conftest.py @@ -8,6 +8,7 @@ from testsuite.backend.httpbin import Httpbin from testsuite.certificates import Certificate from testsuite.gateway import Exposer, CustomReference, Hostname +from testsuite.gateway import GatewayListenerTls from testsuite.gateway.gateway_api.gateway import KuadrantGateway from testsuite.gateway.gateway_api.hostname import DNSPolicyExposer from testsuite.gateway.gateway_api.route import HTTPRoute @@ -106,7 +107,10 @@ def routes(request, gateway, gateway2, blame, hostname, backends, module_label) @pytest.fixture(scope="module") def gateway(request, cluster, blame, label, wildcard_domain): """Deploys Gateway to first Kubernetes cluster""" - gw = KuadrantGateway.create_instance(cluster, blame("gw"), wildcard_domain, {"app": label}, tls=True) + name = blame("gw") + gw = KuadrantGateway.create_instance( + cluster, name, GatewayListenerTls(hostname=wildcard_domain, gateway_name=name, name="api"), {"app": label} + ) request.addfinalizer(gw.delete) gw.commit() gw.wait_for_ready() @@ -116,7 +120,10 @@ def gateway(request, cluster, blame, label, wildcard_domain): @pytest.fixture(scope="module") def gateway2(request, cluster2, blame, label, wildcard_domain): """Deploys Gateway to second Kubernetes cluster""" - gw = KuadrantGateway.create_instance(cluster2, blame("gw"), wildcard_domain, {"app": label}, tls=True) + name = blame("gw") + gw = KuadrantGateway.create_instance( + cluster2, name, GatewayListenerTls(hostname=wildcard_domain, gateway_name=name, name="api"), {"app": label} + ) request.addfinalizer(gw.delete) gw.commit() gw.wait_for_ready() diff --git a/testsuite/tests/singlecluster/conftest.py b/testsuite/tests/singlecluster/conftest.py index 3c57dcaa..eaedebd4 100644 --- a/testsuite/tests/singlecluster/conftest.py +++ b/testsuite/tests/singlecluster/conftest.py @@ -6,7 +6,7 @@ from openshift_client import selector from testsuite.backend.httpbin import Httpbin -from testsuite.gateway import GatewayRoute, Gateway, Hostname +from testsuite.gateway import GatewayRoute, Gateway, Hostname, GatewayListener from testsuite.gateway.envoy import Envoy from testsuite.gateway.envoy.route import EnvoyVirtualRoute from testsuite.gateway.gateway_api.gateway import KuadrantGateway @@ -130,7 +130,7 @@ def backend(request, cluster, blame, label, testconfig): def gateway(request, kuadrant, cluster, blame, label, testconfig, wildcard_domain) -> Gateway: """Deploys Gateway that wires up the Backend behind the reverse-proxy and Authorino instance""" if kuadrant: - gw = KuadrantGateway.create_instance(cluster, blame("gw"), wildcard_domain, {"app": label}) + gw = KuadrantGateway.create_instance(cluster, blame("gw"), GatewayListener(wildcard_domain), {"app": label}) else: authorino = request.getfixturevalue("authorino") gw = Envoy( diff --git a/testsuite/tests/singlecluster/gateway/conftest.py b/testsuite/tests/singlecluster/gateway/conftest.py index f79d8f73..bf872bc8 100644 --- a/testsuite/tests/singlecluster/gateway/conftest.py +++ b/testsuite/tests/singlecluster/gateway/conftest.py @@ -2,7 +2,7 @@ import pytest -from testsuite.gateway import Exposer +from testsuite.gateway import Exposer, GatewayListenerTls from testsuite.gateway.gateway_api.gateway import KuadrantGateway from testsuite.gateway.gateway_api.hostname import DNSPolicyExposer from testsuite.httpx.auth import HttpxOidcClientAuth @@ -13,7 +13,13 @@ @pytest.fixture(scope="module") def gateway(request, cluster, blame, wildcard_domain, module_label): """Returns ready gateway""" - gw = KuadrantGateway.create_instance(cluster, blame("gw"), wildcard_domain, {"app": module_label}, tls=True) + gateway_name = blame("gw") + gw = KuadrantGateway.create_instance( + cluster, + gateway_name, + GatewayListenerTls(hostname=wildcard_domain, gateway_name=gateway_name, name="api"), + {"app": module_label}, + ) request.addfinalizer(gw.delete) gw.commit() gw.wait_for_ready() diff --git a/testsuite/tests/singlecluster/gateway/dnspolicy/test_invalid_credentials.py b/testsuite/tests/singlecluster/gateway/dnspolicy/test_invalid_credentials.py index 146cccb9..c05a2d18 100644 --- a/testsuite/tests/singlecluster/gateway/dnspolicy/test_invalid_credentials.py +++ b/testsuite/tests/singlecluster/gateway/dnspolicy/test_invalid_credentials.py @@ -5,7 +5,7 @@ from testsuite.kubernetes.secret import Secret from testsuite.kuadrant.policy import has_condition from testsuite.kuadrant.policy.dns import has_record_condition -from testsuite.gateway.gateway_api.gateway import KuadrantGateway +from testsuite.gateway.gateway_api.gateway import KuadrantGateway, GatewayListener pytestmark = [pytest.mark.kuadrant_only, pytest.mark.dnspolicy] @@ -13,7 +13,7 @@ @pytest.fixture(scope="module") def gateway(request, cluster, blame, wildcard_domain, module_label): """Create gateway without TLS enabled""" - gw = KuadrantGateway.create_instance(cluster, blame("gw"), wildcard_domain, {"app": module_label}, tls=False) + gw = KuadrantGateway.create_instance(cluster, blame("gw"), GatewayListener(wildcard_domain, name="api"), {"app": module_label}) request.addfinalizer(gw.delete) gw.commit() gw.wait_for_ready() diff --git a/testsuite/tests/singlecluster/gateway/reconciliation/test_gateway_listeners_dns.py b/testsuite/tests/singlecluster/gateway/reconciliation/test_gateway_listeners_dns.py deleted file mode 100644 index 87bb749c..00000000 --- a/testsuite/tests/singlecluster/gateway/reconciliation/test_gateway_listeners_dns.py +++ /dev/null @@ -1,76 +0,0 @@ -"""Testing specific bug that happens when listener hostname in Gateway gets changed while DNSPolicy is applied.""" - -import pytest -from testsuite.gateway.gateway_api.hostname import StaticHostname -from testsuite.utils import is_nxdomain, sleep_ttl - -pytestmark = [pytest.mark.kuadrant_only, pytest.mark.dnspolicy, pytest.mark.tlspolicy] - - -@pytest.fixture(scope="module") -def wildcard_domain(base_domain, blame): - """ - For this test we want specific domain, not wildcard. - This will be used in the first iteration of Gateway and HTTPRoute. - """ - return f"{blame('dnsbug1')}.{base_domain}" - - -@pytest.fixture(scope="module") -def new_domain(base_domain, blame): - """In the test the Gateway and HTTPRoute will change their hostnames to this one.""" - return f"{blame('dnsbug2')}.{base_domain}" - - -@pytest.fixture(scope="module") -def client(gateway, wildcard_domain): - """Make client point to correct domain route domain.""" - return StaticHostname(wildcard_domain, gateway.get_tls_cert).client() - - -@pytest.fixture(scope="module") -def client_new(gateway, new_domain): - """Second client that will be created after Gateway gets updated pointing to new_domain.""" - - def _client_new(): - return StaticHostname(new_domain, gateway.get_tls_cert).client() - - return _client_new - - -@pytest.fixture(scope="module") -def route(route, wildcard_domain): - """So that route hostname matches the gateway hostname.""" - route.remove_all_hostnames() - route.add_hostname(wildcard_domain) - return route - - -@pytest.mark.issue("https://github.com/Kuadrant/kuadrant-operator/issues/794") -def test_change_hostname(client, client_new, auth, gateway, route, new_domain, wildcard_domain): - """ - This test checks if after change of listener hostname in a Gateway while having DNSPolicy applied, that - the old hostname gets deleted from DNS provider. After editing the hostname in HTTPRoute to the new value - this test checks the reconciliation of such procedure. - - WARNING - Running this test in unpatched Kuadrant will leave orphaned DNS records in DNS provider. - If you want to delete them you need to do it manually. The DNS records will contain string 'dnsbug' - """ - result = client.get("/get", auth=auth) - assert not result.has_dns_error() - assert not result.has_cert_verify_error() - assert result.status_code == 200 - - gateway.refresh().model.spec.listeners[0].hostname = new_domain - gateway.apply() - route.refresh().model.spec.hostnames[0] = new_domain - route.apply() - - result = client_new().get("/get", auth=auth) - assert not result.has_dns_error() - assert not result.has_cert_verify_error() - assert result.status_code == 200 - - sleep_ttl(wildcard_domain) - assert is_nxdomain(wildcard_domain)