Skip to content

Commit

Permalink
http: avoid possible digest mismatch error
Browse files Browse the repository at this point in the history
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 <tonistiigi@gmail.com>
  • Loading branch information
tonistiigi committed Oct 5, 2024
1 parent 293ef59 commit 3b35fc3
Showing 1 changed file with 13 additions and 2 deletions.
15 changes: 13 additions & 2 deletions source/http/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
}
}
}
// }
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
}
Expand Down

0 comments on commit 3b35fc3

Please sign in to comment.