vladklokun commented 1 year ago

What this PR changes?

This PR adds support for image scanning in the latest version of Kubescape. Requires kubescape/kubescape#1288. TODOs rely on merging and releasing the linked PR.

dwertent commented 10 months ago

@vladklokun Any update regarding this PR?

vladklokun commented 10 months ago

@dwertent last time we spoke about this, we handed this over to @Daniel-GrunbergerCA some time ago to add support for all the new image scanning features, like changed output, views etc

I can pick this back up if needed.

dwertent commented 10 months ago

We will add this to the backlog

Daniel-GrunbergerCA commented 10 months ago

Waiting for oficial release to be merged (and changing the branch to main)

matthyx commented 10 months ago

I talked to @Daniel-GrunbergerCA about it already... we should build the image kubescape-cli here instead of kubescape repo. Also we could then base it on busybox:latest or similar. Also it looks like the github action is rebuilding the image on every run?

vladklokun commented 10 months ago

we should build the image kubescape-cli here instead of kubescape repo

Yeah, that would be ideal. I thought we had a reason to use an image tag in your Quay. The ideal scenario is that the Github Action repo takes mainline Kubescape as a base image, adds Busybox or a working shell and then uses it to run the action.

Also it looks like the github action is rebuilding the image on every run?

Since our Action definition points to a Dockerfile in the repo now, yes, it does rebuild the image on every run. The best practice is to build it only once per release, but that would require us to revisit our build steps here. You can see the Git blame for context.

matthyx commented 10 months ago

Since our Action definition points to a Dockerfile in the repo now, yes, it does rebuild the image on every run. The best practice is to build it only once per release, but that would require us to revisit our build steps here. You can see the Git blame for context.

I want to revisit everything... so either:

we move entrypoint.sh and all needed modifications for the action in kubescape/kubescape and we build kubescape-cli there as part of the release
we build kubescape-cli here from the latest binary published in kubescape/kubescape and as you say we have an orchestration issue

vladklokun commented 10 months ago

@dwertent @matthyx @Daniel-GrunbergerCA

We seem to be tacking a lot of things onto this PR: first it was about supporting image scanning, then we would like it to be released faster to incidentally solve #48, and now we are talking about changing the build flow for our Action image.

In my opinion, we should focus on:

releasing a version with updated Kubescape (essentially, solving for distroless)
making a stable tag so that people will use kubescape/github-action@v1 instead of kubescape/github-action@main.

The first one is working. We can improve the way it’s done. The second can be done manually if we’re strapped for time. But I would really love if we added automated tagging on release label for this repo.

If we get a blank check on time for improving the Github Action, let’s change the build flow for sure. If not, I suggest we create issues for things we mentioned here and put them in our backlog, too.

Assuming we want to solve #48 ASAP, I suggest we do this:

include Busybox when building the Action image here, not anywhere else. Meaning, we build our image based on mainline Kubescape (FROM github.com/kubescape/kubescape:v2.9.1) and then install Busybox ourselves. I consider this essential.
Make a tag for the Github Action.
Make an automated step in our workflow that creates releases for PRs merged with the release label.
Create an issue for building the image only once per tag, not before every run, and put it in the backlog.

How does that sound?

matthyx commented 10 months ago

You cannot use github.com/kubescape/kubescape:v2.9.1 as the base image since it only contains ksserver: https://github.com/kubescape/kubescape/blob/master/build/Dockerfile#L17

vladklokun commented 10 months ago

we move entrypoint.sh and all needed modifications for the action in kubescape/kubescape and we build kubescape-cli there as part of the release

I think Kubescape should not be concerned with how it should be run inside Github Actions or any other specific CI runner. If you are a contributor or an otherwise outside person that wants to run Kubescape in a CI system we don’t currently support, you should not have to make a PR to Kubescape the CLI tool. You should develop in a different repo that concerns only that specific CI system.

we build kubescape-cli here from the latest binary published in kubescape/kubescape and as you say we have an orchestration issue

So I’m in favor of this.

vladklokun commented 10 months ago

You cannot use github.com/kubescape/kubescape:v2.9.1 as the base image since it only contains ksserver: https://github.com/kubescape/kubescape/blob/master/build/Dockerfile#L17

You’re right, my bad. The correct image would be github.com/kubescape/kubescape-cli:v2.9.1 now, correct?

matthyx commented 10 months ago

You’re right, my bad. The correct image would be github.com/kubescape/kubescape-cli:v2.9.1 now, correct?

Yes the one that didn't build because of libgit on Windows... it will be 2.9.2 if we're lucky

github-advanced-security[bot] commented 10 months ago

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

Daniel-GrunbergerCA commented 10 months ago

For now it was decided that we would only update the cli image on this PR. The tagging and building process will be handled later

Daniel-GrunbergerCA commented 10 months ago

@dwertent see results here

vladklokun commented 10 months ago

I noticed the results say:

Detected by Grype

That’s probably because we use the SARIF presenter as it is, without correcting for the tool that was ran.

Daniel-GrunbergerCA commented 10 months ago

ready for review

vladklokun commented 10 months ago

Fair enough.

On Thu, 19 Oct 2023 at 16:01, David Wertenteil @.***> wrote:

@.**** commented on this pull request.

In .github/workflows/example-scan-image.yaml https://github.com/kubescape/github-action/pull/46#discussion_r1365493082 :

runs-on: ubuntu-latest

permissions:

actions: read

contents: read

security-events: write

steps:

uses: @.***

uses: @.***

continue-on-error: true

with:

image: quay.io/kubescape/kubescape

format: sarif

outputFile: results.sarif

severityThreshold: "critical"

Username for a private registry with the image

registryUsername: ${{secrets.KUBESCAPE_REGISTRY_USERNAME}}

You are mentioning here best practice, but the idea of the readme is to explain to the user what he needs to do. Now, since I was confused when I saw the readme, and I thought that the secret refers to kubescape credentials, that means that the readme is unclear. We should focus on making the readme clear and not educating users with best practices.

— Reply to this email directly, view it on GitHub https://github.com/kubescape/github-action/pull/46#discussion_r1365493082, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFUMGFGW34VPKNUU4FWEBA3YAEQEPAVCNFSM6AAAAAA2QPJH5CVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMOBXGY4DENBSG4 . You are receiving this because you were mentioned.Message ID: @.***>

kubescape / github-action

feat: support image scanning #46

What this PR changes?

@.**** commented on this pull request.

severityThreshold: "critical"

Username for a private registry with the image

registryUsername: ${{secrets.KUBESCAPE_REGISTRY_USERNAME}}