Make imagereader work for all layer paths

Problem:
Docker manifests can define the location of the tarball outside the
root directory of the tarball. For those manifests, the image reader
fails to pull all the required data

Solution:
If the manifest.json defines the location outside that root dir, we need
to traverse the reader past the root files to read all files

Author: kevinrizza

pkg/containertools/containertoolsfakes/fake_image_reader.go

@@ -0,0 +1,14 @@
+package containertools
+
+type GetImageDataOptions struct {
+	WorkingDir string
+}
+
+type GetImageDataOption func(*GetImageDataOptions)
+
+
+func WithWorkingDir(workingDir string) GetImageDataOption {
+	return func(o *GetImageDataOptions) {
+        o.WorkingDir = workingDir
+    }
+}

pkg/containertools/getimagedata_options.go

@@ -25,24 +25,29 @@ type imageManifest struct {
 }
 
 type ImageReader interface {
-	GetImageData(string, string) error
+	GetImageData(string, string, ...GetImageDataOption) error
 }
 
 type ImageLayerReader struct {
-	Cmd CommandRunner
+	Cmd    CommandRunner
 	Logger *logrus.Entry
 }
 
 func NewImageReader(containerTool string, logger *logrus.Entry) ImageReader {
 	cmd := NewCommandRunner(containerTool, logger)
 
 	return &ImageLayerReader{
-		Cmd: cmd,
+		Cmd:    cmd,
 		Logger: logger,
 	}
 }
 
