Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
19aec2e
Set operationName in local API request context
ramosbugs Dec 5, 2020
02011e9
Remove debug line
ramosbugs Dec 5, 2020
0db016c
Merge branch 'develop' into develop-operation-name
awood45 Apr 29, 2021
4b715bb
Merge branch 'develop' into develop-operation-name
awood45 Apr 30, 2021
3e49ec6
Merge branch 'develop' into develop-operation-name
wchengru Sep 7, 2021
ab9a03e
Merge branch 'develop' into develop-operation-name
wchengru Sep 9, 2021
b088caf
Resolve dedupe_function_routes, add integration tests
wchengru Sep 11, 2021
3ea20dd
Merge branch 'develop' into develop-operation-name
wchengru Sep 11, 2021
b824f81
Merge branch 'develop' into develop-operation-name
wchengru Sep 11, 2021
e1c0306
black reformat
wchengru Sep 11, 2021
0f05a65
Merge branch 'develop-operation-name' of github.com:wchengru/aws-sam-…
wchengru Sep 11, 2021
e0376ea
fix an issue that V2 swagger still have opertaion name
wchengru Sep 13, 2021
97a1354
fix operationId issue
wchengru Sep 13, 2021
fc702f8
Do not send any HttpApi operationId/OpertaionName
wchengru Sep 13, 2021
1197bdb
remove unused code
wchengru Sep 13, 2021
59b3b8b
Merge branch 'develop' into develop-operation-name
wchengru Sep 14, 2021
5b01b8c
Merge branch 'develop' into develop-operation-name
wchengru Sep 14, 2021
036cd5c
Merge branch 'develop-operation-name' into develop-operation-name
wchengru Sep 14, 2021
b2e7c99
Merge pull request #1 from wchengru/develop-operation-name
ramosbugs Sep 14, 2021
71fcedd
Merge branch 'develop' into develop-operation-name
jfuss Sep 17, 2021
d61a434
Merge branch 'develop' into develop-operation-name
wchengru Sep 20, 2021
cc5e7e3
Merge branch 'develop' into develop-operation-name
wchengru Sep 21, 2021
73453ac
refactor if conditions, and fix test cases to confirm that the new ke…
Sep 21, 2021
1d9bd8a
small changes on comments
Sep 21, 2021
ec857d9
remove debug line
Sep 21, 2021
964373d
Merge branch 'develop' into develop-operation-name
wchengru Sep 21, 2021
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
1 change: 1 addition & 0 deletions samcli/commands/local/lib/swagger/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ def get_routes(self, event_type=Route.API):
methods=[method],
event_type=event_type,
payload_format_version=payload_format_version,
operation_name=method_config.get("operationId"),
stack_path=self.stack_path,
)
result.append(route)
Expand Down
3 changes: 2 additions & 1 deletion samcli/lib/providers/api_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def dedupe_function_routes(routes: List[Route]) -> List[Route]:
grouped_routes: Dict[str, Route] = {}

