Skip to content

Apigw ng new path style #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions localstack-core/localstack/aws/handlers/cors.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,10 @@ def should_enforce_self_managed_service(context: RequestContext) -> bool:
if not config.DISABLE_CUSTOM_CORS_APIGATEWAY:
# we don't check for service_name == "apigw" here because ``.execute-api.`` can be either apigw v1 or v2
path = context.request.path
is_user_request = ".execute-api." in context.request.host or (
path.startswith("/restapis/") and f"/{PATH_USER_REQUEST}" in context.request.path
is_user_request = (
".execute-api." in context.request.host
or (path.startswith("/restapis/") and f"/{PATH_USER_REQUEST}" in context.request.path)
or (path.startswith("/_aws/apigateway/execute-api"))
)
if is_user_request:
return False
Expand Down
7 changes: 7 additions & 0 deletions localstack-core/localstack/services/apigateway/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,13 @@ def path_based_url(api_id: str, stage_name: str, path: str) -> str:
return pattern.format(api_id=api_id, stage_name=stage_name, path=path)


def localstack_path_based_url(api_id: str, stage_name: str, path: str) -> str:
"""Return URL for inbound API gateway for given API ID, stage name, and path on the _aws namespace"""
return (
f"{config.external_service_url()}/_aws/apigateway/execute-api/{api_id}/{stage_name}{path}"
)


def host_based_url(rest_api_id: str, path: str, stage_name: str = None):
"""Return URL for inbound API gateway for given API ID, stage name, and path with custom dns
format"""
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import datetime
import logging
import re
from collections import defaultdict
from typing import Optional
from urllib.parse import urlparse

from rolo.request import Request, restore_payload
from rolo.request import restore_payload
from werkzeug.datastructures import Headers, MultiDict

from localstack.http import Response
Expand Down Expand Up @@ -58,16 +59,16 @@ def create_invocation_request(self, context: RestApiInvocationContext) -> Invoca
headers=headers,
body=restore_payload(request),
)

self._enrich_with_raw_path(request, invocation_request, stage_name=context.stage)
self._enrich_with_raw_path(context, invocation_request)

return invocation_request

@staticmethod
def _enrich_with_raw_path(
request: Request, invocation_request: InvocationRequest, stage_name: str
context: RestApiInvocationContext, invocation_request: InvocationRequest
):
# Base path is not URL-decoded, so we need to get the `RAW_URI` from the request
request = context.request
raw_uri = request.environ.get("RAW_URI") or request.path

