Skip to content

Commit f17c4c5

Browse files
committed
fix(golden): replace BUILT_ARTIFACT placeholder with deterministic relpath
Real `sam build` writes CodeUri/Code/etc. as a relative path from the output template's directory to the artifact directory — for a top-level resource the result is just the resource id (e.g. `CodeUri: HelloFunction`) because both end up under .aws-sam/build/. The harness was substituting a synthetic <<BUILT_ARTIFACT>> placeholder instead, which obscured what real sam build actually produces and made the build pin disagree with the CLI's own output. Drop the placeholder rewriter entirely. Stage the case dir (template.yaml + src/) into the temp tree before driving BuildContext, and put .aws-sam/build/ inside the staged tree. ApplicationBuilder's relpath then resolves to .aws-sam/build/<full_path> which move_template collapses to <full_path> when writing to .aws-sam/build/template.yaml — exactly what real sam build writes. Net: harness drops the artifact-walker, the path-rewriter, the RESOURCES_WITH_LOCAL_PATHS / RESOURCES_WITH_IMAGE_COMPONENT imports, the BUILT_ARTIFACT_PLACEHOLDER constant, and the sentinel-dirname indirection (~150 lines). All sentinel build pins now show the real deterministic relpath. Tests updated to assert the relpath shape; one test renamed for accuracy (`test_build_cfn_case_replaces_code_with_placeholder` → `test_build_cfn_case_keeps_lambda_function`). 42/42 pass under CI-like env.
1 parent 08af0d5 commit f17c4c5

6 files changed

Lines changed: 93 additions & 157 deletions

File tree

tests/golden/harness.py

Lines changed: 70 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,23 @@
1111
1212
- :class:`samcli.lib.build.app_builder.ApplicationBuilder` ``.build()`` is
1313
replaced with a stub that returns an :class:`ApplicationBuildResult` with
14-
artifacts pointing at a per-resource sentinel directory under
15-
``<build_dir>/__sam_golden_artifact__/<resource_full_path>``. The directory
16-
contains a single file whose content is the resource's full path, which
17-
makes the downstream content-addressed S3 URI deterministic per resource.
14+
one entry per buildable function / layer pointing at a per-resource
15+
artifact directory under ``<build_dir>/<resource_full_path>``. The
16+
directory contains a single file whose content is the resource's full
17+
path, which makes the downstream content-addressed S3 URI deterministic
18+
per resource.
1819
- :meth:`samcli.lib.package.s3_uploader.S3Uploader.upload` and
1920
``upload_with_dedup`` are replaced with deterministic stubs returning
2021
``s3://golden-bucket/<sha256>`` where the digest is taken over the file
21-
content.
22+
content (zip-aware: zip envelopes are hashed by sorted member names plus
23+
per-member content hashes so the digest is OS-deterministic).
2224
23-
After the build phase, the on-disk ``.aws-sam/build/template.yaml`` is read
24-
back and any artifact-property string referencing a sentinel artifact
25-
directory is rewritten to ``BUILT_ARTIFACT_PLACEHOLDER`` so the build pin is
26-
stable across runs and machines.
25+
The case directory (``template.yaml`` + ``src/``) is staged into a fresh
26+
temp dir before driving ``BuildContext``. Real ``sam build`` writes
27+
``CodeUri`` as ``os.path.relpath(absolute_artifact, template_dir)``; with
28+
the case staged into a temp tree, ``original_dir`` is the staged template's
29+
parent and the relpath is the deterministic ``.aws-sam/build/<full_path>``
30+
that real ``sam build`` produces. No post-build path rewriting is needed.
2731
2832
This keeps the harness narrow and lets every other concern (LE expansion,
2933
ForEach merge, Mappings generation, AWS::Include export, etc.) flow through
@@ -34,6 +38,7 @@
3438

