Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions aws_lambda_builders/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ class BuildInSourceSupport(Enum):
EXCLUSIVELY_SUPPORTED = [True]


def _validate_runtime_without_binary(workflow):
runtime_validator = workflow.get_runtime_validator()
if runtime_validator:
runtime_validator.validate(None)


# TODO: Move sanitize out to its own class.
def sanitize(func): # pylint: disable=too-many-statements
"""
Expand All @@ -70,8 +76,16 @@ def wrapper(self, *args, **kwargs): # pylint: disable=too-many-statements
valid_paths = {}
invalid_paths = {}
validation_errors = []
binaries = self.binaries

if not binaries:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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:

  1. 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.
  2. runtime is documented as optional in LambdaBuilder.build (aws_lambda_builders/builder.py:62). A uv build invoked without a runtime now fails with Runtime None is not supported where it previously proceeded. Node.js and Ruby are unaffected here, since both resolved a binary and ran RuntimeValidator before 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.

try:
_validate_runtime_without_binary(self)
except RuntimeValidatorError as ex:
validation_errors.append(str(ex))

# NOTE: we need to access binaries to get paths and resolvers, before validating.
for binary, binary_checker in self.binaries.items():
for binary, binary_checker in binaries.items():
invalid_paths[binary] = []
try:
exec_paths = (
Expand Down Expand Up @@ -103,8 +117,8 @@ def wrapper(self, *args, **kwargs): # pylint: disable=too-many-statements
workflow_name=self.NAME, action_name="Validation", reason="\n".join(validation_errors)
)

if len(self.binaries) != len(valid_paths):
validation_failed_binaries = set(self.binaries.keys()).difference(valid_paths.keys())
if len(binaries) != len(valid_paths):
validation_failed_binaries = set(binaries.keys()).difference(valid_paths.keys())
for validation_failed_binary in validation_failed_binaries:
message = "Binary validation failed for {0}, searched for {0} in following locations : {1} which did not satisfy constraints for runtime: {2}. Do you have {0} for runtime: {2} on your PATH?".format(
validation_failed_binary, invalid_paths[validation_failed_binary], self.runtime
Expand Down Expand Up @@ -330,6 +344,10 @@ def get_validators(self):
"""
return [RuntimeValidator(runtime=self.runtime, architecture=self.architecture)]

def get_runtime_validator(self):
"""Return the validator used when the workflow does not require any binaries."""
return RuntimeValidator(runtime=self.runtime, architecture=self.architecture)

@property
def binaries(self):
if not self._binaries:
Expand Down
10 changes: 9 additions & 1 deletion aws_lambda_builders/workflows/nodejs_npm/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
if osutils is None:
osutils = OSUtils()
self.osutils = osutils
self._use_npm = osutils.file_exists(manifest_path)

if not osutils.file_exists(manifest_path):
if not self._use_npm:
LOG.warning("package.json file not found. Continuing the build without dependencies.")
self.actions = [CopySourceAction(source_dir, artifacts_dir, excludes=self.EXCLUDED_FILES)]
return
Expand Down Expand Up @@ -210,8 +211,15 @@ def get_resolvers(self):
"""
specialized path resolver that just returns the list of executable for the runtime on the path.
"""
if not self._use_npm:
return []
return [PathResolver(runtime=self.runtime, binary="npm")]

def get_validators(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 no package.json → previously WorkflowFailedError wrapping "Runtime nodejs14.x is not supported", now a plain source copy.
  • runtime="ruby2.7" with download_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)]
   return

Note 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.

if not self._use_npm:
return []
return super().get_validators()

