Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add registrable version formats #298

Merged
merged 8 commits into from Jan 3, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 32 additions & 21 deletions api/v1/models.go
Expand Up @@ -21,16 +21,18 @@ import (
"fmt"
"time"

"github.com/coreos/clair/database"
"github.com/coreos/clair/utils/types"
"github.com/coreos/pkg/capnslog"
"github.com/fernet/fernet-go"

"github.com/coreos/clair/database"
"github.com/coreos/clair/ext/versionfmt"
"github.com/coreos/clair/utils/types"
)

var log = capnslog.NewPackageLogger("github.com/coreos/clair", "v1")

type Error struct {
Message string `json:"Layer`
Message string `json:"Layer"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tag name?

}

type Layer struct {
Expand Down Expand Up @@ -63,7 +65,8 @@ func LayerFromDatabaseModel(dbLayer database.Layer, withFeatures, withVulnerabil
feature := Feature{
Name: dbFeatureVersion.Feature.Name,
NamespaceName: dbFeatureVersion.Feature.Namespace.Name,
Version: dbFeatureVersion.Version.String(),
VersionFormat: dbFeatureVersion.Feature.Namespace.VersionFormat,
Version: dbFeatureVersion.Version,
AddedBy: dbFeatureVersion.AddedBy.Name,
}

Expand All @@ -77,8 +80,8 @@ func LayerFromDatabaseModel(dbLayer database.Layer, withFeatures, withVulnerabil
Metadata: dbVuln.Metadata,
}

if dbVuln.FixedBy != types.MaxVersion {
vuln.FixedBy = dbVuln.FixedBy.String()
if dbVuln.FixedBy != versionfmt.MaxVersion {
vuln.FixedBy = dbVuln.FixedBy
}
feature.Vulnerabilities = append(feature.Vulnerabilities, vuln)
}
Expand All @@ -90,7 +93,8 @@ func LayerFromDatabaseModel(dbLayer database.Layer, withFeatures, withVulnerabil
}

type Namespace struct {
Name string `json:"Name,omitempty"`
Name string `json:"Name,omitempty"`
VersionFormat string `json:"VersionFormat,omitempty"`
}

type Vulnerability struct {
Expand Down Expand Up @@ -153,44 +157,51 @@ func VulnerabilityFromDatabaseModel(dbVuln database.Vulnerability, withFixedIn b
type Feature struct {
Name string `json:"Name,omitempty"`
NamespaceName string `json:"NamespaceName,omitempty"`
VersionFormat string `json:"VersionFormat,omitempty"`
Version string `json:"Version,omitempty"`
Vulnerabilities []Vulnerability `json:"Vulnerabilities,omitempty"`
AddedBy string `json:"AddedBy,omitempty"`
}

func FeatureFromDatabaseModel(dbFeatureVersion database.FeatureVersion) Feature {
versionStr := dbFeatureVersion.Version.String()
if versionStr == types.MaxVersion.String() {
versionStr = "None"
version := dbFeatureVersion.Version
if version == versionfmt.MaxVersion {
version = "None"
}

return Feature{
Name: dbFeatureVersion.Feature.Name,
NamespaceName: dbFeatureVersion.Feature.Namespace.Name,
Version: versionStr,
VersionFormat: dbFeatureVersion.Feature.Namespace.VersionFormat,
Version: version,
AddedBy: dbFeatureVersion.AddedBy.Name,
}
}

func (f Feature) DatabaseModel() (database.FeatureVersion, error) {
var version types.Version
func (f Feature) DatabaseModel() (fv database.FeatureVersion, err error) {
var version string
if f.Version == "None" {
version = types.MaxVersion
version = versionfmt.MaxVersion
} else {
var err error
version, err = types.NewVersion(f.Version)
err = versionfmt.Valid(f.VersionFormat, f.Version)
if err != nil {
return database.FeatureVersion{}, err
return
}
version = f.Version
}

return database.FeatureVersion{
fv = database.FeatureVersion{
Feature: database.Feature{
Name: f.Name,
Namespace: database.Namespace{Name: f.NamespaceName},
Name: f.Name,
Namespace: database.Namespace{
Name: f.NamespaceName,
VersionFormat: f.VersionFormat,
},
},
Version: version,
}, nil
}

return
}

type Notification struct {
Expand Down
5 changes: 4 additions & 1 deletion api/v1/routes.go
Expand Up @@ -179,7 +179,10 @@ func getNamespaces(w http.ResponseWriter, r *http.Request, p httprouter.Params,
}
var namespaces []Namespace
for _, dbNamespace := range dbNamespaces {
namespaces = append(namespaces, Namespace{Name: dbNamespace.Name})
namespaces = append(namespaces, Namespace{
Name: dbNamespace.Name,
VersionFormat: dbNamespace.VersionFormat,
})
}

writeResponse(w, r, http.StatusOK, NamespaceEnvelope{Namespaces: &namespaces})
Expand Down
7 changes: 4 additions & 3 deletions database/models.go
Expand Up @@ -40,7 +40,8 @@ type Layer struct {
type Namespace struct {
Model

Name string
Name string
VersionFormat string
}

type Feature struct {
Expand All @@ -54,7 +55,7 @@ type FeatureVersion struct {
Model

Feature Feature
Version types.Version
Version string
AffectedBy []Vulnerability

// For output purposes. Only make sense when the feature version is in the context of an image.
Expand All @@ -78,7 +79,7 @@ type Vulnerability struct {

// For output purposes. Only make sense when the vulnerability
// is already about a specific Feature/FeatureVersion.
FixedBy types.Version `json:",omitempty"`
FixedBy string `json:",omitempty"`
}

type MetadataMap map[string]interface{}
Expand Down
18 changes: 12 additions & 6 deletions database/pgsql/complex_test.go
@@ -1,4 +1,4 @@
// Copyright 2015 clair authors
// Copyright 2016 clair authors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should revert these and make a separated PR for 2017.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than spamming the codebase, I'd rather just update them whenever the files are touched.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sgtm

//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -29,6 +29,9 @@ import (
"github.com/coreos/clair/database"
"github.com/coreos/clair/utils"
"github.com/coreos/clair/utils/types"

// dpkg versioning is used to parse test packages.
_ "github.com/coreos/clair/ext/versionfmt/dpkg"
)

const (
Expand All @@ -46,8 +49,11 @@ func TestRaceAffects(t *testing.T) {

// Insert the Feature on which we'll work.
feature := database.Feature{
Namespace: database.Namespace{Name: "TestRaceAffectsFeatureNamespace1"},
Name: "TestRaceAffecturesFeature1",
Namespace: database.Namespace{
Name: "TestRaceAffectsFeatureNamespace1",
VersionFormat: "dpkg",
},
Name: "TestRaceAffecturesFeature1",
}
_, err = datastore.insertFeature(feature)
if err != nil {
Expand All @@ -66,7 +72,7 @@ func TestRaceAffects(t *testing.T) {

featureVersions[i] = database.FeatureVersion{
Feature: feature,
Version: types.NewVersionUnsafe(strconv.Itoa(version)),
Version: strconv.Itoa(version),
}
}

Expand All @@ -86,7 +92,7 @@ func TestRaceAffects(t *testing.T) {
FixedIn: []database.FeatureVersion{
{
Feature: feature,
Version: types.NewVersionUnsafe(strconv.Itoa(version)),
Version: strconv.Itoa(version),
},
},
Severity: types.Unknown,
Expand Down Expand Up @@ -126,7 +132,7 @@ func TestRaceAffects(t *testing.T) {
var expectedAffectedNames []string

for _, featureVersion := range featureVersions {
featureVersionVersion, _ := strconv.Atoi(featureVersion.Version.String())
featureVersionVersion, _ := strconv.Atoi(featureVersion.Version)

// Get actual affects.
rows, err := datastore.Query(searchComplexTestFeatureVersionAffects,
Expand Down
46 changes: 26 additions & 20 deletions database/pgsql/feature.go
@@ -1,4 +1,4 @@
// Copyright 2015 clair authors
// Copyright 2016 clair authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand All @@ -16,11 +16,13 @@ package pgsql

import (
"database/sql"
"fmt"
"strings"
"time"

"github.com/coreos/clair/database"
"github.com/coreos/clair/ext/versionfmt"
cerrors "github.com/coreos/clair/utils/errors"
"github.com/coreos/clair/utils/types"
)

func (pgSQL *pgSQL) insertFeature(feature database.Feature) (int, error) {
Expand Down Expand Up @@ -61,13 +63,15 @@ func (pgSQL *pgSQL) insertFeature(feature database.Feature) (int, error) {
return id, nil
}

func (pgSQL *pgSQL) insertFeatureVersion(featureVersion database.FeatureVersion) (id int, err error) {
if featureVersion.Version.String() == "" {
func (pgSQL *pgSQL) insertFeatureVersion(fv database.FeatureVersion) (id int, err error) {
err = versionfmt.Valid(fv.Feature.Namespace.VersionFormat, fv.Version)
if err != nil {
fmt.Println(err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover.

return 0, cerrors.NewBadRequestError("could not find/insert invalid FeatureVersion")
}

// Do cache lookup.
cacheIndex := "featureversion:" + featureVersion.Feature.Namespace.Name + ":" + featureVersion.Feature.Name + ":" + featureVersion.Version.String()
cacheIndex := strings.Join([]string{"featureversion", fv.Feature.Namespace.Name, fv.Feature.Name, fv.Version}, ":")
if pgSQL.cache != nil {
promCacheQueriesTotal.WithLabelValues("featureversion").Inc()
id, found := pgSQL.cache.Get(cacheIndex)
Expand All @@ -82,30 +86,29 @@ func (pgSQL *pgSQL) insertFeatureVersion(featureVersion database.FeatureVersion)

// Find or create Feature first.
t := time.Now()
featureID, err := pgSQL.insertFeature(featureVersion.Feature)
featureID, err := pgSQL.insertFeature(fv.Feature)
observeQueryTime("insertFeatureVersion", "insertFeature", t)

if err != nil {
return 0, err
}

featureVersion.Feature.ID = featureID
fv.Feature.ID = featureID

// Try to find the FeatureVersion.
//
// In a populated database, the likelihood of the FeatureVersion already being there is high.
// If we can find it here, we then avoid using a transaction and locking the database.
err = pgSQL.QueryRow(searchFeatureVersion, featureID, &featureVersion.Version).
Scan(&featureVersion.ID)
err = pgSQL.QueryRow(searchFeatureVersion, featureID, fv.Version).Scan(&fv.ID)
if err != nil && err != sql.ErrNoRows {
return 0, handleError("searchFeatureVersion", err)
}
if err == nil {
if pgSQL.cache != nil {
pgSQL.cache.Add(cacheIndex, featureVersion.ID)
pgSQL.cache.Add(cacheIndex, fv.ID)
}

return featureVersion.ID, nil
return fv.ID, nil
}

// Begin transaction.
Expand All @@ -132,8 +135,7 @@ func (pgSQL *pgSQL) insertFeatureVersion(featureVersion database.FeatureVersion)
var created bool

t = time.Now()
err = tx.QueryRow(soiFeatureVersion, featureID, &featureVersion.Version).
Scan(&created, &featureVersion.ID)
err = tx.QueryRow(soiFeatureVersion, featureID, fv.Version).Scan(&created, &fv.ID)
observeQueryTime("insertFeatureVersion", "soiFeatureVersion", t)

if err != nil {
Expand All @@ -147,16 +149,16 @@ func (pgSQL *pgSQL) insertFeatureVersion(featureVersion database.FeatureVersion)
tx.Commit()

if pgSQL.cache != nil {
pgSQL.cache.Add(cacheIndex, featureVersion.ID)
pgSQL.cache.Add(cacheIndex, fv.ID)
}

return featureVersion.ID, nil
return fv.ID, nil
}

// Link the new FeatureVersion with every vulnerabilities that affect it, by inserting in
// Vulnerability_Affects_FeatureVersion.
t = time.Now()
err = linkFeatureVersionToVulnerabilities(tx, featureVersion)
err = linkFeatureVersionToVulnerabilities(tx, fv)
observeQueryTime("insertFeatureVersion", "linkFeatureVersionToVulnerabilities", t)

if err != nil {
Expand All @@ -171,10 +173,10 @@ func (pgSQL *pgSQL) insertFeatureVersion(featureVersion database.FeatureVersion)
}

if pgSQL.cache != nil {
pgSQL.cache.Add(cacheIndex, featureVersion.ID)
pgSQL.cache.Add(cacheIndex, fv.ID)
}

return featureVersion.ID, nil
return fv.ID, nil
}

// TODO(Quentin-M): Batch me
Expand All @@ -195,7 +197,7 @@ func (pgSQL *pgSQL) insertFeatureVersions(featureVersions []database.FeatureVers
type vulnerabilityAffectsFeatureVersion struct {
vulnerabilityID int
fixedInID int
fixedInVersion types.Version
fixedInVersion string
}

func linkFeatureVersionToVulnerabilities(tx *sql.Tx, featureVersion database.FeatureVersion) error {
Expand All @@ -216,7 +218,11 @@ func linkFeatureVersionToVulnerabilities(tx *sql.Tx, featureVersion database.Fea
return handleError("searchVulnerabilityFixedInFeature.Scan()", err)
}

if featureVersion.Version.Compare(affect.fixedInVersion) < 0 {
cmp, err := versionfmt.Compare(featureVersion.Feature.Namespace.VersionFormat, featureVersion.Version, affect.fixedInVersion)
if err != nil {
return handleError("searchVulnerabilityVersionComparison", err)
Copy link
Contributor

@Quentin-M Quentin-M Jan 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleError is specifically for SQL errors, it does a bunch of stuff including adding entries to Prometheus. So it'll show as a pgsql error while it's really..

}
if cmp < 0 {
// The version of the FeatureVersion we are inserting is lower than the fixed version on this
// Vulnerability, thus, this FeatureVersion is affected by it.
affects = append(affects, affect)
Expand Down