From 8285c567c811de952d30ef0583db42237600b2f0 Mon Sep 17 00:00:00 2001 From: Quentin Machu Date: Fri, 20 Nov 2015 21:47:48 -0500 Subject: [PATCH] database: Improve InsertVulnerabilities. --- database/vulnerability.go | 96 +++++++++++++--------------------- database/vulnerability_test.go | 8 +-- 2 files changed, 36 insertions(+), 68 deletions(-) diff --git a/database/vulnerability.go b/database/vulnerability.go index 1ccd7f48..f9331db0 100644 --- a/database/vulnerability.go +++ b/database/vulnerability.go @@ -96,7 +96,6 @@ func (av *AbstractVulnerability) ToVulnerability(fixedInNodes []string) *Vulnera } // InsertVulnerabilities inserts or updates several vulnerabilities in the database in one transaction -// It ensures that a vulnerability can't be fixed by two packages belonging the same Branch. // During an update, if the vulnerability was previously fixed by a version in a branch and a new package of that branch is specified, the previous one is deleted // Otherwise, it simply adds the defined packages, there is currently no way to delete affected packages. // @@ -110,13 +109,15 @@ func InsertVulnerabilities(vulnerabilities []*Vulnerability) ([]Notification, er var err error t := cayley.NewTransaction() cachedVulnerabilities := make(map[string]*Vulnerability) + + var notifications []Notification newVulnerabilityNotifications := make(map[string]*NewVulnerabilityNotification) vulnerabilityPriorityIncreasedNotifications := make(map[string]*VulnerabilityPriorityIncreasedNotification) vulnerabilityPackageChangedNotifications := make(map[string]*VulnerabilityPackageChangedNotification) // Iterate over all the vulnerabilities we need to insert/update for _, vulnerability := range vulnerabilities { - // Is the vulnerability already existing ? + // Check if the vulnerability already exists existingVulnerability, _ := cachedVulnerabilities[vulnerability.ID] if existingVulnerability == nil { existingVulnerability, err = FindOneVulnerability(vulnerability.ID, FieldVulnerabilityAll) @@ -128,23 +129,6 @@ func InsertVulnerabilities(vulnerabilities []*Vulnerability) ([]Notification, er } } - // Don't allow inserting/updating a vulnerability which is fixed in two packages of the same branch - if len(vulnerability.FixedInNodes) > 0 { - fixedInPackages, err := FindAllPackagesByNodes(vulnerability.FixedInNodes, []string{FieldPackageOS, FieldPackageName}) - if err != nil { - return []Notification{}, err - } - fixedInBranches := make(map[string]struct{}) - for _, fixedInPackage := range fixedInPackages { - branch := fixedInPackage.Branch() - if _, branchExists := fixedInBranches[branch]; branchExists { - log.Warningf("could not insert vulnerability %s because it is fixed in two packages of the same branch", vulnerability.ID) - return []Notification{}, cerrors.NewBadRequestError("could not insert a vulnerability which is fixed in two packages of the same branch") - } - fixedInBranches[branch] = struct{}{} - } - } - // Insert/Update vulnerability if existingVulnerability == nil { // The vulnerability does not exist, create it @@ -165,7 +149,6 @@ func InsertVulnerabilities(vulnerabilities []*Vulnerability) ([]Notification, er // Insert it vulnerability.Node = vulnerability.GetNode() - cachedVulnerabilities[vulnerability.ID] = vulnerability t.AddQuad(cayley.Quad(vulnerability.Node, FieldIs, FieldVulnerabilityIsValue, "")) t.AddQuad(cayley.Quad(vulnerability.Node, FieldVulnerabilityID, vulnerability.ID, "")) @@ -177,7 +160,11 @@ func InsertVulnerabilities(vulnerabilities []*Vulnerability) ([]Notification, er } // Add a notification - newVulnerabilityNotifications[vulnerability.ID] = &NewVulnerabilityNotification{VulnerabilityID: vulnerability.ID} + notification := &NewVulnerabilityNotification{VulnerabilityID: vulnerability.ID} + notifications = append(notifications, notification) + newVulnerabilityNotifications[vulnerability.ID] = notification + + cachedVulnerabilities[vulnerability.ID] = vulnerability } else { // The vulnerability already exists, update it if vulnerability.Link != "" && existingVulnerability.Link != vulnerability.Link { @@ -185,6 +172,7 @@ func InsertVulnerabilities(vulnerabilities []*Vulnerability) ([]Notification, er t.AddQuad(cayley.Quad(existingVulnerability.Node, FieldVulnerabilityLink, vulnerability.Link, "")) existingVulnerability.Link = vulnerability.Link } + if vulnerability.Priority != "" && vulnerability.Priority != types.Unknown && existingVulnerability.Priority != vulnerability.Priority { if !vulnerability.Priority.IsValid() { log.Warningf("could not update a vulnerability which has an invalid priority [ID: %s, Link: %s, Priority: %s]. Valid priorities are: %v.", vulnerability.ID, vulnerability.Link, vulnerability.Priority, types.Priorities) @@ -197,10 +185,12 @@ func InsertVulnerabilities(vulnerabilities []*Vulnerability) ([]Notification, er // Any priorityChangeNotification already ? if existingPriorityNotification, _ := vulnerabilityPriorityIncreasedNotifications[vulnerability.ID]; existingPriorityNotification != nil { // There is a priority change notification, replace it but keep the old priority field - vulnerabilityPriorityIncreasedNotifications[vulnerability.ID] = &VulnerabilityPriorityIncreasedNotification{OldPriority: existingPriorityNotification.OldPriority, NewPriority: vulnerability.Priority, VulnerabilityID: existingVulnerability.ID} + existingPriorityNotification.NewPriority = vulnerability.Priority } else { // No previous notification, just add a new one - vulnerabilityPriorityIncreasedNotifications[vulnerability.ID] = &VulnerabilityPriorityIncreasedNotification{OldPriority: existingVulnerability.Priority, NewPriority: vulnerability.Priority, VulnerabilityID: existingVulnerability.ID} + notification := &VulnerabilityPriorityIncreasedNotification{OldPriority: existingVulnerability.Priority, NewPriority: vulnerability.Priority, VulnerabilityID: existingVulnerability.ID} + notifications = append(notifications, notification) + vulnerabilityPriorityIncreasedNotifications[vulnerability.ID] = notification } } } @@ -209,12 +199,15 @@ func InsertVulnerabilities(vulnerabilities []*Vulnerability) ([]Notification, er t.AddQuad(cayley.Quad(existingVulnerability.Node, FieldVulnerabilityPriority, string(vulnerability.Priority), "")) existingVulnerability.Priority = vulnerability.Priority } + if vulnerability.Description != "" && existingVulnerability.Description != vulnerability.Description { t.RemoveQuad(cayley.Quad(existingVulnerability.Node, FieldVulnerabilityDescription, existingVulnerability.Description, "")) t.AddQuad(cayley.Quad(existingVulnerability.Node, FieldVulnerabilityDescription, vulnerability.Description, "")) existingVulnerability.Description = vulnerability.Description } - if len(vulnerability.FixedInNodes) > 0 && len(utils.CompareStringLists(vulnerability.FixedInNodes, existingVulnerability.FixedInNodes)) != 0 { + + newFixedInNodes := utils.CompareStringLists(vulnerability.FixedInNodes, existingVulnerability.FixedInNodes) + if len(newFixedInNodes) > 0 { var removedNodes []string var addedNodes []string @@ -222,19 +215,14 @@ func InsertVulnerabilities(vulnerabilities []*Vulnerability) ([]Notification, er if err != nil { return []Notification{}, err } - vulnerabilityFixedInPackages, err := FindAllPackagesByNodes(vulnerability.FixedInNodes, []string{FieldPackageOS, FieldPackageName, FieldPackageVersion}) + newFixedInPackages, err := FindAllPackagesByNodes(newFixedInNodes, []string{FieldPackageOS, FieldPackageName, FieldPackageVersion}) if err != nil { return []Notification{}, err } - for _, p := range vulnerabilityFixedInPackages { - // Any already existing link ? - fixedInLinkAlreadyExists := false + for _, p := range newFixedInPackages { for _, ep := range existingVulnerabilityFixedInPackages { - if *p == *ep { - // This exact link already exists, we won't insert it again - fixedInLinkAlreadyExists = true - } else if p.Branch() == ep.Branch() { + if p.Branch() == ep.Branch() { // A link to this package branch already exist and is not the same version, we will delete it t.RemoveQuad(cayley.Quad(existingVulnerability.Node, FieldVulnerabilityFixedIn, ep.Node, "")) @@ -250,25 +238,23 @@ func InsertVulnerabilities(vulnerabilities []*Vulnerability) ([]Notification, er } } - if fixedInLinkAlreadyExists == false { - t.AddQuad(cayley.Quad(existingVulnerability.Node, FieldVulnerabilityFixedIn, p.Node, "")) - existingVulnerability.FixedInNodes = append(existingVulnerability.FixedInNodes, p.Node) - addedNodes = append(addedNodes, p.Node) - } + t.AddQuad(cayley.Quad(existingVulnerability.Node, FieldVulnerabilityFixedIn, p.Node, "")) + existingVulnerability.FixedInNodes = append(existingVulnerability.FixedInNodes, p.Node) + addedNodes = append(addedNodes, p.Node) } // Add notification about the FixedIn modification if the vulnerability is not new - if len(removedNodes) > 0 || len(addedNodes) > 0 { - if _, newVulnerabilityNotificationExists := newVulnerabilityNotifications[vulnerability.ID]; !newVulnerabilityNotificationExists { - // Any VulnerabilityPackageChangedNotification already ? - if existingPackageNotification, _ := vulnerabilityPackageChangedNotifications[vulnerability.ID]; existingPackageNotification != nil { - // There is a priority change notification, add the packages modifications to it - existingPackageNotification.AddedFixedInNodes = append(existingPackageNotification.AddedFixedInNodes, addedNodes...) - existingPackageNotification.RemovedFixedInNodes = append(existingPackageNotification.RemovedFixedInNodes, removedNodes...) - } else { - // No previous notification, just add a new one - vulnerabilityPackageChangedNotifications[vulnerability.ID] = &VulnerabilityPackageChangedNotification{VulnerabilityID: vulnerability.ID, AddedFixedInNodes: addedNodes, RemovedFixedInNodes: removedNodes} - } + if _, newVulnerabilityNotificationExists := newVulnerabilityNotifications[vulnerability.ID]; !newVulnerabilityNotificationExists { + // Any VulnerabilityPackageChangedNotification already ? + if existingPackageNotification, _ := vulnerabilityPackageChangedNotifications[vulnerability.ID]; existingPackageNotification != nil { + // There is a priority change notification, add the packages modifications to it + existingPackageNotification.AddedFixedInNodes = append(existingPackageNotification.AddedFixedInNodes, addedNodes...) + existingPackageNotification.RemovedFixedInNodes = append(existingPackageNotification.RemovedFixedInNodes, removedNodes...) + } else { + // No previous notification, just add a new one + notification := &VulnerabilityPackageChangedNotification{VulnerabilityID: vulnerability.ID, AddedFixedInNodes: addedNodes, RemovedFixedInNodes: removedNodes} + notifications = append(notifications, notification) + vulnerabilityPackageChangedNotifications[vulnerability.ID] = notification } } } @@ -281,19 +267,7 @@ func InsertVulnerabilities(vulnerabilities []*Vulnerability) ([]Notification, er return []Notification{}, ErrTransaction } - // Group all notifications - var allNotifications []Notification - for _, notification := range newVulnerabilityNotifications { - allNotifications = append(allNotifications, notification) - } - for _, notification := range vulnerabilityPriorityIncreasedNotifications { - allNotifications = append(allNotifications, notification) - } - for _, notification := range vulnerabilityPackageChangedNotifications { - allNotifications = append(allNotifications, notification) - } - - return allNotifications, nil + return notifications, nil } // DeleteVulnerability deletes the vulnerability having the given ID diff --git a/database/vulnerability_test.go b/database/vulnerability_test.go index 18dfa4c2..b9f2a8cf 100644 --- a/database/vulnerability_test.go +++ b/database/vulnerability_test.go @@ -155,9 +155,7 @@ func TestVulnerability(t *testing.T) { pkg1 = &Package{OS: "testOS", Name: "testpkg1", Version: types.NewVersionUnsafe("1.0")} pkg1b := &Package{OS: "testOS", Name: "testpkg1", Version: types.NewVersionUnsafe("1.1")} InsertPackages([]*Package{pkg1, pkg1b}) - // # A vulnerability can't be inserted if fixed by two packages of the same branch - _, err = InsertVulnerabilities([]*Vulnerability{&Vulnerability{ID: "test6", Link: "link6", Priority: types.Medium, Description: "testDescription6", FixedInNodes: []string{pkg1.Node, pkg1b.Node}}}) - assert.Error(t, err) + // # Two updates of the same vulnerability in the same batch with packages of the same branch pkg0 := &Package{OS: "testOS", Name: "testpkg0", Version: types.NewVersionUnsafe("1.0")} InsertPackages([]*Package{pkg0}) @@ -173,10 +171,6 @@ func TestVulnerability(t *testing.T) { assert.NotContains(t, v7.FixedInNodes, pkg1.Node) assert.Contains(t, v7.FixedInNodes, pkg1b.Node) } - - // # A vulnerability can't be updated if fixed by two packages of the same branch - _, err = InsertVulnerabilities([]*Vulnerability{&Vulnerability{ID: "test7", FixedInNodes: []string{pkg1.Node, pkg1b.Node}}}) - assert.Error(t, err) } } }