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 a7cea6e7402..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,10 +17,9 @@ limitations under the License. package disk import ( + "bytes" "crypto/sha256" - "encoding/binary" "fmt" - "hash/crc32" "net/http" "os" "path/filepath" @@ -44,7 +43,7 @@ func newCacheRoundTripper(cacheDir string, rt http.RoundTripper) http.RoundTripp BasePath: cacheDir, TempDir: filepath.Join(cacheDir, ".diskv-temp"), }) - t := httpcache.NewTransport(&crcDiskCache{disk: d}) + t := httpcache.NewTransport(&sumDiskCache{disk: d}) t.Transport = rt return &cacheRoundTripper{rt: t} @@ -67,28 +66,30 @@ func (rt *cacheRoundTripper) CancelRequest(req *http.Request) { func (rt *cacheRoundTripper) WrappedRoundTripper() http.RoundTripper { return rt.rt.Transport } -// A crcDiskCache is a cache backend for github.com/gregjones/httpcache. It is -// similar to httpcache's diskcache package, but uses checksums to ensure cache -// integrity rather than fsyncing each cache entry in order to avoid performance -// degradation on MacOS. +// 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 crcDiskCache struct { +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 CRC-32 checksum followed by bytes with a matching +// 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 *crcDiskCache) Get(key string) ([]byte, bool) { +func (c *sumDiskCache) Get(key string) ([]byte, bool) { b, err := c.disk.Read(sanitize(key)) - if err != nil || len(b) < binary.MaxVarintLen32 { + if err != nil || len(b) < sha256.Size { return []byte{}, false } - response := b[binary.MaxVarintLen32:] - sum, _ := binary.Uvarint(b[:binary.MaxVarintLen32]) - if crc32.ChecksumIEEE(response) != uint32(sum) { + 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 } @@ -96,15 +97,14 @@ func (c *crcDiskCache) Get(key string) ([]byte, bool) { } // Set writes the response to a file on disk. The filename will be the SHA256 -// hash of the key. The file will contain the CRC-32 checksum of the response -// bytes, followed by said response bytes. -func (c *crcDiskCache) Set(key string, response []byte) { - sum := make([]byte, binary.MaxVarintLen32) - _ = binary.PutUvarint(sum, uint64(crc32.ChecksumIEEE(response))) - _ = c.disk.Write(sanitize(key), append(sum, response...)) // Nothing we can do with this error. +// 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 *crcDiskCache) Delete(key string) { +func (c *sumDiskCache) Delete(key string) { _ = c.disk.Erase(sanitize(key)) // Nothing we can do with this error. } 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 245d83b6053..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,8 +18,7 @@ package disk import ( "bytes" - "encoding/binary" - "hash/crc32" + "crypto/sha256" "io/ioutil" "net/http" "net/url" @@ -63,7 +62,7 @@ func BenchmarkDiskCache(b *testing.B) { b.Fatal(err) } - c := crcDiskCache{disk: d} + c := sumDiskCache{disk: d} for n := 0; n < b.N; n++ { c.Set(k, v) @@ -178,35 +177,35 @@ func TestCacheRoundTripperPathPerm(t *testing.T) { assert.NoError(err) } -func TestCRCDiskCache(t *testing.T) { +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-crc") + 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")}) - crc := &crcDiskCache{disk: d} + c := &sumDiskCache{disk: d} key := "testing" - got, ok := crc.Get(key) + 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-crc") + 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")}) - crc := &crcDiskCache{disk: d} + c := &sumDiskCache{disk: d} key := "testing" @@ -216,7 +215,7 @@ func TestCRCDiskCache(t *testing.T) { } f.Close() - got, ok := crc.Get(key) + got, ok := c.Get(key) assert.False(ok) assert.Equal([]byte{}, got) }) @@ -224,32 +223,31 @@ func TestCRCDiskCache(t *testing.T) { // 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-crc") + 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")}) - crc := &crcDiskCache{disk: d} + c := &sumDiskCache{disk: d} key := "testing" value := []byte("testing") mismatchedValue := []byte("testink") - sum := make([]byte, binary.MaxVarintLen32) - binary.PutUvarint(sum, uint64(crc32.ChecksumIEEE(value))) + sum := sha256.Sum256(value) - // Create a file with the checksum of 'value' followed by the bytes of + // 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(sum[:]) f.Write(mismatchedValue) f.Close() // The mismatched checksum should result in a cache miss. - got, ok := crc.Get(key) + got, ok := c.Get(key) assert.False(ok) assert.Equal([]byte{}, got) }) @@ -260,20 +258,20 @@ func TestCRCDiskCache(t *testing.T) { // 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-crc") + 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")}) - crc := &crcDiskCache{disk: d} + c := &sumDiskCache{disk: d} key := "testing" value := []byte("cool value!") // Write a value. - crc.Set(key, value) - got, ok := crc.Get(key) + c.Set(key, value) + got, ok := c.Get(key) // Ensure we can read back what we wrote. assert.True(ok) @@ -282,8 +280,8 @@ func TestCRCDiskCache(t *testing.T) { differentValue := []byte("I'm different!") // Write a different value. - crc.Set(key, differentValue) - got, ok = crc.Get(key) + c.Set(key, differentValue) + got, ok = c.Get(key) // Ensure we can read back the different value. assert.True(ok) @@ -292,32 +290,32 @@ func TestCRCDiskCache(t *testing.T) { // Ensure that deleting a key does in fact delete it. t.Run("DeleteKey", func(t *testing.T) { - cacheDir, err := ioutil.TempDir("", "cache-crc") + 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")}) - crc := &crcDiskCache{disk: d} + c := &sumDiskCache{disk: d} key := "testing" value := []byte("coolValue") - crc.Set(key, value) + c.Set(key, value) // Ensure we successfully set the value. - got, ok := crc.Get(key) + got, ok := c.Get(key) assert.True(ok) assert.Equal(value, got) - crc.Delete(key) + c.Delete(key) // Ensure the value is gone. - got, ok = crc.Get(key) + got, ok = c.Get(key) assert.False(ok) assert.Equal([]byte{}, got) // Ensure that deleting a non-existent value is a no-op. - crc.Delete(key) + c.Delete(key) }) }