Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 12 commits into from
Jan 8, 2024
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
14 changes: 14 additions & 0 deletions samcli/lib/build/build_strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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
Expand Down
13 changes: 0 additions & 13 deletions samcli/lib/providers/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
68 changes: 62 additions & 6 deletions tests/integration/buildcmd/test_build_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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"

Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
3 changes: 3 additions & 0 deletions tests/unit/lib/build_module/test_build_strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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]

Expand Down
Loading