3539
import hashlib
3640
import os
41+
import shutil
3742
import zipfile
3843
from pathlib import Path
3944
from tempfile import TemporaryDirectory
@@ -44,13 +49,8 @@
4449
from samcli.commands.package.package_context import PackageContext
4550
from samcli.lib.build.app_builder import ApplicationBuilder, ApplicationBuildResult
4651
from samcli.lib.build.build_graph import BuildGraph
47-
from samcli.lib.cfn_language_extensions.utils import is_foreach_key
4852
from samcli.lib.package.s3_uploader import S3Uploader
4953
from samcli.lib.providers.provider import get_full_path
50-
from samcli.lib.utils.resources import (
51-
RESOURCES_WITH_IMAGE_COMPONENT,
52-
RESOURCES_WITH_LOCAL_PATHS,
53-
)
5454
from samcli.yamlhelper import yaml_parse
5555

5656

@@ -84,27 +84,25 @@ def _read_template_as_plain_dict(path: Path) -> Dict[str, Any]:
8484
return parsed
8585

8686

87-
BUILT_ARTIFACT_PLACEHOLDER = "<<BUILT_ARTIFACT>>"
8887
GOLDEN_BUCKET = "golden-bucket"
8988

90-
# Directory name placed under <build_dir> for the stub artifact tree. Kept
91-
# deliberately verbose so the post-build path-rewriter can identify these
92-
# paths unambiguously (a substring match on this prefix would be safe even
93-
# if a real-world resource happened to share the same logical id).
94-
_ARTIFACT_DIRNAME = "__sam_golden_artifact__"
95-
9689

9790
def _stub_application_builder_build(build_dir: str) -> Any:
9891
"""Build a stand-in for ``ApplicationBuilder.build``.
9992
100-
The returned function captures ``build_dir`` and writes one sentinel
101-
directory per buildable function / layer:
93+
The returned function captures ``build_dir`` and writes one artifact
94+
directory per buildable function / layer at the canonical location
95+
real ``sam build`` uses:
10296
103-
<build_dir>/__sam_golden_artifact__/<full_path>/__golden_placeholder__
97+
<build_dir>/<full_path>/__golden_placeholder__
10498
10599
The placeholder file's content is the resource's full path, which makes
106100
the package phase's content-addressed S3 URI distinct per resource even
107-
though no real source code was compiled.
101+
though no real source code was compiled. ``ApplicationBuilder.update_template``
102+
rewrites these absolute paths to ``relpath(artifact, template_dir)``;
103+
when the case is staged under the temp dir before driving
104+
``BuildContext``, the relpath becomes the deterministic
105+
``.aws-sam/build/<full_path>`` string real ``sam build`` produces.
108106
109107
The returned ``ApplicationBuildResult.artifacts`` map is keyed on
110108
``get_full_path(stack_path, function_id)`` exactly as real SAM CLI does;
@@ -113,18 +111,17 @@ def _stub_application_builder_build(build_dir: str) -> Any:
113111

114112
def _fake_build(self: ApplicationBuilder) -> ApplicationBuildResult:
115113
artifacts: Dict[str, str] = {}
116-
sentinel_root = Path(build_dir) / _ARTIFACT_DIRNAME
117114

118115
for function in self._resources_to_build.functions: # type: ignore[attr-defined]
119116
full_path = get_full_path(function.stack_path, function.function_id)
120-
artifact_dir = sentinel_root / full_path.replace("/", os.sep)
117+
artifact_dir = Path(build_dir) / full_path.replace("/", os.sep)
121118
artifact_dir.mkdir(parents=True, exist_ok=True)
122119
(artifact_dir / "__golden_placeholder__").write_text(full_path, encoding="utf-8")
123120
artifacts[full_path] = str(artifact_dir)
124121

125122
for layer in self._resources_to_build.layers: # type: ignore[attr-defined]
126123
full_path = layer.full_path
127-
artifact_dir = sentinel_root / full_path.replace("/", os.sep)
124+
artifact_dir = Path(build_dir) / full_path.replace("/", os.sep)
128125
artifact_dir.mkdir(parents=True, exist_ok=True)
129126
(artifact_dir / "__golden_placeholder__").write_text(full_path, encoding="utf-8")
130127
artifacts[full_path] = str(artifact_dir)
@@ -192,111 +189,46 @@ def _fake_s3_upload_with_dedup(
192189
return _fake_s3_upload(self, file_name)
193190

194191

195-
def _walk_artifact_string_values(template: Dict[str, Any]):
196-
"""Yield ``(setter, value)`` for every artifact-property string in the template.
192+
_STAGED_CASE_DIRNAME = "case"
197193