for route in routes:
key = "{}-{}-{}".format(route.stack_path, route.function_name, route.path)
key = "{}-{}-{}-{}".format(route.stack_path, route.function_name, route.path, route.operation_name or "")
config = grouped_routes.get(key, None)
methods = route.methods
if config:
Expand All @@ -163,6 +163,7 @@ def dedupe_function_routes(routes: List[Route]) -> List[Route]:
methods=sorted_methods,
event_type=route.event_type,
payload_format_version=route.payload_format_version,
operation_name=route.operation_name,
stack_path=route.stack_path,
)
return list(grouped_routes.values())
Expand Down
4 changes: 4 additions & 0 deletions samcli/lib/providers/cfn_api_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ def _extract_cloud_formation_method(
resource_id = properties.get("ResourceId")
rest_api_id = properties.get("RestApiId")
method = properties.get("HttpMethod")
operation_name = properties.get("OperationName")

resource_path = "/"
if isinstance(resource_id, str): # If the resource_id resolves to a string
Expand All @@ -202,6 +203,7 @@ def _extract_cloud_formation_method(
methods=[method],
function_name=self._get_integration_function_name(integration),
path=resource_path,
operation_name=operation_name,
stack_path=stack_path,
)
collector.add_routes(rest_api_id, [routes])
Expand Down Expand Up @@ -295,6 +297,7 @@ def _extract_cfn_gateway_v2_route(
api_id = properties.get("ApiId")
route_key = properties.get("RouteKey")
integration_target = properties.get("Target")
operation_name = properties.get("OperationName")

if integration_target:
function_name, payload_format_version = self._get_route_function_name(resources, integration_target)
Expand All @@ -320,6 +323,7 @@ def _extract_cfn_gateway_v2_route(
function_name=function_name,
event_type=Route.HTTP,
payload_format_version=payload_format_version,
operation_name=operation_name,
stack_path=stack_path,
)
collector.add_routes(api_id, [routes])
Expand Down
28 changes: 26 additions & 2 deletions samcli/local/apigw/local_apigw_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def __init__(
event_type: str = API,
payload_format_version: Optional[str] = None,
is_default_route: bool = False,
operation_name=None,
stack_path: str = "",
):
"""
Expand All @@ -63,6 +64,7 @@ def __init__(
:param str event_type: Type of the event. "Api" or "HttpApi"
:param str payload_format_version: version of payload format
:param bool is_default_route: determines if the default route or not
:param string operation_name: Swagger operationId for the route
:param str stack_path: path of the stack the route is located
"""
self.methods = self.normalize_method(methods)
Expand All @@ -71,6 +73,7 @@ def __init__(
self.event_type = event_type
self.payload_format_version = payload_format_version
self.is_default_route = is_default_route
self.operation_name = operation_name
self.stack_path = stack_path

def __eq__(self, other):
Expand All @@ -79,6 +82,7 @@ def __eq__(self, other):
and sorted(self.methods) == sorted(other.methods)
and self.function_name == other.function_name
and self.path == other.path
and self.operation_name == other.operation_name
and self.stack_path == other.stack_path
)

Expand Down Expand Up @@ -164,6 +168,7 @@ def create(self):
# This will normalize all endpoints and strip any trailing '/'
self._app.url_map.strict_slashes = False
default_route = None

for api_gateway_route in self.api.routes:
if api_gateway_route.path == "$default":
default_route = api_gateway_route
Expand Down Expand Up @@ -318,9 +323,25 @@ def _request_handler(self, **kwargs):
self.api.stage_variables,
route_key,
)
elif route.event_type == Route.API:
# The OperationName is only sent to the Lambda Function from API Gateway V1(Rest API).
event = self._construct_v_1_0_event(
request,
self.port,
self.api.binary_media_types,
self.api.stage_name,
self.api.stage_variables,
route.operation_name,
)
else:
# For Http Apis with payload version 1.0, API Gateway never sends the OperationName.
event = self._construct_v_1_0_event(
request, self.port, self.api.binary_media_types, self.api.stage_name, self.api.stage_variables
request,
self.port,
self.api.binary_media_types,
self.api.stage_name,
self.api.stage_variables,
None,
)
except UnicodeDecodeError:
return ServiceErrorResponses.lambda_failure_response()
Expand Down Expand Up @@ -641,7 +662,9 @@ def _merge_response_headers(headers, multi_headers):
return processed_headers

@staticmethod
def _construct_v_1_0_event(flask_request, port, binary_types, stage_name=None, stage_variables=None):
def _construct_v_1_0_event(
flask_request, port, binary_types, stage_name=None, stage_variables=None, operation_name=None
):
"""
Helper method that constructs the Event to be passed to Lambda

Expand Down Expand Up @@ -687,6 +710,7 @@ def _construct_v_1_0_event(flask_request, port, binary_types, stage_name=None, s
path=endpoint,
protocol=protocol,
domain_name=host,
operation_name=operation_name,
)

headers_dict, multi_value_headers_dict = LocalApigwService._event_headers(flask_request, port)
Expand Down
6 changes: 6 additions & 0 deletions samcli/local/events/api_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ def __init__(
domain_name=None,
request_time_epoch=int(time()),
request_time=datetime.utcnow().strftime("%d/%b/%Y:%H:%M:%S +0000"),
operation_name=None,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used for all API Events? If so, shouldn't we model different RequestsContext's for different events?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we have different RequestContext for V1 and V2 events. This is for v1 events only, it only used in

context = RequestContext(

For v2 api, it is using RequestContextV2:
context = RequestContextV2(http=context_http, route_key=route_key, stage=stage_name)

The httpapi with payload v1 is also using this type of event. For httpapi or restapi without "operationName", it won't be passed into the RequestContext map: https://github.com/aws/aws-sam-cli/pull/2458/files#diff-0ffb9968392fca338c633173f593848b7301fb166fcf56bd3178994d15e1c88aR150

):
"""
Constructs a RequestContext
Expand All @@ -96,6 +97,7 @@ def __init__(
:param ContextIdentity identity: Identity for the Request
:param str extended_request_id:
:param str path:
:param str operation_name: Swagger operationId for the route
:param str protocol: Optional, the protocal to make the request
:param str domain_name: Optional, the name of the domain
:param int request_time_epoch: Optional, an epoch timestamp to override the request time
Expand All @@ -116,6 +118,7 @@ def __init__(
self.domain_name = domain_name
self.request_time_epoch = request_time_epoch
self.request_time = request_time
self.operation_name = operation_name

def to_dict(self):
"""
Expand Down Expand Up @@ -144,6 +147,9 @@ def to_dict(self):
"requestTime": self.request_time,
}

if self.operation_name is not None:
json_dict["operationName"] = self.operation_name

return json_dict


Expand Down
59 changes: 59 additions & 0 deletions tests/integration/local/start_api/test_start_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,32 @@ def test_patch_call_with_path_setup_with_any_swagger(self):
self.assertEqual(response.status_code, 200)
self.assertEqual(response.json(), {"hello": "world"})

@pytest.mark.flaky(reruns=3)
@pytest.mark.timeout(timeout=600, method="thread")
def test_http_api_payload_v1_should_not_have_operation_id(self):
response = requests.get(self.url + "/httpapi-operation-id-v1", timeout=300)
self.assertEqual(response.status_code, 200)

response_data = response.json()
self.assertEqual(response_data.get("version", {}), "1.0")
# operationName or operationId shouldn't be processed by Httpapi swaggers
request_context_keys = [key.lower() for key in response_data.get("requestContext", {}).keys()]
self.assertTrue("operationid" not in request_context_keys)
self.assertTrue("operationname" not in request_context_keys)

@pytest.mark.flaky(reruns=3)
@pytest.mark.timeout(timeout=600, method="thread")
def test_http_api_payload_v2_should_not_have_operation_id(self):
response = requests.get(self.url + "/httpapi-operation-id-v2", timeout=300)
self.assertEqual(response.status_code, 200)

response_data = response.json()
self.assertEqual(response_data.get("version", {}), "2.0")
# operationName or operationId shouldn't be processed by Httpapi swaggers
request_context_keys = [key.lower() for key in response_data.get("requestContext", {}).keys()]
self.assertTrue("operationid" not in request_context_keys)
self.assertTrue("operationname" not in request_context_keys)


class TestStartApiWithSwaggerRestApis(StartApiIntegBaseClass):
template_path = "/testdata/start_api/swagger-rest-api-template.yaml"
Expand Down Expand Up @@ -792,6 +818,16 @@ def test_binary_response(self):
self.assertEqual(response.headers.get("Content-Type"), "image/gif")
self.assertEqual(response.content, expected)

@pytest.mark.flaky(reruns=3)
@pytest.mark.timeout(timeout=600, method="thread")
def test_rest_api_operation_id(self):
"""
Binary data is returned correctly
"""
response = requests.get(self.url + "/printeventwithoperationidfunction", timeout=300)
self.assertEqual(response.status_code, 200)
self.assertEqual(response.json().get("requestContext", {}).get("operationName"), "MyOperationName")


class TestServiceResponses(StartApiIntegBaseClass):
"""
Expand Down Expand Up @@ -1691,6 +1727,18 @@ def test_http_api_is_reachable(self):
self.assertEqual(response.status_code, 200)
self.assertEqual(response.json(), {"hello": "world"})

@pytest.mark.flaky(reruns=3)
@pytest.mark.timeout(timeout=600, method="thread")
def test_http_api_with_operation_name_is_reachable(self):
response = requests.get(self.url + "/http-api-with-operation-name", timeout=300)

self.assertEqual(response.status_code, 200)
response_data = response.json()
# operationName or operationId shouldn't be processed by Httpapi
request_context_keys = [key.lower() for key in response_data.get("requestContext", {}).keys()]
self.assertTrue("operationid" not in request_context_keys)
self.assertTrue("operationname" not in request_context_keys)

@pytest.mark.flaky(reruns=3)
@pytest.mark.timeout(timeout=600, method="thread")
def test_rest_api_is_reachable(self):
Expand All @@ -1699,6 +1747,13 @@ def test_rest_api_is_reachable(self):
self.assertEqual(response.status_code, 200)
self.assertEqual(response.json(), {"hello": "world"})

@pytest.mark.flaky(reruns=3)
@pytest.mark.timeout(timeout=600, method="thread")
def test_rest_api_with_operation_name_is_reachable(self):
response = requests.get(self.url + "/rest-api-with-operation-name", timeout=300)
self.assertEqual(response.status_code, 200)
self.assertEqual(response.json(), {"operation_name": "MyOperationName"})


class TestCFNTemplateHttpApiWithSwaggerBody(StartApiIntegBaseClass):
template_path = "/testdata/start_api/cfn-http-api-with-swagger-body.yaml"
Expand All @@ -1716,6 +1771,10 @@ def test_swagger_got_parsed_and_api_is_reachable_and_payload_version_is_2(self):
self.assertEqual(response_data.get("version", {}), "2.0")
self.assertIsNone(response_data.get("multiValueHeaders"))
self.assertIsNotNone(response_data.get("cookies"))
# operationName or operationId shouldn't be processed by Httpapi swaggers
request_context_keys = [key.lower() for key in response_data.get("requestContext", {}).keys()]
self.assertTrue("operationid" not in request_context_keys)
self.assertTrue("operationname" not in request_context_keys)


class TestWarmContainersBaseClass(StartApiIntegBaseClass):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,20 @@ Resources:
Value: SAM
Timeout: 3
Type: AWS::Lambda::Function
HelloWorldFunctionWithOperationName:
Properties:
Handler: main.operation_name_handler
Code: '.'
Role:
Fn::GetAtt:
- HelloWorldFunctionRole
- Arn
Runtime: python3.6
Tags:
- Key: lambda:createdBy
Value: SAM
Timeout: 3
Type: AWS::Lambda::Function
HelloWorldFunctionRole:
Properties:
AssumeRolePolicyDocument:
Expand Down Expand Up @@ -81,6 +95,15 @@ Resources:
IntegrationMethod: POST
IntegrationUri:
Fn::Sub: arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${HelloWorldFunction.Arn}/invocations
MyIntegrationWithOperationName:
Type: 'AWS::ApiGatewayV2::Integration'
Properties:
ApiId: !Ref HTTPAPIGateway
PayloadFormatVersion: "1.0"
IntegrationType: AWS_PROXY
IntegrationMethod: POST
IntegrationUri:
Fn::Sub: arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${HelloWorldFunctionWithOperationName.Arn}/invocations
MyRoute:
Type: AWS::ApiGatewayV2::Route
Properties:
Expand All @@ -90,6 +113,16 @@ Resources:
- /
- - integrations
- !Ref MyIntegration
MyRouteWithOperationName:
Type: AWS::ApiGatewayV2::Route
Properties:
ApiId: !Ref HTTPAPIGateway
OperationName: 'MyOperationName'
RouteKey: 'GET /http-api-with-operation-name'
Target: !Join
- /
- - integrations
- !Ref MyIntegrationWithOperationName
RestApiGateway:
Type: AWS::ApiGateway::RestApi
Properties:
Expand Down Expand Up @@ -141,4 +174,39 @@ Resources:
- Fn::GetAtt:
- HelloWorldFunction
- Arn
- /invocations
RestApiGatewayWithOperationNameResource:
Type: AWS::ApiGateway::Resource
Properties:
ParentId:
Fn::GetAtt:
- RestApiGateway
- RootResourceId
PathPart: "rest-api-with-operation-name"
RestApiId:
Ref: RestApiGateway
RestApiGatewayWithOperationNameMethod:
Type: AWS::ApiGateway::Method
Properties:
HttpMethod: GET
OperationName: 'MyOperationName'
ResourceId:
Ref: RestApiGatewayWithOperationNameResource
RestApiId:
Ref: RestApiGateway
AuthorizationType: NONE
Integration:
IntegrationHttpMethod: POST
Type: AWS_PROXY
Uri:
Fn::Join:
- ""
- - "arn:"
- Ref: AWS::Partition
- ":apigateway:"
- Ref: AWS::Region
- :lambda:path/2015-03-31/functions/
- Fn::GetAtt:
- HelloWorldFunctionWithOperationName
- Arn
- /invocations
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ Resources:
/echoeventbody:
get:
responses: {}
operationId: 'postOperationIdShouldNotBeInHttpApi'
x-amazon-apigateway-integration:
httpMethod: POST
payloadFormatVersion: '2.0'
Expand Down
3 changes: 3 additions & 0 deletions tests/integration/testdata/start_api/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
def handler(event, context):
return {"statusCode": 200, "body": json.dumps({"hello": "world"})}

def operation_name_handler(event, context):
return {"statusCode": 200, "body": json.dumps({"operation_name": event["requestContext"].get("operationName", "")})}


def echo_event_handler(event, context):
return {"statusCode": 200, "body": json.dumps(event)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ Resources:
type: aws_proxy
uri:
Fn::Sub: arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${MyNonServerlessLambdaFunction.Arn}/invocations
"/printeventwithoperationidfunction":
get:
operationId: 'MyOperationName'
x-amazon-apigateway-integration:
httpMethod: POST
type: aws_proxy
uri:
Fn::Sub: arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${EchoEventBodyFunction.Arn}/invocations
swagger: '2.0'
x-amazon-apigateway-binary-media-types:
- image/gif
Expand Down
Loading