Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
4cdc686
Add third party py27hash code
hawflau Sep 18, 2021
cc06ac0
Add Py27UniStr and unit tests
hawflau Sep 18, 2021
52d0e7a
Add py27hash_fix utils and tests
hawflau Sep 19, 2021
e066921
Add to_py27_compatible_template and tests
hawflau Sep 25, 2021
71e6fbd
Apply py27hash fix to wherever it is needed
hawflau Sep 28, 2021
94cf884
Apply py27hash fix, all tests pass except api_with_any_method_in_swagger
hawflau Sep 30, 2021
687fd60
apply py27hash fix in openapi + run black
torresxb1 Oct 12, 2021
754922c
remove py27 testing
torresxb1 Oct 12, 2021
7503e2f
remove other py27 references
torresxb1 Oct 12, 2021
be860a2
black fixes
torresxb1 Oct 12, 2021
e21ed03
fixes/typos
torresxb1 Oct 13, 2021
18ecddb
remove py27 from tox.ini
torresxb1 Oct 26, 2021
d866388
refactoring
torresxb1 Oct 26, 2021
ff9ef73
third party notice
torresxb1 Oct 26, 2021
61e3d99
Merge branch 'develop' of github.com:aws/serverless-application-model…
torresxb1 Oct 26, 2021
9cfc04a
black
torresxb1 Oct 26, 2021
ab54931
Merge branch 'develop' into py27hash-20210918
hawflau Nov 2, 2021
3aa24c6
Merge branch 'develop' into py27hash-20210918
hawflau Nov 3, 2021
80451e4
Fix py27hash fix to deal with null events
hawflau Nov 6, 2021
fe74979
Fix Py27UniStr repr for unicode literals
hawflau Nov 10, 2021
dd6c992
black reformat
hawflau Nov 10, 2021
621df32
Update _template_has_api_resource to check data type more defensively
hawflau Nov 10, 2021
1defc47
Apply py27Dict in _get_authorizers
hawflau Nov 14, 2021
46e0421
Apply Py27Dict to authorizers and gateway responses which will go int…
hawflau Nov 15, 2021
8e09c99
Update to_py27_compatible_template to handle parameter_values; Add Py…
hawflau Nov 17, 2021
59c4062
Merge branch 'develop' into py27hash-20210918
hawflau Nov 17, 2021
bb58ec5
Rename _convert_to_py27_dict to _convert_to_py27_type
hawflau Nov 18, 2021
41f7857
Apply Py27UniStr to path param name
hawflau Nov 23, 2021
b9dc3ea
Handle HttpApi resource under to_py27_compatible_template
hawflau Dec 3, 2021
c79cb44
Fix InvalidDocumentException to not sort different exceptions
hawflau Dec 3, 2021
596f93d
black reformat
hawflau Dec 5, 2021
62b4953
Merge branch 'develop' into py27hash-20210918
hawflau Jan 3, 2022
9181397
Remove unnecessary test files
hawflau Jan 3, 2022
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
11 changes: 4 additions & 7 deletions DEVELOPMENT_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,8 @@ Environment Setup
-----------------
### 1. Install Python Versions

