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

refactor(scanner): propagate context to scanners/analyzers #1741

Merged
merged 10 commits into from
Jun 11, 2024

Conversation

zsoltkacsandi
Copy link
Contributor

@zsoltkacsandi zsoltkacsandi commented May 27, 2024

Description

Passing context to scanners/analyzers.

Type of Change

[ ] Bug Fix
[ ] New Feature
[ ] Breaking Change
[X] Refactor
[ ] Documentation
[ ] Other (please describe)

Checklist

  • I have read the contributing guidelines
  • Existing issues have been referenced (where applicable)
  • I have verified this change is not present in other open pull requests
  • Functionality is documented
  • All code style checks pass
  • New code contribution is covered by automated tests
  • All new and existing tests pass

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@zsoltkacsandi zsoltkacsandi marked this pull request as ready for review May 29, 2024 12:51
@zsoltkacsandi zsoltkacsandi requested a review from a team as a code owner May 29, 2024 12:51
@zsoltkacsandi zsoltkacsandi self-assigned this May 29, 2024
paralta
paralta previously approved these changes May 29, 2024
Copy link
Contributor

@paralta paralta left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

ramizpolic
ramizpolic previously approved these changes Jun 3, 2024
Copy link
Member

@ramizpolic ramizpolic left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

scanner/families/exploits/exploitdb/exploitdb.go Outdated Show resolved Hide resolved
@zsoltkacsandi zsoltkacsandi dismissed stale reviews from ramizpolic and paralta via c5f1520 June 4, 2024 13:18
ramizpolic
ramizpolic previously approved these changes Jun 4, 2024
@zsoltkacsandi zsoltkacsandi force-pushed the refactor-propagate-context-in-scanners branch from c5f1520 to d660298 Compare June 4, 2024 13:21
@ramizpolic
Copy link
Member

Note: CIS Docker Benchmark scanner needs to be updated upstream to support contexts being passed when used as a library. It should either be added as part of Config or directly in calling function at https://github.com/Portshift/dockle/blob/4f3b165086b7adbaf853ece809c7c61497bb52d7/pkg/run.go#L44. Without this, we cannot force exit for CIS Docker scanner. This can be done in another PR once there is support for passing ctx in Dockle.

This comment has been minimized.

ramizpolic
ramizpolic previously approved these changes Jun 4, 2024

This comment has been minimized.

ramizpolic
ramizpolic previously approved these changes Jun 4, 2024
@akijakya
Copy link
Contributor

akijakya commented Jun 6, 2024

Note: CIS Docker Benchmark scanner needs to be updated upstream to support contexts being passed when used as a library. It should either be added as part of Config or directly in calling function at https://github.com/Portshift/dockle/blob/4f3b165086b7adbaf853ece809c7c61497bb52d7/pkg/run.go#L44. Without this, we cannot force exit for CIS Docker scanner. This can be done in another PR once there is support for passing ctx in Dockle.

Wouldn't that be a breaking change for the Dockle scanner? Time for our own fork? 😃 A ticket should be created for this anyway if there isn't one already...

@ramizpolic
Copy link
Member

Note: CIS Docker Benchmark scanner needs to be updated upstream to support contexts being passed when used as a library. It should either be added as part of Config or directly in calling function at https://github.com/Portshift/dockle/blob/4f3b165086b7adbaf853ece809c7c61497bb52d7/pkg/run.go#L44. Without this, we cannot force exit for CIS Docker scanner. This can be done in another PR once there is support for passing ctx in Dockle.

Wouldn't that be a breaking change for the Dockle scanner? Time for our own fork? 😃 A ticket should be created for this anyway if there isn't one already...

It wont be a breaking change for Dockle. @zsoltkacsandi has a way to address this in a backwards compatible way by making the run function that performs the scan in Dockle public.

Copy link

Hey!

Your images are ready:

  • ghcr.io/openclarity/vmclarity-apiserver-dev:pr1741-474da7ba703b469c1e23d902dc8b24819806804a
  • ghcr.io/openclarity/vmclarity-cli-dev:pr1741-474da7ba703b469c1e23d902dc8b24819806804a
  • ghcr.io/openclarity/vmclarity-cr-discovery-server-dev:pr1741-474da7ba703b469c1e23d902dc8b24819806804a
  • ghcr.io/openclarity/vmclarity-orchestrator-dev:pr1741-474da7ba703b469c1e23d902dc8b24819806804a
  • ghcr.io/openclarity/vmclarity-plugin-kics-dev:pr1741-474da7ba703b469c1e23d902dc8b24819806804a
  • ghcr.io/openclarity/vmclarity-ui-dev:pr1741-474da7ba703b469c1e23d902dc8b24819806804a
  • ghcr.io/openclarity/vmclarity-ui-backend-dev:pr1741-474da7ba703b469c1e23d902dc8b24819806804a

@zsoltkacsandi zsoltkacsandi changed the title refactor: propagate context to scanners/analyzers refactor(scanner): propagate context to scanners/analyzers Jun 11, 2024
@zsoltkacsandi zsoltkacsandi added this pull request to the merge queue Jun 11, 2024
Merged via the queue into main with commit e27ff8b Jun 11, 2024
41 checks passed
@zsoltkacsandi zsoltkacsandi deleted the refactor-propagate-context-in-scanners branch June 11, 2024 13:06
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.

None yet

4 participants