updater/database: do not create notifications during the initial update

api/v1/routes.go

@@ -159,7 +159,7 @@ func postVulnerability(w http.ResponseWriter, r *http.Request, p httprouter.Para
 		return postVulnerabilityRoute, http.StatusBadRequest
 	}
 
-	err = ctx.Store.InsertVulnerabilities([]database.Vulnerability{vuln})
+	err = ctx.Store.InsertVulnerabilities([]database.Vulnerability{vuln}, true)
 	if err != nil {
 		writeResponse(w, http.StatusInternalServerError, VulnerabilityEnvelope{Error: &Error{err.Error()}})
 		return postVulnerabilityRoute, http.StatusInternalServerError
@@ -211,7 +211,7 @@ func putVulnerability(w http.ResponseWriter, r *http.Request, p httprouter.Param
 		return putVulnerabilityRoute, http.StatusBadRequest
 	}
 
-	err = ctx.Store.InsertVulnerabilities([]database.Vulnerability{vuln})
+	err = ctx.Store.InsertVulnerabilities([]database.Vulnerability{vuln}, true)
 	if err != nil {
 		writeResponse(w, http.StatusInternalServerError, VulnerabilityEnvelope{Error: &Error{err.Error()}})
 		return putVulnerabilityRoute, http.StatusInternalServerError

database/database.go

@@ -46,7 +46,7 @@ type Datastore interface {
 	DeleteLayer(name string) error
 
 	// Vulnerability
-	InsertVulnerabilities([]Vulnerability) error
+	InsertVulnerabilities(vulnerabilities []Vulnerability, createNotification bool) error
 	FindVulnerability(namespaceName, name string) (Vulnerability, error)
 	DeleteVulnerability(namespaceName, name string) error
 

database/pgsql/complex_test.go

@@ -102,7 +102,7 @@ func TestRaceAffects(t *testing.T) {
 		defer wg.Done()
 		for _, vulnerabilitiesM := range vulnerabilities {
 			for _, vulnerability := range vulnerabilitiesM {
-				err = datastore.InsertVulnerabilities([]database.Vulnerability{vulnerability})
+				err = datastore.InsertVulnerabilities([]database.Vulnerability{vulnerability}, true)
 				assert.Nil(t, err)
 			}
 		}

database/pgsql/vulnerability.go

@@ -123,9 +123,9 @@ func scanVulnerability(queryer Queryer, queryName string, vulnerabilityRow *sql.
 
 // FixedIn.Namespace are not necessary, they are overwritten by the vuln.
 // By setting the fixed version to minVersion, we can say that the vuln does'nt affect anymore.
-func (pgSQL *pgSQL) InsertVulnerabilities(vulnerabilities []database.Vulnerability) error {
+func (pgSQL *pgSQL) InsertVulnerabilities(vulnerabilities []database.Vulnerability, generateNotifications bool) error {
 	for _, vulnerability := range vulnerabilities {
-		err := pgSQL.insertVulnerability(vulnerability, false)
+		err := pgSQL.insertVulnerability(vulnerability, false, generateNotifications)
 		if err != nil {
 			fmt.Printf("%#v\n", vulnerability)
 			return err
@@ -134,7 +134,7 @@ func (pgSQL *pgSQL) InsertVulnerabilities(vulnerabilities []database.Vulnerabili
 	return nil
 }
 
-func (pgSQL *pgSQL) insertVulnerability(vulnerability database.Vulnerability, onlyFixedIn bool) error {
+func (pgSQL *pgSQL) insertVulnerability(vulnerability database.Vulnerability, onlyFixedIn, generateNotification bool) error {
 	tf := time.Now()
 
 	// Verify parameters
@@ -246,9 +246,11 @@ func (pgSQL *pgSQL) insertVulnerability(vulnerability database.Vulnerability, on
 	}
 
 	// Create a notification.
-	err = createNotification(tx, existingVulnerability.ID, vulnerability.ID)
-	if err != nil {
-		return err
+	if generateNotification {
+		err = createNotification(tx, existingVulnerability.ID, vulnerability.ID)
+		if err != nil {
+			return err
+		}
 	}
 
 	// Commit transaction.
@@ -441,7 +443,7 @@ func (pgSQL *pgSQL) InsertVulnerabilityFixes(vulnerabilityNamespace, vulnerabili
 		FixedIn: fixes,
 	}
 
-	return pgSQL.insertVulnerability(v, true)
+	return pgSQL.insertVulnerability(v, true, true)
 }
 
 func (pgSQL *pgSQL) DeleteVulnerabilityFix(vulnerabilityNamespace, vulnerabilityName, featureName string) error {
@@ -465,7 +467,7 @@ func (pgSQL *pgSQL) DeleteVulnerabilityFix(vulnerabilityNamespace, vulnerability
 		},
 	}
 
-	return pgSQL.insertVulnerability(v, true)
+	return pgSQL.insertVulnerability(v, true, true)
 }
 
 func (pgSQL *pgSQL) DeleteVulnerability(namespaceName, name string) error {

database/pgsql/vulnerability_test.go

@@ -191,7 +191,7 @@ func TestInsertVulnerability(t *testing.T) {
 			Severity:  types.Unknown,
 		},
 	} {
-		err := datastore.InsertVulnerabilities([]database.Vulnerability{vulnerability})
+		err := datastore.InsertVulnerabilities([]database.Vulnerability{vulnerability}, true)
 		assert.Error(t, err)
 	}
 
@@ -213,7 +213,7 @@ func TestInsertVulnerability(t *testing.T) {
 		Link:        "TestInsertVulnerabilityLink1",
 		Metadata:    v1meta,
 	}
-	err = datastore.InsertVulnerabilities([]database.Vulnerability{v1})
+	err = datastore.InsertVulnerabilities([]database.Vulnerability{v1}, true)
 	if assert.Nil(t, err) {
 		v1f, err := datastore.FindVulnerability(n1.Name, v1.Name)
 		if assert.Nil(t, err) {
@@ -229,7 +229,7 @@ func TestInsertVulnerability(t *testing.T) {
 	// adding f8 which is f7 but with MinVersion.
 	v1.FixedIn = []database.FeatureVersion{f4, f5, f6, f8}
 
-	err = datastore.InsertVulnerabilities([]database.Vulnerability{v1})
+	err = datastore.InsertVulnerabilities([]database.Vulnerability{v1}, true)
 	if assert.Nil(t, err) {
 		v1f, err := datastore.FindVulnerability(n1.Name, v1.Name)
 		if assert.Nil(t, err) {
@@ -274,63 +274,3 @@ func equalsVuln(t *testing.T, expected, actual *database.Vulnerability) {
 		}
 	}
 }
-
-// func TestInsertVulnerabilityNotifications(t *testing.T) {
-// 	Open(&config.DatabaseConfig{Type: "memstore"})
-// 	defer Close()
-//
-// 	pkg1 := &Package{OS: "testOS", Name: "testpkg1", Version: types.NewVersionUnsafe("1.0")}
-// 	pkg1b := &Package{OS: "testOS", Name: "testpkg1", Version: types.NewVersionUnsafe("1.2")}
-// 	pkg2 := &Package{OS: "testOS", Name: "testpkg2", Version: types.NewVersionUnsafe("1.0")}
-// 	InsertPackages([]*Package{pkg1, pkg1b, pkg2})
-//
-// 	// Newdatabase.VulnerabilityNotification
-// 	vuln1 := &database.Vulnerability{ID: "test1", Link: "link1", Priority: types.Medium, Description: "testDescription1", FixedInNodes: []string{pkg1.Node}}
-// 	vuln2 := &database.Vulnerability{ID: "test2", Link: "link2", Priority: types.High, Description: "testDescription2", FixedInNodes: []string{pkg1.Node, pkg2.Node}}
-// 	vuln1b := &database.Vulnerability{ID: "test1", Priority: types.High, FixedInNodes: []string{"pkg3"}}
-// 	notifications, err := InsertVulnerabilities([]*database.Vulnerability{vuln1, vuln2, vuln1b})
-// 	if assert.Nil(t, err) {
-// 		// We should only have two Newdatabase.VulnerabilityNotification notifications: one for test1 and one for test2
-// 		// We should not have a database.VulnerabilityPriorityIncreasedNotification or a database.VulnerabilityPackageChangedNotification
-// 		// for test1 because it is in the same batch
-// 		if assert.Len(t, notifications, 2) {
-// 			for _, n := range notifications {
-// 				_, ok := n.(*Newdatabase.VulnerabilityNotification)
-// 				assert.True(t, ok)
-// 			}
-// 		}
-// 	}
-//
-// 	// database.VulnerabilityPriorityIncreasedNotification
-// 	vuln1c := &database.Vulnerability{ID: "test1", Priority: types.Critical}
-// 	notifications, err = InsertVulnerabilities([]*database.Vulnerability{vuln1c})
-// 	if assert.Nil(t, err) {
-// 		if assert.Len(t, notifications, 1) {
-// 			if nn, ok := notifications[0].(*database.VulnerabilityPriorityIncreasedNotification); assert.True(t, ok) {
-// 				assert.Equal(t, vuln1b.Priority, nn.OldPriority)
-// 				assert.Equal(t, vuln1c.Priority, nn.NewPriority)
-// 			}
-// 		}
-// 	}
-//
-// 	notifications, err = InsertVulnerabilities([]*database.Vulnerability{&database.Vulnerability{ID: "test1", Priority: types.Low}})
-// 	assert.Nil(t, err)
-// 	assert.Len(t, notifications, 0)
-//
-// 	// database.VulnerabilityPackageChangedNotification
-// 	vuln1e := &database.Vulnerability{ID: "test1", FixedInNodes: []string{pkg1b.Node}}
-// 	vuln1f := &database.Vulnerability{ID: "test1", FixedInNodes: []string{pkg2.Node}}
-// 	notifications, err = InsertVulnerabilities([]*database.Vulnerability{vuln1e, vuln1f})
-// 	if assert.Nil(t, err) {
-// 		if assert.Len(t, notifications, 1) {
-// 			if nn, ok := notifications[0].(*database.VulnerabilityPackageChangedNotification); assert.True(t, ok) {
-// 				// Here, we say that pkg1b fixes the database.Vulnerability, but as pkg1b is in
-// 				// the same branch as pkg1, pkg1 should be removed and pkg1b added
-// 				// We also add pkg2 as fixed
-// 				assert.Contains(t, nn.AddedFixedInNodes, pkg1b.Node)
-// 				assert.Contains(t, nn.RemovedFixedInNodes, pkg1.Node)
-//
-// 				assert.Contains(t, nn.AddedFixedInNodes, pkg2.Node)
-// 			}
-// 		}
-// 	}

updater/updater.go

@@ -78,15 +78,17 @@ func Run(config *config.UpdaterConfig, datastore database.Datastore, st *utils.S
 	log.Infof("updater service started. lock identifier: %s", whoAmI)
 
 	for {
-		// Set the next update time to (last update time + interval) or now if there
-		// is no last update time stored in database (first update) or if an error
-		// occurs.
-		var nextUpdate time.Time
 		var stop bool
-		if lastUpdate := getLastUpdate(datastore); !lastUpdate.IsZero() {
+
+		// Determine if this is the first update and define the next update time.
+		// The next update time is (last update time + interval) or now if this is the first update.
+		nextUpdate := time.Now().UTC()
+		lastUpdate, firstUpdate, err := getLastUpdate(datastore)
+		if err != nil {
+			log.Errorf("an error occured while getting the last update time")
+			nextUpdate = nextUpdate.Add(config.Interval)
+		} else if firstUpdate == false {
 			nextUpdate = lastUpdate.Add(config.Interval)
-		} else {
-			nextUpdate = time.Now().UTC()
 		}
 
 		// If the next update timer is in the past, then try to update.
@@ -98,7 +100,7 @@ func Run(config *config.UpdaterConfig, datastore database.Datastore, st *utils.S
 				// Launch update in a new go routine.
 				doneC := make(chan bool, 1)
 				go func() {
-					Update(datastore)
+					Update(datastore, firstUpdate)
 					doneC <- true
 				}()
 
@@ -157,7 +159,7 @@ func Run(config *config.UpdaterConfig, datastore database.Datastore, st *utils.S
 
 // Update fetches all the vulnerabilities from the registered fetchers, upserts
 // them into the database and then sends notifications.
-func Update(datastore database.Datastore) {
+func Update(datastore database.Datastore, firstUpdate bool) {
 	defer setUpdaterDuration(time.Now())
 
 	log.Info("updating vulnerabilities")
@@ -167,7 +169,7 @@ func Update(datastore database.Datastore) {
 
 	// Insert vulnerabilities.
 	log.Tracef("inserting %d vulnerabilities for update", len(vulnerabilities))
-	err := datastore.InsertVulnerabilities(vulnerabilities)
+	err := datastore.InsertVulnerabilities(vulnerabilities, !firstUpdate)
 	if err != nil {
 		promUpdaterErrorsTotal.Inc()
 		log.Errorf("an error occured when inserting vulnerabilities for update: %s", err)
@@ -284,13 +286,23 @@ func addMetadata(datastore database.Datastore, vulnerabilities []database.Vulner
 	return vulnerabilities
 }
 
-func getLastUpdate(datastore database.Datastore) time.Time {
-	if lastUpdateTSS, err := datastore.GetKeyValue(flagName); err == nil && lastUpdateTSS != "" {
-		if lastUpdateTS, err := strconv.ParseInt(lastUpdateTSS, 10, 64); err == nil {
-			return time.Unix(lastUpdateTS, 0).UTC()
-		}
+func getLastUpdate(datastore database.Datastore) (time.Time, bool, error) {
+	lastUpdateTSS, err := datastore.GetKeyValue(flagName)
+	if err != nil {
+		return time.Time{}, false, err
+	}
+
+	if lastUpdateTSS == "" {
+		// This is the first update.
+		return time.Time{}, true, nil
 	}
-	return time.Time{}
+
+	lastUpdateTS, err := strconv.ParseInt(lastUpdateTSS, 10, 64)
+	if err != nil {
+		return time.Time{}, false, err
+	}
+
+	return time.Unix(lastUpdateTS, 0).UTC(), false, nil
 }
 
 // doVulnerabilitiesNamespacing takes Vulnerabilities that don't have a Namespace and split them