fix directory leak leading to out of disk (anchore#161)

* fix tmpDirGenerator chain of responsibility associated with anchore#132 Signed-off-by: Joseph Barnett <[email protected]> Signed-off-by: [email protected] <[email protected]> * reduce log message to debug Signed-off-by: Joseph Barnett <[email protected]> Signed-off-by: [email protected] <[email protected]> * restore global cleanup function Signed-off-by: Alex Goodman <[email protected]> --------- Signed-off-by: Joseph Barnett <[email protected]> Signed-off-by: [email protected] <[email protected]> Signed-off-by: Alex Goodman <[email protected]> Co-authored-by: Alex Goodman <[email protected]>
deepfence · Jun 15, 2023 · 19345b8 · 19345b8
1 parent 7aad86f
commit 19345b8
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 16 deletions.
diff --git a/client.go b/client.go
@@ -169,8 +169,8 @@ func SetBus(b *partybus.Bus) {
 	bus.SetPublisher(b)
 }
 
-// Cleanup deletes all directories created by stereoscope calls. Note: please use image.Image.Cleanup() over this
-// function when possible.
+// Cleanup deletes all directories created by stereoscope calls.
+// Deprecated: please use image.Image.Cleanup() over this.
 func Cleanup() {
 	if err := rootTempDirGenerator.Cleanup(); err != nil {
 		log.Errorf("failed to cleanup tempdir root: %w", err)

diff --git a/pkg/image/docker/tarball_provider.go b/pkg/image/docker/tarball_provider.go
@@ -82,5 +82,5 @@ func (p *TarballImageProvider) Provide(_ context.Context, userMetadata ...image.
 		return nil, err
 	}
 
-	return image.New(img, contentTempDir, metadata...), nil
+	return image.New(img, p.tmpDirGen, contentTempDir, metadata...), nil
 }
diff --git a/pkg/image/image.go b/pkg/image/image.go
@@ -6,6 +6,7 @@ import (
 	"io"
 	"os"
 
+	"github.com/hashicorp/go-multierror"
 	"github.com/scylladb/go-set/strset"
 
 	"github.com/anchore/stereoscope/internal/bus"
@@ -23,6 +24,9 @@ import (
 type Image struct {
 	// image is the raw image metadata and content provider from the GCR lib
 	image v1.Image
+	// tmpDirGen is a dir generator used by Providers. Multiple directories may
+	// be created and cleanup must use this to prevent polluting the disk
+	tmpDirGen *file.TempDirGenerator
 	// contentCacheDir is where all layer tar cache is stored.
 	contentCacheDir string
 	// Metadata contains select image attributes
@@ -131,14 +135,15 @@ func WithOS(o string) AdditionalMetadata {
 
 // NewImage provides a new (unread) image object.
 // Deprecated: use New() instead
-func NewImage(image v1.Image, contentCacheDir string, additionalMetadata ...AdditionalMetadata) *Image {
-	return New(image, contentCacheDir, additionalMetadata...)
+func NewImage(image v1.Image, tmpDirGen *file.TempDirGenerator, contentCacheDir string, additionalMetadata ...AdditionalMetadata) *Image {
+	return New(image, tmpDirGen, contentCacheDir, additionalMetadata...)
 }
 
 // New provides a new (unread) image object.
-func New(image v1.Image, contentCacheDir string, additionalMetadata ...AdditionalMetadata) *Image {
+func New(image v1.Image, tmpDirGen *file.TempDirGenerator, contentCacheDir string, additionalMetadata ...AdditionalMetadata) *Image {
 	imgObj := &Image{
 		image:            image,
+		tmpDirGen:        tmpDirGen,
 		contentCacheDir:  contentCacheDir,
 		overrideMetadata: additionalMetadata,
 	}
@@ -341,10 +346,19 @@ func (i *Image) Cleanup() error {
 	if i == nil {
 		return nil
 	}
-	if i.contentCacheDir != "" {
-		if err := os.RemoveAll(i.contentCacheDir); err != nil {
-			return err
+	var errs error
+	if i.tmpDirGen != nil {
+		if err := i.tmpDirGen.Cleanup(); err != nil {
+			errs = multierror.Append(errs, err)
+		}
+
+		if i.contentCacheDir != "" {
+			if _, err := os.Stat(i.contentCacheDir); !os.IsNotExist(err) {
+				if err := os.RemoveAll(i.contentCacheDir); err != nil {
+					errs = multierror.Append(errs, err)
+				}
+			}
 		}
 	}
-	return nil
+	return errs
 }
diff --git a/pkg/image/image_test.go b/pkg/image/image_test.go
@@ -3,11 +3,12 @@ package image
 import (
 	"crypto/sha256"
 	"fmt"
-	"github.com/google/go-cmp/cmp"
-	"github.com/google/go-cmp/cmp/cmpopts"
 	"os"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+
 	"github.com/google/go-containerregistry/pkg/name"
 )
 
@@ -98,7 +99,7 @@ func TestImageAdditionalMetadata(t *testing.T) {
 				os.Remove(tempFile.Name())
 			})
 
-			img := New(nil, tempFile.Name(), test.options...)
+			img := New(nil, nil, tempFile.Name(), test.options...)
 
 			err = img.applyOverrideMetadata()
 			if err != nil {

diff --git a/pkg/image/oci/directory_provider.go b/pkg/image/oci/directory_provider.go
@@ -69,5 +69,5 @@ func (p *DirectoryImageProvider) Provide(_ context.Context, userMetadata ...imag
 		return nil, err
 	}
 
-	return image.New(img, contentTempDir, metadata...), nil
+	return image.New(img, p.tmpDirGen, contentTempDir, metadata...), nil
 }
diff --git a/pkg/image/oci/registry_provider.go b/pkg/image/oci/registry_provider.go
@@ -80,7 +80,7 @@ func (p *RegistryImageProvider) Provide(ctx context.Context, userMetadata ...ima
 	// apply user-supplied metadata last to override any default behavior
 	metadata = append(metadata, userMetadata...)
 
-	return image.New(img, imageTempDir, metadata...), nil
+	return image.New(img, p.tmpDirGen, imageTempDir, metadata...), nil
 }
 
 func prepareReferenceOptions(registryOptions image.RegistryOptions) []name.Option {

diff --git a/pkg/image/sif/provider.go b/pkg/image/sif/provider.go
@@ -51,5 +51,5 @@ func (p *SingularityImageProvider) Provide(ctx context.Context, userMetadata ...
 	}
 	metadata = append(metadata, userMetadata...)
 
-	return image.New(ui, contentCacheDir, metadata...), nil
+	return image.New(ui, p.tmpDirGen, contentCacheDir, metadata...), nil
 }