search

mlevit / aws-auto-cleanup

Programmatically delete AWS resources based on an allowlist and time to live (TTL) settings
MIT License
496 stars 55 forks source link

Unable to add ECR image to the allowlist through the execution log #123

Closed anyi1985 closed 1 year ago

anyi1985 commented 1 year ago

Hello and thank you for working on these cleanup scripts.

My issue is that when I try to whitelist ecr resources through the execution log, it whitelists the repository but is unable to whitelist any image. When trying to whitelist an image I get the following errors:

I have tested on 2 accounts but get the same invalid resource ID error messages.

Thank you.

mlevit commented 1 year ago

Hey @anyi1985 thanks for raising this. Must have never really tested this myself with an actual image digest. There were a few problems including not limiting the split in Python and a difference in the way Python and JavaScript split functions work.

Should be fixed in https://github.com/servian/aws-auto-cleanup/pull/124. Could you do me a favour and pull that branch and test it for yourself before I merge it?

anyi1985 commented 1 year ago

Thank you @mlevit The changes you made to the branch worked for me. I am now able to whitelist my ecr images. Thank you for the fix

anyi1985 commented 1 year ago

@mlevit It looks like there actually wasn't an issue with the ECR image cleanup never being run, because the following error comes up: self.images() TypeError: images() missing 1 required positional argument: 'repository'

The image cleanup takes in a repo name which is only passed in from the repo cleanup, so I think it should be fine to remove self.images(). https://github.com/servian/aws-auto-cleanup/pull/124/commits/f59a36af9f9099b662c1d6f8170c455899ff3ff5

Whenever the repo cleanup runs, it'll delete the images as well for the list of repos.

I guess this could be an issue since theoretically only repos could be enabled and then the images would also get deleted regardless, and then only the repos would get list in the execution log and not the images, causing the exec log not to be accurate. As a result, if the ecr repo cleanup is enable, then the ecr image cleanup needs to be enabled as well so the exec log shows both repos and images that would get deleted. I think it may help to just add a note mentioning that for ecr, both repos and images need to be enabled for the cleanup to work properly.

Lastly, when the ecr images could not be added to the exception list (now fixed), while the error message that came up was correct, the notification bar below was in green instead of red.

mlevit commented 1 year ago

@anyi1985 haha yep, I forgot my own code 😄

The images cleanup is called within the repositories cleanup... you're right.

I'll look into the notification bar colour, I noticed that too.