-func (b ImageLayerReader) GetImageData(image, outputDir string) error {
+func (b ImageLayerReader) GetImageData(image, outputDir string, opts ...GetImageDataOption) error {
+	options := GetImageDataOptions{}
+	for _, o := range opts {
+		o(&options)
+	}
+
 	// Create the output directory if it doesn't exist
 	if _, err := os.Stat(outputDir); os.IsNotExist(err) {
 		os.Mkdir(outputDir, 0777)
@@ -53,11 +58,14 @@ func (b ImageLayerReader) GetImageData(image, outputDir string) error {
 		return err
 	}
 
-	workingDir, err := ioutil.TempDir("./", "bundle_staging_")
-	if err != nil {
-		return err
+	workingDir := options.WorkingDir
+	if workingDir == "" {
+		workingDir, err := ioutil.TempDir("./", "bundle_staging_")
+		if err != nil {
+			return err
+		}
+		defer os.RemoveAll(workingDir)
 	}
-	defer os.RemoveAll(workingDir)
 
 	rootTarfile := filepath.Join(workingDir, "bundle.tar")
 
@@ -77,7 +85,7 @@ func (b ImageLayerReader) GetImageData(image, outputDir string) error {
 	if err != nil {
 		return err
 	}
-	
+
 	// Untar the image layer tarballs and push the bundle manifests to the output directory
 	for _, tarball := range layerTarballs {
 		f, err = os.Open(rootTarfile)
@@ -143,18 +151,16 @@ func extractBundleManifests(layerTarball, outputDir string, tarReader *tar.Reade
 
 		if header.Typeflag == tar.TypeReg {
 			if header.Name == layerTarball {
-				// Found the embedded top layer tarball
+				// Found the embedded tarball for the layer
 				layerReader := tar.NewReader(tarReader)
 
 				err = extractTarballToDir(outputDir, layerReader)
 				if err != nil {
 					return err
 				}
-			}
 
-			continue
-		} else {
-			return nil
+				return nil
+			}
 		}
 	}
 }

pkg/containertools/imagereader.go

@@ -0,0 +1,145 @@
+package containertools_test
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/operator-framework/operator-registry/pkg/containertools"
+	"github.com/operator-framework/operator-registry/pkg/containertools/containertoolsfakes"
+
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	expectedFilePath = "testdata/expected_unpack"
+)
+
+func TestReadImageLayersDocker(t *testing.T) {
+	image := "quay.io/operator-framework/example"
+	testWorkingDir := "testdata/docker"
+	testOutputDir := "testdata/output"
+
+	expectedFiles, err := helperGetExpectedFiles()
+
+	logger := logrus.NewEntry(logrus.New())
+	mockCmd := containertoolsfakes.FakeCommandRunner{}
+
+	mockCmd.PullReturns(nil)
+	mockCmd.SaveReturns(nil)
+
+	imageReader := containertools.ImageLayerReader{
+		Cmd:    &mockCmd,
+		Logger: logger,
+	}
+
+	err = imageReader.GetImageData(image, testOutputDir, containertools.WithWorkingDir(testWorkingDir))
+	require.NoError(t, err)
+
+	for _, file := range expectedFiles {
+		expectedFilePath := filepath.Join(expectedFilePath, file)
+		expectedFile, err := ioutil.ReadFile(expectedFilePath)
+		require.NoError(t, err)
+
+		actualFilePath := filepath.Join(testOutputDir, file)
+		actualFile, err := ioutil.ReadFile(actualFilePath)
+		require.NoError(t, err)
+
+		require.Equal(t, string(expectedFile), string(actualFile))
+	}
+
+	err = os.RemoveAll(testOutputDir)
+	require.NoError(t, err)
+}
+
+func TestReadImageLayersPodman(t *testing.T) {
+	image := "quay.io/operator-framework/example"
+	testWorkingDir := "testdata/podman"
+	testOutputDir := "testdata/output"
+
+	expectedFiles, err := helperGetExpectedFiles()
+
+	logger := logrus.NewEntry(logrus.New())
+	mockCmd := containertoolsfakes.FakeCommandRunner{}
+
+	mockCmd.PullReturns(nil)
+	mockCmd.SaveReturns(nil)
+
+	imageReader := containertools.ImageLayerReader{
+		Cmd:    &mockCmd,
+		Logger: logger,
+	}
+
+	err = imageReader.GetImageData(image, testOutputDir, containertools.WithWorkingDir(testWorkingDir))
+	require.NoError(t, err)
+
+	for _, file := range expectedFiles {
+		expectedFilePath := filepath.Join(expectedFilePath, file)
+		expectedFile, err := ioutil.ReadFile(expectedFilePath)
+		require.NoError(t, err)
+
+		actualFilePath := filepath.Join(testOutputDir, file)
+		actualFile, err := ioutil.ReadFile(actualFilePath)
+		require.NoError(t, err)
+
+		require.Equal(t, string(expectedFile), string(actualFile))
+	}
+
+	err = os.RemoveAll(testOutputDir)
+	require.NoError(t, err)
+}
+
+func TestReadImageLayers_PullError(t *testing.T) {
+	image := "quay.io/operator-framework/example"
+	testOutputDir := "testdata/output"
+
+	logger := logrus.NewEntry(logrus.New())
+	mockCmd := containertoolsfakes.FakeCommandRunner{}
+
+	mockCmd.PullReturns(fmt.Errorf("Unable to pull image"))
+
+	imageReader := containertools.ImageLayerReader{
+		Cmd:    &mockCmd,
+		Logger: logger,
+	}
+
+	err := imageReader.GetImageData(image, testOutputDir)
+	require.Error(t, err)
+}
+
+func TestReadImageLayers_SaveError(t *testing.T) {
+	image := "quay.io/operator-framework/example"
+	testOutputDir := "testdata/output"
+
+	logger := logrus.NewEntry(logrus.New())
+	mockCmd := containertoolsfakes.FakeCommandRunner{}
+
+	mockCmd.PullReturns(nil)
+	mockCmd.SaveReturns(fmt.Errorf("Unable to save image"))
+
+	imageReader := containertools.ImageLayerReader{
+		Cmd:    &mockCmd,
+		Logger: logger,
+	}
+
+	err := imageReader.GetImageData(image, testOutputDir)
+	require.Error(t, err)
+}
+
+func helperGetExpectedFiles() ([]string, error) {
+	var files []string
+
+	err := filepath.Walk(expectedFilePath, func(path string, f os.FileInfo, err error) error {
+		if !f.IsDir() {
+			fileName := strings.Replace(path, expectedFilePath, "", -1)
+			files = append(files, fileName)
+		}
+		return nil
+	})
+
+	return files, err
+}