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: add kubescape patch command #1332

Merged
merged 6 commits into from
Sep 4, 2023

Conversation

anubhav06
Copy link
Contributor

@anubhav06 anubhav06 commented Aug 4, 2023

Overview

Add kubescape patch command

Additional Information

  1. The kubescape patch command can be used to patch container images with vulnerabilities.
  2. It uses copa and buildkit under the hood for patching the images, and grype as its engine for scanning the images (at the moment)
  3. The detailed documentation for this command can be found here.

TODO

  1. Replace anubhav06/copacetic with project-copacetic/copacetic, when the copa team accepts the kubescape and copa integration support PR.

Usage

sudo buildkitd & 
sudo kubescape patch --image <image-name>

The patch command can also be run without sudo privileges. Refer to the documentation here.

Examples/Screenshots

  1. Run sudo buildkitd in a separate terminal

0

  1. Run sudo kubescape patch --image docker.io/library/nginx:1.22:

image
image

cc @craigbox @dwertent

Related issues/PRs:

Resolved #1227

Checklist before requesting a review

put an [x] in the box to get it checked

  • My code follows the style guidelines of this project
  • I have commented on my code, particularly in hard-to-understand areas
  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests.
  • New and existing unit tests pass locally with my changes

Signed-off-by: Anubhav Gupta <mail.anubhav06@gmail.com>
@ghost
Copy link

ghost commented Aug 4, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@craigbox
Copy link
Contributor

craigbox commented Aug 8, 2023

At the present, there's no assertion of levels of support in Kubescape. Everything just "is", in the most part.
I have proposed that we introduce feature gates (with the same syntax as the Kubernetes API server) in order to be able to only include Alpha or Labs-level features if someone enables a feature gate.

The alternative would be keeping it in a separate branch, which would require keeping it in sync, and making builds from it if we wanted anyone to actually use it.

/cc @rotemamsa

@anubhav06
Copy link
Contributor Author

The former sounds better to me. So this will be merged later, when the feature gates have been introduced, right?

@dwertent
Copy link
Contributor

dwertent commented Aug 9, 2023

@anubhav06 Please pull from the master branch and run the kubescape scan image command.
This might help you to understand how Kubescape scans images (and there is no reason to develop another "image scanning" mechanism in Kubescape).
Also, you will see the CLI output, it is essential we keep it aligned.

What do you think?

image

@anubhav06
Copy link
Contributor Author

@dwertent I've updated the output to align with that of image scan command (screenshot in PR desc).
I've also removed the "image scanning" logic I'd added for results handling.
Please let me know if this is what's expected and if any other changes are required.

@Daniel-GrunbergerCA
Copy link
Collaborator

Daniel-GrunbergerCA commented Aug 29, 2023

@anubhav06 This feature looks really cool and works smoothly! I'm very excited for this!

I have some general comments about the feature, which are mostly food for thought:

  1. We should add the image name to the kubescape patch follow-up step (which is printed at the end)
  2. Maybe we should add to the output some comparison between the old image and the new one, so it will become clear to the user the value he is getting from the patch command
  3. Do we need to print the entire copa output? I think is best to have it in debug mode only
  4. We shouldn't use the -r flag. We have the -f flag, where we can specify JSON output
  5. Maybe the commands should be kubescape image scan and kubescape image patch

cc: @craigbox @dwertent

core/core/patch.go Outdated Show resolved Hide resolved
cmd/patch/patch.go Outdated Show resolved Hide resolved
core/pkg/resultshandling/printer/v2/jsonprinter.go Outdated Show resolved Hide resolved
@anubhav06
Copy link
Contributor Author

anubhav06 commented Aug 29, 2023

@Daniel-GrunbergerCA Thanks! To answer your questions:

  1. Even I agree with that. I had done that only previously (screenshot here). But since, David said that the output should align with that of the image scan command, I removed that.
  2. Alright. So I'll try to remove the output of copa to STDOUT and display it only when running with debug mode, and see how it looks. (Personally I believe that it looks good right now, the way it is, because by default the user can see what's happening: copa is building the image, installing the pkg and exporting to docker). But yes, I'll try to do it the way you said and let's see how it looks.

@Daniel-GrunbergerCA
Copy link
Collaborator

You can wait with that, let's see what @craigbox and @dwertent think

@dwertent
Copy link
Contributor

  1. @Daniel-GrunbergerCA I think it is a good suggestion, but still, I believe we should print only the latest state so we don't overwhelm the terminal. Also, this way we can have the same function for printing the image scan command and image patch command.
    As I'm writing this reply, I'm thinking we can print just the summary of the old image, but we can save this to the future.
  2. Thenk you @anubhav06 . The reason Daniel asked to "hide" the logs is because it does not look good when using two different loggers when running a single command. I suggest we should use the spinner (form the kubescape logger repo) instead of printing the copa logs. I'm aware that we are less "verbose", but for now I think it's better :)

In general, thank you @anubhav06 for this PR!
Please merge the changes that have been pushed to the main so we will be able to properly test your PR before accepting it :)

Signed-off-by: Anubhav Gupta <mail.anubhav06@gmail.com>
@anubhav06
Copy link
Contributor Author

anubhav06 commented Sep 1, 2023

changes: (updated screenshot in PR desc)

  • By default, copa's output (logs) will not be shown.
    If a user wants to see copa's logs, run kubescape in debug mode ( --logger='debug')
  • removed the --report flag as it was redundant & was not providing any real value
  • moved normalize_image_name from opaprocessor pkg to cautils pkg
  • removed the "try patching your image with..." text (for now)

@@ -106,6 +106,6 @@ var imageNameNormalizeDefinition = func(bctx rego.BuiltinContext, a *ast.Term) (
if err != nil {
return nil, fmt.Errorf("invalid parameter type: %v", err)
}
normalizedName, err := normalize_image_name(string(aStr))
normalizedName, err := cautils.Normalize_image_name(string(aStr))
Copy link
Contributor

Choose a reason for hiding this comment

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

@YiscahLevySilas1 can this affect the controls that are using this function?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it will affect them

@@ -116,12 +116,7 @@ func (sp *SARIFPrinter) printImageScan(scanResults *models.PresenterConfig) erro
return fmt.Errorf("no no image vulnerability data provided")
}

presenterConfig, err := presenter.ValidatedConfig(printer.SARIFFormat, "", false)
Copy link
Contributor

Choose a reason for hiding this comment

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

was this accidentally removed?
cc @Daniel-GrunbergerCA

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. Because with the new version of grype, the previous syntax is not supported.
Similarly, in the jsonprinter, the ValidatedConfig method has been removed (here)

cmd/patch/README.md Outdated Show resolved Hide resolved
cmd/patch/README.md Outdated Show resolved Hide resolved
core/cautils/normalize_image_name.go Outdated Show resolved Hide resolved
core/core/patch.go Outdated Show resolved Hide resolved
core/core/patch.go Outdated Show resolved Hide resolved
@dwertent dwertent changed the base branch from master to refactor-img-cmd September 4, 2023 06:38
Signed-off-by: Anubhav Gupta <mail.anubhav06@gmail.com>
@Daniel-GrunbergerCA Daniel-GrunbergerCA merged commit e091269 into kubescape:refactor-img-cmd Sep 4, 2023
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.

LFX proposal: Vulnerability-based Dockerfile generator
5 participants