fix: managed stack (#1585)

* fix: managed stack

- always catch ClientError and BotoCoreError
- on windows, create temporary files with delete=False, otherwise it
  results in a PermissionDeniedError

* fix: dont mask inbuilt `file`

samcli/commands/bootstrap/exceptions.py

@@ -0,0 +1,11 @@
+"""
+Exceptions that are raised by sam bootstrap
+"""
+from samcli.commands.exceptions import UserException
+
+
+class ManagedStackError(UserException):
+    def __init__(self, ex):
+        self.ex = ex
+        message_fmt = f"\nFailed to create managed resources: {ex}"
+        super(ManagedStackError, self).__init__(message=message_fmt.format(ex=self.ex))

samcli/commands/deploy/command.py

@@ -2,12 +2,12 @@
 CLI command for "deploy" command
 """
 import json
-import tempfile
 import logging
 
 import click
 from click.types import FuncParamType
 
+from samcli.lib.utils import temp_file_utils
 from samcli.cli.cli_config_file import configuration_option, TomlProvider
 from samcli.cli.context import get_cmd_names
 from samcli.cli.main import pass_context, common_options, aws_creds_options
@@ -256,7 +256,7 @@ def do_cli(
         confirm_changeset=changeset_decision if guided else confirm_changeset,
     )
 
-    with tempfile.NamedTemporaryFile() as output_template_file:
+    with temp_file_utils.tempfile_platform_independent() as output_template_file:
 
         with PackageContext(
             template_file=template_file,

samcli/lib/bootstrap/bootstrap.py

@@ -3,19 +3,23 @@
 """
 
 import json
+import logging
+
 import boto3
 
 import click
 
 from botocore.config import Config
-from botocore.exceptions import ClientError, NoRegionError, NoCredentialsError
+from botocore.exceptions import ClientError, BotoCoreError, NoRegionError, NoCredentialsError
 
+from samcli.commands.bootstrap.exceptions import ManagedStackError
 from samcli import __version__
 from samcli.cli.global_config import GlobalConfig
 from samcli.commands.exceptions import UserException, CredentialsError, RegionError
 
 
 SAM_CLI_STACK_NAME = "aws-sam-cli-managed-default"
+LOG = logging.getLogger(__name__)
 
 
 def manage_stack(profile, region):
@@ -34,46 +38,50 @@ def manage_stack(profile, region):
 
 
 def _create_or_get_stack(cloudformation_client):
-    stack = None
-    try:
-        ds_resp = cloudformation_client.describe_stacks(StackName=SAM_CLI_STACK_NAME)
-        stacks = ds_resp["Stacks"]
-        stack = stacks[0]
-        click.echo("\n\tLooking for resources needed for deployment: Found!")
-    except ClientError:
-        click.echo("\n\tLooking for resources needed for deployment: Not found.")
-        stack = _create_stack(cloudformation_client)  # exceptions are not captured from subcommands
-    # Sanity check for non-none stack? Sanity check for tag?
-    tags = stack["Tags"]
     try:
-        sam_cli_tag = next(t for t in tags if t["Key"] == "ManagedStackSource")
-        if not sam_cli_tag["Value"] == "AwsSamCli":
+        stack = None
+        try:
+            ds_resp = cloudformation_client.describe_stacks(StackName=SAM_CLI_STACK_NAME)
+            stacks = ds_resp["Stacks"]
+            stack = stacks[0]
+            click.echo("\n\tLooking for resources needed for deployment: Found!")
+        except ClientError:
+            click.echo("\n\tLooking for resources needed for deployment: Not found.")
+            stack = _create_stack(cloudformation_client)  # exceptions are not captured from subcommands
+        # Sanity check for non-none stack? Sanity check for tag?
+        tags = stack["Tags"]
+        try:
+            sam_cli_tag = next(t for t in tags if t["Key"] == "ManagedStackSource")
+            if not sam_cli_tag["Value"] == "AwsSamCli":
+                msg = (
+                    "Stack "
+                    + SAM_CLI_STACK_NAME
+                    + " ManagedStackSource tag shows "
+                    + sam_cli_tag["Value"]
+                    + " which does not match the AWS SAM CLI generated tag value of AwsSamCli. "
+                    "Failing as the stack was likely not created by the AWS SAM CLI."
+                )
+                raise UserException(msg)
+        except StopIteration:
             msg = (
-                "Stack "
-                + SAM_CLI_STACK_NAME
-                + " ManagedStackSource tag shows "
-                + sam_cli_tag["Value"]
-                + " which does not match the AWS SAM CLI generated tag value of AwsSamCli. "
+                "Stack  " + SAM_CLI_STACK_NAME + " exists, but the ManagedStackSource tag is missing. "
                 "Failing as the stack was likely not created by the AWS SAM CLI."
             )
             raise UserException(msg)
-    except StopIteration:
-        msg = (
-            "Stack  " + SAM_CLI_STACK_NAME + " exists, but the ManagedStackSource tag is missing. "
-            "Failing as the stack was likely not created by the AWS SAM CLI."
-        )
-        raise UserException(msg)
-    outputs = stack["Outputs"]
-    try:
-        bucket_name = next(o for o in outputs if o["OutputKey"] == "SourceBucket")["OutputValue"]
-    except StopIteration:
-        msg = (
-            "Stack " + SAM_CLI_STACK_NAME + " exists, but is missing the managed source bucket key. "
-            "Failing as this stack was likely not created by the AWS SAM CLI."
-        )
-        raise UserException(msg)
-    # This bucket name is what we would write to a config file
-    return bucket_name
+        outputs = stack["Outputs"]
+        try:
+            bucket_name = next(o for o in outputs if o["OutputKey"] == "SourceBucket")["OutputValue"]
+        except StopIteration:
+            msg = (
+                "Stack " + SAM_CLI_STACK_NAME + " exists, but is missing the managed source bucket key. "
+                "Failing as this stack was likely not created by the AWS SAM CLI."
+            )
+            raise UserException(msg)
+        # This bucket name is what we would write to a config file
+        return bucket_name
+    except (ClientError, BotoCoreError) as ex:
+        LOG.debug("Failed to create managed resources", exc_info=ex)
+        raise ManagedStackError(str(ex))
 
 
 def _create_stack(cloudformation_client):

samcli/lib/utils/temp_file_utils.py

@@ -0,0 +1,26 @@
+"""
+Helper functions for temporary files
+"""
+import os
+import contextlib
+import tempfile
+
+
+def remove(path):
+    if path:
+        try:
+            os.remove(path)
+        except OSError:
+            pass
+
+
+@contextlib.contextmanager
+def tempfile_platform_independent():
+    # NOTE(TheSriram): Setting delete=False is specific to windows.
+    # https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile
+    _tempfile = tempfile.NamedTemporaryFile(delete=False)
+    try:
+        yield _tempfile
+    finally:
+        _tempfile.close()
+        remove(_tempfile.name)

tests/unit/lib/bootstrap/test_bootstrap.py

@@ -6,6 +6,7 @@
 from botocore.exceptions import ClientError, NoCredentialsError, NoRegionError
 from botocore.stub import Stubber
 
+from samcli.commands.bootstrap.exceptions import ManagedStackError
 from samcli.commands.exceptions import UserException, CredentialsError, RegionError
 from samcli.lib.bootstrap.bootstrap import manage_stack, _create_or_get_stack, _get_stack_template, SAM_CLI_STACK_NAME
 
@@ -171,7 +172,7 @@ def test_change_set_creation_fails(self):
         }
         stubber.add_client_error("create_change_set", service_error_code="ClientError", expected_params=ccs_params)
         stubber.activate()
