From 0285522c02dac9752b80428551329bfa6996a74e Mon Sep 17 00:00:00 2001 From: phala Date: Thu, 4 Jan 2024 10:58:45 +0100 Subject: [PATCH 1/4] Move Selector and MatchExpression into testsuite/openshift * It is Kubernetes related, not only for policies --- testsuite/openshift/__init__.py | 27 +++++++++++++++++++ testsuite/openshift/api_key.py | 3 +-- testsuite/policy/authorization/__init__.py | 24 +---------------- testsuite/policy/authorization/sections.py | 3 +-- .../identity/api_key/test_match_expression.py | 2 +- .../identity/api_key/test_reconciliation.py | 2 +- .../authorino/operator/tls/conftest.py | 2 +- 7 files changed, 33 insertions(+), 30 deletions(-) diff --git a/testsuite/openshift/__init__.py b/testsuite/openshift/__init__.py index 84499a57..24ca482c 100644 --- a/testsuite/openshift/__init__.py +++ b/testsuite/openshift/__init__.py @@ -1,5 +1,7 @@ """OpenShift common objects""" import functools +from dataclasses import dataclass, field +from typing import Optional, Literal from openshift import APIObject, timeout @@ -52,3 +54,28 @@ def _wrap(self, *args, **kwargs): func(self, *args, **kwargs) return _wrap + + +@dataclass +class MatchExpression: + """ + Data class intended for defining K8 Label Selector expressions. + Used by selector.matchExpressions API key identity. + """ + + operator: Literal["In", "NotIn", "Exists", "DoesNotExist"] + values: list[str] + key: str = "group" + + +@dataclass +class Selector: + """Dataclass for specifying selectors based on either expression or labels""" + + # pylint: disable=invalid-name + matchExpressions: Optional[list[MatchExpression]] = field(default=None, kw_only=True) + matchLabels: Optional[dict[str, str]] = field(default=None, kw_only=True) + + def __post_init__(self): + if not (self.matchLabels is None) ^ (self.matchExpressions is None): + raise AttributeError("`matchLabels` xor `matchExpressions` argument must be used") diff --git a/testsuite/openshift/api_key.py b/testsuite/openshift/api_key.py index ed945093..00e2400a 100644 --- a/testsuite/openshift/api_key.py +++ b/testsuite/openshift/api_key.py @@ -2,9 +2,8 @@ import base64 from functools import cached_property -from testsuite.policy.authorization import Selector from testsuite.openshift.client import OpenShiftClient -from testsuite.openshift import OpenShiftObject, modify +from testsuite.openshift import OpenShiftObject, modify, Selector class APIKey(OpenShiftObject): diff --git a/testsuite/policy/authorization/__init__.py b/testsuite/policy/authorization/__init__.py index 35ebe778..ca44eed1 100644 --- a/testsuite/policy/authorization/__init__.py +++ b/testsuite/policy/authorization/__init__.py @@ -1,33 +1,11 @@ """Authorization related objects""" import abc -from dataclasses import dataclass, field +from dataclasses import dataclass from typing import Literal, Optional from testsuite.utils import asdict, JSONValues # pylint: disable=invalid-name -@dataclass -class MatchExpression: - """ - Data class intended for defining K8 Label Selector expressions. - Used by selector.matchExpressions API key identity. - """ - - operator: Literal["In", "NotIn", "Exists", "DoesNotExist"] - values: list[str] - key: str = "group" - - -@dataclass -class Selector: - """Dataclass for specifying selectors based on either expression or labels""" - - matchExpressions: Optional[list[MatchExpression]] = field(default=None, kw_only=True) - matchLabels: Optional[dict[str, str]] = field(default=None, kw_only=True) - - def __post_init__(self): - if not (self.matchLabels is None) ^ (self.matchExpressions is None): - raise AttributeError("`matchLabels` xor `matchExpressions` argument must be used") @dataclass diff --git a/testsuite/policy/authorization/sections.py b/testsuite/policy/authorization/sections.py index f23792a3..c7d38d27 100644 --- a/testsuite/policy/authorization/sections.py +++ b/testsuite/policy/authorization/sections.py @@ -2,7 +2,6 @@ from typing import Literal, Iterable, TYPE_CHECKING, Union from testsuite.policy.authorization import ( - Selector, Credentials, Rule, Pattern, @@ -15,7 +14,7 @@ Cache, ) from testsuite.utils import asdict -from testsuite.openshift import modify +from testsuite.openshift import modify, Selector if TYPE_CHECKING: from .auth_config import AuthConfig diff --git a/testsuite/tests/kuadrant/authorino/identity/api_key/test_match_expression.py b/testsuite/tests/kuadrant/authorino/identity/api_key/test_match_expression.py index 2917c683..8c07be87 100644 --- a/testsuite/tests/kuadrant/authorino/identity/api_key/test_match_expression.py +++ b/testsuite/tests/kuadrant/authorino/identity/api_key/test_match_expression.py @@ -5,7 +5,7 @@ """ import pytest -from testsuite.policy.authorization import MatchExpression, Selector +from testsuite.openshift import Selector, MatchExpression @pytest.fixture(scope="module") diff --git a/testsuite/tests/kuadrant/authorino/identity/api_key/test_reconciliation.py b/testsuite/tests/kuadrant/authorino/identity/api_key/test_reconciliation.py index 05afadce..9da52c49 100644 --- a/testsuite/tests/kuadrant/authorino/identity/api_key/test_reconciliation.py +++ b/testsuite/tests/kuadrant/authorino/identity/api_key/test_reconciliation.py @@ -2,7 +2,7 @@ import pytest from testsuite.httpx.auth import HeaderApiKeyAuth -from testsuite.policy.authorization import Selector +from testsuite.openshift import Selector @pytest.fixture(scope="function") diff --git a/testsuite/tests/kuadrant/authorino/operator/tls/conftest.py b/testsuite/tests/kuadrant/authorino/operator/tls/conftest.py index cc299c70..96928a38 100644 --- a/testsuite/tests/kuadrant/authorino/operator/tls/conftest.py +++ b/testsuite/tests/kuadrant/authorino/operator/tls/conftest.py @@ -4,7 +4,7 @@ import pytest from testsuite.certificates import Certificate, CertInfo -from testsuite.policy.authorization import Selector +from testsuite.openshift import Selector from testsuite.gateway import Exposer from testsuite.gateway.envoy.tls import TLSEnvoy from testsuite.gateway.gateway_api.hostname import OpenShiftExposer From 247ebf3901caa66e95a23c0f0166d0946a01f152 Mon Sep 17 00:00:00 2001 From: phala Date: Thu, 4 Jan 2024 10:59:06 +0100 Subject: [PATCH 2/4] Add Deployment and Service objects --- testsuite/openshift/deployment.py | 42 +++++++++++++++++++++++++++++++ testsuite/openshift/service.py | 37 +++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 testsuite/openshift/deployment.py create mode 100644 testsuite/openshift/service.py diff --git a/testsuite/openshift/deployment.py b/testsuite/openshift/deployment.py new file mode 100644 index 00000000..72b5908e --- /dev/null +++ b/testsuite/openshift/deployment.py @@ -0,0 +1,42 @@ +"""Deployment related objects""" +from testsuite.openshift import OpenShiftObject, Selector +from testsuite.utils import asdict + + +class Deployment(OpenShiftObject): + """Kubernetes Deployment object""" + + @classmethod + def create_instance( + cls, openshift, name, container_name, image, ports: dict[str, int], selector: Selector, labels: dict[str, str] + ): + """ + Creates new instance of Deployment + Supports only single container Deployments everything else should be edited directly + """ + model: dict = { + "kind": "Deployment", + "apiVersion": "apps/v1", + "metadata": { + "name": name, + "labels": labels, + }, + "spec": { + "selector": asdict(selector), + "template": { + "metadata": {"labels": {"deployment": name, **labels}}, + "spec": { + "containers": [ + { + "image": image, + "name": container_name, + "imagePullPolicy": "IfNotPresent", + "ports": [{"name": name, "containerPort": port} for name, port in ports.items()], + } + ] + }, + }, + }, + } + + return cls(model, context=openshift.context) diff --git a/testsuite/openshift/service.py b/testsuite/openshift/service.py new file mode 100644 index 00000000..25a92c57 --- /dev/null +++ b/testsuite/openshift/service.py @@ -0,0 +1,37 @@ +"""Service related objects""" +from testsuite.openshift import OpenShiftObject + + +class Service(OpenShiftObject): + """Kubernetes Service object""" + + @classmethod + def create_instance( + cls, + openshift, + name, + selector: dict[str, str], + ports: list[dict[str, str]], + labels: dict[str, str] = None, + ): + """Creates new Service""" + model: dict = { + "kind": "Service", + "apiVersion": "v1", + "metadata": { + "name": name, + "labels": labels, + }, + "spec": { + "ports": ports, + "selector": selector, + }, + } + return cls(model, context=openshift.context) + + def get_port(self, name): + """Returns port definition for a port with said name""" + for port in self.model.spec.ports: + if port["name"] == name: + return port + raise KeyError(f"No port with name {name} exists") From 393ca26beca0f7531a86823713588a377e559724 Mon Sep 17 00:00:00 2001 From: phala Date: Thu, 4 Jan 2024 11:00:14 +0100 Subject: [PATCH 3/4] Use objects instead of template for Httpbin --- testsuite/openshift/httpbin.py | 51 +++++++++++++++++++----------- testsuite/openshift/service.py | 15 +++++++-- testsuite/resources/httpbin.yaml | 53 -------------------------------- 3 files changed, 46 insertions(+), 73 deletions(-) delete mode 100644 testsuite/resources/httpbin.yaml diff --git a/testsuite/openshift/httpbin.py b/testsuite/openshift/httpbin.py index 43024dd7..3c4265cd 100644 --- a/testsuite/openshift/httpbin.py +++ b/testsuite/openshift/httpbin.py @@ -1,14 +1,16 @@ -"""Module for Httpbin backend classes""" +"""Httpbin related classes""" from functools import cached_property -from importlib import resources from testsuite.lifecycle import LifecycleObject from testsuite.gateway import Referencable +from testsuite.openshift import Selector from testsuite.openshift.client import OpenShiftClient +from testsuite.openshift.deployment import Deployment +from testsuite.openshift.service import Service, ServicePort class Httpbin(LifecycleObject, Referencable): - """Httpbin deployed in OpenShift through template""" + """Httpbin deployed in OpenShift""" def __init__(self, openshift: OpenShiftClient, name, label, replicas=1) -> None: super().__init__() @@ -17,7 +19,8 @@ def __init__(self, openshift: OpenShiftClient, name, label, replicas=1) -> None: self.label = label self.replicas = replicas - self.httpbin_objects = None + self.deployment = None + self.service = None @property def reference(self): @@ -29,27 +32,39 @@ def url(self): return f"{self.name}.{self.openshift.project}.svc.cluster.local" def commit(self): - self.httpbin_objects = self.openshift.new_app( - resources.files("testsuite.resources").joinpath("httpbin.yaml"), - {"NAME": self.name, "LABEL": self.label, "REPLICAS": self.replicas}, + match_labels = {"app": self.label, "deployment": self.name} + self.deployment = Deployment.create_instance( + self.openshift, + self.name, + container_name="httpbin", + image="quay.io/jsmadis/httpbin:latest", + ports={"api": 8080}, + selector=Selector(matchLabels=match_labels), + labels={"app": self.label}, ) + self.deployment.commit() - with self.openshift.context: - assert self.openshift.is_ready(self.httpbin_objects.narrow("deployment")), "Httpbin wasn't ready in time" + self.service = Service.create_instance( + self.openshift, + self.name, + selector=match_labels, + ports=[ServicePort(name="http", port=8080, targetPort="api")], + ) + self.service.commit() - def delete(self): with self.openshift.context: - if self.httpbin_objects: - self.httpbin_objects.delete() - self.httpbin_objects = None + assert self.openshift.is_ready(self.deployment.self_selector()), "Httpbin wasn't ready in time" - @cached_property - def service(self): - """Service associated with httpbin""" + def delete(self): with self.openshift.context: - return self.httpbin_objects.narrow("service").object() + if self.service: + self.service.delete() + self.service = None + if self.deployment: + self.deployment.delete() + self.deployment = None @cached_property def port(self): """Service port that httpbin listens on""" - return self.service.model.spec.ports[0].get("port") + return self.service.get_port("http").port diff --git a/testsuite/openshift/service.py b/testsuite/openshift/service.py index 25a92c57..6188eb85 100644 --- a/testsuite/openshift/service.py +++ b/testsuite/openshift/service.py @@ -1,7 +1,18 @@ """Service related objects""" +from dataclasses import dataclass, asdict + from testsuite.openshift import OpenShiftObject +@dataclass +class ServicePort: + """Kubernetes Service Port object""" + + name: str + port: int + targetPort: int | str # pylint: disable=invalid-name + + class Service(OpenShiftObject): """Kubernetes Service object""" @@ -11,7 +22,7 @@ def create_instance( openshift, name, selector: dict[str, str], - ports: list[dict[str, str]], + ports: list[ServicePort], labels: dict[str, str] = None, ): """Creates new Service""" @@ -23,7 +34,7 @@ def create_instance( "labels": labels, }, "spec": { - "ports": ports, + "ports": [asdict(port) for port in ports], "selector": selector, }, } diff --git a/testsuite/resources/httpbin.yaml b/testsuite/resources/httpbin.yaml deleted file mode 100644 index ed519c04..00000000 --- a/testsuite/resources/httpbin.yaml +++ /dev/null @@ -1,53 +0,0 @@ -apiVersion: template.openshift.io/v1 -kind: Template -metadata: - name: httpbin -objects: -- apiVersion: apps/v1 - kind: Deployment - metadata: - name: ${NAME} - labels: - app: ${LABEL} - spec: - replicas: ${{REPLICAS}} - selector: - matchLabels: - app: ${LABEL} - template: - metadata: - labels: - app: ${LABEL} - deployment: ${NAME} - spec: - containers: - - image: quay.io/jsmadis/httpbin:latest - imagePullPolicy: IfNotPresent - name: httpbin - ports: - - containerPort: 8080 -- apiVersion: v1 - kind: Service - metadata: - name: ${NAME} - labels: - app: ${LABEL} - spec: - ports: - - name: http - port: 8080 - targetPort: 8080 - selector: - app: ${LABEL} - deployment: ${NAME} -parameters: -- name: NAME - description: "Httpbin's name" - required: true -- name: LABEL - description: "App label for all resources" - required: true -- name: REPLICAS - description: "Number of httpbin replicas" - required: true - value: "1" From 73d2e2d9abf0139f3dc95390c1f7ade3f48f6493 Mon Sep 17 00:00:00 2001 From: phala Date: Tue, 9 Jan 2024 12:20:18 +0100 Subject: [PATCH 4/4] Add wait_for_ready to Deployment --- testsuite/openshift/deployment.py | 8 ++++++++ testsuite/openshift/httpbin.py | 4 +--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/testsuite/openshift/deployment.py b/testsuite/openshift/deployment.py index 72b5908e..9fd2ef54 100644 --- a/testsuite/openshift/deployment.py +++ b/testsuite/openshift/deployment.py @@ -1,4 +1,6 @@ """Deployment related objects""" +import openshift as oc + from testsuite.openshift import OpenShiftObject, Selector from testsuite.utils import asdict @@ -40,3 +42,9 @@ def create_instance( } return cls(model, context=openshift.context) + + def wait_for_ready(self, timeout=90): + """Waits until Deployment is marked as ready""" + with oc.timeout(timeout): + success, _, _ = self.self_selector().until_all(success_func=lambda obj: "readyReplicas" in obj.model.status) + assert success, f"Deployment {self.name()} did not get ready in time" diff --git a/testsuite/openshift/httpbin.py b/testsuite/openshift/httpbin.py index 3c4265cd..92910133 100644 --- a/testsuite/openshift/httpbin.py +++ b/testsuite/openshift/httpbin.py @@ -43,6 +43,7 @@ def commit(self): labels={"app": self.label}, ) self.deployment.commit() + self.deployment.wait_for_ready() self.service = Service.create_instance( self.openshift, @@ -52,9 +53,6 @@ def commit(self): ) self.service.commit() - with self.openshift.context: - assert self.openshift.is_ready(self.deployment.self_selector()), "Httpbin wasn't ready in time" - def delete(self): with self.openshift.context: if self.service: