Skip to content

Commit 7180d6a

Browse files
committed
Address review: memoize per-service, expand image-mount tests, clarify doc comment
1 parent f8d7320 commit 7180d6a

2 files changed

Lines changed: 105 additions & 4 deletions

File tree

types/project.go

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -575,12 +575,29 @@ func (p *Project) WithServicesDisabled(names ...string) *Project {
575575
return newProject
576576
}
577577

578-
// WithImagesResolved updates services images to include digest computed by a resolver function
579-
// It returns a new Project instance with the changes and keep the original Project unchanged
578+
// WithImagesResolved updates services to pin both service.Image and image-mount
579+
// volume sources (type=image) to digests computed by the resolver. It returns a
580+
// new Project with the changes and keeps the original Project unchanged. Within
581+
// each service, repeated lookups for the same image reference are deduplicated
582+
// so callers that hit a registry per resolve aren't charged twice when the
583+
// service image and an image-mount volume reference the same tag.
580584
func (p *Project) WithImagesResolved(resolver func(named reference.Named) (godigest.Digest, error)) (*Project, error) {
581585
return p.WithServicesTransform(func(_ string, service ServiceConfig) (ServiceConfig, error) {
586+
cache := map[string]string{}
587+
resolve := func(img string) (string, error) {
588+
if r, ok := cache[img]; ok {
589+
return r, nil
590+
}
591+
r, err := resolveImageDigest(img, resolver)
592+
if err != nil {
593+
return img, err
594+
}
595+
cache[img] = r
596+
return r, nil
597+
}
598+
582599
if service.Image != "" {
583-
resolved, err := resolveImageDigest(service.Image, resolver)
600+
resolved, err := resolve(service.Image)
584601
if err != nil {
585602
return service, err
586603
}
@@ -591,7 +608,7 @@ func (p *Project) WithImagesResolved(resolver func(named reference.Named) (godig
591608
if volume.Type != VolumeTypeImage || volume.Source == "" {
592609
continue
593610
}
594-
resolved, err := resolveImageDigest(volume.Source, resolver)
611+
resolved, err := resolve(volume.Source)
595612
if err != nil {
596613
return service, err
597614
}

types/project_test.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,90 @@ func Test_ResolveImages_imageMount(t *testing.T) {
247247
assert.Equal(t, app.Volumes[1].Source, "/host/path")
248248
}
249249

250+
func Test_ResolveImages_imageMount_noServiceImage(t *testing.T) {
251+
const dgst = "sha256:1234567890123456789012345678901234567890123456789012345678901234"
252+
resolver := func(_ reference.Named) (digest.Digest, error) {
253+
return dgst, nil
254+
}
255+
p := &Project{
256+
Services: Services{
257+
"app": ServiceConfig{
258+
Name: "app",
259+
Volumes: []ServiceVolumeConfig{
260+
{
261+
Type: VolumeTypeImage,
262+
Source: "busybox:latest",
263+
Target: "/test_mount",
264+
},
265+
},
266+
},
267+
},
268+
}
269+
270+
p, err := p.WithImagesResolved(resolver)
271+
assert.NilError(t, err)
272+
273+
app := p.Services["app"]
274+
assert.Equal(t, app.Image, "")
275+
assert.Equal(t, app.Volumes[0].Source, "docker.io/library/busybox:latest@"+dgst)
276+
}
277+
278+
func Test_ResolveImages_imageMount_alreadyCanonical(t *testing.T) {
279+
const dgst = "sha256:1234567890123456789012345678901234567890123456789012345678901234"
280+
resolverCalls := 0
281+
resolver := func(_ reference.Named) (digest.Digest, error) {
282+
resolverCalls++
283+
return dgst, nil
284+
}
285+
pinned := "docker.io/library/busybox@" + dgst
286+
p := &Project{
287+
Services: Services{
288+
"app": ServiceConfig{
289+
Name: "app",
290+
Image: pinned,
291+
Volumes: []ServiceVolumeConfig{
292+
{
293+
Type: VolumeTypeImage,
294+
Source: pinned,
295+
Target: "/test_mount",
296+
},
297+
},
298+
},
299+
},
300+
}
301+
302+
p, err := p.WithImagesResolved(resolver)
303+
assert.NilError(t, err)
304+
305+
app := p.Services["app"]
306+
assert.Equal(t, app.Image, pinned)
307+
assert.Equal(t, app.Volumes[0].Source, pinned)
308+
assert.Equal(t, resolverCalls, 0)
309+
}
310+
311+
func Test_ResolveImages_imageMount_unresolvable(t *testing.T) {
312+
resolver := func(_ reference.Named) (digest.Digest, error) {
313+
return "", errors.New("manifest unknown")
314+
}
315+
p := &Project{
316+
Services: Services{
317+
"app": ServiceConfig{
318+
Name: "app",
319+
Volumes: []ServiceVolumeConfig{
320+
{
321+
Type: VolumeTypeImage,
322+
Source: "registry.example/missing:1.0",
323+
Target: "/test_mount",
324+
},
325+
},
326+
},
327+
},
328+
}
329+
330+
_, err := p.WithImagesResolved(resolver)
331+
assert.Error(t, err, "manifest unknown")
332+
}
333+
250334
func Test_ResolveImages_concurrent(t *testing.T) {
251335
const garfield = "sha256:1234567890123456789012345678901234567890123456789012345678901234"
252336
resolver := func(_ reference.Named) (digest.Digest, error) {

0 commit comments

Comments
 (0)