Our officially supported Python versions are 2.7, 3.6, 3.7 and 3.8. Follow the idioms from this [excellent cheatsheet](http://python-future.org/compatible_idioms.html) to
make sure your code is compatible with both Python 2.7 and 3 (>=3.6) versions.
Our CI/CD pipeline is setup to run unit tests against both Python 2.7 and 3 versions. So make sure you test it with both versions before sending a Pull Request.
Our officially supported Python versions are 3.6, 3.7 and 3.8.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we're deprecating python 2, and this PR removes tests from running in python 2, I decided to also remove all other references to it. Let me know if I should undo the changes to documentation in case we want to do them more formally later on.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should update our setup.py to constraint to specific py versions too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did remove python 2.7 from setup.py in this PR, but I wasn't sure about Programming Language :: Python so that one is still in the file. Should I also remove it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking more like this: https://github.com/aws/aws-sam-cli/blob/develop/setup.py#L57 Maybe outside of this PR, but we don't actually constrain what python version are supported. So we can make the code none compatible but it could still be install-able through pip in python2.7

Our CI/CD pipeline is setup to run unit tests against Python 3 versions. Make sure you test it before sending a Pull Request.
See [Unit testing with multiple Python versions](#unit-testing-with-multiple-python-versions).

[pyenv](https://github.com/pyenv/pyenv) is a great tool to
Expand All @@ -41,12 +40,11 @@ easily setup multiple Python versions. For
1. Install PyEnv -
`curl -L https://github.com/pyenv/pyenv-installer/raw/master/bin/pyenv-installer | bash`
1. Restart shell so the path changes take effect - `exec $SHELL`
1. `pyenv install 2.7.17`
1. `pyenv install 3.6.12`
1. `pyenv install 3.7.9`
1. `pyenv install 3.8.6`
1. Make Python versions available in the project:
`pyenv local 2.7.17 3.6.12 3.7.9 3.8.6`
`pyenv local 3.6.12 3.7.9 3.8.6`

Note: also make sure the following lines were written into your `.bashrc` (or `.zshrc`, depending on which shell you are using):
```
Expand Down Expand Up @@ -117,11 +115,10 @@ Running Tests
### Unit testing with one Python version

If you're trying to do a quick run, it's ok to use the current python version. Run `make pr`.
If you're using Python2.7, you can run `make pr2.7` instead.

### Unit testing with multiple Python versions

Currently, our officially supported Python versions are 2.7, 3.6, 3.7 and 3.8. For the most
Currently, our officially supported Python versions are 3.6, 3.7 and 3.8. For the most
part, code that works in Python3.6 will work in Python3.7 and Python3.8. You only run into problems if you are
trying to use features released in a higher version (for example features introduced into Python3.7
will not work in Python3.6). If you want to test in many versions, you can create a virtualenv for
Expand Down
3 changes: 0 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ dev: test
# Verifications to run before sending a pull request
pr: black-check init dev

# Verifications to run before sending a pull request, skipping black check because black requires Python 3.6+
pr2.7: init dev

define HELP_MESSAGE

Usage: $ make [TARGETS]
Expand Down
2 changes: 0 additions & 2 deletions appveyor-integration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ image: Ubuntu

environment:
matrix:
- TOXENV: py27
PYTHON_VERSION: '2.7'
- TOXENV: py36
PYTHON_VERSION: '3.6'
- TOXENV: py37
Expand Down
2 changes: 0 additions & 2 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ image: Ubuntu

environment:
matrix:
- TOXENV: py27
PYTHON_VERSION: '2.7'
- TOXENV: py36
PYTHON_VERSION: '3.6'
- TOXENV: py37
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.black]
line-length = 120
target_version = ['py27', 'py37', 'py36', 'py38']
target_version = ['py37', 'py36', 'py38']
exclude = '''

(
Expand Down
15 changes: 8 additions & 7 deletions samtranslator/intrinsics/actions.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import re

from six import string_types
from samtranslator.utils.py27hash_fix import Py27UniStr
from samtranslator.model.exceptions import InvalidTemplateException, InvalidDocumentException


Expand Down Expand Up @@ -375,13 +376,13 @@ def handler_method(full_ref, ref_value):

# Find all the pattern, and call the handler to decide how to substitute them.
# Do the substitution and return the final text
return re.sub(
ref_pattern,
# Pass the handler entire string ${logicalId.property} as first parameter and "logicalId.property"
# as second parameter. Return value will be substituted
lambda match: handler_method(match.group(0), match.group(1)),
text,
)
# NOTE: in order to make sure Py27UniStr strings won't be converted to plain string,
# we need to iterate through each match and do the replacement
substituted = text
for match in re.finditer(ref_pattern, text):
sub_value = handler_method(match.group(0), match.group(1))
substituted = substituted.replace(match.group(0), sub_value, 1)
return substituted


class GetAttAction(Action):
Expand Down
27 changes: 19 additions & 8 deletions samtranslator/model/api/api_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from samtranslator.translator import logical_id_generator
from samtranslator.translator.arn_generator import ArnGenerator
from samtranslator.model.tags.resource_tagging import get_tag_list
from samtranslator.utils.py27hash_fix import Py27Dict, Py27UniStr

LOG = logging.getLogger(__name__)

Expand Down Expand Up @@ -317,7 +318,18 @@ def _construct_body_s3_dict(self):
"'s3://bucket/key' with optional versionId query parameter.",
)

body_s3 = {"Bucket": s3_pointer["Bucket"], "Key": s3_pointer["Key"]}
if isinstance(self.definition_uri, Py27UniStr):
# self.defintion_uri is a Py27UniStr instance if it is defined in the template
# we need to preserve the Py27UniStr type
s3_pointer["Bucket"] = Py27UniStr(s3_pointer["Bucket"])
s3_pointer["Key"] = Py27UniStr(s3_pointer["Key"])
if "Version" in s3_pointer:
s3_pointer["Version"] = Py27UniStr(s3_pointer["Version"])

# Construct body_s3 as py27 dict
body_s3 = Py27Dict()
body_s3["Bucket"] = s3_pointer["Bucket"]
body_s3["Key"] = s3_pointer["Key"]
if "Version" in s3_pointer:
body_s3["Version"] = s3_pointer["Version"]
return body_s3
Expand Down Expand Up @@ -952,12 +964,12 @@ def _openapi_postprocess(self, definition_body):
SwaggerEditor.get_openapi_version_3_regex(), self.open_api_version
):
if definition_body.get("securityDefinitions"):
components = definition_body.get("components", {})
components = definition_body.get("components", Py27Dict())
components["securitySchemes"] = definition_body["securityDefinitions"]
definition_body["components"] = components
del definition_body["securityDefinitions"]
if definition_body.get("definitions"):
components = definition_body.get("components", {})
components = definition_body.get("components", Py27Dict())
components["schemas"] = definition_body["definitions"]
definition_body["components"] = components
del definition_body["definitions"]
Expand All @@ -981,11 +993,10 @@ def _openapi_postprocess(self, definition_body):
):
headers = definition_body["paths"][path]["options"][field]["200"]["headers"]
for header in headers.keys():
header_value = {
"schema": definition_body["paths"][path]["options"][field]["200"][
"headers"
][header]
}
header_value = Py27Dict()
header_value["schema"] = definition_body["paths"][path]["options"][field][
"200"
]["headers"][header]
definition_body["paths"][path]["options"][field]["200"]["headers"][
header
] = header_value
Expand Down
86 changes: 59 additions & 27 deletions samtranslator/model/apigateway.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from samtranslator.model.intrinsics import ref, fnSub
from samtranslator.translator import logical_id_generator
from samtranslator.translator.arn_generator import ArnGenerator
from samtranslator.utils.py27hash_fix import Py27Dict, Py27UniStr


class ApiGatewayRestApi(Resource):
Expand Down Expand Up @@ -133,10 +134,10 @@ def __init__(self, api_logical_id=None, response_parameters=None, response_templ
self.status_code = status_code_str

def generate_swagger(self):
swagger = {
"responseParameters": self._add_prefixes(self.response_parameters),
"responseTemplates": self.response_templates,
}
# Applying Py27Dict here as this goes into swagger
swagger = Py27Dict()
swagger["responseParameters"] = self._add_prefixes(self.response_parameters)
swagger["responseTemplates"] = self.response_templates

# Prevent "null" being written.
if self.status_code:
Expand All @@ -146,13 +147,26 @@ def generate_swagger(self):

def _add_prefixes(self, response_parameters):
GATEWAY_RESPONSE_PREFIX = "gatewayresponse."
prefixed_parameters = {}
# applying Py27Dict as this is part of swagger
prefixed_parameters = Py27Dict()
for key, value in response_parameters.get("Headers", {}).items():
prefixed_parameters[GATEWAY_RESPONSE_PREFIX + "header." + key] = value
param_key = GATEWAY_RESPONSE_PREFIX + "header." + key
if isinstance(key, Py27UniStr):
# if key is from template, we need to convert param_key to Py27UniStr
param_key = Py27UniStr(param_key)
prefixed_parameters[param_key] = value

for key, value in response_parameters.get("Paths", {}).items():
prefixed_parameters[GATEWAY_RESPONSE_PREFIX + "path." + key] = value
param_key = GATEWAY_RESPONSE_PREFIX + "path." + key
if isinstance(key, Py27UniStr):
param_key = Py27UniStr(param_key)
prefixed_parameters[param_key] = value

for key, value in response_parameters.get("QueryStrings", {}).items():
prefixed_parameters[GATEWAY_RESPONSE_PREFIX + "querystring." + key] = value
param_key = GATEWAY_RESPONSE_PREFIX + "querystring." + key
if isinstance(key, Py27UniStr):
param_key = Py27UniStr(param_key)
prefixed_parameters[param_key] = value

return prefixed_parameters

Expand Down Expand Up @@ -287,21 +301,20 @@ def _is_missing_identity_source(self, identity):
def generate_swagger(self):
authorizer_type = self._get_type()
APIGATEWAY_AUTHORIZER_KEY = "x-amazon-apigateway-authorizer"
swagger = {
"type": "apiKey",
"name": self._get_swagger_header_name(),
"in": "header",
"x-amazon-apigateway-authtype": self._get_swagger_authtype(),
}
swagger = Py27Dict()
swagger["type"] = "apiKey"
swagger["name"] = self._get_swagger_header_name()
swagger["in"] = "header"
swagger["x-amazon-apigateway-authtype"] = self._get_swagger_authtype()

if authorizer_type == "COGNITO_USER_POOLS":
swagger[APIGATEWAY_AUTHORIZER_KEY] = {
"type": self._get_swagger_authorizer_type(),
"providerARNs": self._get_user_pool_arn_array(),
}
authorizer_dict = Py27Dict()
authorizer_dict["type"] = self._get_swagger_authorizer_type()
authorizer_dict["providerARNs"] = self._get_user_pool_arn_array()
swagger[APIGATEWAY_AUTHORIZER_KEY] = authorizer_dict

elif authorizer_type == "LAMBDA":
swagger[APIGATEWAY_AUTHORIZER_KEY] = {"type": self._get_swagger_authorizer_type()}
swagger[APIGATEWAY_AUTHORIZER_KEY] = Py27Dict({"type": self._get_swagger_authorizer_type()})
partition = ArnGenerator.get_partition_name()
resource = "lambda:path/2015-03-31/functions/${__FunctionArn__}/invocations"
authorizer_uri = fnSub(
Expand Down Expand Up @@ -347,20 +360,36 @@ def _get_identity_source(self):
identity_source_context = []

if self.identity.get("Headers"):
identity_source_headers = list(map(lambda h: "method.request.header." + h, self.identity.get("Headers")))
identity_source_headers = []
for header in self.identity.get("Headers"):
item = "method.request.header." + header
if isinstance(header, Py27UniStr):
item = Py27UniStr(item)
identity_source_headers.append(item)

if self.identity.get("QueryStrings"):
identity_source_query_strings = list(
map(lambda qs: "method.request.querystring." + qs, self.identity.get("QueryStrings"))
)
identity_source_query_strings = []
for qs in self.identity.get("QueryStrings"):
item = "method.request.querystring." + qs
if isinstance(qs, Py27UniStr):
item = Py27UniStr(item)
identity_source_query_strings.append(item)

if self.identity.get("StageVariables"):
identity_source_stage_variables = list(
map(lambda sv: "stageVariables." + sv, self.identity.get("StageVariables"))
)
identity_source_stage_variables = []
for sv in self.identity.get("StageVariables"):
item = "stageVariables." + sv
if isinstance(sv, Py27UniStr):
item = Py27UniStr(item)
identity_source_stage_variables.append(item)

if self.identity.get("Context"):
identity_source_context = list(map(lambda c: "context." + c, self.identity.get("Context")))
identity_source_context = []
for c in self.identity.get("Context"):
Copy link
Contributor

Choose a reason for hiding this comment

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

I see a pattern across these functions. Can we abstract this into a function?

item = "context." + c
if isinstance(c, Py27UniStr):
item = Py27UniStr(item)
identity_source_context.append(item)

identity_source_array = (
identity_source_headers
Expand All @@ -369,6 +398,9 @@ def _get_identity_source(self):
+ identity_source_context
)
identity_source = ", ".join(identity_source_array)
if any(isinstance(i, Py27UniStr) for i in identity_source_array):
# Convert identity_source to Py27UniStr if any part of it is Py27UniStr
identity_source = Py27UniStr(identity_source)

return identity_source

Expand Down
11 changes: 9 additions & 2 deletions samtranslator/model/eventsources/push.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from samtranslator.model.exceptions import InvalidEventException, InvalidResourceException
from samtranslator.swagger.swagger import SwaggerEditor
from samtranslator.open_api.open_api import OpenApiEditor
from samtranslator.utils.py27hash_fix import Py27Dict, Py27UniStr

CONDITION = "Condition"

Expand Down Expand Up @@ -665,13 +666,16 @@ def _add_swagger_integration(self, api, function, intrinsics_resolver):

function_arn = function.get_runtime_attr("arn")
partition = ArnGenerator.get_partition_name()
uri = fnSub(
arn = (
"arn:"
+ partition
+ ":apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/"
+ make_shorthand(function_arn)
+ "/invocations"
)
if function_arn.get("Fn::GetAtt") and isinstance(function_arn["Fn::GetAtt"][0], Py27UniStr):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why only [0]? What happens if Fn::GetAtt:[] is defined?

Copy link
Contributor

Choose a reason for hiding this comment

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

function_arn is always of the form {"Fn::GetAtt": ["<function_logical_id>", "Arn"]} as it's created from function.get_runtime_attr("arn"). We only want to check if the function logical id is a Py27UniStr instance.

arn = Py27UniStr(arn)
uri = Py27Dict(fnSub(arn))

editor = SwaggerEditor(swagger_body)

Expand Down Expand Up @@ -1163,11 +1167,14 @@ def _add_openapi_integration(self, api, function, manage_swagger=False):
return

function_arn = function.get_runtime_attr("arn")
uri = fnSub(
arn = (
"arn:${AWS::Partition}:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/"
+ make_shorthand(function_arn)
+ "/invocations"
)
if function_arn.get("Fn::GetAtt") and isinstance(function_arn["Fn::GetAtt"][0], Py27UniStr):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why only GetAtt?

Copy link
Contributor

Choose a reason for hiding this comment

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

function_arn is always of the form {"Fn::GetAtt": ["<function_logical_id>", "Arn"]} as it's created from function.get_runtime_attr("arn"). So we only need to check Fn::GetAtt.

Copy link
Contributor

Choose a reason for hiding this comment

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

That is a big assumption. It may be true today but a change could impact this code path unknowingly. I'll trust the team this is sufficient but intrinsic functions have bitten SAM so many times that checking like this seems risk. We should find ways to encapsulate this logic in some manner to reduce the intrinsic function reading spread throughout the code base.

arn = Py27UniStr(arn)
uri = Py27Dict(fnSub(arn))

editor = OpenApiEditor(open_api_body)

Expand Down
Loading