Skip to content

Commit d197ff4

Browse files
committed
cmd, pkg/podman: Add Name() and apply feedback for Image interface
+ Add Name() method to Image interface to encapsulate flattening assertions: - For imageImages, it returns the only present name (as it should be flattened, otherwise it panics) - For imageInspect, it returns the last name from the image NamesHistory (according to the implementation, the latest name is on index 0 -> see https://github.com/containers/storage/blob/main/images.go) + Remove unnecessary setNames() private method + Move image name assertions from cmd into podman package This addresses review feedback from @debarshiray in PR #1724. Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
1 parent 6b25acd commit d197ff4

5 files changed

Lines changed: 171 additions & 29 deletions

File tree

src/cmd/completion.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -152,11 +152,8 @@ func completionImageNames(cmd *cobra.Command, _ []string, _ string) ([]string, c
152152
} else {
153153
for images.Next() {
154154
image := images.Get()
155-
if len(image.Names()) != 1 {
156-
panic("cannot complete unflattened Image")
157-
}
158155

159-
imageNames = append(imageNames, image.Names()[0])
156+
imageNames = append(imageNames, image.Name())
160157
}
161158
}
162159

@@ -175,12 +172,9 @@ func completionImageNamesFiltered(_ *cobra.Command, args []string, _ string) ([]
175172
skip := false
176173

177174
image := images.Get()
178-
if len(image.Names()) != 1 {
179-
panic("cannot complete unflattened Image")
180-
}
181175

182176
for _, arg := range args {
183-
if arg == image.Names()[0] {
177+
if arg == image.Name() {
184178
skip = true
185179
break
186180
}
@@ -190,7 +184,7 @@ func completionImageNamesFiltered(_ *cobra.Command, args []string, _ string) ([]
190184
continue
191185
}
192186

193-
imageNames = append(imageNames, image.Names()[0])
187+
imageNames = append(imageNames, image.Name())
194188
}
195189
}
196190

src/cmd/list.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,13 +137,10 @@ func listOutput(images *podman.Images, containers *podman.Containers) {
137137

138138
for images.Next() {
139139
image := images.Get()
140-
if len(image.Names()) != 1 {
141-
panic("cannot list unflattened Image")
142-
}
143140

144141
fmt.Fprintf(writer, "%s\t%s\t%s\n",
145142
utils.ShortID(image.ID()),
146-
image.Names()[0],
143+
image.Name(),
147144
image.Created())
148145
}
149146

src/pkg/podman/image.go

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ type Image interface {
2727
ID() string
2828
IsToolbx() bool
2929
Labels() map[string]string
30+
Name() string
3031
Names() []string
3132
RepoTags() []string
3233
}
@@ -98,9 +99,9 @@ func (image *imageImages) flattenNames(fillNameWithID bool) []imageImages {
9899

99100
if fillNameWithID {
100101
shortID := utils.ShortID(image.ID())
101-
flattenedImage.setNames([]string{shortID})
102+
flattenedImage.names = []string{shortID}
102103
} else {
103-
flattenedImage.setNames([]string{"<none>"})
104+
flattenedImage.names = []string{"<none>"}
104105
}
105106

106107
ret = []imageImages{flattenedImage}
@@ -111,7 +112,7 @@ func (image *imageImages) flattenNames(fillNameWithID bool) []imageImages {
111112

112113
for _, name := range image.Names() {
113114
flattenedImage := *image
114-
flattenedImage.setNames([]string{name})
115+
flattenedImage.names = []string{name}
115116
ret = append(ret, flattenedImage)
116117
}
117118

@@ -130,12 +131,15 @@ func (image *imageImages) Labels() map[string]string {
130131
return image.labels
131132
}
132133

133-
func (image *imageImages) Names() []string {
134-
return image.names
134+
func (image *imageImages) Name() string {
135+
if len(image.names) != 1 {
136+
panic("cannot get name from unflattened Image")
137+
}
138+
return image.names[0]
135139
}
136140

137-
func (image *imageImages) setNames(names []string) {
138-
image.names = names
141+
func (image *imageImages) Names() []string {
142+
return image.names
139143
}
140144

141145
func (image *imageImages) RepoTags() []string {
@@ -200,6 +204,13 @@ func (image *imageInspect) Names() []string {
200204
return image.namesHistory
201205
}
202206

207+
func (image *imageInspect) Name() string {
208+
if len(image.namesHistory) == 0 {
209+
panic("no name is available in Image name history")
210+
}
211+
return image.namesHistory[0]
212+
}
213+
203214
func (image *imageInspect) RepoTags() []string {
204215
return image.repoTags
205216
}
@@ -242,15 +253,7 @@ func (images imageSlice) Len() int {
242253
}
243254

244255
func (images imageSlice) Less(i, j int) bool {
245-
if len(images[i].Names()) != 1 {
246-
panic("cannot sort unflattened Images")
247-
}
248-
249-
if len(images[j].Names()) != 1 {
250-
panic("cannot sort unflattened Images")
251-
}
252-
253-
return images[i].Names()[0] < images[j].Names()[0]
256+
return images[i].Name() < images[j].Name()
254257
}
255258

256259
func (images imageSlice) Swap(i, j int) {

src/pkg/podman/imageImages_test.go

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,134 @@ func TestImageImages(t *testing.T) {
475475
},
476476
imageCnt: 1,
477477
},
478+
{
479+
name: "podman 5.6.1, fedora-toolbox:41, no name specified",
480+
data: "" +
481+
"[" +
482+
" {" +
483+
" \"Id\": \"b65ee158f921088db52e8b98a6f3267de75324c8a6a04afc9ff095338c40e59b\"," +
484+
" \"ParentId\": \"\"," +
485+
" \"RepoTags\": null," +
486+
" \"RepoDigests\": [" +
487+
" \"registry.fedoraproject.org/fedora-toolbox@sha256:0a51adc6bab55d49ff00da8aaad81ca1f02315511ed23d55ee5bbbe1a976a663\"," +
488+
" \"registry.fedoraproject.org/fedora-toolbox@sha256:8599f0c0d421c0dc01c4b7fb1c07b2780c0ab1931d0f22dd7b6da3b93ff6b77b\"" +
489+
" ]," +
490+
" \"Size\": 2308518290," +
491+
" \"SharedSize\": 0," +
492+
" \"VirtualSize\": 2308518290," +
493+
" \"Labels\": {" +
494+
" \"com.github.containers.toolbox\": \"true\"," +
495+
" \"io.buildah.version\": \"1.41.4\"," +
496+
" \"license\": \"MIT\"," +
497+
" \"name\": \"fedora-toolbox\"," +
498+
" \"org.opencontainers.image.license\": \"MIT\"," +
499+
" \"org.opencontainers.image.name\": \"fedora-toolbox\"," +
500+
" \"org.opencontainers.image.url\": \"https://fedoraproject.org/\"," +
501+
" \"org.opencontainers.image.vendor\": \"Fedora Project\"," +
502+
" \"org.opencontainers.image.version\": \"41\"," +
503+
" \"vendor\": \"Fedora Project\"," +
504+
" \"version\": \"41\"" +
505+
" }," +
506+
" \"Containers\": 1," +
507+
" \"Digest\": \"sha256:0a51adc6bab55d49ff00da8aaad81ca1f02315511ed23d55ee5bbbe1a976a663\"," +
508+
" \"History\": [" +
509+
" \"registry.fedoraproject.org/fedora-toolbox:41\"" +
510+
" ]," +
511+
" \"Names\": []," +
512+
" \"Created\": 1759729685," +
513+
" \"CreatedAt\": \"2025-10-06T05:48:05Z\"" +
514+
" }" +
515+
"]",
516+
expects: []expect{
517+
{
518+
id: "b65ee158f921088db52e8b98a6f3267de75324c8a6a04afc9ff095338c40e59b",
519+
isToolbx: true,
520+
labels: map[string]string{
521+
"com.github.containers.toolbox": "true",
522+
"io.buildah.version": "1.41.4",
523+
"license": "MIT",
524+
"name": "fedora-toolbox",
525+
"org.opencontainers.image.license": "MIT",
526+
"org.opencontainers.image.name": "fedora-toolbox",
527+
"org.opencontainers.image.url": "https://fedoraproject.org/",
528+
"org.opencontainers.image.vendor": "Fedora Project",
529+
"org.opencontainers.image.version": "41",
530+
"vendor": "Fedora Project",
531+
"version": "41",
532+
},
533+
names: []string{},
534+
repoTags: nil,
535+
},
536+
},
537+
imageCnt: 1,
538+
},
539+
{
540+
name: "podman 5.6.1, fedora-toolbox:41, multiple names specified",
541+
data: "" +
542+
"[" +
543+
" {" +
544+
" \"Id\": \"b65ee158f921088db52e8b98a6f3267de75324c8a6a04afc9ff095338c40e59b\"," +
545+
" \"ParentId\": \"\"," +
546+
" \"RepoTags\": null," +
547+
" \"RepoDigests\": [" +
548+
" \"registry.fedoraproject.org/fedora-toolbox@sha256:0a51adc6bab55d49ff00da8aaad81ca1f02315511ed23d55ee5bbbe1a976a663\"," +
549+
" \"registry.fedoraproject.org/fedora-toolbox@sha256:8599f0c0d421c0dc01c4b7fb1c07b2780c0ab1931d0f22dd7b6da3b93ff6b77b\"" +
550+
" ]," +
551+
" \"Size\": 2308518290," +
552+
" \"SharedSize\": 0," +
553+
" \"VirtualSize\": 2308518290," +
554+
" \"Labels\": {" +
555+
" \"com.github.containers.toolbox\": \"true\"," +
556+
" \"io.buildah.version\": \"1.41.4\"," +
557+
" \"license\": \"MIT\"," +
558+
" \"name\": \"fedora-toolbox\"," +
559+
" \"org.opencontainers.image.license\": \"MIT\"," +
560+
" \"org.opencontainers.image.name\": \"fedora-toolbox\"," +
561+
" \"org.opencontainers.image.url\": \"https://fedoraproject.org/\"," +
562+
" \"org.opencontainers.image.vendor\": \"Fedora Project\"," +
563+
" \"org.opencontainers.image.version\": \"41\"," +
564+
" \"vendor\": \"Fedora Project\"," +
565+
" \"version\": \"41\"" +
566+
" }," +
567+
" \"Containers\": 1," +
568+
" \"Digest\": \"sha256:0a51adc6bab55d49ff00da8aaad81ca1f02315511ed23d55ee5bbbe1a976a663\"," +
569+
" \"History\": [" +
570+
" \"registry.fedoraproject.org/fedora-toolbox:41\"" +
571+
" ]," +
572+
" \"Names\": [" +
573+
" \"registry.fedoraproject.org/fedora-toolbox:41-test\"," +
574+
" \"registry.fedoraproject.org/fedora-toolbox:41\"" +
575+
" ]," +
576+
" \"Created\": 1759729685," +
577+
" \"CreatedAt\": \"2025-10-06T05:48:05Z\"" +
578+
" }" +
579+
"]",
580+
expects: []expect{
581+
{
582+
id: "b65ee158f921088db52e8b98a6f3267de75324c8a6a04afc9ff095338c40e59b",
583+
isToolbx: true,
584+
labels: map[string]string{
585+
"com.github.containers.toolbox": "true",
586+
"io.buildah.version": "1.41.4",
587+
"license": "MIT",
588+
"name": "fedora-toolbox",
589+
"org.opencontainers.image.license": "MIT",
590+
"org.opencontainers.image.name": "fedora-toolbox",
591+
"org.opencontainers.image.url": "https://fedoraproject.org/",
592+
"org.opencontainers.image.vendor": "Fedora Project",
593+
"org.opencontainers.image.version": "41",
594+
"vendor": "Fedora Project",
595+
"version": "41",
596+
},
597+
names: []string{
598+
"registry.fedoraproject.org/fedora-toolbox:41-test",
599+
"registry.fedoraproject.org/fedora-toolbox:41",
600+
},
601+
repoTags: nil,
602+
},
603+
},
604+
imageCnt: 1,
605+
},
478606
{
479607
name: "podman 5.6.2, fedora-toolbox:42, docker.io-postgres:18, fedora-toolbox:42-aarch64",
480608
data: "" +
@@ -644,6 +772,20 @@ func TestImageImages(t *testing.T) {
644772
assert.Equal(t, expect.labels, image.Labels())
645773
assert.Equal(t, expect.names, image.Names())
646774
assert.Equal(t, expect.repoTags, image.RepoTags())
775+
776+
if len(expect.names) == 1 {
777+
assert.Equal(t, expect.names[0], image.Name())
778+
} else if len(expect.names) > 1 {
779+
assert.Panics(t, func() { _ = image.Name() })
780+
781+
imagesFlattened := image.flattenNames(false)
782+
for j, imageFlattened := range imagesFlattened {
783+
assert.Equal(t, expect.names[j], imageFlattened.Name())
784+
}
785+
786+
} else {
787+
assert.Panics(t, func() { _ = image.Name() })
788+
}
647789
}
648790
})
649791
}

src/pkg/podman/imageInspect_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,6 +1515,12 @@ func TestImageInspect(t *testing.T) {
15151515
assert.Equal(t, tc.expect.repoTags, image.RepoTags())
15161516
assert.Equal(t, tc.expect.entrypoint, image.Entrypoint())
15171517
assert.Equal(t, tc.expect.envVars, image.EnvVars())
1518+
1519+
if len(tc.expect.namesHistory) > 0 {
1520+
assert.Equal(t, tc.expect.namesHistory[0], image.Name())
1521+
} else {
1522+
assert.Panics(t, func() { _ = image.Name() })
1523+
}
15181524
})
15191525
}
15201526
}

0 commit comments

Comments
 (0)