cmd, pkg/podman: Introduce Image Interface and Unit Tests by DaliborKr · Pull Request #1724 · containers/toolbox

DaliborKr · 2025-10-07T15:44:03Z

Introduces an Image interface based on the conversation in PR #1707.

Similar to how container information is handled (introduced in commits e611969 and ec7eb59), this PR abstracts the representation of an image. This is necessary because podman inspect --type image and podman images return different JSON structures. The Image interface provides a unified way to access image data, with two distinct implementations to handle the different JSON formats.

Updates related functions:

GetImages() now returns an Images iterator, which handles image flattening and sorting internally.
InspectImage() now returns an Image interface instead of a generic map[string]interface{}.
The IsToolboxImage() function has been replaced with an IsToolbx() method on the Image interface.

Adds unit tests: This PR includes unit tests for the new Image interface implementation, covering the output of both podman inspect --type image and podman images.

softwarefactory-project-zuul · 2025-10-07T17:02:32Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/5c0d0f34d9124050a0549fb6437fc0c8

debarshiray

Thanks for working on this, @DaliborKr ! My apologies for the delay.

I am still reading through the changes, but one thing popped out at me.

src/pkg/podman/image.go

src/pkg/podman/podman.go

src/pkg/podman/image.go

debarshiray · 2026-03-10T00:25:30Z

src/pkg/podman/image.go