# if the request comes from the LocalStack only `_user_request_` route, we need to remove this prefix from the
Expand All @@ -76,8 +77,17 @@ def _enrich_with_raw_path(
# in this format, the stage is before `_user_request_`, so we don't need to remove it
raw_uri = raw_uri.partition("_user_request_")[2]
else:
if raw_uri.startswith("/_aws/apigateway/execute-api"):
# the API can be cased in the path, so we need to ignore it to remove it
raw_uri = re.sub(
f"^/_aws/apigateway/execute-api/{context.api_id}",
"",
raw_uri,
flags=re.IGNORECASE,
Comment on lines +82 to +86
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider using a constant for the path prefix '/_aws/apigateway/execute-api' to improve maintainability

)

# remove the stage from the path, only replace the first occurrence
raw_uri = raw_uri.replace(f"/{stage_name}", "", 1)
raw_uri = raw_uri.replace(f"/{context.stage}", "", 1)

if raw_uri.startswith("//"):
# TODO: AWS validate this assumption
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from werkzeug.routing import Rule

from localstack.constants import APPLICATION_JSON, AWS_REGION_US_EAST_1, DEFAULT_AWS_ACCOUNT_ID
from localstack.deprecations import deprecated_endpoint
from localstack.http import Response
from localstack.services.apigateway.models import ApiGatewayStore, apigateway_stores
from localstack.services.edge import ROUTER
Expand Down Expand Up @@ -104,6 +105,7 @@ def create_response(request: Request) -> Response:
class ApiGatewayRouter:
router: Router[Handler]
handler: ApiGatewayEndpoint
EXECUTE_API_INTERNAL_PATH = "/_aws/apigateway/execute-api"

def __init__(self, router: Router[Handler] = None, handler: ApiGatewayEndpoint = None):
self.router = router or ROUTER
Expand All @@ -113,6 +115,12 @@ def __init__(self, router: Router[Handler] = None, handler: ApiGatewayEndpoint =
def register_routes(self) -> None:
LOG.debug("Registering API Gateway routes.")
host_pattern = "<regex('[^-]+'):api_id><regex('(-vpce-[^.]+)?'):vpce_suffix>.execute-api.<regex('.*'):server>"
deprecated_route_endpoint = deprecated_endpoint(
endpoint=self.handler,
previous_path="/restapis/<api_id>/<stage>/_user_request_",
deprecation_version="3.7.0",
new_path=f"{self.EXECUTE_API_INTERNAL_PATH}/<api_id>/<stage>",
)
rules = [
self.router.add(
path="/",
Expand All @@ -134,14 +142,32 @@ def register_routes(self) -> None:
endpoint=self.handler,
strict_slashes=True,
),
# add the localstack-specific _user_request_ routes
# add the deprecated localstack-specific _user_request_ routes
self.router.add(
path="/restapis/<api_id>/<stage>/_user_request_",
endpoint=self.handler,
endpoint=deprecated_route_endpoint,
defaults={"path": ""},
),
self.router.add(
path="/restapis/<api_id>/<stage>/_user_request_/<greedy_path:path>",
endpoint=deprecated_route_endpoint,
strict_slashes=True,
),
# add the localstack-specific so-called "path-style" routes when DNS resolving is not possible
self.router.add(
path=f"{self.EXECUTE_API_INTERNAL_PATH}/<api_id>/",
endpoint=self.handler,
defaults={"path": "", "stage": None},
strict_slashes=True,
),
self.router.add(
path=f"{self.EXECUTE_API_INTERNAL_PATH}/<api_id>/<stage>/",
endpoint=self.handler,
defaults={"path": ""},
strict_slashes=False,
),
self.router.add(
path=f"{self.EXECUTE_API_INTERNAL_PATH}/<api_id>/<stage>/<greedy_path:path>",
endpoint=self.handler,
strict_slashes=True,
),
Expand Down
13 changes: 11 additions & 2 deletions tests/aws/services/apigateway/apigateway_fixtures.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
from enum import Enum
from typing import Dict

from localstack.services.apigateway.helpers import host_based_url, path_based_url
from localstack.services.apigateway.helpers import (
host_based_url,
localstack_path_based_url,
path_based_url,
)
from localstack.testing.aws.util import is_aws_cloud
from localstack.utils.aws import aws_stack

Expand Down Expand Up @@ -195,6 +199,7 @@ def delete_cognito_user_pool_client(cognito_idp, **kwargs):
class UrlType(Enum):
HOST_BASED = 0
PATH_BASED = 1
LS_PATH_BASED = 2


def api_invoke_url(
Expand All @@ -209,6 +214,10 @@ def api_invoke_url(
region = aws_stack.get_boto3_region()
stage = f"/{stage}" if stage else ""
return f"https://{api_id}.execute-api.{region}.amazonaws.com{stage}{path}"

if url_type == UrlType.HOST_BASED:
return host_based_url(api_id, stage_name=stage, path=path)
return path_based_url(api_id, stage_name=stage, path=path)
elif url_type == UrlType.PATH_BASED:
return path_based_url(api_id, stage_name=stage, path=path)
else:
return localstack_path_based_url(api_id, stage_name=stage, path=path)
14 changes: 12 additions & 2 deletions tests/aws/services/apigateway/test_apigateway_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
get_resource_for_path,
get_rest_api_paths,
host_based_url,
localstack_path_based_url,
path_based_url,
)
from localstack.testing.aws.util import in_default_partition
Expand Down Expand Up @@ -152,10 +153,14 @@ def test_delete_rest_api_with_invalid_id(self, aws_client):
assert "Invalid API identifier specified" in e.value.response["Error"]["Message"]
assert "foobar" in e.value.response["Error"]["Message"]

@pytest.mark.parametrize("url_function", [path_based_url, host_based_url])
@pytest.mark.parametrize(
"url_function", [path_based_url, host_based_url, localstack_path_based_url]
)
@markers.aws.only_localstack
# This is not a possible feature on aws.
def test_create_rest_api_with_custom_id(self, create_rest_apigw, url_function, aws_client):
if not is_next_gen_api() and url_function == localstack_path_based_url:
pytest.skip("This URL type is not supported in the legacy implementation")
apigw_name = f"gw-{short_uid()}"
test_id = "testId123"
api_id, name, _ = create_rest_apigw(name=apigw_name, tags={TAG_KEY_CUSTOM_ID: test_id})
Expand Down Expand Up @@ -311,13 +316,18 @@ def test_api_gateway_lambda_integration_aws_type(
assert response.headers["Content-Type"] == "text/html"
assert response.headers["Access-Control-Allow-Origin"] == "*"

@pytest.mark.parametrize("url_type", [UrlType.HOST_BASED, UrlType.PATH_BASED])
@pytest.mark.parametrize(
"url_type", [UrlType.HOST_BASED, UrlType.PATH_BASED, UrlType.LS_PATH_BASED]
)
@pytest.mark.parametrize("disable_custom_cors", [True, False])
@pytest.mark.parametrize("origin", ["http://allowed", "http://denied"])
@markers.aws.only_localstack
def test_invoke_endpoint_cors_headers(
self, url_type, disable_custom_cors, origin, monkeypatch, aws_client
):
if not is_next_gen_api() and url_type == UrlType.LS_PATH_BASED:
pytest.skip("This URL type is not supported with the legacy implementation")

monkeypatch.setattr(config, "DISABLE_CUSTOM_CORS_APIGATEWAY", disable_custom_cors)
monkeypatch.setattr(
cors, "ALLOWED_CORS_ORIGINS", cors.ALLOWED_CORS_ORIGINS + ["http://allowed"]
Expand Down
63 changes: 54 additions & 9 deletions tests/unit/services/apigateway/test_handler_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,34 @@ def test_parse_user_request_path(
assert context.invocation_request["path"] == "/foo%2Fbar/ed"
assert context.invocation_request["raw_path"] == "//foo%2Fbar/ed"

@pytest.mark.parametrize("addressing", ["host", "user_request"])
def test_parse_localstack_only_path(
self, dummy_deployment, parse_handler_chain, get_invocation_context
):
# simulate a path request
request = Request(
"GET",
path=f"/_aws/apigateway/execute-api/{TEST_API_ID}/{TEST_API_STAGE}/foo/bar/ed",
raw_path=f"/_aws/apigateway/execute-api/{TEST_API_ID}/{TEST_API_STAGE}//foo%2Fbar/ed",
)

context = get_invocation_context(request)
context.deployment = dummy_deployment

parse_handler_chain.handle(context, Response())

# assert that the user request prefix has been stripped off
assert context.invocation_request["path"] == "/foo%2Fbar/ed"
assert context.invocation_request["raw_path"] == "//foo%2Fbar/ed"

@pytest.mark.parametrize("addressing", ["host", "user_request", "path_style"])
def test_parse_path_same_as_stage(
self, dummy_deployment, parse_handler_chain, get_invocation_context, addressing
):
path = TEST_API_STAGE
if addressing == "host":
full_path = f"/{TEST_API_STAGE}/{path}"
elif addressing == "path_style":
full_path = f"/_aws/apigateway/execute-api/{TEST_API_ID}/{TEST_API_STAGE}/{path}"
else:
full_path = f"/restapis/{TEST_API_ID}/{TEST_API_STAGE}/_user_request_/{path}"

Expand All @@ -181,6 +202,27 @@ def test_parse_path_same_as_stage(
assert context.invocation_request["path"] == f"/{TEST_API_STAGE}"
assert context.invocation_request["raw_path"] == f"/{TEST_API_STAGE}"

@pytest.mark.parametrize("addressing", ["user_request", "path_style"])
def test_cased_api_id_in_path(
self, dummy_deployment, parse_handler_chain, get_invocation_context, addressing
):
if addressing == "path_style":
full_path = f"/_aws/apigateway/execute-api/TestApi/{TEST_API_STAGE}/test"
else:
full_path = f"/restapis/{TEST_API_ID}/TestApi/_user_request_/test"

# simulate a path request
request = Request("GET", path=full_path)

context = get_invocation_context(request)
context.deployment = dummy_deployment

parse_handler_chain.handle(context, Response())

# assert that the user request prefix has been stripped off
assert context.invocation_request["path"] == "/test"
assert context.invocation_request["raw_path"] == "/test"

def test_trace_id_logic(self):
headers = Headers({"x-amzn-trace-id": "Root=trace;Parent=parent"})
trace = InvocationRequestParser.populate_trace_id(headers)
Expand Down Expand Up @@ -336,10 +378,13 @@ def deployment_with_any_routes(self, dummy_deployment):
def get_path_from_addressing(path: str, addressing: str) -> str:
if addressing == "host":
return f"/{TEST_API_STAGE}{path}"
else:
elif addressing == "user_request":
return f"/restapis/{TEST_API_ID}/{TEST_API_STAGE}/_user_request_/{path}"
else:
# this new style allows following the regular order in an easier way, stage is always before path
return f"/_aws/apigateway/execute-api/{TEST_API_ID}/{TEST_API_STAGE}{path}"

@pytest.mark.parametrize("addressing", ["host", "user_request"])
@pytest.mark.parametrize("addressing", ["host", "user_request", "path_style"])
def test_route_request_no_param(
self, deployment_with_routes, parse_handler_chain, get_invocation_context, addressing
):
Expand All @@ -364,7 +409,7 @@ def test_route_request_no_param(
assert context.invocation_request["path_parameters"] == {}
assert context.stage_variables == {"foo": "bar"}

@pytest.mark.parametrize("addressing", ["host", "user_request"])
@pytest.mark.parametrize("addressing", ["host", "user_request", "path_style"])
def test_route_request_with_path_parameter(
self, deployment_with_routes, parse_handler_chain, get_invocation_context, addressing
):
Expand All @@ -389,7 +434,7 @@ def test_route_request_with_path_parameter(
assert context.context_variables["resourcePath"] == "/foo/{param}"
assert context.context_variables["resourceId"] == context.resource["id"]

@pytest.mark.parametrize("addressing", ["host", "user_request"])
@pytest.mark.parametrize("addressing", ["host", "user_request", "path_style"])
def test_route_request_with_greedy_parameter(
self, deployment_with_routes, parse_handler_chain, get_invocation_context, addressing
):
Expand Down Expand Up @@ -448,7 +493,7 @@ def test_route_request_with_greedy_parameter(
"proxy": "test2/is/a/proxy/req2%Fuest"
}

@pytest.mark.parametrize("addressing", ["host", "user_request"])
@pytest.mark.parametrize("addressing", ["host", "user_request", "path_style"])
def test_route_request_no_match_on_path(
self, deployment_with_routes, parse_handler_chain, get_invocation_context, addressing
):
Expand All @@ -466,7 +511,7 @@ def test_route_request_no_match_on_path(
with pytest.raises(MissingAuthTokenError):
handler(parse_handler_chain, context, Response())

@pytest.mark.parametrize("addressing", ["host", "user_request"])
@pytest.mark.parametrize("addressing", ["host", "user_request", "path_style"])
def test_route_request_no_match_on_method(
self, deployment_with_routes, parse_handler_chain, get_invocation_context, addressing
):
Expand All @@ -484,7 +529,7 @@ def test_route_request_no_match_on_method(
with pytest.raises(MissingAuthTokenError):
handler(parse_handler_chain, context, Response())

@pytest.mark.parametrize("addressing", ["host", "user_request"])
@pytest.mark.parametrize("addressing", ["host", "user_request", "path_style"])
def test_route_request_with_double_slash_and_trailing_and_encoded(
self, deployment_with_routes, parse_handler_chain, get_invocation_context, addressing
):
Expand All @@ -504,7 +549,7 @@ def test_route_request_with_double_slash_and_trailing_and_encoded(
assert context.resource["path"] == "/foo/{param}"
assert context.invocation_request["path_parameters"] == {"param": "foo%2Fbar"}

@pytest.mark.parametrize("addressing", ["host", "user_request"])
@pytest.mark.parametrize("addressing", ["host", "user_request", "path_style"])
def test_route_request_any_is_last(
self, deployment_with_any_routes, parse_handler_chain, get_invocation_context, addressing
):
Expand Down