@staticmethod
def get_install_action(
source_dir: str,
Expand Down
4 changes: 4 additions & 0 deletions aws_lambda_builders/workflows/python_uv/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,3 +173,7 @@ def get_validators(self):
external validation of Python runtime paths.
"""
return []

def get_runtime_validator(self):
"""UV manages the requested Python runtime without BaseWorkflow validation."""
return None
12 changes: 12 additions & 0 deletions aws_lambda_builders/workflows/ruby_bundler/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
source_dir, artifacts_dir, scratch_dir, manifest_path, runtime=runtime, **kwargs
)

self._use_bundler = self.download_dependencies

if osutils is None:
osutils = OSUtils()

Expand Down Expand Up @@ -61,3 +63,13 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
"download_dependencies is False and dependencies_dir is None. Copying the source files into the "
"artifacts directory. "
)

def get_resolvers(self):
if not self._use_bundler:
return []
return super().get_resolvers()

def get_validators(self):
if not self._use_bundler:
return []
return super().get_validators()
7 changes: 6 additions & 1 deletion tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,12 @@ def test_builds_project_without_dependencies(self, runtime):
def test_builds_project_without_manifest(self, runtime):
source_dir = os.path.join(self.TEST_DATA_FOLDER, "no-manifest")

with mock.patch.object(logger, "warning") as mock_warning:
with (
mock.patch.object(logger, "warning") as mock_warning,
mock.patch(
"aws_lambda_builders.path_resolver.which", side_effect=AssertionError("npm should not be resolved")
),
):
self.builder.build(
source_dir,
self.artifacts_dir,
Expand Down
7 changes: 6 additions & 1 deletion tests/integration/workflows/ruby_bundler/test_ruby.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,12 @@ def test_builds_project_with_downloaded_dependencies_and_dependencies_dir(self):

def test_builds_project_without_downloaded_dependencies_without_dependencies_dir(self):
source_dir = os.path.join(self.TEST_DATA_FOLDER, "with-deps")
with mock.patch.object(workflow_logger, "info") as mock_info:
with (
mock.patch.object(workflow_logger, "info") as mock_info,
mock.patch(
"aws_lambda_builders.path_resolver.which", side_effect=AssertionError("Ruby should not be resolved")
),
):
self.builder.build(
source_dir,
self.artifacts_dir,
Expand Down
53 changes: 53 additions & 0 deletions tests/unit/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,59 @@ def test_must_raise_for_incompatible_runtime_and_architecture(self):

self.assertIn("Architecture invalid_arch is not supported for runtime python3.12", str(ex.exception))

def test_empty_binaries_with_supported_runtime_executes_actions(self):
self.work.runtime = "python3.12"
self.work.architecture = "arm64"
self.work.get_resolvers = Mock(return_value=[])
self.work.get_validators = Mock(return_value=[])
action_mock = Mock()
self.work.actions = [action_mock]

self.work.run()

self.work.get_resolvers.assert_called_once_with()
self.work.get_validators.assert_called_once_with()
action_mock.execute.assert_called_once_with()

def test_empty_binaries_with_unsupported_runtime_raises_workflow_failed_error(self):
self.work.runtime = "python1.0"
self.work.get_resolvers = Mock(return_value=[])
self.work.get_validators = Mock(return_value=[])
self.work.actions = [Mock()]

with self.assertRaises(WorkflowFailedError) as raised:
self.work.run()

self.assertEqual(str(raised.exception), "MyWorkflow:Validation - Runtime python1.0 is not supported")

def test_empty_binaries_with_unsupported_architecture_raises_workflow_failed_error(self):
self.work.runtime = "python3.12"
self.work.architecture = "invalid_arch"
self.work.get_resolvers = Mock(return_value=[])
self.work.get_validators = Mock(return_value=[])
self.work.actions = [Mock()]

with self.assertRaises(WorkflowFailedError) as raised:
self.work.run()

self.assertEqual(
str(raised.exception),
"MyWorkflow:Validation - Architecture invalid_arch is not supported for runtime python3.12",
)

def test_empty_binaries_can_skip_runtime_validation(self):
self.work.runtime = "unsupported"
self.work.get_resolvers = Mock(return_value=[])
self.work.get_validators = Mock(return_value=[])
self.work.get_runtime_validator = Mock(return_value=None)
action_mock = Mock()
self.work.actions = [action_mock]

self.work.run()

self.work.get_runtime_validator.assert_called_once_with()
action_mock.execute.assert_called_once_with()


class TestBaseWorkflow_repr(TestCase):
class MyWorkflow(BaseWorkflow):
Expand Down
65 changes: 65 additions & 0 deletions tests/unit/workflows/nodejs_npm/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
MoveDependenciesAction,
)
from aws_lambda_builders.architecture import ARM64
from aws_lambda_builders.exceptions import WorkflowFailedError
from aws_lambda_builders.path_resolver import PathResolver
from aws_lambda_builders.validator import RuntimeValidator
from aws_lambda_builders.workflows.nodejs_npm.workflow import NodejsNpmWorkflow
from aws_lambda_builders.workflows.nodejs_npm.actions import (
NodejsNpmPackAction,
Expand Down Expand Up @@ -206,6 +209,68 @@ def test_workflow_sets_up_npm_actions_without_download_dependencies_and_without_
self.assertIsInstance(workflow.actions[3], NodejsNpmrcCleanUpAction)
self.assertIsInstance(workflow.actions[4], NodejsNpmLockFileCleanUpAction)

def test_workflow_without_manifest_skips_npm_resolution_and_validation(self):
self.osutils.file_exists.return_value = False

workflow = NodejsNpmWorkflow(
"source", "artifacts", "scratch_dir", "source/manifest", runtime="nodejs20.x", osutils=self.osutils
)

self.assertEqual(workflow.get_resolvers(), [])
self.assertEqual(workflow.get_validators(), [])

def test_workflow_with_manifest_retains_npm_resolution_and_validation(self):
self.osutils.file_exists.return_value = True

workflow = NodejsNpmWorkflow(
"source",
"artifacts",
"scratch_dir",
"source/manifest",
runtime="nodejs20.x",
download_dependencies=False,
osutils=self.osutils,
)

resolvers = workflow.get_resolvers()
validators = workflow.get_validators()
self.assertEqual(len(resolvers), 1)
self.assertIsInstance(resolvers[0], PathResolver)
self.assertEqual(resolvers[0].binary, "npm")
self.assertEqual(len(validators), 1)
self.assertIsInstance(validators[0], RuntimeValidator)

def test_workflow_without_manifest_rejects_unsupported_runtime(self):
self.osutils.file_exists.return_value = False

workflow = NodejsNpmWorkflow(
"source", "artifacts", "scratch_dir", "source/manifest", runtime="nodejs1.x", osutils=self.osutils
)
with self.assertRaises(WorkflowFailedError) as raised:
workflow.run()

self.assertEqual(str(raised.exception), "NodejsNpmBuilder:Validation - Runtime nodejs1.x is not supported")

def test_workflow_without_manifest_rejects_unsupported_architecture(self):
self.osutils.file_exists.return_value = False

workflow = NodejsNpmWorkflow(
"source",
"artifacts",
"scratch_dir",
"source/manifest",
runtime="nodejs20.x",
architecture="invalid_arch",
osutils=self.osutils,
)
with self.assertRaises(WorkflowFailedError) as raised:
workflow.run()

self.assertEqual(
str(raised.exception),
"NodejsNpmBuilder:Validation - Architecture invalid_arch is not supported for runtime nodejs20.x",
)

def test_workflow_sets_up_npm_actions_without_combine_dependencies(self):
self.osutils.file_exists.side_effect = [True, False, False]

Expand Down
27 changes: 27 additions & 0 deletions tests/unit/workflows/python_uv/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,33 @@ def test_get_validators(self):
# UV has built-in Python version handling, no external validators needed
self.assertEqual(len(validators), 0)

def test_get_runtime_validator(self):
self.assertIsNone(self.workflow.get_runtime_validator())

def test_supported_runtime_runs_without_binary_resolution(self):
action_mock = Mock()
self.workflow.actions = [action_mock]

with patch("aws_lambda_builders.path_resolver.which", side_effect=AssertionError("binary resolution called")):
self.workflow.run()

action_mock.execute.assert_called_once_with()

def test_runtime_validation_opt_out_preserves_previous_behavior(self):
for runtime, architecture in (("python1.0", "x86_64"), ("python3.9", "invalid_arch"), (None, "x86_64")):
with self.subTest(runtime=runtime, architecture=architecture):
action_mock = Mock()
self.workflow.runtime = runtime
self.workflow.architecture = architecture
self.workflow.actions = [action_mock]

with patch(
"aws_lambda_builders.path_resolver.which", side_effect=AssertionError("binary resolution called")
):
self.workflow.run()

action_mock.execute.assert_called_once_with()

@patch("aws_lambda_builders.workflows.python_uv.workflow.detect_uv_manifest")
def test_workflow_auto_detects_manifest(self, mock_detect):
mock_detect.return_value = "/path/to/pyproject.toml"
Expand Down
66 changes: 66 additions & 0 deletions tests/unit/workflows/ruby_bundler/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

from aws_lambda_builders.actions import CopySourceAction, CopyDependenciesAction, CleanUpAction
from aws_lambda_builders.architecture import ARM64
from aws_lambda_builders.exceptions import WorkflowFailedError
from aws_lambda_builders.path_resolver import PathResolver
from aws_lambda_builders.validator import RuntimeValidator
from aws_lambda_builders.workflows.ruby_bundler.workflow import RubyBundlerWorkflow
from aws_lambda_builders.workflows.ruby_bundler.actions import RubyBundlerInstallAction, RubyBundlerVendorAction

Expand Down Expand Up @@ -44,6 +47,69 @@ def test_workflow_sets_up_bundler_actions_without_download_dependencies_without_

self.assertIsInstance(workflow.actions[0], CopySourceAction)

def test_workflow_without_download_dependencies_skips_ruby_resolution_and_validation(self):
workflow = RubyBundlerWorkflow(
"source",
"artifacts",
"scratch_dir",
"manifest",
runtime="ruby3.3",
download_dependencies=False,
)

self.assertEqual(workflow.get_resolvers(), [])
self.assertEqual(workflow.get_validators(), [])

def test_workflow_with_download_dependencies_retains_ruby_resolution_and_validation(self):
workflow = RubyBundlerWorkflow(
"source",
"artifacts",
"scratch_dir",
"manifest",
runtime="ruby3.3",
download_dependencies=True,
)

resolvers = workflow.get_resolvers()
validators = workflow.get_validators()
self.assertEqual(len(resolvers), 1)
self.assertIsInstance(resolvers[0], PathResolver)
self.assertEqual(resolvers[0].binary, "ruby")
self.assertEqual(len(validators), 1)
self.assertIsInstance(validators[0], RuntimeValidator)

def test_workflow_without_download_dependencies_rejects_unsupported_runtime(self):
workflow = RubyBundlerWorkflow(
"source",
"artifacts",
"scratch_dir",
"manifest",
runtime="ruby1.0",
download_dependencies=False,
)
with self.assertRaises(WorkflowFailedError) as raised:
workflow.run()

self.assertEqual(str(raised.exception), "RubyBundlerBuilder:Validation - Runtime ruby1.0 is not supported")

def test_workflow_without_download_dependencies_rejects_unsupported_architecture(self):
workflow = RubyBundlerWorkflow(
"source",
"artifacts",
"scratch_dir",
"manifest",
runtime="ruby3.3",
architecture="invalid_arch",
download_dependencies=False,
)
with self.assertRaises(WorkflowFailedError) as raised:
workflow.run()

self.assertEqual(
str(raised.exception),
"RubyBundlerBuilder:Validation - Architecture invalid_arch is not supported for runtime ruby3.3",
)

def test_must_validate_architecture(self):
workflow = RubyBundlerWorkflow(
"source",
Expand Down