Skip to content

Commit afec192

Browse files
committed
Check first header in each layer
In awslabs#1147, we cherry-picked a commit from stargz where we don't make registry calls if layers are fully pulled. This works fine for stargz, but for SOCI, we skip reading the first header for each layer, as we don't need it. This caused a bug where our fetched size never matched our expected size, so this condition was never met. This commit fixes this by reading the initially skipped header. This commit also checks this header to ensure that it is not malformed for any reason. Signed-off-by: David Son <[email protected]>
1 parent cf3f3d9 commit afec192

File tree

6 files changed

+67
-0
lines changed

6 files changed

+67
-0
lines changed

fs/span-manager/span_manager.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/awslabs/soci-snapshotter/cache"
2828
"github.com/awslabs/soci-snapshotter/ztoc"
2929
"github.com/awslabs/soci-snapshotter/ztoc/compression"
30+
"github.com/containerd/log"
3031
"github.com/opencontainers/go-digest"
3132
"golang.org/x/sync/errgroup"
3233
)
@@ -88,12 +89,25 @@ type spanInfo struct {
8889

8990
// New creates a SpanManager with given ztoc and content reader, and builds all
9091
// spans based on the ztoc.
92+
93+
// TODO: return errors/nil objects on failure
9194
func New(ztoc *ztoc.Ztoc, r *io.SectionReader, cache cache.BlobCache, retries int, cacheOpt ...cache.Option) *SpanManager {
9295
index, err := ztoc.Zinfo()
9396
if err != nil {
9497
return nil
9598
}
9699
ztoc.Checkpoints = nil
100+
101+
// We don't need the header anywhere else, but we want to ensure we read every byte from the layer.
102+
// This also serves as a sanity check to make sure the file isn't corrupted.
103+
b := make([]byte, index.StartCompressedOffset(0))
104+
if _, err := r.Read(b); err != nil {
105+
log.L.Errorf("unable to read ztoc header: %v", err)
106+
}
107+
if err := index.VerifyHeader(bytes.NewReader(b)); err != nil {
108+
log.L.Errorf("unable to verify %v header: %v", ztoc.CompressionAlgorithm, err)
109+
}
110+
97111
spans := make([]*span, ztoc.MaxSpanID+1)
98112
m := &SpanManager{
99113
cache: cache,

fs/span-manager/span_manager_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ func TestSpanManager(t *testing.T) {
6060
}
6161

6262
for _, tc := range testCases {
63+
tc := tc
6364
t.Run(tc.name, func(t *testing.T) {
6465
var err error
6566
defer func() {

integration/pull_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"bytes"
3737
"encoding/json"
3838
"fmt"
39+
"math"
3940
"os"
4041
"path/filepath"
4142
"strconv"
@@ -872,3 +873,31 @@ min_layer_size=` + strconv.FormatInt(middleSize, 10) + `
872873

873874
checkFuseMounts(t, sh, layerCount-middleIndex)
874875
}
876+
877+
// This checks if the initial header is properly recorded in the image descriptor on image pull
878+
func TestFullLayerRead(t *testing.T) {
879+
regConfig := newRegistryConfig()
880+
sh, done := newShellWithRegistry(t, regConfig)
881+
defer done()
882+
rebootContainerd(t, sh, getContainerdConfigToml(t, false), "")
883+
884+
containerImage := alpineImage
885+
copyImage(sh, dockerhub(containerImage), regConfig.mirror(containerImage))
886+
indexDigest := buildIndex(sh, regConfig.mirror(containerImage), withMinLayerSize(0), withSpanSize(math.MaxInt64))
887+
// Max span size is used to ensure that the entire image will always be fetched at once.
888+
889+
sh.X(append(imagePullCmd, "--soci-index-digest", indexDigest, regConfig.mirror(containerImage).ref)...)
890+
sh.X(append(runSociCmd, "-d", "--name", t.Name(), regConfig.mirror(containerImage).ref, "sleep", "infinity")...)
891+
jsonFile := sh.O("nerdctl", "exec", t.Name(), "ls", "/.soci-snapshotter")
892+
rawJSON := sh.O("nerdctl", "exec", t.Name(), "cat", "/.soci-snapshotter/"+strings.TrimSpace(string(jsonFile)))
893+
894+
var layers struct {
895+
FetchedPercent float64
896+
}
897+
if err := json.Unmarshal(rawJSON, &layers); err != nil {
898+
t.Fatalf("cannot unmarshal image layer JSON: %v", err)
899+
}
900+
if layers.FetchedPercent != 100 {
901+
t.Fatalf("Expected 100%% fetchedPercent, found %v", layers.FetchedPercent)
902+
}
903+
}

ztoc/compression/gzip_zinfo.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ package compression
2424
import "C"
2525

2626
import (
27+
"compress/gzip"
2728
"fmt"
29+
"io"
2830
"unsafe"
2931
)
3032

@@ -181,6 +183,15 @@ func (i *GzipZinfo) EndUncompressedOffset(spanID SpanID, fileSize Offset) Offset
181183
return i.getUncompressedOffset(spanID + 1)
182184
}
183185

186+
// VerifyHeader checks if the given zinfo has a proper header
187+
func (i *GzipZinfo) VerifyHeader(r io.Reader) error {
188+
gz, err := gzip.NewReader(r)
189+
if gz != nil {
190+
gz.Close()
191+
}
192+
return err
193+
}
194+
184195
// getCompressedOffset wraps `C.get_comp_off` and returns the offset for the span in the compressed stream.
185196
func (i *GzipZinfo) getCompressedOffset(spanID SpanID) Offset {
186197
return Offset(C.get_comp_off(i.cZinfo, C.int(spanID)))

ztoc/compression/tar_zinfo.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package compression
1818

1919
import (
2020
"fmt"
21+
"io"
2122
"os"
2223

2324
zinfo_flatbuffers "github.com/awslabs/soci-snapshotter/ztoc/compression/fbs/zinfo"
@@ -186,6 +187,14 @@ func (i *TarZinfo) EndUncompressedOffset(spanID SpanID, fileSize Offset) Offset
186187
return i.spanIDToOffset(spanID + 1)
187188
}
188189

190+
// VerifyHeader checks if the given zinfo has a proper header
191+
func (i *TarZinfo) VerifyHeader(r io.Reader) error {
192+
// As this is a catch-all for all compression algorithms,
193+
// there's not really a way to verify the header,
194+
// so blindly assume it's correct.
195+
return nil
196+
}
197+
189198
func (i *TarZinfo) spanIDToOffset(spanID SpanID) Offset {
190199
return Offset(i.spanSize * int64(spanID))
191200
}

ztoc/compression/zinfo.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package compression
1818

1919
import (
2020
"fmt"
21+
"io"
2122
)
2223

2324
// Zinfo is the interface for dealing with compressed data efficiently. It chunks
@@ -75,6 +76,8 @@ type Zinfo interface {
7576
// EndUncompressedOffset returns the offset (in uncompressed stream)
7677
// of the last byte belonging to `spanID`. If it's the last span, `fileSize` is returned.
7778
EndUncompressedOffset(spanID SpanID, fileSize Offset) Offset
79+
// VerifyHeader checks if the given zinfo has a proper header
80+
VerifyHeader(r io.Reader) error
7881
}
7982

8083
// NewZinfo deseralizes given zinfo bytes into a zinfo struct.

0 commit comments

Comments
 (0)