vulnsrc: Refactor debian and alpine sources

database/dbutil.go

@@ -53,6 +53,20 @@ func ConvertFeatureSetToFeatures(features mapset.Set) []Feature {
 	return uniqueFeatures
 }
 
+// FindKeyValueAndRollback wraps session FindKeyValue function with begin and
+// roll back.
+func FindKeyValueAndRollback(datastore Datastore, key string) (value string, ok bool, err error) {
+	var tx Session
+	tx, err = datastore.Begin()
+	if err != nil {
+		return
+	}
+	defer tx.Rollback()
+
+	value, ok, err = tx.FindKeyValue(key)
+	return
+}
+
 // PersistPartialLayerAndCommit wraps session PersistLayer function with begin and
 // commit.
 func PersistPartialLayerAndCommit(datastore Datastore, layer *Layer) error {

ext/vulnsrc/alpine/alpine.go

@@ -18,10 +18,8 @@ package alpine
 
 import (
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
-	"strings"
 
 	log "github.com/sirupsen/logrus"
 	"gopkg.in/yaml.v2"
@@ -30,10 +28,13 @@ import (
 	"github.com/coreos/clair/ext/versionfmt"
 	"github.com/coreos/clair/ext/versionfmt/dpkg"
 	"github.com/coreos/clair/ext/vulnsrc"
+	"github.com/coreos/clair/pkg/fsutil"
 	"github.com/coreos/clair/pkg/gitutil"
 )
 
 const (
+	// This Alpine vulnerability database affects origin packages, which has
+	// `origin` field of itself.
 	secdbGitURL  = "https://github.com/alpinelinux/alpine-secdb"
 	updaterFlag  = "alpine-secdbUpdater"
 	nvdURLPrefix = "https://cve.mitre.org/cgi-bin/cvename.cgi?name="
@@ -51,61 +52,44 @@ type updater struct {
 }
 
 func (u *updater) Update(db database.Datastore) (resp vulnsrc.UpdateResponse, err error) {
-	log.WithField("package", "Alpine").Info("Start fetching vulnerabilities")
-
+	log.WithField("package", "Alpine").Info("start fetching vulnerabilities")
 	// Pull the master branch.
-	var commit string
-	u.repositoryLocalPath, commit, err = gitutil.CloneOrPull(secdbGitURL, u.repositoryLocalPath, updaterFlag)
-	if err != nil {
-		return
-	}
-
-	// Open a database transaction.
-	tx, err := db.Begin()
-	if err != nil {
+	var (
+		commit         string
+		existingCommit string
+		foundCommit    bool
+		namespaces     []string
+		vulns          []database.VulnerabilityWithAffected
+	)
+
+	if u.repositoryLocalPath, commit, err = gitutil.CloneOrPull(secdbGitURL, u.repositoryLocalPath, updaterFlag); err != nil {
 		return
 	}
-	defer tx.Rollback()
-
-	// Ask the database for the latest commit we successfully applied.
-	var dbCommit string
-	var ok bool
-	dbCommit, ok, err = tx.FindKeyValue(updaterFlag)
-	if err != nil {
-		return
-	}
-	if !ok {
-		dbCommit = ""
-	}
 
 	// Set the updaterFlag to equal the commit processed.
 	resp.FlagName = updaterFlag
 	resp.FlagValue = commit
+	if existingCommit, foundCommit, err = database.FindKeyValueAndRollback(db, updaterFlag); err != nil {
+		return
+	}
 
 	// Short-circuit if there have been no updates.
-	if commit == dbCommit {
-		log.WithField("package", "alpine").Debug("no update")
+	if foundCommit && commit == existingCommit {
+		log.WithField("package", "alpine").Debug("no update, skip")
 		return
 	}
 
 	// Get the list of namespaces from the repository.
-	var namespaces []string
-	namespaces, err = ls(u.repositoryLocalPath, directoriesOnly)
-	if err != nil {
+	if namespaces, err = fsutil.Readdir(u.repositoryLocalPath, fsutil.DirectoriesOnly); err != nil {
 		return
 	}
 
 	// Append any changed vulnerabilities to the response.
 	for _, namespace := range namespaces {
-		var vulns []database.VulnerabilityWithAffected
-		var note string
-		vulns, note, err = parseVulnsFromNamespace(u.repositoryLocalPath, namespace)
-		if err != nil {
+		if vulns, err = parseVulnsFromNamespace(u.repositoryLocalPath, namespace); err != nil {
 			return
 		}
-		if note != "" {
-			resp.Notes = append(resp.Notes, note)
-		}
+
 		resp.Vulnerabilities = append(resp.Vulnerabilities, vulns...)
 	}
 
@@ -118,74 +102,26 @@ func (u *updater) Clean() {
 	}
 }
 
-type lsFilter int
-
-const (
-	filesOnly lsFilter = iota
-	directoriesOnly
-)
-
-func ls(path string, filter lsFilter) ([]string, error) {
-	dir, err := os.Open(path)
-	if err != nil {
-		return nil, err
-	}
-	defer dir.Close()
-
-	finfos, err := dir.Readdir(0)
-	if err != nil {
-		return nil, err
-	}
-
-	var files []string
-	for _, info := range finfos {
-		if filter == directoriesOnly && !info.IsDir() {
-			continue
-		}
-
-		if filter == filesOnly && info.IsDir() {
-			continue
-		}
-
-		if strings.HasPrefix(info.Name(), ".") {
-			continue
-		}
-
-		files = append(files, info.Name())
-	}
-
-	return files, nil
-}
-
-func parseVulnsFromNamespace(repositoryPath, namespace string) (vulns []database.VulnerabilityWithAffected, note string, err error) {
+func parseVulnsFromNamespace(repositoryPath, namespace string) (vulns []database.VulnerabilityWithAffected, err error) {
 	nsDir := filepath.Join(repositoryPath, namespace)
 	var dbFilenames []string
-	dbFilenames, err = ls(nsDir, filesOnly)
-	if err != nil {
+	if dbFilenames, err = fsutil.Readdir(nsDir, fsutil.FilesOnly); err != nil {
 		return
 	}
 
 	for _, filename := range dbFilenames {
-		var file io.ReadCloser
-		file, err = os.Open(filepath.Join(nsDir, filename))
-		if err != nil {
-			return
-		}
-
-		var fileVulns []database.VulnerabilityWithAffected
-		fileVulns, err = parseYAML(file)
-		if err != nil {
+		var db *secDB
+		if db, err = newSecDB(filepath.Join(nsDir, filename)); err != nil {
 			return
 		}
 
-		vulns = append(vulns, fileVulns...)
-		file.Close()
+		vulns = append(vulns, db.Vulnerabilities()...)
 	}
 
 	return
 }
 
-type secDBFile struct {
+type secDB struct {
 	Distro   string `yaml:"distroversion"`
 	Packages []struct {
 		Pkg struct {
@@ -195,42 +131,54 @@ type secDBFile struct {
 	} `yaml:"packages"`
 }
 
-func parseYAML(r io.Reader) (vulns []database.VulnerabilityWithAffected, err error) {
-	var rBytes []byte
-	rBytes, err = ioutil.ReadAll(r)
+func newSecDB(filePath string) (file *secDB, err error) {
+	var f io.ReadCloser
+	f, err = os.Open(filePath)
 	if err != nil {
 		return
 	}
 
-	var file secDBFile
-	err = yaml.Unmarshal(rBytes, &file)
-	if err != nil {
+	defer f.Close()
+	file = &secDB{}
+	err = yaml.NewDecoder(f).Decode(file)
+	return
+}
+
+func (file *secDB) Vulnerabilities() (vulns []database.VulnerabilityWithAffected) {
+	if file == nil {
 		return
 	}
 
-	for _, pack := range file.Packages {
-		pkg := pack.Pkg
-		for version, vulnStrs := range pkg.Fixes {
-			err := versionfmt.Valid(dpkg.ParserName, version)
-			if err != nil {
-				log.WithError(err).WithField("version", version).Warning("could not parse package version. skipping")
+	namespace := database.Namespace{Name: "alpine:" + file.Distro, VersionFormat: dpkg.ParserName}
+	for _, pkg := range file.Packages {
+		for version, cveNames := range pkg.Pkg.Fixes {
+			if err := versionfmt.Valid(dpkg.ParserName, version); err != nil {
+				log.WithError(err).WithFields(log.Fields{
+					"version":      version,
+					"package name": pkg.Pkg.Name,
+				}).Warning("could not parse package version, skipping")
 				continue
 			}
 
-			for _, vulnStr := range vulnStrs {
-				var vuln database.VulnerabilityWithAffected
-				vuln.Severity = database.UnknownSeverity
-				vuln.Name = vulnStr
-				vuln.Link = nvdURLPrefix + vulnStr
+			for _, cve := range cveNames {
+				vuln := database.VulnerabilityWithAffected{
+					Vulnerability: database.Vulnerability{
+						Name:      cve,
+						Link:      nvdURLPrefix + cve,
+						Severity:  database.UnknownSeverity,
+						Namespace: namespace,
+					},
+				}
 
 				var fixedInVersion string
 				if version != versionfmt.MaxVersion {
 					fixedInVersion = version
 				}
+
 				vuln.Affected = []database.AffectedFeature{
 					{
 						AffectedType:    affectedType,
-						FeatureName:     pkg.Name,
+						FeatureName:     pkg.Pkg.Name,
 						AffectedVersion: version,
 						FixedInVersion:  fixedInVersion,
 						Namespace: database.Namespace{

ext/vulnsrc/alpine/alpine_test.go

@@ -15,28 +15,24 @@
 package alpine
 
 import (
-	"os"
 	"path/filepath"
 	"runtime"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestYAMLParsing(t *testing.T) {
 	_, filename, _, _ := runtime.Caller(0)
 	path := filepath.Join(filepath.Dir(filename))
+	secdb, err := newSecDB(filepath.Join(path, "/testdata/v34_main.yaml"))
+	require.Nil(t, err)
+	vulns := secdb.Vulnerabilities()
 
-	testData, _ := os.Open(path + "/testdata/v34_main.yaml")
-	defer testData.Close()
-
-	vulns, err := parseYAML(testData)
-	if err != nil {
-		assert.Nil(t, err)
-	}
 	assert.Equal(t, 105, len(vulns))
 	assert.Equal(t, "CVE-2016-5387", vulns[0].Name)
-	assert.Equal(t, "alpine:v3.4", vulns[0].Affected[0].Namespace.Name)
+	assert.Equal(t, "alpine:v3.4", vulns[0].Namespace.Name)
 	assert.Equal(t, "apache2", vulns[0].Affected[0].FeatureName)
 	assert.Equal(t, "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-5387", vulns[0].Link)
 }

ext/vulnsrc/debian/debian.go

@@ -62,22 +62,9 @@ func init() {
 
 func (u *updater) Update(datastore database.Datastore) (resp vulnsrc.UpdateResponse, err error) {
 	log.WithField("package", "Debian").Info("Start fetching vulnerabilities")
-
-	tx, err := datastore.Begin()
-	if err != nil {
-		return resp, err
-	}
-
-	// Get the hash of the latest update's JSON data
-	latestHash, ok, err := tx.FindKeyValue(updaterFlag)
+	latestHash, ok, err := database.FindKeyValueAndRollback(datastore, updaterFlag)
 	if err != nil {
-		return resp, err
-	}
-
-	// NOTE(sida): The transaction won't mutate the database and I want the
-	// transaction to be short.
-	if err := tx.Rollback(); err != nil {
-		return resp, err
+		return
 	}
 
 	if !ok {
@@ -136,7 +123,7 @@ func buildResponse(jsonReader io.Reader, latestKnownHash string) (resp vulnsrc.U
 	// Calculate the hash and skip updating if the hash has been seen before.
 	hash = hex.EncodeToString(jsonSHA.Sum(nil))
 	if latestKnownHash == hash {
-		log.WithField("package", "Debian").Debug("no update")
+		log.WithField("package", "Debian").Debug("no update, skip")
 		return resp, nil
 	}
 

ext/vulnsrc/debian/debian_test.go

@@ -30,7 +30,7 @@ func TestDebianParser(t *testing.T) {
 	_, filename, _, _ := runtime.Caller(0)
 
 	// Test parsing testdata/fetcher_debian_test.json
-	testFile, _ := os.Open(filepath.Join(filepath.Dir(filename)) + "/testdata/fetcher_debian_test.json")
+	testFile, _ := os.Open(filepath.Join(filepath.Dir(filename), "/testdata/fetcher_debian_test.json"))
 	response, err := buildResponse(testFile, "")
 	if assert.Nil(t, err) && assert.Len(t, response.Vulnerabilities, 2) {
 		for _, vulnerability := range response.Vulnerabilities {

ext/vulnsrc/oracle/oracle.go

@@ -119,15 +119,9 @@ func compareELSA(left, right int) int {
 func (u *updater) Update(datastore database.Datastore) (resp vulnsrc.UpdateResponse, err error) {
 	log.WithField("package", "Oracle Linux").Info("Start fetching vulnerabilities")
 	// Get the first ELSA we have to manage.
-	tx, err := datastore.Begin()
+	flagValue, ok, err := database.FindKeyValueAndRollback(datastore, updaterFlag)
 	if err != nil {
-		return resp, err
-	}
-	defer tx.Rollback()
-
-	flagValue, ok, err := tx.FindKeyValue(updaterFlag)
-	if err != nil {
-		return resp, err
+		return
 	}
 
 	if !ok {

ext/vulnsrc/oracle/oracle_test.go

@@ -30,7 +30,7 @@ func TestOracleParser(t *testing.T) {
 	path := filepath.Join(filepath.Dir(filename))
 
 	// Test parsing testdata/fetcher_oracle_test.1.xml
-	testFile, _ := os.Open(path + "/testdata/fetcher_oracle_test.1.xml")
+	testFile, _ := os.Open(filepath.Join(path, "/testdata/fetcher_oracle_test.1.xml"))
 	defer testFile.Close()
 
 	vulnerabilities, err := parseELSA(testFile)
@@ -78,7 +78,7 @@ func TestOracleParser(t *testing.T) {
 		}
 	}
 
-	testFile2, _ := os.Open(path + "/testdata/fetcher_oracle_test.2.xml")
+	testFile2, _ := os.Open(filepath.Join(path, "/testdata/fetcher_oracle_test.2.xml"))
 	defer testFile2.Close()
 
 	vulnerabilities, err = parseELSA(testFile2)