Fix Erroneous "No BuildArchitecture specified in Layer" Warning (#6508)

* Move Missing BuildArchitecture warning to build_layer * Formatting * Fix typo * Narrow warnings to mismatches between buildArch and Compatible Architectures * Fix some tests * Fix integ test * More integ tests * Format fix * Fix integ test * Fix typo * Parameterize test with use_container --------- Co-authored-by: Jared Bentvelsen <[email protected]>
aws · Jan 8, 2024 · f51e020 · f51e020
1 parent c49c218
commit f51e020
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 20 deletions.
diff --git a/samcli/lib/build/build_strategy.py b/samcli/lib/build/build_strategy.py
@@ -26,6 +26,7 @@
 from samcli.lib.build.exceptions import MissingBuildMethodException
 from samcli.lib.build.utils import warn_on_invalid_architecture
 
+from samcli.lib.utils.architecture import X86_64, ARM64
 
 LOG = logging.getLogger(__name__)
 
@@ -218,6 +219,19 @@ def build_single_layer_definition(self, layer_definition: LayerBuildDefinition)
         if layer.build_method == "makefile":
             warn_on_invalid_architecture(layer_definition)
 
+        # There are two cases where we'd like to warn the customer
+        # 1. Compatible Architectures is only x86 (or not present) but Build Architecture is arm64
+        # 2. Build Architecture is x86 (or not present) but Compatible Architectures is only arm64
+
+        build_architecture = layer.build_architecture or X86_64
+        compatible_architectures = layer.compatible_architectures or [X86_64]
+
+        if build_architecture not in compatible_architectures:
+            LOG.warning(
+                "WARNING: Layer '%s' has BuildArchitecture %s, which is not listed in CompatibleArchitectures",
+                layer.layer_id,
+                build_architecture,
+            )
         single_build_dir = layer.get_build_dir(self._build_dir)
         # when a layer is passed here, it is ZIP function, codeuri and runtime are not None
         # codeuri and compatible_runtimes are not None

diff --git a/samcli/lib/providers/provider.py b/samcli/lib/providers/provider.py
@@ -244,22 +244,9 @@ def __init__(
         self._compatible_runtimes = compatible_runtimes
         self._custom_layer_id = metadata.get(SAM_RESOURCE_ID_KEY)
 
-        if "BuildArchitecture" not in metadata:
-            LOG.warning(
-                "WARNING: No BuildArchitecture specifed in Layer `%s`" + " Metadata. Defaulting to x86_64.",
-                self._custom_layer_id,
-            )
-
         self._build_architecture = cast(str, metadata.get("BuildArchitecture", X86_64))
         self._compatible_architectures = compatible_architectures
 
-        if self._compatible_architectures and self._build_architecture not in self._compatible_architectures:
-            LOG.warning(
-                "WARNING: Layer `%s` has BuildArchitecture `%s`," + " which is not listed in CompatibleArchitectures.",
-                self._custom_layer_id,
-                self._build_architecture,
-            )
-
         self._skip_build = bool(metadata.get(SAM_METADATA_SKIP_BUILD_KEY, False))
 
     @staticmethod

diff --git a/tests/integration/buildcmd/test_build_cmd.py b/tests/integration/buildcmd/test_build_cmd.py
@@ -46,6 +46,7 @@
     BuildIntegEsbuildBase,
 )
 
+
 LOG = logging.getLogger(__name__)
 
 # SAR tests require credentials. This is to skip running the test where credentials are not available.
@@ -1675,6 +1676,8 @@ def test_build_layer_with_makefile_no_compatible_runtimes(self):
     )
     def test_build_layer_with_architecture_not_compatible(self, build_method, use_container):
         # The BuildArchitecture is not one of the listed CompatibleArchitectures
+        if use_container and (SKIP_DOCKER_TESTS or SKIP_DOCKER_BUILD):
+            self.skipTest(SKIP_DOCKER_MESSAGE)
 
         layer_identifier = "LayerWithNoCompatibleArchitectures"
 
