Merge from aws/aws-sam-cli/develop

Author: aws-sam-cli-bot

samcli/__init__.py

@@ -2,4 +2,4 @@
 SAM CLI version
 """
 
-__version__ = "1.145.0"
+__version__ = "1.145.1"

samcli/commands/package/package_context.py

@@ -34,7 +34,6 @@
 from samcli.lib.utils.boto_utils import get_boto_config_with_user_agent
 from samcli.lib.utils.preview_runtimes import PREVIEW_RUNTIMES
 from samcli.lib.utils.resources import AWS_LAMBDA_FUNCTION, AWS_SERVERLESS_FUNCTION
-from samcli.local.docker.utils import get_validated_container_client
 from samcli.yamlhelper import yaml_dump
 
 LOG = logging.getLogger(__name__)
@@ -123,7 +122,8 @@ def run(self):
         )
         ecr_client = boto3.client("ecr", config=get_boto_config_with_user_agent(region_name=region_name))
 
-        docker_client = get_validated_container_client()
+        # Pass None instead of validating Docker client upfront - ECRUploader will validate only when needed
+        docker_client = None
 
         s3_uploader = S3Uploader(
             s3_client, self.s3_bucket, self.s3_prefix, self.kms_key_id, self.force_upload, self.no_progressbar

samcli/lib/package/ecr_uploader.py

@@ -38,7 +38,9 @@ class ECRUploader:
     def __init__(
         self, docker_client, ecr_client, ecr_repo, ecr_repo_multi, no_progressbar=False, tag="latest", stream=stderr()
     ):
-        self.docker_client = docker_client if docker_client else get_validated_container_client()
+        # Allow None docker_client, validate only when needed
+        self._docker_client_param = docker_client
+        self._validated_docker_client = None
         self.ecr_client = ecr_client
         self.ecr_repo = ecr_repo
         self.ecr_repo_multi = ecr_repo_multi
@@ -49,6 +51,17 @@ def __init__(
         self.log_streamer = LogStreamer(stream=self.stream)
         self.login_session_active = False
 
+    @property
+    def docker_client(self):
+        """Lazy initialization of Docker client - only validates when ECR operations are needed."""
+        if self._validated_docker_client is None:
+            if self._docker_client_param is None:
+                # Only validate Docker client when ECR operations are actually needed
+                self._validated_docker_client = get_validated_container_client()
+            else:
+                self._validated_docker_client = self._docker_client_param
+        return self._validated_docker_client
+
     def login(self):
         """
         Logs into the supplied ECR with credentials.

tests/unit/commands/package/test_package_context.py

@@ -32,7 +32,7 @@ def setUp(self):
             profile=None,
         )
 
-    @patch("samcli.commands.package.package_context.get_validated_container_client")
+    @patch("samcli.lib.package.ecr_uploader.get_validated_container_client")
     @patch.object(SamLocalStackProvider, "get_stacks")
     @patch.object(Template, "export", MagicMock(sideeffect=OSError))
     @patch("boto3.client")
@@ -46,7 +46,7 @@ def test_template_permissions_error(self, patched_boto, patched_get_stacks, mock
             with patch.object(self.package_command_context, "_warn_preview_runtime") as patched_warn_preview_runtime:
                 self.package_command_context.run()
 
-    @patch("samcli.commands.package.package_context.get_validated_container_client")
+    @patch("samcli.lib.package.ecr_uploader.get_validated_container_client")
     @patch.object(ResourceMetadataNormalizer, "normalize", MagicMock())
     @patch.object(Template, "export", MagicMock(return_value={}))
     @patch("boto3.client")
@@ -74,7 +74,7 @@ def test_template_path_valid_with_output_template(self, patched_boto, mock_get_v
                 )
                 package_command_context.run()
 
-    @patch("samcli.commands.package.package_context.get_validated_container_client")
+    @patch("samcli.lib.package.ecr_uploader.get_validated_container_client")
     @patch.object(ResourceMetadataNormalizer, "normalize", MagicMock())
     @patch.object(Template, "export", MagicMock(return_value={}))
     @patch("boto3.client")
@@ -101,7 +101,7 @@ def test_template_path_valid(self, patched_boto, mock_get_validated_client):
             )
             package_command_context.run()
 
