Fix Footer to make it compatible with RFC1952

This commit reflects the recent change on eStargz's footer format for making it
compatible with RFC1952[0].

Since this commit the footer is the following 51 bytes:

```
10 bytes  gzip header
2  bytes  XLEN (length of Extra field) = 26 (4 bytes header + 16 hex digits + len("STARGZ"))
2  bytes  Extra: SI1 = 'S', SI2 = 'G'
2  bytes  Extra: LEN = 22 (16 hex digits + len("STARGZ"))
22 bytes  Extra: subfield = fmt.Sprintf("%016xSTARGZ", offsetOfTOC)
5  bytes  flate header
8  bytes  gzip footer
(End of the eStargz blob)
```

According to RFC1952, Extra field is defined as the following (quoted from [1]):

```
  If the FLG.FEXTRA bit is set, an "extra field" is present in
  the header, with total length XLEN bytes.  It consists of a
  series of subfields, each of the form:

     +---+---+---+---+==================================+
     |SI1|SI2|  LEN  |... LEN bytes of subfield data ...|
     +---+---+---+---+==================================+

  SI1 and SI2 provide a subfield ID, typically two ASCII letters
  with some mnemonic value.  Jean-Loup Gailly
  <[email protected]> is maintaining a registry of subfield
  IDs; please send him any subfield ID you wish to use.  Subfield
  IDs with SI2 = 0 are reserved for future use.
```

This commit adds SI1, SI2 and LEN fields to Extra field, which are lacked in the
stargz. As SI1 and SI2, we use (SI1, SI2) = ('S', 'G') for eStargz footer.
(P.S. ('A', 'P') is used by "Apollo file type information"[1] and ('R', 'A') is
used by dictzip[2])

Notes:
- This commit forks stargz lib for applying this change.
- This change makes eStargz incompatible with stargz (but still compatible with
  tar.gz). Images provided by this commit's `ctr-remote` cannot be lazily pulled
  by the stargz snapshotter before this commit. `--stargz-only` option is also
  deprecated since this commit. Users should use `--no-optimize` option instead.
- As a fallback behaviour, the legacy footer is still supported by Stargz
  Snapshotter plugin (i.e. it can lazily pull eStargz images older than this
  commit).

Some minor changes for making linter happy are also included in this patch.

[0] https://tools.ietf.org/html/rfc1952
[1] https://tools.ietf.org/html/rfc1952#section-2.3.1.1
[2] https://linux.die.net/man/1/dictzip

Signed-off-by: Kohei Tokunaga <[email protected]>

Author: ktock

cmd/ctr-remote/commands/optimize.go

