Skip to content

Commit 2484620

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

2 files changed

Lines changed: 110 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: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,95 @@ 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+
// no service.Image — the service is defined purely by its image-mount volume.
260+
Volumes: []ServiceVolumeConfig{
261+
{
262+
Type: VolumeTypeImage,
263+
Source: "busybox:latest",
264+
Target: "/test_mount",
265+
},
266+
},
267+
},
268+
},
269+
}
270+
271+
p, err := p.WithImagesResolved(resolver)
272+
assert.NilError(t, err)
273+
274+
app := p.Services["app"]
275+
assert.Equal(t, app.Image, "")
276+
assert.Equal(t, app.Volumes[0].Source, "docker.io/library/busybox:latest@"+dgst)
277+
}
278+
279+
func Test_ResolveImages_imageMount_alreadyCanonical(t *testing.T) {
280+
const dgst = "sha256:1234567890123456789012345678901234567890123456789012345678901234"
281+
resolverCalls := 0
282+
resolver := func(_ reference.Named) (digest.Digest, error) {
283+
resolverCalls++
284+
return dgst, nil
285+
}
286+
pinned := "docker.io/library/busybox@" + dgst
287+
p := &Project{
288+
Services: Services{
289+
"app": ServiceConfig{
290+
Name: "app",
291+
Image: pinned,
292+
Volumes: []ServiceVolumeConfig{
293+
{
294+
Type: VolumeTypeImage,
295+
Source: pinned,
296+
Target: "/test_mount",
297+
},
298+
},
299+
},
300+
},
301+
}
302+
303+
p, err := p.WithImagesResolved(resolver)
304+
assert.NilError(t, err)
305+
306+
app := p.Services["app"]
307+
// already-canonical references are passed through unchanged.
308+
assert.Equal(t, app.Image, pinned)
309+
assert.Equal(t, app.Volumes[0].Source, pinned)
310+
// and the resolver is never called for canonical references.
311+
assert.Equal(t, resolverCalls, 0)
312+
}
313+
314+
func Test_ResolveImages_imageMount_unresolvable(t *testing.T) {
315+
resolver := func(_ reference.Named) (digest.Digest, error) {
316+
return "", errors.New("manifest unknown")
317+
}
318+
p := &Project{
319+
Services: Services{
320+
"app": ServiceConfig{
321+
Name: "app",
322+
Volumes: []ServiceVolumeConfig{
323+
{
324+
Type: VolumeTypeImage,
325+
Source: "registry.example/missing:1.0",
326+
Target: "/test_mount",
327+
},
328+
},
329+
},
330+
},
331+
}
332+
333+
_, err := p.WithImagesResolved(resolver)
334+
// failure to resolve an image-mount source must surface as an error,
335+
// same as for service.Image.
336+
assert.Error(t, err, "manifest unknown")
337+
}
338+
250339
func Test_ResolveImages_concurrent(t *testing.T) {
251340
const garfield = "sha256:1234567890123456789012345678901234567890123456789012345678901234"
252341
resolver := func(_ reference.Named) (digest.Digest, error) {

0 commit comments

Comments
 (0)