fix: handle edge cases with function sync flow in sam sync command (a…

…ws#5222)

* fix: handle special cases for function sync flow

* update with unit tests

* add integration tests

* set ADL to false

* fix update file methods

* address comments

* address comments to instantiate FunctionBuildInfo in the beginning

samcli/commands/build/build_context.py

@@ -5,44 +5,46 @@
 import os
 import pathlib
 import shutil
-from typing import Dict, Optional, List, Tuple, cast
+from typing import Dict, Optional, List, Tuple
 
 import click
 
-from samcli.commands.build.utils import prompt_user_to_enable_mount_with_write_if_needed, MountMode
-from samcli.lib.build.bundler import EsbuildBundlerManager
-from samcli.lib.providers.sam_api_provider import SamApiProvider
-from samcli.lib.telemetry.event import EventTracker
-from samcli.lib.utils.packagetype import IMAGE
-
-from samcli.commands._utils.template import get_template_data
+from samcli.commands._utils.constants import DEFAULT_BUILD_DIR
 from samcli.commands._utils.experimental import ExperimentalFlag, prompt_experimental
+from samcli.commands._utils.template import (
+    get_template_data,
+    move_template,
+)
 from samcli.commands.build.exceptions import InvalidBuildDirException, MissingBuildMethodException
+from samcli.commands.build.utils import prompt_user_to_enable_mount_with_write_if_needed, MountMode
+from samcli.commands.exceptions import UserException
 from samcli.lib.bootstrap.nested_stack.nested_stack_manager import NestedStackManager
+from samcli.lib.build.app_builder import (
+    ApplicationBuilder,
+    BuildError,
+    UnsupportedBuilderLibraryVersionError,
+    ApplicationBuildResult,
+)
 from samcli.lib.build.build_graph import DEFAULT_DEPENDENCIES_DIR
+from samcli.lib.build.bundler import EsbuildBundlerManager
+from samcli.lib.build.exceptions import (
+    BuildInsideContainerError,
+    InvalidBuildGraphException,
+)
+from samcli.lib.build.workflow_config import UnsupportedRuntimeException
 from samcli.lib.intrinsic_resolver.intrinsics_symbol_table import IntrinsicsSymbolTable
 from samcli.lib.providers.provider import ResourcesToBuildCollector, Stack, Function, LayerVersion
+from samcli.lib.providers.sam_api_provider import SamApiProvider
 from samcli.lib.providers.sam_function_provider import SamFunctionProvider
 from samcli.lib.providers.sam_layer_provider import SamLayerProvider
 from samcli.lib.providers.sam_stack_provider import SamLocalStackProvider
+from samcli.lib.telemetry.event import EventTracker
 from samcli.lib.utils.osutils import BUILD_DIR_PERMISSIONS
 from samcli.local.docker.manager import ContainerManager
-from samcli.local.lambdafn.exceptions import ResourceNotFound
-from samcli.lib.build.exceptions import BuildInsideContainerError
-
-from samcli.commands.exceptions import UserException
-
-from samcli.lib.build.app_builder import (
-    ApplicationBuilder,
-    BuildError,
-    UnsupportedBuilderLibraryVersionError,
-    ApplicationBuildResult,
+from samcli.local.lambdafn.exceptions import (
+    FunctionNotFound,
+    ResourceNotFound,
 )
-from samcli.commands._utils.constants import DEFAULT_BUILD_DIR
-from samcli.lib.build.workflow_config import UnsupportedRuntimeException
-from samcli.local.lambdafn.exceptions import FunctionNotFound
-from samcli.commands._utils.template import move_template
-from samcli.lib.build.exceptions import InvalidBuildGraphException
 
 LOG = logging.getLogger(__name__)
 
@@ -586,7 +588,7 @@ def collect_all_build_resources(self) -> ResourcesToBuildCollector:
             [
                 f
                 for f in self.function_provider.get_all()
-                if (f.name not in excludes) and BuildContext._is_function_buildable(f)
+                if (f.name not in excludes) and f.function_build_info.is_buildable()
             ]
         )
         result.add_layers(
@@ -650,34 +652,6 @@ def _collect_single_buildable_layer(
 
         resource_collector.add_layer(layer)
 
-    @staticmethod
-    def _is_function_buildable(function: Function):
-        # no need to build inline functions
-        if function.inlinecode:
-            LOG.debug("Skip building inline function: %s", function.full_path)
-            return False
-        # no need to build functions that are already packaged as a zip file
-        if isinstance(function.codeuri, str) and function.codeuri.endswith(".zip"):
-            LOG.debug("Skip building zip function: %s", function.full_path)
-            return False
-        # skip build the functions that marked as skip-build
-        if function.skip_build:
-            LOG.debug("Skip building pre-built function: %s", function.full_path)
-            return False
-        # skip build the functions with Image Package Type with no docker context or docker file metadata
-        if function.packagetype == IMAGE:
-            metadata = function.metadata if function.metadata else {}
-            dockerfile = cast(str, metadata.get("Dockerfile", ""))
-            docker_context = cast(str, metadata.get("DockerContext", ""))
-            if not dockerfile or not docker_context:
-                LOG.debug(
-                    "Skip Building %s function, as it is missing either Dockerfile or DockerContext "
-                    "metadata properties.",
-                    function.full_path,
-                )
-                return False
-        return True
-
     @staticmethod
     def is_layer_buildable(layer: LayerVersion):
         # if build method is not specified, it is not buildable

samcli/lib/providers/provider.py

@@ -7,6 +7,7 @@
 import os
 import posixpath
 from collections import namedtuple
+from enum import Enum, auto
 from typing import TYPE_CHECKING, Any, Dict, Iterator, List, NamedTuple, Optional, Set, Union, cast
 
 from samcli.commands.local.cli_common.user_exceptions import (
@@ -21,6 +22,7 @@
     ResourceMetadataNormalizer,
 )
 from samcli.lib.utils.architecture import X86_64
+from samcli.lib.utils.packagetype import IMAGE
 
 if TYPE_CHECKING:  # pragma: no cover
     # avoid circular import, https://docs.python.org/3/library/typing.html#typing.TYPE_CHECKING
@@ -35,6 +37,27 @@
 CORS_MAX_AGE_HEADER = "Access-Control-Max-Age"
 
 
+class FunctionBuildInfo(Enum):
+    """
+    Represents information about function's build, see values for details
+    """
+
+    # buildable
+    BuildableZip = auto(), "Regular ZIP function which can be build with SAM CLI"
+    BuildableImage = auto(), "Regular IMAGE function which can be build with SAM CLI"
+    # non-buildable
+    InlineCode = auto(), "A ZIP function which has inline code, non buildable"
+    PreZipped = auto(), "A ZIP function which points to a .zip file, non buildable"
+    SkipBuild = auto(), "A Function which is denoted with SkipBuild in metadata, non buildable"
+    NonBuildableImage = auto(), "An IMAGE function which is missing some information to build, non buildable"
+
+    def is_buildable(self) -> bool:
+        """
+        Returns whether this build info can be buildable nor not
+        """
+        return self in {FunctionBuildInfo.BuildableZip, FunctionBuildInfo.BuildableImage}
+
+
 class Function(NamedTuple):
     """
     Named Tuple to representing the properties of a Lambda Function
@@ -82,6 +105,8 @@ class Function(NamedTuple):
     architectures: Optional[List[str]]
     # The function url configuration
     function_url_config: Optional[Dict]
+    # FunctionBuildInfo see implementation doc for its details
+    function_build_info: FunctionBuildInfo
     # The path of the stack relative to the root stack, it is empty for functions in root stack
     stack_path: str = ""
     # Configuration for runtime management. Includes the fields `UpdateRuntimeOn` and `RuntimeVersionArn` (optional).
@@ -105,7 +130,7 @@ def skip_build(self) -> bool:
         resource. It means that the customer is building the Lambda function code outside SAM, and the provided code
         path is already built.
         """
-        return self.metadata.get(SAM_METADATA_SKIP_BUILD_KEY, False) if self.metadata else False
+        return get_skip_build(self.metadata)
 
     def get_build_dir(self, build_root_dir: str) -> str:
         """
@@ -872,6 +897,52 @@ def get_unique_resource_ids(
     return output_resource_ids
 
 
+def get_skip_build(metadata: Optional[Dict]) -> bool:
+    """
+    Returns the value of SkipBuild property from Metadata, False if it is not defined
+    """
+    return metadata.get(SAM_METADATA_SKIP_BUILD_KEY, False) if metadata else False
+
+
+def get_function_build_info(
+    full_path: str,
+    packagetype: str,
+    inlinecode: Optional[str],
+    codeuri: Optional[str],
+    metadata: Optional[Dict],
+) -> FunctionBuildInfo:
+    """
+    Populates FunctionBuildInfo from the given information.
+    """
+    if inlinecode:
+        LOG.debug("Skip building inline function: %s", full_path)
+        return FunctionBuildInfo.InlineCode
+
+    if isinstance(codeuri, str) and codeuri.endswith(".zip"):
+        LOG.debug("Skip building zip function: %s", full_path)
+        return FunctionBuildInfo.PreZipped
+
+    if get_skip_build(metadata):
+        LOG.debug("Skip building pre-built function: %s", full_path)
+        return FunctionBuildInfo.SkipBuild
+
+    if packagetype == IMAGE:
+        metadata = metadata or {}
+        dockerfile = cast(str, metadata.get("Dockerfile", ""))
+        docker_context = cast(str, metadata.get("DockerContext", ""))
+
+        if not dockerfile or not docker_context:
+            LOG.debug(
+                "Skip Building %s function, as it is missing either Dockerfile or DockerContext "
+                "metadata properties.",
+                full_path,
+            )
+            return FunctionBuildInfo.NonBuildableImage
+        return FunctionBuildInfo.BuildableImage
+
+    return FunctionBuildInfo.BuildableZip
+
+
 def _get_build_dir(resource: Union[Function, LayerVersion], build_root: str) -> str:
     """
     Return the build directory to place build artifact

samcli/lib/providers/sam_function_provider.py

@@ -20,7 +20,7 @@
 )
 
 from ..build.constants import DEPRECATED_RUNTIMES
-from .provider import Function, LayerVersion, Stack
+from .provider import Function, LayerVersion, Stack, get_full_path, get_function_build_info
 from .sam_base_provider import SamBaseProvider
 from .sam_stack_provider import SamLocalStackProvider
 
@@ -444,12 +444,17 @@ def _build_function_configuration(
             LOG.debug("--base-dir is not presented, adjusting uri %s relative to %s", codeuri, stack.location)
             codeuri = SamLocalStackProvider.normalize_resource_path(stack.location, codeuri)
 
+        package_type = resource_properties.get("PackageType", ZIP)
+        function_build_info = get_function_build_info(
+            get_full_path(stack.stack_path, function_id), package_type, inlinecode, codeuri, metadata
+        )
+
         return Function(
             stack_path=stack.stack_path,
             function_id=function_id,
             name=name,
             functionname=resource_properties.get("FunctionName", name),
-            packagetype=resource_properties.get("PackageType", ZIP),
+            packagetype=package_type,
             runtime=resource_properties.get("Runtime"),
             memory=resource_properties.get("MemorySize"),
             timeout=resource_properties.get("Timeout"),
@@ -467,6 +472,7 @@ def _build_function_configuration(
             architectures=resource_properties.get("Architectures", None),
             function_url_config=resource_properties.get("FunctionUrlConfig"),
             runtime_management_config=resource_properties.get("RuntimeManagementConfig"),
+            function_build_info=function_build_info,
         )
 
     @staticmethod

samcli/lib/sync/flows/zip_function_sync_flow.py

@@ -3,6 +3,7 @@
 import hashlib
 import logging
 import os
+import shutil
 import tempfile
 import uuid
 from contextlib import ExitStack
@@ -226,3 +227,28 @@ def _get_function_api_calls(self) -> List[ResourceAPICall]:
     @staticmethod
     def _combine_dependencies() -> bool:
         return True
+
+
+class ZipFunctionSyncFlowSkipBuildZipFile(ZipFunctionSyncFlow):
+    """
+    Alternative implementation for ZipFunctionSyncFlow, which uses pre-built zip file for running sync flow
+    """
+
+    def gather_resources(self) -> None:
+        self._zip_file = os.path.join(tempfile.gettempdir(), f"data-{uuid.uuid4().hex}")
+        shutil.copy2(cast(str, self._function.codeuri), self._zip_file)
+        LOG.debug("%sCreated artifact ZIP file: %s", self.log_prefix, self._zip_file)
+        self._local_sha = file_checksum(self._zip_file, hashlib.sha256())
+
+
+class ZipFunctionSyncFlowSkipBuildDirectory(ZipFunctionSyncFlow):
+    """
+    Alternative implementation for ZipFunctionSyncFlow, which doesn't build function but zips folder directly
+    since function is annotated with SkipBuild inside its Metadata
+    """
+
+    def gather_resources(self) -> None:
+        zip_file_path = os.path.join(tempfile.gettempdir(), f"data-{uuid.uuid4().hex}")
+        self._zip_file = make_zip_with_lambda_permissions(zip_file_path, self._function.codeuri)
+        LOG.debug("%sCreated artifact ZIP file: %s", self.log_prefix, self._zip_file)
+        self._local_sha = file_checksum(cast(str, self._zip_file), hashlib.sha256())