Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amend destroy_env_no_terraform.sh to purge container repos #4230

Merged
merged 7 commits into from
Jan 1, 2025

Conversation

jonnyry
Copy link
Collaborator

@jonnyry jonnyry commented Dec 29, 2024

Problem

When using Defender to scan container images -

If the container registry is deleted as a whole (instead of deleting individual container repositories first), Defender does not remove scans of container images.

If redeploying a fresh TRE using the same name as previously, this leads to misleading results in Defender in dev/test environments with repeated scans 'stacking up' and making the results look larger than they actually are (see screen shots below).

Example of Defender results that are 'stacking up':

image

Example of Defender holding on to scans of deleted images (there is only 1 image in each repository):

image

Diagnosis

According to this Microsoft documentation, container scans should be deleted within an hour, or maximum three days, however our experience is that container scans for deleted images (where the registry was deleted as a whole) can persist for up to two weeks.

If I remove an image from my registry, how long before vulnerabilities reports on that image would be removed?

Azure Container Registries notifies Defender for Cloud when images are deleted, and removes the vulnerability assessment for deleted images within one hour. In some rare cases, Defender for Cloud might not be notified on the deletion, and deletion of associated vulnerabilities in such cases might take up to three days.

It appears the 'hook' Defender uses to remove scans does not work when the ACR is deleted as a whole with images remaining in it, and only functions as intended when repositories/images are deleted individually first.

Solution

This PR amends the destroy_env_no_terraform.sh script to individually delete container repositories, before the script continues as usual to delete the resource groups (including the container registry).

This ensures the 'hook' Defender uses to remove scans fires and scans are cleared up when a TRE is destroyed.

Copy link

github-actions bot commented Dec 29, 2024

Unit Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 07880e7.

♻️ This comment has been updated with latest results.

@tamirkamara
Copy link
Collaborator

@jonnyry shouldn't the script just find the ACRs in the groups it's going to delete?

@jonnyry
Copy link
Collaborator Author

jonnyry commented Dec 29, 2024

@jonnyry shouldn't the script just find the ACRs in the groups it's going to delete?

Yes good point, probably should - I'll amend.

@jonnyry jonnyry requested a review from tamirkamara December 29, 2024 20:08
@jonnyry
Copy link
Collaborator Author

jonnyry commented Dec 29, 2024

@tamirkamara Now updated to search for containter registries rather than requiring a separate parameter.

@jonnyry
Copy link
Collaborator Author

jonnyry commented Dec 31, 2024

/test 07880e7

Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/12562577242 (with refid 853a7c83)

(in response to this comment from @jonnyry)

@jonnyry
Copy link
Collaborator Author

jonnyry commented Dec 31, 2024

/test 07880e7

Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/12563009383 (with refid 853a7c83)

(in response to this comment from @jonnyry)

@jonnyry
Copy link
Collaborator Author

jonnyry commented Dec 31, 2024

/destroy-test-env

Copy link

🤖 pr-bot 🤖

/destroy-test-env is not recognised as a valid command.

You can use the following commands:
    /test - build, deploy and run smoke tests on a PR
    /test-extended - build, deploy and run smoke & extended tests on a PR
    /test-extended-aad - build, deploy and run smoke & extended AAD tests on a PR
    /test-shared-services - test the deployment of shared services on a PR build
    /test-force-approve - force approval of the PR tests (i.e. skip the deployment checks)
    /test-destroy-env - delete the validation environment for a PR (e.g. to enable testing a deployment from a clean start after previous tests)
    /help - show this help

(in response to this comment from @jonnyry)

@jonnyry
Copy link
Collaborator Author

jonnyry commented Dec 31, 2024

/test-destroy-env

Copy link

Destroying PR test environment (RG: rg-tre853a7c83)... (run: https://github.com/microsoft/AzureTRE/actions/runs/12563557593)

Copy link

PR test environment destroy complete (RG: rg-tre853a7c83)

Copy link
Collaborator

@tim-p-allen tim-p-allen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jonnyry jonnyry merged commit a555afc into microsoft:main Jan 1, 2025
12 checks passed
@jonnyry jonnyry deleted the jr/85-purge-container-repos branch January 1, 2025 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants