Skip to content

Commit 3b35fc3

Browse files
committed
http: avoid possible digest mismatch error
There is a possibility to get a digest mismatch error if the metadata for previous download does not point to a valid reference anymore. To mitigate this, check that ref that etag points to is still valid before using it. Additionally `.cacheKey` property was not previously set in the cases where old reference was reused. This caused a case where even if the download needed to be performed again, it always failed validation, even if the digest had not actually changed since previous download. There is still a small possibility that gc/prune request will delete the downloaded record in between cachemap and exec call and that the contents changes in the server at that exact time. To fix that case we would need to modify cachemap so that it can keep hold of references until build is complete. Signed-off-by: Tonis Tiigi <[email protected]>
1 parent 293ef59 commit 3b35fc3

File tree

1 file changed

+13
-2
lines changed

1 file changed

+13
-2
lines changed

source/http/source.go

+13-2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/moby/buildkit/solver/pb"
2525
"github.com/moby/buildkit/source"
2626
srctypes "github.com/moby/buildkit/source/types"
27+
"github.com/moby/buildkit/util/bklog"
2728
"github.com/moby/buildkit/util/tracing"
2829
digest "github.com/opencontainers/go-digest"
2930
"github.com/pkg/errors"
@@ -192,7 +193,12 @@ func (hs *httpSourceHandler) CacheKey(ctx context.Context, g session.Group, inde
192193
// if metaDigest := getMetaDigest(si); metaDigest == hs.formatCacheKey("") {
193194
if etag := md.getETag(); etag != "" {
194195
if dgst := md.getHTTPChecksum(); dgst != "" {
195-
m[etag] = md
196+
// check that ref still exists
197+
ref, err := hs.cache.Get(ctx, md.ID(), nil)
198+
if err == nil {
199+
m[etag] = md
200+
defer ref.Release(context.WithoutCancel(ctx))
201+
}
196202
}
197203
}
198204
// }
@@ -235,6 +241,7 @@ func (hs *httpSourceHandler) CacheKey(ctx context.Context, g session.Group, inde
235241
hs.refID = md.ID()
236242
dgst := md.getHTTPChecksum()
237243
if dgst != "" {
244+
hs.cacheKey = dgst
238245
modTime := md.getHTTPModTime()
239246
resp.Body.Close()
240247
return hs.formatCacheKey(getFileName(hs.src.URL, hs.src.Filename, resp), dgst, modTime).String(), dgst.String(), nil, true, nil
@@ -275,8 +282,10 @@ func (hs *httpSourceHandler) CacheKey(ctx context.Context, g session.Group, inde
275282
if dgst == "" {
276283
return "", "", nil, false, errors.Errorf("invalid metadata change")
277284
}
285+
hs.cacheKey = dgst
278286
modTime := md.getHTTPModTime()
279287
resp.Body.Close()
288+
280289
return hs.formatCacheKey(getFileName(hs.src.URL, hs.src.Filename, resp), dgst, modTime).String(), dgst.String(), nil, true, nil
281290
}
282291

@@ -421,7 +430,9 @@ func (hs *httpSourceHandler) save(ctx context.Context, resp *http.Response, s se
421430
func (hs *httpSourceHandler) Snapshot(ctx context.Context, g session.Group) (cache.ImmutableRef, error) {
422431
if hs.refID != "" {
423432
ref, err := hs.cache.Get(ctx, hs.refID, nil)
424-
if err == nil {
433+
if err != nil {
434+
bklog.G(ctx).WithError(err).Warnf("failed to get HTTP snapshot for ref %s (%s)", hs.refID, hs.src.URL)
435+
} else {
425436
return ref, nil
426437
}
427438
}

0 commit comments

Comments
 (0)