Repository navigation
Conversation
sam package now zips a MicrovmImage source directory, uploads it to S3
and points the template at the S3 URI. The MicroVM service builds the
image from that zip. This replaces the exporters that failed for local
code.
- AWS::Serverless::MicrovmImage: CodeUri is set to the S3 URI.
- AWS::Lambda::MicrovmImage: CodeArtifact is written as {Uri: <S3 URI>},
which CloudFormation requires. A local path is accepted as {Uri: ...}
or as a plain string, the form CDK and sam build use.
S3 URIs and intrinsics are left unchanged. A missing code path is not
packaged, so the template directory is never uploaded instead.
A MicrovmImage CodeUri that is a file gave one message that always mentioned .zip files, even for a file such as a Dockerfile. For a .zip, explain that sam build needs the source directory and that sam package uploads the .zip as is. For any other file, suggest the directory that contains it.
🧪 Hands-on test: No problems observed in 7 scenariosCommit Scenarios
Runner's notes in full
🤖 Automated hands-on test — not a review approval · model claude-opus-5.5 · Kiro kiro-cli 2.33.1 · build History (5)
|
There was a problem hiding this comment.
🧪 Hands-on test reproduced 2 new problems on commit 2798834. Commands and evidence are in the hands-on comment.
- [BUG] sam package --language-extensions crashes with TypeError for AWS::Lambda::MicrovmImage in Fn::ForEach using plain-string CodeArtifact: ./${N} · medium · samcli/lib/package/language_extensions_packaging.py:342
- [UX] sam package accepts a MicrovmImage CodeUri pointing at a single file (Dockerfile) and silently uploads a zip with only that file · low · samcli/lib/package/packageable_resources.py:774
🤖 Automated hands-on test — not a review approval
|
|
||
| temp_dir = None | ||
| if is_local_file(local_path) and not is_zip_file(local_path): | ||
| temp_dir = copy_to_temp_dir(local_path) |
There was a problem hiding this comment.
🧪 Hands-on: [UX] sam package accepts a MicrovmImage CodeUri pointing at a single file (Dockerfile) and silently uploads a zip with only that file (reproduced 2 times in S3). Commands and evidence: hands-on comment.
Add an AWS::Serverless::MicrovmImage and an AWS::Lambda::MicrovmImage template and run them in every zip package test that runs the function templates.
With --language-extensions, an AWS::Lambda::MicrovmImage in Fn::ForEach
with a plain-string CodeArtifact such as ./${N} crashed with a
TypeError. The exporter writes CodeArtifact back as {Uri: <S3 URI>}, and
copying CodeArtifact.Uri into a body that still held a string failed. A
dynamic string would also have become an Fn::FindInMap string, while
CloudFormation requires the object form.
Before expansion, wrap a local string CodeArtifact as {Uri: <string>},
so it is packaged like the {Uri: ./${N}} form. S3 URIs and intrinsics
are left unchanged.
A MicrovmImage CodeUri or CodeArtifact that pointed at a single file, such as the Dockerfile, was zipped on its own and uploaded, so files the Dockerfile copies were left out and the remote build failed. Fail instead, and suggest the directory that contains the file, as sam build does. A .zip is still uploaded as is.
|
Both problems are fixed:
|
bnusunny
left a comment
There was a problem hiding this comment.
Looks good overall. One test gap to fix before merge, plus three small nits. I also ran sam package --language-extensions end to end, and CloudFormation created MicroVM images from the packaged template.
End-to-end test (5e72d0c, us-east-1)
I used one template with two Fn::ForEach loops over [Alpha, Beta]:
AWS::Lambda::MicrovmImagewithCodeArtifact: ./${Name}as a plain string. This is the form that crashed before 84f3254.AWS::Serverless::MicrovmImagewithCodeUri: ./${Name}.
sam package uploaded one zip per source directory. It kept the Fn::ForEach blocks and wrote CodeArtifact: {Uri: !FindInMap [...]} and CodeUri: !FindInMap [...], with the per-name S3 URIs in Mappings. I deployed the packaged template with aws cloudformation deploy, so SAM did not package it again. All four images reached CREATE_COMPLETE, and the MicroVM service built each one from its zip.
The packaging itself had no problems. Two notes for anyone repeating the test:
AWS::Lambda::MicrovmImagefails CloudFormation early validation unlessBuildRoleArn,Hooks,CpuConfigurations,Logging,Resources,EnvironmentVariables,EgressNetworkConnectors,AdditionalOsCapabilitiesandDescriptionare all set. The SAM transform fills these in, sotests/integration/testdata/package/aws-lambda-microvmimage.yamlpackages but would not deploy as written.- The image must start a long-running server. With the
microvm-image-foreachbuild fixtures, which have noCMD, the Docker build succeeds but the image never stabilizes.
Findings
- Test gap (please fix). The two calls to
wrap_microvm_image_code_artifact_stringshave no tests (package_context.py:264,artifact_exporter.py:318). If you delete either call, every package unit test still passes. Deleting the top-level one brings back theTypeErrorfrom the hands-on report. The new tests call the helper directly, and no integ fixture usesFn::ForEach. APackageContext-level test with a plain-string./${N}inFn::ForEach, plus a nested-stack version, would cover both calls. - Nit.
FORCE_ZIP = Trueon both MicroVM exporters never takes effect._check_microvm_image_sourcerejects single non-zip files first, andLambdaMicrovmImageResource.exportnever reads the flag. It suggests that a single file gets zipped, which can't happen. Drop it or add a comment. - Low. A plain-string S3
CodeArtifact: s3://...is left as a string, but CloudFormation requires{Uri: ...}. Local strings are already rewritten into that form, so S3 strings could be too. This isn't a regression. - Nit.
sam builddecides whether a file is a .zip from its extension (app_builder.py:1219).sam packagechecks whether the file really is a zip. So a corruptx.zipgets "sam package uploads a .zip as is" from build, and then package rejects it as a file.
Checks: CI is green. 695 unit tests pass in the package and build suites. I broke 11 of the new branches one at a time to see whether a test fails. 9 were caught; the two that weren't are the calls in finding 1.
Neither call to wrap_microvm_image_code_artifact_strings had a test, so
removing one still passed. Package a plain-string CodeArtifact in
Fn::ForEach through PackageContext and through a nested stack, and check
that each ends as {Uri: Fn::FindInMap}.
A single non-zip file is rejected before export, and the Lambda exporter never read the flag, so FORCE_ZIP had no effect and suggested that a single file is zipped.
sam build chose its .zip message from the file extension, while sam package checks the content. A corrupt x.zip was told that sam package uploads it as is, and then package rejected it as a file. Check the content in build too.
CloudFormation requires AWS::Lambda::MicrovmImage CodeArtifact as an
object, but an S3 URI given as a plain string was left as a string, so
the packaged template could not deploy. Local strings were already
rewritten. Write S3 strings as {Uri: <S3 URI>} too, without uploading,
as the Lambda function and layer exporters write an S3 string Code or
Content as {S3Bucket, S3Key}. This also applies inside Fn::ForEach.
|
@bnusunny Thanks for the review and the end-to-end run. All four findings are addressed:
On your deploy notes: |
Which issue(s) does this change fix?
Part of #9283. #9357 added
sam buildsupport; this PR addssam packagesupport for local MicrovmImage code, whichsam deployuses too.Why is this change necessary?
A MicrovmImage is deployed as a zip of its source directory (Dockerfile plus application code), which the MicroVM service builds remotely. Until now
sam packagefailed for a MicrovmImage with local code (a placeholder added in #9357), so users had to zip and upload the source themselves.How does it address the issue?
The placeholder exporters in
samcli/lib/package/packageable_resources.pyare replaced with exporters that upload the code:AWS::Serverless::MicrovmImage(CodeUri): aResourceZipwithFORCE_ZIP. A local directory is zipped and uploaded, andCodeUriis set to the S3 URI. This matchesAWS::Serverless::Function.AWS::Lambda::MicrovmImage(CodeArtifact): CloudFormation requiresCodeArtifact: {Uri: s3://...}. A local path is accepted as{Uri: ./src}or as a plain string, the form CDK templates and built templates use, and is written back as{Uri: <S3 URI>}.get_property_valuereads the S3 URI from either form, forsam delete..zipfile is uploaded as is. Intrinsics are left unchanged, and an S3 URI is not uploaded again. A missing code path is not packaged (PACKAGE_NULL_PROPERTY = False), so the template directory is never uploaded in its place.Fn::ForEachtemplates (--language-extensions): the packaging map is derived fromRESOURCES_WITH_LOCAL_PATHS, which already lists both types. Before expansion, a localAWS::Lambda::MicrovmImageCodeArtifactgiven as a plain string is wrapped as{Uri: ...}, so a dynamic./${N}is packaged like{Uri: ./${N}}and ends as{Uri: !FindInMap [...]}.A
CodeUriorCodeArtifactthat points at a single file other than a.zip, such as the Dockerfile, fails with a message that suggests the directory that contains it, assam builddoes. Otherwise the package step does not validate the contents of the source, the same as for functions.The PR also improves a
sam builderror: when a MicrovmImageCodeUriis a file, a.zipnow gets its own message (point at the source directory to build locally;sam packageuploads the.zipas is), and any other file, such as a Dockerfile, gets a message that suggests the directory that contains it.What side effects does this change have?
sam packageandsam deploynow upload local MicrovmImage code instead of failing.AWS::Lambda::MicrovmImage, aCodeArtifactgiven as a plain string, local path or S3 URI, is written as{Uri: ...}in the packaged template, the object form CloudFormation requires. An S3 URI is not uploaded again. This matches the Lambda function and layer exporters, which write an S3 stringCodeorContentas{S3Bucket, S3Key}.Testing
test_artifact_exporter.pycover a local directory, bothCodeArtifactforms, a local.zip, S3 URIs, intrinsics, a missing value, a missing directory,get_property_value, and a full template export.AWS::Serverless::MicrovmImageand anAWS::Lambda::MicrovmImagetemplate, in every test that runs the function templates.sam buildfollowed bysam packageon the built template, with the S3 upload stubbed, for both resource types and for theFn::ForEachfixture.al2023-1version 2):sam build, thensam deploy --resolve-s3 --capabilities CAPABILITY_IAMon anAWS::Serverless::MicrovmImagewith a localCodeUriand noBuildRoleArn. The stack was created, the uploaded zip contained the Dockerfile and app code at its root, the transformedAWS::Lambda::MicrovmImagehadCodeArtifact: {Uri: <that S3 URI>}, and the MicroVM service built the image (state: CREATED, active version1.0).sam deletethen removed the stack, the image, and the uploaded zip.Mandatory Checklist
PRs will only be reviewed after checklist is complete
make prpassesmake update-reproducible-reqsif dependencies were changedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.