-    @patch("samcli.commands.package.package_context.get_validated_container_client")
+    @patch("samcli.lib.package.ecr_uploader.get_validated_container_client")
     @patch.object(ResourceMetadataNormalizer, "normalize", MagicMock())
     @patch.object(Template, "export", MagicMock(return_value={}))
     @patch("boto3.client")
@@ -128,7 +128,7 @@ def test_template_path_valid_no_json(self, patched_boto, mock_get_validated_clie
             )
             package_command_context.run()
 
-    @patch("samcli.commands.package.package_context.get_validated_container_client")
+    @patch("samcli.lib.package.ecr_uploader.get_validated_container_client")
     @patch("samcli.commands.package.package_context.PackageContext._warn_preview_runtime")
     @patch("samcli.commands.package.package_context.get_resource_full_path_by_id")
     @patch.object(SamLocalStackProvider, "get_stacks")
@@ -214,3 +214,176 @@ def test_warn_preview_runtime(self, runtime, should_warn, function_type, patched
             patched_click.secho.assert_called_once()
         else:
             patched_click.secho.assert_not_called()
+
+
+class TestPackageContextDockerLazyInitialization(TestCase):
+    """Test cases for lazy Docker client initialization in PackageContext"""
+
+    def setUp(self):
+        self.package_command_context = PackageContext(
+            template_file="template-file",
+            s3_bucket="s3-bucket",
+            s3_prefix="s3-prefix",
+            image_repository="image-repo",
+            image_repositories=None,
+            kms_key_id="kms-key-id",
+            output_template_file=None,
+            use_json=True,
+            force_upload=True,
+            no_progressbar=False,
+            metadata={},
+            region=None,
+            profile=None,
+        )
+
+    @patch("samcli.lib.package.ecr_uploader.get_validated_container_client")
+    @patch.object(ResourceMetadataNormalizer, "normalize", MagicMock())
+    @patch.object(Template, "export", MagicMock(return_value={}))
+    @patch("boto3.client")
+    def test_docker_client_not_validated_during_package_context_run(self, patched_boto, mock_get_validated_client):
+        """Test that Docker client is not validated during PackageContext.run()"""
+        mock_docker_client = Mock()
+        mock_get_validated_client.return_value = mock_docker_client
+
+        with tempfile.NamedTemporaryFile(mode="w", delete=False) as temp_template_file:
+            package_command_context = PackageContext(
+                template_file=temp_template_file.name,
+                s3_bucket="s3-bucket",
+                s3_prefix="s3-prefix",
+                image_repository="image-repo",
+                image_repositories=None,
+                kms_key_id="kms-key-id",
+                output_template_file=None,
+                use_json=True,
+                force_upload=True,
+                no_progressbar=False,
+                metadata={},
+                region="us-east-2",
+                profile=None,
+            )
+
+            # Run the package context
+            package_command_context.run()
+
+            # Docker client validation should not have been called during run()
+            mock_get_validated_client.assert_not_called()
+
+            # ECR uploader should be created but Docker client not validated yet
+            ecr_uploader = package_command_context.uploaders.ecr
+            self.assertIsNone(ecr_uploader._validated_docker_client)
+
+    @patch("samcli.lib.package.ecr_uploader.get_validated_container_client")
+    @patch.object(ResourceMetadataNormalizer, "normalize", MagicMock())
+    @patch.object(Template, "export", MagicMock(return_value={}))
+    @patch("boto3.client")
+    def test_docker_client_validated_only_when_ecr_operations_performed(self, patched_boto, mock_get_validated_client):
+        """Test that Docker client is validated only when ECR operations are performed"""
+        mock_docker_client = Mock()
+        mock_get_validated_client.return_value = mock_docker_client
+
+        with tempfile.NamedTemporaryFile(mode="w", delete=False) as temp_template_file:
+            package_command_context = PackageContext(
+                template_file=temp_template_file.name,
+                s3_bucket="s3-bucket",
+                s3_prefix="s3-prefix",
+                image_repository="image-repo",
+                image_repositories=None,
+                kms_key_id="kms-key-id",
+                output_template_file=None,
+                use_json=True,
+                force_upload=True,
+                no_progressbar=False,
+                metadata={},
+                region="us-east-2",
+                profile=None,
+            )
+
+            # Run the package context
+            package_command_context.run()
+
+            # Docker client validation should not have been called yet
+            mock_get_validated_client.assert_not_called()
+
+            # Access the docker_client property to trigger validation
+            ecr_uploader = package_command_context.uploaders.ecr
+            docker_client = ecr_uploader.docker_client
+
+            # Now Docker client validation should have been called
+            mock_get_validated_client.assert_called_once()
+            self.assertEqual(docker_client, mock_docker_client)
+
+    @patch("samcli.lib.package.ecr_uploader.get_validated_container_client")
+    @patch.object(ResourceMetadataNormalizer, "normalize", MagicMock())
+    @patch.object(Template, "export", MagicMock(return_value={}))
+    @patch("boto3.client")
+    def test_ecr_uploader_created_with_none_docker_client(self, patched_boto, mock_get_validated_client):
+        """Test that ECRUploader is created with None docker_client parameter"""
+        mock_docker_client = Mock()
+        mock_get_validated_client.return_value = mock_docker_client
+
+        with tempfile.NamedTemporaryFile(mode="w", delete=False) as temp_template_file:
+            package_command_context = PackageContext(
+                template_file=temp_template_file.name,
+                s3_bucket="s3-bucket",
+                s3_prefix="s3-prefix",
+                image_repository="image-repo",
+                image_repositories=None,
+                kms_key_id="kms-key-id",
+                output_template_file=None,
+                use_json=True,
+                force_upload=True,
+                no_progressbar=False,
+                metadata={},
+                region="us-east-2",
+                profile=None,
+            )
+
+            # Run the package context
+            package_command_context.run()
+
+            # Verify ECRUploader was created with None docker_client parameter
+            ecr_uploader = package_command_context.uploaders.ecr
+            self.assertIsNone(ecr_uploader._docker_client_param)
+            self.assertIsNone(ecr_uploader._validated_docker_client)
+
+    @patch("samcli.lib.package.ecr_uploader.get_validated_container_client")
+    @patch.object(ResourceMetadataNormalizer, "normalize", MagicMock())
+    @patch.object(Template, "export", MagicMock(return_value={}))
+    @patch("boto3.client")
+    def test_multiple_docker_client_accesses_only_validate_once(self, patched_boto, mock_get_validated_client):
+        """Test that multiple accesses to docker_client only validate once"""
+        mock_docker_client = Mock()
+        mock_get_validated_client.return_value = mock_docker_client
+
+        with tempfile.NamedTemporaryFile(mode="w", delete=False) as temp_template_file:
+            package_command_context = PackageContext(
+                template_file=temp_template_file.name,
+                s3_bucket="s3-bucket",
+                s3_prefix="s3-prefix",
+                image_repository="image-repo",
+                image_repositories=None,
+                kms_key_id="kms-key-id",
+                output_template_file=None,
+                use_json=True,
+                force_upload=True,
+                no_progressbar=False,
+                metadata={},
+                region="us-east-2",
+                profile=None,
+            )
+
+            # Run the package context
+            package_command_context.run()
+
+            ecr_uploader = package_command_context.uploaders.ecr
+
+            # Access docker_client multiple times
+            docker_client1 = ecr_uploader.docker_client
+            docker_client2 = ecr_uploader.docker_client
+            docker_client3 = ecr_uploader.docker_client
+
+            # Docker client validation should only be called once
+            mock_get_validated_client.assert_called_once()
+            self.assertEqual(docker_client1, docker_client2)
+            self.assertEqual(docker_client2, docker_client3)
+            self.assertEqual(docker_client1, mock_docker_client)

tests/unit/lib/package/test_ecr_uploader.py

@@ -49,6 +49,84 @@ def test_ecr_uploader_init(self):
             tag=self.tag,
         )
 
