Skip to content

Commit

Permalink
Sam delete fix (#3180)
Browse files Browse the repository at this point in the history
* feat: Delete methods for CF stacks and S3 files (aws#2981)

* Added methods for cf and s3 files and init UI

* Added unit tests for utils methods and s3_uploader

* Removed s3_bucket and s3_prefix click options

* Fixed lint errors and added few unit tests

* Make black happy

* Added LOG statements

* Added and updated changes based on CR

* Fixed the unit tests in artifact_exporter.py

* Update HELP_TEXT in delete/command.py

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Updated code based on Chris' comments

* Small changes and fixes based on the comments

* Removed region prompt

* Update SAM context values for profile and region in delete_context.py

* Added typing for get_cf_template_name method

* Added stack_name prompt if the stack_name is not present in samconfig file

* Replace [] with get() for stack-name in delete_context.py

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Delete template artifacts (aws#3022)

* Added methods for cf and s3 files and init UI

* Added unit tests for utils methods and s3_uploader

* Removed s3_bucket and s3_prefix click options

* chore: Increase awareness of same file warning during package (aws#2946)

* chore: increase awareness of same file warning during package

* fix formatting & grammar

Co-authored-by: Mathieu Grandis <73313235+mgrandis@users.noreply.github.com>

* fix: Allow the base64Encoded field in REST Api, skip validation of unknown fields and validate missing statusCode for Http Api (aws#2941)

* fix API Gateway emulator:
 - skip validating the non allowed fields for Http Api Gateway, as it always skip the unknown fields
 - add base64Encoded as an allowed field for Rest Api gateway
 - base64 decoding will be always done for Http API gateway if the lambda response isBase64Encoded is true regardless the content-type
 - validate if statusCode is missing in case of Http API, and payload version 1.0

* - accept "true", "True", "false", "False" as valid isBase64Encoded values.
- Validate on other isBase64Encoded Values
- add more integration && unit test cases

* fix lint && black issues

* use smaller image to test Base64 response

* Fixed lint errors and added few unit tests

* Make black happy

* Added methods for deleting template artifacts

* Wait method added for delete cf api

* Added LOG statements

* Added and updated changes based on CR

* Fixed the unit tests in artifact_exporter.py

* Update HELP_TEXT in delete/command.py

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Updated code based on Chris' comments

* Added condition for resources that have deletionpolicy specified

* Small changes and fixes based on the comments

* Removed region prompt

* Added unit tests for ecr delete method and typing for methods

* Reformatted delete_context and added option to skip user prompts

* Removed return type from artifact_exporter for delete method

* Added unit tests for artifact_exporter and delete_context

* Added more unit tests for delete_context and artifact_exporter

* Added more unit tests for delete_context and artifact_exporter

* Added docs and comments for artifact_exporter and ecr_uploader

* Added log statements in delete_context and some updates in unit tests

* Changed force to no-prompts  and updated ecr delete method error handling

* Created a separate function for parsing ecr url in ecr_uploader

* Reformatted Template class init to pass template_str and init template_dict

* Changed how s3 url is obtained for resource_zip edge-case: aws:glue:job

* Fixed edge case where resource artifact points to a path style url

* run Make black

* Made the parse s3 url funcs protected and defined a parent method and modified delete method for ResourceImageDict

* Changed parse_ecr_url function name to parse_image_url

Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
Co-authored-by: Mathieu Grandis <73313235+mgrandis@users.noreply.github.com>
Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>
Co-authored-by: Chris Rehn <crehn@outlook.com>

* Get s3 info cf template (aws#3050)

* Added methods for cf and s3 files and init UI

* Added unit tests for utils methods and s3_uploader

* Removed s3_bucket and s3_prefix click options

* chore: Increase awareness of same file warning during package (aws#2946)

* chore: increase awareness of same file warning during package

* fix formatting & grammar

Co-authored-by: Mathieu Grandis <73313235+mgrandis@users.noreply.github.com>

* fix: Allow the base64Encoded field in REST Api, skip validation of unknown fields and validate missing statusCode for Http Api (aws#2941)

* fix API Gateway emulator:
 - skip validating the non allowed fields for Http Api Gateway, as it always skip the unknown fields
 - add base64Encoded as an allowed field for Rest Api gateway
 - base64 decoding will be always done for Http API gateway if the lambda response isBase64Encoded is true regardless the content-type
 - validate if statusCode is missing in case of Http API, and payload version 1.0

* - accept "true", "True", "false", "False" as valid isBase64Encoded values.
- Validate on other isBase64Encoded Values
- add more integration && unit test cases

* fix lint && black issues

* use smaller image to test Base64 response

* Fixed lint errors and added few unit tests

* Make black happy

* Added methods for deleting template artifacts

* Wait method added for delete cf api

* Added LOG statements

* Added and updated changes based on CR

* Fixed the unit tests in artifact_exporter.py

* Update HELP_TEXT in delete/command.py

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Updated code based on Chris' comments

* Added condition for resources that have deletionpolicy specified

* Small changes and fixes based on the comments

* Removed region prompt

* Added unit tests for ecr delete method and typing for methods

* Reformatted delete_context and added option to skip user prompts

* Removed return type from artifact_exporter for delete method

* Added unit tests for artifact_exporter and delete_context

* Added more unit tests for delete_context and artifact_exporter

* Added more unit tests for delete_context and artifact_exporter

* Added docs and comments for artifact_exporter and ecr_uploader

* Added log statements in delete_context and some updates in unit tests

* Changed force to no-prompts  and updated ecr delete method error handling

* Created a separate function for parsing ecr url in ecr_uploader

* Reformatted Template class init to pass template_str and init template_dict

* Changed how s3 url is obtained for resource_zip edge-case: aws:glue:job

* Fixed edge case where resource artifact points to a path style url

* run Make black

* Made the parse s3 url funcs protected and defined a parent method and modified delete method for ResourceImageDict

* Added methods to extract s3 info from cf template

* Added testing for get_s3_info method for artifact_exporter and s3_uploader methods

* Removed commented code and updated method docstring

* Better error handling for s3 delete artifacts and fixed bug for getting s3 resources information

* Changed get_s3_info to get_property_value and changed output text for s3 delete method

Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
Co-authored-by: Mathieu Grandis <73313235+mgrandis@users.noreply.github.com>
Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>
Co-authored-by: Chris Rehn <crehn@outlook.com>

* Sam delete integration testing (aws#3076)

* Added methods for cf and s3 files and init UI

* Added unit tests for utils methods and s3_uploader

* Removed s3_bucket and s3_prefix click options

* chore: Increase awareness of same file warning during package (aws#2946)

* chore: increase awareness of same file warning during package

* fix formatting & grammar

Co-authored-by: Mathieu Grandis <73313235+mgrandis@users.noreply.github.com>

* fix: Allow the base64Encoded field in REST Api, skip validation of unknown fields and validate missing statusCode for Http Api (aws#2941)

* fix API Gateway emulator:
 - skip validating the non allowed fields for Http Api Gateway, as it always skip the unknown fields
 - add base64Encoded as an allowed field for Rest Api gateway
 - base64 decoding will be always done for Http API gateway if the lambda response isBase64Encoded is true regardless the content-type
 - validate if statusCode is missing in case of Http API, and payload version 1.0

* - accept "true", "True", "false", "False" as valid isBase64Encoded values.
- Validate on other isBase64Encoded Values
- add more integration && unit test cases

* fix lint && black issues

* use smaller image to test Base64 response

* Fixed lint errors and added few unit tests

* Make black happy

* Added methods for deleting template artifacts

* Wait method added for delete cf api

* Added LOG statements

* Added and updated changes based on CR

* Fixed the unit tests in artifact_exporter.py

* Update HELP_TEXT in delete/command.py

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Updated code based on Chris' comments

* Added condition for resources that have deletionpolicy specified

* Small changes and fixes based on the comments

* Removed region prompt

* Added unit tests for ecr delete method and typing for methods

* Reformatted delete_context and added option to skip user prompts

* Removed return type from artifact_exporter for delete method

* Added unit tests for artifact_exporter and delete_context

* Added more unit tests for delete_context and artifact_exporter

* Added more unit tests for delete_context and artifact_exporter

* Added docs and comments for artifact_exporter and ecr_uploader

* Added log statements in delete_context and some updates in unit tests

* Init integration tests methods for sam delete

* Added more template files as a list for sam delete integration testing

* Changed force to no-prompts  and updated ecr delete method error handling

* Created a separate function for parsing ecr url in ecr_uploader

* Reformatted Template class init to pass template_str and init template_dict

* Changed how s3 url is obtained for resource_zip edge-case: aws:glue:job

* Fixed edge case where resource artifact points to a path style url

* run Make black

* Added 2 more integrations tests no_stack_deployed and delete for image type resources

* Made the parse s3 url funcs protected and defined a parent method and modified delete method for ResourceImageDict

* Added methods to extract s3 info from cf template

* Added testing for get_s3_info method for artifact_exporter and s3_uploader methods

* Added few more integration tests for delete

* Merged delete-template-artifacts branch code and updated this branch code

* Added tests for no prefix present for zip and image

* Added a check to confirm input stack is deleted for all the integration tests

* Removed commented code and updated method docstring

* Better error handling for s3 delete artifacts and fixed bug for getting s3 resources information

* Changed get_s3_info to get_property_value and changed output text for s3 delete method

* Added integration test for deleting nested stacks

Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
Co-authored-by: Mathieu Grandis <73313235+mgrandis@users.noreply.github.com>
Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>
Co-authored-by: Chris Rehn <crehn@outlook.com>

* Auto ECR Companion Stack Deletion (aws#3080)

* Added ecr_bootstrap

* Added companion_stack_manager

* Added Companion Stack Manager

* Added update_companion_stack

* Updated companion_stack_builder File Name

* Formatted with Black

* Updated get_unreferenced_repos

* Updated guided_context to Use Companion Stack

* Added Delete Auto Create ECR Repo Prompt

* Updated prompt_image_repository Flow

* Added --resolve-image-repos

* Addressed Some of Pylint Issues

* Updated Helper Text

* Updated Comments

* Fixed Typing

* Removed Unused Imports

* Updated Unit Tests

* Updated UX and Fixed Windows ANSI

* Updated Unit Tests

* Fixed Import Order

* Added Ignore Import Check

* Added Integration Tests

* Updated help text.

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Added Comments for Name Generation

* Updated Image Option Validator

* Updated CompanionStackBuilder to Use Dict instead of String

* Fixed Argument Ordering

* Added Mapping Information to Help Text

* Updated delete_unreferenced_repos Doc String

* Updated sync_repos Doc String

* Added Justification for ECR Repo Physical ID

* Refactored to be Less Coupled

* Refactored for prompt_specify_repos

* Fixed Unit Test

* Moved WaiterConfig Out of Methods

* Updated Typing

* Updated Managed S3 Template to be Dict

* Fixed Typo

* Added Comments for _save_image_repositories

* Fixed Pylint Issue

* Added Missing Check for unreferenced_repo_uris

* Updated Variable Name

* Fixed Typo

* Updated Windows Check to Use platform.system()

* Updated update_companion_stack Logic

* Fixed Comment Typo

* Fixed Typos

* Fixed Test Name

* Added methods for cf and s3 files and init UI

* Added unit tests for utils methods and s3_uploader

* Removed s3_bucket and s3_prefix click options

* chore: Increase awareness of same file warning during package (aws#2946)

* chore: increase awareness of same file warning during package

* fix formatting & grammar

Co-authored-by: Mathieu Grandis <73313235+mgrandis@users.noreply.github.com>

* fix: Allow the base64Encoded field in REST Api, skip validation of unknown fields and validate missing statusCode for Http Api (aws#2941)

* fix API Gateway emulator:
 - skip validating the non allowed fields for Http Api Gateway, as it always skip the unknown fields
 - add base64Encoded as an allowed field for Rest Api gateway
 - base64 decoding will be always done for Http API gateway if the lambda response isBase64Encoded is true regardless the content-type
 - validate if statusCode is missing in case of Http API, and payload version 1.0

* - accept "true", "True", "false", "False" as valid isBase64Encoded values.
- Validate on other isBase64Encoded Values
- add more integration && unit test cases

* fix lint && black issues

* use smaller image to test Base64 response

* Fixed lint errors and added few unit tests

* Make black happy

* Added methods for deleting template artifacts

* Wait method added for delete cf api

* fix: pass copy of environment variables for keeping cache valid (aws#2943)

* fix: pass copy of environment variables for keeping cache valid

* add integ tests

* update docs

* make black happy

Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>

* Added LOG statements

* Added and updated changes based on CR

* Fixed the unit tests in artifact_exporter.py

* Update HELP_TEXT in delete/command.py

Co-authored-by: Chris Rehn <crehn@outlook.com>

* fix: Skip build of Docker image if ImageUri is a valid ECR URL (aws#2934) (aws#2935)

* Updated code based on Chris' comments

* Added condition for resources that have deletionpolicy specified

* Small changes and fixes based on the comments

* Add condition to managed bucket policy (aws#2999)

* Removed region prompt

* Update appveyor.yml to do docker login on both dockerhub and Public ECR (aws#3005) (aws#3006)

Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>

* chore: bump version to 1.25.0 (aws#3007)

Co-authored-by: Sriram Madapusi Vasudevan <3770774+sriram-mv@users.noreply.github.com>

* temp: reduce python testing matrix (aws#3008)

* temp: disable testing against python 3.8, and enabled 3.7 (aws#3009)

* temp: disable testing against python 3.8, and enabled 3.7

* temp: disable testing against python 3.8, and enabled 3.7 & 3.6

* fix: enable all runtimes in python testing matrix (aws#3011)

* revert: enable all runtimes in python testing matrix

* fix indentation for yml

* Added unit tests for ecr delete method and typing for methods

* Reformatted delete_context and added option to skip user prompts

* Removed return type from artifact_exporter for delete method

* Added unit tests for artifact_exporter and delete_context

* Added more unit tests for delete_context and artifact_exporter

* chore: update to aws-sam-translator 1.37.0 (aws#3019)

* chore: bump version to 1.26.0 (aws#3020)

* Added more unit tests for delete_context and artifact_exporter

* Added docs and comments for artifact_exporter and ecr_uploader

* Added log statements in delete_context and some updates in unit tests

* Changed force to no-prompts  and updated ecr delete method error handling

* chore: Improved --resolve-s3 option documentation and deployment without s3 error messages (aws#2983)

* Improve documentation on --resolve-s3 option and improve s3 failure messages

* Changed indentation for integration test on s3 error message

* Fixed a typo in description

* Improve spacing on help text for resolve-s3 option

* Created a separate function for parsing ecr url in ecr_uploader

* Reformatted Template class init to pass template_str and init template_dict

* Changed how s3 url is obtained for resource_zip edge-case: aws:glue:job

* Fixed edge case where resource artifact points to a path style url

* run Make black

* Made the parse s3 url funcs protected and defined a parent method and modified delete method for ResourceImageDict

* Changed parse_ecr_url function name to parse_image_url

* Defined UI for auto ecr deleton and method calls from companion_stack_manager

* Added code for deleting repos from companion stack

* Handle json templates deployed to cf

* Changed the order of companion stack and ecr repos deletion

* Handle delete_failed status for ecr companion stack and changed delete_stack to include retain_resources

* Reformatted auto ecr deletion to handle deleting companion stack as input stack name

* Fixed and added more unit tests for delete_context

* When region is not provided, prompt user to enter profile and region

* Removed region prompt and reading it from current session or assign a default instead

* Added ECR resource in packageable_resources and refactored ecr companion stack deletion

* Added log statements and unit tests for ECRResource

* Better error handling for ecr delete_artifact

* Revert "Merge remote-tracking branch 'wiltons-repo/feat/auto-ecr' into auto-ecr-delete"

This reverts commit 0e159c2fa3630b874f13f19336802f6085a92de9, reversing
changes made to 1675b7ed231b6472d38eeeeb25e39f6310bbb86f.

* Added unit test for delete ecr repository

* Fixed small string nits and added docstring for ECRResource

* Added some unit tests for s3_uploader, ecr_uploader and delete_context

* Updated to context refresh only when region and profile have non None values and removed unused class variable in delete_context

* Added unit test for ResourceImageDict class methods

Co-authored-by: Wilton Wang <CoshUS@users.noreply.github.com>
Co-authored-by: Chris Rehn <crehn@outlook.com>
Co-authored-by: Mathieu Grandis <73313235+mgrandis@users.noreply.github.com>
Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>
Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>
Co-authored-by: Alexis Facques <mail@alexisfacques.com>
Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>
Co-authored-by: Sriram Madapusi Vasudevan <3770774+sriram-mv@users.noreply.github.com>

* Sam delete bug fixes (aws#3122)

* Fixed small bugs for no_prompts and for deleting artifacts

* Updated integration tests and added test for termination protection for sam delete

* Added few more integration tests for guided and non-guided delete

* Updated handling termination protection and changed the order of deleting artifacts and stack

* Added comments for delete artifacts to handle intrinsic ref functions and handled this for image resources

* Added integration test for retaining s3 artifact

* Changed option_name to the correct values in delete_context

* Small UX fix and updated delete_prefix_artifacts method prefix to fetch S3 files

* Added a note in s3_uploader.py about using the api list_objects_v2

* add sam delete to pyinstaller hooks file

* fix typo

* resolve pr comments

* fix refactoring issue, and add unit testing

Co-authored-by: hnnasit <84355507+hnnasit@users.noreply.github.com>
Co-authored-by: Chris Rehn <crehn@outlook.com>
Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
Co-authored-by: Mathieu Grandis <73313235+mgrandis@users.noreply.github.com>
Co-authored-by: Wilton Wang <CoshUS@users.noreply.github.com>
Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>
Co-authored-by: Alexis Facques <mail@alexisfacques.com>
Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>
Co-authored-by: Sriram Madapusi Vasudevan <3770774+sriram-mv@users.noreply.github.com>
  • Loading branch information
10 people committed Aug 18, 2021
1 parent bfbcc16 commit f61fb17
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 2 deletions.
4 changes: 2 additions & 2 deletions samcli/lib/package/local_files_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,6 @@ def get_uploaded_s3_object_name(
raise Exception("Either File Content, File Path, or Precomputed Hash should has a value")

if extension:
remote_path = filemd5 + "." + extension
filemd5 = filemd5 + "." + extension

return remote_path
return filemd5
78 changes: 78 additions & 0 deletions tests/unit/lib/package/test_local_files_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import unittest
from unittest.mock import MagicMock, patch

from samcli.lib.package.local_files_utils import get_uploaded_s3_object_name


class GetUploadedS3ObjectNameUtils(unittest.TestCase):
def setUp(self):
self.file_hash_value = "123456789"
self.extension = "template"
self.pre_calculated_hash = "345654323456543"
self.file_content = MagicMock()
self.file_path = MagicMock()

@patch("samcli.lib.package.local_files_utils.mktempfile")
@patch("samcli.lib.package.local_files_utils.file_checksum")
def test_get_uploaded_s3_object_name_with_only_file_content(self, mock_file_checksum, mock_mktempfile):
mock_file_checksum.return_value = self.file_hash_value
res = get_uploaded_s3_object_name(file_content=self.file_content)
self.assertEqual(res, self.file_hash_value)
mock_mktempfile.assert_called_once()
mock_file_checksum.assert_called_once()

@patch("samcli.lib.package.local_files_utils.mktempfile")
@patch("samcli.lib.package.local_files_utils.file_checksum")
def test_get_uploaded_s3_object_name_with_file_content_and_extension(self, mock_file_checksum, mock_mktempfile):
mock_file_checksum.return_value = self.file_hash_value
res = get_uploaded_s3_object_name(file_content=self.file_content, extension=self.extension)
self.assertEqual(res, f"{self.file_hash_value}.{self.extension}")
mock_mktempfile.assert_called_once()
mock_file_checksum.assert_called_once()

@patch("samcli.lib.package.local_files_utils.mktempfile")
@patch("samcli.lib.package.local_files_utils.file_checksum")
def test_get_uploaded_s3_object_name_with_only_file_path(self, mock_file_checksum, mock_mktempfile):
mock_file_checksum.return_value = self.file_hash_value
res = get_uploaded_s3_object_name(file_path=self.file_path)
self.assertEqual(res, self.file_hash_value)
mock_mktempfile.assert_not_called()
mock_file_checksum.assert_called_once_with(self.file_path)

@patch("samcli.lib.package.local_files_utils.mktempfile")
@patch("samcli.lib.package.local_files_utils.file_checksum")
def test_get_uploaded_s3_object_name_with_file_path_and_extension(self, mock_file_checksum, mock_mktempfile):
mock_file_checksum.return_value = self.file_hash_value
res = get_uploaded_s3_object_name(file_path=self.file_path, extension=self.extension)
self.assertEqual(res, f"{self.file_hash_value}.{self.extension}")
mock_mktempfile.assert_not_called()
mock_file_checksum.assert_called_once_with(self.file_path)

@patch("samcli.lib.package.local_files_utils.mktempfile")
@patch("samcli.lib.package.local_files_utils.file_checksum")
def test_get_uploaded_s3_object_name_with_only_pre_calculated_hash(self, mock_file_checksum, mock_mktempfile):
mock_file_checksum.return_value = self.file_hash_value
res = get_uploaded_s3_object_name(precomputed_md5=self.pre_calculated_hash)
self.assertEqual(res, self.pre_calculated_hash)
mock_mktempfile.assert_not_called()
mock_file_checksum.assert_not_called()

@patch("samcli.lib.package.local_files_utils.mktempfile")
@patch("samcli.lib.package.local_files_utils.file_checksum")
def test_get_uploaded_s3_object_name_with_pre_calculated_hash_and_extension(
self, mock_file_checksum, mock_mktempfile
):
mock_file_checksum.return_value = self.file_hash_value
res = get_uploaded_s3_object_name(precomputed_md5=self.pre_calculated_hash, extension=self.extension)
self.assertEqual(res, f"{self.pre_calculated_hash}.{self.extension}")
mock_mktempfile.assert_not_called()
mock_file_checksum.assert_not_called()

@patch("samcli.lib.package.local_files_utils.mktempfile")
@patch("samcli.lib.package.local_files_utils.file_checksum")
def test_get_uploaded_s3_object_name_with_no_calue(self, mock_file_checksum, mock_mktempfile):
mock_file_checksum.return_value = self.file_hash_value
with self.assertRaises(Exception):
get_uploaded_s3_object_name()
mock_mktempfile.assert_not_called()
mock_file_checksum.assert_not_called()

0 comments on commit f61fb17

Please sign in to comment.