Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
29 changes: 29 additions & 0 deletions samtranslator/model/eventsources/push.py
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,35 @@ def _add_swagger_integration(self, api, function, intrinsics_resolver):
path=self.Path, method_name=self.Method, request_model=self.RequestModel
)

validate_body = self.RequestModel.get("ValidateBody")
validate_request = self.RequestModel.get("ValidateRequest")

# Checking if any of the fields are defined as it can be false we are checking if the field are not None
if not isinstance(validate_body, type(None)) or not isinstance(validate_request, type(None)):

# as we are setting two different fields we are here setting as default False
# In case one of them are not defined
validate_body = False if type(validate_body) is type(None) else validate_body
validate_request = False if type(validate_request) is type(None) else validate_request

# If not type None but any other type it should explicitly invalidate the Spec
# Those fields should be only a boolean
if not isinstance(validate_body, bool) or not isinstance(validate_request, bool):
raise InvalidEventException(
self.relative_id,
"Unable to set Validator to RequestModel [{model}] on API method [{method}] for path [{path}] "
"ValidateBody and ValidateRequest must be a boolean type, strings or intrinsics are not supported.".format(
model=method_model, method=self.Method, path=self.Path
),
)

editor.add_request_validator_to_method(
path=self.Path,
method_name=self.Method,
validate_body=validate_body,
validate_request=validate_request,
)

if self.RequestParameters:

default_value = {"Required": False, "Caching": False}
Expand Down
56 changes: 56 additions & 0 deletions samtranslator/swagger/swagger.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class SwaggerEditor(object):
_X_APIGW_GATEWAY_RESPONSES = "x-amazon-apigateway-gateway-responses"
_X_APIGW_POLICY = "x-amazon-apigateway-policy"
_X_ANY_METHOD = "x-amazon-apigateway-any-method"
_X_APIGW_REQUEST_VALIDATOR = "x-amazon-apigateway-request-validators"
_CACHE_KEY_PARAMETERS = "cacheKeyParameters"
# https://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html
_ALL_HTTP_METHODS = ["OPTIONS", "GET", "HEAD", "POST", "PUT", "DELETE", "PATCH"]
Expand Down Expand Up @@ -781,6 +782,41 @@ def _set_method_apikey_handling(self, path, method_name, apikey_required):
if security != existing_security:
method_definition["security"] = security

def add_request_validator_to_method(self, path, method_name, validate_body=False, validate_request=False):
"""
Adds request model body parameter for this path/method.

:param string path: Path name
:param string method_name: Method name
:param bool validate_body: Add validator parameter on the body
:param bool validate_request: Validate request
"""

normalized_method_name = self._normalize_method_name(method_name)
validator_name = SwaggerEditor.get_validator_name(validate_body, validate_request)

# Creating validator
request_validator_definition = {
validator_name: {"validateRequestBody": validate_body, "validateRequestParameters": validate_request}
}
if not self._doc.get(self._X_APIGW_REQUEST_VALIDATOR):
self._doc[self._X_APIGW_REQUEST_VALIDATOR] = {}

if not self._doc[self._X_APIGW_REQUEST_VALIDATOR].get(validator_name):
# Adding only if the validator hasn't been defined already
self._doc[self._X_APIGW_REQUEST_VALIDATOR].update(request_validator_definition)

# It is possible that the method could have two definitions in a Fn::If block.
for method_definition in self.get_method_contents(self.get_path(path)[normalized_method_name]):
Copy link
Contributor

Choose a reason for hiding this comment

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

We have been moving away from this kind of access pattern and moving towards what is in this pr: https://github.com/aws/serverless-application-model/pull/2031/files

The reason is the normalization causes KeyErrors when methods like "any" are used, since that will get translated to x-amazon-apigateway-any-method. Can you match what is in the PR I linked? for method_definition in self.get_method_contents(method):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jfuss done!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jfuss thinking a bit more about this last change, it doesn't seems right to me:

for method_name, method in self.get_path(path).items():
            # It is possible that the method could have two definitions in a Fn::If block.
            for method_definition in self.get_method_contents(method):
                # If no integration given, then we don't need to process this

So, it would be something like:

        for method_name, method in self.get_path(path).items():
            # Adding it to only path  
            if method_name == normalized_method_name:
                for method_definition in self.get_method_contents(method):

right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

without checking the method_name with the normalized_method_name it would add this to all methods for the given path.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems reasonable to me. I am more comfortable with this since it is a new code path (unlikely to accidentally break existing customers for something we are unaware of).


# If no integration given, then we don't need to process this definition (could be AWS::NoValue)
if not self.method_definition_has_integration(method_definition):
continue

