more code and tests cleanups

asserts/account_key_test.go

@@ -41,7 +41,7 @@ var _ = Suite(&accountKeySuite{})
 
 func (aks *accountKeySuite) SetUpSuite(c *C) {
 	cfg1 := &asserts.DatabaseConfig{
-		KeypairManager: asserts.NewMemoryKeypairMananager(),
+		KeypairManager: asserts.NewMemoryKeypairManager(),
 	}
 	accDb, err := asserts.OpenDatabase(cfg1)
 	c.Assert(err, IsNil)

asserts/database.go

@@ -61,14 +61,11 @@ type KeypairManager interface {
 
 // DatabaseConfig for an assertion database.
 type DatabaseConfig struct {
-	// database filesystem backstores path
-	Path string
 	// trusted account keys
 	TrustedKeys []*AccountKey
-	// backstore for assertions, falls back to a filesystem based backstrore
-	// if not set
+	// backstore for assertions
 	Backstore Backstore
-	// manager/backstore for keypairs, falls back to a filesystem based manager
+	// manager/backstore for keypairs
 	KeypairManager KeypairManager
 }
 
@@ -96,39 +93,10 @@ func OpenDatabase(cfg *DatabaseConfig) (*Database, error) {
 	bs := cfg.Backstore
 	keypairMgr := cfg.KeypairManager
 
-	// falling back to at least one of the filesytem backstores,
-	// ensure the main directory cfg.Path
-	// TODO: decide what should be the final defaults/fallbacks
-	if bs == nil || keypairMgr == nil {
-		//var err error
-		/*
-			err := os.MkdirAll(cfg.Path, 0775)
-			if err != nil {
-				return nil, fmt.Errorf("failed to create assert database root: %v", err)
-			}
-		*/
-		/*info, err := os.Stat(cfg.Path)
-		if err != nil {
-			return nil, fmt.Errorf("failed to create assert database root: %v", err)
-		}
-		if info.Mode().Perm()&0002 != 0 {
-			return nil, fmt.Errorf("assert database root unexpectedly world-writable: %v", cfg.Path)
-		}*/
-
-		/*
-			if bs == nil {
-				bs, err = OpenFilesystemBackstore(cfg.Path)
-				if err != nil {
-					return nil, err
-				}
-			}
-			if keypairMgr == nil {
-				keypairMgr, err = OpenFilesystemKeypairManager(cfg.Path)
-				if err != nil {
-					return nil, err
-				}
-			}
-		*/
+	if bs == nil && keypairMgr == nil {
+		// TODO: actually have Null* variants of at least the Backstore,
+		// so we can check that they are both set instead and it's safer
+		return nil, fmt.Errorf("database cannot be used with backstore and keypair manager both unset")
 	}
 
 	trustedKeys := make(map[string][]*AccountKey)

asserts/database_test.go

@@ -44,45 +44,24 @@ type openSuite struct{}
 
 var _ = Suite(&openSuite{})
 
-/* xxx
 func (opens *openSuite) TestOpenDatabaseOK(c *C) {
-	// ensure umask has clean when creating the DB dir
-	oldUmask := syscall.Umask(0)
-	defer func() { syscall.Umask(oldUmask) }()
-
-	topDir := filepath.Join(c.MkDir(), "asserts-db")
-	cfg := &asserts.DatabaseConfig{Path: topDir}
+	cfg := &asserts.DatabaseConfig{
+		KeypairManager: asserts.NewMemoryKeypairManager(),
+	}
 	db, err := asserts.OpenDatabase(cfg)
 	c.Assert(err, IsNil)
 	c.Assert(db, NotNil)
-	info, err := os.Stat(topDir)
-	c.Assert(err, IsNil)
-	c.Assert(info.IsDir(), Equals, true)
-	c.Check(info.Mode().Perm(), Equals, os.FileMode(0775))
-}
-
-func (opens *openSuite) TestOpenDatabaseRootCreateFail(c *C) {
-	parent := filepath.Join(c.MkDir(), "var")
-	// make it not writable
-	os.MkdirAll(parent, 555)
-	topDir := filepath.Join(parent, "asserts-db")
-	cfg := &asserts.DatabaseConfig{Path: topDir}
-	db, err := asserts.OpenDatabase(cfg)
-	c.Assert(err, ErrorMatches, "failed to create assert database root: .*")
-	c.Check(db, IsNil)
 }
-*/
 
 type databaseSuite struct {
 	topDir string
-	db      *asserts.Database
+	db     *asserts.Database
 }
 
 var _ = Suite(&databaseSuite{})
 
 func (dbs *databaseSuite) SetUpTest(c *C) {
 	dbs.topDir = filepath.Join(c.MkDir(), "asserts-db")
-	os.Mkdir(dbs.topDir, 0775) // xxx
 	fsKeypairMgr, err := asserts.OpenFilesystemKeypairManager(dbs.topDir)
 	c.Assert(err, IsNil)
 	cfg := &asserts.DatabaseConfig{
@@ -179,7 +158,6 @@ func (chks *checkSuite) SetUpTest(c *C) {
 	var err error
 
 	topDir := filepath.Join(c.MkDir(), "asserts-db")
-	os.Mkdir(topDir, 0775) // xxx
 	chks.bs, err = asserts.OpenFilesystemBackstore(topDir)
 	c.Assert(err, IsNil)
 
@@ -247,7 +225,7 @@ var _ = Suite(&signAddFindSuite{})
 
 func (safs *signAddFindSuite) SetUpTest(c *C) {
 	cfg0 := &asserts.DatabaseConfig{
-		KeypairManager: asserts.NewMemoryKeypairMananager(),
+		KeypairManager: asserts.NewMemoryKeypairManager(),
 	}
 	db0, err := asserts.OpenDatabase(cfg0)
 	c.Assert(err, IsNil)
@@ -259,7 +237,6 @@ func (safs *signAddFindSuite) SetUpTest(c *C) {
 	safs.signingKeyID = pk.PublicKey().ID()
 
 	topDir := filepath.Join(c.MkDir(), "asserts-db")
-	os.Mkdir(topDir, 0775) // xxx
 	bs, err := asserts.OpenFilesystemBackstore(topDir)
 	c.Assert(err, IsNil)
 

asserts/fsbackstore.go

@@ -38,12 +38,13 @@ type filesystemBackstore struct {
 }
 
 // OpenFilesystemBackstore opens a filesystem backed assertions backstore under path.
-// path must already exist.
 func OpenFilesystemBackstore(path string) (Backstore, error) {
-	if err := checkRoot(path); err != nil {
+	top := filepath.Join(path, assertionsRoot)
+	err := ensureTop(top)
+	if err != nil {
 		return nil, err
 	}
-	return &filesystemBackstore{top: filepath.Join(path, assertionsRoot)}, nil
+	return &filesystemBackstore{top: top}, nil
 }
 
 // guarantees that result assertion is of the expected type (both in the AssertionType and go type sense)

asserts/fsbackstore_test.go

@@ -34,30 +34,42 @@ type fsBackstoreSuite struct{}
 var _ = Suite(&fsBackstoreSuite{})
 
 func (fsbss *fsBackstoreSuite) TestOpenOK(c *C) {
-	rootDir := filepath.Join(c.MkDir(), "asserts-db")
-	err := os.MkdirAll(rootDir, 0775)
-	c.Assert(err, IsNil)
+	// ensure umask is clean when creating the DB dir
+	oldUmask := syscall.Umask(0)
+	defer syscall.Umask(oldUmask)
 
-	bs, err := asserts.OpenFilesystemBackstore(rootDir)
+	topDir := filepath.Join(c.MkDir(), "asserts-db")
+
+	bs, err := asserts.OpenFilesystemBackstore(topDir)
 	c.Check(err, IsNil)
 	c.Check(bs, NotNil)
+
+	info, err := os.Stat(filepath.Join(topDir, "asserts-v0"))
+	c.Assert(err, IsNil)
+	c.Assert(info.IsDir(), Equals, true)
+	c.Check(info.Mode().Perm(), Equals, os.FileMode(0775))
 }
 
-func (fsbss *fsBackstoreSuite) TestOpenRootNotThere(c *C) {
+func (fsbss *fsBackstoreSuite) TestOpenCreateFail(c *C) {
 	parent := filepath.Join(c.MkDir(), "var")
-	rootDir := filepath.Join(parent, "asserts-db")
-	bs, err := asserts.OpenFilesystemBackstore(rootDir)
-	// xxx special case not there as error
-	c.Assert(err, ErrorMatches, "failed to check assert storage root: .*")
+	topDir := filepath.Join(parent, "asserts-db")
+	// make it not writable
+	err := os.Mkdir(parent, 0555)
+	c.Assert(err, IsNil)
+
+	bs, err := asserts.OpenFilesystemBackstore(topDir)
+	c.Assert(err, ErrorMatches, "failed to create assert storage root: .*")
 	c.Check(bs, IsNil)
 }
 
 func (fsbss *fsBackstoreSuite) TestOpenWorldWritableFail(c *C) {
-	rootDir := filepath.Join(c.MkDir(), "asserts-db")
+	topDir := filepath.Join(c.MkDir(), "asserts-db")
+	// make it world-writable
 	oldUmask := syscall.Umask(0)
-	os.MkdirAll(rootDir, 0777)
+	os.MkdirAll(filepath.Join(topDir, "asserts-v0"), 0777)
 	syscall.Umask(oldUmask)
-	bs, err := asserts.OpenFilesystemBackstore(rootDir)
+
+	bs, err := asserts.OpenFilesystemBackstore(topDir)
 	c.Assert(err, ErrorMatches, "assert storage root unexpectedly world-writable: .*")
 	c.Check(bs, IsNil)
 }

asserts/fsentryutils.go

@@ -30,10 +30,14 @@ import (
 
 // utilities to read/write fs entries
 
-func checkRoot(path string) error {
+func ensureTop(path string) error {
+	err := os.MkdirAll(path, 0775)
+	if err != nil {
+		return fmt.Errorf("failed to create assert storage root: %v", err)
+	}
 	info, err := os.Stat(path)
 	if err != nil {
-		return fmt.Errorf("failed to check assert storage root: %v", err)
+		return fmt.Errorf("failed to create assert storage root: %v", err)
 	}
 	if info.Mode().Perm()&0002 != 0 {
 		return fmt.Errorf("assert storage root unexpectedly world-writable: %v", path)

asserts/fskeypairmgr.go

@@ -39,12 +39,13 @@ type filesystemKeypairManager struct {
 }
 
 // OpenFilesystemKeypairManager opens a filesystem backed assertions backstore under path.
-// path must already exist.
 func OpenFilesystemKeypairManager(path string) (KeypairManager, error) {
-	if err := checkRoot(path); err != nil {
+	top := filepath.Join(path, privateKeysRoot)
+	err := ensureTop(top)
+	if err != nil {
 		return nil, err
 	}
-	return &filesystemKeypairManager{top: filepath.Join(path, privateKeysRoot)}, nil
+	return &filesystemKeypairManager{top: top}, nil
 }
 
 var errKeypairAlreadyExists = errors.New("key pair with given key id already exists")

asserts/fskeypairmgr_test.go

@@ -34,30 +34,32 @@ type fsKeypairMgrSuite struct{}
 var _ = Suite(&fsKeypairMgrSuite{})
 
 func (fsbss *fsKeypairMgrSuite) TestOpenOK(c *C) {
-	rootDir := filepath.Join(c.MkDir(), "asserts-db")
-	err := os.MkdirAll(rootDir, 0775)
+	// ensure umask is clean when creating the DB dir
+	oldUmask := syscall.Umask(0)
+	defer syscall.Umask(oldUmask)
+
+	topDir := filepath.Join(c.MkDir(), "asserts-db")
+	err := os.MkdirAll(topDir, 0775)
 	c.Assert(err, IsNil)
 
-	bs, err := asserts.OpenFilesystemKeypairManager(rootDir)
+	bs, err := asserts.OpenFilesystemKeypairManager(topDir)
 	c.Check(err, IsNil)
 	c.Check(bs, NotNil)
-}
 
-func (fsbss *fsKeypairMgrSuite) TestOpenRootNotThere(c *C) {
-	parent := filepath.Join(c.MkDir(), "var")
-	rootDir := filepath.Join(parent, "asserts-db")
-	bs, err := asserts.OpenFilesystemKeypairManager(rootDir)
-	// xxx special case not there as error
-	c.Assert(err, ErrorMatches, "failed to check assert storage root: .*")
-	c.Check(bs, IsNil)
+	info, err := os.Stat(filepath.Join(topDir, "private-keys-v0"))
+	c.Assert(err, IsNil)
+	c.Assert(info.IsDir(), Equals, true)
+	c.Check(info.Mode().Perm(), Equals, os.FileMode(0775))
 }
 
 func (fsbss *fsKeypairMgrSuite) TestOpenWorldWritableFail(c *C) {
-	rootDir := filepath.Join(c.MkDir(), "asserts-db")
+	topDir := filepath.Join(c.MkDir(), "asserts-db")
+	// make it world-writable
 	oldUmask := syscall.Umask(0)
-	os.MkdirAll(rootDir, 0777)
+	os.MkdirAll(filepath.Join(topDir, "private-keys-v0"), 0777)
 	syscall.Umask(oldUmask)
-	bs, err := asserts.OpenFilesystemKeypairManager(rootDir)
+
+	bs, err := asserts.OpenFilesystemKeypairManager(topDir)
 	c.Assert(err, ErrorMatches, "assert storage root unexpectedly world-writable: .*")
 	c.Check(bs, IsNil)
 }

asserts/memkeypairmgr.go

@@ -23,8 +23,8 @@ type memoryKeypairManager struct {
 	pairs map[string]map[string]PrivateKey
 }
 
-// NewMemoryKeypairMananager creates a new key pair manager with a memory backstore.
-func NewMemoryKeypairMananager() KeypairManager {
+// NewMemoryKeypairManager creates a new key pair manager with a memory backstore.
+func NewMemoryKeypairManager() KeypairManager {
 	return memoryKeypairManager{
 		pairs: make(map[string]map[string]PrivateKey),
 	}

asserts/memkeypairmgr_test.go

@@ -32,7 +32,7 @@ type memKeypairMgtSuite struct {
 var _ = Suite(&memKeypairMgtSuite{})
 
 func (mkms *memKeypairMgtSuite) SetUpTest(c *C) {
-	mkms.keypairMgr = asserts.NewMemoryKeypairMananager()
+	mkms.keypairMgr = asserts.NewMemoryKeypairManager()
 }
 
 func (mkms *memKeypairMgtSuite) TestPutAndGet(c *C) {

asserts/snap_asserts_test.go

@@ -104,7 +104,7 @@ func makeSignAndCheckDbWithAccountKey(c *C, accountID string) (signingKeyID stri
 	trustedKey := testPrivKey0
 
 	cfg1 := &asserts.DatabaseConfig{
-		KeypairManager: asserts.NewMemoryKeypairMananager(),
+		KeypairManager: asserts.NewMemoryKeypairManager(),
 	}
 	accSignDB, err := asserts.OpenDatabase(cfg1)
 	c.Assert(err, IsNil)

asserts/sysdb.go

@@ -22,16 +22,11 @@ package asserts
 import (
 	"fmt"
 	"io/ioutil"
-	"os"
 
 	"github.com/ubuntu-core/snappy/dirs"
 )
 
 func openDatabaseAt(path string, cfg *DatabaseConfig) (*Database, error) {
-	err := os.MkdirAll(path, 0775)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create assert database root: %v", err)
-	}
 	bs, err := OpenFilesystemBackstore(path)
 	if err != nil {
 		return nil, err