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

fix: print all dep-graphs of a container scan #5399

Closed
wants to merge 9 commits into from

Conversation

mcombuechen
Copy link
Contributor

What does this PR do?

When printing the dep-graph results from Snyk container SCA, the code would escape early and omit all the dep-graphs that might have been found during remote analysis. This commit moves the dep-graph printing during container SCA to after the remote analysis call, this way including additional dep-graphs of applications within the container image.

When printing the dep-graph results from Snyk container SCA, the code
would escape early and omit all the dep-graphs that might have been
found during remote analysis. This commit moves the dep-graph printing
during container SCA to after the remote analysis call, this way including
additional dep-graphs of applications within the container image.
@mcombuechen mcombuechen requested a review from a team as a code owner August 2, 2024 13:13
Copy link
Contributor

@cmars cmars left a comment

Choose a reason for hiding this comment

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

First pass looks good! See suggestion below to help this scale better for large objects. Heap space is unfortunately capped in the NodeJS process though, which can lead to incorrect outputs (or possibly process crashes due to OOMs). We have a couple open tickets where containers have tripped this limit.

src/lib/snyk-test/run-test.ts Outdated Show resolved Hide resolved
src/lib/snyk-test/run-test.ts Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Aug 6, 2024

Fails
🚫

"wip: print output via streams" is not using a valid commit message format. For commit guidelines, see: CONTRIBUTING.

Warnings
⚠️

Since the CLI is unifying on a standard and improved tooling, we're starting to migrate old-style imports and exports to ES6 ones.
A file you've modified is using either module.exports or require(). If you can, please update them to ES6 import syntax and export syntax.
Files found:

  • test/tap/cli-test/cli-test.docker.spec.ts

Generated by 🚫 dangerJS against 4ffe84d

@mcombuechen mcombuechen marked this pull request as draft August 9, 2024 07:52
@mcombuechen
Copy link
Contributor Author

Too much happening, too much failing, tap is driving me up the wall. Will open smaller-scoped PRs instead.

@mcombuechen
Copy link
Contributor Author

@cmars will close this in favour of two other PRs.

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.

2 participants