set_validator_to_method = {"x-amazon-apigateway-request-validator": validator_name}
# Setting validator to the given method
method_definition.update(set_validator_to_method)

def add_request_model_to_method(self, path, method_name, request_model):
"""
Adds request model body parameter for this path/method.
Expand Down Expand Up @@ -1265,6 +1301,26 @@ def get_path_without_trailing_slash(path):
# convert greedy paths to such as {greedy+}, {proxy+} to "*"
return re.sub(r"{([a-zA-Z0-9._-]+|[a-zA-Z0-9._-]+\+|proxy\+)}", "*", path)

@staticmethod
def get_validator_name(validate_body, validate_request):
"""
Get a readable path name to use as validator name

:param boolean validate_body: Boolean if validate body
:param boolean validate_request: Boolean if validate request
:return string: Normalized validator name
"""
if validate_body and validate_request:
return "FULL"

if validate_body and not validate_request:
return "BODY"

if not validate_body and validate_request:
return "REQUEST"

return "NO_VALIDATE"

@staticmethod
def _validate_list_property_is_resolved(property_list):
"""
Expand Down
102 changes: 102 additions & 0 deletions tests/swagger/test_swagger.py
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,108 @@ def test_must_add_body_parameter_to_method_openapi_required_true(self):
self.assertEqual(expected, editor.swagger["paths"]["/foo"]["get"]["requestBody"])


class TestSwaggerEditor_add_request_validator_to_method(TestCase):
def setUp(self):

self.original_swagger = {
"swagger": "2.0",
"paths": {
"/foo": {
"get": {
"x-amazon-apigateway-integration": {"test": "must have integration"},
"parameters": [{"test": "existing parameter"}],
}
}
},
}

self.editor = SwaggerEditor(self.original_swagger)

def test_must_add_validator_parameters_to_method_with_validators_true(self):

self.editor.add_request_validator_to_method("/foo", "get", True, True)
expected = {"FULL": {"validateRequestBody": True, "validateRequestParameters": True}}

self.assertEqual(expected, self.editor.swagger["x-amazon-apigateway-request-validators"])
self.assertEqual("FULL", self.editor.swagger["paths"]["/foo"]["get"]["x-amazon-apigateway-request-validator"])

def test_must_add_validator_parameters_to_method_with_validators_false(self):

self.editor.add_request_validator_to_method("/foo", "get", False, False)

expected = {"NO_VALIDATE": {"validateRequestBody": False, "validateRequestParameters": False}}

self.assertEqual(expected, self.editor.swagger["x-amazon-apigateway-request-validators"])
self.assertEqual(
"NO_VALIDATE", self.editor.swagger["paths"]["/foo"]["get"]["x-amazon-apigateway-request-validator"]
)

def test_must_add_validator_parameters_to_method_with_validators_mixing(self):

self.editor.add_request_validator_to_method("/foo", "get", True, False)

expected = {"BODY": {"validateRequestBody": True, "validateRequestParameters": False}}

self.assertEqual(expected, self.editor.swagger["x-amazon-apigateway-request-validators"])
self.assertEqual("BODY", self.editor.swagger["paths"]["/foo"]["get"]["x-amazon-apigateway-request-validator"])

def test_must_add_validator_parameters_to_method_and_not_duplicate(self):
self.original_swagger["paths"].update(
{
"/bar": {
"get": {
"x-amazon-apigateway-integration": {"test": "must have integration"},
"parameters": [{"test": "existing parameter"}],
}
},
"/foo-bar": {
"get": {
"x-amazon-apigateway-integration": {"test": "must have integration"},
"parameters": [{"test": "existing parameter"}],
}
},
}
)

editor = SwaggerEditor(self.original_swagger)

editor.add_request_validator_to_method("/foo", "get", True, True)
editor.add_request_validator_to_method("/bar", "get", True, True)
editor.add_request_validator_to_method("/foo-bar", "get", True, True)

expected = {"FULL": {"validateRequestBody": True, "validateRequestParameters": True}}

self.assertEqual(expected, editor.swagger["x-amazon-apigateway-request-validators"])
self.assertEqual("FULL", editor.swagger["paths"]["/foo"]["get"]["x-amazon-apigateway-request-validator"])
self.assertEqual("FULL", editor.swagger["paths"]["/bar"]["get"]["x-amazon-apigateway-request-validator"])
self.assertEqual("FULL", editor.swagger["paths"]["/foo-bar"]["get"]["x-amazon-apigateway-request-validator"])

self.assertEqual(1, len(editor.swagger["x-amazon-apigateway-request-validators"].keys()))

