Skip to content

Commit c0fda82

Browse files
authored
Merge pull request #2608 from norio-nomura/re-download-digest-less-cached-images-if-last-modified-differs
downloader: re-download digest-less cached images if `last-modified` differs
2 parents cebc751 + f853230 commit c0fda82

File tree

3 files changed

+135
-9
lines changed

3 files changed

+135
-9
lines changed

pkg/downloader/downloader.go

Lines changed: 56 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ func Download(ctx context.Context, local, remote string, opts ...Opt) (*Result,
229229
}
230230
if _, err := os.Stat(shadData); err == nil {
231231
logrus.Debugf("file %q is cached as %q", localPath, shadData)
232+
useCache := true
232233
if _, err := os.Stat(shadDigest); err == nil {
233234
logrus.Debugf("Comparing digest %q with the cached digest file %q, not computing the actual digest of %q",
234235
o.expectedDigest, shadDigest, shadData)
@@ -239,18 +240,27 @@ func Download(ctx context.Context, local, remote string, opts ...Opt) (*Result,
239240
return nil, err
240241
}
241242
} else {
242-
if err := copyLocal(ctx, localPath, shadData, ext, o.decompress, o.description, o.expectedDigest); err != nil {
243-
return nil, err
243+
if match, lmCached, lmRemote, err := matchLastModified(ctx, shadTime, remote); err != nil {
244+
logrus.WithError(err).Info("Failed to retrieve last-modified for cached digest-less image; using cached image.")
245+
} else if match {
246+
if err := copyLocal(ctx, localPath, shadData, ext, o.decompress, o.description, o.expectedDigest); err != nil {
247+
return nil, err
248+
}
249+
} else {
250+
logrus.Infof("Re-downloading digest-less image: last-modified mismatch (cached: %q, remote: %q)", lmCached, lmRemote)
251+
useCache = false
244252
}
245253
}
246-
res := &Result{
247-
Status: StatusUsedCache,
248-
CachePath: shadData,
249-
LastModified: readTime(shadTime),
250-
ContentType: readFile(shadType),
251-
ValidatedDigest: o.expectedDigest != "",
254+
if useCache {
255+
res := &Result{
256+
Status: StatusUsedCache,
257+
CachePath: shadData,
258+
LastModified: readTime(shadTime),
259+
ContentType: readFile(shadType),
260+
ValidatedDigest: o.expectedDigest != "",
261+
}
262+
return res, nil
252263
}
253-
return res, nil
254264
}
255265
if err := os.RemoveAll(shad); err != nil {
256266
return nil, err
@@ -548,6 +558,43 @@ func validateLocalFileDigest(localPath string, expectedDigest digest.Digest) err
548558
return nil
549559
}
550560

561+
// mathLastModified takes params:
562+
// - ctx: context for calling httpclientutil.Head
563+
// - lastModifiedPath: path of the cached last-modified time file
564+
// - url: URL to fetch the last-modified time
565+
//
566+
// returns:
567+
// - matched: whether the last-modified time matches
568+
// - lmCached: last-modified time string from the lastModifiedPath
569+
// - lmRemote: last-modified time string from the URL
570+
// - err: error if fetching the last-modified time from the URL fails
571+
func matchLastModified(ctx context.Context, lastModifiedPath, url string) (matched bool, lmCached, lmRemote string, err error) {
572+
lmCached = readFile(lastModifiedPath)
573+
if lmCached == "" {
574+
return false, "<not cached>", "<not checked>", nil
575+
}
576+
resp, err := httpclientutil.Head(ctx, http.DefaultClient, url)
577+
if err != nil {
578+
return false, lmCached, "<failed to fetch remote>", err
579+
}
580+
defer resp.Body.Close()
581+
lmRemote = resp.Header.Get("Last-Modified")
582+
if lmRemote == "" {
583+
return false, lmCached, "<missing Last-Modified header>", nil
584+
}
585+
lmCachedTime, errParsingCachedTime := time.Parse(http.TimeFormat, lmCached)
586+
lmRemoteTime, errParsingRemoteTime := time.Parse(http.TimeFormat, lmRemote)
587+
if errParsingCachedTime != nil && errParsingRemoteTime != nil {
588+
// both time strings are failed to parse, so compare them as strings
589+
return lmCached == lmRemote, lmCached, lmRemote, nil
590+
} else if errParsingCachedTime == nil && errParsingRemoteTime == nil {
591+
// both time strings are successfully parsed, so compare them as times
592+
return lmRemoteTime.Equal(lmCachedTime), lmCached, lmRemote, nil
593+
}
594+
// ignore parsing errors for either time string and assume they are different
595+
return false, lmCached, lmRemote, nil
596+
}
597+
551598
func downloadHTTP(ctx context.Context, localPath, lastModified, contentType, url, description string, expectedDigest digest.Digest) error {
552599
if localPath == "" {
553600
return fmt.Errorf("downloadHTTP: got empty localPath")

pkg/downloader/downloader_test.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,69 @@ func TestDownloadRemote(t *testing.T) {
121121
})
122122
}
123123

124+
func TestRedownloadRemote(t *testing.T) {
125+
remoteDir, err := os.MkdirTemp("", "redownloadRemote")
126+
assert.NilError(t, err)
127+
t.Cleanup(func() { _ = os.RemoveAll(remoteDir) })
128+
ts := httptest.NewServer(http.FileServer(http.Dir(remoteDir)))
129+
t.Cleanup(ts.Close)
130+
131+
cacheDir, err := os.MkdirTemp("", "redownloadCache")
132+
assert.NilError(t, err)
133+
t.Cleanup(func() { _ = os.RemoveAll(cacheDir) })
134+
135+
downloadDir, err := os.MkdirTemp("", "redownloadLocal")
136+
assert.NilError(t, err)
137+
t.Cleanup(func() { _ = os.RemoveAll(downloadDir) })
138+
139+
cacheOpt := WithCacheDir(cacheDir)
140+
141+
t.Run("digest-less", func(t *testing.T) {
142+
remoteFile := filepath.Join(remoteDir, "digest-less.txt")
143+
assert.NilError(t, os.WriteFile(remoteFile, []byte("digest-less"), 0o644))
144+
assert.NilError(t, os.Chtimes(remoteFile, time.Now(), time.Now().Add(-time.Hour)))
145+
opt := []Opt{cacheOpt}
146+
147+
r, err := Download(context.Background(), filepath.Join(downloadDir, "digest-less1.txt"), ts.URL+"/digest-less.txt", opt...)
148+
assert.NilError(t, err)
149+
assert.Equal(t, StatusDownloaded, r.Status)
150+
r, err = Download(context.Background(), filepath.Join(downloadDir, "digest-less2.txt"), ts.URL+"/digest-less.txt", opt...)
151+
assert.NilError(t, err)
152+
assert.Equal(t, StatusUsedCache, r.Status)
153+
154+
// modifying remote file will cause redownload
155+
assert.NilError(t, os.Chtimes(remoteFile, time.Now(), time.Now()))
156+
r, err = Download(context.Background(), filepath.Join(downloadDir, "digest-less3.txt"), ts.URL+"/digest-less.txt", opt...)
157+
assert.NilError(t, err)
158+
assert.Equal(t, StatusDownloaded, r.Status)
159+
})
160+
161+
t.Run("has-digest", func(t *testing.T) {
162+
remoteFile := filepath.Join(remoteDir, "has-digest.txt")
163+
bytes := []byte("has-digest")
164+
assert.NilError(t, os.WriteFile(remoteFile, bytes, 0o644))
165+
assert.NilError(t, os.Chtimes(remoteFile, time.Now(), time.Now().Add(-time.Hour)))
166+
167+
digester := digest.SHA256.Digester()
168+
_, err := digester.Hash().Write(bytes)
169+
assert.NilError(t, err)
170+
opt := []Opt{cacheOpt, WithExpectedDigest(digester.Digest())}
171+
172+
r, err := Download(context.Background(), filepath.Join(downloadDir, "has-digest1.txt"), ts.URL+"/has-digest.txt", opt...)
173+
assert.NilError(t, err)
174+
assert.Equal(t, StatusDownloaded, r.Status)
175+
r, err = Download(context.Background(), filepath.Join(downloadDir, "has-digest2.txt"), ts.URL+"/has-digest.txt", opt...)
176+
assert.NilError(t, err)
177+
assert.Equal(t, StatusUsedCache, r.Status)
178+
179+
// modifying remote file won't cause redownload because expected digest is provided
180+
assert.NilError(t, os.Chtimes(remoteFile, time.Now(), time.Now()))
181+
r, err = Download(context.Background(), filepath.Join(downloadDir, "has-digest3.txt"), ts.URL+"/has-digest.txt", opt...)
182+
assert.NilError(t, err)
183+
assert.Equal(t, StatusUsedCache, r.Status)
184+
})
185+
}
186+
124187
func TestDownloadLocal(t *testing.T) {
125188
const emptyFileDigest = "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
126189
const testDownloadLocalDigest = "sha256:0c1e0fba69e8919b306d030bf491e3e0c46cf0a8140ff5d7516ba3a83cbea5b3"

pkg/httpclientutil/httpclientutil.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,22 @@ func Get(ctx context.Context, c *http.Client, url string) (*http.Response, error
3232
return resp, nil
3333
}
3434

35+
func Head(ctx context.Context, c *http.Client, url string) (*http.Response, error) {
36+
req, err := http.NewRequestWithContext(ctx, "HEAD", url, http.NoBody)
37+
if err != nil {
38+
return nil, err
39+
}
40+
resp, err := c.Do(req)
41+
if err != nil {
42+
return nil, err
43+
}
44+
if err := Successful(resp); err != nil {
45+
resp.Body.Close()
46+
return nil, err
47+
}
48+
return resp, nil
49+
}
50+
3551
func Post(ctx context.Context, c *http.Client, url string, body io.Reader) (*http.Response, error) {
3652
req, err := http.NewRequestWithContext(ctx, "POST", url, body)
3753
if err != nil {

0 commit comments

Comments
 (0)