From aac0dac0dea61e8864922a4adcc79a6041b4f169 Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Sat, 26 Jun 2021 23:29:25 -0400 Subject: [PATCH] pull in temp dir generator from stereoscope Signed-off-by: Alex Goodman --- cmd/event_loop.go | 11 +++++++---- cmd/packages.go | 12 +++++++----- cmd/power_user.go | 8 ++++++-- go.mod | 2 ++ go.sum | 2 -- internal/temp_dir_generator.go | 5 +++++ syft/lib.go | 11 +++++++++++ syft/pkg/cataloger/java/archive_parser.go | 8 ++++++-- syft/pkg/cataloger/java/archive_parser_test.go | 10 +++++++--- syft/pkg/cataloger/java/save_archive_to_tmp.go | 18 ++++++------------ syft/source/source.go | 18 +++++++++--------- test/integration/catalog_packages_test.go | 5 ++++- test/integration/utils_test.go | 9 +++++++-- 13 files changed, 77 insertions(+), 42 deletions(-) create mode 100644 internal/temp_dir_generator.go diff --git a/cmd/event_loop.go b/cmd/event_loop.go index 4a86f1bae..1f974b089 100644 --- a/cmd/event_loop.go +++ b/cmd/event_loop.go @@ -4,6 +4,7 @@ import ( "errors" "os" + "github.com/anchore/syft/syft" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/internal/ui" "github.com/hashicorp/go-multierror" @@ -50,10 +51,12 @@ func eventLoop(workerErrs <-chan error, signals <-chan os.Signal, subscription * } } case <-signals: - if err := subscription.Unsubscribe(); err != nil { - log.Warnf("unable to unsubscribe from the event bus: %+v", err) - events = nil - } + // ignore further results from any event source and exit ASAP, but ensure that all cache is cleaned up. + // we ignore further errors since cleaning up the tmp directories will affect running catalogers that are + // reading/writing from/to their nested temp dirs. This is acceptable since we are bailing without result. + events = nil + workerErrs = nil + syft.Cleanup() } } diff --git a/cmd/packages.go b/cmd/packages.go index 4fae5c41c..dd6e267b9 100644 --- a/cmd/packages.go +++ b/cmd/packages.go @@ -6,10 +6,6 @@ import ( "io/ioutil" "os" - "github.com/anchore/syft/syft/presenter/packages" - - "github.com/spf13/viper" - "github.com/anchore/syft/internal" "github.com/anchore/syft/internal/anchore" "github.com/anchore/syft/internal/bus" @@ -19,10 +15,12 @@ import ( "github.com/anchore/syft/syft/distro" "github.com/anchore/syft/syft/event" "github.com/anchore/syft/syft/pkg" + "github.com/anchore/syft/syft/presenter/packages" "github.com/anchore/syft/syft/source" "github.com/pkg/profile" "github.com/spf13/cobra" "github.com/spf13/pflag" + "github.com/spf13/viper" "github.com/wagoodman/go-partybus" ) @@ -206,7 +204,11 @@ func packagesExecWorker(userInput string) <-chan error { errs <- fmt.Errorf("failed to determine image source: %+v", err) return } - defer cleanup() + defer func() { + if err := cleanup(); err != nil { + log.Warnf("unable to cleanup source temp dir: %+v", err) + } + }() catalog, d, err := syft.CatalogPackages(src, appConfig.Package.Cataloger.ScopeOpt) if err != nil { diff --git a/cmd/power_user.go b/cmd/power_user.go index a5728fa64..4f8bc9a3f 100644 --- a/cmd/power_user.go +++ b/cmd/power_user.go @@ -5,8 +5,8 @@ import ( "sync" "github.com/anchore/syft/internal" - "github.com/anchore/syft/internal/bus" + "github.com/anchore/syft/internal/log" "github.com/anchore/syft/internal/presenter/poweruser" "github.com/anchore/syft/internal/ui" "github.com/anchore/syft/syft/event" @@ -89,7 +89,11 @@ func powerUserExecWorker(userInput string) <-chan error { errs <- err return } - defer cleanup() + defer func() { + if err := cleanup(); err != nil { + log.Warnf("unable to cleanup source temp dir: %+v", err) + } + }() if src.Metadata.Scheme != source.ImageScheme { errs <- fmt.Errorf("the power-user subcommand only allows for 'image' schemes, given %q", src.Metadata.Scheme) diff --git a/go.mod b/go.mod index 89e6352f8..2a148cb5b 100644 --- a/go.mod +++ b/go.mod @@ -46,3 +46,5 @@ require ( golang.org/x/mod v0.3.0 gopkg.in/yaml.v2 v2.3.0 ) + +replace github.com/anchore/stereoscope => ../stereoscope diff --git a/go.sum b/go.sum index 71d5f646a..772104414 100644 --- a/go.sum +++ b/go.sum @@ -115,8 +115,6 @@ github.com/anchore/go-testutils v0.0.0-20200925183923-d5f45b0d3c04 h1:VzprUTpc0v github.com/anchore/go-testutils v0.0.0-20200925183923-d5f45b0d3c04/go.mod h1:6dK64g27Qi1qGQZ67gFmBFvEHScy0/C8qhQhNe5B5pQ= github.com/anchore/go-version v1.2.2-0.20200701162849-18adb9c92b9b h1:e1bmaoJfZVsCYMrIZBpFxwV26CbsuoEh5muXD5I1Ods= github.com/anchore/go-version v1.2.2-0.20200701162849-18adb9c92b9b/go.mod h1:Bkc+JYWjMCF8OyZ340IMSIi2Ebf3uwByOk6ho4wne1E= -github.com/anchore/stereoscope v0.0.0-20210524175238-3b7662f3a66f h1:bFadyOLOkzME3BrZFZ5m8cf/b2hsn3aMSS9s+SKubRk= -github.com/anchore/stereoscope v0.0.0-20210524175238-3b7662f3a66f/go.mod h1:vhh1M99rfWx5ejMvz1lkQiFZUrC5wu32V12R4JXH+ZI= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8= github.com/antihax/optional v1.0.0 h1:xK2lYat7ZLaVVcIuj82J8kIro4V6kDe0AUDFboUCwcg= github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= diff --git a/internal/temp_dir_generator.go b/internal/temp_dir_generator.go new file mode 100644 index 000000000..173460819 --- /dev/null +++ b/internal/temp_dir_generator.go @@ -0,0 +1,5 @@ +package internal + +import "github.com/anchore/stereoscope/pkg/file" + +var RootTempDirGenerator = file.NewTempDirGenerator(ApplicationName) diff --git a/syft/lib.go b/syft/lib.go index ed9a86ef6..2e9d7027e 100644 --- a/syft/lib.go +++ b/syft/lib.go @@ -19,6 +19,9 @@ package syft import ( "fmt" + "github.com/anchore/stereoscope" + "github.com/anchore/syft/internal" + "github.com/anchore/syft/internal/bus" "github.com/anchore/syft/internal/log" "github.com/anchore/syft/syft/distro" @@ -76,3 +79,11 @@ func SetLogger(logger logger.Logger) { func SetBus(b *partybus.Bus) { bus.SetPublisher(b) } + +func Cleanup() { + stereoscope.Cleanup() + + if err := internal.RootTempDirGenerator.Cleanup(); err != nil { + log.Errorf("failed to cleanup temp directories: %w", err) + } +} diff --git a/syft/pkg/cataloger/java/archive_parser.go b/syft/pkg/cataloger/java/archive_parser.go index a3e3af3e9..cc6efe605 100644 --- a/syft/pkg/cataloger/java/archive_parser.go +++ b/syft/pkg/cataloger/java/archive_parser.go @@ -36,7 +36,11 @@ type archiveParser struct { func parseJavaArchive(virtualPath string, reader io.Reader) ([]pkg.Package, error) { parser, cleanupFn, err := newJavaArchiveParser(virtualPath, reader, true) // note: even on error, we should always run cleanup functions - defer cleanupFn() + defer func() { + if err := cleanupFn(); err != nil { + log.Warnf("unable to clean up java archive temp dir: %+v", err) + } + }() if err != nil { return nil, err } @@ -53,7 +57,7 @@ func uniquePkgKey(p *pkg.Package) string { // newJavaArchiveParser returns a new java archive parser object for the given archive. Can be configured to discover // and parse nested archives or ignore them. -func newJavaArchiveParser(virtualPath string, reader io.Reader, detectNested bool) (*archiveParser, func(), error) { +func newJavaArchiveParser(virtualPath string, reader io.Reader, detectNested bool) (*archiveParser, func() error, error) { contentPath, archivePath, cleanupFn, err := saveArchiveToTmp(reader) if err != nil { return nil, cleanupFn, fmt.Errorf("unable to process java archive: %w", err) diff --git a/syft/pkg/cataloger/java/archive_parser_test.go b/syft/pkg/cataloger/java/archive_parser_test.go index d0453679f..ce75344fe 100644 --- a/syft/pkg/cataloger/java/archive_parser_test.go +++ b/syft/pkg/cataloger/java/archive_parser_test.go @@ -226,7 +226,9 @@ func TestParseJar(t *testing.T) { } parser, cleanupFn, err := newJavaArchiveParser(fixture.Name(), fixture, false) - defer cleanupFn() + t.Cleanup(func() { + assert.NoError(t, cleanupFn()) + }) if err != nil { t.Fatalf("should not have filed... %+v", err) } @@ -845,9 +847,11 @@ func TestPackagesFromPomProperties(t *testing.T) { assert.NoError(t, err) // make the parser - parser, cleanup, err := newJavaArchiveParser(virtualPath, nop, false) + parser, cleanupFn, err := newJavaArchiveParser(virtualPath, nop, false) assert.NoError(t, err) - t.Cleanup(cleanup) + t.Cleanup(func() { + assert.NoError(t, cleanupFn()) + }) // get the test data actualPackage := parser.newPackageFromPomProperties(*test.props, test.parent) diff --git a/syft/pkg/cataloger/java/save_archive_to_tmp.go b/syft/pkg/cataloger/java/save_archive_to_tmp.go index 69d786693..cbaa4a3b4 100644 --- a/syft/pkg/cataloger/java/save_archive_to_tmp.go +++ b/syft/pkg/cataloger/java/save_archive_to_tmp.go @@ -3,25 +3,19 @@ package java import ( "fmt" "io" - "io/ioutil" "os" "path/filepath" - "github.com/anchore/syft/internal/log" + "github.com/anchore/syft/internal" ) -func saveArchiveToTmp(reader io.Reader) (string, string, func(), error) { - tempDir, err := ioutil.TempDir("", "syft-jar-contents-") +func saveArchiveToTmp(reader io.Reader) (string, string, func() error, error) { + generator := internal.RootTempDirGenerator.NewGenerator() + tempDir, err := generator.NewDirectory("java-cataloger-content-cache") if err != nil { - return "", "", func() {}, fmt.Errorf("unable to create tempdir for jar processing: %w", err) - } - - cleanupFn := func() { - err = os.RemoveAll(tempDir) - if err != nil { - log.Errorf("unable to cleanup jar tempdir: %+v", err) - } + return "", "", func() error { return nil }, fmt.Errorf("unable to create tempdir for jar processing: %w", err) } + cleanupFn := generator.Cleanup archivePath := filepath.Join(tempDir, "archive") contentDir := filepath.Join(tempDir, "contents") diff --git a/syft/source/source.go b/syft/source/source.go index 71cb77fd3..683a25212 100644 --- a/syft/source/source.go +++ b/syft/source/source.go @@ -23,33 +23,33 @@ type Source struct { type sourceDetector func(string) (image.Source, string, error) // New produces a Source based on userInput like dir: or image:tag -func New(userInput string, registryOptions *image.RegistryOptions) (Source, func(), error) { +func New(userInput string, registryOptions *image.RegistryOptions) (Source, func() error, error) { fs := afero.NewOsFs() + noCleanupFn := func() error { return nil } parsedScheme, imageSource, location, err := detectScheme(fs, image.DetectSource, userInput) if err != nil { - return Source{}, func() {}, fmt.Errorf("unable to parse input=%q: %w", userInput, err) + return Source{}, noCleanupFn, fmt.Errorf("unable to parse input=%q: %w", userInput, err) } switch parsedScheme { case DirectoryScheme: fileMeta, err := fs.Stat(location) if err != nil { - return Source{}, func() {}, fmt.Errorf("unable to stat dir=%q: %w", location, err) + return Source{}, noCleanupFn, fmt.Errorf("unable to stat dir=%q: %w", location, err) } if !fileMeta.IsDir() { - return Source{}, func() {}, fmt.Errorf("given path is not a directory (path=%q): %w", location, err) + return Source{}, noCleanupFn, fmt.Errorf("given path is not a directory (path=%q): %w", location, err) } s, err := NewFromDirectory(location) if err != nil { - return Source{}, func() {}, fmt.Errorf("could not populate source from path=%q: %w", location, err) + return Source{}, noCleanupFn, fmt.Errorf("could not populate source from path=%q: %w", location, err) } - return s, func() {}, nil + return s, noCleanupFn, nil case ImageScheme: - img, err := stereoscope.GetImageFromSource(location, imageSource, registryOptions) - cleanup := stereoscope.Cleanup + img, cleanup, err := stereoscope.GetImageFromSource(location, imageSource, registryOptions) if err != nil || img == nil { return Source{}, cleanup, fmt.Errorf("could not fetch image '%s': %w", location, err) @@ -62,7 +62,7 @@ func New(userInput string, registryOptions *image.RegistryOptions) (Source, func return s, cleanup, nil } - return Source{}, func() {}, fmt.Errorf("unable to process input for scanning: '%s'", userInput) + return Source{}, noCleanupFn, fmt.Errorf("unable to process input for scanning: '%s'", userInput) } // NewFromDirectory creates a new source object tailored to catalog a given filesystem directory recursively. diff --git a/test/integration/catalog_packages_test.go b/test/integration/catalog_packages_test.go index 118c4ad6f..5c75157fd 100644 --- a/test/integration/catalog_packages_test.go +++ b/test/integration/catalog_packages_test.go @@ -1,6 +1,7 @@ package integration import ( + "github.com/stretchr/testify/assert" "testing" "github.com/anchore/syft/syft/distro" @@ -24,7 +25,9 @@ func BenchmarkImagePackageCatalogers(b *testing.B) { for _, c := range cataloger.ImageCatalogers() { // in case of future alteration where state is persisted, assume no dependency is safe to reuse theSource, cleanupSource, err := source.New("docker-archive:"+tarPath, nil) - b.Cleanup(cleanupSource) + b.Cleanup(func() { + assert.NoError(b, cleanupSource()) + }) if err != nil { b.Fatalf("unable to get source: %+v", err) } diff --git a/test/integration/utils_test.go b/test/integration/utils_test.go index 56cbc7b09..b5753d103 100644 --- a/test/integration/utils_test.go +++ b/test/integration/utils_test.go @@ -1,6 +1,7 @@ package integration import ( + "github.com/stretchr/testify/assert" "testing" "github.com/anchore/stereoscope/pkg/imagetest" @@ -15,7 +16,9 @@ func catalogFixtureImage(t *testing.T, fixtureImageName string) (*pkg.Catalog, * tarPath := imagetest.GetFixtureImageTarPath(t, fixtureImageName) theSource, cleanupSource, err := source.New("docker-archive:"+tarPath, nil) - t.Cleanup(cleanupSource) + t.Cleanup(func() { + assert.NoError(t, cleanupSource()) + }) if err != nil { t.Fatalf("unable to get source: %+v", err) } @@ -30,7 +33,9 @@ func catalogFixtureImage(t *testing.T, fixtureImageName string) (*pkg.Catalog, * func catalogDirectory(t *testing.T, dir string) (*pkg.Catalog, *distro.Distro, source.Source) { theSource, cleanupSource, err := source.New("dir:"+dir, nil) - t.Cleanup(cleanupSource) + t.Cleanup(func() { + assert.NoError(t, cleanupSource()) + }) if err != nil { t.Fatalf("unable to get source: %+v", err) }