-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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): customer podman host or socket option #6256
feat(image): customer podman host or socket option #6256
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @parvez0
Thank you for your work!
Left a comment. Please take a look when you have time.
Can you also add a test for podman-host?
You can create a similar test:
trivy/pkg/fanal/image/daemon/image_test.go
Lines 74 to 116 in c1d26ec
func Test_image_ConfigNameWithCustomDockerHost(t *testing.T) { | |
ref, err := name.ParseReference("alpine:3.11") | |
require.NoError(t, err) | |
eo := engine.Option{ | |
APIVersion: opt.APIVersion, | |
ImagePaths: opt.ImagePaths, | |
} | |
var dockerHostParam string | |
if runtime.GOOS != "windows" { | |
runtimeDir, err := os.MkdirTemp("", "daemon") | |
require.NoError(t, err) | |
dir := filepath.Join(runtimeDir, "image") | |
err = os.MkdirAll(dir, os.ModePerm) | |
require.NoError(t, err) | |
customDockerHost := filepath.Join(dir, "image-test-unix-socket.sock") | |
eo.UnixDomainSocket = customDockerHost | |
dockerHostParam = "unix://" + customDockerHost | |
} | |
te := engine.NewDockerEngine(eo) | |
defer te.Close() | |
if runtime.GOOS == "windows" { | |
dockerHostParam = te.Listener.Addr().Network() + "://" + te.Listener.Addr().String() | |
} | |
img, cleanup, err := DockerImage(ref, dockerHostParam) | |
require.NoError(t, err) | |
defer cleanup() | |
conf, err := img.ConfigName() | |
assert.Equal(t, v1.Hash{ | |
Algorithm: "sha256", | |
Hex: "a187dde48cd289ac374ad8539930628314bc581a481cdb41409c9289419ddb72", | |
}, conf) | |
assert.Nil(t, err) | |
} |
Regards, Dmitriy
Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pkg/fanal/image/daemon/podman.go
Outdated
// Get Podman socket location | ||
sockDir := os.Getenv("XDG_RUNTIME_DIR") | ||
socket := filepath.Join(sockDir, "podman", "podman.sock") | ||
if host != "" { | ||
socket = host | ||
} else if s := os.Getenv("PODMAN_HOST"); s != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DOCKER_HOST
is natively supported by Docker.
https://docs.docker.com/engine/reference/commandline/cli/
But I didn't find PODMAN_HOST support in Podman. If Podman doesn't support it, there is no reason we do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @knqyf263 thanks for the review, I'll remove PODMAN_HOST and reraise the PR
LGTM! Thanks. |
Sorry, I accidentally closed it... never mind. |
Description
This pull request adds a new feature option
--podman-host
to the image command. This option allows users to specify a custom Podman host.command output before introducing this feature
command output after introducing the feature
Related issues
Related PRs
Remove this section if you don't have related PRs.
Checklist