mirror of
https://github.com/moby/buildkit.git
synced 2026-11-06 05:16:45 +00:00
http: fix resolver cache nil race
Avoid storing typed nil HTTP metadata in the resolver cache after failed metadata resolution. Also make resolver cache release functions consistently callable when no error is returned. Add a client integration test covering concurrent same-URL HTTP sources with and without checksum when the server returns 401. Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
This commit is contained in:
3 files changed
+50
-3
No files matched your search
@@ -116,6 +116,7 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){
|
||||
testCallDiskUsage,
|
||||
testBuildMultiMount,
|
||||
testBuildHTTPSource,
|
||||
testBuildHTTPSourceUnauthorizedChecksumRace,
|
||||
testBuildHTTPSourceEtagScope,
|
||||
testBuildHTTPSourceAuthHeaderSecret,
|
||||
testBuildHTTPSourceHostTokenSecret,
|
||||
@@ -3785,6 +3786,50 @@ func testBuildHTTPSource(t *testing.T, sb integration.Sandbox) {
|
||||
// TODO: check that second request was marked as cached
|
||||
}
|
||||
|
||||
func testBuildHTTPSourceUnauthorizedChecksumRace(t *testing.T, sb integration.Sandbox) {
|
||||
integration.SkipOnPlatform(t, "windows")
|
||||
|
||||
c, err := New(sb.Context(), sb.Address())
|
||||
require.NoError(t, err)
|
||||
defer c.Close()
|
||||
|
||||
var requests atomic.Int32
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if requests.Add(1) == 1 {
|
||||
time.Sleep(200 * time.Millisecond)
|
||||
}
|
||||
w.WriteHeader(http.StatusUnauthorized)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
url := server.URL + "/test.txt"
|
||||
lower := llb.Scratch()
|
||||
// Reproduce the race between same-URL HTTP sources with and without
|
||||
// checksum sharing the resolver cache while the server returns 401.
|
||||
withoutChecksum := lower.File(llb.Copy(llb.HTTP(url), "test.txt", "/a.txt"))
|
||||
withChecksum := lower.File(llb.Copy(llb.HTTP(url, llb.Checksum(digest.FromBytes(nil))), "test.txt", "/b.txt"))
|
||||
st := llb.Merge([]llb.State{
|
||||
llb.Diff(lower, withoutChecksum),
|
||||
llb.Diff(lower, withChecksum),
|
||||
})
|
||||
|
||||
def, err := st.Marshal(sb.Context())
|
||||
require.NoError(t, err)
|
||||
|
||||
for range 10 {
|
||||
_, err := c.Solve(sb.Context(), def, SolveOpt{
|
||||
Exports: []ExportEntry{
|
||||
{
|
||||
Type: ExporterLocal,
|
||||
OutputDir: t.TempDir(),
|
||||
},
|
||||
},
|
||||
}, nil)
|
||||
require.Error(t, err)
|
||||
require.ErrorContains(t, err, "invalid response status 401")
|
||||
}
|
||||
}
|
||||
|
||||
// docker/buildx#2803
|
||||
func testBuildHTTPSourceEtagScope(t *testing.T, sb integration.Sandbox) {
|
||||
c, err := New(sb.Context(), sb.Address())
|
||||
|
||||
@@ -127,7 +127,9 @@ func (c *combinedCache) Lock(key any) (values []any, release func(any) error, er
|
||||
|
||||
mu.Lock()
|
||||
valuesAll = append(valuesAll, vals...)
|
||||
releasers = append(releasers, rel)
|
||||
if rel != nil {
|
||||
releasers = append(releasers, rel)
|
||||
}
|
||||
mu.Unlock()
|
||||
}(rc)
|
||||
}
|
||||
|
||||
@@ -866,8 +866,8 @@ func (hs *httpSourceHandler) Snapshot(ctx context.Context, jobCtx solver.JobCont
|
||||
}
|
||||
for _, v := range vals {
|
||||
v2, ok := v.(*metadataWithRef)
|
||||
if !ok {
|
||||
return nil, errors.Errorf("invalid HTTP resolver cache value: %T", vals[0])
|
||||
if !ok || v2 == nil {
|
||||
return nil, errors.Errorf("invalid HTTP resolver cache value: %T", v)
|
||||
}
|
||||
if hs.src.Checksum != "" && v2.Digest != hs.src.Checksum {
|
||||
continue
|
||||
|
||||
Reference in new issue
Block a user