Skip to content

Commit

Permalink
Clear cache on startup, use tempDir for unpacking (#1669)
Browse files Browse the repository at this point in the history 
Signed-off-by: Per Goncalves da Silva <[email protected]>
  • Loading branch information
@oceanc80
oceanc80 authored Jan 30, 2025
1 parent c5e9a17 commit 3998a3b
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 140 deletions.
5 changes: 3 additions & 2 deletions catalogd/cmd/catalogd/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +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/util"
)

var (
Expand Down Expand Up @@ -257,8 +258,8 @@ func main() {
systemNamespace = podNamespace()
}

if err := os.MkdirAll(cacheDir, 0700); err != nil {
setupLog.Error(err, "unable to create cache directory")
if err := util.EnsureEmptyDirectory(cacheDir, 0700); err != nil {
setupLog.Error(err, "unable to ensure empty cache directory")
os.Exit(1)
}

Expand Down
81 changes: 13 additions & 68 deletions catalogd/internal/source/containers_image.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,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/rukpak/source"
"github.com/operator-framework/operator-controller/internal/util"
)

const ConfigDirLabel = "operators.operatorframework.io.index.configs.v1"
Expand Down Expand Up @@ -76,12 +78,11 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, catalog *catalogdv
//
//////////////////////////////////////////////////////
unpackPath := i.unpackPath(catalog.Name, canonicalRef.Digest())
if unpackStat, err := os.Stat(unpackPath); err == nil {
if !unpackStat.IsDir() {
panic(fmt.Sprintf("unexpected file at unpack path %q: expected a directory", unpackPath))
}
if isUnpacked, unpackTime, err := source.IsImageUnpacked(unpackPath); isUnpacked && err == nil {
l.Info("image already unpacked", "ref", imgRef.String(), "digest", canonicalRef.Digest().String())
return successResult(unpackPath, canonicalRef, unpackStat.ModTime()), nil
return successResult(unpackPath, canonicalRef, unpackTime), nil
} else if err != nil {
return nil, fmt.Errorf("error checking image already unpacked: %w", err)
}

//////////////////////////////////////////////////////
Expand Down Expand Up @@ -154,7 +155,7 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, catalog *catalogdv
//
//////////////////////////////////////////////////////
if err := i.unpackImage(ctx, unpackPath, layoutRef, specIsCanonical, srcCtx); err != nil {
if cleanupErr := deleteRecursive(unpackPath); cleanupErr != nil {
if cleanupErr := source.DeleteReadOnlyRecursive(unpackPath); cleanupErr != nil {
err = errors.Join(err, cleanupErr)
}
return nil, fmt.Errorf("error unpacking image: %w", err)
Expand Down Expand Up @@ -195,7 +196,7 @@ func successResult(unpackPath string, canonicalRef reference.Canonical, lastUnpa
}

func (i *ContainersImageRegistry) Cleanup(_ context.Context, catalog *catalogdv1.ClusterCatalog) error {
if err := deleteRecursive(i.catalogPath(catalog.Name)); err != nil {
if err := source.DeleteReadOnlyRecursive(i.catalogPath(catalog.Name)); err != nil {
return fmt.Errorf("error deleting catalog cache: %w", err)
}
return nil
Expand Down Expand Up @@ -296,8 +297,8 @@ 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 := os.MkdirAll(unpackPath, 0700); err != nil {
return fmt.Errorf("error creating unpack directory: %w", err)
if err := util.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)
Expand All @@ -315,10 +316,10 @@ func (i *ContainersImageRegistry) unpackImage(ctx context.Context, unpackPath st
l.Info("applied layer", "layer", i)
return nil
}(); err != nil {
return errors.Join(err, deleteRecursive(unpackPath))
return errors.Join(err, source.DeleteReadOnlyRecursive(unpackPath))
}
}
if err := setReadOnlyRecursive(unpackPath); err != nil {
if err := source.SetReadOnlyRecursive(unpackPath); err != nil {
return fmt.Errorf("error making unpack directory read-only: %w", err)
}
return nil
Expand Down Expand Up @@ -362,69 +363,13 @@ func (i *ContainersImageRegistry) deleteOtherImages(catalogName string, digestTo
continue
}
imgDirPath := filepath.Join(catalogPath, imgDir.Name())
if err := deleteRecursive(imgDirPath); err != nil {
if err := source.DeleteReadOnlyRecursive(imgDirPath); err != nil {
return fmt.Errorf("error removing image directory: %w", err)
}
}
return nil
}

func setReadOnlyRecursive(root string) error {
if err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error {
if err != nil {
return err
}

fi, err := d.Info()
if err != nil {
return err
}

if err := func() error {
switch typ := fi.Mode().Type(); typ {
case os.ModeSymlink:
// do not follow symlinks
// 1. if they resolve to other locations in the root, we'll find them anyway
// 2. if they resolve to other locations outside the root, we don't want to change their permissions
return nil
case os.ModeDir:
return os.Chmod(path, 0500)
case 0: // regular file
return os.Chmod(path, 0400)
default:
return fmt.Errorf("refusing to change ownership of file %q with type %v", path, typ.String())
}
}(); err != nil {
return err
}
return nil
}); err != nil {
return fmt.Errorf("error making catalog cache read-only: %w", err)
}
return nil
}

func deleteRecursive(root string) error {
if err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error {
if os.IsNotExist(err) {
return nil
}
if err != nil {
return err
}
if !d.IsDir() {
return nil
}
if err := os.Chmod(path, 0700); err != nil {
return err
}
return nil
}); err != nil {
return fmt.Errorf("error making catalog cache writable for deletion: %w", err)
}
return os.RemoveAll(root)
}

func wrapTerminal(err error, isTerminal bool) error {
if !isTerminal {
return err
Expand Down
10 changes: 8 additions & 2 deletions cmd/operator-controller/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import (
"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"
"github.com/operator-framework/operator-controller/internal/util"
"github.com/operator-framework/operator-controller/internal/version"
)

Expand Down Expand Up @@ -297,6 +298,11 @@ func main() {
}
}

if err := util.EnsureEmptyDirectory(cachePath, 0700); err != nil {
setupLog.Error(err, "unable to ensure empty cache directory")
os.Exit(1)
}

unpacker := &source.ContainersImageRegistry{
BaseCachePath: filepath.Join(cachePath, "unpack"),
SourceContextFunc: func(logger logr.Logger) (*types.SystemContext, error) {
Expand Down Expand Up @@ -362,7 +368,7 @@ func main() {
crdupgradesafety.NewPreflight(aeClient.CustomResourceDefinitions()),
}

applier := &applier.Helm{
helmApplier := &applier.Helm{
ActionClientGetter: acg,
Preflights: preflights,
}
Expand All @@ -382,7 +388,7 @@ func main() {
Client: cl,
Resolver: resolver,
Unpacker: unpacker,
Applier: applier,
Applier: helmApplier,
InstalledBundleGetter: &controllers.DefaultInstalledBundleGetter{ActionClientGetter: acg},
Finalizers: clusterExtensionFinalizers,
Manager: cm,
Expand Down
79 changes: 12 additions & 67 deletions internal/rukpak/source/containers_image.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"github.com/opencontainers/go-digest"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/operator-framework/operator-controller/internal/util"
)

var insecurePolicy = []byte(`{"default":[{"type":"insecureAcceptAnything"}]}`)
Expand Down Expand Up @@ -69,12 +71,11 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, bundle *BundleSour
//
//////////////////////////////////////////////////////
unpackPath := i.unpackPath(bundle.Name, canonicalRef.Digest())
if unpackStat, err := os.Stat(unpackPath); err == nil {
if !unpackStat.IsDir() {
panic(fmt.Sprintf("unexpected file at unpack path %q: expected a directory", unpackPath))
}
if isUnpacked, _, err := IsImageUnpacked(unpackPath); isUnpacked && 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)
}

//////////////////////////////////////////////////////
Expand Down Expand Up @@ -147,7 +148,7 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, bundle *BundleSour
//
//////////////////////////////////////////////////////
if err := i.unpackImage(ctx, unpackPath, layoutRef, srcCtx); err != nil {
if cleanupErr := deleteRecursive(unpackPath); cleanupErr != nil {
if cleanupErr := DeleteReadOnlyRecursive(unpackPath); cleanupErr != nil {
err = errors.Join(err, cleanupErr)
}
return nil, fmt.Errorf("error unpacking image: %w", err)
Expand Down Expand Up @@ -175,7 +176,7 @@ func successResult(bundleName, unpackPath string, canonicalRef reference.Canonic
}

func (i *ContainersImageRegistry) Cleanup(_ context.Context, bundle *BundleSource) error {
return deleteRecursive(i.bundlePath(bundle.Name))
return DeleteReadOnlyRecursive(i.bundlePath(bundle.Name))
}

func (i *ContainersImageRegistry) bundlePath(bundleName string) string {
Expand Down Expand Up @@ -265,8 +266,8 @@ func (i *ContainersImageRegistry) unpackImage(ctx context.Context, unpackPath st
}
}()

if err := os.MkdirAll(unpackPath, 0700); err != nil {
return fmt.Errorf("error creating unpack directory: %w", err)
if err := util.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)
Expand All @@ -284,10 +285,10 @@ func (i *ContainersImageRegistry) unpackImage(ctx context.Context, unpackPath st
l.Info("applied layer", "layer", i)
return nil
}(); err != nil {
return errors.Join(err, deleteRecursive(unpackPath))
return errors.Join(err, DeleteReadOnlyRecursive(unpackPath))
}
}
if err := setReadOnlyRecursive(unpackPath); err != nil {
if err := SetReadOnlyRecursive(unpackPath); err != nil {
return fmt.Errorf("error making unpack directory read-only: %w", err)
}
return nil
Expand Down Expand Up @@ -324,65 +325,9 @@ func (i *ContainersImageRegistry) deleteOtherImages(bundleName string, digestToK
continue
}
imgDirPath := filepath.Join(bundlePath, imgDir.Name())
if err := deleteRecursive(imgDirPath); err != nil {
if err := DeleteReadOnlyRecursive(imgDirPath); err != nil {
return fmt.Errorf("error removing image directory: %w", err)
}
}
return nil
}

func setReadOnlyRecursive(root string) error {
if err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error {
if err != nil {
return err
}

fi, err := d.Info()
if err != nil {
return err
}

if err := func() error {
switch typ := fi.Mode().Type(); typ {
case os.ModeSymlink:
// do not follow symlinks
// 1. if they resolve to other locations in the root, we'll find them anyway
// 2. if they resolve to other locations outside the root, we don't want to change their permissions
return nil
case os.ModeDir:
return os.Chmod(path, 0500)
case 0: // regular file
return os.Chmod(path, 0400)
default:
return fmt.Errorf("refusing to change ownership of file %q with type %v", path, typ.String())
}
}(); err != nil {
return err
}
return nil
}); err != nil {
return fmt.Errorf("error making bundle cache read-only: %w", err)
}
return nil
}

func deleteRecursive(root string) error {
if err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error {
if os.IsNotExist(err) {
return nil
}
if err != nil {
return err
}
if !d.IsDir() {
return nil
}
if err := os.Chmod(path, 0700); err != nil {
return err
}
return nil
}); err != nil {
return fmt.Errorf("error making bundle cache writable for deletion: %w", err)
}
return os.RemoveAll(root)
}
11 changes: 10 additions & 1 deletion internal/rukpak/source/containers_image_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,16 @@ func TestUnpackUnexpectedFile(t *testing.T) {
require.NoError(t, os.WriteFile(unpackPath, []byte{}, 0600))

// Attempt to pull and unpack the image
assert.Panics(t, func() { _, _ = unpacker.Unpack(context.Background(), bundleSource) })
_, err := unpacker.Unpack(context.Background(), bundleSource)
require.NoError(t, err)

// Ensure unpack path is now a directory
stat, err := os.Stat(unpackPath)
require.NoError(t, err)
require.True(t, stat.IsDir())

// Unset read-only to allow cleanup
require.NoError(t, source.UnsetReadOnlyRecursive(unpackPath))
}

func TestUnpackCopySucceedsMountFails(t *testing.T) {
Expand Down
Loading

0 comments on commit 3998a3b

Please sign in to comment.