-
Notifications
You must be signed in to change notification settings - Fork 5
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
Initial CLI scanner commit #18
Merged
Jujuyeh
merged 18 commits into
sysdiglabs:dev-new-engine
from
aaronm-sysdig:cli-scanner-v2
May 22, 2024
Merged
Initial CLI scanner commit #18
Jujuyeh
merged 18 commits into
sysdiglabs:dev-new-engine
from
aaronm-sysdig:cli-scanner-v2
May 22, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ented out until agree to commit to main
…ented out until agree to commit to main
…ented out until agree to commit to main
# Conflicts: # README.md # docs/install.md
Jujuyeh
approved these changes
May 22, 2024
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.
Awesome work - LGTM
Jujuyeh
added a commit
that referenced
this pull request
May 27, 2024
* Initial CLI scanner commit (#18) * Initial CLI scanner commit * Update install.md * Using bash not sh * Update for CVSS and temp removal of description * Update to revert to v1Beta1 public APIs. Leaving in most V2 code commented out until agree to commit to main * Update to revert to v1Beta1 public APIs. Leaving in most V2 code commented out until agree to commit to main * Update to revert to v1Beta1 public APIs. Leaving in most V2 code commented out until agree to commit to main * Removing v2 code altogether * Update README.md * Update for SaaS * Update install.md * Readme Updates. * Readme Updates. * Mock Update(s) * Test updates and removal of backend tests that are no longer needed * Update ci.yaml * Update image digest for tests * Set original test image * Update image digest for tests * Pump up golang builder version in Dockerfile * Updated tests, commented out backend adapter (#20) * Dev new engine (#21) * Updated tests, commented out backend adapter * Update of scanner job logic to handle the sysdig-cli-scanner container * Dev new engine (#22) * Updated tests, commented out backend adapter * Update of scanner job logic to handle the sysdig-cli-scanner container * Test update to cater for new job spec --------- Co-authored-by: Aaron Miles <[email protected]>
Jujuyeh
added a commit
that referenced
this pull request
Jun 20, 2024
* Initial CLI scanner commit (#18) * Initial CLI scanner commit * Update install.md * Using bash not sh * Update for CVSS and temp removal of description * Update to revert to v1Beta1 public APIs. Leaving in most V2 code commented out until agree to commit to main * Update to revert to v1Beta1 public APIs. Leaving in most V2 code commented out until agree to commit to main * Update to revert to v1Beta1 public APIs. Leaving in most V2 code commented out until agree to commit to main * Removing v2 code altogether * Update README.md * Update for SaaS * Update install.md * Readme Updates. * Readme Updates. * Mock Update(s) * Test updates and removal of backend tests that are no longer needed * Update ci.yaml * Update image digest for tests * Set original test image * Update image digest for tests * Pump up golang builder version in Dockerfile * Updated tests, commented out backend adapter * Update of scanner job logic to handle the sysdig-cli-scanner container * Test update to cater for new job spec * Update to inherit pod and container security context from main job * revert vuln sha to master version * removed errant fmt.printf and replaced with proper logging * Testing giving scanner time to spin up before checking if scan is taking place * Disable report creation still ongoing until we acn ascertain the time sensitive condition breaking the test --------- Co-authored-by: Paul Hodgetts <[email protected]>
Merged
Jujuyeh
added a commit
that referenced
this pull request
Jun 20, 2024
* Initial CLI scanner commit (#18) * Initial CLI scanner commit * Update install.md * Using bash not sh * Update for CVSS and temp removal of description * Update to revert to v1Beta1 public APIs. Leaving in most V2 code commented out until agree to commit to main * Update to revert to v1Beta1 public APIs. Leaving in most V2 code commented out until agree to commit to main * Update to revert to v1Beta1 public APIs. Leaving in most V2 code commented out until agree to commit to main * Removing v2 code altogether * Update README.md * Update for SaaS * Update install.md * Readme Updates. * Readme Updates. * Mock Update(s) * Test updates and removal of backend tests that are no longer needed * Update ci.yaml * Update image digest for tests * Set original test image * Update image digest for tests * Pump up golang builder version in Dockerfile * Updated tests, commented out backend adapter * Update of scanner job logic to handle the sysdig-cli-scanner container * Test update to cater for new job spec * Update to inherit pod and container security context from main job * revert vuln sha to master version * removed errant fmt.printf and replaced with proper logging * Testing giving scanner time to spin up before checking if scan is taking place * Disable report creation still ongoing until we acn ascertain the time sensitive condition breaking the test --------- Co-authored-by: Aaron Miles <[email protected]>
Jujuyeh
added a commit
that referenced
this pull request
Jul 2, 2024
* Initial CLI scanner commit (#18) * Initial CLI scanner commit * Update install.md * Using bash not sh * Update for CVSS and temp removal of description * Update to revert to v1Beta1 public APIs. Leaving in most V2 code commented out until agree to commit to main * Update to revert to v1Beta1 public APIs. Leaving in most V2 code commented out until agree to commit to main * Update to revert to v1Beta1 public APIs. Leaving in most V2 code commented out until agree to commit to main * Removing v2 code altogether * Update README.md * Update for SaaS * Update install.md * Readme Updates. * Readme Updates. * Mock Update(s) * Test updates and removal of backend tests that are no longer needed * Update ci.yaml * Update image digest for tests * Set original test image * Update image digest for tests * Pump up golang builder version in Dockerfile * Updated tests, commented out backend adapter * Update of scanner job logic to handle the sysdig-cli-scanner container * Test update to cater for new job spec * Update to inherit pod and container security context from main job * revert vuln sha to master version * removed errant fmt.printf and replaced with proper logging * Testing giving scanner time to spin up before checking if scan is taking place * Disable report creation still ongoing until we acn ascertain the time sensitive condition breaking the test * Fixed namespace -> namespace_name typo * Fixed namespace -> namespace_name typo * Fixed namespace -> namespace_name typo * logging output change * Updated job spec and code cleanup for lendi * Updated job spec and code cleanup for lendi * Updated job spec and code cleanup for lendi * Updated job spec and code cleanup for lendi * test update --------- Co-authored-by: Paul Hodgetts <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Initial PR for CLI v2 scanner functionality.
The main pain points here are the use of V2 api's due to the way that the
v1beta
apis do not carry the description and URL which we need (or if we don't we can cut it out... but its good to have them imho)The methodology that I have taken is to try and be as respectful to the current logic as much as possible. So some things are a little longer than they would be had we have re-written everything from scratch. So for example rather than creating a new
VulnerabilityItem
construct for V2, it simply pulls the data from the V2 endpoints then transposes them into the v1 construct. This aides in allowing the logic to flow through the various other functions unchanged.Points of Note that still need to be considered
API's used are v2 api's as the current v1beta APIs for v2 functionality do not have (so far as I could tell) an API that would let you link together a scan result with an API that would let you query the description for a vulnerability like the v1 anchore API's do
Descriptions are pulled from vulnPkgs endpoint. Sadly I have not found an API that allows us to get more than one description at once (i.e more than one image). This means that we hit up against the APi rate limit
To the point of API rate limiting, I have modified the
doRequest
function to handle429 - Too many requests
and some rudimentary backoff functionality is implemented. Ideally we need to either 1) find a better API that gives all the descriptions and URLs we need OR have somewhere in the README that tells people to contact sysdig support to have their rate limit upgraded from 50 to xxx ?Vuln URLS come from the same description endpoint as could not find it in v1beta APi's.
Logging has been extended retrospectively into the base adapter and logging of payloads is present in Debug mode.
The API payload spec has been moved to
1.1
, i.e"application/vnd.security.vulnerability.report; version=1.1
. This allows us to present CVSS data into the report which was not present in1.0
APi spec as provided by Harbor is not correct. I had to scrape one from the trivvy scanner to see how to make it work (using vendor attributes
Updates vendor tag from
Sysdig 3.x
toSysdig SaaS
.Readme has been updated to replace
inline
scanning toCLI
scanningCommand line parameter for scanning has changed from
--inline_scanning
to--cli_scanning
Pod spec has changed to use a container that implements the CLI scanner. Currently just using a version that I wrote, but we will need to change this to sysdig hosted one. Dockerfile is below
CVSS Datai is now present with the 'tag' being
Base_Score
. This looks pretty good in the UI and is how other scanners present the data.Updated artifact tag stanza with image data from V2. Some transposing is needed so I have split it up into segments. Means more lines of code, but easier to read imho.
Implemented
REGISTRY_USER
andREGISTRY_PASSWORD
functionality that is (quietly) part of the CLI scanner to handle the robot account that is created for each scan so you can pull the imageThere is a bug in the UI at the moment that causes new vuln scans to now show.
https://github.com/goharbor/harbor/issues/18523
is where I found it (took HOURS of beating head against a wall to work this out). The workaround is to clear the vuln tables for the scans in question. For debugging I just truncate the table withtruncate scan_report, vulnerability_record cascade;
. I wonder if the customer knows this??