Skip to content

Commit a405a48

Browse files
committed
fix(cfn-lang-ext): derive Mapping-prefix list and artifact-property set from canonical dict
Two consumers of PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES were left out of sync when this PR began emitting Mappings for newly-supported resource types (Application, Glue, EB, AppSync, CFN Module/ResourceVersion) and for dotted-leaf entries (Code.ImageUri): - samcli/lib/cfn_language_extensions/utils.py — _SAM_GENERATED_MAPPING_PREFIXES was a hand-maintained tuple covering only the pre-PR set, so is_sam_generated_mapping returned False for SAMLocation*, SAMScriptLocation*, SAMSourceBundle*, SAMModulePackage*, SAMSchemaHandlerPackage*. That made _update_sam_mappings_relative_paths skip these mappings on template move (paths weren't adjusted) and prevented _create_deploy_error from wrapping CFN "key not found" errors as MissingMappingKeyError. - samcli/commands/_utils/template.py — _ARTIFACT_PATH_PROPERTIES was built from the dotted form ("Command.ScriptLocation") but the SAM-generated Mapping value-dicts use the leaf ("ScriptLocation") as the key. So the inner property check rejected leaf names and Glue script paths never got their relative paths adjusted on template move. Both lists now derive from PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES at module import (mirroring the single-source-of-truth approach used for the dict itself in models.py). Adds two sync tests so future drift fails loudly. Addresses second review comment on PR #9009.
1 parent 054251e commit a405a48

4 files changed

Lines changed: 125 additions & 18 deletions

File tree

samcli/commands/_utils/template.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,17 @@
3333
# Artifact path property names derived from PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES
3434
# so the two lists cannot drift. Used by _update_sam_mappings_relative_paths to
3535
# identify which SAM-generated Mapping values are local paths needing adjustment.
36+
#
37+
# Includes both the dotted form (e.g. "Command.ScriptLocation") and its leaf
38+
# (e.g. "ScriptLocation"), because SAM-generated Mapping value-dicts are keyed
39+
# by the leaf segment so the third argument of Fn::FindInMap stays alphanumeric
40+
# (see _compute_mapping_name and _generate_artifact_mappings in
41+
# samcli/lib/package/language_extensions_packaging.py).
3642
_ARTIFACT_PATH_PROPERTIES = frozenset(
37-
prop for props in PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES.values() for prop in props
43+
name
44+
for props in PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES.values()
45+
for prop in props
46+
for name in (prop, prop.rsplit(".", 1)[-1])
3847
)
3948

4049

samcli/lib/cfn_language_extensions/utils.py

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -94,25 +94,34 @@ def is_unresolved_param_or_pseudo_ref(value: Any, context: "TemplateProcessingCo
9494

9595

9696
# Mapping-name prefixes that SAM CLI emits for dynamic Fn::ForEach handling:
97-
# - SAM + <packageable artifact property> + <nesting path> + [resource suffix]
97+
# - SAM + <leaf of packageable artifact property> + <nesting path> + [resource suffix]
9898
# (see language_extensions_packaging._compute_mapping_name)
9999
# - SAMLayers + <nesting path> (see build_context — auto dependency layer refs)
100-
# Customer-authored mappings should never start with one of these exact
101-
# PascalCase prefixes, so exact-prefix matching avoids the false positives
102-
# a regex like r"^SAM[A-Z]..." would hit on names like SAMPLE / SAMSUNG.
103-
_SAM_GENERATED_MAPPING_PREFIXES: Tuple[str, ...] = (
104-
"SAMCodeUri",
105-
"SAMImageUri",
106-
"SAMContentUri",
107-
"SAMDefinitionUri",
108-
"SAMSchemaUri",
109-
"SAMBodyS3Location",
110-
"SAMDefinitionS3Location",
111-
"SAMTemplateURL",
112-
"SAMCode",
113-
"SAMContent",
114-
"SAMLayers",
115-
)
100+
# Derived from PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES so the set automatically
101+
# tracks the canonical packageable-resource list. Customer-authored mappings
102+
# should never start with one of these exact PascalCase prefixes; exact-prefix
103+
# matching avoids the false positives a regex like r"^SAM[A-Z]..." would hit on
104+
# names like SAMPLE / SAMSUNG.
105+
def _build_sam_generated_mapping_prefixes() -> Tuple[str, ...]:
106+
from samcli.lib.cfn_language_extensions.models import PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES
107+
108+
seen: set = set()
109+
prefixes: list = []
110+
for props in PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES.values():
111+
for prop in props:
112+
leaf = prop.rsplit(".", 1)[-1]
113+
prefix = f"SAM{leaf}"
114+
if prefix not in seen:
115+
seen.add(prefix)
116+
prefixes.append(prefix)
117+
# SAMLayers is emitted by sam build for auto dependency layer references and
118+
# has no corresponding artifact property; add it explicitly.
119+
if "SAMLayers" not in seen:
120+
prefixes.append("SAMLayers")
121+
return tuple(prefixes)
122+
123+
124+
_SAM_GENERATED_MAPPING_PREFIXES: Tuple[str, ...] = _build_sam_generated_mapping_prefixes()
116125

117126

118127
def is_sam_generated_mapping(mapping_name: str) -> bool:

tests/unit/commands/_utils/test_template.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -688,6 +688,53 @@ def test_move_template_preserves_docker_imageuri_in_sam_mappings(self):
688688
"emulation-python3.9-beta:latest",
689689
)
690690

691+
def test_updates_glue_script_location_mapping(self):
692+
"""Mappings emitted for AWS::Glue::Job (key 'ScriptLocation' from leaf of 'Command.ScriptLocation')
693+
must be recognized as SAM-generated and have their relative paths adjusted on template move.
694+
"""
695+
mappings = {
696+
"SAMScriptLocationJobs": {
697+
"Etl1": {"ScriptLocation": os.path.join(".aws-sam", "build", "Etl1Job", "job.py")},
698+
"Etl2": {"ScriptLocation": os.path.join(".aws-sam", "build", "Etl2Job", "job.py")},
699+
}
700+
}
701+
702+
with tempfile.TemporaryDirectory() as tmpdir:
703+
original_root = tmpdir
704+
new_root = os.path.join(tmpdir, ".aws-sam", "build")
705+
os.makedirs(new_root, exist_ok=True)
706+
707+
_update_sam_mappings_relative_paths(mappings, original_root, new_root)
708+
709+
self.assertEqual(
710+
mappings["SAMScriptLocationJobs"]["Etl1"]["ScriptLocation"],
711+
os.path.join("Etl1Job", "job.py"),
712+
)
713+
self.assertEqual(
714+
mappings["SAMScriptLocationJobs"]["Etl2"]["ScriptLocation"],
715+
os.path.join("Etl2Job", "job.py"),
716+
)
717+
718+
def test_updates_serverless_application_location_mapping(self):
719+
"""Mappings emitted for AWS::Serverless::Application (key 'Location') must be recognized."""
720+
mappings = {
721+
"SAMLocationApps": {
722+
"Pub": {"Location": os.path.join(".aws-sam", "build", "PubApi", "template.yaml")},
723+
}
724+
}
725+
726+
with tempfile.TemporaryDirectory() as tmpdir:
727+
original_root = tmpdir
728+
new_root = os.path.join(tmpdir, ".aws-sam", "build")
729+
os.makedirs(new_root, exist_ok=True)
730+
731+
_update_sam_mappings_relative_paths(mappings, original_root, new_root)
732+
733+
self.assertEqual(
734+
mappings["SAMLocationApps"]["Pub"]["Location"],
735+
os.path.join("PubApi", "template.yaml"),
736+
)
737+
691738

692739
class Test_resolve_relative_to(TestCase):
693740
def setUp(self):

tests/unit/lib/cfn_language_extensions/test_utils.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,48 @@ def test_layers_prefix_matches(self):
7676
def test_layers_digit_matches(self):
7777
self.assertTrue(is_sam_generated_mapping("SAMLayers1stBatch"))
7878

79+
# Mappings with leaf names derived from dotted artifact paths
80+
def test_glue_script_location_mapping_matches(self):
81+
# AWS::Glue::Job.Command.ScriptLocation -> leaf "ScriptLocation"
82+
self.assertTrue(is_sam_generated_mapping("SAMScriptLocationJobs"))
83+
84+
def test_lambda_image_uri_mapping_matches(self):
85+
# AWS::Lambda::Function.Code.ImageUri -> leaf "ImageUri" (already covered by SAMImageUri)
86+
self.assertTrue(is_sam_generated_mapping("SAMImageUriFuncs"))
87+
88+
# Mappings produced by leaf names from non-dotted artifact paths added by #9005
89+
def test_serverless_application_location_mapping_matches(self):
90+
self.assertTrue(is_sam_generated_mapping("SAMLocationApps"))
91+
92+
def test_eb_source_bundle_mapping_matches(self):
93+
self.assertTrue(is_sam_generated_mapping("SAMSourceBundleVersions"))
94+
95+
def test_cfn_module_package_mapping_matches(self):
96+
self.assertTrue(is_sam_generated_mapping("SAMModulePackageMods"))
97+
98+
def test_cfn_resource_version_mapping_matches(self):
99+
self.assertTrue(is_sam_generated_mapping("SAMSchemaHandlerPackageRes"))
100+
101+
# Sync test mirroring test_dict_is_in_sync_with_canonical_lists in test_models.py
102+
def test_prefixes_in_sync_with_packageable_artifact_properties(self):
103+
"""Every leaf in PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES must be classifiable
104+
as a SAM-generated Mapping prefix. Future additions to the canonical list
105+
propagate automatically, so missing one fails this test loudly.
106+
"""
107+
from samcli.lib.cfn_language_extensions.models import PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES
108+
109+
for resource_type, props in PACKAGEABLE_RESOURCE_ARTIFACT_PROPERTIES.items():
110+
for prop in props:
111+
leaf = prop.rsplit(".", 1)[-1]
112+
# A Mapping name is SAM<leaf><LoopName>; using "X" as a stand-in loop name.
113+
mapping_name = f"SAM{leaf}X"
114+
self.assertTrue(
115+
is_sam_generated_mapping(mapping_name),
116+
f"Mapping name {mapping_name!r} (from {resource_type}.{prop}) "
117+
f"is not recognized as SAM-generated; "
118+
f"_SAM_GENERATED_MAPPING_PREFIXES is out of sync.",
119+
)
120+
79121

80122
class TestIsUnresolvedParamOrPseudoRef(TestCase):
81123
def _ctx(self, parameters=None) -> TemplateProcessingContext:

0 commit comments

Comments
 (0)