-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add CICD Tool Discovery Plugin to Backend #344
Conversation
backend/engine/plugins/cicd_tools/detectors/aws_codebuild_detector.py
Outdated
Show resolved
Hide resolved
|
||
def get_details(results: list[DetectorResult]) -> dict[str, dict[str, CICDToolsDetails]]: | ||
return { | ||
"cicd_tools": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is the JSON output I proposed for this plugin, but this was just a swag - please review just to ensure that it complies with output from similar APIs in the system... for example, scan_options.plugins
in a scan object is just just an array of the internal plugin ids (no display name), and we maintain an internal mapping in backend & UI to the display names. Would this be a better and more consistent approach?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think a scan_options.plugins
type mapping would make sense in this context, but I also don't have a good grasp of what you are envisioning there. At a minimum, it would have the added complexity of causing us to have to edit things in two places when we want to add additional CICD tools or modify the current ones.
The current pattern is similar to the one in the repo healthcheck plugins, where there's an ID for the particular finding and then the plugins also provide a plaintext display_name
analogue that eventually gets displayed in the UI. It keeps everything localized in the plugin, so changing things is more intuitive, IMO.
Adds a CICD Tool Discovery plugin to the backend
Description
Motivation and Context
How Has This Been Tested?
Types of changes
Checklist