@@ -1691,7 +1694,59 @@ def test_build_layer_with_architecture_not_compatible(self, build_method, use_co
         command_result = run_command(cmdlist, cwd=self.working_dir)
         # Capture warning
         self.assertIn(
-            f"Layer `{layer_identifier}` has BuildArchitecture `x86_64`, which is not listed in CompatibleArchitectures.",
+            f"Layer '{layer_identifier}' has BuildArchitecture x86_64, which is not listed in CompatibleArchitectures",
+            str(command_result.stderr.decode("utf-8")),
+        )
+        # Build should still succeed
+        self.assertEqual(command_result.process.returncode, 0)
+
+    @parameterized.expand([("python3.8", False), ("python3.8", "use_container")])
+    def test_build_arch_no_compatible_arch(self, runtime, use_container):
+        # BuildArchitecture is present, but CompatibleArchitectures section is missing
+        if use_container and (SKIP_DOCKER_TESTS or SKIP_DOCKER_BUILD):
+            self.skipTest(SKIP_DOCKER_MESSAGE)
+
+        layer_identifier = "LayerWithBuildArchButNoCompatibleArchs"
+
+        overrides = {
+            "LayerBuildMethod": runtime,
+            "LayerMakeContentUri": "PyLayer",
+            "LayerBuildArchitecture": "arm64",
+        }
+        cmdlist = self.get_command_list(
+            use_container=use_container, parameter_overrides=overrides, function_identifier=layer_identifier
+        )
+
+        command_result = run_command(cmdlist, cwd=self.working_dir)
+        # Capture warning
+        self.assertIn(
+            f"Layer '{layer_identifier}' has BuildArchitecture arm64, which is not listed in CompatibleArchitectures",
+            str(command_result.stderr),
+        )
+        # Build should still succeed
+        self.assertEqual(command_result.process.returncode, 0)
+
+    @parameterized.expand([("python3.8", False), ("python3.8", "use_container")])
+    def test_compatible_arch_no_build_arch(self, runtime, use_container):
+        # CompatibleArchitectures is present, but BuildArchitecture section is missing
+        if use_container and (SKIP_DOCKER_TESTS or SKIP_DOCKER_BUILD):
+            self.skipTest(SKIP_DOCKER_MESSAGE)
+
+        layer_identifier = "LayerWithCompatibleArchsButNoBuildArch"
+
+        overrides = {
+            "LayerBuildMethod": runtime,
+            "LayerMakeContentUri": "PyLayer",
+            "LayerCompatibleArchitecture": "arm64",
+        }
+        cmdlist = self.get_command_list(
+            use_container=use_container, parameter_overrides=overrides, function_identifier=layer_identifier
+        )
+
+        command_result = run_command(cmdlist, cwd=self.working_dir)
+        # Capture warning
+        self.assertIn(
+            f"Layer '{layer_identifier}' has BuildArchitecture x86_64, which is not listed in CompatibleArchitectures",
             str(command_result.stderr),
         )
         # Build should still succeed
@@ -1736,18 +1791,19 @@ def test_build_fails_with_missing_metadata(self, runtime, use_container, layer_i
         self.assertEqual(command_result.process.returncode, 1)
         self.assertFalse(self.default_build_dir.joinpath(layer_identifier).exists())
 
-    @parameterized.expand([("python3.7", False, "LayerOne"), ("python3.7", "use_container", "LayerOne")])
-    def test_build_with_missing_buildarchitecture(self, runtime, use_container, layer_identifier):
+    @parameterized.expand([False, "use_container"])
+    def test_function_build_succeeds_with_referenced_layer(self, use_container):
         if use_container and (SKIP_DOCKER_TESTS or SKIP_DOCKER_BUILD):
             self.skipTest(SKIP_DOCKER_MESSAGE)
 
-        overrides = {"LayerBuildMethod": runtime, "LayerContentUri": "PyLayer"}
+        overrides = {"Runtime": "python3.8", "CodeUri": "Python"}
+
         cmdlist = self.get_command_list(
-            use_container=use_container, parameter_overrides=overrides, function_identifier=layer_identifier
+            use_container=use_container, parameter_overrides=overrides, function_identifier="FunctionTwo"
         )
+
         command_result = run_command(cmdlist, cwd=self.working_dir)
         self.assertEqual(command_result.process.returncode, 0)
-        self.assertIn("No BuildArchitecture specifed", str(command_result.stderr))
 
     @parameterized.expand([("python3.7", False), ("python3.7", "use_container")])
     def test_build_function_and_layer(self, runtime, use_container):

diff --git a/tests/integration/testdata/buildcmd/layers-functions-template.yaml b/tests/integration/testdata/buildcmd/layers-functions-template.yaml
@@ -32,6 +32,16 @@ Resources:
       Layers:
         - !Ref LayerOne
 
+  FunctionTwo:
+    Type: AWS::Serverless::Function
+    Properties:
+      Handler: !Ref Handler
+      Runtime: !Ref Runtime
+      CodeUri: !Ref CodeUri
+      Timeout: 600
+      Layers:
+        - !Sub arn:${AWS::Partition}:lambda:${AWS::Region}:012345678912:layer:layerName:1
+
   LayerOne:
     Type: AWS::Serverless::LayerVersion
     Properties:
@@ -87,4 +97,23 @@ Resources:
         - !Ref LayerCompatibleArchitecture
     Metadata:
       BuildMethod: !Ref LayerBuildMethod
-      BuildArchitecture: !Ref LayerBuildArchitecture
+      BuildArchitecture: !Ref LayerBuildArchitecture
+
+  LayerWithBuildArchButNoCompatibleArchs:
+    Type: AWS::Serverless::LayerVersion
+    Properties:
+      Description: Build arch present but CompatibleArchs not present
+      ContentUri: !Ref LayerMakeContentUri
+    Metadata:
+      BuildMethod: !Ref LayerBuildMethod
+      BuildArchitecture: !Ref LayerBuildArchitecture
+
+  LayerWithCompatibleArchsButNoBuildArch:
+    Type: AWS::Serverless::LayerVersion
+    Properties:
+      Description: Build arch present but CompatibleArchs not present
+      ContentUri: !Ref LayerMakeContentUri
+      CompatibleArchitectures:
+        - !Ref LayerCompatibleArchitecture
+    Metadata:
+      BuildMethod: !Ref LayerBuildMethod
diff --git a/tests/unit/lib/build_module/test_build_strategy.py b/tests/unit/lib/build_module/test_build_strategy.py
@@ -56,7 +56,9 @@ def setUp(self):
         self.build_graph.put_function_build_definition(self.function_build_definition2, self.function2)
 
         self.layer1 = Mock()
+        self.layer1.compatible_architectures = None
         self.layer2 = Mock()
+        self.layer2.compatible_architectures = None
 
         self.layer_build_definition1 = LayerBuildDefinition("layer1", "codeuri", "build_method", [], X86_64)
         self.layer_build_definition2 = LayerBuildDefinition("layer2", "codeuri", "build_method", [], X86_64)
@@ -699,6 +701,7 @@ def test_assert_incremental_build_layer(self, patched_manifest_hash, patched_os,
         given_layer_build_def = Mock(
             manifest_hash=build_toml_manifest_hash, functions=[Mock()], dependencies_dir=dependency_dir
         )
+        given_layer_build_def.layer.compatible_architectures = None
         self.build_graph.get_function_build_definitions.return_value = []
         self.build_graph.get_layer_build_definitions.return_value = [given_layer_build_def]