@parameterized.expand(
[
param(True, False, "BODY"),
param(True, True, "FULL"),
param(False, True, "REQUEST"),
param(False, False, "NO_VALIDATE"),
]
)
def test_must_normalize_validator_names(self, validate_body, validate_request, normalized_name):
normalized_validator_name_conversion = SwaggerEditor.get_validator_name(validate_body, validate_request)
self.assertEqual(normalized_validator_name_conversion, normalized_name)

def test_must_add_validator_parameters_to_method_with_validators_false_by_default(self):

self.editor.add_request_validator_to_method("/foo", "get")

expected = {"NO_VALIDATE": {"validateRequestBody": False, "validateRequestParameters": False}}

self.assertEqual(expected, self.editor.swagger["x-amazon-apigateway-request-validators"])
self.assertEqual(
"NO_VALIDATE", self.editor.swagger["paths"]["/foo"]["get"]["x-amazon-apigateway-request-validator"]
)


class TestSwaggerEditor_add_auth(TestCase):
def setUp(self):

Expand Down
157 changes: 157 additions & 0 deletions tests/translator/input/api_request_model_with_validator.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
Resources:
HtmlFunction:
Type: AWS::Serverless::Function
Properties:
CodeUri: s3://sam-demo-bucket/member_portal.zip
Handler: index.gethtml
Runtime: nodejs12.x
Events:
GetHtml:
Type: Api
Properties:
RestApiId: HtmlApi
Path: /
Method: get
RequestModel:
Model: User
Required: true
ValidateBody: true
ValidateRequest: true

HtmlFunctionNoValidation:
Type: AWS::Serverless::Function
Properties:
CodeUri: s3://sam-demo-bucket/member_portal.zip
Handler: index.gethtml
Runtime: nodejs12.x
Events:
GetHtml:
Type: Api
Properties:
RestApiId: HtmlApi
Path: /no-validation
Method: get
RequestModel:
Model: User
Required: true
ValidateBody: false
ValidateRequest: false

HtmlFunctionMixinValidation:
Type: AWS::Serverless::Function
Properties:
CodeUri: s3://sam-demo-bucket/member_portal.zip
Handler: index.gethtml
Runtime: nodejs12.x
Events:
GetHtml:
Type: Api
Properties:
RestApiId: HtmlApi
Path: /mixin
Method: get
RequestModel:
Model: User
Required: true
ValidateBody: true
ValidateRequest: false

HtmlFunctionOnlyBodyDefinition:
Type: AWS::Serverless::Function
Properties:
CodeUri: s3://sam-demo-bucket/member_portal.zip
Handler: index.gethtml
Runtime: nodejs12.x
Events:
GetHtml:
Type: Api
Properties:
RestApiId: HtmlApi
Path: /only-body-true
Method: get
RequestModel:
Model: User
Required: true
ValidateBody: true

HtmlFunctionOnlyRequestDefinition:
Type: AWS::Serverless::Function
Properties:
CodeUri: s3://sam-demo-bucket/member_portal.zip
Handler: index.gethtml
Runtime: nodejs12.x
Events:
GetHtml:
Type: Api
Properties:
RestApiId: HtmlApi
Path: /only-request-true
Method: get
RequestModel:
Model: User
Required: true
ValidateRequest: true

HtmlFunctionOnlyRequestDefinitionFalse:
Type: AWS::Serverless::Function
Properties:
CodeUri: s3://sam-demo-bucket/member_portal.zip
Handler: index.gethtml
Runtime: nodejs12.x
Events:
GetHtml:
Type: Api
Properties:
RestApiId: HtmlApi
Path: /only-request-false
Method: get
RequestModel:
Model: User
Required: true
ValidateRequest: false

HtmlFunctionOnlyBodyDefinitionFalse:
Type: AWS::Serverless::Function
Properties:
CodeUri: s3://sam-demo-bucket/member_portal.zip
Handler: index.gethtml
Runtime: nodejs12.x
Events:
GetHtml:
Type: Api
Properties:
RestApiId: HtmlApi
Path: /only-body-false
Method: get
RequestModel:
Model: User
Required: true
ValidateBody: false

HtmlFunctionNotDefinedValidation:
Type: AWS::Serverless::Function
Properties:
CodeUri: s3://sam-demo-bucket/member_portal.zip
Handler: index.gethtml
Runtime: nodejs12.x
Events:
GetHtml:
Type: Api
Properties:
RestApiId: HtmlApi
Path: /not-defined
Method: get
RequestModel:
Model: User
Required: true

HtmlApi:
Type: AWS::Serverless::Api
Properties:
StageName: Prod
Models:
User:
type: object
properties:
username:
type: string
Loading