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

feat: support dockerfile base image vulnerability scan #150

Merged
merged 8 commits into from
Apr 23, 2024

Conversation

xieshenzh
Copy link
Collaborator

@xieshenzh xieshenzh commented Mar 19, 2024

Support scanning base image in dockerfile

Jira: https://issues.redhat.com/browse/APPENG-2253

Changed the RHDA plugin to support IntelliJ 2022.1+, due to breaking changes to the Docker plugin which is required for dockerfile scanning.

@xieshenzh xieshenzh force-pushed the dockerfile branch 3 times, most recently from 93132de to be0e95c Compare March 21, 2024 18:43
@xieshenzh xieshenzh marked this pull request as ready for review March 21, 2024 18:45
@xieshenzh
Copy link
Collaborator Author

@zvigrinberg Could you please take a look at this PR for dockerfile base image scanning? Thanks.

This PR depends on the changes of RHEcosystemAppEng/exhort-java-api#96

Copy link
Collaborator

@zvigrinberg zvigrinberg left a comment

Choose a reason for hiding this comment

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

@xieshenzh in Overall good job
for what concern snyk token, let's either remove it completely ( as snyk integration is discontinued) , or just hide its label and text field ( there is a chance that there will be arrangement and agreement between us and snyk, so once it will happen, we can just toggle the two setVisible method' argument from false to true and that's it, rather than re-insert the code again , you call, either do that or keep snyk text field + label code aside in a topic branch in your fork or in the upstream repository , and in case snyk will do "comeback", then it will be easier to merge it instantly...

In addition, please let QE test this version according to the agreed scenarios , and once we get green light from them, and we removing either way the snyk token text field and label, we're good to go ( you just need to bump version of exhort-java-api to 0.0.7-SNAPSHOT after we'll merge exhort-java-api' Image scanning PR).

@xieshenzh
Copy link
Collaborator Author

@xieshenzh in Overall good job for what concern snyk token, let's either remove it completely ( as snyk integration is discontinued) , or just hide its label and text field ( there is a chance that there will be arrangement and agreement between us and snyk, so once it will happen, we can just toggle the two setVisible method' argument from false to true and that's it, rather than re-insert the code again , you call, either do that or keep snyk text field + label code aside in a topic branch in your fork or in the upstream repository , and in case snyk will do "comeback", then it will be easier to merge it instantly...

In addition, please let QE test this version according to the agreed scenarios , and once we get green light from them, and we removing either way the snyk token text field and label, we're good to go ( you just need to bump version of exhort-java-api to 0.0.7-SNAPSHOT after we'll merge exhort-java-api' Image scanning PR).

Thanks @zvigrinberg . I removed the code for snyk, the changes are in a separate commit. We can revert it if we want to support it again in the future.

@zvigrinberg
Copy link
Collaborator

@xieshenzh I Couldn't find any usages or references in this PR for the new EXHORT_SYFT_IMAGE_SOURCE property
I Remember that i saw it before as a setting implemented as radiobutton that let the user selecting the image source container runtime cli tool.
But maybe it exists and i just missed it.

@xieshenzh
Copy link
Collaborator Author

@xieshenzh I Couldn't find any usages or references in this PR for the new EXHORT_SYFT_IMAGE_SOURCE property I Remember that i saw it before as a setting implemented as radiobutton that let the user selecting the image source container runtime cli tool. But maybe it exists and i just missed it.

Hi @zvigrinberg ,

EXHORT_SYFT_IMAGE_SOURCE is used to specify the source from which Syft gets the images.

Originally, there are four values (radio buttons) for users to select in the plugin configuration:

  • registry: get image directly from remote registry
  • docker: get image from docker daemon offline
  • podman: get image from podman daemon offline
  • default: use docker, podman, and containerd daemons, if no success try the remote registry

Then I figured, the plugin always sends request to the exhort backend, it will not work offline.
And the default option covers all the other three options.
So I deleted this property, and simplified the plugin configuration.
The plugin will always use the default option.

But this property still exists in the exhort-java-api, in case there are more scenarios to support in the future (e.g. read image from local disk).

Copy link
Collaborator

@zvigrinberg zvigrinberg left a comment

Choose a reason for hiding this comment

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

LGTM + Approved by QE.
Approved for merge and release.

README.md Show resolved Hide resolved
@ritz303
Copy link

ritz303 commented Apr 22, 2024

@xieshenzh : The content looks good to me.

Copy link

sonarcloud bot commented Apr 23, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@xieshenzh
Copy link
Collaborator Author

@ruromero Could you please merge this PR? Thanks.

@ruromero ruromero merged commit 3657233 into redhat-developer:main Apr 23, 2024
7 of 8 checks passed
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.

5 participants