diff --git a/ext/featurefmt/apk/apk.go b/ext/featurefmt/apk/apk.go index 195c8920..eda25cd4 100644 --- a/ext/featurefmt/apk/apk.go +++ b/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{} diff --git a/ext/featurefmt/dpkg/dpkg.go b/ext/featurefmt/dpkg/dpkg.go index 6b987cf3..0ac30a90 100644 --- a/ext/featurefmt/dpkg/dpkg.go +++ b/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) { diff --git a/ext/featurefmt/driver.go b/ext/featurefmt/driver.go index 0f48b0e7..6ade140b 100644 --- a/ext/featurefmt/driver.go +++ b/ext/featurefmt/driver.go @@ -31,9 +31,8 @@ import ( ) var ( - listersM sync.RWMutex - listers = make(map[string]Lister) - versionfmtListerName = make(map[string][]string) + listersM sync.RWMutex + listers = make(map[string]lister) ) // 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) { - if name == "" { - panic("featurefmt: could not register a Lister with an empty name") +func RegisterLister(name string, version string, l Lister) { + if name == "" || version == "" { + 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 - versionfmtListerName[versionfmt] = append(versionfmtListerName[versionfmt], name) + listers[name] = lister{l, database.NewFeatureDetector(name, version)} } // 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[name]; ok { - features, err := lister.ListFeatures(files) + if lister, ok := listers[d.Name]; ok { + fs, 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 -// registered Listers. -func RequiredFilenames(listerNames []string) (files []string) { +// RequiredFilenames returns all files required by the give extensions. Any +// extension metadata that has non feature-detector type will be skipped. +func RequiredFilenames(toUse []database.Detector) (files []string) { listersM.RLock() defer listersM.RUnlock() - for _, lister := range listers { - files = append(files, lister.RequiredFilenames()...) + for _, d := range toUse { + 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 { - r := []string{} - for name := range listers { - r = append(r, name) +func ListListers() []database.Detector { + r := []database.Detector{} + for _, d := range listers { + r = append(r, d.info) } return r } diff --git a/ext/featurefmt/rpm/rpm.go b/ext/featurefmt/rpm/rpm.go index 5a0e1fa1..be582384 100644 --- a/ext/featurefmt/rpm/rpm.go +++ b/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) { diff --git a/ext/featurens/alpinerelease/alpinerelease.go b/ext/featurens/alpinerelease/alpinerelease.go index fafe5c9f..8fe6f808 100644 --- a/ext/featurens/alpinerelease/alpinerelease.go +++ b/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{} diff --git a/ext/featurens/aptsources/aptsources.go b/ext/featurens/aptsources/aptsources.go index c43818e7..287073f4 100644 --- a/ext/featurens/aptsources/aptsources.go +++ b/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) { diff --git a/ext/featurens/driver.go b/ext/featurens/driver.go index b7e0ad37..781ba5d4 100644 --- a/ext/featurens/driver.go +++ b/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) { - if name == "" { - panic("namespace: could not register a Detector with an empty name") +func RegisterDetector(name string, version string, d Detector) { + if name == "" || version == "" { + 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 -func Detect(files tarutil.FilesMap, detectorNames []string) ([]database.Namespace, error) { +// Detect uses detectors specified to retrieve the detect result. +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 { - if detector, ok := detectors[name]; ok { + + namespaces := []database.LayerNamespace{} + 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") - namespaces[namespace.Name] = namespace + log.WithFields(log.Fields{"detector": d, "namespace": namespace.Name}).Debug("detected 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{} - for _, ns := range namespaces { - nslist = append(nslist, *ns) - } - return nslist, nil + return namespaces, nil } -// RequiredFilenames returns the total list of files required for all -// registered Detectors. -func RequiredFilenames(detectorNames []string) (files []string) { +// RequiredFilenames returns all files required by the give extensions. Any +// extension metadata that has non namespace-detector type will be skipped. +func RequiredFilenames(toUse []database.Detector) (files []string) { detectorsM.RLock() defer detectorsM.RUnlock() - for _, detector := range detectors { - files = append(files, detector.RequiredFilenames()...) + for _, d := range toUse { + if d.DType != database.NamespaceDetectorType { + continue + } + + files = append(files, detectors[d.Name].RequiredFilenames()...) } return } -// ListDetectors returns the names of all registered namespace detectors. -func ListDetectors() []string { - r := []string{} - for name := range detectors { - r = append(r, name) +// ListDetectors returns the info of all registered namespace detectors. +func ListDetectors() []database.Detector { + r := make([]database.Detector, 0, len(detectors)) + for _, d := range detectors { + r = append(r, d.info) } return r } diff --git a/ext/featurens/driver_test.go b/ext/featurens/driver_test.go index 0d46b0b6..1218a852 100644 --- a/ext/featurens/driver_test.go +++ b/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 { - Files tarutil.FilesMap - ExpectedNamespaces []database.Namespace -} - -func assertnsNameEqual(t *testing.T, nslist_expected, nslist []database.Namespace) { - assert.Equal(t, len(nslist_expected), len(nslist)) - expected := map[string]struct{}{} - 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(` +var namespaceDetectorTests = []struct { + in tarutil.FilesMap + out []database.LayerNamespace + err string +}{ + { + in: 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{ + {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) } - testMultipleNamespace(t, testData) } diff --git a/ext/featurens/lsbrelease/lsbrelease.go b/ext/featurens/lsbrelease/lsbrelease.go index d883215f..de528327 100644 --- a/ext/featurens/lsbrelease/lsbrelease.go +++ b/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) { diff --git a/ext/featurens/osrelease/osrelease.go b/ext/featurens/osrelease/osrelease.go index 1139739d..8e736743 100644 --- a/ext/featurens/osrelease/osrelease.go +++ b/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) { diff --git a/ext/featurens/redhatrelease/redhatrelease.go b/ext/featurens/redhatrelease/redhatrelease.go index 0dabc3fa..a8d27081 100644 --- a/ext/featurens/redhatrelease/redhatrelease.go +++ b/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) { diff --git a/ext/imagefmt/driver.go b/ext/imagefmt/driver.go index 46724401..9838efec 100644 --- a/ext/imagefmt/driver.go +++ b/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 } }