198-
``setter`` is a callable that, given a new value, replaces the original
199-
in place. We use this to swap built-artifact relative paths for the
200-
sentinel placeholder string without having to re-parse the YAML.
201194

202-
The walk handles both top-level resources and resources nested inside
203-
``Fn::ForEach::*`` bodies.
204-
"""
205-
resources = template.get("Resources", {}) or {}
206-
for resource_key, resource_value in resources.items():
207-
if is_foreach_key(resource_key):
208-
yield from _walk_foreach_artifact_strings(resource_value)
209-
continue
210-
yield from _walk_resource_artifact_strings(resource_value)
211-
212-
213-
def _walk_foreach_artifact_strings(foreach_value: Any):
214-
if not isinstance(foreach_value, list) or len(foreach_value) < 3: # noqa: PLR2004
215-
return
216-
body = foreach_value[2]
217-
if not isinstance(body, dict):
218-
return
219-
for body_key, body_value in body.items():
220-
if is_foreach_key(body_key):
221-
yield from _walk_foreach_artifact_strings(body_value)
222-
continue
223-
yield from _walk_resource_artifact_strings(body_value)
224-
225-
226-
def _walk_resource_artifact_strings(resource: Any):
227-
if not isinstance(resource, dict):
228-
return
229-
rtype = resource.get("Type")
230-
if not isinstance(rtype, str):
231-
return
232-
properties = resource.get("Properties")
233-
if not isinstance(properties, dict):
234-
return
235-
for paths_dict in (RESOURCES_WITH_LOCAL_PATHS, RESOURCES_WITH_IMAGE_COMPONENT):
236-
for prop_path in paths_dict.get(rtype, []):
237-
yield from _yield_at_path(properties, prop_path.split("."))
238-
239-
240-
def _yield_at_path(container: Any, parts):
241-
if not isinstance(container, dict) or not parts:
242-
return
243-
head, rest = parts[0], parts[1:]
244-
if head not in container:
245-
return
246-
if not rest:
247-
value = container[head]
248-
if isinstance(value, str):
249-
250-
def _set(new_value, _container=container, _head=head):
251-
_container[_head] = new_value
252-
253-
yield _set, value
254-
return
255-
yield from _yield_at_path(container[head], rest)
256-
257-
258-
def _replace_artifact_paths_with_placeholder(template: Dict[str, Any], build_dir: str) -> None:
259-
"""Rewrite every artifact-property string that resolves under the sentinel
260-
directory to :data:`BUILT_ARTIFACT_PLACEHOLDER`.
261-
262-
A value is considered an artifact path if, when joined relative to
263-
``build_dir``, it lands inside ``<build_dir>/__sam_golden_artifact__/``.
264-
Both relative and absolute forms are recognized, which covers the two
265-
paths SAM CLI may write (``move_template`` rewrites relative; an
266-
absolute path survives if Windows cross-drive logic kicked in).
195+
def _stage_case_dir(case_dir: Path, tmp_root: Path) -> Path:
196+
"""Copy the case directory (template + src/) into ``tmp_root/case``.
197+
198+
Real ``sam build`` rewrites artifact paths to
199+
``relpath(absolute_artifact, template_dir)``. Staging the case dir into
200+
a known location under the temp root makes that relpath the canonical
201+
deterministic ``.aws-sam/build/<full_path>`` string regardless of where
202+
the host's tempdir lives.
267203
"""
268-
sentinel_root = (Path(build_dir) / _ARTIFACT_DIRNAME).resolve()
269-
for setter, value in _walk_artifact_string_values(template):
270-
if not isinstance(value, str):
271-
continue
272-
# Plain string artifact location can be either relative (to build_dir)
273-
# or absolute. Resolve once against build_dir to handle both shapes.
274-
candidate = (Path(build_dir) / value).resolve() if not os.path.isabs(value) else Path(value).resolve()
275-
try:
276-
candidate.relative_to(sentinel_root)
277-
except ValueError:
278-
continue
279-
setter(BUILT_ARTIFACT_PLACEHOLDER)
204+
staged = tmp_root / _STAGED_CASE_DIRNAME
205+
shutil.copytree(case_dir, staged)
206+
return staged
280207