-        with self.assertRaises(ClientError):
+        with self.assertRaises(ManagedStackError):
             _create_or_get_stack(stub_cf)
         stubber.assert_no_pending_responses()
         stubber.deactivate()
@@ -201,7 +202,7 @@ def test_change_set_execution_fails(self):
             "execute_change_set", service_error_code="InsufficientCapabilities", expected_params=ecs_params
         )
         stubber.activate()
-        with self.assertRaises(ClientError):
+        with self.assertRaises(ManagedStackError):
             _create_or_get_stack(stub_cf)
         stubber.assert_no_pending_responses()
         stubber.deactivate()

tests/unit/lib/utils/test_file_utils.py

@@ -0,0 +1,20 @@
+import os
+import tempfile
+from unittest import TestCase
+
+from samcli.lib.utils.temp_file_utils import remove, tempfile_platform_independent
+
+
+class TestFile(TestCase):
+    def test_file_remove(self):
+        _file = tempfile.NamedTemporaryFile(delete=False)
+        remove(_file.name)
+        self.assertFalse(os.path.exists(_file.name))
+        # No Exception thrown
+        remove(os.path.join(os.getcwd(), "random"))
+
+    def test_temp_file(self):
+        _path = None
+        with tempfile_platform_independent() as _tempf:
+            _path = _tempf.name
+        self.assertFalse(os.path.exists(_path))