fixup: address review comments

pkg/mutate/squashfs.go

@@ -20,10 +20,10 @@ import (
 const layerMediaType types.MediaType = "application/vnd.sylabs.image.layer.v1.squashfs"
 
 type squashfsConverter struct {
-	converter         string   // Path to converter program.
-	args              []string // Arguments required for converter program.
-	dir               string   // Working directory.
-	noConvertWhiteout bool     // Skip default conversion of whiteout markers from AUFS -> OverlayFS
+	converter       string   // Path to converter program.
+	args            []string // Arguments required for converter program.
+	dir             string   // Working directory.
+	convertWhiteout bool     // Convert whiteout markers from AUFS -> OverlayFS
 }
 
 // SquashfsConverterOpt are used to specify squashfs converter options.
@@ -46,11 +46,11 @@ func OptSquashfsLayerConverter(converter string) SquashfsConverterOpt {
 
 var errSquashfsConverterNotSupported = errors.New("squashfs converter not supported")
 
-// OptNoConvertWhiteout is set to skip the default conversion of whiteout /
+// OptSquashfsSkipWhiteoutConversion is set to skip the default conversion of whiteout /
 // opaque markers from AUFS to OverlayFS format.
-func OptNoConvertWhiteout(b bool) SquashfsConverterOpt {
+func OptSquashfsSkipWhiteoutConversion(b bool) SquashfsConverterOpt {
 	return func(c *squashfsConverter) error {
-		c.noConvertWhiteout = b
+		c.convertWhiteout = !b
 		return nil
 	}
 }
@@ -65,13 +65,14 @@ func OptNoConvertWhiteout(b bool) SquashfsConverterOpt {
 //
 // By default, AUFS whiteout markers in the base TAR layer will be converted to OverlayFS whiteout
 // markers in the SquashFS layer. This can be disabled, e.g. where it is known that the layer is
-// part of a squashed image that will not have any whiteouts, using OptNoConvertWhiteout.
+// part of a squashed image that will not have any whiteouts, using OptSquashfsSkipWhiteoutConversion.
 //
 // Note - when whiteout conversion is performed the base layer will be read twice. Callers should
 // ensure it is cached, and is not a streaming layer.
 func SquashfsLayer(base v1.Layer, dir string, opts ...SquashfsConverterOpt) (v1.Layer, error) {
 	c := squashfsConverter{
-		dir: dir,
+		dir:             dir,
+		convertWhiteout: true,
 	}
 
 	for _, opt := range opts {
@@ -141,51 +142,45 @@ func (c *squashfsConverter) makeSquashfs(r io.Reader) (string, error) {
 	return path, nil
 }
 
-// convertWhiteout accepts a Layer l and returns:
-//
-//   - A ReadCloser providing the layer tar, passed through a filter that converts
-//     whiteout markers from AUFS -> OverlayFS, if c.noConvertWhiteout is false.
-//     In this case, any error from the filter will propagate via the returned channel.
-//   - A ReadCloser providing the layer tar directly, if c.noConvertWhiteout is true.
-//
-// Note that when conversion is performed, the layer is read twice.
-func (c *squashfsConverter) convertWhiteout(l v1.Layer) (io.ReadCloser, chan error, error) {
+// Uncompressed returns an io.ReadCloser for the uncompressed layer contents. If
+// c.convertWhiteout is true it will convert whiteout markers from AUFS ->
+// OverlayFS format. Note that when conversion is performed, the underlying
+// layer TAR is read twice.
+func (c *squashfsConverter) Uncompressed(l v1.Layer) (io.ReadCloser, error) {
 	rc, err := l.Uncompressed()
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	// No conversion - direct tar stream from the layer.
-	if c.noConvertWhiteout {
-		return rc, nil, nil
+	if !c.convertWhiteout {
+		return rc, nil
 	}
 
 	// Conversion - first, scan for opaque directories and presence of file
 	// whiteout markers.
 	opaquePaths, fileWhiteout, err := scanAUFSWhiteouts(rc)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 	rc.Close()
 
 	rc, err = l.Uncompressed()
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	// Nothing found to filter
 	if len(opaquePaths) == 0 && !fileWhiteout {
-		return rc, nil, nil
+		return rc, nil
 	}
 
-	filterErr := make(chan error, 1)
 	pr, pw := io.Pipe()
 	go func() {
 		defer rc.Close()
-		filterErr <- whiteoutFilter(rc, pw, opaquePaths)
-		close(filterErr)
+		pw.CloseWithError(whiteoutFilter(rc, pw, opaquePaths))
 	}()
-	return pr, filterErr, nil
+	return pr, nil
 }
 
 type squashfsLayer struct {
@@ -234,7 +229,7 @@ func (l *squashfsLayer) populate() error {
 		return nil
 	}
 
-	rc, filterErr, err := l.converter.convertWhiteout(l.base)
+	rc, err := l.converter.Uncompressed(l.base)
 	if err != nil {
 		return err
 	}
@@ -245,12 +240,6 @@ func (l *squashfsLayer) populate() error {
 		return err
 	}
 
-	if filterErr != nil {
-		if err = <-filterErr; err != nil {
-			return err
-		}
-	}
-
 	f, err := os.Open(path)
 	if err != nil {
 		return err

pkg/mutate/squashfs_test.go

@@ -78,7 +78,7 @@ func Test_SquashfsLayer(t *testing.T) {
 			noConvertWhiteout: false,
 		},
 		{
-			name: "HelloWorldBlob_sqfstar_noConvertWhiteout",
+			name: "HelloWorldBlob_sqfstar_SkipWhiteoutConversion",
 			layer: testLayer(t, "hello-world-docker-v2-manifest", v1.Hash{
 				Algorithm: "sha256",
 				Hex:       "7050e35b49f5e348c4809f5eff915842962cb813f32062d3bbdd35c750dd7d01",
@@ -90,7 +90,7 @@ func Test_SquashfsLayer(t *testing.T) {
 			diffArgs: []string{"--no-owner"},
 		},
 		{
-			name: "HelloWorldBlob_tar2sqfs_noConvertWhiteout",
+			name: "HelloWorldBlob_tar2sqfs_SkipWhiteoutConversion",
 			layer: testLayer(t, "hello-world-docker-v2-manifest", v1.Hash{
 				Algorithm: "sha256",
 				Hex:       "7050e35b49f5e348c4809f5eff915842962cb813f32062d3bbdd35c750dd7d01",
@@ -138,7 +138,7 @@ func Test_SquashfsLayer(t *testing.T) {
 			noConvertWhiteout: false,
 		},
 		{
-			name: "AUFSBlob_sqfstar_noConvertWhiteout",
+			name: "AUFSBlob_sqfstar_SkipWhiteoutConversion",
 			layer: testLayer(t, "aufs-docker-v2-manifest", v1.Hash{
 				Algorithm: "sha256",
 				Hex:       "da55812559dec81445c289c3832cee4a2f725b15aeb258791640185c3126b2bf",
@@ -150,7 +150,7 @@ func Test_SquashfsLayer(t *testing.T) {
 			diffArgs: []string{"--no-owner"},
 		},
 		{
-			name: "AUFSBlob_tar2sqfs_noConvertWhiteout",
+			name: "AUFSBlob_tar2sqfs_SkipWhiteoutConversion",
 			layer: testLayer(t, "aufs-docker-v2-manifest", v1.Hash{
 				Algorithm: "sha256",
 				Hex:       "da55812559dec81445c289c3832cee4a2f725b15aeb258791640185c3126b2bf",
@@ -169,7 +169,7 @@ func Test_SquashfsLayer(t *testing.T) {
 
 			l, err := SquashfsLayer(tt.layer, t.TempDir(),
 				OptSquashfsLayerConverter(tt.converter),
-				OptNoConvertWhiteout(tt.noConvertWhiteout),
+				OptSquashfsSkipWhiteoutConversion(tt.noConvertWhiteout),
 			)
 			if err != nil {
 				t.Fatal(err)

pkg/mutate/whiteout.go

@@ -56,10 +56,9 @@ func scanAUFSWhiteouts(in io.Reader) (map[string]bool, bool, error) {
 // markers with OverlayFS whiteout markers. Due to unrestricted ordering of
 // markers vs their target, the list of opaquePaths must be obtained prior to
 // filtering and provided to this filter.
-func whiteoutFilter(in io.ReadCloser, out io.WriteCloser, opaquePaths map[string]bool) error {
+func whiteoutFilter(in io.Reader, out io.Writer, opaquePaths map[string]bool) error {
 	tr := tar.NewReader(in)
 	tw := tar.NewWriter(out)
-	defer out.Close()
 	defer tw.Close()
 
 	for {