From f4f081b3abc355623b51b06ee76139e516cde5d3 Mon Sep 17 00:00:00 2001 From: Roger Zhang Date: Fri, 31 Jul 2026 16:50:35 -0700 Subject: [PATCH 1/2] fix: publish new Lambda version when AutoPublishAliasAllProperties property references a changed parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes aws/serverless-application-model#3820. With `AutoPublishAliasAllProperties: true`, a property whose value comes from a template parameter — e.g. an Environment variable set to `!Ref SomeParam` — did not trigger a new Lambda version when the parameter value changed, so `sam deploy` published nothing. The version's logical id is a hash of the properties that should trigger a new version. The non-AllProperties (CodeUri) path resolves parameter references before hashing, with a comment explaining exactly why: an unresolved `{"Ref": "SomeParam"}` hashes identically regardless of the value supplied. The AllProperties path skipped that step and hashed the raw resource dict, so `{"Ref": "TestParameter"}` produced the same id whether the override was `2` or `3`. Resolve template parameter references on the AllProperties dict too. Pseudo parameters (AWS::Region, AWS::Partition, ...) are excluded: they are present in the resolver's parameter map but do not represent a template change, and resolving them would rewrite `Fn::Sub` strings that reference them and shift the version id of existing, unchanged templates. Excluding them keeps this a no-op for any template that does not reference a real parameter in a version-tracked property, so existing version ids are preserved — verified by the full translator suite passing unchanged (2174 tests). Testing: - New regression tests in test_function_resources.py assert that changing a referenced parameter value yields a different version logical id, and that an unchanged value is stable. Confirmed both fail against the pre-fix code. - Full tests/translator suite passes (2229 tests), so no golden-file version id changed — confirming backward compatibility, including the pseudo-parameter fixture (function_with_alias_and_all_properties_property) that a naive fix regressed. --- samtranslator/model/sam_resources.py | 14 ++++- tests/translator/test_function_resources.py | 63 +++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/samtranslator/model/sam_resources.py b/samtranslator/model/sam_resources.py index f9bcdd76b..bbe43fdb4 100644 --- a/samtranslator/model/sam_resources.py +++ b/samtranslator/model/sam_resources.py @@ -1202,7 +1202,19 @@ def _construct_version( # noqa: PLR0912 if publish_lambda_version: properties.update({layer_logical_id: layer_properties}) - logical_dict = properties + # Resolve template parameter references for the same reason the CodeUri path above + # does: an unresolved `{"Ref": "SomeParameter"}` hashes identically no matter what + # value is supplied, so a parameter-driven property change would not produce a new + # version logical id and no version would be published. + # + # Pseudo parameters (AWS::Region, AWS::Partition, ...) are deliberately excluded. + # They are present in the resolver's parameter map but their values do not represent + # a template change, and resolving them would rewrite `Fn::Sub` strings that + # reference them -- shifting the version logical id of existing templates that have + # not changed at all. + logical_dict = IntrinsicsResolver( + {key: value for key, value in intrinsics_resolver.parameters.items() if not key.startswith("AWS::")} + ).resolve_parameter_refs(properties) else: with suppress(AttributeError, UnboundLocalError): logical_dict = code_dict.copy() diff --git a/tests/translator/test_function_resources.py b/tests/translator/test_function_resources.py index 73afe8b17..52ada56f5 100644 --- a/tests/translator/test_function_resources.py +++ b/tests/translator/test_function_resources.py @@ -1,3 +1,4 @@ +import json from unittest import TestCase from unittest.mock import Mock, call, patch @@ -7,6 +8,8 @@ from samtranslator.model.preferences.deployment_preference import DeploymentPreference from samtranslator.model.sam_resources import SamFunction from samtranslator.model.update_policy import UpdatePolicy +from samtranslator.parser.parser import Parser +from samtranslator.translator.translator import Translator class TestVersionsAndAliases(TestCase): @@ -893,3 +896,63 @@ def test_must_not_break_support(self): self.assertEqual(func.referable_properties["Version"], "AWS::Lambda::Version") self.assertEqual(func.referable_properties["DestinationTopic"], "AWS::SNS::Topic") self.assertEqual(func.referable_properties["DestinationQueue"], "AWS::SQS::Queue") + + +class TestAutoPublishAliasAllPropertiesParameterHash(TestCase): + """Regression tests for GitHub issue #3820. + + With AutoPublishAliasAllProperties, a property whose value comes from a + template parameter (e.g. an Environment variable set to `!Ref SomeParam`) + must produce a different Lambda Version logical id when the parameter value + changes, otherwise `sam deploy` publishes no new version. Pseudo parameters + (AWS::Region, AWS::Partition, ...) must NOT influence the id, since they do + not represent a template change. + """ + + def _version_logical_ids(self, parameter_values): + template = { + "AWSTemplateFormatVersion": "2010-09-09", + "Transform": "AWS::Serverless-2016-10-31", + "Parameters": {"TestParameter": {"Type": "String"}}, + "Resources": { + "HelloWorldFunction": { + "Type": "AWS::Serverless::Function", + "Properties": { + "InlineCode": "def handler(event, context): pass", + "Handler": "app.handler", + "Runtime": "python3.12", + "Architectures": ["x86_64"], + "AutoPublishAlias": "live", + "AutoPublishAliasAllProperties": True, + "Environment": {"Variables": {"TEST_PARAMETER": {"Ref": "TestParameter"}}}, + }, + } + }, + } + output = Translator({}, Parser()).translate(json.loads(json.dumps(template)), parameter_values=parameter_values) + return sorted( + logical_id + for logical_id, resource in output["Resources"].items() + if resource["Type"] == "AWS::Lambda::Version" + ) + + @patch("boto3.session.Session.region_name", "us-east-1") + def test_parameter_value_change_produces_new_version(self): + ids_a = self._version_logical_ids({"TestParameter": "value-a"}) + ids_b = self._version_logical_ids({"TestParameter": "value-b"}) + + self.assertEqual(len(ids_a), 1) + self.assertEqual(len(ids_b), 1) + self.assertNotEqual( + ids_a, + ids_b, + "AutoPublishAliasAllProperties must publish a new version when a " + "referenced parameter value changes (issue #3820)", + ) + + @patch("boto3.session.Session.region_name", "us-east-1") + def test_same_parameter_value_is_stable(self): + self.assertEqual( + self._version_logical_ids({"TestParameter": "value-a"}), + self._version_logical_ids({"TestParameter": "value-a"}), + ) From 49f76536a1b4e953eab9224af715d74805284c4d Mon Sep 17 00:00:00 2001 From: Roger Zhang Date: Fri, 31 Jul 2026 17:06:03 -0700 Subject: [PATCH 2/2] fix: resolve parameter refs against a copy so values do not leak into output Addresses review feedback on #3965. `resolve_parameter_refs` mutates the dict it is given -- `_traverse_dict` assigns back via `input_dict[key] = ...` -- and the values reachable from `_generate_resource_dict()` are the live objects from the user's template (`properties["Environment"] is function.Environment`). Resolving in place therefore inlined parameter values into the resources the translator actually emits, replacing `{"Ref": "TestParameter"}` with the literal on the AWS::Lambda::Function. Confirmed a NoEcho parameter value appeared as plaintext in the transformed template. The layer branch is affected the same way and is worse: `layer_properties` comes from ResourceResolver over the output template, so resolving could rewrite a different resource's emitted properties. Resolve against `copy.deepcopy(properties)` instead. This is what the resolver's own docstring warns about: "Don't pass this dictionary directly into transform's output because it changes the template structure by inlining parameter values." Adds two tests that a version-logical-id assertion cannot cover: the emitted function still carries `{"Ref": "TestParameter"}`, and a NoEcho value never appears anywhere in the output. Both fail without the deepcopy. Testing: 4 tests in the new class pass, and both new ones confirmed to fail against the pre-deepcopy code. Full tests/translator suite passes (2425). ruff and black clean. --- samtranslator/model/sam_resources.py | 11 +++++- tests/translator/test_function_resources.py | 37 +++++++++++++++++++-- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/samtranslator/model/sam_resources.py b/samtranslator/model/sam_resources.py index bbe43fdb4..f9bb00b0e 100644 --- a/samtranslator/model/sam_resources.py +++ b/samtranslator/model/sam_resources.py @@ -1212,9 +1212,18 @@ def _construct_version( # noqa: PLR0912 # a template change, and resolving them would rewrite `Fn::Sub` strings that # reference them -- shifting the version logical id of existing templates that have # not changed at all. + # + # Resolve against a deep copy. `resolve_parameter_refs` mutates the dict it is given + # (`_traverse_dict` assigns back into `input_dict`), and the values reachable from + # `_generate_resource_dict()` are the live objects from the user's template -- as are + # the layer properties above, which come from the output template via + # ResourceResolver. Resolving in place would inline parameter values into the emitted + # resources, replacing `{"Ref": "Param"}` with the literal (leaking NoEcho values and + # dropping the parameter reference from the deployed resource). The resolver's own + # docstring warns against passing its result into the transform's output. logical_dict = IntrinsicsResolver( {key: value for key, value in intrinsics_resolver.parameters.items() if not key.startswith("AWS::")} - ).resolve_parameter_refs(properties) + ).resolve_parameter_refs(copy.deepcopy(properties)) else: with suppress(AttributeError, UnboundLocalError): logical_dict = code_dict.copy() diff --git a/tests/translator/test_function_resources.py b/tests/translator/test_function_resources.py index 52ada56f5..881ee4cbe 100644 --- a/tests/translator/test_function_resources.py +++ b/tests/translator/test_function_resources.py @@ -909,11 +909,14 @@ class TestAutoPublishAliasAllPropertiesParameterHash(TestCase): not represent a template change. """ - def _version_logical_ids(self, parameter_values): + def _translate(self, parameter_values, no_echo=False): + parameter = {"Type": "String"} + if no_echo: + parameter["NoEcho"] = True template = { "AWSTemplateFormatVersion": "2010-09-09", "Transform": "AWS::Serverless-2016-10-31", - "Parameters": {"TestParameter": {"Type": "String"}}, + "Parameters": {"TestParameter": parameter}, "Resources": { "HelloWorldFunction": { "Type": "AWS::Serverless::Function", @@ -929,7 +932,10 @@ def _version_logical_ids(self, parameter_values): } }, } - output = Translator({}, Parser()).translate(json.loads(json.dumps(template)), parameter_values=parameter_values) + return Translator({}, Parser()).translate(json.loads(json.dumps(template)), parameter_values=parameter_values) + + def _version_logical_ids(self, parameter_values): + output = self._translate(parameter_values) return sorted( logical_id for logical_id, resource in output["Resources"].items() @@ -956,3 +962,28 @@ def test_same_parameter_value_is_stable(self): self._version_logical_ids({"TestParameter": "value-a"}), self._version_logical_ids({"TestParameter": "value-a"}), ) + + @patch("boto3.session.Session.region_name", "us-east-1") + def test_parameter_reference_is_preserved_in_output_template(self): + """Resolving parameter refs for the hash must not leak into the emitted template. + + `resolve_parameter_refs` mutates the dict it is given, and the values reachable + from `_generate_resource_dict()` are the live objects from the user's template. + Resolving in place would replace `{"Ref": "TestParameter"}` with the literal + value on the emitted AWS::Lambda::Function. A version-logical-id assertion + cannot see that, so assert on the output resource directly. + """ + output = self._translate({"TestParameter": "some-value"}) + + self.assertEqual( + output["Resources"]["HelloWorldFunction"]["Properties"]["Environment"]["Variables"]["TEST_PARAMETER"], + {"Ref": "TestParameter"}, + ) + + @patch("boto3.session.Session.region_name", "us-east-1") + def test_no_echo_parameter_value_is_not_inlined(self): + """A NoEcho parameter value must never appear as plaintext in the output.""" + secret = "super-secret-value" + output = self._translate({"TestParameter": secret}, no_echo=True) + + self.assertNotIn(secret, json.dumps(output))