From 3b35fc32078391453a8d8d1f15d6b0986f452911 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Thu, 19 Sep 2024 18:14:59 -0700 Subject: [PATCH] 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 --- source/http/source.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/source/http/source.go b/source/http/source.go index d6c6050199fb..82d26eb34a59 100644 --- a/source/http/source.go +++ b/source/http/source.go @@ -24,6 +24,7 @@ import ( "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/source" srctypes "github.com/moby/buildkit/source/types" + "github.com/moby/buildkit/util/bklog" "github.com/moby/buildkit/util/tracing" digest "github.com/opencontainers/go-digest" "github.com/pkg/errors" @@ -192,7 +193,12 @@ func (hs *httpSourceHandler) CacheKey(ctx context.Context, g session.Group, inde // if metaDigest := getMetaDigest(si); metaDigest == hs.formatCacheKey("") { if etag := md.getETag(); etag != "" { if dgst := md.getHTTPChecksum(); dgst != "" { - m[etag] = md + // check that ref still exists + ref, err := hs.cache.Get(ctx, md.ID(), nil) + if err == nil { + m[etag] = md + defer ref.Release(context.WithoutCancel(ctx)) + } } } // } @@ -235,6 +241,7 @@ func (hs *httpSourceHandler) CacheKey(ctx context.Context, g session.Group, inde hs.refID = md.ID() dgst := md.getHTTPChecksum() if dgst != "" { + hs.cacheKey = dgst modTime := md.getHTTPModTime() resp.Body.Close() 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 if dgst == "" { return "", "", nil, false, errors.Errorf("invalid metadata change") } + hs.cacheKey = dgst modTime := md.getHTTPModTime() resp.Body.Close() + return hs.formatCacheKey(getFileName(hs.src.URL, hs.src.Filename, resp), dgst, modTime).String(), dgst.String(), nil, true, nil } @@ -421,7 +430,9 @@ func (hs *httpSourceHandler) save(ctx context.Context, resp *http.Response, s se func (hs *httpSourceHandler) Snapshot(ctx context.Context, g session.Group) (cache.ImmutableRef, error) { if hs.refID != "" { ref, err := hs.cache.Get(ctx, hs.refID, nil) - if err == nil { + if err != nil { + bklog.G(ctx).WithError(err).Warnf("failed to get HTTP snapshot for ref %s (%s)", hs.refID, hs.src.URL) + } else { return ref, nil } }