Skip to content

Commit

Permalink
Fix panic in legacy tarball writer when config has no history (#569)
Browse files Browse the repository at this point in the history
* Add package comments to fix lint errors

* Fix year

* Fix panic in legacy tarball writer when config has no history

* Address review comments
  • Loading branch information
smukherj1 authored and jonjohnsonjr committed Oct 15, 2019
1 parent 2454386 commit 71da34e
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 2 deletions.
11 changes: 9 additions & 2 deletions pkg/legacy/tarball/write.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,13 @@ func MultiWrite(refToImage map[name.Reference]v1.Image, w io.Writer) error {
if err != nil {
return err
}
history := cfg.History
// Create a blank config history if the config didn't have a history.
if len(history) == 0 && len(layers) != 0 {
history = make([]v1.History, len(layers))
} else if len(layers) != len(history) {
return errors.Errorf("image config had layer history which did not match the number of layers, got len(history)=%d, len(layers)=%d, want len(history)=len(layers)", len(history), len(layers))
}
layerFiles := make([]string, len(layers))
var prev *v1Layer
for i, l := range layers {
Expand All @@ -222,9 +229,9 @@ func MultiWrite(refToImage map[name.Reference]v1.Image, w io.Writer) error {
}
var cur *v1Layer
if i < (len(layers) - 1) {
cur, err = newV1Layer(l, prev, cfg.History[i])
cur, err = newV1Layer(l, prev, history[i])
} else {
cur, err = newTopV1Layer(l, prev, cfg.History[i], cfg, cfgBlob)
cur, err = newTopV1Layer(l, prev, history[i], cfg, cfgBlob)
}
if err != nil {
return err
Expand Down
72 changes: 72 additions & 0 deletions pkg/legacy/tarball/write_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package tarball
import (
"io/ioutil"
"os"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -320,6 +321,77 @@ func TestWriteForeignLayers(t *testing.T) {
}
}

func TestMultiWriteNoHistory(t *testing.T) {
// Make a random image
img, err := random.Image(256, 8)
if err != nil {
t.Fatalf("Error creating random image: %v", err)
}
cfg, err := img.ConfigFile()
if err != nil {
t.Fatalf("Error getting image config: %v", err)
}
// Blank out the layer history.
cfg.History = nil
tag, err := name.NewTag("gcr.io/foo/bar:latest", name.StrictValidation)
if err != nil {
t.Fatalf("Error creating test tag: %v", err)
}
// Make a tempfile for tarball writes.
fp, err := ioutil.TempFile("", "")
if err != nil {
t.Fatalf("Error creating temp file: %v", err)
}
t.Log(fp.Name())
defer fp.Close()
defer os.Remove(fp.Name())
if err := Write(tag, img, fp); err != nil {
t.Fatalf("Unexpected error writing tarball: %v", err)
}
tarImage, err := tarball.ImageFromPath(fp.Name(), &tag)
if err != nil {
t.Fatalf("Unexpected error reading tarball: %v", err)
}
if err := validate.Image(tarImage); err != nil {
t.Fatalf("validate.Image(): %v", err)
}
}

func TestMultiWriteMismatchedHistory(t *testing.T) {
// Make a random image
img, err := random.Image(256, 8)
if err != nil {
t.Fatalf("Error creating random image: %v", err)
}
cfg, err := img.ConfigFile()
if err != nil {
t.Fatalf("Error getting image config: %v", err)
}
// Set the history such that number of history entries != layers. This
// should trigger an error during the image write.
cfg.History = make([]v1.History, 1)
tag, err := name.NewTag("gcr.io/foo/bar:latest", name.StrictValidation)
if err != nil {
t.Fatalf("Error creating test tag: %v", err)
}
// Make a tempfile for tarball writes.
fp, err := ioutil.TempFile("", "")
if err != nil {
t.Fatalf("Error creating temp file: %v", err)
}
t.Log(fp.Name())
defer fp.Close()
defer os.Remove(fp.Name())
err = Write(tag, img, fp)
if err == nil {
t.Fatal("Unexpected success writing tarball, got nil, want error.")
}
want := "image config had layer history which did not match the number of layers"
if !strings.Contains(err.Error(), want) {
t.Errorf("Got unexpected error when writing image with mismatched history & layer, got %v, want substring %q", err, want)
}
}

func assertLayersAreIdentical(t *testing.T, a, b v1.Image) {
t.Helper()

Expand Down

0 comments on commit 71da34e

Please sign in to comment.