From b63ad9c9339f7c97cef9bd19982f5b4aff18dfc9 Mon Sep 17 00:00:00 2001 From: Giovanni Grano Date: Tue, 10 Sep 2024 15:31:23 -0400 Subject: [PATCH 1/6] Improvements to load oas specs - use fixed files in importlib_resources - add a utility function we can patch in ext --- .../localstack/aws/handlers/validation.py | 33 +++++++++++-------- localstack-core/localstack/plugins.py | 19 +++++++++++ 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/localstack-core/localstack/aws/handlers/validation.py b/localstack-core/localstack/aws/handlers/validation.py index 3a0f55bd880e7..c56c9b07fe083 100644 --- a/localstack-core/localstack/aws/handlers/validation.py +++ b/localstack-core/localstack/aws/handlers/validation.py @@ -4,7 +4,6 @@ import logging import os -from pathlib import Path from openapi_core import OpenAPI from openapi_core.contrib.werkzeug import WerkzeugOpenAPIRequest, WerkzeugOpenAPIResponse @@ -13,28 +12,34 @@ RequestValidationError, ) from openapi_core.validation.response.exceptions import ResponseValidationError +from plux import PluginManager from localstack import config from localstack.aws.api import RequestContext from localstack.aws.chain import Handler, HandlerChain from localstack.constants import INTERNAL_RESOURCE_PATH from localstack.http import Response +from localstack.plugins import OASPlugin LOG = logging.getLogger(__name__) -# TODO: replace with from importlib.resources.files when https://github.com/python/importlib_resources/issues/311 is -# resolved. Import from a namespace package is broken when installing in editable mode. -oas_path = os.path.join(os.path.dirname(__file__), "..", "..", "openapi.yaml") +class CoreOASPlugin(OASPlugin): + name = "localstack.core" + + def __init__(self): + path = os.path.join(os.path.dirname(__file__), "..", "..", "openapi.yaml") + super().__init__(path) class OpenAPIValidator(Handler): - openapi: "OpenAPI" + open_apis: list["OpenAPI"] def __init__(self) -> None: - path = Path(oas_path) - assert path.exists() - self.openapi = OpenAPI.from_path(path) + specs = PluginManager("localstack.openapi.spec").load_all() + self.open_apis = [] + for spec in specs: + self.open_apis.append(OpenAPI.from_path(spec.spec_path)) class OpenAPIRequestValidator(OpenAPIValidator): @@ -51,7 +56,8 @@ def __call__(self, chain: HandlerChain, context: RequestContext, response: Respo if path.startswith(f"{INTERNAL_RESOURCE_PATH}/") or path.startswith("/_aws/"): try: - self.openapi.validate_request(WerkzeugOpenAPIRequest(context.request)) + for openapi in self.open_apis: + openapi.validate_request(WerkzeugOpenAPIRequest(context.request)) except RequestValidationError as e: # Note: in this handler we only check validation errors, e.g., wrong body, missing required in the body. response.status_code = 400 @@ -77,10 +83,11 @@ def __call__(self, chain: HandlerChain, context: RequestContext, response: Respo if path.startswith(f"{INTERNAL_RESOURCE_PATH}/") or path.startswith("/_aws/"): try: - self.openapi.validate_response( - WerkzeugOpenAPIRequest(context.request), - WerkzeugOpenAPIResponse(response), - ) + for openapi in self.open_apis: + openapi.validate_response( + WerkzeugOpenAPIRequest(context.request), + WerkzeugOpenAPIResponse(response), + ) except ResponseValidationError as exc: LOG.error("Response validation failed for %s: $s", path, exc) response.status_code = 500 diff --git a/localstack-core/localstack/plugins.py b/localstack-core/localstack/plugins.py index 3e6ed1bf14764..72bea7a02b5ea 100644 --- a/localstack-core/localstack/plugins.py +++ b/localstack-core/localstack/plugins.py @@ -1,4 +1,9 @@ import logging +import os +from pathlib import Path + +import yaml +from plux import Plugin from localstack import config from localstack.runtime import hooks @@ -21,3 +26,17 @@ def delete_cached_certificate(): LOG.debug("Removing the cached local SSL certificate") target_file = get_cert_pem_file_path() rm_rf(target_file) + + +class OASPlugin(Plugin): + namespace = "localstack.openapi.spec" + + def __init__(self, spec_path: os.PathLike | str) -> None: + if isinstance(spec_path, str): + spec_path = Path(spec_path) + self.spec_path = spec_path + self.spec = {} + + def load(self): + with self.spec_path.open("r") as f: + self.spec = yaml.safe_load(f) From 934a0f0854953b44d4956a3c64bbb40d01e51e0a Mon Sep 17 00:00:00 2001 From: Giovanni Grano Date: Fri, 13 Sep 2024 11:05:49 -0400 Subject: [PATCH 2/6] minor feedback --- .../localstack/aws/handlers/validation.py | 28 ++++++++++++++----- localstack-core/localstack/plugins.py | 21 ++++---------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/localstack-core/localstack/aws/handlers/validation.py b/localstack-core/localstack/aws/handlers/validation.py index c56c9b07fe083..bba51d7098bef 100644 --- a/localstack-core/localstack/aws/handlers/validation.py +++ b/localstack-core/localstack/aws/handlers/validation.py @@ -4,7 +4,9 @@ import logging import os +from pathlib import Path +import yaml from openapi_core import OpenAPI from openapi_core.contrib.werkzeug import WerkzeugOpenAPIRequest, WerkzeugOpenAPIResponse from openapi_core.exceptions import OpenAPIError @@ -12,24 +14,36 @@ RequestValidationError, ) from openapi_core.validation.response.exceptions import ResponseValidationError -from plux import PluginManager +from plux import Plugin, PluginManager from localstack import config from localstack.aws.api import RequestContext from localstack.aws.chain import Handler, HandlerChain from localstack.constants import INTERNAL_RESOURCE_PATH from localstack.http import Response -from localstack.plugins import OASPlugin LOG = logging.getLogger(__name__) -class CoreOASPlugin(OASPlugin): - name = "localstack.core" +class OASPlugin(Plugin): + """ + This plugin allows to register an arbitrary number of OpenAPI specs, e.g., the spec for the public endpoints + of localstack.core. + The OpenAPIValidator handler uses (as opt-in) all the collected specs to validate the requests and the responses + to these public endpoints. + """ + + namespace = "localstack.openapi.spec" + + def __init__(self, spec_path: os.PathLike | str) -> None: + if isinstance(spec_path, str): + spec_path = Path(spec_path) + self.spec_path = spec_path + self.spec = {} - def __init__(self): - path = os.path.join(os.path.dirname(__file__), "..", "..", "openapi.yaml") - super().__init__(path) + def load(self): + with self.spec_path.open("r") as f: + self.spec = yaml.safe_load(f) class OpenAPIValidator(Handler): diff --git a/localstack-core/localstack/plugins.py b/localstack-core/localstack/plugins.py index 72bea7a02b5ea..a3fab6e8b07c2 100644 --- a/localstack-core/localstack/plugins.py +++ b/localstack-core/localstack/plugins.py @@ -1,11 +1,8 @@ import logging import os -from pathlib import Path - -import yaml -from plux import Plugin from localstack import config +from localstack.aws.handlers.validation import OASPlugin from localstack.runtime import hooks from localstack.utils.files import rm_rf from localstack.utils.ssl import get_cert_pem_file_path @@ -28,15 +25,9 @@ def delete_cached_certificate(): rm_rf(target_file) -class OASPlugin(Plugin): - namespace = "localstack.openapi.spec" - - def __init__(self, spec_path: os.PathLike | str) -> None: - if isinstance(spec_path, str): - spec_path = Path(spec_path) - self.spec_path = spec_path - self.spec = {} +class CoreOASPlugin(OASPlugin): + name = "localstack.core" - def load(self): - with self.spec_path.open("r") as f: - self.spec = yaml.safe_load(f) + def __init__(self): + path = os.path.join(os.path.dirname(__file__), "openapi.yaml") + super().__init__(path) From b46c11a04d32c66d44bef0b108c452f6d612013b Mon Sep 17 00:00:00 2001 From: Giovanni Grano Date: Fri, 13 Sep 2024 14:59:05 -0400 Subject: [PATCH 3/6] minor --- .../localstack/aws/handlers/validation.py | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/localstack-core/localstack/aws/handlers/validation.py b/localstack-core/localstack/aws/handlers/validation.py index bba51d7098bef..057abda4007bd 100644 --- a/localstack-core/localstack/aws/handlers/validation.py +++ b/localstack-core/localstack/aws/handlers/validation.py @@ -69,21 +69,21 @@ def __call__(self, chain: HandlerChain, context: RequestContext, response: Respo path = context.request.path if path.startswith(f"{INTERNAL_RESOURCE_PATH}/") or path.startswith("/_aws/"): - try: - for openapi in self.open_apis: + for openapi in self.open_apis: + try: openapi.validate_request(WerkzeugOpenAPIRequest(context.request)) - except RequestValidationError as e: - # Note: in this handler we only check validation errors, e.g., wrong body, missing required in the body. - response.status_code = 400 - response.set_json({"error": "Bad Request", "message": str(e)}) - chain.stop() - except OpenAPIError: - # Other errors can be raised when validating a request against the OpenAPI specification. - # The most common are: ServerNotFound, OperationNotFound, or PathNotFound. - # We explicitly do not check any other error but RequestValidationError ones. - # We shallow the exception to avoid excessive logging (e.g., a lot of ServerNotFound), as the only - # purpose of this handler is to check for request validation errors. - pass + except RequestValidationError as e: + # Note: in this handler we only check validation errors, e.g., wrong body, missing required. + response.status_code = 400 + response.set_json({"error": "Bad Request", "message": str(e)}) + chain.stop() + except OpenAPIError: + # Other errors can be raised when validating a request against the OpenAPI specification. + # The most common are: ServerNotFound, OperationNotFound, or PathNotFound. + # We explicitly do not check any other error but RequestValidationError ones. + # We shallow the exception to avoid excessive logging (e.g., a lot of ServerNotFound), as the only + # purpose of this handler is to check for request validation errors. + pass class OpenAPIResponseValidator(OpenAPIValidator): @@ -96,14 +96,14 @@ def __call__(self, chain: HandlerChain, context: RequestContext, response: Respo path = context.request.path if path.startswith(f"{INTERNAL_RESOURCE_PATH}/") or path.startswith("/_aws/"): - try: - for openapi in self.open_apis: + for openapi in self.open_apis: + try: openapi.validate_response( WerkzeugOpenAPIRequest(context.request), WerkzeugOpenAPIResponse(response), ) - except ResponseValidationError as exc: - LOG.error("Response validation failed for %s: $s", path, exc) - response.status_code = 500 - response.set_json({"error": exc.__class__.__name__, "message": str(exc)}) - chain.stop() + except ResponseValidationError as exc: + LOG.error("Response validation failed for %s: $s", path, exc) + response.status_code = 500 + response.set_json({"error": exc.__class__.__name__, "message": str(exc)}) + chain.stop() From 39fb1bbd76d5661693e07c4593dcd8d0df54e927 Mon Sep 17 00:00:00 2001 From: Giovanni Grano Date: Fri, 13 Sep 2024 15:14:15 -0400 Subject: [PATCH 4/6] added unit test for multiple specs --- tests/unit/aws/handlers/openapi.py | 49 ++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/unit/aws/handlers/openapi.py b/tests/unit/aws/handlers/openapi.py index 4a11d1ef5f604..fa4b78fe0d60f 100644 --- a/tests/unit/aws/handlers/openapi.py +++ b/tests/unit/aws/handlers/openapi.py @@ -1,6 +1,8 @@ import json import pytest +import yaml +from openapi_core import OpenAPI from rolo import Request, Response from rolo.gateway import RequestContext from rolo.gateway.handlers import EmptyResponseHandler @@ -9,6 +11,35 @@ from localstack.aws.chain import HandlerChain from localstack.aws.handlers.validation import OpenAPIRequestValidator +test_spec = """ +openapi: 3.0.0 +info: + title: Test API + version: 0.0.1 + description: Sample +paths: + /_localstack/dummy/{entityId}: + get: + parameters: + - name: entityId + in: path + required: true + schema: + type: number + example: 4 + responses: + '200': + description: Response list + content: + application/json: {} +""" + + +@pytest.fixture() +def openapi() -> OpenAPI: + spec = yaml.safe_load(test_spec) + return OpenAPI.from_dict(spec) + @pytest.fixture(autouse=True) def enable_validation_flag(monkeypatch): @@ -127,3 +158,21 @@ def test_body_validation_errors(self): assert response.status_code == 400 assert response.json["error"] == "Bad Request" assert response.json["message"] == "Request body validation error" + + def test_multiple_specs(self, openapi): + validator = OpenAPIRequestValidator() + validator.open_apis.append(openapi) + chain = HandlerChain([validator]) + context = RequestContext( + Request( + path="/_localstack/dummy/dummyName", + method="GET", + scheme="http", + headers={"Host": "localhost.localstack.cloud:4566"}, + ) + ) + response = Response() + chain.handle(context=context, response=response) + assert response.status_code == 400 + assert response.json["error"] == "Bad Request" + assert "Path parameter error" in response.json["message"] From f14d87e0ef3d8e6bd213f017e4591910440f6fc4 Mon Sep 17 00:00:00 2001 From: Giovanni Grano Date: Sat, 14 Sep 2024 17:10:10 -0400 Subject: [PATCH 5/6] minor optimization and some docs --- .../localstack/aws/handlers/validation.py | 31 +++++++++++++++---- localstack-core/localstack/plugins.py | 7 +---- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/localstack-core/localstack/aws/handlers/validation.py b/localstack-core/localstack/aws/handlers/validation.py index 057abda4007bd..26c1a1128c5a4 100644 --- a/localstack-core/localstack/aws/handlers/validation.py +++ b/localstack-core/localstack/aws/handlers/validation.py @@ -3,9 +3,8 @@ """ import logging -import os -from pathlib import Path +import importlib_resources import yaml from openapi_core import OpenAPI from openapi_core.contrib.werkzeug import WerkzeugOpenAPIRequest, WerkzeugOpenAPIResponse @@ -31,14 +30,34 @@ class OASPlugin(Plugin): of localstack.core. The OpenAPIValidator handler uses (as opt-in) all the collected specs to validate the requests and the responses to these public endpoints. + + An OAS plugin assumes the following directory layout. + + my_package + ├── sub_package + │ ├── __init__.py <-- spec file + │ ├── openapi.yaml + │ └── style.css + └── openapi.yaml <-- spec file + + Each package can have its own OpenAPI yaml spec. The only convention (and prerequisite) is to have the openapi.yaml + file at the root of the package. + + To declare a plugin you should simply implement OASPlugin and use the package name as the plugin name. E.g.,: + + class MyPackageOASPlugin(OASPlugin): + name = "my_package" + + class MySubPackagePlugin(OASPlugin): + name = "my_package.sub_package" """ namespace = "localstack.openapi.spec" - def __init__(self, spec_path: os.PathLike | str) -> None: - if isinstance(spec_path, str): - spec_path = Path(spec_path) - self.spec_path = spec_path + def __init__(self) -> None: + # By convention, the anchor is the package and is the name of the plugin. An openapi.yaml file is resolved + # from there. + self.spec_path = importlib_resources.files(self.name).joinpath("openapi.yaml") self.spec = {} def load(self): diff --git a/localstack-core/localstack/plugins.py b/localstack-core/localstack/plugins.py index a3fab6e8b07c2..c7efc527884d5 100644 --- a/localstack-core/localstack/plugins.py +++ b/localstack-core/localstack/plugins.py @@ -1,5 +1,4 @@ import logging -import os from localstack import config from localstack.aws.handlers.validation import OASPlugin @@ -26,8 +25,4 @@ def delete_cached_certificate(): class CoreOASPlugin(OASPlugin): - name = "localstack.core" - - def __init__(self): - path = os.path.join(os.path.dirname(__file__), "openapi.yaml") - super().__init__(path) + name = "localstack" From 2bf9217634bfa473fcfa6f58be9fc09febfb0cd4 Mon Sep 17 00:00:00 2001 From: Giovanni Grano Date: Sat, 14 Sep 2024 18:31:15 -0400 Subject: [PATCH 6/6] Switch to import from module path Forgot we don't have importlib_resources in the runtime dependencies and the standard library does not handle namespace packages installed in editable mode --- .../localstack/aws/handlers/validation.py | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/localstack-core/localstack/aws/handlers/validation.py b/localstack-core/localstack/aws/handlers/validation.py index 26c1a1128c5a4..cc931320453ee 100644 --- a/localstack-core/localstack/aws/handlers/validation.py +++ b/localstack-core/localstack/aws/handlers/validation.py @@ -3,8 +3,10 @@ """ import logging +import os +import sys +from pathlib import Path -import importlib_resources import yaml from openapi_core import OpenAPI from openapi_core.contrib.werkzeug import WerkzeugOpenAPIRequest, WerkzeugOpenAPIResponse @@ -37,27 +39,29 @@ class OASPlugin(Plugin): ├── sub_package │ ├── __init__.py <-- spec file │ ├── openapi.yaml - │ └── style.css + │ └── plugins.py <-- plugins + ├── plugins.py <-- plugins └── openapi.yaml <-- spec file - Each package can have its own OpenAPI yaml spec. The only convention (and prerequisite) is to have the openapi.yaml - file at the root of the package. - - To declare a plugin you should simply implement OASPlugin and use the package name as the plugin name. E.g.,: + Each package can have its own OpenAPI yaml spec which is loaded by the correspondent plugin in plugins.py + You can simply create a plugin like the following: class MyPackageOASPlugin(OASPlugin): name = "my_package" - class MySubPackagePlugin(OASPlugin): - name = "my_package.sub_package" + The only convention is that plugins.py and openapi.yaml have the same pathname. """ namespace = "localstack.openapi.spec" def __init__(self) -> None: - # By convention, the anchor is the package and is the name of the plugin. An openapi.yaml file is resolved - # from there. - self.spec_path = importlib_resources.files(self.name).joinpath("openapi.yaml") + # By convention a plugins.py is at the same level (i.e., same pathname) of the openapi.yaml file. + # importlib.resources would be a better approach but has issues with namespace packages in editable mode + _module = sys.modules[self.__module__] + self.spec_path = Path( + os.path.join(os.path.dirname(os.path.abspath(_module.__file__)), "openapi.yaml") + ) + assert self.spec_path.exists() self.spec = {} def load(self): @@ -69,6 +73,9 @@ class OpenAPIValidator(Handler): open_apis: list["OpenAPI"] def __init__(self) -> None: + # avoid to load the specs if we don't have to perform any validation + if not (config.OPENAPI_VALIDATE_REQUEST or config.OPENAPI_VALIDATE_RESPONSE): + return specs = PluginManager("localstack.openapi.spec").load_all() self.open_apis = [] for spec in specs: