diff --git a/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper.go b/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper.go index bda2e5cf4ad..f3a4b2947f3 100644 --- a/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper.go +++ b/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper.go @@ -17,12 +17,14 @@ limitations under the License. package disk import ( + "bytes" + "crypto/sha256" + "fmt" "net/http" "os" "path/filepath" "github.com/gregjones/httpcache" - "github.com/gregjones/httpcache/diskcache" "github.com/peterbourgon/diskv" "k8s.io/klog/v2" ) @@ -41,7 +43,7 @@ func newCacheRoundTripper(cacheDir string, rt http.RoundTripper) http.RoundTripp BasePath: cacheDir, TempDir: filepath.Join(cacheDir, ".diskv-temp"), }) - t := httpcache.NewTransport(diskcache.NewWithDiskv(d)) + t := httpcache.NewTransport(&sumDiskCache{disk: d}) t.Transport = rt return &cacheRoundTripper{rt: t} @@ -63,3 +65,56 @@ func (rt *cacheRoundTripper) CancelRequest(req *http.Request) { } func (rt *cacheRoundTripper) WrappedRoundTripper() http.RoundTripper { return rt.rt.Transport } + +// A sumDiskCache is a cache backend for github.com/gregjones/httpcache. It is +// similar to httpcache's diskcache package, but uses SHA256 sums to ensure +// cache integrity at read time rather than fsyncing each cache entry to +// increase the likelihood they will be persisted at write time. This avoids +// significant performance degradation on MacOS. +// +// See https://github.com/kubernetes/kubernetes/issues/110753 for more. +type sumDiskCache struct { + disk *diskv.Diskv +} + +// Get the requested key from the cache on disk. If Get encounters an error, or +// the returned value is not a SHA256 sum followed by bytes with a matching +// checksum it will return false to indicate a cache miss. +func (c *sumDiskCache) Get(key string) ([]byte, bool) { + b, err := c.disk.Read(sanitize(key)) + if err != nil || len(b) < sha256.Size { + return []byte{}, false + } + + response := b[sha256.Size:] + want := b[:sha256.Size] // The first 32 bytes of the file should be the SHA256 sum. + got := sha256.Sum256(response) + if !bytes.Equal(want, got[:]) { + return []byte{}, false + } + + return response, true +} + +// Set writes the response to a file on disk. The filename will be the SHA256 +// sum of the key. The file will contain a SHA256 sum of the response bytes, +// followed by said response bytes. +func (c *sumDiskCache) Set(key string, response []byte) { + s := sha256.Sum256(response) + _ = c.disk.Write(sanitize(key), append(s[:], response...)) // Nothing we can do with this error. +} + +func (c *sumDiskCache) Delete(key string) { + _ = c.disk.Erase(sanitize(key)) // Nothing we can do with this error. +} + +// Sanitize an httpcache key such that it can be used as a diskv key, which must +// be a valid filename. The httpcache key will either be the requested URL (if +// the request method was GET) or " " for other methods, per the +// httpcache.cacheKey function. +func sanitize(key string) string { + // These keys are not sensitive. We use sha256 to avoid a (potentially + // malicious) collision causing the wrong cache data to be written or + // accessed. + return fmt.Sprintf("%x", sha256.Sum256([]byte(key))) +} diff --git a/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper_test.go b/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper_test.go index 13002c63d22..5f1626c9aad 100644 --- a/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper_test.go +++ b/staging/src/k8s.io/client-go/discovery/cached/disk/round_tripper_test.go @@ -18,6 +18,7 @@ package disk import ( "bytes" + "crypto/sha256" "io/ioutil" "net/http" "net/url" @@ -25,6 +26,7 @@ import ( "path/filepath" "testing" + "github.com/peterbourgon/diskv" "github.com/stretchr/testify/assert" ) @@ -40,6 +42,35 @@ func (rt *testRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) return rt.Response, rt.Err } +func BenchmarkDiskCache(b *testing.B) { + cacheDir, err := ioutil.TempDir("", "cache-rt") + if err != nil { + b.Fatal(err) + } + defer os.RemoveAll(cacheDir) + + d := diskv.New(diskv.Options{ + PathPerm: os.FileMode(0750), + FilePerm: os.FileMode(0660), + BasePath: cacheDir, + TempDir: filepath.Join(cacheDir, ".diskv-temp"), + }) + + k := "localhost:8080/apis/batch/v1.json" + v, err := ioutil.ReadFile("../../testdata/apis/batch/v1.json") + if err != nil { + b.Fatal(err) + } + + c := sumDiskCache{disk: d} + + for n := 0; n < b.N; n++ { + c.Set(k, v) + c.Get(k) + c.Delete(k) + } +} + func TestCacheRoundTripper(t *testing.T) { rt := &testRoundTripper{} cacheDir, err := ioutil.TempDir("", "cache-rt") @@ -145,3 +176,146 @@ func TestCacheRoundTripperPathPerm(t *testing.T) { }) assert.NoError(err) } + +func TestSumDiskCache(t *testing.T) { + assert := assert.New(t) + + // Ensure that we'll return a cache miss if the backing file doesn't exist. + t.Run("NoSuchKey", func(t *testing.T) { + cacheDir, err := ioutil.TempDir("", "cache-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(cacheDir) + d := diskv.New(diskv.Options{BasePath: cacheDir, TempDir: filepath.Join(cacheDir, ".diskv-temp")}) + c := &sumDiskCache{disk: d} + + key := "testing" + + got, ok := c.Get(key) + assert.False(ok) + assert.Equal([]byte{}, got) + }) + + // Ensure that we'll return a cache miss if the backing file is empty. + t.Run("EmptyFile", func(t *testing.T) { + cacheDir, err := ioutil.TempDir("", "cache-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(cacheDir) + d := diskv.New(diskv.Options{BasePath: cacheDir, TempDir: filepath.Join(cacheDir, ".diskv-temp")}) + c := &sumDiskCache{disk: d} + + key := "testing" + + f, err := os.Create(filepath.Join(cacheDir, sanitize(key))) + if err != nil { + t.Fatal(err) + } + f.Close() + + got, ok := c.Get(key) + assert.False(ok) + assert.Equal([]byte{}, got) + }) + + // Ensure that we'll return a cache miss if the backing has an invalid + // checksum. + t.Run("InvalidChecksum", func(t *testing.T) { + cacheDir, err := ioutil.TempDir("", "cache-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(cacheDir) + d := diskv.New(diskv.Options{BasePath: cacheDir, TempDir: filepath.Join(cacheDir, ".diskv-temp")}) + c := &sumDiskCache{disk: d} + + key := "testing" + value := []byte("testing") + mismatchedValue := []byte("testink") + sum := sha256.Sum256(value) + + // Create a file with the sum of 'value' followed by the bytes of + // 'mismatchedValue'. + f, err := os.Create(filepath.Join(cacheDir, sanitize(key))) + if err != nil { + t.Fatal(err) + } + f.Write(sum[:]) + f.Write(mismatchedValue) + f.Close() + + // The mismatched checksum should result in a cache miss. + got, ok := c.Get(key) + assert.False(ok) + assert.Equal([]byte{}, got) + }) + + // Ensure that our disk cache will happily cache over the top of an existing + // value. We depend on this behaviour to recover from corrupted cache + // entries. When Get detects a bad checksum it will return a cache miss. + // This should cause httpcache to fall back to its underlying transport and + // to subsequently cache the new value, overwriting the corrupt one. + t.Run("OverwriteExistingKey", func(t *testing.T) { + cacheDir, err := ioutil.TempDir("", "cache-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(cacheDir) + d := diskv.New(diskv.Options{BasePath: cacheDir, TempDir: filepath.Join(cacheDir, ".diskv-temp")}) + c := &sumDiskCache{disk: d} + + key := "testing" + value := []byte("cool value!") + + // Write a value. + c.Set(key, value) + got, ok := c.Get(key) + + // Ensure we can read back what we wrote. + assert.True(ok) + assert.Equal(value, got) + + differentValue := []byte("I'm different!") + + // Write a different value. + c.Set(key, differentValue) + got, ok = c.Get(key) + + // Ensure we can read back the different value. + assert.True(ok) + assert.Equal(differentValue, got) + }) + + // Ensure that deleting a key does in fact delete it. + t.Run("DeleteKey", func(t *testing.T) { + cacheDir, err := ioutil.TempDir("", "cache-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(cacheDir) + d := diskv.New(diskv.Options{BasePath: cacheDir, TempDir: filepath.Join(cacheDir, ".diskv-temp")}) + c := &sumDiskCache{disk: d} + + key := "testing" + value := []byte("coolValue") + + c.Set(key, value) + + // Ensure we successfully set the value. + got, ok := c.Get(key) + assert.True(ok) + assert.Equal(value, got) + + c.Delete(key) + + // Ensure the value is gone. + got, ok = c.Get(key) + assert.False(ok) + assert.Equal([]byte{}, got) + + // Ensure that deleting a non-existent value is a no-op. + c.Delete(key) + }) +} diff --git a/vendor/github.com/gregjones/httpcache/diskcache/diskcache.go b/vendor/github.com/gregjones/httpcache/diskcache/diskcache.go deleted file mode 100644 index 42e3129d823..00000000000 --- a/vendor/github.com/gregjones/httpcache/diskcache/diskcache.go +++ /dev/null @@ -1,61 +0,0 @@ -// Package diskcache provides an implementation of httpcache.Cache that uses the diskv package -// to supplement an in-memory map with persistent storage -// -package diskcache - -import ( - "bytes" - "crypto/md5" - "encoding/hex" - "github.com/peterbourgon/diskv" - "io" -) - -// Cache is an implementation of httpcache.Cache that supplements the in-memory map with persistent storage -type Cache struct { - d *diskv.Diskv -} - -// Get returns the response corresponding to key if present -func (c *Cache) Get(key string) (resp []byte, ok bool) { - key = keyToFilename(key) - resp, err := c.d.Read(key) - if err != nil { - return []byte{}, false - } - return resp, true -} - -// Set saves a response to the cache as key -func (c *Cache) Set(key string, resp []byte) { - key = keyToFilename(key) - c.d.WriteStream(key, bytes.NewReader(resp), true) -} - -// Delete removes the response with key from the cache -func (c *Cache) Delete(key string) { - key = keyToFilename(key) - c.d.Erase(key) -} - -func keyToFilename(key string) string { - h := md5.New() - io.WriteString(h, key) - return hex.EncodeToString(h.Sum(nil)) -} - -// New returns a new Cache that will store files in basePath -func New(basePath string) *Cache { - return &Cache{ - d: diskv.New(diskv.Options{ - BasePath: basePath, - CacheSizeMax: 100 * 1024 * 1024, // 100MB - }), - } -} - -// NewWithDiskv returns a new Cache using the provided Diskv as underlying -// storage. -func NewWithDiskv(d *diskv.Diskv) *Cache { - return &Cache{d} -} diff --git a/vendor/modules.txt b/vendor/modules.txt index 8af7320d0bc..beeb185afbd 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -507,7 +507,6 @@ github.com/gorilla/websocket # github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 => github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 ## explicit github.com/gregjones/httpcache -github.com/gregjones/httpcache/diskcache # github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 => github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 ## explicit; go 1.14 github.com/grpc-ecosystem/go-grpc-middleware