feat(Public ECR): Build Images (#2793)

* feat(Public ECR): Changed Build Image Registry to Public ECR (#2708) * Changed Build Image Registry to Public ECR * Added _IMAGE_URI_PREFIX * Fixed Unit Tests * Updated Image Format * feat(Public ECR): Updated Test Image Name and Added Latest Tag Check (#2720) * Added Tests for Latest Tag * Added use_container Check for Verify Latest Tag Tests * Fixed DotNet and Go Build Tests * Added Missing use_container Check for test_build_function_and_layer * feat(Public ECR): Changed Build Image Registry to Public ECR (#2708) * Changed Build Image Registry to Public ECR * Added _IMAGE_URI_PREFIX * Fixed Unit Tests * Updated Image Format * feat(Public ECR): Updated Test Image Name and Added Latest Tag Check (#2720) * Added Tests for Latest Tag * Added use_container Check for Verify Latest Tag Tests * Fixed DotNet and Go Build Tests * Added Missing use_container Check for test_build_function_and_layer * feat(Public ECR): Added --build-image Options (#2725) * Added Latest Tag Check * Added --build-image Option * Added Unit Tests * Added Help Text and Fixed PyLint * Fixed Path Validation for Unit Tests * Updated Type Hintings * Added _parse_key_value_pair * Updated tag Handling for Image Pulls * Added Throwing Exception with Invalid Build Images * Fixed PyLint Issue * Feat: added integration tests, container tag check and layer support (#2780) * Added integration tests * Added click requirement on -u presence for container based options * Added click class that checks -u presence for container based options * Reformatting and fixing test cases * Fix for integration test failures * Added support for layers * Resolve conflict * Refactoring file location * Addressing review comments, refactoring * Reformatting logging sentence * Refactoring container check for simplicity * Clarifying test case * Added unit tests on layer builds * Improving readability * Refactoring to simplify * Removing unnecessary lines * Refactoring integration test * Shortening error message * Adjusting test behaviour * Added new unit tests * Test refactoring * Added new test class * Removed unused import * Improving help text Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>
aws · Apr 5, 2021 · 54644dd · 54644dd
1 parent 4543732
commit 54644dd
Show file tree

Hide file tree

Showing 16 changed files with 550 additions and 80 deletions.
diff --git a/samcli/commands/build/build_context.py b/samcli/commands/build/build_context.py
@@ -44,6 +44,7 @@ def __init__(
         skip_pull_image: bool = False,
         container_env_var: Optional[dict] = None,
         container_env_var_file: Optional[str] = None,
+        build_images: Optional[dict] = None,
     ) -> None:
 
         self._resource_identifier = resource_identifier
@@ -65,6 +66,7 @@ def __init__(
         self._cached = cached
         self._container_env_var = container_env_var
         self._container_env_var_file = container_env_var_file
+        self._build_images = build_images
 
         self._function_provider: Optional[SamFunctionProvider] = None
         self._layer_provider: Optional[SamLayerProvider] = None

diff --git a/samcli/commands/build/click_container.py b/samcli/commands/build/click_container.py
@@ -0,0 +1,19 @@
+"""
+Module to check container based cli parameters
+"""
+import click
+
+
+class ContainerOptions(click.Option):
+    """
+    Preprocessing checks for presence of --use-container flag for container based options.
+    """
+
+    def handle_parse_result(self, ctx, opts, args):
+        if "use_container" not in opts and self.name in opts:
+            opt_name = self.name.replace("_", "-")
+            msg = f"Missing required parameter, need the --use-container flag in order to use --{opt_name} flag."
+            raise click.UsageError(msg)
+        # To make sure no unser input prompting happens
+        self.prompt = None
+        return super().handle_parse_result(ctx, opts, args)
diff --git a/samcli/commands/build/command.py b/samcli/commands/build/command.py
@@ -4,7 +4,7 @@
 
 import os
 import logging
-from typing import List, Optional, Dict
+from typing import List, Optional, Dict, Tuple
 import click
 
 from samcli.cli.context import Context
@@ -19,6 +19,8 @@
 from samcli.lib.telemetry.metric import track_command
 from samcli.cli.cli_config_file import configuration_option, TomlProvider
 from samcli.lib.utils.version_checker import check_newer_version
+from samcli.commands.build.exceptions import InvalidBuildImageException
+from samcli.commands.build.click_container import ContainerOptions
 
 LOG = logging.getLogger(__name__)
 
@@ -116,13 +118,30 @@
     help="Input environment variables through command line to pass into build containers, you can either "
     "input function specific format (FuncName.VarName=Value) or global format (VarName=Value). e.g., "
     "sam build --use-container --container-env-var Func1.VAR1=value1 --container-env-var VAR2=value2",
+    cls=ContainerOptions,
 )
 @click.option(
     "--container-env-var-file",
     "-ef",
     default=None,
     type=click.Path(),  # Must be a json file
     help="Path to environment variable json file (e.g., env_vars.json) to pass into build containers",
+    cls=ContainerOptions,
+)
+@click.option(
+    "--build-image",
+    "-bi",
+    default=None,
+    multiple=True,  # Can pass in multiple build images
+    required=False,
+    help="Container image URIs for building functions/layers. "
+    "You can specify for all functions/layers with just the image URI "
+    "(--build-image public.ecr.aws/sam/build-nodejs14.x:latest). "
+    "You can specify for each individual function with "
+    "(--build-image FunctionLogicalID=public.ecr.aws/sam/build-nodejs14.x:latest). "
+    "A combination of the two can be used. If a function does not have build image specified or "
+    "an image URI for all functions, the default SAM CLI build images will be used.",
+    cls=ContainerOptions,
 )
 @click.option(
     "--parallel",
@@ -173,8 +192,9 @@ def cli(
     parallel: bool,
     manifest: Optional[str],
     docker_network: Optional[str],
-    container_env_var: Optional[List[str]],
+    container_env_var: Optional[Tuple[str]],
     container_env_var_file: Optional[str],
+    build_image: Optional[Tuple[str]],
     skip_pull_image: bool,
     parameter_overrides: dict,
     config_file: str,
@@ -204,6 +224,7 @@ def cli(
         mode,
         container_env_var,
         container_env_var_file,
+        build_image,
     )  # pragma: no cover
 
 
@@ -222,8 +243,9 @@ def do_cli(  # pylint: disable=too-many-locals, too-many-statements
     skip_pull_image: bool,
     parameter_overrides: Dict,
     mode: Optional[str],
-    container_env_var: Optional[List[str]],
+    container_env_var: Optional[Tuple[str]],
     container_env_var_file: Optional[str],
+    build_image: Optional[Tuple[str]],
 ) -> None:
     """
     Implementation of the ``cli`` method
@@ -250,6 +272,7 @@ def do_cli(  # pylint: disable=too-many-locals, too-many-statements
         LOG.info("Starting Build inside a container")
 
     processed_env_vars = _process_env_var(container_env_var)
+    processed_build_images = _process_image_options(build_image)
 
     with BuildContext(
         function_identifier,
@@ -267,6 +290,7 @@ def do_cli(  # pylint: disable=too-many-locals, too-many-statements
         mode=mode,
         container_env_var=processed_env_vars,
         container_env_var_file=container_env_var_file,
+        build_images=processed_build_images,
     ) as ctx:
         try:
             builder = ApplicationBuilder(
@@ -282,6 +306,7 @@ def do_cli(  # pylint: disable=too-many-locals, too-many-statements
                 parallel=parallel,
                 container_env_var=processed_env_vars,
                 container_env_var_file=container_env_var_file,
+                build_images=processed_build_images,
             )
         except FunctionNotFound as ex:
             raise UserException(str(ex), wrapped_from=ex.__class__.__name__) from ex
@@ -376,12 +401,12 @@ def _get_mode_value_from_envvar(name: str, choices: List[str]) -> Optional[str]:
     return mode
 
 
-def _process_env_var(container_env_var: Optional[List[str]]) -> Dict:
+def _process_env_var(container_env_var: Optional[Tuple[str]]) -> Dict:
     """
     Parameters
     ----------
-    container_env_var : list
-        the list of command line env vars received from --container-env-var flag
+    container_env_var : Tuple
+        the tuple of command line env vars received from --container-env-var flag
         Each input format needs to be either function specific format (FuncName.VarName=Value)
         or global format (VarName=Value)
 
@@ -396,19 +421,14 @@ def _process_env_var(container_env_var: Optional[List[str]]) -> Dict:
         for env_var in container_env_var:
             location_key = "Parameters"
 
-            if "=" not in env_var:
-                LOG.error("Invalid command line --container-env-var input %s, skipped", env_var)
-                continue
-
-            key, value = env_var.split("=", 1)
-            env_var_name = key
+            env_var_name, value = _parse_key_value_pair(env_var)
 
-            if not value.strip():
+            if not env_var_name or not value:
                 LOG.error("Invalid command line --container-env-var input %s, skipped", env_var)
                 continue
 
-            if "." in key:
-                location_key, env_var_name = key.split(".", 1)
+            if "." in env_var_name:
+                location_key, env_var_name = env_var_name.split(".", 1)
                 if not location_key.strip() or not env_var_name.strip():
                     LOG.error("Invalid command line --container-env-var input %s, skipped", env_var)
                     continue
@@ -418,3 +438,53 @@ def _process_env_var(container_env_var: Optional[List[str]]) -> Dict:
             processed_env_vars[location_key][env_var_name] = value
 
     return processed_env_vars
+
+
+def _process_image_options(image_args: Optional[Tuple[str]]) -> Dict:
+    """
+    Parameters
+    ----------
+    image_args : Tuple
+        Tuple of command line image options in the format of
+        "Function1=public.ecr.aws/abc/abc:latest" or
+        "public.ecr.aws/abc/abc:latest"
+
+    Returns
+    -------
+    dictionary
+        Function as key and the corresponding image URI as value.
+        Global default image URI is contained in the None key.
+    """
+    build_images: Dict[Optional[str], str] = dict()
+    if image_args:
+        for build_image_string in image_args:
+            function_name, image_uri = _parse_key_value_pair(build_image_string)
+            if not image_uri:
+                raise InvalidBuildImageException(f"Invalid command line --build-image input {build_image_string}.")
+            build_images[function_name] = image_uri
+
+    return build_images
+
+
+def _parse_key_value_pair(arg: str) -> Tuple[Optional[str], str]:
+    """
+    Parameters
+    ----------
+    arg : str
+        Arg in the format of "Value" or "Key=Value"
+    Returns
+    -------
+    key : Optional[str]
+        If key is not specified, None will be the key.
+    value : str
+    """
+    key: Optional[str]
+    value: str
+    if "=" in arg:
+        parts = arg.split("=", 1)
+        key = parts[0].strip()
+        value = parts[1].strip()
+    else:
+        key = None
+        value = arg.strip()
+    return key, value
diff --git a/samcli/commands/build/exceptions.py b/samcli/commands/build/exceptions.py
@@ -13,3 +13,9 @@ class MissingBuildMethodException(UserException):
     """
     Exception to be thrown when a layer is tried to build without BuildMethod
     """
+
+
+class InvalidBuildImageException(UserException):
+    """
+    Value provided to --build-image is invalid
+    """
diff --git a/samcli/lib/build/app_builder.py b/samcli/lib/build/app_builder.py
@@ -67,6 +67,7 @@ def __init__(
         docker_client: Optional[docker.DockerClient] = None,
         container_env_var: Optional[Dict] = None,
         container_env_var_file: Optional[str] = None,
+        build_images: Optional[Dict] = None,
     ) -> None:
         """
         Initialize the class
@@ -103,6 +104,8 @@ def __init__(
             An optional dictionary of environment variables to pass to the container
         container_env_var_file : Optional[str]
             An optional path to file that contains environment variables to pass to the container
+        build_images : Optional[Dict]
+            An optional dictionary of build images to be used for building functions
         """
         self._resources_to_build = resources_to_build
         self._build_dir = build_dir
@@ -122,6 +125,7 @@ def __init__(
         self._colored = Colored()
         self._container_env_var = container_env_var
         self._container_env_var_file = container_env_var_file
+        self._build_images = build_images or {}
 
     def build(self) -> Dict[str, str]:
         """
@@ -428,28 +432,26 @@ def _build_layer(
 
             # By default prefer to build in-process for speed
             build_runtime = specified_workflow
-            build_method = self._build_function_in_process
+            options = ApplicationBuilder._get_build_options(layer_name, config.language, None)
             if self._container_manager:
-                build_method = self._build_function_on_container
                 if config.language == "provided":
                     LOG.warning(
                         "For container layer build, first compatible runtime is chosen as build target for container."
                     )
                     # Only set to this value if specified workflow is makefile
                     # which will result in config language as provided
                     build_runtime = compatible_runtimes[0]
-            options = ApplicationBuilder._get_build_options(layer_name, config.language, None)
+                # None key represents the global build image for all functions/layers
+                global_image = self._build_images.get(None)
+                image = self._build_images.get(layer_name, global_image)
+                self._build_function_on_container(
+                    config, code_dir, artifact_subdir, manifest_path, build_runtime, options, container_env_vars, image
+                )
+            else:
+                self._build_function_in_process(
+                    config, code_dir, artifact_subdir, scratch_dir, manifest_path, build_runtime, options
+                )
 
-            build_method(
-                config,
-                code_dir,
-                artifact_subdir,
-                scratch_dir,
-                manifest_path,
-                build_runtime,
-                options,
-                container_env_vars,
-            )
             # Not including subfolder in return so that we copy subfolder, instead of copying artifacts inside it.
             return artifact_dir
 
@@ -520,21 +522,25 @@ def _build_function(  # pylint: disable=R1710
 
                 options = ApplicationBuilder._get_build_options(function_name, config.language, handler)
                 # By default prefer to build in-process for speed
-                build_method = self._build_function_in_process
                 if self._container_manager:
-                    build_method = self._build_function_on_container
-                    return build_method(
+                    # None represents the global build image for all functions/layers
+                    global_image = self._build_images.get(None)
+                    image = self._build_images.get(function_name, global_image)
+
+                    return self._build_function_on_container(
                         config,
                         code_dir,
                         artifact_dir,
-                        scratch_dir,
                         manifest_path,
                         runtime,
                         options,
                         container_env_vars,
+                        image,
                     )
 
-                return build_method(config, code_dir, artifact_dir, scratch_dir, manifest_path, runtime, options)
+                return self._build_function_in_process(
+                    config, code_dir, artifact_dir, scratch_dir, manifest_path, runtime, options
+                )
 
         # pylint: disable=fixme
         # FIXME: we need to throw an exception here, packagetype could be something else
@@ -571,8 +577,7 @@ def _build_function_in_process(
         scratch_dir: str,
         manifest_path: str,
         runtime: str,
-        options: Optional[dict],
-        container_env_vars: Optional[Dict] = None,
+        options: Optional[Dict],
     ) -> str:
 
         builder = LambdaBuilder(
@@ -604,11 +609,11 @@ def _build_function_on_container(
         config: CONFIG,
         source_dir: str,
         artifacts_dir: str,
-        scratch_dir: str,
         manifest_path: str,
         runtime: str,
         options: Optional[Dict],
         container_env_vars: Optional[Dict] = None,
+        build_image: Optional[str] = None,
     ) -> str:
         # _build_function_on_container() is only called when self._container_manager if not None
         if not self._container_manager:
@@ -642,6 +647,7 @@ def _build_function_on_container(
             executable_search_paths=config.executable_search_paths,
             mode=self._mode,
             env_vars=container_env_vars,
+            image=build_image,
         )
 
         try: