consolidate image layer handling; move fs utils (#1690)

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

Author: joelanford

.golangci.yaml

@@ -66,6 +66,8 @@ linters-settings:
       alias: ctrl
     - pkg: github.com/blang/semver/v4
       alias: bsemver
+    - pkg: "^github.com/operator-framework/operator-controller/internal/util/([^/]+)$"
+      alias: "${1}util"
 
 output:
   formats:

catalogd/cmd/catalogd/main.go

@@ -63,7 +63,7 @@ import (
 	"github.com/operator-framework/operator-controller/catalogd/internal/storage"
 	"github.com/operator-framework/operator-controller/catalogd/internal/version"
 	"github.com/operator-framework/operator-controller/catalogd/internal/webhook"
-	"github.com/operator-framework/operator-controller/internal/fsutil"
+	fsutil "github.com/operator-framework/operator-controller/internal/util/fs"
 )
 
 var (

catalogd/internal/source/containers_image.go

@@ -1,25 +1,18 @@
 package source
 
 import (
-	"archive/tar"
 	"context"
 	"errors"
 	"fmt"
-	"io"
 	"os"
-	"path"
 	"path/filepath"
-	"strings"
 	"time"
 
-	"github.com/containerd/containerd/archive"
 	"github.com/containers/image/v5/copy"
 	"github.com/containers/image/v5/docker"
 	"github.com/containers/image/v5/docker/reference"
 	"github.com/containers/image/v5/manifest"
 	"github.com/containers/image/v5/oci/layout"
-	"github.com/containers/image/v5/pkg/blobinfocache/none"
-	"github.com/containers/image/v5/pkg/compression"
 	"github.com/containers/image/v5/pkg/sysregistriesv2"
 	"github.com/containers/image/v5/signature"
 	"github.com/containers/image/v5/types"
@@ -30,8 +23,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	catalogdv1 "github.com/operator-framework/operator-controller/catalogd/api/v1"
-	"github.com/operator-framework/operator-controller/internal/fsutil"
-	"github.com/operator-framework/operator-controller/internal/rukpak/source"
+	fsutil "github.com/operator-framework/operator-controller/internal/util/fs"
+	imageutil "github.com/operator-framework/operator-controller/internal/util/image"
 )
 
 const ConfigDirLabel = "operators.operatorframework.io.index.configs.v1"
@@ -78,10 +71,14 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, catalog *catalogdv
 	//
 	//////////////////////////////////////////////////////
 	unpackPath := i.unpackPath(catalog.Name, canonicalRef.Digest())
-	if isUnpacked, unpackTime, err := source.IsImageUnpacked(unpackPath); isUnpacked && err == nil {
+	if unpackTime, err := fsutil.GetDirectoryModTime(unpackPath); err == nil {
 		l.Info("image already unpacked", "ref", imgRef.String(), "digest", canonicalRef.Digest().String())
 		return successResult(unpackPath, canonicalRef, unpackTime), nil
-	} else if err != nil {
+	} else if errors.Is(err, fsutil.ErrNotDirectory) {
+		if err := fsutil.DeleteReadOnlyRecursive(unpackPath); err != nil {
+			return nil, err
+		}
+	} else if err != nil && !os.IsNotExist(err) {
 		return nil, fmt.Errorf("error checking image already unpacked: %w", err)
 	}
 
@@ -297,59 +294,11 @@ func (i *ContainersImageRegistry) unpackImage(ctx context.Context, unpackPath st
 		return wrapTerminal(fmt.Errorf("catalog image is missing the required label %q", ConfigDirLabel), specIsCanonical)
 	}
 
-	if err := fsutil.EnsureEmptyDirectory(unpackPath, 0700); err != nil {
-		return fmt.Errorf("error ensuring empty unpack directory: %w", err)
-	}
-	l := log.FromContext(ctx)
-	l.Info("unpacking image", "path", unpackPath)
-	for i, layerInfo := range img.LayerInfos() {
-		if err := func() error {
-			layerReader, _, err := layoutSrc.GetBlob(ctx, layerInfo, none.NoCache)
-			if err != nil {
-				return fmt.Errorf("error getting blob for layer[%d]: %w", i, err)
-			}
-			defer layerReader.Close()
-
-			if err := applyLayer(ctx, unpackPath, dirToUnpack, layerReader); err != nil {
-				return fmt.Errorf("error applying layer[%d]: %w", i, err)
-			}
-			l.Info("applied layer", "layer", i)
-			return nil
-		}(); err != nil {
-			return errors.Join(err, fsutil.DeleteReadOnlyRecursive(unpackPath))
-		}
-	}
-	if err := fsutil.SetReadOnlyRecursive(unpackPath); err != nil {
-		return fmt.Errorf("error making unpack directory read-only: %w", err)
-	}
-	return nil
-}
-
-func applyLayer(ctx context.Context, destPath string, srcPath string, layer io.ReadCloser) error {
-	decompressed, _, err := compression.AutoDecompress(layer)
-	if err != nil {
-		return fmt.Errorf("auto-decompress failed: %w", err)
-	}
-	defer decompressed.Close()
-
-	_, err = archive.Apply(ctx, destPath, decompressed, archive.WithFilter(applyLayerFilter(srcPath)))
-	return err
-}
-
-func applyLayerFilter(srcPath string) archive.Filter {
-	cleanSrcPath := path.Clean(strings.TrimPrefix(srcPath, "/"))
-	return func(h *tar.Header) (bool, error) {
-		h.Uid = os.Getuid()
-		h.Gid = os.Getgid()
-		h.Mode |= 0700
-
-		cleanName := path.Clean(strings.TrimPrefix(h.Name, "/"))
-		relPath, err := filepath.Rel(cleanSrcPath, cleanName)
-		if err != nil {
-			return false, fmt.Errorf("error getting relative path: %w", err)
-		}
-		return relPath != ".." && !strings.HasPrefix(relPath, "../"), nil
-	}
+	applyFilter := imageutil.AllFilters(
+		imageutil.OnlyPath(dirToUnpack),
+		imageutil.ForceOwnershipRWX(),
+	)
+	return imageutil.ApplyLayersToDisk(ctx, unpackPath, img, layoutSrc, applyFilter)
 }
 
 func (i *ContainersImageRegistry) deleteOtherImages(catalogName string, digestToKeep digest.Digest) error {

cmd/operator-controller/main.go

@@ -63,12 +63,12 @@ import (
 	"github.com/operator-framework/operator-controller/internal/controllers"
 	"github.com/operator-framework/operator-controller/internal/features"
 	"github.com/operator-framework/operator-controller/internal/finalizers"
-	"github.com/operator-framework/operator-controller/internal/fsutil"
 	"github.com/operator-framework/operator-controller/internal/httputil"
 	"github.com/operator-framework/operator-controller/internal/resolve"
 	"github.com/operator-framework/operator-controller/internal/rukpak/preflights/crdupgradesafety"
 	"github.com/operator-framework/operator-controller/internal/rukpak/source"
 	"github.com/operator-framework/operator-controller/internal/scheme"
+	fsutil "github.com/operator-framework/operator-controller/internal/util/fs"
 	"github.com/operator-framework/operator-controller/internal/version"
 )
 

internal/rukpak/source/containers_image.go

@@ -1,22 +1,17 @@
 package source
 
 import (
-	"archive/tar"
 	"context"
 	"errors"
 	"fmt"
-	"io"
 	"os"
 	"path/filepath"
 
-	"github.com/containerd/containerd/archive"
 	"github.com/containers/image/v5/copy"
 	"github.com/containers/image/v5/docker"
 	"github.com/containers/image/v5/docker/reference"
 	"github.com/containers/image/v5/manifest"
 	"github.com/containers/image/v5/oci/layout"
-	"github.com/containers/image/v5/pkg/blobinfocache/none"
-	"github.com/containers/image/v5/pkg/compression"
 	"github.com/containers/image/v5/pkg/sysregistriesv2"
 	"github.com/containers/image/v5/signature"
 	"github.com/containers/image/v5/types"
@@ -25,7 +20,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
-	"github.com/operator-framework/operator-controller/internal/fsutil"
+	fsutil "github.com/operator-framework/operator-controller/internal/util/fs"
+	imageutil "github.com/operator-framework/operator-controller/internal/util/image"
 )
 
 var insecurePolicy = []byte(`{"default":[{"type":"insecureAcceptAnything"}]}`)
@@ -71,11 +67,15 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, bundle *BundleSour
 	//
 	//////////////////////////////////////////////////////
 	unpackPath := i.unpackPath(bundle.Name, canonicalRef.Digest())
-	if isUnpacked, _, err := IsImageUnpacked(unpackPath); isUnpacked && err == nil {
+	if _, err := fsutil.GetDirectoryModTime(unpackPath); err == nil {
 		l.Info("image already unpacked", "ref", imgRef.String(), "digest", canonicalRef.Digest().String())
 		return successResult(bundle.Name, unpackPath, canonicalRef), nil
-	} else if err != nil {
-		return nil, fmt.Errorf("error checking bundle already unpacked: %w", err)
+	} else if errors.Is(err, fsutil.ErrNotDirectory) {
+		if err := fsutil.DeleteReadOnlyRecursive(unpackPath); err != nil {
+			return nil, err
+		}
+	} else if err != nil && !os.IsNotExist(err) {
+		return nil, fmt.Errorf("error checking image already unpacked: %w", err)
 	}
 
 	//////////////////////////////////////////////////////
@@ -265,53 +265,7 @@ func (i *ContainersImageRegistry) unpackImage(ctx context.Context, unpackPath st
 			panic(err)
 		}
 	}()
-
-	if err := fsutil.EnsureEmptyDirectory(unpackPath, 0700); err != nil {
-		return fmt.Errorf("error ensuring empty unpack directory: %w", err)
-	}
-	l := log.FromContext(ctx)
-	l.Info("unpacking image", "path", unpackPath)
-	for i, layerInfo := range img.LayerInfos() {
-		if err := func() error {
-			layerReader, _, err := layoutSrc.GetBlob(ctx, layerInfo, none.NoCache)
-			if err != nil {
-				return fmt.Errorf("error getting blob for layer[%d]: %w", i, err)
-			}
-			defer layerReader.Close()
-
-			if err := applyLayer(ctx, unpackPath, layerReader); err != nil {
-				return fmt.Errorf("error applying layer[%d]: %w", i, err)
-			}
-			l.Info("applied layer", "layer", i)
-			return nil
-		}(); err != nil {
-			return errors.Join(err, fsutil.DeleteReadOnlyRecursive(unpackPath))
-		}
-	}
-	if err := fsutil.SetReadOnlyRecursive(unpackPath); err != nil {
-		return fmt.Errorf("error making unpack directory read-only: %w", err)
-	}
-	return nil
-}
-
-func applyLayer(ctx context.Context, unpackPath string, layer io.ReadCloser) error {
-	decompressed, _, err := compression.AutoDecompress(layer)
-	if err != nil {
-		return fmt.Errorf("auto-decompress failed: %w", err)
-	}
-	defer decompressed.Close()
-
-	_, err = archive.Apply(ctx, unpackPath, decompressed, archive.WithFilter(applyLayerFilter()))
-	return err
-}
-
-func applyLayerFilter() archive.Filter {
-	return func(h *tar.Header) (bool, error) {
-		h.Uid = os.Getuid()
-		h.Gid = os.Getgid()
-		h.Mode |= 0700
-		return true, nil
-	}
+	return imageutil.ApplyLayersToDisk(ctx, unpackPath, img, layoutSrc, imageutil.ForceOwnershipRWX())
 }
 
 func (i *ContainersImageRegistry) deleteOtherImages(bundleName string, digestToKeep digest.Digest) error {

internal/rukpak/source/containers_image_test.go

@@ -22,8 +22,8 @@ import (
 	"github.com/stretchr/testify/require"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
-	"github.com/operator-framework/operator-controller/internal/fsutil"
 	"github.com/operator-framework/operator-controller/internal/rukpak/source"
+	fsutil "github.com/operator-framework/operator-controller/internal/util/fs"
 )
 
 const (

internal/rukpak/source/helpers.go

@@ -1,10 +1,12 @@
-package fsutil
+package fs
 
 import (
+	"errors"
 	"fmt"
 	"io/fs"
 	"os"
 	"path/filepath"
+	"time"
 )
 
 // EnsureEmptyDirectory ensures the directory given by `path` is empty.
@@ -42,7 +44,10 @@ func SetWritableRecursive(root string) error {
 	return setModeRecursive(root, ownerWritableFileMode, ownerWritableDirMode)
 }
 
-// DeleteReadOnlyRecursive deletes read-only directory with path given by `root`
+// DeleteReadOnlyRecursive deletes the directory with path given by `root`.
+// Prior to deleting the directory, the directory and all descendant files
+// and directories are set as writable. If any chmod or deletion error occurs
+// it is immediately returned.
 func DeleteReadOnlyRecursive(root string) error {
 	if err := SetWritableRecursive(root); err != nil {
 		return fmt.Errorf("error making directory writable for deletion: %w", err)
@@ -78,3 +83,21 @@ func setModeRecursive(path string, fileMode os.FileMode, dirMode os.FileMode) er
 		}
 	})
 }
+
+var (
+	ErrNotDirectory = errors.New("not a directory")
+)
+
+// GetDirectoryModTime returns the modification time of the directory at dirPath.
+// If stat(dirPath) fails, an error is returned with a zero-value time.Time.
+// If dirPath is not a directory, an ErrNotDirectory error is returned.
+func GetDirectoryModTime(dirPath string) (time.Time, error) {
+	dirStat, err := os.Stat(dirPath)
+	if err != nil {
+		return time.Time{}, err
+	}
+	if !dirStat.IsDir() {
+		return time.Time{}, ErrNotDirectory
+	}
+	return dirStat.ModTime(), nil
+}