Added a warning when using a non-Toolbx container#1707
Conversation
|
Build failed. ✔️ unit-test SUCCESS in 2m 12s |
|
recheck |
|
Build succeeded. ✔️ unit-test SUCCESS in 2m 10s |
f3f59b3 to
c9215d4
Compare
|
I have updated this PR by adding detection of more aspects in the base images used for Toolbx containers, which users should know about. To the already existing Toolbx images detection (according to its labels 1. Presence of environmental variable 2. Already set image |
|
Build succeeded. ✔️ unit-test SUCCESS in 2m 12s |
debarshiray
left a comment
There was a problem hiding this comment.
Thanks for working on this, @DaliborKr ! It's really good that you added so many tests to ensure that things keep working as expected.
After seeing your code changes, I realized that I might have underestimated the complications hidden in this effort. I will try to explain inline.
In reaction to the conversation in PR containers#1707, I made suggested changes to the implementation in terms of how Image information, received from Podman, is represented and treated in the Toolbx code. I used the same procedure as in the case of representing Containers: - Commit e611969 - Commit ec7eb59 In short, the JSON from 'podman inspect --type image' and 'podman images' are different, so logically, there should be different implementations of the JSON.Unmarshaler interface [1] for them as well. The new Image interface provides access to the values parsed from the JSONs and two different concrete types, which are implemented separately to handle the differences in the JSONs. GetImages() now returns an Images iterator-like structure that handles image flattening and sorting internally. InspectImage() returns an Image interface instead of map[string]interface{}, and the IsToolboxImage() function was replaced with the IsToolbx() method on the Image interface. [1] https://pkg.go.dev/encoding/json#Unmarshaler Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
In reaction to the conversation in PR containers#1707, I made suggested changes to the implementation in terms of how Image information, received from Podman, is represented and treated in the Toolbx code. I used the same procedure as in the case of representing Containers: - Commit e611969 - Commit ec7eb59 In short, the JSON from 'podman inspect --type image' and 'podman images' are different, so logically, there should be different implementations of the JSON.Unmarshaler interface [1] for them as well. The new Image interface provides access to the values parsed from the JSONs and two different concrete types, which are implemented separately to handle the differences in the JSONs. GetImages() now returns an Images iterator-like structure that handles image flattening and sorting internally. InspectImage() returns an Image interface instead of map[string]interface{}, and the IsToolboxImage() function was replaced with the IsToolbx() method on the Image interface. [1] https://pkg.go.dev/encoding/json#Unmarshaler Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
In reaction to the conversation in PR containers#1707, I made suggested changes to the implementation in terms of how Image information, received from Podman, is represented and treated in the Toolbx code. I used the same procedure as in the case of representing Containers: - Commit e611969 - Commit ec7eb59 In short, the JSON from 'podman inspect --type image' and 'podman images' are different, so logically, there should be different implementations of the JSON.Unmarshaler interface [1] for them as well. The new Image interface provides access to the values parsed from the JSONs and two different concrete types, which are implemented separately to handle the differences in the JSONs. GetImages() now returns an Images iterator-like structure that handles image flattening and sorting internally. InspectImage() returns an Image interface instead of map[string]interface{}, and the IsToolboxImage() function was replaced with the IsToolbx() method on the Image interface. Tested were results of Podman starting on version 1.1.2, which should be sufficient since the minimum required Podman version is 1.6.4 (see commit 8e80dd5). Covered are structures representing podman commands 'podman inspect --type image' and 'podman images'. [1] https://pkg.go.dev/encoding/json#Unmarshaler containers#1724 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
In reaction to the conversation in PR containers#1707, I made suggested changes to the implementation in terms of how Image information, received from Podman, is represented and treated in the Toolbx code. I used the same procedure as in the case of representing Containers: - Commit e611969 - Commit ec7eb59 In short, the JSON from 'podman inspect --type image' and 'podman images' are different, so logically, there should be different implementations of the JSON.Unmarshaler interface [1] for them as well. The new Image interface provides access to the values parsed from the JSONs and two different concrete types, which are implemented separately to handle the differences in the JSONs. GetImages() now returns an Images iterator-like structure that handles image flattening and sorting internally. InspectImage() returns an Image interface instead of map[string]interface{}, and the IsToolboxImage() function was replaced with the IsToolbx() method on the Image interface. Tested were results of Podman starting on version 1.1.2, which should be sufficient since the minimum required Podman version is 1.6.4 (see commit 8e80dd5). Covered are structures representing podman commands 'podman inspect --type image' and 'podman images'. [1] https://pkg.go.dev/encoding/json#Unmarshaler containers#1724 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
In reaction to the conversation in PR containers#1707, I made suggested changes to the implementation in terms of how Image information, received from Podman, is represented and treated in the Toolbx code. I used the same procedure as in the case of representing Containers: - Commit e611969 - Commit ec7eb59 In short, the JSON from 'podman inspect --type image' and 'podman images' are different, so logically, there should be different implementations of the JSON.Unmarshaler interface [1] for them as well. The new Image interface provides access to the values parsed from the JSONs and two different concrete types, which are implemented separately to handle the differences in the JSONs. GetImages() now returns an Images iterator-like structure that handles image flattening and sorting internally. InspectImage() returns an Image interface instead of map[string]interface{}, and the IsToolboxImage() function was replaced with the IsToolbx() method on the Image interface. Tested were results of Podman starting on version 1.1.2, which should be sufficient since the minimum required Podman version is 1.6.4 (see commit 8e80dd5). Covered are structures representing podman commands 'podman inspect --type image' and 'podman images'. [1] https://pkg.go.dev/encoding/json#Unmarshaler containers#1724 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
In reaction to the conversation in PR containers#1707, I made suggested changes to the implementation in terms of how Image information, received from Podman, is represented and treated in the Toolbx code. I used the same procedure as in the case of representing Containers: - Commit e611969 - Commit ec7eb59 In short, the JSON from 'podman inspect --type image' and 'podman images' are different, so logically, there should be different implementations of the JSON.Unmarshaler interface [1] for them as well. The new Image interface provides access to the values parsed from the JSONs and two different concrete types, which are implemented separately to handle the differences in the JSONs. GetImages() now returns an Images iterator-like structure that handles image flattening and sorting internally. InspectImage() returns an Image interface instead of map[string]interface{}, and the IsToolboxImage() function was replaced with the IsToolbx() method on the Image interface. Tested were results of Podman starting on version 1.1.2, which should be sufficient since the minimum required Podman version is 1.6.4 (see commit 8e80dd5). Covered are structures representing podman commands 'podman inspect --type image' and 'podman images'. [1] https://pkg.go.dev/encoding/json#Unmarshaler containers#1724 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
debarshiray
left a comment
There was a problem hiding this comment.
Maybe it's time to rebase/rewrite this on top of #1724 , and the Image interface that's now in main?
Add compatibility checks for images used to create Toolbx containers,
as requested in [1]. When any requirement is not met, warnings are
printed and the user is prompted for confirmation. The --assumeyes flag
bypasses the prompt.
Currently, three requirements are verified:
- the image has Toolbx labels (`com.github.containers.toolbox=true` or
`com.github.debarshiray.toolbox=true``)
- doesn't have the LD_PRELOAD environment variable set (it may cause
vulnerability, see [2])
- doesn't have a pre-defined entrypoint that would conflict with
Toolbx's own.
The implementation extends the imageInspect type introduced in [3] and
[4] with entrypoint and env fields, and adds a CheckImageCompatibility()
function in the podman package. Additional image requirement checks can
be simply added to this function.
[1] containers#1622
[2] https://www.wiz.io/blog/nvidia-ai-vulnerability-cve-2025-23266-nvidiascape
[3] containers#1724
[4] containers#1779
containers#1707
Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
Add system tests for the image Toolbx compatibility checks introduced in the commit [1]. Each of the three checks (missing Toolbx labels, LD_PRELOAD environment variable set, pre-defined entrypoint) is tested individually and combined, covering three scenarios: user accepts the prompt, user declines the prompt, and the --assumeyes flag. Helper functions for building test images and generating expected warning messages are added to libs/helpers.bash. [1] Commit 86d5e7c containers#1707 containers#1707 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
|
The requested changes have been applied in a new approach based on the The compatibility check is currently only performed in |
…x requirements
Add compatibility checks for images used to create Toolbx containers,
as requested in [1]. When any requirement is not met, warnings are
printed and the user is prompted for confirmation. The --assumeyes flag
bypasses the prompt.
Currently, three requirements are verified:
- the image has Toolbx labels (`com.github.containers.toolbox=true` or
`com.github.debarshiray.toolbox=true`)
- doesn't have the LD_PRELOAD environment variable set (it may cause
vulnerability, see [2])
- doesn't have a pre-defined entrypoint that would conflict with
Toolbx's own.
The implementation extends the imageInspect type introduced in [3] and
[4] with entrypoint and env fields, and adds a CheckImageCompatibility()
function in the podman package. Additional image requirement checks can
be simply added to this function.
The warning display and optional prompting logic is extracted into a
checkAndWarnImageCompatibility() helper in cmd/utils.go, so it can be
reused across commands.
When a container is created from an image that doesn't meet the
requirements, it is labeled with
`com.github.containers.toolbox.image-unknown=true`. An IsImageUnknown()
method is added to the Container interface to check for this label.
This allows subsequent commands to identify containers created from
incompatible images.
[1] containers#1622
[2] https://www.wiz.io/blog/nvidia-ai-vulnerability-cve-2025-23266-nvidiascape
[3] containers#1724
[4] containers#1779
containers#1707
Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
Show image compatibility warnings when running or entering a container that was created from an image that doesn't meet Toolbx requirements. The enter command calls runCommand() internally, so it is covered transitively. Two cases are handled differently: 1. Containers not created via Toolbx (missing the `com.github.containers.toolbox=true` label) are prompted for confirmation, because the user may not be aware of the requirements that Toolbx has on its container's base images. 2. Containers created via Toolbx that were labeled with `com.github.containers.toolbox.image-unknown=true` (the new label introduced in previous commit [1]) during creation only show the warnings without prompting, because the user already accepted the incompatibility when creating the container. When a container is created by Toolbox and its base image is compatible, the checks are skipped completely to avoid unnecessary podman inspect calls. This prevents significant overhead during the run and enter commands, especially since using incompatible images is a rather rare exception. [1] Commit 2c4cb5c containers#1707 containers#1707 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
Add system tests for the image Toolbx compatibility checks introduced in the commits [1] and [2]. Each of the three checks (missing Toolbx labels, LD_PRELOAD environment variable set, pre-defined entrypoint) is tested individually and combined, covering three scenarios during creation phase: user accepts the prompt, user declines the prompt, and the --assumeyes flag. Helper functions for building test images and generating expected warning messages are added to libs/helpers.bash. [1] Commit 2c4cb5c containers#1707 [2] Commit 2ccbf24 containers#1707 containers#1707 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
8d9ef89 to
7eaeb14
Compare
|
Added the image compatibility check for |
Show containers created from images that don't meet Toolbx requirements in yellow in the 'toolbox list' output. This gives users a visual hint that the container may not work as expected. The yellow color is only applied when the container is stopped. Running containers are shown in green as usual, following the coloring approach introduced in [1]. Containers with incompatible images are identified by the `com.github.containers.toolbox.image-unknown=true` label, which is set during container creation when the image fails the compatibility check. [1] containers#494 containers#1707 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
debarshiray
left a comment
There was a problem hiding this comment.
Thanks for resurrecting this pull request so quickly, @DaliborKr !
Here are some high level comments after a quick look. I know you are busy and don't have too much time, so no pressure.
| } | ||
|
|
||
| if hasWarnings { | ||
| imageUnknownLabel = []string{"--label", "com.github.containers.toolbox.image-unknown=true"} |
There was a problem hiding this comment.
Let's use com.github.containers.toolbx.image-unknown=true (ie., without the o). That way we won't have to do a migration in the future, which is always painful. :)
|
|
||
| var imageUnknownLabel []string | ||
|
|
||
| hasWarnings, shouldContinue := checkAndWarnImageCompatibility(imageFull, true) |
There was a problem hiding this comment.
It will be good if we could somehow re-use the Image object from the earlier InspectImage() call in GetFullyQualifiedImageFromRepoTags().
It's not a huge problem - just that one of the big advantages of your work on the Image interface is that we can carry the object around without repeatedly calling InspectImage(). So, it will be good to take advantage of it.
I haven't fully tested this, so I could be wrong. I am wondering if something like this will work, where we move some code out of the existing GetFullyQualifiedImageFromRepoTags() function:
imageObj, err := InspectImage(image)
if err != nil {
return "", fmt.Errorf("failed to inspect image %s", image)
}
var imageFull string
if utils.ImageReferenceHasDomain(image) {
imageFull = image
} else {
imageFull, err := podman.GetFullyQualifiedImageFromRepoTags(imageObj)
}
var imageUnknownLabel []string
hasWarnings, shouldContinue := checkAndWarnImageCompatibility(imageObj, true)
if !shouldContinue {
return nil
}
| } | ||
|
|
||
| image.entrypoint = raw.Config.Entrypoint | ||
| image.env = raw.Config.Env |
There was a problem hiding this comment.
We should restore the Go unit tests for entry point and environment variables that you had originally written for #1724
I have a backup of the older version of your code. So, if you no longer have them, I can help with the restoration. :)
As mentioned in #1622, not all images are guaranteed to work with Toolbx. Therefore, every Toolbx verified image contains specific labels (
com.github.containers.toolbox="true"orcom.github.debarshiray.toolbox="true"), which means that the image was previously tested with Toolbx.In this PR, the detection of usage of non-Toolbx images has been added to the
create,run, andentercommands. If such an image is detected, the user is prompted with a message like:Continuing may lead to an error while opening the container.