fix: skip binary validation for copy-only workflows - #913
Conversation
![aws-sam-tooling-bot[bot]](https://avatars.githubusercontent.com/b/1290?s=60&v=4){kind=small}
| return [] | ||
| return [PathResolver(runtime=self.runtime, binary="npm")] | ||
|
|
||
| def get_validators(self): |
There was a problem hiding this comment.
[BUG] Returning an empty list from both get_resolvers() and get_validators() skips more than binary resolution — it also disables runtime support validation.
BaseWorkflow.binaries builds its dict by zipping resolvers with validators, so an empty resolver list means self.binaries is {} and the sanitize loop in aws_lambda_builders/workflow.py never runs any validator. But RuntimeValidator.validate() does not only check the resolved executable — it checks the requested runtime and architecture independently of runtime_path:
runtime_architectures = SUPPORTED_RUNTIMES.get(self.runtime, None)
if not runtime_architectures:
raise UnsupportedRuntimeError(runtime=self.runtime)
if self.architecture not in runtime_architectures:
raise UnsupportedArchitectureError(runtime=self.runtime, architecture=self.architecture)This is the only place in the package that enforces that check (UnsupportedRuntimeError / UnsupportedArchitectureError are raised nowhere else). Concretely, after this change a build with an unsupported runtime silently succeeds instead of failing:
runtime="nodejs14.x"with nopackage.json→ previouslyWorkflowFailedErrorwrapping "Runtime nodejs14.x is not supported", now a plain source copy.runtime="ruby2.7"withdownload_dependencies=False→ same silent success (aws_lambda_builders/workflows/ruby_bundler/workflow.py:67-75, which has the identical pattern via_use_bundler).
Since nodejs14.x and ruby2.7 are absent from ALL_RUNTIMES in supported_runtimes.py, callers using the library or JSON-RPC interface directly lose the only guard against packaging for a runtime this builder doesn't support.
Skipping the binary lookup is the right goal; the runtime check should be preserved. One option is to keep the validator and run the runtime check explicitly rather than relying on the resolver loop, for example by validating in __init__ before taking the copy-only path:
if not self._use_npm:
LOG.warning("package.json file not found. Continuing the build without dependencies.")
# still reject unsupported runtime/architecture combinations
RuntimeValidator(runtime=self.runtime, architecture=self.architecture).validate(runtime_path=None)
self.actions = [CopySourceAction(source_dir, artifacts_dir, excludes=self.EXCLUDED_FILES)]
returnNote that validate() assigns runtime_path only after the checks pass, so it is safe to call without a resolved executable — but please confirm the exception type surfaced here is acceptable for the copy-only path, since it would no longer be wrapped by sanitize into a WorkflowFailedError. If dropping runtime validation for copy-only builds is intentional, it is worth stating that explicitly in the PR description, as it is a user-visible behavior change not covered by the current title.
| return super().get_validators() | ||
|
|
||
| def run(self): | ||
| if not self._use_npm: |
There was a problem hiding this comment.
[GENERAL] The run() + _validate_runtime() pair added here is byte-for-byte identical to the block added in aws_lambda_builders/workflows/ruby_bundler/workflow.py. Consider hoisting it into BaseWorkflow instead of duplicating it.
The reason this matters beyond style: the root cause is in sanitize (aws_lambda_builders/workflow.py). It builds self.binaries from zip(get_resolvers(), get_validators()), so when the resolver list is empty the validation loop iterates zero times and len(self.binaries) != len(valid_paths) is 0 != 0 — the whole function becomes a no-op and no runtime/architecture check runs. Any workflow that returns [] from get_resolvers() silently loses runtime support validation unless it remembers to also override run(). Two workflows now carry that compensating override; a third will likely forget, and the failure is silent (an unsupported runtime or architecture just builds successfully) rather than loud.
Putting the fallback in the base class makes the guarantee structural:
# aws_lambda_builders/workflow.py, inside sanitize's wrapper
def wrapper(self, args, kwargs):
if not self.binaries:
# no build tool needed (copy-only workflow), but runtime/arch
# support must still be enforced
try:
RuntimeValidator(runtime=self.runtime, architecture=self.architecture).validate(None)
except RuntimeValidatorError as ex:
raise WorkflowFailedError(
workflowname=self.NAME, action_name="Validation", reason=str(ex)
) from ex
return func(self, args, kwargs)
...Both subclasses could then drop run(), validate_runtime(), and the RuntimeValidator/exception imports, keeping only the get_resolvers()/get_validators() guards. The existing unit tests asserting the "...:Validation - Runtime nodejs1.x is not supported" and "...:Validation - Architecture invalid_arch is not supported..." messages would continue to pass unchanged, since the message construction is the same.
| validation_errors = [] | ||
| binaries = self.binaries | ||
|
|
||
| if not binaries: |
There was a problem hiding this comment.
[BUG] This branch changes behavior for PythonUvWorkflow, which is outside the PR's stated scope of Node.js and Ruby, and can fail builds that previously succeeded.
PythonUvWorkflow returns [] from both get_resolvers() and get_validators() unconditionally (aws_lambda_builders/workflows/python_uv/workflow.py:158-175), with the docstring stating that "UV doesn't need external validation of Python runtime paths." Before this change its self.binaries was {}, the loop body never executed, and no runtime/architecture check ran. After this change every workflow with empty binaries goes through RuntimeValidator. The PR's own new tests confirm the shift — test_unsupported_runtime_is_rejected and test_unsupported_architecture_is_rejected assert that uv builds now raise WorkflowFailedError.
Two concrete consequences:
- A uv build for a runtime absent from
RUNTIME_ARCHITECTURES(aws_lambda_builders/supported_runtimes.py) now hard-fails, even though uv can download that interpreter itself. That table has to be updated for each new Lambda Python runtime; uv builds previously did not depend on it. runtimeis documented as optional inLambdaBuilder.build(aws_lambda_builders/builder.py:62). A uv build invoked without a runtime now fails withRuntime None is not supportedwhere it previously proceeded. Node.js and Ruby are unaffected here, since both resolved a binary and ranRuntimeValidatorbefore this PR.
Separately, hardcoding RuntimeValidator bypasses get_validators() entirely, so a workflow with a custom validator (e.g. GoRuntimeValidator, PythonRuntimeValidator) that adopts the same copy-only pattern would silently get base-class validation rather than its own.
Consider making the opt-out explicit so the fallback only applies to workflows that want it, rather than to any workflow whose binaries happen to be empty:
# BaseWorkflow
def get_runtime_validator(self):
"""Validator applied when no binaries need to be resolved. Return None to opt out."""
return RuntimeValidator(runtime=self.runtime, architecture=self.architecture)# sanitize
if not binaries:
runtime_validator = self.get_runtime_validator()
if runtime_validator:
try:
runtime_validator.validate(None)
except RuntimeValidatorError as ex:
validation_errors.append(str(ex))If applying runtime validation to uv builds is intentional, please call it out in the PR description and update the PythonUvWorkflow.get_validators() docstring, which now contradicts the actual behavior.
github-actions
Bot
added
pr/external
stage/needs-triage
1 participant
Issue #, if available:
#831
Description of changes
Skip binary resolution and validation for copy-only workflows:
package.jsonis presentdownload_dependencies=FalseExisting behavior is preserved when the corresponding build tools are required. Added unit and integration coverage for both cases.
Description of how you validated changes
git diff --checkpassed@reedham-aws (tagging you as requested would appreciate a look!)
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.