Fix panic when pulling OCI-packaged helm chart

go.mod

@@ -98,7 +98,7 @@ require (
 	go.opentelemetry.io/otel/trace v1.19.0 // indirect
 	golang.org/x/mod v0.11.0 // indirect
 	golang.org/x/net v0.23.0 // indirect
-	golang.org/x/oauth2 v0.10.0 // indirect
+	golang.org/x/oauth2 v0.18.0 // indirect
 	golang.org/x/sync v0.3.0 // indirect
 	golang.org/x/sys v0.18.0 // indirect
 	golang.org/x/term v0.18.0 // indirect

go.sum

@@ -306,8 +306,8 @@ golang.org/x/net v0.0.0-20210505024714-0287a6fb4125/go.mod h1:9nx3DQGgdP8bBQD5qx
 golang.org/x/net v0.23.0 h1:7EYJ93RZ9vYSZAIb2x3lnuvqO5zneoD6IvWjuhfxjTs=
 golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
-golang.org/x/oauth2 v0.10.0 h1:zHCpF2Khkwy4mMB4bv0U37YtJdTGW8jI0glAApi0Kh8=
-golang.org/x/oauth2 v0.10.0/go.mod h1:kTpgurOux7LqtuxjuyZa4Gj2gdezIt/jQtGnNFfypQI=
+golang.org/x/oauth2 v0.18.0 h1:09qnuIAgzdx1XplqJvW6CQqMCtGZykZWcXzPMPUusvI=
+golang.org/x/oauth2 v0.18.0/go.mod h1:Wf7knwG0MPoWIMMBgFlEaSUDaKskp0dCfrlJRJXbBi8=
 golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=

pkg/image/image.go

@@ -224,7 +224,7 @@ func (i *Image) Read() error {
 
 	for idx, v1Layer := range v1Layers {
 		layer := NewLayer(v1Layer)
-		err := layer.Read(fileCatalog, i.Metadata, idx, i.contentCacheDir)
+		err := layer.Read(fileCatalog, idx, i.contentCacheDir)
 		if err != nil {
 			return err
 		}

pkg/image/layer.go

@@ -80,24 +80,16 @@ func (l *Layer) uncompressedTarCache(uncompressedLayersCacheDir string) (string,
 
 // Read parses information from the underlying layer tar into this struct. This includes layer metadata, the layer
 // file tree, and the layer squash tree.
-func (l *Layer) Read(catalog *FileCatalog, imgMetadata Metadata, idx int, uncompressedLayersCacheDir string) error {
-	var err error
-	tree := filetree.New()
-	l.Tree = tree
-	l.fileCatalog = catalog
-	l.Metadata, err = newLayerMetadata(imgMetadata, l.layer, idx)
+func (l *Layer) Read(catalog *FileCatalog, idx int, uncompressedLayersCacheDir string) error {
+	mediaType, err := l.layer.MediaType()
 	if err != nil {
 		return err
 	}
+	tree := filetree.New()
+	l.Tree = tree
+	l.fileCatalog = catalog
 
-	log.Debugf("layer metadata: index=%+v digest=%+v mediaType=%+v",
-		l.Metadata.Index,
-		l.Metadata.Digest,
-		l.Metadata.MediaType)
-
-	monitor := trackReadProgress(l.Metadata)
-
-	switch l.Metadata.MediaType {
+	switch mediaType {
 	case types.OCILayer,
 		types.OCIUncompressedLayer,
 		types.OCIRestrictedLayer,
@@ -107,44 +99,84 @@ func (l *Layer) Read(catalog *FileCatalog, imgMetadata Metadata, idx int, uncomp
 		types.DockerForeignLayer,
 		types.DockerUncompressedLayer:
 
-		tarFilePath, err := l.uncompressedTarCache(uncompressedLayersCacheDir)
+		err := l.readStandardImageLayer(idx, uncompressedLayersCacheDir, tree)
 		if err != nil {
 			return err
 		}
-
-		l.indexedContent, err = file.NewTarIndex(
-			tarFilePath,
-			layerTarIndexer(tree, l.fileCatalog, &l.Metadata.Size, l, monitor),
-		)
-		if err != nil {
-			return fmt.Errorf("failed to read layer=%q tar : %w", l.Metadata.Digest, err)
-		}
-
 	case SingularitySquashFSLayer:
-		r, err := l.layer.Uncompressed()
-		if err != nil {
-			return fmt.Errorf("failed to read layer=%q: %w", l.Metadata.Digest, err)
-		}
-		// defer r.Close() // TODO: if we close this here, we can't read file contents after we return.
-
-		// Walk the more efficient walk if we're blessed with an io.ReaderAt.
-		if ra, ok := r.(io.ReaderAt); ok {
-			err = file.WalkSquashFS(ra, squashfsVisitor(tree, l.fileCatalog, &l.Metadata.Size, l, monitor))
-		} else {
-			err = file.WalkSquashFSFromReader(r, squashfsVisitor(tree, l.fileCatalog, &l.Metadata.Size, l, monitor))
-		}
+		err := l.readSingularityImageLayer(idx, tree)
 		if err != nil {
-			return fmt.Errorf("failed to walk layer=%q: %w", l.Metadata.Digest, err)
+			return err
 		}
-
 	default:
-		return fmt.Errorf("unknown layer media type: %+v", l.Metadata.MediaType)
+		return fmt.Errorf("unknown layer media type: %+v", mediaType)
 	}
 
 	l.SearchContext = filetree.NewSearchContext(l.Tree, l.fileCatalog.Index)
 
+	return nil
+}
+
+func (l *Layer) readStandardImageLayer(idx int, uncompressedLayersCacheDir string, tree *filetree.FileTree) error {
+	var err error
+	l.Metadata, err = newLayerMetadata(l.layer, idx)
+	monitor := trackReadProgress(l.Metadata)
+	if err != nil {
+		return err
+	}
+
+	log.Debugf("layer metadata: index=%+v digest=%+v mediaType=%+v",
+		l.Metadata.Index,
+		l.Metadata.Digest,
+		l.Metadata.MediaType)
+
+	tarFilePath, err := l.uncompressedTarCache(uncompressedLayersCacheDir)
+	if err != nil {
+		return err
+	}
+
+	l.indexedContent, err = file.NewTarIndex(
+		tarFilePath,
+		layerTarIndexer(tree, l.fileCatalog, &l.Metadata.Size, l, monitor),
+	)
+	if err != nil {
+		return fmt.Errorf("failed to read layer=%q tar : %w", l.Metadata.Digest, err)
+	}
+
 	monitor.SetCompleted()
+	return nil
+}
 
+func (l *Layer) readSingularityImageLayer(idx int, tree *filetree.FileTree) error {
+	var err error
+	l.Metadata, err = newLayerMetadata(l.layer, idx)
+	if err != nil {
+		return err
+	}
+
+	log.Debugf("layer metadata: index=%+v digest=%+v mediaType=%+v",
+		l.Metadata.Index,
+		l.Metadata.Digest,
+		l.Metadata.MediaType)
+
+	monitor := trackReadProgress(l.Metadata)
+	r, err := l.layer.Uncompressed()
+	if err != nil {
+		return fmt.Errorf("failed to read layer=%q: %w", l.Metadata.Digest, err)
+	}
+	// defer r.Close() // TODO: if we close this here, we can't read file contents after we return.
+
+	// Walk the more efficient walk if we're blessed with an io.ReaderAt.
+	if ra, ok := r.(io.ReaderAt); ok {
+		err = file.WalkSquashFS(ra, squashfsVisitor(tree, l.fileCatalog, &l.Metadata.Size, l, monitor))
+	} else {
+		err = file.WalkSquashFSFromReader(r, squashfsVisitor(tree, l.fileCatalog, &l.Metadata.Size, l, monitor))
+	}
+	if err != nil {
+		return fmt.Errorf("failed to walk layer=%q: %w", l.Metadata.Digest, err)
+	}
+
+	monitor.SetCompleted()
 	return nil
 }
 

pkg/image/layer_metadata.go

@@ -5,7 +5,7 @@ import (
 	v1Types "github.com/google/go-containerregistry/pkg/v1/types"
 )
 
-// Metadata represents container layer metadata.
+// LayerMetadata represents container layer metadata.
 type LayerMetadata struct {
 	Index uint
 	// Digest is the sha256 digest of the layer contents (the docker "diff id")
@@ -16,17 +16,19 @@ type LayerMetadata struct {
 }
 
 // newLayerMetadata aggregates pertinent layer metadata information.
-func newLayerMetadata(imgMetadata Metadata, layer v1.Layer, idx int) (LayerMetadata, error) {
+func newLayerMetadata(layer v1.Layer, idx int) (LayerMetadata, error) {
 	mediaType, err := layer.MediaType()
 	if err != nil {
 		return LayerMetadata{}, err
 	}
+	diffID, err := layer.DiffID()
+	if err != nil {
+		return LayerMetadata{}, err
+	}
 
-	// digest = diff-id = a digest of the uncompressed layer content
-	diffIDHash := imgMetadata.Config.RootFS.DiffIDs[idx]
 	return LayerMetadata{
 		Index:     uint(idx),
-		Digest:    diffIDHash.String(),
+		Digest:    diffID.String(),
 		MediaType: mediaType,
 	}, nil
 }

pkg/image/layer_test.go

@@ -0,0 +1,99 @@
+package image
+
+import (
+	"errors"
+	"io"
+	"strings"
+	"testing"
+
+	v1 "github.com/google/go-containerregistry/pkg/v1"
+	v1Types "github.com/google/go-containerregistry/pkg/v1/types"
+	"github.com/stretchr/testify/require"
+)
+
+type mockLayer struct {
+	mediaType v1Types.MediaType
+	err       error
+}
+
+func (m mockLayer) Digest() (v1.Hash, error) {
+	return v1.Hash{
+		Algorithm: "sha256",
+		Hex:       "aaaaaaaaaa1234",
+	}, nil
+}
+
+func (m mockLayer) DiffID() (v1.Hash, error) {
+	return v1.Hash{
+		Algorithm: "sha256",
+		Hex:       "aaaaaaaaaa1234",
+	}, nil
+}
+
+func (m mockLayer) Compressed() (io.ReadCloser, error) {
+	panic("implement me")
+}
+
+func (m mockLayer) Uncompressed() (io.ReadCloser, error) {
+	return io.NopCloser(strings.NewReader("")), nil
+}
+
+func (m mockLayer) Size() (int64, error) {
+	return 0, nil
+}
+
+func (m mockLayer) MediaType() (v1Types.MediaType, error) {
+	return m.mediaType, m.err
+}
+
+var _ v1.Layer = &mockLayer{}
+
+func fakeLayer(mediaType v1Types.MediaType, err error) v1.Layer {
+	return mockLayer{
+		mediaType: mediaType,
+		err:       err,
+	}
+}
+
+func TestRead(t *testing.T) {
+	tests := []struct {
+		name            string
+		mediaType       v1Types.MediaType
+		mediaTypeErr    error
+		wantErrContents string
+	}{
+		{
+			name:            "unsupported media type",
+			mediaType:       "garbage",
+			mediaTypeErr:    nil,
+			wantErrContents: "unknown layer media type: garbage",
+		},
+		{
+			name:            "unsupported media type: helm chart",
+			mediaType:       "application/vnd.cncf.helm.chart.content.v1.tar+gzip",
+			wantErrContents: "application/vnd.cncf.helm.chart.content.v1.tar+gzip",
+		},
+		{
+			name:            "err on media type returned",
+			mediaTypeErr:    errors.New("no media type for you"),
+			wantErrContents: "no media type for you",
+		},
+		{
+			name:      "no error",
+			mediaType: v1Types.DockerLayer,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			layer := Layer{layer: fakeLayer(tt.mediaType, tt.mediaTypeErr)}
+			catalog := NewFileCatalog()
+			err := layer.Read(catalog, 0, t.TempDir())
+			if tt.wantErrContents != "" {
+				require.ErrorContains(t, err, tt.wantErrContents)
+				return
+			}
+			require.NoError(t, err)
+		})
+	}
+}