From 1f2b55a7096b2adedda7caf441b726ff5f03fa16 Mon Sep 17 00:00:00 2001 From: Ben Krieger Date: Mon, 3 Jan 2022 16:21:40 -0500 Subject: [PATCH] Unexport `layout.WriteLayer`; Implement suggested changes for validateLayers and temp file naming --- pkg/v1/layout/write.go | 28 +++++++++++++--------------- pkg/v1/layout/write_test.go | 2 +- pkg/v1/validate/image.go | 27 +++++++++++++++++---------- 3 files changed, 31 insertions(+), 26 deletions(-) diff --git a/pkg/v1/layout/write.go b/pkg/v1/layout/write.go index f27f293401..b8726ebdfb 100644 --- a/pkg/v1/layout/write.go +++ b/pkg/v1/layout/write.go @@ -16,8 +16,6 @@ package layout import ( "bytes" - "crypto/rand" - "encoding/hex" "encoding/json" "io" "io/ioutil" @@ -238,11 +236,12 @@ func (l Path) writeBlob(hash v1.Hash, size int64, r io.ReadCloser, renamer func( return nil } - // If a rename func was provided, use a temporary file suffix + // If a renamer func was provided write to a temporary file + open := func() (*os.File, error) { return os.Create(file) } if renamer != nil { - file += ".tmp" + open = func() (*os.File, error) { return ioutil.TempFile(dir, hash.Hex) } } - w, err := os.Create(file) + w, err := open() if err != nil { return err } @@ -256,31 +255,30 @@ func (l Path) writeBlob(hash v1.Hash, size int64, r io.ReadCloser, renamer func( if err != nil { return err } + renamePath := l.path("blobs", finalHash.Algorithm, finalHash.Hex) + // Always close file before renaming if err := w.Close(); err != nil { return err } - return os.Rename(file, l.path("blobs", finalHash.Algorithm, finalHash.Hex)) + return os.Rename(w.Name(), renamePath) } -// WriteLayer writes the compressed layer to a blob. Unlike WriteBlob it will +// writeLayer writes the compressed layer to a blob. Unlike WriteBlob it will // write to a temporary file (suffixed with .tmp) within the layout until the // compressed reader is fully consumed and written to disk. Also unlike // WriteBlob, it will not skip writing and exit without error when a blob file // exists, but does not have the correct size. (The blob hash is not // considered, because it may be expensive to compute.) -func (l Path) WriteLayer(layer v1.Layer) error { +func (l Path) writeLayer(layer v1.Layer) error { d, err := layer.Digest() if err != nil { // Allow digest errors, since streams may not have calculated the hash - // yet. Instead, use a random value for the digest and require it to be + // yet. Instead, use an empty value, which will be transformed into a + // random file name with `ioutil.TempFile` and the final digest will be // calculated after writing to a temp file and before renaming to the // final path. - var randHash [32]byte - if _, err := rand.Read(randHash[:]); err != nil { - return err - } - d = v1.Hash{Algorithm: "sha256", Hex: hex.EncodeToString(randHash[:])} + d = v1.Hash{Algorithm: "sha256", Hex: ""} } s, err := layer.Size() @@ -332,7 +330,7 @@ func (l Path) WriteImage(img v1.Image) error { for _, layer := range layers { layer := layer g.Go(func() error { - return l.WriteLayer(layer) + return l.writeLayer(layer) }) } if err := g.Wait(); err != nil { diff --git a/pkg/v1/layout/write_test.go b/pkg/v1/layout/write_test.go index ce6fcbed34..9d92b5fd8e 100644 --- a/pkg/v1/layout/write_test.go +++ b/pkg/v1/layout/write_test.go @@ -627,7 +627,7 @@ func TestOverwriteWithWriteLayer(t *testing.T) { } // try writing expected contents with WriteLayer - if err := l.WriteLayer(layer); err != nil { + if err := l.writeLayer(layer); err != nil { t.Fatalf("error attempting to overwrite truncated layer with valid layer; (Path).WriteLayer = %v", err) } diff --git a/pkg/v1/validate/image.go b/pkg/v1/validate/image.go index 0820efec11..94fb767bbc 100644 --- a/pkg/v1/validate/image.go +++ b/pkg/v1/validate/image.go @@ -114,16 +114,6 @@ func validateLayers(img v1.Image, opt ...Option) error { return layersExist(layers) } - cf, err := img.ConfigFile() - if err != nil { - return err - } - - m, err := img.Manifest() - if err != nil { - return err - } - digests := []v1.Hash{} diffids := []v1.Hash{} udiffids := []v1.Hash{} @@ -131,6 +121,13 @@ func validateLayers(img v1.Image, opt ...Option) error { for i, layer := range layers { cl, err := computeLayer(layer) if errors.Is(err, io.ErrUnexpectedEOF) { + // Errored while reading tar content of layer because a header or + // content section was not the correct length. This is most likely + // due to an incomplete download or otherwise interrupted process. + m, err := img.Manifest() + if err != nil { + return fmt.Errorf("undersized layer[%d] content", i) + } return fmt.Errorf("undersized layer[%d] content: Manifest.Layers[%d].Size=%d", i, i, m.Layers[i].Size) } if err != nil { @@ -144,6 +141,16 @@ func validateLayers(img v1.Image, opt ...Option) error { sizes = append(sizes, cl.size) } + cf, err := img.ConfigFile() + if err != nil { + return err + } + + m, err := img.Manifest() + if err != nil { + return err + } + errs := []string{} for i, layer := range layers { digest, err := layer.Digest()