alexjurkiewicz / ecr-scan-image

Github Action to run AWS ECR vulnerability scan on Docker image
MIT License
28 stars 23 forks source link

feat: adds support for enhanced continuous scanning #46

Closed jdew89 closed 7 months ago

jdew89 commented 8 months ago

Adding support for enhanced continuous scanning in ECR. This setting returns a different JSON structure which breaks the action.

This PR resolves #45

The changes are as follows:

alexjurkiewicz commented 8 months ago

Thanks for this PR! It's a long-overdue enhancement.

One concern I have is that this PR duplicates key code paths to handle differences in basic & enhanced scan message formats.

Why don't we drop basic scan support?

This action has history of breaking backwards compatibility on the master branch, when v2.0.0 was released last year. So I think it's fine to once again break backwards compat and drop basic scanning support.

jdew89 commented 8 months ago

Yeah, I'm disappointed AWS made them have different JSON structures. Makes this more complicated than it really needs to be. I could remove the basic scanning support and you could bump to version 3.0. People looking for basic scanning support could still use v2.0.1 and enhanced scanners can use v3.0. Is that what you would like to do?

alexjurkiewicz commented 8 months ago

Yes please!

pzi commented 8 months ago

My first thought was to make it a setting and assume "Basic" until a "useEnhancedScan" feature flag is set to true. Having said that, I don't use this workflow any more... whatever works best for the people using it :)

alexjurkiewicz commented 8 months ago

I saw this AWS announcement, basic scanning will get some new responses soon anyway. Seems like a good time to drop support:

Starting May 1, 2024, you will receive the following error when using describe-image-scan-findings API [2] or describe-images API [3] to retrieve the scan findings for images running an operating system version that is not in our supported version list [1]:

    "imageScanStatus": {
        "status": "FAILED",
        "description": "UnsupportedImageError: The operating system '<OS>' version '<version>' is not supported."
    }

You will receive the following error message when attempting to view scan findings for those images via the AWS console:
"UnsupportedImageError: The operating system '<OS>' version '<version>' is not supported."

In addition, you will see a ‘FAILED’ status in the ‘detail’ field of the EventBridge event [4] that is sent when the image scan is completed.
{
    "version":"0",
    "id":"7c19afad-7203-e3ae-61c9-5a905e6963d2",
    "detail-type":"ECR Image Scan",
    "source":"aws.ecr",
    "account":"<accountId>",
    "time":"<timestamp>",
    "region":"<region>",
    "resources":["<resource_arn>"],
    "detail": {
        "scan-status":"FAILED",
        "repository-name":"<repository_name>",
        "image-digest":"<image_digest>",
        "image-tags":["<image_tags>"]
    }
}

For continued coverage, please use supported operating system versions and make the necessary updates prior to May 1, 2024. If you have any questions or concerns, please contact AWS Support [5]

[1] https://docs.aws.amazon.com/AmazonECR/latest/userguide/image-scanning-basic.html
[2] https://docs.aws.amazon.com/cli/latest/reference/ecr/describe-image-scan-findings.html
[3] https://docs.aws.amazon.com/cli/latest/reference/ecr/describe-images.html
[4] https://docs.aws.amazon.com/AmazonECR/latest/userguide/ecr-eventbridge.html
[5] https://aws.amazon.com/support
jdew89 commented 8 months ago

Makes sense. I'll try and finish up the PR this week. I've been pretty busy.

jdew89 commented 8 months ago

@alexjurkiewicz I found some time today to update it. Basic scanning dependent code was removed. Let me know what you think.

pzi commented 8 months ago

Thank you! This looks great. If you comments from @pzi , I'll merge and cut a release ASAP

No need to wait for me. Get cutting :)

pzi commented 8 months ago

Thank you! This looks great. If you comments from @pzi , I'll merge and cut a release ASAP

No need to wait for me. Get cutting :)

Having said that... does the README need updating at all to make it clear it now relies on the enhanced scan and it will fail if that's not used? Might as well be upfront and not wait until people complain :)

jdew89 commented 8 months ago

I've added a README update. Let me know if you think this is sufficient.

pzi commented 8 months ago

I've added a README update. Let me know if you think this is sufficient.

LGTM, thanks for that!

alexjurkiewicz commented 7 months ago

Sorry for the big delays. You know... life. This is merged and let's wait for bug reports from people pinned to @master :)