aws / aws/serverless-application-model
transform() mutates the caller's template; transforming the same template twice fails
- Dominant language
- Python
- Stars
- 9.6k
- Forks
- 2.5k
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 7
Description
### Description
`transform()` edits the template it is given. `Translator.translate()` documents the opposite:
> :returns: a **copy of the template** with SAM resources replaced with the corresponding CloudFormation
What actually happens, measured on `main` (`ab5503a2`) by deep-copying the input, transforming, and comparing:
| template | caller's dict after `transform()` |
|---|---|
| bare function | unchanged |
| function + `Globals` | **`Globals` section deleted; merged `Timeout` written into the resource's `Properties`** |
| function + `Api` event | **function properties rewritten in place** |
| function + explicit `AWS::Serverless::Api` | **generated `DefinitionBody` written into the `Api` resource** |
| function + explicit `AWS::Serverless::HttpApi` | **generated `DefinitionBody` written into the `Api` resource** |
| explicit `Api` alone, state machine | unchanged |
The consequence is worse than the mutation itself. Because the first transform leaves its own generated `DefinitionBody` behind in the caller's template, **transforming the same object a second time fails**:
```
InvalidDocumentException: Event with id [Api] is invalid.
API method "get" defined multiple times for path "/x".
```
This hits any caller that holds on to the template — transforming once per region or partition, tooling that shows the user their original template after transforming, or anything that simply calls `transform()` twice. The error message points at the user's event definition, which is not where the problem is.
I checked for cross-invocation state leakage as well (transform template A, then B, and compare B against a clean run): that is clean. The damage is confined to the object passed in.
### Steps to reproduce
```python
import copy
from unittest.mock import MagicMock, patch
from samtranslator.translator.transform import transform
template = {
"Transform": "AWS::Serverless-2016-10-31",
"Resources": {
"Fn": {
"Type": "AWS::Serverless::Function",
"Properties": {
"CodeUri": "s3://bucket/key", "Handler": "index.handler", "Runtime": "python3.11",
"Events": {"Api": {"Type": "Api", "Properties": {
"Path": "/x", "Method": "get", "RestApiId": {"Ref": "Api"}}}},
},
},
"Api": {"Type": "AWS::Serverless::Api", "Properties": {"StageName": "prod"}},
},
}
before = copy.deepcopy(template)
loader = MagicMock()
loader.load.return_value = {}
with patch("boto3.session.Session.region_name", "us-east-1"):
transform(template, {}, loader)
print("input mutated:", template != before) # True
transform(template, {}, loader) # raises InvalidDocumentException
```
### Observed result
`input mutated: True`, then `InvalidDocumentException: API method "get" defined multiple times for path "/x"` on the second call.
### Expected result
The caller's template is left as it was, and transforming the same template twice produces the same output both times.
### Possible fix
Take a copy at the `transform()` boundary.
One constraint worth recording, because it is not obvious and it rules out the tempting placement: the copy has to be taken **before** `to_py27_compatible_template()` runs. That function installs `Py27Dict`/`Py27UniStr` wrappers whose hash-ordering state logical ID generation depends on, and `copy.deepcopy` does not preserve it. I first put the copy at the top of `Translator.translate()` — the method whose docstring promises a copy — and `tests/translator/test_translator.py` caught it immediately: `test_transform_feature_toggle_0_feature_toggle_api_open_api_version_override` fails there because generated logical IDs change. Copying earlier, while the template is still plain dicts and strings, avoids that entirely.
Cost is negligible: on a 246 KiB template with 200 functions, the copy is 3.2 ms against a 1924 ms transform, or 0.2%.
Note this fixes the public `transform()` entry point. `Translator.translate()` called directly still mutates its argument, so its docstring is still inaccurate for that path; fixing it there needs `__deepcopy__` support on the Py27 types, which is a larger change and I have left it alone.
### Additional environment details
- SAM translator `main` @ `ab5503a2`, installed with `pip install -e .`
- Python 3.12, macOS 15
Contributor guide
Research direction
Start at the public transform() entry point and inspect its ordering relative to to_py27_compatible_template(). Run the reproduction and tests/translator/test_translator.py, especially test_transform_feature_toggle_0_feature_toggle_api_open_api_version_override. Done means the input remains unchanged and transforming the same template twice produces the same output without changing generated logical IDs.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- aws, python
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100