Skip to content
Merged
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
60 changes: 60 additions & 0 deletions samcli/local/docker/lambda_build_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,18 @@ def __init__(self, # pylint: disable=too-many-locals

container_dirs = LambdaBuildContainer._get_container_dirs(source_dir, manifest_dir)

# `executable_search_paths` are provided as a list of paths on the host file system that needs to passed to
Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect 🔥

# the builder. But these paths don't exist within the container. We use the following method to convert the
# host paths to container paths. But if a host path is NOT mounted within the container, we will simply ignore
# it. In essence, only when the path is already in the mounted path, can the path resolver within the
# container even find the executable.
executable_search_paths = LambdaBuildContainer._convert_to_container_dirs(
host_paths_to_convert=executable_search_paths,
host_to_container_path_mapping={
source_dir: container_dirs["source_dir"],
manifest_dir: container_dirs["manifest_dir"]
})

request_json = self._make_request(protocol_version,
language,
dependency_manager,
Expand Down Expand Up @@ -163,6 +175,54 @@ def _get_container_dirs(source_dir, manifest_dir):

return result

@staticmethod
def _convert_to_container_dirs(host_paths_to_convert, host_to_container_path_mapping):
"""
Use this method to convert a list of host paths to a list of equivalent paths within the container
where the given host path is mounted. This is necessary when SAM CLI needs to pass path information to
the Lambda Builder running within the container.

If a host path is not mounted within the container, then this method simply passes the path to the result
Copy link
Contributor

Choose a reason for hiding this comment

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

ok this is a passthrough, but searching this path within the container will not turn up anything right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct

without any changes.

Ex:
[ "/home/foo", "/home/bar", "/home/not/mounted"] => ["/tmp/source", "/tmp/manifest", "/home/not/mounted"]

Parameters
----------
host_paths_to_convert : list
List of paths in host that needs to be converted

host_to_container_path_mapping : dict
Mapping of paths in host to the equivalent paths within the container

Returns
-------
list
Equivalent paths within the container
"""

if not host_paths_to_convert:
# Nothing to do
return host_paths_to_convert

# Make sure the key is absolute host path. Relative paths are tricky to work with because two different
# relative paths can point to the same directory ("../foo", "../../foo")
mapping = {str(pathlib.Path(p).resolve()): v for p, v in host_to_container_path_mapping.items()}

result = []
for original_path in host_paths_to_convert:
abspath = str(pathlib.Path(original_path).resolve())

if abspath in mapping:
Copy link
Contributor

Choose a reason for hiding this comment

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

In other words, if the path that is supplied to be added as executable_search_path within the container is already present in the mapping of paths to be mounted, we add the path to be mounted within the container to list of executable search paths.

Also this is very opinionated to adding just source_dir or manifest_dir within the container to search paths, and not any other sub paths.

eg: instead of just /tmp/source_dir and /tmp/manifest_dir , we dont yet support /tmp/source_dir/myexecutable_versions/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, we don't support that. This is a good call out. We should be okay for the gradlew case now, but we definitely need to implement a generic path-translation logic that handles lot of these cases

result.append(mapping[abspath])
Copy link
Contributor

Choose a reason for hiding this comment

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

This is under the assumption that, the base path we are looking for, is already mounted within the container correct?

eg:
/a/b is already mounted within container, and we are just passing additional /a/b/c as additional path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, result is a list. I am just adding elements to the list without altering the container paths themselves.

else:
result.append(original_path)
LOG.debug("Cannot convert host path '%s' to its equivalent path within the container. "
"Host path is not mounted within the container", abspath)

return result

@staticmethod
def _get_image(runtime):
return "{}:build-{}".format(LambdaBuildContainer._IMAGE_REPO_NAME, runtime)
46 changes: 46 additions & 0 deletions tests/unit/local/docker/test_lambda_build_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
"""

import json
try:
import pathlib
except ImportError:
import pathlib2 as pathlib


from unittest import TestCase
from mock import patch
Expand Down Expand Up @@ -157,3 +162,44 @@ class TestLambdaBuildContainer_get_entrypoint(TestCase):
def test_must_get_entrypoint(self):
self.assertEquals(["lambda-builders", "requestjson"],
LambdaBuildContainer._get_entrypoint("requestjson"))


class TestLambdaBuildContainer_convert_to_container_dirs(TestCase):

def test_must_work_on_abs_and_relative_paths(self):

input = [".", "../foo", "/some/abs/path"]
mapping = {
str(pathlib.Path(".").resolve()): "/first",
"../foo": "/second",
"/some/abs/path": "/third"
}

expected = ["/first", "/second", "/third"]
result = LambdaBuildContainer._convert_to_container_dirs(input, mapping)

self.assertEquals(result, expected)

def test_must_skip_unknown_paths(self):

input = ["/known/path", "/unknown/path"]
mapping = {
"/known/path": "/first"
}

expected = ["/first", "/unknown/path"]
result = LambdaBuildContainer._convert_to_container_dirs(input, mapping)

self.assertEquals(result, expected)

def test_must_skip_on_empty_input(self):

input = None
mapping = {
"/known/path": "/first"
}

expected = None
result = LambdaBuildContainer._convert_to_container_dirs(input, mapping)

self.assertEquals(result, expected)