updater: Refactor and merge fetcher responses

Fixes #17 and lays the groundwork for #19.

updater/fetchers.go

@@ -14,10 +14,7 @@
 
 package updater
 
-import (
-	"github.com/coreos/clair/database"
-	"github.com/coreos/clair/utils/types"
-)
+import "github.com/coreos/clair/database"
 
 var fetchers = make(map[string]Fetcher)
 
@@ -31,17 +28,8 @@ type FetcherResponse struct {
 	FlagName        string
 	FlagValue       string
 	Notes           []string
-	Vulnerabilities []FetcherVulnerability
-}
-
-// FetcherVulnerability represents an individual vulnerability processed from
-// an update.
-type FetcherVulnerability struct {
-	ID          string
-	Link        string
-	Description string
-	Priority    types.Priority
-	FixedIn     []*database.Package
+	Vulnerabilities []*database.Vulnerability
+	Packages        []*database.Package
 }
 
 // RegisterFetcher makes a Fetcher available by the provided name.

updater/fetchers/debian.go

@@ -24,8 +24,8 @@ import (
 	"strings"
 
 	"github.com/coreos/clair/database"
-	cerrors "github.com/coreos/clair/utils/errors"
 	"github.com/coreos/clair/updater"
+	cerrors "github.com/coreos/clair/utils/errors"
 	"github.com/coreos/clair/utils/types"
 )
 
@@ -114,7 +114,8 @@ func buildResponse(jsonReader io.Reader, latestKnownHash string) (resp updater.F
 	}
 
 	// Extract vulnerability data from Debian's JSON schema.
-	vulnerabilities, unknownReleases := parseDebianJSON(&data)
+	var unknownReleases map[string]struct{}
+	resp.Vulnerabilities, resp.Packages, unknownReleases = parseDebianJSON(&data)
 
 	// Log unknown releases
 	for k := range unknownReleases {
@@ -123,16 +124,11 @@ func buildResponse(jsonReader io.Reader, latestKnownHash string) (resp updater.F
 		log.Warning(note)
 	}
 
-	// Convert the vulnerabilities map to a slice in the response
-	for _, v := range vulnerabilities {
-		resp.Vulnerabilities = append(resp.Vulnerabilities, v)
-	}
-
 	return resp, nil
 }
 
