Fix #1463. Allow to expose multiple ports when running in debug mode #1479

jfuss · 2019-10-23T20:54:52Z

If -d is passed as a string, would this cause any issues? I understand why you would add this but not sure if this affects if the integer was pass as a string ("33333" for example). I want to make sure we don't accidentally break the existing contract here.

I didn't find any specific information on click, but will the debug-port always be a list when passed to our cli methods? I am having a hard time understanding how type in impacts what click will end up passing us.

No issues with passing integer as a string here. Looked to the click parameters type processing logic - it tries to cast a value to integer. Verified it works well with strings unless you specify an invalid value, e.g. "abc" that sounds like an expected behavior.

Looked through click implementation for handling multiple flag - it wraps values into tuple if flag is set and return them or return an empty tuple if no values. So, I would expect we always have tuple after processing --debug-port option. Verified with single and multiple values.

Perfect. Thanks for digging into that.

jfuss · 2019-10-25T12:59:41Z

This check is fine but I think it is not needed. __bool__ on DebugContext is bool(self.debug_ports). This should never execute given the first if statement (if not debug_options).

-Original file line number
+Diff line change
@@ -0,0 +1,121 @@
+    Title: Template for design documents
+    ====================================
+    What is the problem?
+    --------------------
+    Currently, there is one port is exposed from Docker instance when running lambda in debug mode.
+    This port is used to connect a debugger. In my case, I need two ports to be exposed due to Debugger
+    implementation specific (the Debugger connect to two sockets to collect different information).
+    What will be changed?
+    ---------------------
+    SAM CLI has a ``--debug-port`` parameter that provide a port. This parameter is stored in DebugContext object.
+    ``DebugContext`` should store an array of ports instead of a single port. This array should be transformed
+    into a map containing each stored port when passing to docker container arguments.
+    Success criteria for the change
+    -------------------------------
+    All ports specified via single or multiple ``--debug-port`` SAM CLI options should be exposed by docker container.
+    Out-of-Scope
+    ------------
+    User Experience Walkthrough
+    ---------------------------
+    From the user perspective, it should only provide an ability to specify multiple ``--debug-port`` options:
+    ``--debug-port 5600 --debug-port 5601``
+    Implementation
+    ==============
+    CLI Changes
+    -----------
+    SAM CLI provide an option to specify multiple ports ``--debug-port 5600 --debug-port 5601``.
+    ### Breaking Change
+    No changes.
+    Design
+    ------
+    Update ``--debug-port`` option to allow to use it multiple times in SAM CLI.
+    The option type should take only integer values. The value is stored in ``DebugContext``.
+    This value should be converted into a map of ``{ container_port : host_port }``
+    that is passed to ``ports`` argument when creating a docker container.
+    `.samrc` Changes
+    ----------------
+    No changes.
+    Security
+    --------
+    No changes.
+    **What new dependencies (libraries/cli) does this change require?**
+    **What other Docker container images are you using?**
+    **Are you creating a new HTTP endpoint? If so explain how it will be
+    created & used**
+    **Are you connecting to a remote API? If so explain how is this
+    connection secured**
+    **Are you reading/writing to a temporary folder? If so, what is this
+    used for and when do you clean up?**
+    **How do you validate new .samrc configuration?**
+    What is your Testing Plan (QA)?
+    ===============================
+    Goal
+    ----
+    Make sure SAM CLI users can specify multiple ports and those ports are exposed
+    after creating a docker container in debug mode:
+    ``sam local invoke --template <path_to_template>/template.yaml --event <path_to_event>/event.json --debugger-path <path_to_debugger> --debug-port 5600 --debug-port 5601``
+    Pre-requesites
+    --------------
+    Running SAM CLI with debug mode.
+    Test Scenarios/Cases
+    --------------------
+. Single port is specified: ``--debug-port 5600``
+. Multiple ports are specified: ``--debug-port 5600 --debug-port 5601``
+. No ports specified: ``--debug-port ``
+. No ``--debug-port`` parameter is specified
+    Expected Results
+    ----------------
+. Single port is exposed in docker container
+. All specified ports are exposed in docker container
+. No ports exposed.
+. No ports exposed.
+    Pass/Fail
+    ---------
+    Documentation Changes
+    =====================
+    Open Issues
+    ============
+    - [1463](https://github.com/awslabs/aws-sam-cli/issues/1463)
+    Task Breakdown
+    ==============
+    -   \[x\] Send a Pull Request with this design document
+    -   \[ \] Build the command line interface
+    -   \[ \] Build the underlying library
+    -   \[x\] Unit tests
+    -   \[x\] Functional Tests
+    -   \[x\] Integration tests
+    -   \[ \] Run all tests on Windows
+    -   \[x\] Update documentation

-Original file line number
+Diff line change
@@ Expand Up / @@ -45,7 +45,7 @@ def __init__( @@
             docker_network=None,
             log_file=None,
             skip_pull_image=None,
-            debug_port=None,
+            debug_ports=None,
             debug_args=None,
             debugger_path=None,
             parameter_overrides=None,
@@ Expand Down Expand Up / @@ -76,8 +76,8 @@ def __init__( @@
                 Should we skip pulling the Docker container image?
             aws_profile str
                 Name of the profile to fetch AWS credentials from
-            debug_port int
-                Port to bind the debugger to
+            debug_ports tuple(int)
+                Ports to bind the debugger to
             debug_args str
                 Additional arguments passed to the debugger
             debugger_path str
@@ Expand All / @@ -98,7 +98,7 @@ def __init__( @@
             self._docker_network = docker_network
             self._log_file = log_file
             self._skip_pull_image = skip_pull_image
-            self._debug_port = debug_port
+            self._debug_ports = debug_ports
             self._debug_args = debug_args
             self._debugger_path = debugger_path
             self._parameter_overrides = parameter_overrides or {}
@@ Expand Down Expand Up / @@ -129,7 +129,7 @@ def __enter__(self): @@
             self._env_vars_value = self._get_env_vars_value(self._env_vars_file)
             self._log_file_handle = self._setup_log_file(self._log_file)
-            self._debug_context = self._get_debug_context(self._debug_port, self._debug_args, self._debugger_path)
+            self._debug_context = self._get_debug_context(self._debug_ports, self._debug_args, self._debugger_path)
             self._container_manager = self._get_container_manager(self._docker_network, self._skip_pull_image)
@@ Expand Down Expand Up / @@ -314,14 +314,14 @@ def _setup_log_file(log_file): @@
             return open(log_file, "wb")
         @staticmethod
-        def _get_debug_context(debug_port, debug_args, debugger_path):
+        def _get_debug_context(debug_ports, debug_args, debugger_path):
             """
             Creates a DebugContext if the InvokeContext is in a debugging mode
             Parameters
             ----------
-            debug_port int
-                 Port to bind the debugger to
+            debug_ports tuple(int)
+                 Ports to bind the debugger to
             debug_args str
                 Additional arguments passed to the debugger
             debugger_path str
@@ Expand All @@
             samcli.commands.local.cli_common.user_exceptions.DebugContext
                 When the debugger_path is not valid
             """
-            if debug_port and debugger_path:
+            if debug_ports and debugger_path:
                 try:
                     debugger = Path(debugger_path).resolve(strict=True)
                 except OSError as error:
@@ Expand All @@
                     raise DebugContextException("'{}' should be a directory with the debugger in it.".format(debugger_path))
                 debugger_path = str(debugger)
-            return DebugContext(debug_port=debug_port, debug_args=debug_args, debugger_path=debugger_path)
+            return DebugContext(debug_ports=debug_ports, debug_args=debug_args, debugger_path=debugger_path)
         @staticmethod
         def _get_container_manager(docker_network, skip_pull_image):
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -92,6 +92,8 @@ def invoke_common_options(f): @@
                     help="When specified, Lambda function container will start in debug mode and will expose this "
                     "port on localhost.",
                     envvar="SAM_DEBUG_PORT",
+                    type=click.INT,
+                    multiple=True,
                 ),
                 click.option(
                     "--debugger-path", help="Host path to a debugger that will be mounted into the Lambda container."
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -136,7 +136,7 @@ def do_cli( # pylint: disable=R0914 @@
                 docker_network=docker_network,
                 log_file=log_file,
                 skip_pull_image=skip_pull_image,
-                debug_port=debug_port,
+                debug_ports=debug_port,
                 debug_args=debug_args,
                 debugger_path=debugger_path,
                 parameter_overrides=parameter_overrides,
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -4,14 +4,21 @@ @@
     class DebugContext:
-        def __init__(self, debug_port=None, debugger_path=None, debug_args=None):
+        def __init__(self, debug_ports=None, debugger_path=None, debug_args=None):
+            """
+            Initialize the Debug Context with Lambda debugger options
-            self.debug_port = debug_port
+            :param tuple(int) debug_ports: Collection of debugger ports to be exposed from a docker container
+            :param Path debugger_path: Path to a debugger to be launched
+            :param string debug_args: Additional arguments to be passed to the debugger
+            """
+            self.debug_ports = debug_ports
             self.debugger_path = debugger_path
             self.debug_args = debug_args
         def __bool__(self):
-            return bool(self.debug_port)
+            return bool(self.debug_ports)
         def __nonzero__(self):
             return self.__bool__()

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #1463. Allow to expose multiple ports when running in debug mode #1479

Uh oh!

Diff view

Diff view

There are no files selected for viewing

jfuss Oct 23, 2019

Uh oh!

sdubov Oct 24, 2019 •

edited

Loading

Uh oh!

jfuss Oct 24, 2019

Uh oh!

jfuss Oct 25, 2019

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ Expand Up / @@ -40,9 +40,10 @@ def __init__( @@
             :param samcli.commands.local.lib.provider.FunctionProvider function_provider: Provider that can return a
                 Lambda function
             :param string cwd: Current working directory. We will resolve all function CodeURIs relative to this directory.
-            :param dict env_vars_values: Optional. Dictionary containing values of environment variables
-            :param integer debug_port: Optional. Port to bind the debugger to
-            :param string debug_args: Optional. Additional arguments passed to the debugger
+            :param string aws_profile: Optional. Name of the profile to fetch AWS credentials from.
+            :param string aws_region: Optional. AWS Region to use.
+            :param dict env_vars_values: Optional. Dictionary containing values of environment variables.
+            :param DebugContext debug_context: Optional. Debug context for the function (includes port, args, and path).
             """
             self.local_runtime = local_runtime
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -92,20 +92,25 @@ def __init__( @@
         @staticmethod
         def _get_exposed_ports(debug_options):
             """
-            Return Docker container port binding information. If a debug port is given, then we will ask Docker to
-            bind to same port both inside and outside the container ie. Runtime process is started in debug mode with
+            Return Docker container port binding information. If a debug port tuple is given, then we will ask Docker to
+            bind every given port to same port both inside and outside the container ie. Runtime process is started in debug mode with
             at given port inside the container and exposed to the host machine at the same port
-            :param int debug_port: Optional, integer value of debug port
+            :param DebugContext debug_options: Debugging options for the function (includes debug port, args, and path)
             :return dict: Dictionary containing port binding information. None, if debug_port was not given
             """
             if not debug_options:
                 return None
-            return {
-                # container port : host port
-                debug_options.debug_port: debug_options.debug_port
-            }
+            if not debug_options.debug_ports:
+                return None
+            # container port : host port
+            ports_map = {}
+            for port in debug_options.debug_ports:
+                ports_map[port] = port
+            return ports_map
         @staticmethod
         def _get_additional_options(runtime, debug_options):
@@ Expand Down Expand Up @@
             Dockerfile. We override this default specifically when enabling debugging. The overridden entry point includes
             a few extra flags to start the runtime in debug mode.
-            :param string runtime: Lambda function runtime name
-            :param int debug_port: Optional, port for debugger
-            :param string debug_args: Optional additional arguments passed to the entry point.
+            :param string runtime: Lambda function runtime name.
+            :param DebugContext debug_options: Optional. Debug context for the function (includes port, args, and path).
             :return list: List containing the new entry points. Each element in the list is one portion of the command.
                 ie. if command is ``node index.js arg1 arg2``, then this list will be ["node", "index.js", "arg1", "arg2"]
             """
             if not debug_options:
                 return None
-            debug_port = debug_options.debug_port
+            debug_ports = debug_options.debug_ports
+            if not debug_ports:
+                return None
+            debug_port = debug_ports[0]
             debug_args_list = []
             if debug_options.debug_args:
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -51,8 +51,8 @@ def setUp(self): @@
             self.expected_docker_image = self.IMAGE_NAME
             self.handler = "index.handler"
             self.layers = []
-            self.debug_port = _rand_port()
-            self.debug_context = DebugContext(debug_port=self.debug_port, debugger_path=None, debug_args=None)
+            self.debug_port = [_rand_port()]
+            self.debug_context = DebugContext(debug_ports=self.debug_port, debugger_path=None, debug_args=None)
             self.code_dir = nodejs_lambda(self.HELLO_WORLD_CODE)
             self.network_prefix = "sam_cli_test_network"
@@ Expand Down @@

Fix #1463. Allow to expose multiple ports when running in debug mode #1479

Uh oh!

Fix #1463. Allow to expose multiple ports when running in debug mode #1479

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

jfuss Oct 23, 2019

Choose a reason for hiding this comment

Uh oh!

sdubov Oct 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jfuss Oct 24, 2019

Choose a reason for hiding this comment

Uh oh!

jfuss Oct 25, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sdubov Oct 24, 2019 •

edited

Loading