+    def test_ecr_uploader_init_with_none_docker_client(self):
+        """Test ECRUploader initialization with None docker_client (lazy initialization)"""
+        ecr_uploader = ECRUploader(
+            docker_client=None,  # Pass None to trigger lazy initialization
+            ecr_client=self.ecr_client,
+            ecr_repo=self.ecr_repo,
+            ecr_repo_multi=self.ecr_repo_multi,
+            tag=self.tag,
+        )
+
+        # Docker client should not be validated yet
+        self.assertIsNone(ecr_uploader._docker_client_param)
+        self.assertIsNone(ecr_uploader._validated_docker_client)
+
+    @patch("samcli.lib.package.ecr_uploader.get_validated_container_client")
+    def test_lazy_docker_client_validation_on_first_access(self, mock_get_validated_client):
+        """Test that Docker client is only validated when first accessed"""
+        mock_docker_client = MagicMock()
+        mock_get_validated_client.return_value = mock_docker_client
+
+        ecr_uploader = ECRUploader(
+            docker_client=None,  # Trigger lazy initialization
+            ecr_client=self.ecr_client,
+            ecr_repo=self.ecr_repo,
+            ecr_repo_multi=self.ecr_repo_multi,
+            tag=self.tag,
+        )
+
+        # Docker client should not be validated yet
+        self.assertIsNone(ecr_uploader._validated_docker_client)
+        mock_get_validated_client.assert_not_called()
+
+        # First access should trigger validation
+        docker_client = ecr_uploader.docker_client
+
+        # Validation should have been called
+        mock_get_validated_client.assert_called_once()
+        self.assertEqual(docker_client, mock_docker_client)
+        self.assertEqual(ecr_uploader._validated_docker_client, mock_docker_client)
+
+    @patch("samcli.lib.package.ecr_uploader.get_validated_container_client")
+    def test_lazy_docker_client_validation_cached_after_first_access(self, mock_get_validated_client):
+        """Test that Docker client validation is cached after first access"""
+        mock_docker_client = MagicMock()
+        mock_get_validated_client.return_value = mock_docker_client
+
+        ecr_uploader = ECRUploader(
+            docker_client=None,
+            ecr_client=self.ecr_client,
+            ecr_repo=self.ecr_repo,
+            ecr_repo_multi=self.ecr_repo_multi,
+            tag=self.tag,
+        )
+
+        # First access
+        docker_client1 = ecr_uploader.docker_client
+        # Second access
+        docker_client2 = ecr_uploader.docker_client
+
+        # Validation should only be called once
+        mock_get_validated_client.assert_called_once()
+        self.assertEqual(docker_client1, docker_client2)
+        self.assertEqual(docker_client1, mock_docker_client)
+
+    def test_docker_client_property_with_provided_client(self):
+        """Test that provided Docker client is used without validation"""
+        ecr_uploader = ECRUploader(
+            docker_client=self.docker_client,  # Provide actual client
+            ecr_client=self.ecr_client,
+            ecr_repo=self.ecr_repo,
+            ecr_repo_multi=self.ecr_repo_multi,
+            tag=self.tag,
+        )
+
+        # Should return the provided client
+        docker_client = ecr_uploader.docker_client
+        self.assertEqual(docker_client, self.docker_client)
+
         self.assertEqual(ecr_uploader.docker_client, self.docker_client)
         self.assertEqual(ecr_uploader.ecr_repo, self.ecr_repo)
         self.assertEqual(ecr_uploader.tag, self.tag)