+}
+
+func (images Images) Len() int {
+	return len(images.data)


It might be better to change these pre-existing methods from ImageSlice that earlier had value receivers to pointer receivers.

I am not an expert on value versus pointer receivers, and I get confused all the time. It seems that the general advice is to not mix pointer and value receivers for a given type.

In the case of the Images type, the Reset() method must have a pointer receiver because it modifies a field of the type.

In case you wonder how the Swap() method was working, just like I did, it's because in Go slices are references to an underlying array and changing an element of a slice modifies the array and other slices see the same change. The value receiver copies the slice and the underlying array stays the same.

src/cmd/list.go

This is a step towards eliminating or reducing the code in getImages() by doing everything or more in podman.GetImages(). If nothing else, this removes the need to export Image.FlattenNames() from the podman package. The getImages() function originated as listContainers(), when it invoked 'podman images --filter label=...' through podman.GetImages() separately for each label used to identify Toolbx images, and then joined and sorted the results. This changed in commit 2369da5 when getImages() started invoking 'podman images' only once through podman.GetImages() to get all available images, and then filtered them itself based on the labels. Before this change in commit 2369da5, it probably made some sense to keep podman.GetImages() only as a thin wrapper to invoke 'podman images' with different options, and to do everything else in getImages(). However, since then more is being done inside the podman package (eg., unmarshalling the 'podman images' JSON in commit 5f324d5 and flattening the images in commit 6aab0a6), and getImages() is the only caller of podman.GetImages(). Therefore, it looks awkward to have the code to get all Toolbx images split across two functions in different packages. containers#1724

This is a step towards eliminating or reducing the code in getImages() by doing everything or more in podman.GetImages(). If nothing else, this removes the need to export Image.FlattenNames() from the podman package. The getImages() function originated as listContainers(), when it invoked 'podman images --filter label=...' through podman.GetImages() separately for each label used to identify Toolbx images, and then joined and sorted the results. This changed in commit 2369da5 when getImages() started invoking 'podman images' only once through podman.GetImages() to get all available images, and then filtered them itself based on the labels. Before this change in commit 2369da5, it probably made some sense to keep podman.GetImages() only as a thin wrapper to invoke 'podman images' with different options, and to do everything else in getImages(). However, since then more is being done inside the podman package (eg., unmarshalling the 'podman images' JSON in commit 5f324d5 and flattening the images in commit 6aab0a6), and getImages() is the only caller of podman.GetImages(). Therefore, it looks awkward to have the code to get all Toolbx images split across two functions in different packages. containers#1724 containers#1772

Subsequent commits will use isToolbx() for image labels. So, it can't be in a file that's specific to containers. containers#1724 containers#1772

This is a step towards eliminating or reducing the code in getImages() by doing everything or more in podman.GetImages(). If nothing else, this removes the need to keep a copy of the labels used to identify Toolbx images for getImages(). The getImages() function originated as listContainers(), when it invoked 'podman images --filter label=...' through podman.GetImages() separately for each label used to identify Toolbx images, and then joined and sorted the results. This changed in commit 2369da5 when getImages() started invoking 'podman images' only once through podman.GetImages() to get all available images, and then filtered them itself based on the labels. Before this change in commit 2369da5, it probably made some sense to keep podman.GetImages() only as a thin wrapper to invoke 'podman images' with different options, and to do everything else in getImages(). However, since then more is being done inside the podman package (eg., unmarshalling the 'podman images' JSON in commit 5f324d5 and flattening the images in commit 6aab0a6), and getImages() is the only caller of podman.GetImages(). Therefore, it looks awkward to have the code to get all Toolbx images split across two functions in different packages. containers#1724 containers#1772

This is a step towards eliminating the code in getImages() by doing everything in podman.GetImages(). This removes the need to export the podman.ImageSlice type from the podman package. The getImages() function originated as listContainers(), when it invoked 'podman images --filter label=...' through podman.GetImages() separately for each label used to identify Toolbx images, and then joined and sorted the results. This changed in commit 2369da5 when getImages() started invoking 'podman images' only once through podman.GetImages() to get all available images, and then filtered them itself based on the labels. Before this change in commit 2369da5, it probably made some sense to keep podman.GetImages() only as a thin wrapper to invoke 'podman images' with different options, and to do everything else in getImages(). However, since then more is being done inside the podman package (eg., unmarshalling the 'podman images' JSON in commit 5f324d5 and flattening the images in commit 6aab0a6), and getImages() is the only caller of podman.GetImages(). Therefore, it looks awkward to have the code to get all Toolbx images split across two functions in different packages. containers#1724 containers#1772

softwarefactory-project-zuul · 2026-03-20T13:40:30Z

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/containers/toolbox for 1724,12863ee0ffd8db76b55849a1ef9073f57afef1ee

debarshiray

I noticed a few more relatively minor things while trying to rebase this against main.

src/pkg/podman/podman.go

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 · 2026-03-23T23:24:13Z

I rebased this branch on top of main, and squashed the two commits together so that the tests stay as close as possible to the code being tested.

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 · 2026-03-23T23:36:25Z

src/pkg/podman/image.go

+	return image.names
+}
+
+func (image *imageImages) setNames(names []string) {


Do we really need this private setter method? We don't have them for the other fields, and it might be overkill because these values can't be set from outside the podman package.

Yes, you are right. This setter function is really unnecessary here. In other functions in image.go (e.g., UnmarshalJSON()), fields are set directly so that we can do the same here.

src/meson.build

debarshiray · 2026-03-24T00:00:16Z

src/pkg/podman/image.go

+	ID() string
+	IsToolbx() bool
+	Labels() map[string]string
+	Names() []string


I am wondering if we can have a Name() variant (ie., singular) of the Names() method. In practical terms, it would simplify the rest of the code because we almost always want Names()[0], except in imageImages.flattenNames.

In philosophical terms, I am a bit worried about returning a slice because in Go slices are references to a underlying arrays. Changing the elements of a slice modifies the corresponding elements of its underlying array. Other slices that share the same underlying array will see those changes. So, it's theoretically possible that after copying and slicing a slice several times, a piece of code can unexpectedly change the elements to the surprise of a different piece of code.

One way to avoid that is to return a copy of the internal slice using copy.

I had originally added the Names() method to the podman.Containers type (in commit e611969) purely for the sake of completeness. So, maybe it's simpler to remove the method? What do you think?

One inconvenient detail is that we have assertions that check the number of elements in Names() to ensure that the images were flattened. Even if we retain the Names() method by returning a copy of the slice, it would be good to avoid the copies just for assertions.

I wonder if we can move the assertions to the Name() variant (ie., singular). Now that getImages() is removed and everything happens in podman.GetImages(), it seems that all images should be already flattened before any code outside this function sees them, and podman.GetImages() doesn't use the names. What do you think?

This is a very good idea! We can have a method Name() like the example below, which also encapsulates the assertion, and use the Names() method only to flatten the names within the podman package, ensuring that only flattened images are returned from its functions.

func (image *imageImages) Name() string { if len(image.names) != 1 { panic("cannot use Name() on unflattened Image") } return image.names[0] }

Did you mean something like this?

softwarefactory-project-zuul · 2026-03-24T00:49:01Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/c7a6f78f5913406b8a6c0668d463c627

softwarefactory-project-zuul · 2026-03-24T02:21:51Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/156ff515bf304bb0b85489f06f4755cd

DaliborKr requested a review from debarshiray as a code owner October 7, 2025 15:44

debarshiray requested changes Feb 27, 2026

View reviewed changes

src/pkg/podman/image.go Outdated Show resolved Hide resolved

debarshiray requested changes Mar 10, 2026

View reviewed changes

debarshiray mentioned this pull request Mar 17, 2026

cmd/list, pkg/podman: Eliminate getImages() by doing everything in podman.GetImages() #1772

Merged

debarshiray pushed a commit to debarshiray/toolbox that referenced this pull request Mar 17, 2026

pkg/podman: Shuffle some code around

163d863

Subsequent commits will use isToolbx() for image labels. So, it can't be in a file that's specific to containers. containers#1724 containers#1772

debarshiray force-pushed the image_interface branch from 5a6721a to 12863ee Compare March 20, 2026 13:40

debarshiray requested changes Mar 23, 2026

View reviewed changes

src/pkg/podman/podman.go Show resolved Hide resolved

src/pkg/podman/podman.go Outdated Show resolved Hide resolved

src/pkg/podman/podman.go Outdated Show resolved Hide resolved

debarshiray force-pushed the image_interface branch from 12863ee to aa553fc Compare March 23, 2026 22:14

debarshiray force-pushed the image_interface branch from aa553fc to 904a92e Compare March 23, 2026 23:22

debarshiray requested changes Mar 24, 2026

View reviewed changes

debarshiray force-pushed the image_interface branch from 904a92e to 40496b2 Compare March 24, 2026 01:11

Conversation

DaliborKr commented Oct 7, 2025

Uh oh!

softwarefactory-project-zuul bot commented Oct 7, 2025

Uh oh!

debarshiray left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

debarshiray Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

softwarefactory-project-zuul bot commented Mar 20, 2026

Uh oh!

debarshiray left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

debarshiray commented Mar 23, 2026

Uh oh!

debarshiray Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

DaliborKr Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

debarshiray Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

DaliborKr Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

softwarefactory-project-zuul bot commented Mar 24, 2026

Uh oh!

softwarefactory-project-zuul bot commented Mar 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants