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(image): Add image-src flag to specify which runtime(s) to use #4047

Merged
merged 40 commits into from
May 15, 2023

Conversation

pmengelbert
Copy link
Contributor

@pmengelbert pmengelbert commented Apr 12, 2023

Description

Enable a new trivy image --image-src flag. For example

before:

trivy image alpine:3.7.3 # tries docker, containerd, podman, remote in that order (hardcoded)

after:

trivy image alpine:3.7.3 # behavior is the same as in the *before* example
trivy image --image-src=podman,containerd alpine:3.7.3 # tries only podman, containerd in the specified order

Related issues

Related PRs

Remove this section if you don't have related PRs.

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works. (there were existing tests that hit this codepath, but no new tests written)
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

=======

Description

Related issues

@knqyf263 This is what I got done today on the socket selection. Feel free to use this as a starting point for your future work to include this feature in 0.41.0

Opening as a draft PR since I have not written tests or docs for this yet.

@knqyf263
Copy link
Collaborator

@DmitriyLewen I think we may want to replace your PR with this approach. Can you take a look?

Also, I suggested as below.
#3049 (comment)

Copy link
Contributor

@DmitriyLewen DmitriyLewen left a comment

Choose a reason for hiding this comment

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

@pmengelbert Thanks for your work!

Looks good. I added some comments. Can you take a look?

pkg/fanal/image/image.go Outdated Show resolved Hide resolved
pkg/fanal/image/image.go Outdated Show resolved Hide resolved
@pmengelbert pmengelbert changed the title Add runtime flag to specify which runtime(s) to use feat: Add runtime flag to specify which runtime(s) to use Apr 18, 2023
@pmengelbert pmengelbert changed the title feat: Add runtime flag to specify which runtime(s) to use feat(image): Add runtime flag to specify which runtime(s) to use Apr 18, 2023
Copy link
Contributor

@DmitriyLewen DmitriyLewen left a comment

Choose a reason for hiding this comment

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

LGTM
I added some comments.

Take a look, please.

Also i think we need add some information about this in docs - https://github.com/aquasecurity/trivy/blob/main/docs/docs/target/container_image.md#supported
i mean default value, priority, etc...

docs/docs/references/configuration/cli/trivy_filesystem.md Outdated Show resolved Hide resolved
docs/docs/references/configuration/cli/trivy_image.md Outdated Show resolved Hide resolved
pkg/fanal/image/image.go Outdated Show resolved Hide resolved
pkg/fanal/test/integration/registry_test.go Outdated Show resolved Hide resolved
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Also, update docs to reflect the change.

Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
@pmengelbert
Copy link
Contributor Author

pmengelbert commented Apr 20, 2023

LGTM I added some comments.

Take a look, please.

Also i think we need add some information about this in docs - https://github.com/aquasecurity/trivy/blob/main/docs/docs/target/container_image.md#supported i mean default value, priority, etc...

I added some docs in container_image.md:

diff --git a/docs/docs/target/container_image.md b/docs/docs/target/container_image.md
index 7350a6a93..108c667a9 100644
--- a/docs/docs/target/container_image.md
+++ b/docs/docs/target/container_image.md
@@ -224,6 +224,23 @@ GitHub Personal Access Token
     You can see environment variables with `docker inspect`.
 
 ## Supported
+
+Trivy will look for the specified image in a series of locations. By default, it
+will first look in the local Docker Engine, then Containerd, Podman, and
+finally container registry.
+
+This behavior can be modified with the `--runtimes` flag. For example, the
+command
+
+```bash
+trivy image --runtimes podman,containerd alpine:3.7.3
+```
+
+Will first search in Podman. If the image is found there, it will be scanned
+and the results returned. If the image is not found in Podman, then Trivy will
+search in Containerd. If the image is not found there either, the scan will
+fail and no more runtimes will be searched.
+
 ### Docker Engine
 Trivy tries to looks for the specified image in your local Docker Engine.
 It will be skipped if Docker Engine is not running locally.

Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
@DmitriyLewen
Copy link
Contributor

Good work! Can you resolve conflicts?

Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
type Runtime string

// Runtimes is a slice of runtimes
type Runtimes []Runtime
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be defined under ImageOptions.

type ImageOptions struct {
RegistryOptions RegistryOptions
DockerOptions DockerOptions
PodmanOptions PodmanOptions
ContainerdOptions ContainerdOptions
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

type Runtime string

// Runtimes is a slice of runtimes
type Runtimes []Runtime
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be defined under ImageOptions.

type ImageOptions struct {
RegistryOptions RegistryOptions
DockerOptions DockerOptions
PodmanOptions PodmanOptions
ContainerdOptions ContainerdOptions
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

opts.podman = false
}
}
func WithRuntimes(runtimes ftypes.Runtimes) ([]RuntimeFunc, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it can be done in NewContainerImage. Is there any benefit in exporting this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved this to NewContainerImage, and made the function private. The function was also renamed

Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
@pmengelbert
Copy link
Contributor Author

--runtimes strings Runtime(s) to use, in priority order (docker,containerd,podman,remote) (default [docker,containerd,podman,remote])

remote runtime sounds like a remote Docker daemon or other daemons. How about --image-src, --image-from, or something like that? docker,containerd,podman,registry.

Renamed to image-src, and in the code things like Runtime have been renamed to ImageSource

@pmengelbert pmengelbert changed the title feat(image): Add runtime flag to specify which runtime(s) to use feat(image): Add image-src flag to specify which runtime(s) to use Apr 25, 2023
@pmengelbert
Copy link
Contributor Author

@knqyf263 Is there anything else that this PR needs to be accepted?

Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
@knqyf263
Copy link
Collaborator

knqyf263 commented May 4, 2023

@pmengelbert I'm back from vacation. We plan to take another look at this PR next week. Thanks for your patience.

Copy link
Collaborator

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

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

I've added some tweaks, such as comments, formatting, etc., but it looks great to me! Thanks for your contribution.

@knqyf263 knqyf263 merged commit 6a0e152 into aquasecurity:main May 15, 2023
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.

Force remote image registry scanning Select scan socket (containerd, podman, docker)
4 participants