diff --git a/api/v1/models.go b/api/v1/models.go index 5d41cbaa..f063417c 100644 --- a/api/v1/models.go +++ b/api/v1/models.go @@ -162,7 +162,7 @@ type Notification struct { Page string `json:"Page,omitempty"` NextPage string `json:"NextPage,omitempty"` Old *VulnerabilityWithLayers `json:"Old,omitempty"` - New VulnerabilityWithLayers `json:"New,omitempty"` + New *VulnerabilityWithLayers `json:"New,omitempty"` Changed []string `json:"Changed,omitempty"` } @@ -172,6 +172,11 @@ func NotificationFromDatabaseModel(dbNotification database.VulnerabilityNotifica *oldVuln = VulnerabilityWithLayersFromDatabaseModel(*dbNotification.OldVulnerability) } + var newVuln *VulnerabilityWithLayers + if dbNotification.NewVulnerability != nil { + *newVuln = VulnerabilityWithLayersFromDatabaseModel(*dbNotification.NewVulnerability) + } + var nextPageStr string if nextPage != database.NoVulnerabilityNotificationPage { nextPageStr = DBPageNumberToString(nextPage) @@ -187,7 +192,7 @@ func NotificationFromDatabaseModel(dbNotification database.VulnerabilityNotifica Page: DBPageNumberToString(page), NextPage: nextPageStr, Old: oldVuln, - New: VulnerabilityWithLayersFromDatabaseModel(dbNotification.NewVulnerability), + New: newVuln, } } diff --git a/database/models.go b/database/models.go index de1e73c6..97de789e 100644 --- a/database/models.go +++ b/database/models.go @@ -61,8 +61,9 @@ type FeatureVersion struct { type Vulnerability struct { Model - Name string - Namespace Namespace + Name string + Namespace Namespace + Description string Link string Severity types.Priority @@ -102,7 +103,7 @@ type VulnerabilityNotification struct { Deleted time.Time OldVulnerability *Vulnerability - NewVulnerability Vulnerability + NewVulnerability *Vulnerability } type VulnerabilityNotificationPageNumber struct { diff --git a/database/pgsql/complex_test.go b/database/pgsql/complex_test.go index b7ca911b..ac08f001 100644 --- a/database/pgsql/complex_test.go +++ b/database/pgsql/complex_test.go @@ -155,6 +155,4 @@ func TestRaceAffects(t *testing.T) { assert.Len(t, utils.CompareStringLists(expectedAffectedNames, actualAffectedNames), 0) assert.Len(t, utils.CompareStringLists(actualAffectedNames, expectedAffectedNames), 0) } - - // TODO(Quentin-M): May be worth having a test for updates as well. } diff --git a/database/pgsql/migrations/20151222113213_Initial.sql b/database/pgsql/migrations/20151222113213_Initial.sql index d1db537b..77f0bd76 100644 --- a/database/pgsql/migrations/20151222113213_Initial.sql +++ b/database/pgsql/migrations/20151222113213_Initial.sql @@ -89,8 +89,8 @@ CREATE TABLE IF NOT EXISTS Vulnerability ( link VARCHAR(128) NULL, severity severity NOT NULL, metadata TEXT NULL, - - UNIQUE (namespace_id, name)); + created_at TIMESTAMP WITH TIME ZONE, + deleted_at TIMESTAMP WITH TIME ZONE NULL); -- ----------------------------------------------------- @@ -152,8 +152,8 @@ CREATE TABLE IF NOT EXISTS Vulnerability_Notification ( created_at TIMESTAMP WITH TIME ZONE, notified_at TIMESTAMP WITH TIME ZONE NULL, deleted_at TIMESTAMP WITH TIME ZONE NULL, - old_vulnerability TEXT NULL, - new_vulnerability TEXT); + old_vulnerability_id INT NULL REFERENCES Vulnerability ON DELETE CASCADE, + new_vulnerability_id INT NULL REFERENCES Vulnerability ON DELETE CASCADE); CREATE INDEX ON Vulnerability_Notification (notified_at); diff --git a/database/pgsql/notification.go b/database/pgsql/notification.go index 63d6beb3..3989f360 100644 --- a/database/pgsql/notification.go +++ b/database/pgsql/notification.go @@ -2,7 +2,6 @@ package pgsql import ( "database/sql" - "encoding/json" "time" "github.com/coreos/clair/database" @@ -13,29 +12,13 @@ import ( // do it in tx so we won't insert/update a vuln without notification and vice-versa. // name and created doesn't matter. -// Vuln ID must be filled in. -func (pgSQL *pgSQL) insertNotification(tx *sql.Tx, notification database.VulnerabilityNotification) error { - defer observeQueryTime("insertNotification", "all", time.Now()) - - // Marshal old and new Vulnerabilities. - var oldVulnerability sql.NullString - if notification.OldVulnerability != nil { - oldVulnerabilityJSON, err := json.Marshal(notification.OldVulnerability) - if err != nil { - tx.Rollback() - return cerrors.NewBadRequestError("could not marshal old Vulnerability in insertNotification") - } - oldVulnerability = sql.NullString{String: string(oldVulnerabilityJSON), Valid: true} - } - - newVulnerability, err := json.Marshal(notification.NewVulnerability) - if err != nil { - tx.Rollback() - return cerrors.NewBadRequestError("could not marshal new Vulnerability in insertNotification") - } +func createNotification(tx *sql.Tx, oldVulnerabilityID, newVulnerabilityID int) error { + defer observeQueryTime("createNotification", "all", time.Now()) // Insert Notification. - _, err = tx.Exec(getQuery("i_notification"), uuid.New(), oldVulnerability, newVulnerability) + oldVulnerabilityNullableID := sql.NullInt64{Int64: int64(oldVulnerabilityID), Valid: oldVulnerabilityID != 0} + newVulnerabilityNullableID := sql.NullInt64{Int64: int64(newVulnerabilityID), Valid: newVulnerabilityID != 0} + _, err := tx.Exec(getQuery("i_notification"), uuid.New(), oldVulnerabilityNullableID, newVulnerabilityNullableID) if err != nil { tx.Rollback() return handleError("i_notification", err) @@ -51,7 +34,7 @@ func (pgSQL *pgSQL) GetAvailableNotification(renotifyInterval time.Duration) (da before := time.Now().Add(-renotifyInterval) row := pgSQL.QueryRow(getQuery("s_notification_available"), before) - notification, err := scanNotification(row, false) + notification, err := pgSQL.scanNotification(row, false) return notification, handleError("s_notification_available", err) } @@ -60,20 +43,28 @@ func (pgSQL *pgSQL) GetNotification(name string, limit int, page database.Vulner defer observeQueryTime("GetNotification", "all", time.Now()) // Get Notification. - notification, err := scanNotification(pgSQL.QueryRow(getQuery("s_notification"), name), true) + notification, err := pgSQL.scanNotification(pgSQL.QueryRow(getQuery("s_notification"), name), true) if err != nil { return notification, page, handleError("s_notification", err) } // Load vulnerabilities' LayersIntroducingVulnerability. page.OldVulnerability, err = pgSQL.loadLayerIntroducingVulnerability( - notification.OldVulnerability, limit, page.OldVulnerability) + notification.OldVulnerability, + limit, + page.OldVulnerability, + ) + if err != nil { return notification, page, err } page.NewVulnerability, err = pgSQL.loadLayerIntroducingVulnerability( - ¬ification.NewVulnerability, limit, page.NewVulnerability) + notification.NewVulnerability, + limit, + page.NewVulnerability, + ) + if err != nil { return notification, page, err } @@ -81,22 +72,35 @@ func (pgSQL *pgSQL) GetNotification(name string, limit int, page database.Vulner return notification, page, nil } -func scanNotification(row *sql.Row, hasVulns bool) (notification database.VulnerabilityNotification, err error) { +func (pgSQL *pgSQL) scanNotification(row *sql.Row, hasVulns bool) (database.VulnerabilityNotification, error) { + var notification database.VulnerabilityNotification var created zero.Time var notified zero.Time var deleted zero.Time - var oldVulnerability []byte - var newVulnerability []byte + var oldVulnerabilityNullableID sql.NullInt64 + var newVulnerabilityNullableID sql.NullInt64 - // Query notification. + // Scan notification. if hasVulns { - err = row.Scan(¬ification.ID, ¬ification.Name, &created, ¬ified, &deleted, - &oldVulnerability, &newVulnerability) + err := row.Scan( + ¬ification.ID, + ¬ification.Name, + &created, + ¬ified, + &deleted, + &oldVulnerabilityNullableID, + &newVulnerabilityNullableID, + ) + + if err != nil { + return notification, err + } } else { - err = row.Scan(¬ification.ID, ¬ification.Name, &created, ¬ified, &deleted) - } - if err != nil { - return + err := row.Scan(¬ification.ID, ¬ification.Name, &created, ¬ified, &deleted) + + if err != nil { + return notification, err + } } notification.Created = created.Time @@ -104,19 +108,26 @@ func scanNotification(row *sql.Row, hasVulns bool) (notification database.Vulner notification.Deleted = deleted.Time if hasVulns { - // Unmarshal old and new Vulnerabilities. - err = json.Unmarshal(oldVulnerability, notification.OldVulnerability) - if err != nil { - err = cerrors.NewBadRequestError("could not unmarshal old Vulnerability in GetNotification") + if oldVulnerabilityNullableID.Valid { + vulnerability, err := pgSQL.findVulnerabilityByIDWithDeleted(int(oldVulnerabilityNullableID.Int64)) + if err != nil { + return notification, err + } + + notification.OldVulnerability = &vulnerability } - err = json.Unmarshal(newVulnerability, ¬ification.NewVulnerability) - if err != nil { - err = cerrors.NewBadRequestError("could not unmarshal new Vulnerability in GetNotification") + if newVulnerabilityNullableID.Valid { + vulnerability, err := pgSQL.findVulnerabilityByIDWithDeleted(int(newVulnerabilityNullableID.Int64)) + if err != nil { + return notification, err + } + + notification.NewVulnerability = &vulnerability } } - return + return notification, nil } // Fills Vulnerability.LayersIntroducingVulnerability. diff --git a/database/pgsql/notification_test.go b/database/pgsql/notification_test.go index 6041f58f..05edb41d 100644 --- a/database/pgsql/notification_test.go +++ b/database/pgsql/notification_test.go @@ -4,8 +4,6 @@ import ( "testing" "time" - "fmt" - "github.com/coreos/clair/database" cerrors "github.com/coreos/clair/utils/errors" "github.com/coreos/clair/utils/types" @@ -60,64 +58,68 @@ func TestNotification(t *testing.T) { }, } - if assert.Nil(t, datastore.InsertLayer(l1)) && assert.Nil(t, datastore.InsertLayer(l2)) && - assert.Nil(t, datastore.InsertLayer(l3)) { - - // Insert a new vulnerability that is introduced by three layers. - v1 := database.Vulnerability{ - Name: "TestNotificationVulnerability1", - Namespace: f1.Namespace, - Description: "TestNotificationDescription1", - Link: "TestNotificationLink1", - Severity: "Unknown", - FixedIn: []database.FeatureVersion{ - database.FeatureVersion{ - Feature: f1, - Version: types.NewVersionUnsafe("1.0"), - }, - }, - } - assert.Nil(t, datastore.insertVulnerability(v1)) - - // Get the notification associated to the previously inserted vulnerability. - notification, err := datastore.GetAvailableNotification(time.Second) - assert.Nil(t, err) - assert.NotEmpty(t, notification.Name) - - // Verify the renotify behaviour. - if assert.Nil(t, datastore.SetNotificationNotified(notification.Name)) { - _, err := datastore.GetAvailableNotification(time.Second) - assert.Equal(t, cerrors.ErrNotFound, err) - - time.Sleep(50 * time.Millisecond) - notificationB, err := datastore.GetAvailableNotification(20 * time.Millisecond) - assert.Nil(t, err) - assert.Equal(t, notification.Name, notificationB.Name) - - datastore.SetNotificationNotified(notification.Name) - } - - // Get notification. - filledNotification, nextPage, err := datastore.GetNotification(notification.Name, 2, database.VulnerabilityNotificationFirstPage) - assert.Nil(t, err) - assert.NotEqual(t, database.NoVulnerabilityNotificationPage, nextPage) - assert.Nil(t, filledNotification.OldVulnerability) - assert.Equal(t, v1.Name, filledNotification.NewVulnerability.Name) - assert.Len(t, filledNotification.NewVulnerability.LayersIntroducingVulnerability, 2) - - // Get second page. - filledNotification, nextPage, err = datastore.GetNotification(notification.Name, 2, nextPage) - assert.Nil(t, err) - assert.Equal(t, database.NoVulnerabilityNotificationPage, nextPage) - assert.Nil(t, filledNotification.OldVulnerability) - assert.Equal(t, v1.Name, filledNotification.NewVulnerability.Name) - assert.Len(t, filledNotification.NewVulnerability.LayersIntroducingVulnerability, 1) - - // Delete notification. - assert.Nil(t, datastore.DeleteNotification(notification.Name)) - - n, err := datastore.GetAvailableNotification(time.Millisecond) - assert.Equal(t, cerrors.ErrNotFound, err) - fmt.Println(n) + if !assert.Nil(t, datastore.InsertLayer(l1)) || !assert.Nil(t, datastore.InsertLayer(l2)) || !assert.Nil(t, datastore.InsertLayer(l3)) { + return } + + // Insert a new vulnerability that is introduced by three layers. + v1 := database.Vulnerability{ + Name: "TestNotificationVulnerability1", + Namespace: f1.Namespace, + Description: "TestNotificationDescription1", + Link: "TestNotificationLink1", + Severity: "Unknown", + FixedIn: []database.FeatureVersion{ + database.FeatureVersion{ + Feature: f1, + Version: types.NewVersionUnsafe("1.0"), + }, + }, + } + assert.Nil(t, datastore.insertVulnerability(v1, false)) + + // Get the notification associated to the previously inserted vulnerability. + notification, err := datastore.GetAvailableNotification(time.Second) + assert.Nil(t, err) + assert.NotEmpty(t, notification.Name) + + // Verify the renotify behaviour. + if assert.Nil(t, datastore.SetNotificationNotified(notification.Name)) { + _, err := datastore.GetAvailableNotification(time.Second) + assert.Equal(t, cerrors.ErrNotFound, err) + + time.Sleep(50 * time.Millisecond) + notificationB, err := datastore.GetAvailableNotification(20 * time.Millisecond) + assert.Nil(t, err) + assert.Equal(t, notification.Name, notificationB.Name) + + datastore.SetNotificationNotified(notification.Name) + } + + // Get notification. + filledNotification, nextPage, err := datastore.GetNotification(notification.Name, 2, database.VulnerabilityNotificationFirstPage) + assert.Nil(t, err) + assert.NotEqual(t, database.NoVulnerabilityNotificationPage, nextPage) + assert.Nil(t, filledNotification.OldVulnerability) + assert.Equal(t, v1.Name, filledNotification.NewVulnerability.Name) + assert.Len(t, filledNotification.NewVulnerability.LayersIntroducingVulnerability, 2) + + // Get second page. + filledNotification, nextPage, err = datastore.GetNotification(notification.Name, 2, nextPage) + assert.Nil(t, err) + assert.Equal(t, database.NoVulnerabilityNotificationPage, nextPage) + assert.Nil(t, filledNotification.OldVulnerability) + assert.Equal(t, v1.Name, filledNotification.NewVulnerability.Name) + assert.Len(t, filledNotification.NewVulnerability.LayersIntroducingVulnerability, 1) + + // Delete notification. + assert.Nil(t, datastore.DeleteNotification(notification.Name)) + + _, err = datastore.GetAvailableNotification(time.Millisecond) + assert.Equal(t, cerrors.ErrNotFound, err) + + // Update a vulnerability and ensure that the old/new vulnerabilities are correct. + + // Delete a vulnerability and verify the notification. + } diff --git a/database/pgsql/queries.go b/database/pgsql/queries.go index 62d2a99c..773cfeaf 100644 --- a/database/pgsql/queries.go +++ b/database/pgsql/queries.go @@ -111,7 +111,8 @@ func init() { WHERE vafv.featureversion_id = ANY($1::integer[]) AND vafv.vulnerability_id = v.id AND vafv.fixedin_id = vfif.id - AND v.namespace_id = vn.id` + AND v.namespace_id = vn.id + AND v.deleted_at IS NULL` queries["i_layer"] = ` INSERT INTO Layer(name, engineversion, parent_id, namespace_id) @@ -143,63 +144,43 @@ func init() { queries["r_lock_expired"] = `DELETE FROM LOCK WHERE until < CURRENT_TIMESTAMP` // vulnerability.go - queries["f_vulnerability"] = ` - SELECT v.id, n.id, v.description, v.link, v.severity, v.metadata, vfif.version, f.id, f.Name - FROM Vulnerability v - JOIN Namespace n ON v.namespace_id = n.id - LEFT JOIN Vulnerability_FixedIn_Feature vfif ON v.id = vfif.vulnerability_id - LEFT JOIN Feature f ON vfif.feature_id = f.id - WHERE n.Name = $1 AND v.Name = $2` + queries["f_vulnerability_base"] = ` + SELECT v.id, v.name, n.id, n.name, v.description, v.link, v.severity, v.metadata + FROM Vulnerability v JOIN Namespace n ON v.namespace_id = n.id` - queries["f_vulnerability_for_update"] = ` - SELECT FOR UPDATE v.id, n.id, v.description, v.link, v.severity, v.metadata, vfif.version, f.id, f.Name - FROM Vulnerability v - JOIN Namespace n ON v.namespace_id = n.id - LEFT JOIN Vulnerability_FixedIn_Feature vfif ON v.id = vfif.vulnerability_id - LEFT JOIN Feature f ON vfif.feature_id = f.id - WHERE n.Name = $1 AND v.Name = $2` + queries["f_vulnerability_for_update"] = ` FOR UPDATE OF v` + queries["f_vulnerability_+by_name_namespace"] = ` WHERE n.name = $1 AND v.name = $2 AND v.deleted_at IS NULL` + queries["f_vulnerability_+by_id"] = ` WHERE v.id = $1` + + queries["f_vulnerability_fixedin"] = ` + SELECT vfif.version, f.id, f.Name + FROM Vulnerability_FixedIn_Feature vfif JOIN Feature f ON vfif.feature_id = f.id + WHERE vfif.vulnerability_id = $1` queries["i_vulnerability"] = ` INSERT INTO Vulnerability(namespace_id, name, description, link, severity, metadata) VALUES($1, $2, $3, $4, $5, $6) RETURNING id` - queries["u_vulnerability"] = ` - UPDATE Vulnerability - SET description = $2, link = $3, severity = $4, metadata = $5 - WHERE id = $1` - queries["i_vulnerability_fixedin_feature"] = ` INSERT INTO Vulnerability_FixedIn_Feature(vulnerability_id, feature_id, version) VALUES($1, $2, $3) RETURNING id` - queries["u_vulnerability_fixedin_feature"] = ` - UPDATE Vulnerability_FixedIn_Feature - SET version = $3 - WHERE vulnerability_id = $1 AND feature_id = $2 - RETURNING id` - - queries["r_vulnerability_fixedin_feature"] = ` - DELETE FROM Vulnerability_FixedIn_Feature - WHERE vulnerability_id = $1 AND feature_id = $2 - RETURNING id` - - queries["r_vulnerability_affects_featureversion"] = ` - DELETE FROM Vulnerability_Affects_FeatureVersion - WHERE fixedin_id = $1` - queries["f_featureversion_by_feature"] = ` SELECT id, version FROM FeatureVersion WHERE feature_id = $1` queries["r_vulnerability"] = ` - DELETE FROM Vulnerability + UPDATE Vulnerability + SET deleted_at = CURRENT_TIMESTAMP WHERE namespace_id = (SELECT id FROM Namespace WHERE name = $1) - AND name = $2` + AND name = $2 + AND deleted_at IS NULL + RETURNING id` // notification.go queries["i_notification"] = ` - INSERT INTO Vulnerability_Notification(name, created_at, old_vulnerability, new_vulnerability) + INSERT INTO Vulnerability_Notification(name, created_at, old_vulnerability_id, new_vulnerability_id) VALUES($1, CURRENT_TIMESTAMP, $2, $3)` queries["u_notification_notified"] = ` @@ -222,7 +203,7 @@ func init() { LIMIT 1` queries["s_notification"] = ` - SELECT id, name, created_at, notified_at, deleted_at, old_vulnerability, new_vulnerability + SELECT id, name, created_at, notified_at, deleted_at, old_vulnerability_id, new_vulnerability_id FROM Vulnerability_Notification WHERE name = $1` diff --git a/database/pgsql/vulnerability.go b/database/pgsql/vulnerability.go index 8525aab4..fdd0d7b3 100644 --- a/database/pgsql/vulnerability.go +++ b/database/pgsql/vulnerability.go @@ -35,36 +35,67 @@ func (pgSQL *pgSQL) FindVulnerability(namespaceName, name string) (database.Vuln func findVulnerability(queryer Queryer, namespaceName, name string, forUpdate bool) (database.Vulnerability, error) { defer observeQueryTime("findVulnerability", "all", time.Now()) - vulnerability := database.Vulnerability{ - Name: name, - Namespace: database.Namespace{ - Name: namespaceName, - }, - } - - // Find Vulnerability. queryName := "f_vulnerability" + query := getQuery("f_vulnerability_base") + getQuery("f_vulnerability_+by_name_namespace") if forUpdate { - queryName = "f_vulnerability_for_update" + queryName = queryName + "+for_update" + query = query + getQuery("f_vulnerability_for_update") } - rows, err := queryer.Query(getQuery(queryName), namespaceName, name) + return scanVulnerability(queryer, queryName, queryer.QueryRow(query, namespaceName, name)) +} + +func (pgSQL *pgSQL) findVulnerabilityByIDWithDeleted(id int) (database.Vulnerability, error) { + defer observeQueryTime("findVulnerabilityByIDWithDeleted", "all", time.Now()) + + queryName := "f_vulnerability" + query := getQuery("f_vulnerability_base") + getQuery("f_vulnerability_+by_id") + + return scanVulnerability(pgSQL, queryName, pgSQL.QueryRow(query, id)) +} + +func scanVulnerability(queryer Queryer, queryName string, vulnerabilityRow *sql.Row) (database.Vulnerability, error) { + var vulnerability database.Vulnerability + + err := vulnerabilityRow.Scan( + &vulnerability.ID, + &vulnerability.Name, + &vulnerability.Namespace.ID, + &vulnerability.Namespace.Name, + &vulnerability.Description, + &vulnerability.Link, + &vulnerability.Severity, + &vulnerability.Metadata, + ) + if err != nil { - return vulnerability, handleError(queryName, err) + return vulnerability, handleError(queryName+".Scan()", err) + } + + if vulnerability.ID == 0 { + return vulnerability, cerrors.ErrNotFound + } + + // Query the FixedIn FeatureVersion now. + rows, err := queryer.Query(getQuery("f_vulnerability_fixedin"), vulnerability.ID) + if err != nil { + return vulnerability, handleError("f_vulnerability_fixedin.Scan()", err) } defer rows.Close() - // Iterate to scan the Vulnerability and its FixedIn FeatureVersions. for rows.Next() { var featureVersionID zero.Int var featureVersionVersion zero.String var featureVersionFeatureName zero.String - err := rows.Scan(&vulnerability.ID, &vulnerability.Namespace.ID, &vulnerability.Description, - &vulnerability.Link, &vulnerability.Severity, &vulnerability.Metadata, - &featureVersionVersion, &featureVersionID, &featureVersionFeatureName) + err := rows.Scan( + &featureVersionVersion, + &featureVersionID, + &featureVersionFeatureName, + ) + if err != nil { - return vulnerability, handleError(queryName+".Scan()", err) + return vulnerability, handleError("f_vulnerability_fixedin.Scan()", err) } if !featureVersionID.IsZero() { @@ -82,11 +113,9 @@ func findVulnerability(queryer Queryer, namespaceName, name string, forUpdate bo vulnerability.FixedIn = append(vulnerability.FixedIn, featureVersion) } } - if err = rows.Err(); err != nil { - return vulnerability, handleError("s_featureversions_vulnerabilities.Rows()", err) - } - if vulnerability.ID == 0 { - return vulnerability, cerrors.ErrNotFound + + if err := rows.Err(); err != nil { + return vulnerability, handleError("f_vulnerability_fixedin.Rows()", err) } return vulnerability, nil @@ -96,7 +125,7 @@ func findVulnerability(queryer Queryer, namespaceName, name string, forUpdate bo // 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 { for _, vulnerability := range vulnerabilities { - err := pgSQL.insertVulnerability(vulnerability) + err := pgSQL.insertVulnerability(vulnerability, false) if err != nil { fmt.Printf("%#v\n", vulnerability) return err @@ -105,22 +134,27 @@ func (pgSQL *pgSQL) InsertVulnerabilities(vulnerabilities []database.Vulnerabili return nil } -func (pgSQL *pgSQL) insertVulnerability(vulnerability database.Vulnerability) error { +func (pgSQL *pgSQL) insertVulnerability(vulnerability database.Vulnerability, onlyFixedIn bool) error { tf := time.Now() // Verify parameters - if vulnerability.Name == "" || len(vulnerability.FixedIn) == 0 || - vulnerability.Namespace.Name == "" || !vulnerability.Severity.IsValid() { - log.Warning("could not insert an invalid vulnerability") - return cerrors.NewBadRequestError("could not insert an invalid vulnerability") + if vulnerability.Name == "" || vulnerability.Namespace.Name == "" { + return cerrors.NewBadRequestError("insertVulnerability needs at least the Name and the Namespace") } + if !onlyFixedIn && !vulnerability.Severity.IsValid() { + msg := fmt.Sprintf("could not insert a vulnerability that has an invalid Severity: %s", vulnerability.Severity) + log.Warning(msg) + return cerrors.NewBadRequestError(msg) + } + for i := 0; i < len(vulnerability.FixedIn); i++ { + fifv := &vulnerability.FixedIn[i] - for _, fixedInFeatureVersion := range vulnerability.FixedIn { - if fixedInFeatureVersion.Feature.Namespace.Name == "" { - fixedInFeatureVersion.Feature.Namespace.Name = vulnerability.Namespace.Name - } else if fixedInFeatureVersion.Feature.Namespace.Name != vulnerability.Namespace.Name { - msg := "could not insert an invalid vulnerability: FixedIn FeatureVersion must be in the " + - "same namespace as the Vulnerability" + if fifv.Feature.Namespace.Name == "" { + // As there is no Namespace on that FixedIn FeatureVersion, set it to the Vulnerability's + // Namespace. + fifv.Feature.Namespace.Name = vulnerability.Namespace.Name + } else if fifv.Feature.Namespace.Name != vulnerability.Namespace.Name { + msg := "could not insert an invalid vulnerability that contains FixedIn FeatureVersion that are not in the same namespace as the Vulnerability" log.Warning(msg) return cerrors.NewBadRequestError(msg) } @@ -129,12 +163,6 @@ func (pgSQL *pgSQL) insertVulnerability(vulnerability database.Vulnerability) er // We do `defer observeQueryTime` here because we don't want to observe invalid vulnerabilities. defer observeQueryTime("insertVulnerability", "all", tf) - // Find or insert Vulnerability's Namespace. - namespaceID, err := pgSQL.insertNamespace(vulnerability.Namespace) - if err != nil { - return err - } - // Begin transaction. tx, err := pgSQL.Begin() if err != nil { @@ -142,71 +170,84 @@ func (pgSQL *pgSQL) insertVulnerability(vulnerability database.Vulnerability) er return handleError("insertVulnerability.Begin()", err) } - // Find vulnerability and its Vulnerability_FixedIn_Features. - existingVulnerability, err := findVulnerability(tx, vulnerability.Namespace.Name, - vulnerability.Name, true) + // Find existing vulnerability and its Vulnerability_FixedIn_Features (for update). + existingVulnerability, err := findVulnerability(tx, vulnerability.Namespace.Name, vulnerability.Name, true) if err != nil && err != cerrors.ErrNotFound { tx.Rollback() return err } - // Insert or update vulnerability. - if existingVulnerability.ID == 0 { - // The vulnerability is a new one, insert it. - err = tx.QueryRow(getQuery("i_vulnerability"), namespaceID, vulnerability.Name, - vulnerability.Description, vulnerability.Link, &vulnerability.Severity, - &vulnerability.Metadata).Scan(&vulnerability.ID) - if err != nil { - tx.Rollback() - return handleError("i_vulnerability", err) - } - } else { - // The vulnerability exists, update it. - if vulnerability.Description != existingVulnerability.Description || - vulnerability.Link != existingVulnerability.Link || - vulnerability.Severity != existingVulnerability.Severity || - !reflect.DeepEqual(castMetadata(vulnerability.Metadata), existingVulnerability.Metadata) { - _, err = tx.Exec(getQuery("u_vulnerability"), existingVulnerability.ID, - vulnerability.Description, vulnerability.Link, &vulnerability.Severity, - &vulnerability.Metadata) - if err != nil { - tx.Rollback() - return handleError("u_vulnerability", err) - } + if onlyFixedIn { + // Because this call tries to update FixedIn FeatureVersion, import all other data from the + // existing one. + if existingVulnerability.ID == 0 { + return cerrors.ErrNotFound } - vulnerability.ID = existingVulnerability.ID + fixedIn := vulnerability.FixedIn + vulnerability = existingVulnerability + vulnerability.FixedIn = fixedIn } - // Get the new/updated/removed FeatureVersions and the resulting full list. - var newFIFV, updatedFIFV, removedFIFV []database.FeatureVersion - if existingVulnerability.ID == 0 { - // The vulnerability is a new new, the new FeatureVersions are the entire list of FixedIn. - newFIFV = vulnerability.FixedIn - } else { - // The vulnerability exists, compute the lists using diffFixedIn. - // We overwrite vulnerability.FixedIn with the entire list of FixedIn FeatureVersions, we'll - // then use the vulnerability in the notification, with that list instead of a potential diff. - newFIFV, updatedFIFV, removedFIFV, vulnerability.FixedIn = - diffFixedIn(existingVulnerability.FixedIn, vulnerability.FixedIn) + if existingVulnerability.ID != 0 { + updateMetadata := vulnerability.Description != existingVulnerability.Description || + vulnerability.Link != existingVulnerability.Link || + vulnerability.Severity != existingVulnerability.Severity || + !reflect.DeepEqual(castMetadata(vulnerability.Metadata), existingVulnerability.Metadata) + + // Construct the entire list of FixedIn FeatureVersion, by using the + // the FixedIn list of the old vulnerability. + // + // TODO(Quentin-M): We could use !updateFixedIn to just copy FixedIn/Affects rows from the + // existing vulnerability in order to make metadata updates much faster. + fixedIn, updateFixedIn := applyFixedInDiff(existingVulnerability.FixedIn, vulnerability.FixedIn) + vulnerability.FixedIn = fixedIn + + if !updateMetadata && !updateFixedIn { + tx.Commit() + return nil + } + + // Mark the old vulnerability as non latest. + _, err = tx.Exec(getQuery("r_vulnerability"), vulnerability.Namespace.Name, vulnerability.Name) + if err != nil { + tx.Rollback() + return handleError("r_vulnerability", err) + } + } + + // Find or insert Vulnerability's Namespace. + namespaceID, err := pgSQL.insertNamespace(vulnerability.Namespace) + if err != nil { + return err + } + + // Insert vulnerability. + err = tx.QueryRow( + getQuery("i_vulnerability"), + namespaceID, + vulnerability.Name, + vulnerability.Description, + vulnerability.Link, + &vulnerability.Severity, + &vulnerability.Metadata, + ).Scan(&vulnerability.ID) + + if err != nil { + tx.Rollback() + return handleError("i_vulnerability", err) } // Update Vulnerability_FixedIn_Feature and Vulnerability_Affects_FeatureVersion now. - if err = pgSQL.updateVulnerabilityFeatureVersions(tx, vulnerability.ID, newFIFV, updatedFIFV, - removedFIFV); err != nil { + err = pgSQL.insertVulnerabilityFixedInFeatureVersions(tx, vulnerability.ID, vulnerability.FixedIn) + if err != nil { tx.Rollback() return err } - // Create notification. - notification := database.VulnerabilityNotification{ - NewVulnerability: vulnerability, - } - if existingVulnerability.ID != 0 { - notification.OldVulnerability = &existingVulnerability - } - - if err := pgSQL.insertNotification(tx, notification); err != nil { + // Create a notification. + err = createNotification(tx, existingVulnerability.ID, vulnerability.ID) + if err != nil { return err } @@ -231,55 +272,48 @@ func castMetadata(m database.MetadataMap) database.MetadataMap { return c } -func diffFixedIn(existingFIFVList, newFIFVList []database.FeatureVersion) (newFIFV, updatedFIFV, removedFIFV, allFIFV []database.FeatureVersion) { - // Build FeatureVersion.Feature.Namespace.Name:FeatureVersion.Feature.Name (NaN) structures. - allFIFVMap, _ := createFeatureVersionNameMap(existingFIFVList) - vulnerabilityFixedInNameMap, vulnerabilityFixedInNameSlice := createFeatureVersionNameMap(newFIFVList) - existingFixedInMapNameMap, existingFixedInNameSlice := createFeatureVersionNameMap(existingFIFVList) +// applyFixedInDiff applies a FeatureVersion diff on a FeatureVersion list and returns the result. +func applyFixedInDiff(currentList, diff []database.FeatureVersion) ([]database.FeatureVersion, bool) { + currentMap, currentNames := createFeatureVersionNameMap(currentList) + diffMap, diffNames := createFeatureVersionNameMap(diff) - // Calculate the new FixedIn FeatureVersion NaN and updated ones. - newFixedInName := utils.CompareStringLists(vulnerabilityFixedInNameSlice, - existingFixedInNameSlice) - updatedFixedInName := utils.CompareStringListsInBoth(vulnerabilityFixedInNameSlice, - existingFixedInNameSlice) + addedNames := utils.CompareStringLists(diffNames, currentNames) + inBothNames := utils.CompareStringListsInBoth(diffNames, currentNames) + + different := false + + for _, name := range addedNames { + if diffMap[name].Version == types.MinVersion { + // MinVersion only makes sense when a Feature is already fixed in some version, + // in which case we would be in the "inBothNames". + continue + } + + currentMap[name] = diffMap[name] + different = true + } + + for _, name := range inBothNames { + fv := diffMap[name] - for _, nan := range newFixedInName { - fv := vulnerabilityFixedInNameMap[nan] if fv.Version == types.MinVersion { - // We don't want to mark a Feature as fixed in MinVersion. MinVersion only makes sense when a - // Feature is already marked as fixed in some version, in which case we would be in the - // "updatedFixedInFeatureVersions" loop and removes the fixed in mark. - continue - } - - newFIFV = append(newFIFV, fv) - allFIFVMap[fv.Feature.Namespace.Name+":"+fv.Feature.Name] = fv - } - - for _, nan := range updatedFixedInName { - fv := existingFixedInMapNameMap[nan] - fv.Version = vulnerabilityFixedInNameMap[nan].Version - - if existingFixedInMapNameMap[nan].Version == fv.Version { - // Versions are actually the same! - // Even though they appear in both lists, it's not an update. - continue - } - - if fv.Version != types.MinVersion { - updatedFIFV = append(updatedFIFV, fv) - allFIFVMap[fv.Feature.Namespace.Name+":"+fv.Feature.Name] = fv - } else { - removedFIFV = append(removedFIFV, fv) - delete(allFIFVMap, fv.Feature.Namespace.Name+":"+fv.Feature.Name) + // MinVersion means that the Feature doesn't affect the Vulnerability anymore. + delete(currentMap, name) + different = true + } else if fv.Version != currentMap[name].Version { + // The version got updated. + currentMap[name] = diffMap[name] + different = true } } - for _, fv := range allFIFVMap { - allFIFV = append(allFIFV, fv) + // Convert currentMap to a slice and return it. + var newList []database.FeatureVersion + for _, fv := range currentMap { + newList = append(newList, fv) } - return + return newList, different } func createFeatureVersionNameMap(features []database.FeatureVersion) (map[string]database.FeatureVersion, []string) { @@ -295,123 +329,19 @@ func createFeatureVersionNameMap(features []database.FeatureVersion) (map[string return m, s } -func (pgSQL *pgSQL) InsertVulnerabilityFixes(vulnerabilityNamespace, vulnerabilityName string, fixes []database.FeatureVersion) error { - // Verify parameters - for _, fifv := range fixes { - if fifv.Feature.Namespace.Name == "" { - fifv.Feature.Namespace.Name = vulnerabilityNamespace - } else if fifv.Feature.Namespace.Name != vulnerabilityNamespace { - msg := "could not add/update a FixedIn FeatureVersion: FixedIn FeatureVersion must be in the " + - "same namespace as the Vulnerability" - log.Warning(msg) - return cerrors.NewBadRequestError(msg) - } - } - - f := func(vulnerability database.Vulnerability) (newFIFV, updatedFIFV, removedFIFV, allFIFV []database.FeatureVersion, err error) { - newFIFV, updatedFIFV, _, allFIFV = diffFixedIn(vulnerability.FixedIn, fixes) - return - } - - return pgSQL.doVulnerabilityFixes(vulnerabilityNamespace, vulnerabilityName, f) -} - -func (pgSQL *pgSQL) DeleteVulnerabilityFix(vulnerabilityNamespace, vulnerabilityName, featureName string) error { - f := func(vulnerability database.Vulnerability) (newFIFV, updatedFIFV, removedFIFV, allFIFV []database.FeatureVersion, err error) { - // Search the specified featureName. - for i, vulnerabilityFV := range vulnerability.FixedIn { - if vulnerabilityFV.Feature.Name == featureName { - removedFIFV = append(removedFIFV, vulnerabilityFV) - allFIFV = append(vulnerability.FixedIn[:i], vulnerability.FixedIn[i+1:]...) - return - } - } - - err = cerrors.ErrNotFound - return - } - - return pgSQL.doVulnerabilityFixes(vulnerabilityNamespace, vulnerabilityName, f) -} - -// doVulnerabilityFixes is used by InsertVulnerabilityFixes and DeleteVulnerabilityFix. It -// adds/updates/removes FeatureVersions on the specified vulnerability using -// updateVulnerabilityFeatureVersions and creates a database.VulnerabilityNotification. -func (pgSQL *pgSQL) doVulnerabilityFixes(vulnerabilityNamespace, vulnerabilityName string, f func(vulnerability database.Vulnerability) (newFIFV, updatedFIFV, removedFIFV, allFIFV []database.FeatureVersion, err error)) error { - // Begin transaction. - tx, err := pgSQL.Begin() - if err != nil { - tx.Rollback() - return handleError("doVulnerabilityFixes.Begin()", err) - } - - // Select for update the vulnerability in order to prevent everyone else from executing updates - // on the vulnerability (and consequently on Vulnerability_FixedIn_Feature for that particular - // vulnerability) - vulnerability, err := findVulnerability(tx, vulnerabilityNamespace, vulnerabilityName, true) - if err != nil { - tx.Rollback() - return err - } - - // Get the new/updated/removed FeatureVersions and the resulting full list, using the given fct. - newFIFV, updatedFIFV, removedFIFV, allFIFV, err := f(vulnerability) - if err != nil { - tx.Rollback() - return err - } - if len(newFIFV) == 0 && len(updatedFIFV) == 0 && len(removedFIFV) == 0 { - // Nothing to do. - tx.Commit() - return nil - } - - // Update Vulnerability_FixedIn_Feature and Vulnerability_Affects_FeatureVersion now. - err = pgSQL.updateVulnerabilityFeatureVersions(tx, vulnerability.ID, newFIFV, updatedFIFV, - removedFIFV) - if err != nil { - tx.Rollback() - return err - } - - // Create notification. - newVulnerability := vulnerability - newVulnerability.FixedIn = allFIFV - - notification := database.VulnerabilityNotification{ - NewVulnerability: newVulnerability, - OldVulnerability: &vulnerability, - } - - if err := pgSQL.insertNotification(tx, notification); err != nil { - return err - } - - // Commit transaction. - err = tx.Commit() - if err != nil { - tx.Rollback() - return handleError("insertVulnerability.Commit()", err) - } - - return nil -} - -func (pgSQL *pgSQL) updateVulnerabilityFeatureVersions(tx *sql.Tx, vulnerabilityID int, newFIFV, updatedFIFV, removedFIFV []database.FeatureVersion) error { - defer observeQueryTime("updateVulnerabilityFeatureVersions", "all", time.Now()) +// insertVulnerabilityFixedInFeatureVersions populates Vulnerability_FixedIn_Feature for the given +// vulnerability with the specified database.FeatureVersion list and uses +// linkVulnerabilityToFeatureVersions to propagate the changes on Vulnerability_FixedIn_Feature to +// Vulnerability_Affects_FeatureVersion. +func (pgSQL *pgSQL) insertVulnerabilityFixedInFeatureVersions(tx *sql.Tx, vulnerabilityID int, fixedIn []database.FeatureVersion) error { + defer observeQueryTime("insertVulnerabilityFixedInFeatureVersions", "all", time.Now()) // Insert or find the Features. // TODO(Quentin-M): Batch me. var err error var features []*database.Feature - for _, fv := range newFIFV { - features = append(features, &fv.Feature) - } - for _, fv := range updatedFIFV { - features = append(features, &fv.Feature) - } - for _, fv := range removedFIFV { - features = append(features, &fv.Feature) + for i := 0; i < len(fixedIn); i++ { + features = append(features, &fixedIn[i].Feature) } for _, feature := range features { if feature.ID == 0 { @@ -434,56 +364,27 @@ func (pgSQL *pgSQL) updateVulnerabilityFeatureVersions(tx *sql.Tx, vulnerability return handleError("insertVulnerability.l_vulnerability_affects_featureversion", err) } - var fixedInID int + for _, fv := range fixedIn { + var fixedInID int - for _, fv := range newFIFV { // Insert Vulnerability_FixedIn_Feature. - err = tx.QueryRow(getQuery("i_vulnerability_fixedin_feature"), vulnerabilityID, fv.Feature.ID, - &fv.Version).Scan(&fixedInID) + err = tx.QueryRow( + getQuery("i_vulnerability_fixedin_feature"), + vulnerabilityID, fv.Feature.ID, + &fv.Version, + ).Scan(&fixedInID) + if err != nil { return handleError("i_vulnerability_fixedin_feature", err) } // Insert Vulnerability_Affects_FeatureVersion. - err = linkVulnerabilityToFeatureVersions(tx, fixedInID, vulnerabilityID, fv.Feature.ID, - fv.Version) + err = linkVulnerabilityToFeatureVersions(tx, fixedInID, vulnerabilityID, fv.Feature.ID, fv.Version) if err != nil { return err } } - for _, fv := range updatedFIFV { - // Update Vulnerability_FixedIn_Feature. - err = tx.QueryRow(getQuery("u_vulnerability_fixedin_feature"), vulnerabilityID, - fv.Feature.ID, &fv.Version).Scan(&fixedInID) - if err != nil { - return handleError("u_vulnerability_fixedin_feature", err) - } - - // Drop all old Vulnerability_Affects_FeatureVersion. - _, err = tx.Exec(getQuery("r_vulnerability_affects_featureversion"), fixedInID) - if err != nil { - return handleError("r_vulnerability_affects_featureversion", err) - } - - // Insert Vulnerability_Affects_FeatureVersion. - err = linkVulnerabilityToFeatureVersions(tx, fixedInID, vulnerabilityID, fv.Feature.ID, - fv.Version) - if err != nil { - return err - } - } - - for _, fv := range removedFIFV { - // Drop it from Vulnerability_FixedIn_Feature and let it cascade to - // Vulnerability_Affects_FeatureVersion. - err = tx.QueryRow(getQuery("r_vulnerability_fixedin_feature"), vulnerabilityID, - fv.Feature.ID).Scan(&fixedInID) - if err != nil && err != sql.ErrNoRows { - return handleError("r_vulnerability_fixedin_feature", err) - } - } - return nil } @@ -529,21 +430,72 @@ func linkVulnerabilityToFeatureVersions(tx *sql.Tx, fixedInID, vulnerabilityID, return nil } +func (pgSQL *pgSQL) InsertVulnerabilityFixes(vulnerabilityNamespace, vulnerabilityName string, fixes []database.FeatureVersion) error { + defer observeQueryTime("InsertVulnerabilityFixes", "all", time.Now()) + + v := database.Vulnerability{ + Name: vulnerabilityName, + Namespace: database.Namespace{ + Name: vulnerabilityNamespace, + }, + FixedIn: fixes, + } + + return pgSQL.insertVulnerability(v, true) +} + +func (pgSQL *pgSQL) DeleteVulnerabilityFix(vulnerabilityNamespace, vulnerabilityName, featureName string) error { + defer observeQueryTime("DeleteVulnerabilityFix", "all", time.Now()) + + v := database.Vulnerability{ + Name: vulnerabilityName, + Namespace: database.Namespace{ + Name: vulnerabilityNamespace, + }, + FixedIn: []database.FeatureVersion{ + database.FeatureVersion{ + Feature: database.Feature{ + Name: featureName, + Namespace: database.Namespace{ + Name: vulnerabilityNamespace, + }, + }, + Version: types.MinVersion, + }, + }, + } + + return pgSQL.insertVulnerability(v, true) +} + func (pgSQL *pgSQL) DeleteVulnerability(namespaceName, name string) error { defer observeQueryTime("DeleteVulnerability", "all", time.Now()) - result, err := pgSQL.Exec(getQuery("r_vulnerability"), namespaceName, name) + // Begin transaction. + tx, err := pgSQL.Begin() if err != nil { + tx.Rollback() + return handleError("DeleteVulnerability.Begin()", err) + } + + var vulnerabilityID int + err = tx.QueryRow(getQuery("r_vulnerability"), namespaceName, name).Scan(&vulnerabilityID) + if err != nil { + tx.Rollback() return handleError("r_vulnerability", err) } - affected, err := result.RowsAffected() + // Create a notification. + err = createNotification(tx, vulnerabilityID, 0) if err != nil { - return handleError("r_vulnerability.RowsAffected()", err) + return err } - if affected <= 0 { - return cerrors.ErrNotFound + // Commit transaction. + err = tx.Commit() + if err != nil { + tx.Rollback() + return handleError("DeleteVulnerability.Commit()", err) } return nil