feat: Option to fail the build when scan results cannot be downloaded

[lucaswilric]

This change adds a boolean configuration item fail-build-on-plugin-failure. It allows users to treat all errors in the plugin as build failures, as suggested in the README.

fail-build-on-plugin-failure is false by default, preserving current behaviour.

When fail-build-on-plugin-failure is set to true, any error returned from runCommand will be treated as fatal.

Note: Since the load-bearing part of this change is in main.go, I was not quite sure how you would prefer to have it tested. I'd love some feedback on that in particular :)

[github-actions]

Package	Line Rate	Health
github.com/cultureamp/ecrscanresults/finding	95%	✔
github.com/cultureamp/ecrscanresults/findingconfig	70%	➖
github.com/cultureamp/ecrscanresults/registry	11%	❌
github.com/cultureamp/ecrscanresults/report	86%	✔
github.com/cultureamp/ecrscanresults/runtimeerrors	83%	✔
Summary	66% (444 / 674)	➖

[jamestelfer]

@lucaswilric Thanks for this PR. I apologise that I haven't seen this till now: that's on me.

I will have a look and respond: please let me know if you're still happy to work on this!

[lucaswilric]

Hi @jamestelfer! I'm still happy to contribute on this one. I have some AFK time coming up, so it's possible one of my teammates may pitch in instead.

Since I posted this PR, my team has thought it'd be helpful to use separate exit codes for "couldn't get results" and "got results and they're bad". This would provide a nice hook into Buildkite's soft_fail functionality. What do you think?

[jamestelfer]

I'm still happy to contribute on this one.

Thank you!

be helpful to use separate exit codes for "couldn't get results" and "got results and they're bad".

Sounds good to me!

This plugin runs in a post-checkout hook: we're generally using it as a follow up to a docker-compose plugin definition that pushes the container. If we retry the whole step, it will republish the image. However: it's definitely possible to run this plugin with a no-op script on the step so it runs standalone.

Depending on the use-case this would make sense, so I'm happy to see this go in (with the caveats documented).

Main reasons for failures in my experience are:

• can't process this image at all
• didn't get a report in time (timeout waiting for the status to resolve)
• image reference was wrong
• AWS credentials timed out
• IAM permissions borked

For the "didn't get a report in time" issue, an ability to extend the timeout could be given.

How are you using the plugin? Would retries help for your failure scenarios?

I was not quite sure how you would prefer to have it tested.

(from the PR description) Gotta say, I'm not sure how I'd like it tested either. I've toyed with the idea of factoring it around to enable this, but I'm really happy to listen to something that sounds simpler.

[jamestelfer]

Overall LGTM.

Testing main is currently not really there: I'd be happy for you to suggest what you'd like to see, either in this or another PR.

The exit_code change would also be OK: feel free to use another PR if you'd like to. The current one has hung around for ages so it would be good to get this one moving.

[jamestelfer]

nit: it would be nice for this to link, but not essential