-func parseDebianJSON(data *jsonData) (vulnerabilities map[string]updater.FetcherVulnerability, unknownReleases map[string]struct{}) {
-	vulnerabilities = make(map[string]updater.FetcherVulnerability)
+func parseDebianJSON(data *jsonData) (vulnerabilities []*database.Vulnerability, packages []*database.Package, unknownReleases map[string]struct{}) {
+	mvulnerabilities := make(map[string]*database.Vulnerability)
 	unknownReleases = make(map[string]struct{})
 
 	for pkgName, pkgNode := range *data {
@@ -150,9 +146,9 @@ func parseDebianJSON(data *jsonData) (vulnerabilities map[string]updater.Fetcher
 				}
 
 				// Get or create the vulnerability.
-				vulnerability, vulnerabilityAlreadyExists := vulnerabilities[vulnName]
+				vulnerability, vulnerabilityAlreadyExists := mvulnerabilities[vulnName]
 				if !vulnerabilityAlreadyExists {
-					vulnerability = updater.FetcherVulnerability{
+					vulnerability = &database.Vulnerability{
 						ID:          vulnName,
 						Link:        strings.Join([]string{cveURLPrefix, "/", vulnName}, ""),
 						Priority:    types.Unknown,
@@ -191,14 +187,20 @@ func parseDebianJSON(data *jsonData) (vulnerabilities map[string]updater.Fetcher
 					Name:    pkgName,
 					Version: version,
 				}
-				vulnerability.FixedIn = append(vulnerability.FixedIn, pkg)
+				vulnerability.FixedInNodes = append(vulnerability.FixedInNodes, pkg.GetNode())
+				packages = append(packages, pkg)
 
 				// Store the vulnerability.
-				vulnerabilities[vulnName] = vulnerability
+				mvulnerabilities[vulnName] = vulnerability
 			}
 		}
 	}
 
+	// Convert the vulnerabilities map to a slice
+	for _, v := range mvulnerabilities {
+		vulnerabilities = append(vulnerabilities, v)
+	}
+
 	return
 }
 

updater/fetchers/debian_test.go

@@ -38,39 +38,49 @@ func TestDebianParser(t *testing.T) {
 				assert.Equal(t, types.Low, vulnerability.Priority)
 				assert.Equal(t, "This vulnerability is not very dangerous.", vulnerability.Description)
 
-				if assert.Len(t, vulnerability.FixedIn, 2) {
-					assert.Contains(t, vulnerability.FixedIn, &database.Package{
+				expectedPackages := []*database.Package{
+					&database.Package{
 						OS:      "debian:8",
 						Name:    "aptdaemon",
 						Version: types.MaxVersion,
-					})
-					assert.Contains(t, vulnerability.FixedIn, &database.Package{
+					},
+					&database.Package{
 						OS:      "debian:unstable",
 						Name:    "aptdaemon",
 						Version: types.NewVersionUnsafe("1.1.1+bzr982-1"),
-					})
+					},
+				}
+
+				for _, expectedPackage := range expectedPackages {
+					assert.Contains(t, response.Packages, expectedPackage)
+					assert.Contains(t, vulnerability.FixedInNodes, expectedPackage.GetNode())
 				}
 			} else if vulnerability.ID == "CVE-2003-0779" {
 				assert.Equal(t, "https://security-tracker.debian.org/tracker/CVE-2003-0779", vulnerability.Link)
 				assert.Equal(t, types.High, vulnerability.Priority)
 				assert.Equal(t, "But this one is very dangerous.", vulnerability.Description)
 
-				if assert.Len(t, vulnerability.FixedIn, 3) {
-					assert.Contains(t, vulnerability.FixedIn, &database.Package{
+				expectedPackages := []*database.Package{
+					&database.Package{
 						OS:      "debian:8",
 						Name:    "aptdaemon",
 						Version: types.NewVersionUnsafe("0.7.0"),
-					})
-					assert.Contains(t, vulnerability.FixedIn, &database.Package{
+					},
+					&database.Package{
 						OS:      "debian:unstable",
 						Name:    "aptdaemon",
 						Version: types.NewVersionUnsafe("0.7.0"),
-					})
-					assert.Contains(t, vulnerability.FixedIn, &database.Package{
+					},
+					&database.Package{
 						OS:      "debian:8",
 						Name:    "asterisk",
 						Version: types.NewVersionUnsafe("0.5.56"),
-					})
+					},
+				}
+
+				for _, expectedPackage := range expectedPackages {
+					assert.Contains(t, response.Packages, expectedPackage)
+					assert.Contains(t, vulnerability.FixedInNodes, expectedPackage.GetNode())
 				}
 			} else {
 				assert.Fail(t, "Wrong vulnerability name: ", vulnerability.ID)

updater/fetchers/rhel.go

@@ -24,8 +24,8 @@ import (
 	"strings"
 
 	"github.com/coreos/clair/database"
-	cerrors "github.com/coreos/clair/utils/errors"
 	"github.com/coreos/clair/updater"
+	cerrors "github.com/coreos/clair/utils/errors"
 	"github.com/coreos/clair/utils/types"
 )
 
@@ -128,17 +128,14 @@ func (f *RHELFetcher) FetchUpdate() (resp updater.FetcherResponse, err error) {
 		}
 
 		// Parse the XML.
-		vs, err := parseRHSA(r.Body)
+		vs, pkgs, err := parseRHSA(r.Body)
 		if err != nil {
 			return resp, err
 		}
 
 		// Collect vulnerabilities.
-		for _, v := range vs {
-			if len(v.FixedIn) > 0 {
-				resp.Vulnerabilities = append(resp.Vulnerabilities, v)
-			}
-		}
+		resp.Vulnerabilities = append(resp.Vulnerabilities, vs...)
+		resp.Packages = append(resp.Packages, pkgs...)
 	}
 
 	// Set the flag if we found anything.
@@ -152,7 +149,7 @@ func (f *RHELFetcher) FetchUpdate() (resp updater.FetcherResponse, err error) {
 	return resp, nil
 }
 
-func parseRHSA(ovalReader io.Reader) (vulnerabilities []updater.FetcherVulnerability, err error) {
+func parseRHSA(ovalReader io.Reader) (vulnerabilities []*database.Vulnerability, packages []*database.Package, err error) {
 	// Decode the XML.
 	var ov oval
 	err = xml.NewDecoder(ovalReader).Decode(&ov)
@@ -163,18 +160,21 @@ func parseRHSA(ovalReader io.Reader) (vulnerabilities []updater.FetcherVulnerabi
 	}
 
 	// Iterate over the definitions and collect any vulnerabilities that affect
-	// more than one package.
+	// at least one package.
 	for _, definition := range ov.Definitions {
-		packages := toPackages(definition.Criteria)
-		if len(packages) > 0 {
-			vuln := updater.FetcherVulnerability{
+		pkgs := toPackages(definition.Criteria)
+		if len(pkgs) > 0 {
+			vulnerability := &database.Vulnerability{
 				ID:          name(definition),
 				Link:        link(definition),
 				Priority:    priority(definition),
 				Description: description(definition),
-				FixedIn:     packages,
 			}
-			vulnerabilities = append(vulnerabilities, vuln)
+			for _, p := range pkgs {
+				vulnerability.FixedInNodes = append(vulnerability.FixedInNodes, p.GetNode())
+			}
+			vulnerabilities = append(vulnerabilities, vulnerability)
+			packages = append(packages, pkgs...)
 		}
 	}
 

updater/fetchers/rhel_test.go

@@ -31,52 +31,62 @@ func TestRHELParser(t *testing.T) {
 
 	// Test parsing testdata/fetcher_rhel_test.1.xml
 	testFile, _ := os.Open(path + "/testdata/fetcher_rhel_test.1.xml")
-	vulnerabilities, err := parseRHSA(testFile)
+	vulnerabilities, packages, err := parseRHSA(testFile)
 	if assert.Nil(t, err) && assert.Len(t, vulnerabilities, 1) {
 		assert.Equal(t, "RHSA-2015:1193", vulnerabilities[0].ID)
 		assert.Equal(t, "https://rhn.redhat.com/errata/RHSA-2015-1193.html", vulnerabilities[0].Link)
 		assert.Equal(t, types.Medium, vulnerabilities[0].Priority)
 		assert.Equal(t, `Xerces-C is a validating XML parser written in a portable subset of C++. A flaw was found in the way the Xerces-C XML parser processed certain XML documents. A remote attacker could provide specially crafted XML input that, when parsed by an application using Xerces-C, would cause that application to crash.`, vulnerabilities[0].Description)
 
-		if assert.Len(t, vulnerabilities[0].FixedIn, 3) {
-			assert.Contains(t, vulnerabilities[0].FixedIn, &database.Package{
+		expectedPackages := []*database.Package{
+			&database.Package{
 				OS:      "centos:7",
 				Name:    "xerces-c",
 				Version: types.NewVersionUnsafe("3.1.1-7.el7_1"),
-			})
-			assert.Contains(t, vulnerabilities[0].FixedIn, &database.Package{
+			},
+			&database.Package{
 				OS:      "centos:7",
 				Name:    "xerces-c-devel",
 				Version: types.NewVersionUnsafe("3.1.1-7.el7_1"),
-			})
-			assert.Contains(t, vulnerabilities[0].FixedIn, &database.Package{
+			},
+			&database.Package{
 				OS:      "centos:7",
 				Name:    "xerces-c-doc",
 				Version: types.NewVersionUnsafe("3.1.1-7.el7_1"),
-			})
+			},
 		}
+
+			for _, expectedPackage := range expectedPackages {
+				assert.Contains(t, packages, expectedPackage)
+				assert.Contains(t, vulnerabilities[0].FixedInNodes, expectedPackage.GetNode())
+			}
 	}
 
 	// Test parsing testdata/fetcher_rhel_test.2.xml
 	testFile, _ = os.Open(path + "/testdata/fetcher_rhel_test.2.xml")
-	vulnerabilities, err = parseRHSA(testFile)
+	vulnerabilities, packages, err = parseRHSA(testFile)
 	if assert.Nil(t, err) && assert.Len(t, vulnerabilities, 1) {
 		assert.Equal(t, "RHSA-2015:1207", vulnerabilities[0].ID)
 		assert.Equal(t, "https://rhn.redhat.com/errata/RHSA-2015-1207.html", vulnerabilities[0].Link)
 		assert.Equal(t, types.Critical, vulnerabilities[0].Priority)
 		assert.Equal(t, `Mozilla Firefox is an open source web browser. XULRunner provides the XUL Runtime environment for Mozilla Firefox. Several flaws were found in the processing of malformed web content. A web page containing malicious content could cause Firefox to crash or, potentially, execute arbitrary code with the privileges of the user running Firefox.`, vulnerabilities[0].Description)
 
-		if assert.Len(t, vulnerabilities[0].FixedIn, 2) {
-			assert.Contains(t, vulnerabilities[0].FixedIn, &database.Package{
+		expectedPackages := []*database.Package{
+			&database.Package{
 				OS:      "centos:6",
 				Name:    "firefox",
 				Version: types.NewVersionUnsafe("38.1.0-1.el6_6"),
-			})
-			assert.Contains(t, vulnerabilities[0].FixedIn, &database.Package{
+			},
+			&database.Package{
 				OS:      "centos:7",
 				Name:    "firefox",
 				Version: types.NewVersionUnsafe("38.1.0-1.el7_1"),
-			})
+			},
 		}
+
+			for _, expectedPackage := range expectedPackages {
+				assert.Contains(t, packages, expectedPackage)
+				assert.Contains(t, vulnerabilities[0].FixedInNodes, expectedPackage.GetNode())
+			}
 	}
 }

updater/fetchers/ubuntu.go

@@ -133,13 +133,14 @@ func (fetcher *UbuntuFetcher) FetchUpdate() (resp updater.FetcherResponse, err e
 		}
 		defer file.Close()
 
-		v, unknownReleases, err := parseUbuntuCVE(file)
+		v, pkgs, unknownReleases, err := parseUbuntuCVE(file)
 		if err != nil {
 			return resp, err
 		}
 
-		if len(v.FixedIn) > 0 {
+		if len(v.FixedInNodes) > 0 {
 			resp.Vulnerabilities = append(resp.Vulnerabilities, v)
+			resp.Packages = append(resp.Packages, pkgs...)
 		}
 
 		// Log any unknown releases.
@@ -255,7 +256,8 @@ func getRevisionNumber(pathToRepo string) (int, error) {
 	return revno, nil
 }
 
-func parseUbuntuCVE(fileContent io.Reader) (vulnerability updater.FetcherVulnerability, unknownReleases map[string]struct{}, err error) {
+func parseUbuntuCVE(fileContent io.Reader) (vulnerability *database.Vulnerability, packages []*database.Package, unknownReleases map[string]struct{}, err error) {
+	vulnerability = &database.Vulnerability{}
 	unknownReleases = make(map[string]struct{})
 	readingDescription := false
 	scanner := bufio.NewScanner(fileContent)
@@ -351,7 +353,13 @@ func parseUbuntuCVE(fileContent io.Reader) (vulnerability updater.FetcherVulnera
 				}
 
 				// Create and add the new package.
-				vulnerability.FixedIn = append(vulnerability.FixedIn, &database.Package{OS: "ubuntu:" + database.UbuntuReleasesMapping[md["release"]], Name: md["package"], Version: version})
+				pkg := &database.Package{
+					OS:      "ubuntu:" + database.UbuntuReleasesMapping[md["release"]],
+					Name:    md["package"],
+					Version: version,
+				}
+				packages = append(packages, pkg)
+				vulnerability.FixedInNodes = append(vulnerability.FixedInNodes, pkg.GetNode())
 			}
 		}
 	}

updater/fetchers/ubuntu_test.go

@@ -32,7 +32,7 @@ func TestUbuntuParser(t *testing.T) {
 	// Test parsing testdata/fetcher_
 	testData, _ := os.Open(path + "/testdata/fetcher_ubuntu_test.txt")
 	defer testData.Close()
-	vulnerability, unknownReleases, err := parseUbuntuCVE(testData)
+	vulnerability, packages, unknownReleases, err := parseUbuntuCVE(testData)
 	if assert.Nil(t, err) {
 		assert.Equal(t, "CVE-2015-4471", vulnerability.ID)
 		assert.Equal(t, types.Medium, vulnerability.Priority)
@@ -42,22 +42,27 @@ func TestUbuntuParser(t *testing.T) {
 		_, hasUnkownRelease := unknownReleases["unknown"]
 		assert.True(t, hasUnkownRelease)
 
-		if assert.Len(t, vulnerability.FixedIn, 3) {
-			assert.Contains(t, vulnerability.FixedIn, &database.Package{
+		expectedPackages := []*database.Package{
+			&database.Package{
 				OS:      "ubuntu:14.04",
 				Name:    "libmspack",
 				Version: types.MaxVersion,
-			})
-			assert.Contains(t, vulnerability.FixedIn, &database.Package{
+			},
+			&database.Package{
 				OS:      "ubuntu:15.04",
 				Name:    "libmspack",
 				Version: types.NewVersionUnsafe("0.4-3"),
-			})
-			assert.Contains(t, vulnerability.FixedIn, &database.Package{
+			},
+			&database.Package{
 				OS:      "ubuntu:15.10",
 				Name:    "libmspack-anotherpkg",
 				Version: types.NewVersionUnsafe("0.1"),
-			})
+			},
+		}
+
+		for _, expectedPackage := range expectedPackages {
+			assert.Contains(t, packages, expectedPackage)
+			assert.Contains(t, vulnerability.FixedInNodes, expectedPackage.GetNode())
 		}
 	}
 }