ext: Lister and Detector returns detector info with detected content

1. Every Lister and Detector are versioned
2. detected content, are returned in a map with detector info as the key

ext/featurefmt/apk/apk.go

@@ -29,7 +29,7 @@ import (
 )
 
 func init() {
-	featurefmt.RegisterLister("apk", dpkg.ParserName, &lister{})
+	featurefmt.RegisterLister("apk", "1.0", &lister{})
 }
 
 type lister struct{}

ext/featurefmt/dpkg/dpkg.go

@@ -37,7 +37,7 @@ var (
 type lister struct{}
 
 func init() {
-	featurefmt.RegisterLister("dpkg", dpkg.ParserName, &lister{})
+	featurefmt.RegisterLister("dpkg", "1.0", &lister{})
 }
 
 func (l lister) ListFeatures(files tarutil.FilesMap) ([]database.Feature, error) {

ext/featurefmt/driver.go

@@ -31,9 +31,8 @@ import (
 )
 
 var (
-	listersM             sync.RWMutex
+	listersM sync.RWMutex
-	listers              = make(map[string]Lister)
+	listers  = make(map[string]lister)
-	versionfmtListerName = make(map[string][]string)
 )
 
 // Lister represents an ability to list the features present in an image layer.
@@ -48,13 +47,19 @@ type Lister interface {
 	RequiredFilenames() []string
 }
 
+type lister struct {
+	Lister
+
+	info database.Detector
+}
+
 // RegisterLister makes a Lister available by the provided name.
 //
 // If called twice with the same name, the name is blank, or if the provided
 // Lister is nil, this function panics.
-func RegisterLister(name string, versionfmt string, l Lister) {
+func RegisterLister(name string, version string, l Lister) {
-	if name == "" {
+	if name == "" || version == "" {
-		panic("featurefmt: could not register a Lister with an empty name")
+		panic("featurefmt: could not register a Lister with an empty name or version")
 	}
 	if l == nil {
 		panic("featurefmt: could not register a nil Lister")
@@ -67,51 +72,65 @@ func RegisterLister(name string, versionfmt string, l Lister) {
 		panic("featurefmt: RegisterLister called twice for " + name)
 	}
 
-	listers[name] = l
+	listers[name] = lister{l, database.NewFeatureDetector(name, version)}
-	versionfmtListerName[versionfmt] = append(versionfmtListerName[versionfmt], name)
 }
 
 // ListFeatures produces the list of Features in an image layer using
 // every registered Lister.
-func ListFeatures(files tarutil.FilesMap, listerNames []string) ([]database.Feature, error) {
+func ListFeatures(files tarutil.FilesMap, toUse []database.Detector) ([]database.LayerFeature, error) {
 	listersM.RLock()
 	defer listersM.RUnlock()
 
-	var totalFeatures []database.Feature
+	features := []database.LayerFeature{}
+	for _, d := range toUse {
+		// Only use the detector with the same type
+		if d.DType != database.FeatureDetectorType {
+			continue
+		}
 
-	for _, name := range listerNames {
+		if lister, ok := listers[d.Name]; ok {
-		if lister, ok := listers[name]; ok {
+			fs, err := lister.ListFeatures(files)
-			features, err := lister.ListFeatures(files)
 			if err != nil {
-				return []database.Feature{}, err
+				return nil, err
 			}
-			totalFeatures = append(totalFeatures, features...)
+
+			for _, f := range fs {
+				features = append(features, database.LayerFeature{
+					Feature: f,
+					By:      lister.info,
+				})
+			}
+
 		} else {
-			log.WithField("Name", name).Warn("Unknown Lister")
+			log.WithField("Name", d).Fatal("unknown feature detector")
 		}
 	}
 
-	return totalFeatures, nil
+	return features, nil
 }
 
-// RequiredFilenames returns the total list of files required for all
+// RequiredFilenames returns all files required by the give extensions. Any
-// registered Listers.
+// extension metadata that has non feature-detector type will be skipped.
-func RequiredFilenames(listerNames []string) (files []string) {
+func RequiredFilenames(toUse []database.Detector) (files []string) {
 	listersM.RLock()
 	defer listersM.RUnlock()
 
-	for _, lister := range listers {
+	for _, d := range toUse {
-		files = append(files, lister.RequiredFilenames()...)
+		if d.DType != database.FeatureDetectorType {
+			continue
+		}
+
+		files = append(files, listers[d.Name].RequiredFilenames()...)
 	}
 
 	return
 }
 
 // ListListers returns the names of all the registered feature listers.
-func ListListers() []string {
+func ListListers() []database.Detector {
-	r := []string{}
+	r := []database.Detector{}
-	for name := range listers {
+	for _, d := range listers {
-		r = append(r, name)
+		r = append(r, d.info)
 	}
 	return r
 }

ext/featurefmt/rpm/rpm.go

@@ -35,7 +35,7 @@ import (
 type lister struct{}
 
 func init() {
-	featurefmt.RegisterLister("rpm", rpm.ParserName, &lister{})
+	featurefmt.RegisterLister("rpm", "1.0", &lister{})
 }
 
 func (l lister) ListFeatures(files tarutil.FilesMap) ([]database.Feature, error) {

ext/featurens/alpinerelease/alpinerelease.go

@@ -36,7 +36,7 @@ const (
 var versionRegexp = regexp.MustCompile(`^(\d)+\.(\d)+\.(\d)+$`)
 
 func init() {
-	featurens.RegisterDetector("alpine-release", &detector{})
+	featurens.RegisterDetector("alpine-release", "1.0", &detector{})
 }
 
 type detector struct{}

ext/featurens/aptsources/aptsources.go

@@ -32,7 +32,7 @@ import (
 type detector struct{}
 
 func init() {
-	featurens.RegisterDetector("apt-sources", &detector{})
+	featurens.RegisterDetector("apt-sources", "1.0", &detector{})
 }
 
 func (d detector) Detect(files tarutil.FilesMap) (*database.Namespace, error) {

ext/featurens/driver.go

@@ -29,7 +29,7 @@ import (
 
 var (
 	detectorsM sync.RWMutex
-	detectors  = make(map[string]Detector)
+	detectors  = make(map[string]detector)
 )
 
 // Detector represents an ability to detect a namespace used for organizing
@@ -46,13 +46,19 @@ type Detector interface {
 	RequiredFilenames() []string
 }
 
+type detector struct {
+	Detector
+
+	info database.Detector
+}
+
 // RegisterDetector makes a detector available by the provided name.
 //
 // If called twice with the same name, the name is blank, or if the provided
 // Detector is nil, this function panics.
-func RegisterDetector(name string, d Detector) {
+func RegisterDetector(name string, version string, d Detector) {
-	if name == "" {
+	if name == "" || version == "" {
-		panic("namespace: could not register a Detector with an empty name")
+		panic("namespace: could not register a Detector with an empty name or version")
 	}
 	if d == nil {
 		panic("namespace: could not register a nil Detector")
@@ -61,60 +67,69 @@ func RegisterDetector(name string, d Detector) {
 	detectorsM.Lock()
 	defer detectorsM.Unlock()
 
-	if _, dup := detectors[name]; dup {
+	if _, ok := detectors[name]; ok {
 		panic("namespace: RegisterDetector called twice for " + name)
 	}
 
-	detectors[name] = d
+	detectors[name] = detector{d, database.NewNamespaceDetector(name, version)}
 }
 
-// Detect iterators through all registered Detectors and returns all non-nil detected namespaces
+// Detect uses detectors specified to retrieve the detect result.
-func Detect(files tarutil.FilesMap, detectorNames []string) ([]database.Namespace, error) {
+func Detect(files tarutil.FilesMap, toUse []database.Detector) ([]database.LayerNamespace, error) {
 	detectorsM.RLock()
 	defer detectorsM.RUnlock()
-	namespaces := map[string]*database.Namespace{}
+
-	for _, name := range detectorNames {
+	namespaces := []database.LayerNamespace{}
-		if detector, ok := detectors[name]; ok {
+	for _, d := range toUse {
+		// Only use the detector with the same type
+		if d.DType != database.NamespaceDetectorType {
+			continue
+		}
+
+		if detector, ok := detectors[d.Name]; ok {
 			namespace, err := detector.Detect(files)
 			if err != nil {
-				log.WithError(err).WithField("name", name).Warning("failed while attempting to detect namespace")
+				log.WithError(err).WithField("detector", d).Warning("failed while attempting to detect namespace")
 				return nil, err
 			}
 
 			if namespace != nil {
-				log.WithFields(log.Fields{"name": name, "namespace": namespace.Name}).Debug("detected namespace")
+				log.WithFields(log.Fields{"detector": d, "namespace": namespace.Name}).Debug("detected namespace")
-				namespaces[namespace.Name] = namespace
+				namespaces = append(namespaces, database.LayerNamespace{
+					Namespace: *namespace,
+					By:        detector.info,
+				})
 			}
 		} else {
-			log.WithField("Name", name).Warn("Unknown namespace detector")
+			log.WithField("detector", d).Fatal("unknown namespace detector")
 		}
 	}
 
-	nslist := []database.Namespace{}
+	return namespaces, nil
-	for _, ns := range namespaces {
-		nslist = append(nslist, *ns)
-	}
-	return nslist, nil
 }
 
-// RequiredFilenames returns the total list of files required for all
+// RequiredFilenames returns all files required by the give extensions. Any
-// registered Detectors.
+// extension metadata that has non namespace-detector type will be skipped.
-func RequiredFilenames(detectorNames []string) (files []string) {
+func RequiredFilenames(toUse []database.Detector) (files []string) {
 	detectorsM.RLock()
 	defer detectorsM.RUnlock()
 
-	for _, detector := range detectors {
+	for _, d := range toUse {
-		files = append(files, detector.RequiredFilenames()...)
+		if d.DType != database.NamespaceDetectorType {
+			continue
+		}
+
+		files = append(files, detectors[d.Name].RequiredFilenames()...)
 	}
 
 	return
 }
 
-// ListDetectors returns the names of all registered namespace detectors.
+// ListDetectors returns the info of all registered namespace detectors.
-func ListDetectors() []string {
+func ListDetectors() []database.Detector {
-	r := []string{}
+	r := make([]database.Detector, 0, len(detectors))
-	for name := range detectors {
+	for _, d := range detectors {
-		r = append(r, name)
+		r = append(r, d.info)
 	}
 	return r
 }

ext/featurens/driver_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/coreos/clair/database"
 	"github.com/coreos/clair/ext/featurens"
 	"github.com/coreos/clair/pkg/tarutil"
+	"github.com/coreos/clair/pkg/testutil"
 
 	_ "github.com/coreos/clair/ext/featurens/alpinerelease"
 	_ "github.com/coreos/clair/ext/featurens/aptsources"
@@ -16,40 +17,14 @@ import (
 	_ "github.com/coreos/clair/ext/featurens/redhatrelease"
 )
 
-type MultipleNamespaceTestData struct {
+var namespaceDetectorTests = []struct {
-	Files              tarutil.FilesMap
+	in  tarutil.FilesMap
-	ExpectedNamespaces []database.Namespace
+	out []database.LayerNamespace
-}
+	err string
-
+}{
-func assertnsNameEqual(t *testing.T, nslist_expected, nslist []database.Namespace) {
+	{
-	assert.Equal(t, len(nslist_expected), len(nslist))
+		in: tarutil.FilesMap{
-	expected := map[string]struct{}{}
+			"etc/os-release": []byte(`
-	input := map[string]struct{}{}
-	// compare the two sets
-	for i := range nslist_expected {
-		expected[nslist_expected[i].Name] = struct{}{}
-		input[nslist[i].Name] = struct{}{}
-	}
-	assert.Equal(t, expected, input)
-}
-
-func testMultipleNamespace(t *testing.T, testData []MultipleNamespaceTestData) {
-	for _, td := range testData {
-		nslist, err := featurens.Detect(td.Files, featurens.ListDetectors())
-		assert.Nil(t, err)
-		assertnsNameEqual(t, td.ExpectedNamespaces, nslist)
-	}
-}
-
-func TestMultipleNamespaceDetector(t *testing.T) {
-	testData := []MultipleNamespaceTestData{
-		{
-			ExpectedNamespaces: []database.Namespace{
-				{Name: "debian:8", VersionFormat: "dpkg"},
-				{Name: "alpine:v3.3", VersionFormat: "dpkg"},
-			},
-			Files: tarutil.FilesMap{
-				"etc/os-release": []byte(`
 PRETTY_NAME="Debian GNU/Linux 8 (jessie)"
 NAME="Debian GNU/Linux"
 VERSION_ID="8"
@@ -58,9 +33,23 @@ ID=debian
 HOME_URL="http://www.debian.org/"
 SUPPORT_URL="http://www.debian.org/support/"
 BUG_REPORT_URL="https://bugs.debian.org/"`),
-				"etc/alpine-release": []byte(`3.3.4`),
+			"etc/alpine-release": []byte(`3.3.4`),
-			},
 		},
-	}
+		out: []database.LayerNamespace{
-	testMultipleNamespace(t, testData)
+			{database.Namespace{"debian:8", "dpkg"}, database.NewNamespaceDetector("os-release", "1.0")},
+			{database.Namespace{"alpine:v3.3", "dpkg"}, database.NewNamespaceDetector("alpine-release", "1.0")},
+		},
+	},
+}
+
+func TestNamespaceDetector(t *testing.T) {
+	for _, test := range namespaceDetectorTests {
+		out, err := featurens.Detect(test.in, featurens.ListDetectors())
+		if test.err != "" {
+			assert.EqualError(t, err, test.err)
+			return
+		}
+
+		testutil.AssertLayerNamespacesEqual(t, test.out, out)
+	}
 }

ext/featurens/lsbrelease/lsbrelease.go

@@ -38,7 +38,7 @@ var (
 type detector struct{}
 
 func init() {
-	featurens.RegisterDetector("lsb-release", &detector{})
+	featurens.RegisterDetector("lsb-release", "1.0", &detector{})
 }
 
 func (d detector) Detect(files tarutil.FilesMap) (*database.Namespace, error) {

ext/featurens/osrelease/osrelease.go

@@ -45,7 +45,7 @@ var (
 type detector struct{}
 
 func init() {
-	featurens.RegisterDetector("os-release", &detector{})
+	featurens.RegisterDetector("os-release", "1.0", &detector{})
 }
 
 func (d detector) Detect(files tarutil.FilesMap) (*database.Namespace, error) {

ext/featurens/redhatrelease/redhatrelease.go

@@ -38,7 +38,7 @@ var (
 type detector struct{}
 
 func init() {
-	featurens.RegisterDetector("redhat-release", &detector{})
+	featurens.RegisterDetector("redhat-release", "1.0", &detector{})
 }
 
 func (d detector) Detect(files tarutil.FilesMap) (*database.Namespace, error) {

ext/imagefmt/driver.go

@@ -33,6 +33,7 @@ import (
 	log "github.com/sirupsen/logrus"
 
 	"github.com/coreos/clair/pkg/commonerr"
+	"github.com/coreos/clair/pkg/strutil"
 	"github.com/coreos/clair/pkg/tarutil"
 )
 
@@ -106,7 +107,7 @@ func UnregisterExtractor(name string) {
 func Extract(format, path string, headers map[string]string, toExtract []string) (tarutil.FilesMap, error) {
 	var layerReader io.ReadCloser
 	if strings.HasPrefix(path, "http://") || strings.HasPrefix(path, "https://") {
-		// Create a new HTTP request object.
+		log.WithField("path", strutil.CleanURL(path)).Debug("start downloading layer blob...")
 		request, err := http.NewRequest("GET", path, nil)
 		if err != nil {
 			return nil, ErrCouldNotFindLayer
@@ -127,21 +128,23 @@ func Extract(format, path string, headers map[string]string, toExtract []string)
 		client := &http.Client{Transport: tr}
 		r, err := client.Do(request)
 		if err != nil {
-			log.WithError(err).Warning("could not download layer")
+			log.WithError(err).Error("could not download layer")
 			return nil, ErrCouldNotFindLayer
 		}
 
 		// Fail if we don't receive a 2xx HTTP status code.
 		if math.Floor(float64(r.StatusCode/100)) != 2 {
-			log.WithField("status code", r.StatusCode).Warning("could not download layer: expected 2XX")
+			log.WithError(ErrCouldNotFindLayer).WithField("status code", r.StatusCode).Error("could not download layer: expected 2XX")
 			return nil, ErrCouldNotFindLayer
 		}
 
 		layerReader = r.Body
 	} else {
+		log.WithField("path", strutil.CleanURL(path)).Debug("start reading layer blob from local file system...")
 		var err error
 		layerReader, err = os.Open(path)
 		if err != nil {
+			log.WithError(ErrCouldNotFindLayer).Error("could not open layer")
 			return nil, ErrCouldNotFindLayer
 		}
 	}