@@ -45,8 +45,8 @@ import (
 	"github.com/containerd/stargz-snapshotter/cmd/ctr-remote/logger"
 	"github.com/containerd/stargz-snapshotter/cmd/ctr-remote/sampler"
 	"github.com/containerd/stargz-snapshotter/estargz"
+	"github.com/containerd/stargz-snapshotter/estargz/stargz"
 	"github.com/containerd/stargz-snapshotter/util/tempfiles"
-	"github.com/google/crfs/stargz"
 	"github.com/google/go-containerregistry/pkg/authn"
 	reglogs "github.com/google/go-containerregistry/pkg/logs"
 	"github.com/google/go-containerregistry/pkg/name"
@@ -77,10 +77,6 @@ var OptimizeCommand = cli.Command{
 			Name:  "plain-http",
 			Usage: "allow HTTP connections to the registry which has the prefix \"http://\"",
 		},
-		cli.BoolFlag{
-			Name:  "stargz-only",
-			Usage: "only stargzify and do not optimize layers",
-		},
 		cli.BoolFlag{
 			Name:  "reuse",
 			Usage: "reuse eStargz (already optimized) layers without further conversion",
@@ -384,27 +380,7 @@ func convertImage(ctx gocontext.Context, clicontext *cli.Context, srcImg regpkg.
 		return nil, errors.Wrap(err, "failed to get image layers")
 	}
 	addendums := make([]mutate.Addendum, len(layers))
-	if clicontext.Bool("stargz-only") {
-		// TODO: enable to reuse layers
-		var eg errgroup.Group
-		var addendumsMu sync.Mutex
-		for i, l := range layers {
-			i, l := i, l
-			eg.Go(func() error {
-				newL, err := buildStargzLayer(l, tf)
-				if err != nil {
-					return err
-				}
-				addendumsMu.Lock()
-				addendums[i] = mutate.Addendum{Layer: newL}
-				addendumsMu.Unlock()
-				return nil
-			})
-		}
-		if err := eg.Wait(); err != nil {
-			return nil, errors.Wrapf(err, "failed to convert layer to stargz")
-		}
-	} else if clicontext.Bool("no-optimize") || !platforms.NewMatcher(platforms.DefaultSpec()).Match(*platform) {
+	if clicontext.Bool("no-optimize") || !platforms.NewMatcher(platforms.DefaultSpec()).Match(*platform) {
 		// Do not run the optimization container if the option requires it or
 		// the source image doesn't match to the platform where this command runs on.
 		log.G(ctx).Warn("Platform mismatch or optimization disabled; converting without optimization")
@@ -703,27 +679,6 @@ func converters(cs ...func() (mutate.Addendum, error)) func() (mutate.Addendum,
 	}
 }
 
-func buildStargzLayer(uncompressed regpkg.Layer, tf *tempfiles.TempFiles) (regpkg.Layer, error) {
-	r, err := uncompressed.Uncompressed()
-	if err != nil {
-		return nil, err
-	}
-	pr, pw := io.Pipe()
-	go func() {
-		w := stargz.NewWriter(pw)
-		if err := w.AppendTar(r); err != nil {
-			pw.CloseWithError(err)
-			return
-		}
-		if err := w.Close(); err != nil {
-			pw.CloseWithError(err)
-			return
-		}
-		pw.Close()
-	}()
-	return newStaticCompressedLayer(pr, tf)
-}
-
 func buildEStargzLayer(uncompressed regpkg.Layer, tf *tempfiles.TempFiles) (regpkg.Layer, ocidigest.Digest, error) {
 	tftmp := tempfiles.NewTempFiles() // Shorter lifetime than tempfiles passed by argument
 	defer tftmp.CleanupAll()

estargz/build.go

@@ -27,9 +27,9 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/containerd/stargz-snapshotter/estargz/stargz"
 	"github.com/containerd/stargz-snapshotter/util/positionwatcher"
 	"github.com/containerd/stargz-snapshotter/util/tempfiles"
-	"github.com/google/crfs/stargz"
 	digest "github.com/opencontainers/go-digest"
 	"github.com/pkg/errors"
 	"golang.org/x/sync/errgroup"

estargz/build_test.go

@@ -29,7 +29,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/google/crfs/stargz"
+	"github.com/containerd/stargz-snapshotter/estargz/stargz"
 )
 
 // TestBuild tests the resulting stargz blob built by this pkg has the same
@@ -367,8 +367,8 @@ func equalEntry(a, b *stargz.TOCEntry) bool {
 		a.Stat().ModTime().Equal(b.Stat().ModTime()) && // modTime     time.Time
 		a.LinkName == b.LinkName &&
 		a.Mode == b.Mode &&
-		a.Uid == b.Uid &&
-		a.Gid == b.Gid &&
+		a.UID == b.UID &&
+		a.GID == b.GID &&
 		a.Uname == b.Uname &&
 		a.Gname == b.Gname &&
 		(a.Offset > 0) == (b.Offset > 0) &&

estargz/parse.go

@@ -23,41 +23,24 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"strconv"
 
-	"github.com/google/crfs/stargz"
+	"github.com/containerd/stargz-snapshotter/estargz/stargz"
 	digest "github.com/opencontainers/go-digest"
 	"github.com/pkg/errors"
 )
 
 // parseStargz parses the footer and TOCJSON of the stargz file
 func parseStargz(sgz *io.SectionReader) (blob *stargzBlob, err error) {
 	// Parse stargz footer and get the offset of TOC JSON
-	if sgz.Size() < stargz.FooterSize {
-		return nil, errors.New("stargz data is too small")
-	}
-	footerReader := io.NewSectionReader(sgz, sgz.Size()-stargz.FooterSize, stargz.FooterSize)
-	zr, err := gzip.NewReader(footerReader)
+	tocOffset, footerSize, err := stargz.OpenFooter(sgz)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to uncompress footer")
-	}
-	defer zr.Close()
-	if len(zr.Header.Extra) != 22 {
-		return nil, errors.Wrap(err, "invalid extra size; must be 22 bytes")
-	} else if string(zr.Header.Extra[16:]) != "STARGZ" {
-		return nil, errors.New("invalid footer; extra must contain magic string \"STARGZ\"")
-	}
-	tocOffset, err := strconv.ParseInt(string(zr.Header.Extra[:16]), 16, 64)
-	if err != nil {
-		return nil, errors.Wrap(err, "invalid footer; failed to get the offset of TOC JSON")
-	} else if tocOffset > sgz.Size() {
-		return nil, fmt.Errorf("invalid footer; offset of TOC JSON is too large (%d > %d)",
-			tocOffset, sgz.Size())
+		return nil, errors.Wrapf(err, "failed to parse footer")
 	}
 
 	// Decode the TOC JSON
-	tocReader := io.NewSectionReader(sgz, tocOffset, sgz.Size()-tocOffset-stargz.FooterSize)
-	if err := zr.Reset(tocReader); err != nil {
+	tocReader := io.NewSectionReader(sgz, tocOffset, sgz.Size()-tocOffset-footerSize)
+	zr, err := gzip.NewReader(tocReader)
+	if err != nil {
 		return nil, errors.Wrap(err, "failed to uncompress TOC JSON targz entry")
 	}
 	tr := tar.NewReader(zr)
@@ -87,7 +70,7 @@ func parseStargz(sgz *io.SectionReader) (blob *stargzBlob, err error) {
 		jtocDigest: dgstr.Digest(),
 		jtocOffset: tocOffset,
 		payload:    io.NewSectionReader(sgz, 0, tocOffset),
-		footer:     footerReader,
+		footer:     bytes.NewReader(stargz.FooterBytes(tocOffset)),
 	}, nil
 }
 
@@ -128,21 +111,10 @@ func combineBlobs(sgz ...*io.SectionReader) (newSgz io.Reader, tocDgst digest.Di
 	if err != nil {
 		return nil, "", err
 	}
-	footerBuf := bytes.NewBuffer(make([]byte, 0, stargz.FooterSize))
-	zw, err := gzip.NewWriterLevel(footerBuf, gzip.BestCompression)
-	if err != nil {
-		return nil, "", err
-	}
-	zw.Extra = []byte(fmt.Sprintf("%016xSTARGZ", currentOffset)) // Extra header indicating the offset of TOCJSON: https://github.com/google/crfs/blob/71d77da419c90be7b05d12e59945ac7a8c94a543/stargz/stargz.go#L46
-	zw.Close()
-	if footerBuf.Len() != stargz.FooterSize {
-		return nil, "", fmt.Errorf("failed to make the footer: invalid size %d; must be %d",
-			footerBuf.Len(), stargz.FooterSize)
-	}
 	return io.MultiReader(
 		io.MultiReader(mpayload...),
 		tocjson,
-		bytes.NewReader(footerBuf.Bytes()),
+		bytes.NewReader(stargz.FooterBytes(currentOffset)),
 	), tocDgst, nil
 }
 
@@ -159,7 +131,6 @@ func marshalTOCJSON(toc *jtoc) (io.Reader, digest.Digest, error) {
 			pw.CloseWithError(err)
 			return
 		}
-		zw.Extra = []byte("stargz.toc") // this magic string might not be necessary but let's follow the official behaviour: https://github.com/google/crfs/blob/71d77da419c90be7b05d12e59945ac7a8c94a543/stargz/stargz.go#L596
 		tw := tar.NewWriter(zw)
 		if err := tw.WriteHeader(&tar.Header{
 			Typeflag: tar.TypeReg,