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 |
Prompt users if they want to continue creating or running a Toolbox container with an image that is not Toolbox-verified. Verified images are guaranteed to work with Toolbx because they were previously tested. Such an image contains at least one of these labels (see https://containertoolbx.org/doc/): - com.github.containers.toolbox="true" - com.github.debarshiray.toolbox="true" containers#1622
The original version of the IsToolboxImage() function in pkg/podman/podman.go returned an error when an image was not compatible with Toolbx. The new version returns 'false' without error on a non-Toolbx container, so it can be distinguished when the image inspection actually fails. This new behavior is used in detecting such images when creating or running Toolbx containers: f06a819
Test cases for non-Toolbx image usage warning added in the commit f06a819
Add unified image validation function that checks multiple compatibility requirements including Toolbx labels, LD_PRELOAD environment variable, and image entrypoint. Returns boolean compatibility status along with detailed warning messages for any detected issues. Signed-off-by: Dalibor Kricka <[email protected]>
Display compatibility warnings for images that don't meet Toolbx requirements. Only prompt for confirmation when assumeyes is not set. Update both create and run commands to use unified DoesImageFulfillRequirements() function inctroduced in commit e7bef55. Signed-off-by: Dalibor Kricka <[email protected]>
Add test coverage for image compatibility warnings displayed during container creation and execution with various image types. dfaed70 Signed-off-by: Dalibor Kricka <[email protected]>
DaliborKr
force-pushed
the
not_compactible_image_detection
branch
from
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 |
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.
| func DoesImageFulfillRequirements(image string) (bool, string, error) { | ||
| var warnings []string | ||
|
|
||
| isToolboxImage, err := IsToolboxImage(image) |
There was a problem hiding this comment.
We are invoking podman inspect --type image several times while doing the checks. First, to check the labels; second, to check LD_PRELOAD; and third, to check the EntryPoint. Generally, each invocation of podman(1) is sufficiently expensive, because it does a lot of heavy initialization when it starts. So, it's good to avoid them, if possible. This is especially true for Toolbx commands like enter and run that are used very often, and a delay of one extra second can start to annoy the user over time.
For example, here are some past commits where we optimized enter and run by reducing the number of podman(1) invocations:
- commit 74d4fcf00c6ec3d1 removed an
podman startandpodman inspect - commit4536e2c8c28f6c4f removed two
podman execcalls
The create command is not that frequently used and already does enough disk and network I/O that people expect it to be slow, but we also shouldn't make it slower than it needs to be. :)
We can reduce the number of podman inspect invocations if we parse the JSON into an Image object and then keep using the object repeatedly.
| } | ||
| default: | ||
| return false, fmt.Errorf("unexpected type '%T' of environment variables in image %s", env, image) | ||
| } |
There was a problem hiding this comment.
Decoding the JSON in this way can be fragile if the structure of the JSON changes, and over time Podman has sometimes done that. :(
See the comments in src/pkg/podman/container.go, and the UnmarshalJSON() method of the Image object.
Fortunately, Go can decode JSON on a best effort basis, which can cover-up many of the superficial changes and needs a lot less code. Here's an introductory article about how it works. Sometimes, Go can't automatically handle the changes and we need to handle it ourselves, like in the above comments.
To take advantage of this, and to avoid invoking podman inspect repeatedly, we can decode the JSON into an Image object that implements the json.Unmarshaler interface. It's implemented by the containerInspect, containerPS and Image objects above. The interface will limit all manual interventions during decoding in one place.
Unfortunately, the JSON used by Podman to represent images differs between podman images and podman inspect --type image. The Image object only handles the former, but not the latter.
We have the same problem for containers. That's why we have a Container interface to hide the details, and it's implemented by the private containerInspect and containerPS objects that actually decode the JSON.
So, our first step would be to have a commit that introduces an Image interface, and renames the existing Image object to make it private and makes it implement the interface. I don't know what would be the best name for the object, because imageImages looks awkward, but it's also a private type so it won't be seen in too many places.
Then, we have to add a second implementation of the Image interface that decodes the JSON from podman inspect --type image.
Here are some commits that did the same thing for containers that you can use for help:
- commit e611969726ed4260
cmd, pkg/podman: Turn Container into an interface - commit ec7eb59bb058c12d
cmd/run, pkg/podman: Make podman.InspectContainer() return a Container
Then, the IsToolboxImage() function can turn into the IsToolbx() method of the Image interface:
- commit d363eb26d10cd93c
cmd, pkg/podman: Turn IsToolboxContainer() into Container.IsToolbx()
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 <[email protected]>
2 participants
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.