Merge pull request #303 from anchore/fix-site-packages-dir

Handle site packages based on which egg file is parsed

.golangci.yaml

@@ -31,7 +31,6 @@ linters:
     - misspell
     - nakedret
     - nolintlint
-    - prealloc
     - rowserrcheck
     - scopelint
     - staticcheck
@@ -55,5 +54,6 @@ linters:
 #    - lll         # without a way to specify per-line exception cases, this is not usable
 #    - maligned    # this is an excellent linter, but tricky to optimize and we are not sensitive to memory layout optimizations
 #    - nestif
+#    - prealloc    # following this rule isn't consistently a good idea, as it sometimes forces unnecessary allocations that result in less idiomatic code
 #    - testpackage
-#    - wsl
+#    - wsl

syft/cataloger/python/package_cataloger_test.go

@@ -1,154 +1,28 @@
 package python
 
 import (
-	"fmt"
-	"io"
-	"io/ioutil"
-	"os"
-	"strings"
 	"testing"
 
-	"github.com/anchore/syft/internal/file"
-
 	"github.com/anchore/syft/syft/source"
 
 	"github.com/anchore/syft/syft/pkg"
 	"github.com/go-test/deep"
 )
 
-// TODO: make this generic (based on maps of source.FileData) and make a generic mock to move to the source pkg
-type pythonTestResolverMock struct {
-	metadataReader io.Reader
-	recordReader   io.Reader
-	topLevelReader io.Reader
-	metadataRef    *source.Location
-	recordRef      *source.Location
-	topLevelRef    *source.Location
-	contents       map[source.Location]string
-}
-
-func newTestResolver(metaPath, recordPath, topPath string) *pythonTestResolverMock {
-	metadataReader, err := os.Open(metaPath)
-	if err != nil {
-		panic(fmt.Errorf("failed to open metadata: %+v", err))
-	}
-
-	var recordReader io.Reader
-	if recordPath != "" {
-		recordReader, err = os.Open(recordPath)
-		if err != nil {
-			panic(fmt.Errorf("failed to open record: %+v", err))
-		}
-	}
-
-	var topLevelReader io.Reader
-	if topPath != "" {
-		topLevelReader, err = os.Open(topPath)
-		if err != nil {
-			panic(fmt.Errorf("failed to open top level: %+v", err))
-		}
-	}
-
-	var recordRef *source.Location
-	if recordReader != nil {
-		ref := source.NewLocation("test-fixtures/dist-info/RECORD")
-		recordRef = &ref
-	}
-	var topLevelRef *source.Location
-	if topLevelReader != nil {
-		ref := source.NewLocation("test-fixtures/dist-info/top_level.txt")
-		topLevelRef = &ref
-	}
-	metadataRef := source.NewLocation("test-fixtures/dist-info/METADATA")
-	return &pythonTestResolverMock{
-		recordReader:   recordReader,
-		metadataReader: metadataReader,
-		topLevelReader: topLevelReader,
-		metadataRef:    &metadataRef,
-		recordRef:      recordRef,
-		topLevelRef:    topLevelRef,
-		contents:       make(map[source.Location]string),
-	}
-}
-
-func (r *pythonTestResolverMock) FileContentsByLocation(location source.Location) (string, error) {
-	switch {
-	case r.topLevelRef != nil && location.Path == r.topLevelRef.Path:
-		b, err := ioutil.ReadAll(r.topLevelReader)
-		if err != nil {
-			return "", err
-		}
-		return string(b), nil
-	case location.Path == r.metadataRef.Path:
-		b, err := ioutil.ReadAll(r.metadataReader)
-		if err != nil {
-			return "", err
-		}
-		return string(b), nil
-	case location.Path == r.recordRef.Path:
-		b, err := ioutil.ReadAll(r.recordReader)
-		if err != nil {
-			return "", err
-		}
-		return string(b), nil
-	}
-	return "", fmt.Errorf("invalid value given")
-}
-
-func (r *pythonTestResolverMock) MultipleFileContentsByLocation(locations []source.Location) (map[source.Location]string, error) {
-	var results = make(map[source.Location]string)
-	var err error
-	for _, l := range locations {
-		results[l], err = r.FileContentsByLocation(l)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	return results, nil
-}
-
-func (r *pythonTestResolverMock) FilesByPath(_ ...string) ([]source.Location, error) {
-	return nil, fmt.Errorf("not implemented")
-}
-
-func (r *pythonTestResolverMock) FilesByGlob(patterns ...string) ([]source.Location, error) {
-	var results []source.Location
-	for _, pattern := range patterns {
-		for _, l := range []*source.Location{r.topLevelRef, r.metadataRef, r.recordRef} {
-			if l == nil {
-				continue
-			}
-			if file.GlobMatch(pattern, l.Path) {
-				results = append(results, *l)
-			}
-		}
-	}
-	return results, nil
-}
-func (r *pythonTestResolverMock) RelativeFileByPath(_ source.Location, path string) *source.Location {
-	switch {
-	case strings.Contains(path, "RECORD"):
-		return r.recordRef
-	case strings.Contains(path, "top_level.txt"):
-		return r.topLevelRef
-	default:
-		panic(fmt.Errorf("invalid RelativeFileByPath value given: %q", path))
-	}
-}
-
 func TestPythonPackageWheelCataloger(t *testing.T) {
 	tests := []struct {
-		MetadataFixture string
+		name            string
-		RecordFixture   string
+		fixtures        []string
-		TopLevelFixture string
+		expectedPackage pkg.Package
-		ExpectedPackage pkg.Package
 	}{
 		{
-			MetadataFixture: "test-fixtures/egg-info/PKG-INFO",
+			name: "egg-info directory",
-			RecordFixture:   "test-fixtures/egg-info/RECORD",
+			fixtures: []string{
-			TopLevelFixture: "test-fixtures/egg-info/top_level.txt",
+				"test-fixtures/egg-info/PKG-INFO",
-			ExpectedPackage: pkg.Package{
+				"test-fixtures/egg-info/RECORD",
+				"test-fixtures/egg-info/top_level.txt",
+			},
+			expectedPackage: pkg.Package{
 				Name:         "requests",
 				Version:      "2.22.0",
 				Type:         pkg.PythonPkg,
@@ -177,10 +51,13 @@ func TestPythonPackageWheelCataloger(t *testing.T) {
 			},
 		},
 		{
-			MetadataFixture: "test-fixtures/dist-info/METADATA",
+			name: "dist-info directory",
-			RecordFixture:   "test-fixtures/dist-info/RECORD",
+			fixtures: []string{
-			TopLevelFixture: "test-fixtures/dist-info/top_level.txt",
+				"test-fixtures/dist-info/METADATA",
-			ExpectedPackage: pkg.Package{
+				"test-fixtures/dist-info/RECORD",
+				"test-fixtures/dist-info/top_level.txt",
+			},
+			expectedPackage: pkg.Package{
 				Name:         "Pygments",
 				Version:      "2.6.1",
 				Type:         pkg.PythonPkg,
@@ -210,8 +87,9 @@ func TestPythonPackageWheelCataloger(t *testing.T) {
 		{
 			// in cases where the metadata file is available and the record is not we should still record there is a package
 			// additionally empty top_level.txt files should not result in an error
-			MetadataFixture: "test-fixtures/partial.dist-info/METADATA",
+			name:     "partial dist-info directory",
-			ExpectedPackage: pkg.Package{
+			fixtures: []string{"test-fixtures/partial.dist-info/METADATA"},
+			expectedPackage: pkg.Package{
 				Name:         "Pygments",
 				Version:      "2.6.1",
 				Type:         pkg.PythonPkg,
@@ -231,8 +109,9 @@ func TestPythonPackageWheelCataloger(t *testing.T) {
 			},
 		},
 		{
-			MetadataFixture: "test-fixtures/test.egg-info",
+			name:     "egg-info regular file",
-			ExpectedPackage: pkg.Package{
+			fixtures: []string{"test-fixtures/test.egg-info"},
+			expectedPackage: pkg.Package{
 				Name:         "requests",
 				Version:      "2.22.0",
 				Type:         pkg.PythonPkg,
@@ -254,19 +133,15 @@ func TestPythonPackageWheelCataloger(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		t.Run(test.MetadataFixture, func(t *testing.T) {
+		t.Run(test.name, func(t *testing.T) {
-			resolver := newTestResolver(test.MetadataFixture, test.RecordFixture, test.TopLevelFixture)
+			resolver := source.NewMockResolverForPaths(test.fixtures...)
 
-			// note that the source is the record ref created by the resolver mock... attach the expected values
+			locations, err := resolver.FilesByPath(test.fixtures...)
-			test.ExpectedPackage.Locations = []source.Location{*resolver.metadataRef}
+			if err != nil {
-			if resolver.recordRef != nil {
+				t.Fatal(err)
-				test.ExpectedPackage.Locations = append(test.ExpectedPackage.Locations, *resolver.recordRef)
 			}
 
-			if resolver.topLevelRef != nil {
+			test.expectedPackage.Locations = locations
-				test.ExpectedPackage.Locations = append(test.ExpectedPackage.Locations, *resolver.topLevelRef)
-			}
-			// end patching expected values with runtime data...
 
 			actual, err := NewPythonPackageCataloger().Catalog(resolver)
 			if err != nil {
@@ -274,22 +149,20 @@ func TestPythonPackageWheelCataloger(t *testing.T) {
 			}
 
 			if len(actual) != 1 {
-				t.Fatalf("unexpected length: %d", len(actual))
+				t.Fatalf("unexpected number of packages: %d", len(actual))
 			}
 
-			for _, d := range deep.Equal(actual[0], test.ExpectedPackage) {
+			for _, d := range deep.Equal(actual[0], test.expectedPackage) {
 				t.Errorf("diff: %+v", d)
 			}
 		})
 	}
-
 }
 
 func TestIgnorePackage(t *testing.T) {
 	tests := []struct {
 		MetadataFixture string
 	}{
-
 		{
 			MetadataFixture: "test-fixtures/Python-2.7.egg-info",
 		},
@@ -297,7 +170,7 @@ func TestIgnorePackage(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.MetadataFixture, func(t *testing.T) {
-			resolver := newTestResolver(test.MetadataFixture, "", "")
+			resolver := source.NewMockResolverForPaths(test.MetadataFixture)
 
 			actual, err := NewPythonPackageCataloger().Catalog(resolver)
 			if err != nil {
@@ -309,5 +182,4 @@ func TestIgnorePackage(t *testing.T) {
 			}
 		})
 	}
-
 }

syft/cataloger/python/parse_wheel_egg_metadata.go

@@ -7,6 +7,8 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/anchore/syft/internal/file"
+
 	"github.com/mitchellh/mapstructure"
 
 	"github.com/anchore/syft/syft/pkg"
@@ -70,11 +72,28 @@ func parseWheelOrEggMetadata(path string, reader io.Reader) (pkg.PythonPackageMe
 
 	// add additional metadata not stored in the egg/wheel metadata file
 
-	metadata.SitePackagesRootPath = filepath.Clean(filepath.Join(filepath.Dir(path), ".."))
+	metadata.SitePackagesRootPath = determineSitePackagesRootPath(path)
 
 	return metadata, nil
 }
 
+// isEggRegularFile determines if the specified path is the regular file variant
+// of egg metadata (as opposed to a directory that contains more metadata
+// files).
+func isEggRegularFile(path string) bool {
+	return file.GlobMatch(eggFileMetadataGlob, path)
+}
+
+// determineSitePackagesRootPath returns the path of the site packages root,
+// given the egg metadata file or directory specified in the path.
+func determineSitePackagesRootPath(path string) string {
+	if isEggRegularFile(path) {
+		return filepath.Clean(filepath.Dir(path))
+	}
+
+	return filepath.Clean(filepath.Dir(filepath.Dir(path)))
+}
+
 // handleFieldBodyContinuation returns the updated value for the specified field after processing the specified line.
 // If the continuation cannot be processed, it returns an error.
 func handleFieldBodyContinuation(key, line string, fields map[string]string) (string, error) {

syft/cataloger/python/parse_wheel_egg_metadata_test.go

@@ -56,5 +56,64 @@ func TestParseWheelEggMetadata(t *testing.T) {
 			}
 		})
 	}
-
+}
+
+func TestIsRegularEggFile(t *testing.T) {
+	cases := []struct {
+		path     string
+		expected bool
+	}{
+		{
+			"/usr/lib64/python2.6/site-packages/M2Crypto-0.20.2-py2.6.egg-info",
+			true,
+		},
+		{
+			"/usr/lib64/python2.6/site-packages/M2Crypto-0.20.2-py2.6.egg-info/PKG-INFO",
+			false,
+		},
+		{
+			"/usr/lib64/python2.6/site-packages/M2Crypto-0.20.2-py2.6.dist-info/METADATA",
+			false,
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.path, func(t *testing.T) {
+			actual := isEggRegularFile(c.path)
+
+			if actual != c.expected {
+				t.Errorf("expected %t but got %t", c.expected, actual)
+			}
+		})
+	}
+}
+
+func TestDetermineSitePackagesRootPath(t *testing.T) {
+	cases := []struct {
+		inputPath string
+		expected  string
+	}{
+		{
+			inputPath: "/usr/lib64/python2.6/site-packages/ethtool-0.6-py2.6.egg-info",
+			expected:  "/usr/lib64/python2.6/site-packages",
+		},
+		{
+			inputPath: "/usr/lib/python2.7/dist-packages/configobj-5.0.6.egg-info/top_level.txt",
+			expected:  "/usr/lib/python2.7/dist-packages",
+		},
+		{
+			inputPath: "/usr/lib/python2.7/dist-packages/six-1.10.0.egg-info/PKG-INFO",
+			expected:  "/usr/lib/python2.7/dist-packages",
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.inputPath, func(t *testing.T) {
+			actual := determineSitePackagesRootPath(c.inputPath)
+
+			if actual != c.expected {
+				t.Errorf("expected %s but got %s", c.expected, actual)
+			}
+		})
+	}
 }

syft/source/mock_resolver.go

@@ -0,0 +1,116 @@
+package source
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+
+	"github.com/anchore/syft/internal/file"
+)
+
+var _ Resolver = (*MockResolver)(nil)
+
+// MockResolver implements the Resolver interface and is intended for use *only in test code*.
+// It provides an implementation that can resolve local filesystem paths using only a provided discrete list of file
+// paths, which are typically paths to test fixtures.
+type MockResolver struct {
+	Locations []Location
+}
+
+// NewMockResolverForPaths creates a new MockResolver, where the only resolvable
+// files are those specified by the supplied paths.
+func NewMockResolverForPaths(paths ...string) *MockResolver {
+	var locations []Location
+	for _, p := range paths {
+		locations = append(locations, NewLocation(p))
+	}
+
+	return &MockResolver{Locations: locations}
+}
+
+// String returns the string representation of the MockResolver.
+func (r MockResolver) String() string {
+	return fmt.Sprintf("mock:(%s,...)", r.Locations[0].Path)
+}
+
+// FileContentsByLocation fetches file contents for a single location. If the
+// path does not exist, an error is returned.
+func (r MockResolver) FileContentsByLocation(location Location) (string, error) {
+	for _, l := range r.Locations {
+		if l == location {
+			return stringContent(location.Path)
+		}
+	}
+
+	return "", fmt.Errorf("no file for location: %v", location)
+}
+
+// MultipleFileContentsByLocation returns the file contents for all specified Locations.
+func (r MockResolver) MultipleFileContentsByLocation(locations []Location) (map[Location]string, error) {
+	results := make(map[Location]string)
+	for _, l := range locations {
+		contents, err := r.FileContentsByLocation(l)
+		if err != nil {
+			return nil, err
+		}
+		results[l] = contents
+	}
+
+	return results, nil
+}
+
+// FilesByPath returns all Locations that match the given paths.
+func (r MockResolver) FilesByPath(paths ...string) ([]Location, error) {
+	var results []Location
+	for _, p := range paths {
+		for _, location := range r.Locations {
+			if p == location.Path {
+				results = append(results, NewLocation(p))
+			}
+		}
+	}
+
+	return results, nil
+}
+
+// FilesByGlob returns all Locations that match the given path glob pattern.
+func (r MockResolver) FilesByGlob(patterns ...string) ([]Location, error) {
+	var results []Location
+	for _, pattern := range patterns {
+		for _, location := range r.Locations {
+			if file.GlobMatch(pattern, location.Path) {
+				results = append(results, location)
+			}
+		}
+	}
+
+	return results, nil
+}
+
+// RelativeFileByPath returns a single Location for the given path.
+func (r MockResolver) RelativeFileByPath(_ Location, path string) *Location {
+	paths, err := r.FilesByPath(path)
+	if err != nil {
+		return nil
+	}
+
+	if len(paths) < 1 {
+		return nil
+	}
+
+	return &paths[0]
+}
+
+func stringContent(path string) (string, error) {
+	reader, err := os.Open(path)
+	if err != nil {
+		return "", err
+	}
+
+	b, err := ioutil.ReadAll(reader)
+	if err != nil {
+		return "", err
+	}
+
+	return string(b), nil
+}