fix: allow copilot to delete access logs s3 buckets on env delete#5184
Merged
mergify[bot] merged 29 commits intoaws:mainlinefrom Aug 16, 2023
Merged
Conversation
added 6 commits
August 9, 2023 11:09
CaptainCarpensir
requested review from
dannyrandall
and removed request for
a team
|
🍕 Here are the new binary sizes!
|
Lou1415926
reviewed
Aug 10, 2023
Codecov Report
@@ Coverage Diff @@
## mainline #5184 +/- ##
============================================
- Coverage 69.53% 69.52% -0.02%
============================================
Files 295 295
Lines 43819 43896 +77
Branches 285 285
============================================
+ Hits 30471 30517 +46
- Misses 11863 11889 +26
- Partials 1485 1490 +5
|
Contributor
|
👏🏼 Great job! And nice unit tests! |
huanjani
added
the
do-not-merge
huanjani
approved these changes
Aug 11, 2023
Contributor
huanjani
left a comment
huanjani
left a comment
There was a problem hiding this comment.
Feel free to remove the DNM label when you're done with the error-handling piece.
…com/CaptainCarpensir/copilot-cli into aiden/env-access-logs-bucket-delete
CaptainCarpensir
removed
the
do-not-merge
added 3 commits
August 15, 2023 15:18
go doesn't like when i format strings without using a formatting directive
Lou1415926
reviewed
Aug 15, 2023
Need to merge to fix unit test error
…com/CaptainCarpensir/copilot-cli into aiden/env-access-logs-bucket-delete
Lou1415926
approved these changes
Aug 16, 2023
Contributor
Lou1415926
left a comment
Lou1415926
left a comment
There was a problem hiding this comment.
lgtm, feel free to remove the label when you are ready!
Lou1415926
added
the
do-not-merge
CaptainCarpensir
removed
the
do-not-merge
mergify Bot
pushed a commit
that referenced
this pull request
Aug 29, 2023
Removes the spinner for emptying buckets. This is because most applications won't actually have any s3 buckets to delete, and showing this is highly misleading to users. Follow up to #5184 By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
KollaAdithya
pushed a commit
to KollaAdithya/copilot-cli
that referenced
this pull request
Oct 18, 2023
Removes the spinner for emptying buckets. This is because most applications won't actually have any s3 buckets to delete, and showing this is highly misleading to users. Follow up to aws#5184 By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes four changes to
env deleteto allow copilot to delete S3 buckets managed by the environment, particularly the access logs bucket. These are the following changes:CreateALBcondition from the access logs bucket. This will cause the bucket to be created as soon as it’s configured in the manifest and deployed to the environment, and not require any services to be deployed. This will then mean that the resource isn’t deleted as the final service is deleted from the environment.EnvManagerRolepermission to empty the access logs bucket.env deleteempty the bucket before the stack deletion. Then, the stack deletion should run normally and continue without errors.env deletefails to delete a bucket, warn the user that they need to delete it manually.If a user redeploys their environment before deleting an environment, this change fixes the problem completely. If not, it will warn them to delete the bucket manually.
Addresses #4218
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.