281208

282209
def run_build_pipeline_to_dir(
283-
template_path: Path,
210+
staged_template_path: Path,
284211
language_extensions: bool,
285212
build_dir: Path,
286213
cache_dir: Path,
287214
) -> Dict[str, Any]:
288215
"""Drive ``BuildContext`` to produce a built template under ``build_dir``.
289216
290-
Returns the parsed dict read back from
291-
``<build_dir>/template.yaml``. The on-disk file is left in place so
292-
callers (notably :func:`run_package_pipeline`) can hand the path to
293-
``PackageContext`` directly.
217+
The caller is responsible for staging the case dir under a temp root
218+
(see :func:`_stage_case_dir`) and pointing ``staged_template_path`` at
219+
the staged copy. ``build_dir`` should sit inside the same staged tree
220+
so ``ApplicationBuilder.update_template`` produces the canonical
221+
``.aws-sam/build/<full_path>`` relative paths.
222+
223+
Returns the parsed dict read back from ``<build_dir>/template.yaml``.
224+
The on-disk file is left in place so callers (notably
225+
:func:`run_package_pipeline`) can hand the path to ``PackageContext``
226+
directly.
294227
"""
295-
case_dir = template_path.parent
296228
bctx = BuildContext(
297229
resource_identifier=None,
298-
template_file=str(template_path),
299-
base_dir=str(case_dir),
230+
template_file=str(staged_template_path),
231+
base_dir=str(staged_template_path.parent),
300232
build_dir=str(build_dir),
301233
cache_dir=str(cache_dir),
302234
cached=False,
@@ -312,46 +244,46 @@ def run_build_pipeline_to_dir(
312244
with patch.object(ApplicationBuilder, "build", fake_build):
313245
bctx.run()
314246

315-
template = _read_template_as_plain_dict(build_dir / "template.yaml")
316-
_replace_artifact_paths_with_placeholder(template, str(build_dir))
317-
return template
247+
return _read_template_as_plain_dict(build_dir / "template.yaml")
318248

319249

320250
def run_build_pipeline(template_path: Path, language_extensions: bool) -> Dict[str, Any]:
321251
"""In-process equivalent of ``sam build`` for a single template.
322252
323-
Allocates a fresh temp dir for the build / cache outputs, drives
324-
:class:`BuildContext` end-to-end, post-processes the on-disk
325-
``template.yaml`` to substitute the artifact placeholder, and returns
326-
the parsed dict.
253+
Stages the case dir (``template.yaml`` + ``src/``) into a fresh temp
254+
tree, drives :class:`BuildContext` end-to-end, and returns the parsed
255+
built template dict. ``CodeUri`` etc. emerge as the canonical
256+
``.aws-sam/build/<full_path>`` relative path that real ``sam build``
257+
writes — no post-processing needed.
327258
"""
328259
with TemporaryDirectory() as tmp:
329260
tmp_path = Path(tmp)
330-
build_dir = tmp_path / ".aws-sam" / "build"
331-
cache_dir = tmp_path / ".aws-sam" / "cache"
332-
return run_build_pipeline_to_dir(template_path, language_extensions, build_dir, cache_dir)
261+
staged = _stage_case_dir(template_path.parent, tmp_path)
262+
build_dir = staged / ".aws-sam" / "build"
263+
cache_dir = staged / ".aws-sam" / "cache"
264+
return run_build_pipeline_to_dir(staged / template_path.name, language_extensions, build_dir, cache_dir)
333265

334266

335267
def run_package_pipeline(template_path: Path, language_extensions: bool) -> Dict[str, Any]:
336268
"""In-process equivalent of ``sam package``.
337269
338-
Runs the build pipeline first to produce a real on-disk
339-
``.aws-sam/build/template.yaml`` (with artifact directories that the
340-
:class:`Template` exporter can actually read), then drives
270+
Runs the build pipeline first (against a staged copy of the case dir)
271+
to produce a real on-disk ``.aws-sam/build/template.yaml``, then drives
341272
:class:`PackageContext` against that template. The S3 uploader is
342273
stubbed for determinism; everything else (LE expansion, ForEach merge,
343274
Mappings generation, AWS::Include export) runs through real code.
344275
"""
345276
with TemporaryDirectory() as tmp:
346277
tmp_path = Path(tmp)
347-
build_dir = tmp_path / ".aws-sam" / "build"
348-
cache_dir = tmp_path / ".aws-sam" / "cache"
278+
staged = _stage_case_dir(template_path.parent, tmp_path)
279+
build_dir = staged / ".aws-sam" / "build"
280+
cache_dir = staged / ".aws-sam" / "cache"
349281

350282
# Re-run the build phase so we have a freshly built template on
351283
# disk that PackageContext can read. We can't reuse a previously
352284
# produced dict because PackageContext re-loads the template from
353285
# the path itself.
354-
run_build_pipeline_to_dir(template_path, language_extensions, build_dir, cache_dir)
286+
run_build_pipeline_to_dir(staged / template_path.name, language_extensions, build_dir, cache_dir)
355287

356288
built_template_path = build_dir / "template.yaml"
357289
output_template_path = tmp_path / "packaged" / "template.yaml"
@@ -395,9 +327,10 @@ def run_build_and_package(template_path: Path, language_extensions: bool) -> Tup
395327
"""
396328
with TemporaryDirectory() as tmp:
397329
tmp_path = Path(tmp)
398-
build_dir = tmp_path / ".aws-sam" / "build"
399-
cache_dir = tmp_path / ".aws-sam" / "cache"
400-
build_dict = run_build_pipeline_to_dir(template_path, language_extensions, build_dir, cache_dir)
330+
staged = _stage_case_dir(template_path.parent, tmp_path)
331+
build_dir = staged / ".aws-sam" / "build"
332+
cache_dir = staged / ".aws-sam" / "cache"
333+
build_dict = run_build_pipeline_to_dir(staged / template_path.name, language_extensions, build_dir, cache_dir)
401334

402335
built_template_path = build_dir / "template.yaml"
403336
output_template_path = tmp_path / "packaged" / "template.yaml"

tests/golden/templates/language_extensions/foreach_static_zip/expected.build.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Resources:
66
- Beta
77
- ${WorkerName}Function:
88
Properties:
9-
CodeUri: <<BUILT_ARTIFACT>>
9+
CodeUri: AlphaFunction
1010
Handler: app.handler
1111
Runtime: python3.11
1212
Type: AWS::Serverless::Function

tests/golden/templates/packageable_resources/lambda_function_zip/expected.build.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ Resources:
44
Metadata:
55
SamResourceId: HelloFunction
66
Properties:
7-
Code: <<BUILT_ARTIFACT>>
7+
Code: HelloFunction
88
FunctionName: hello
99
Handler: app.handler
1010
MemorySize: 128

tests/golden/templates/sam_resources/serverless_function_zip/README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,6 @@
22

33
Sentinel SAM case. Vanilla `AWS::Serverless::Function` with local `CodeUri`
44
pointing at a ZIP-packaged source dir. Locks the post-build `template.yaml`
5-
shape (artifact path normalized to `<<BUILT_ARTIFACT>>`) and post-package
6-
shape (CodeUri rewritten to `s3://golden-bucket/<sha256>`).
5+
shape (`CodeUri` rewritten to the resource-id-relative path real `sam build`
6+
produces) and post-package shape (CodeUri rewritten to
7+
`s3://golden-bucket/<sha256>`).

tests/golden/templates/sam_resources/serverless_function_zip/expected.build.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ Resources:
44
Metadata:
55
SamResourceId: HelloFunction
66
Properties:
7-
CodeUri: <<BUILT_ARTIFACT>>
7+
CodeUri: HelloFunction
88
Handler: app.handler
99
MemorySize: 128
1010
Runtime: python3.11

0 commit comments

Comments
 (0)