Merge pull request #613 from jzelinskie/pkg-pagination

Introduce pkg/pagination

api/v3/rpc.go

@@ -25,6 +25,7 @@ import (
 	pb "github.com/coreos/clair/api/v3/clairpb"
 	"github.com/coreos/clair/database"
 	"github.com/coreos/clair/pkg/commonerr"
+	"github.com/coreos/clair/pkg/pagination"
 )
 
 // NotificationServer implements NotificationService interface for serving RPC.
@@ -214,8 +215,8 @@ func (s *NotificationServer) GetNotification(ctx context.Context, req *pb.GetNot
 	dbNotification, ok, err := tx.FindVulnerabilityNotification(
 		req.GetName(),
 		int(req.GetLimit()),
-		database.PageNumber(req.GetOldVulnerabilityPage()),
-		database.PageNumber(req.GetNewVulnerabilityPage()),
+		pagination.Token(req.GetOldVulnerabilityPage()),
+		pagination.Token(req.GetNewVulnerabilityPage()),
 	)
 
 	if err != nil {

cmd/clair/config.go

@@ -1,4 +1,4 @@
-// Copyright 2017 clair authors
+// Copyright 2018 clair authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -20,7 +20,6 @@ import (
 	"os"
 	"time"
 
-	"github.com/fernet/fernet-go"
 	log "github.com/sirupsen/logrus"
 	"gopkg.in/yaml.v2"
 
@@ -31,6 +30,7 @@ import (
 	"github.com/coreos/clair/ext/featurens"
 	"github.com/coreos/clair/ext/notification"
 	"github.com/coreos/clair/ext/vulnsrc"
+	"github.com/coreos/clair/pkg/pagination"
 )
 
 // ErrDatasourceNotLoaded is returned when the datasource variable in the
@@ -108,15 +108,10 @@ func LoadConfig(path string) (config *Config, err error) {
 	// Generate a pagination key if none is provided.
 	if v, ok := config.Database.Options["paginationkey"]; !ok || v == nil || v.(string) == "" {
 		log.Warn("pagination key is empty, generating...")
-		var key fernet.Key
-		if err = key.Generate(); err != nil {
-			return
-		}
-		config.Database.Options["paginationkey"] = key.Encode()
+		config.Database.Options["paginationkey"] = pagination.Must(pagination.NewKey()).String()
 	} else {
-		_, err = fernet.DecodeKey(config.Database.Options["paginationkey"].(string))
+		_, err = pagination.KeyFromString(config.Database.Options["paginationkey"].(string))
 		if err != nil {
-			err = errors.New("Invalid Pagination key; must be 32-bit URL-safe base64")
 			return
 		}
 	}

cmd/clair/main.go

@@ -1,4 +1,4 @@
-// Copyright 2017 clair authors
+// Copyright 2018 clair authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -61,7 +61,17 @@ import (
 	_ "github.com/coreos/clair/ext/vulnsrc/ubuntu"
 )
 
-const maxDBConnectionAttempts = 20
+// MaxDBConnectionAttempts is the total number of tries that Clair will use to
+// initially connect to a database at start-up.
+const MaxDBConnectionAttempts = 20
+
+// BinaryDependencies are the programs that Clair expects to be on the $PATH
+// because it creates subprocesses of these programs.
+var BinaryDependencies = []string{
+	"git",
+	"rpm",
+	"xz",
+}
 
 func waitForSignals(signals ...os.Signal) {
 	interrupts := make(chan os.Signal, 1)
@@ -136,7 +146,7 @@ func Boot(config *Config) {
 	// Open database
 	var db database.Datastore
 	var dbError error
-	for attempts := 1; attempts <= maxDBConnectionAttempts; attempts++ {
+	for attempts := 1; attempts <= MaxDBConnectionAttempts; attempts++ {
 		db, dbError = database.Open(config.Database)
 		if dbError == nil {
 			break
@@ -180,7 +190,7 @@ func main() {
 	flag.Parse()
 
 	// Check for dependencies.
-	for _, bin := range []string{"git", "rpm", "xz"} {
+	for _, bin := range BinaryDependencies {
 		_, err := exec.LookPath(bin)
 		if err != nil {
 			log.WithError(err).WithField("dependency", bin).Fatal("failed to find dependency")

database/database.go

@@ -20,6 +20,8 @@ import (
 	"errors"
 	"fmt"
 	"time"
+
+	"github.com/coreos/clair/pkg/pagination"
 )
 
 var (
@@ -168,16 +170,9 @@ type Session interface {
 	// affected ancestries affected by old or new vulnerability.
 	//
 	// Because the number of affected ancestries maybe large, they are paginated
-	// and their pages are specified by the given encrypted PageNumbers, which,
-	// if empty, are always considered first page.
-	//
-	// Session interface implementation should have encrypt and decrypt
-	// functions for PageNumber.
-	FindVulnerabilityNotification(name string, limit int,
-		oldVulnerabilityPage PageNumber,
-		newVulnerabilityPage PageNumber) (
-		noti VulnerabilityNotificationWithVulnerable,
-		found bool, err error)
+	// and their pages are specified by the paination token, which, if empty, are
+	// always considered first page.
+	FindVulnerabilityNotification(name string, limit int, oldVulnerabilityPage pagination.Token, newVulnerabilityPage pagination.Token) (noti VulnerabilityNotificationWithVulnerable, found bool, err error)
 
 	// MarkNotificationNotified marks a Notification as notified now, assuming
 	// the requested notification is in the database.

database/mock.go

@@ -14,7 +14,11 @@
 
 package database
 
-import "time"
+import (
+	"time"
+
+	"github.com/coreos/clair/pkg/pagination"
+)
 
 // MockSession implements Session and enables overriding each available method.
 // The default behavior of each method is to simply panic.
@@ -38,7 +42,7 @@ type MockSession struct {
 	FctDeleteVulnerabilities            func([]VulnerabilityID) error
 	FctInsertVulnerabilityNotifications func([]VulnerabilityNotification) error
 	FctFindNewNotification              func(lastNotified time.Time) (NotificationHook, bool, error)
-	FctFindVulnerabilityNotification    func(name string, limit int, oldPage PageNumber, newPage PageNumber) (
+	FctFindVulnerabilityNotification    func(name string, limit int, oldPage pagination.Token, newPage pagination.Token) (
 		vuln VulnerabilityNotificationWithVulnerable, ok bool, err error)
 	FctMarkNotificationNotified func(name string) error
 	FctDeleteNotification       func(name string) error
@@ -182,7 +186,7 @@ func (ms *MockSession) FindNewNotification(lastNotified time.Time) (Notification
 	panic("required mock function not implemented")
 }
 
-func (ms *MockSession) FindVulnerabilityNotification(name string, limit int, oldPage PageNumber, newPage PageNumber) (
+func (ms *MockSession) FindVulnerabilityNotification(name string, limit int, oldPage pagination.Token, newPage pagination.Token) (
 	VulnerabilityNotificationWithVulnerable, bool, error) {
 	if ms.FctFindVulnerabilityNotification != nil {
 		return ms.FctFindVulnerabilityNotification(name, limit, oldPage, newPage)

database/models.go

@@ -18,6 +18,8 @@ import (
 	"database/sql/driver"
 	"encoding/json"
 	"time"
+
+	"github.com/coreos/clair/pkg/pagination"
 )
 
 // Processors are extentions to scan a layer's content.
@@ -173,8 +175,8 @@ type PagedVulnerableAncestries struct {
 	Affected map[int]string
 
 	Limit   int
-	Current PageNumber
-	Next    PageNumber
+	Current pagination.Token
+	Next    pagination.Token
 
 	// End signals the end of the pages.
 	End bool
@@ -209,9 +211,6 @@ type VulnerabilityNotificationWithVulnerable struct {
 	New *PagedVulnerableAncestries
 }
 
-// PageNumber is used to do pagination.
-type PageNumber string
-
 // MetadataMap is for storing the metadata returned by vulnerability database.
 type MetadataMap map[string]interface{}
 

database/pgsql/notification.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/coreos/clair/database"
 	"github.com/coreos/clair/pkg/commonerr"
+	"github.com/coreos/clair/pkg/pagination"
 )
 
 var (
@@ -163,12 +164,12 @@ func (tx *pgSession) FindNewNotification(notifiedBefore time.Time) (database.Not
 	return notification, true, nil
 }
 
-func (tx *pgSession) findPagedVulnerableAncestries(vulnID int64, limit int, currentPage database.PageNumber) (database.PagedVulnerableAncestries, error) {
+func (tx *pgSession) findPagedVulnerableAncestries(vulnID int64, limit int, currentToken pagination.Token) (database.PagedVulnerableAncestries, error) {
 	vulnPage := database.PagedVulnerableAncestries{Limit: limit}
-	current := idPageNumber{0}
-	if currentPage != "" {
+	currentPage := Page{0}
+	if currentToken != pagination.FirstPageToken {
 		var err error
-		current, err = decryptPage(currentPage, tx.paginationKey)
+		err = tx.key.UnmarshalToken(currentToken, &currentPage)
 		if err != nil {
 			return vulnPage, err
 		}
@@ -188,7 +189,7 @@ func (tx *pgSession) findPagedVulnerableAncestries(vulnID int64, limit int, curr
 	}
 
 	// the last result is used for the next page's startID
-	rows, err := tx.Query(searchNotificationVulnerableAncestry, vulnID, current.StartID, limit+1)
+	rows, err := tx.Query(searchNotificationVulnerableAncestry, vulnID, currentPage.StartID, limit+1)
 	if err != nil {
 		return vulnPage, handleError("searchNotificationVulnerableAncestry", err)
 	}
@@ -209,13 +210,9 @@ func (tx *pgSession) findPagedVulnerableAncestries(vulnID int64, limit int, curr
 		lastIndex = len(ancestries)
 		vulnPage.End = true
 	} else {
-		// Use the last ancestry's ID as the next PageNumber.
+		// Use the last ancestry's ID as the next page.
 		lastIndex = len(ancestries) - 1
-		vulnPage.Next, err = encryptPage(
-			idPageNumber{
-				ancestries[len(ancestries)-1].id,
-			}, tx.paginationKey)
-
+		vulnPage.Next, err = tx.key.MarshalToken(Page{ancestries[len(ancestries)-1].id})
 		if err != nil {
 			return vulnPage, err
 		}
@@ -226,15 +223,15 @@ func (tx *pgSession) findPagedVulnerableAncestries(vulnID int64, limit int, curr
 		vulnPage.Affected[int(ancestry.id)] = ancestry.name
 	}
 
-	vulnPage.Current, err = encryptPage(current, tx.paginationKey)
+	vulnPage.Current, err = tx.key.MarshalToken(currentPage)
 	if err != nil {
 		return vulnPage, err
 	}
 
 	return vulnPage, nil
 }
 
-func (tx *pgSession) FindVulnerabilityNotification(name string, limit int, oldPage database.PageNumber, newPage database.PageNumber) (
+func (tx *pgSession) FindVulnerabilityNotification(name string, limit int, oldPageToken pagination.Token, newPageToken pagination.Token) (
 	database.VulnerabilityNotificationWithVulnerable, bool, error) {
 	var (
 		noti      database.VulnerabilityNotificationWithVulnerable
@@ -274,15 +271,15 @@ func (tx *pgSession) FindVulnerabilityNotification(name string, limit int, oldPa
 	}
 
 	if oldVulnID.Valid {
-		page, err := tx.findPagedVulnerableAncestries(oldVulnID.Int64, limit, oldPage)
+		page, err := tx.findPagedVulnerableAncestries(oldVulnID.Int64, limit, oldPageToken)
 		if err != nil {
 			return noti, false, err
 		}
 		noti.Old = &page
 	}
 
 	if newVulnID.Valid {
-		page, err := tx.findPagedVulnerableAncestries(newVulnID.Int64, limit, newPage)
+		page, err := tx.findPagedVulnerableAncestries(newVulnID.Int64, limit, newPageToken)
 		if err != nil {
 			return noti, false, err
 		}

database/pgsql/notification_test.go

@@ -1,4 +1,4 @@
-// Copyright 2017 clair authors
+// Copyright 2018 clair authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -73,22 +73,25 @@ func TestPagination(t *testing.T) {
 	if assert.Nil(t, err) && assert.True(t, ok) {
 		assert.Equal(t, "test", noti.Name)
 		if assert.NotNil(t, noti.Old) && assert.NotNil(t, noti.New) {
-			oldPageNum, err := decryptPage(noti.Old.Current, tx.paginationKey)
+			var oldPage Page
+			err := tx.key.UnmarshalToken(noti.Old.Current, &oldPage)
 			if !assert.Nil(t, err) {
 				assert.FailNow(t, "")
 			}
 
-			assert.Equal(t, int64(0), oldPageNum.StartID)
-			newPageNum, err := decryptPage(noti.New.Current, tx.paginationKey)
+			assert.Equal(t, int64(0), oldPage.StartID)
+			var newPage Page
+			err = tx.key.UnmarshalToken(noti.New.Current, &newPage)
 			if !assert.Nil(t, err) {
 				assert.FailNow(t, "")
 			}
-			newPageNextNum, err := decryptPage(noti.New.Next, tx.paginationKey)
+			var newPageNext Page
+			err = tx.key.UnmarshalToken(noti.New.Next, &newPageNext)
 			if !assert.Nil(t, err) {
 				assert.FailNow(t, "")
 			}
-			assert.Equal(t, int64(0), newPageNum.StartID)
-			assert.Equal(t, int64(4), newPageNextNum.StartID)
+			assert.Equal(t, int64(0), newPage.StartID)
+			assert.Equal(t, int64(4), newPageNext.StartID)
 
 			noti.Old.Current = ""
 			noti.New.Current = ""
@@ -98,26 +101,28 @@ func TestPagination(t *testing.T) {
 		}
 	}
 
-	page1, err := encryptPage(idPageNumber{0}, tx.paginationKey)
+	pageNum1, err := tx.key.MarshalToken(Page{0})
 	if !assert.Nil(t, err) {
 		assert.FailNow(t, "")
 	}
 
-	page2, err := encryptPage(idPageNumber{4}, tx.paginationKey)
+	pageNum2, err := tx.key.MarshalToken(Page{4})
 	if !assert.Nil(t, err) {
 		assert.FailNow(t, "")
 	}
 
-	noti, ok, err = tx.FindVulnerabilityNotification("test", 1, page1, page2)
+	noti, ok, err = tx.FindVulnerabilityNotification("test", 1, pageNum1, pageNum2)
 	if assert.Nil(t, err) && assert.True(t, ok) {
 		assert.Equal(t, "test", noti.Name)
 		if assert.NotNil(t, noti.Old) && assert.NotNil(t, noti.New) {
-			oldCurrentPage, err := decryptPage(noti.Old.Current, tx.paginationKey)
+			var oldCurrentPage Page
+			err = tx.key.UnmarshalToken(noti.Old.Current, &oldCurrentPage)
 			if !assert.Nil(t, err) {
 				assert.FailNow(t, "")
 			}
 
-			newCurrentPage, err := decryptPage(noti.New.Current, tx.paginationKey)
+			var newCurrentPage Page
+			err = tx.key.UnmarshalToken(noti.New.Current, &newCurrentPage)
 			if !assert.Nil(t, err) {
 				assert.FailNow(t, "")
 			}