Add dependency injection of remote store for NotaryRepository initial…

client/changelist/changelist.go

@@ -21,6 +21,11 @@ func (cl *memChangelist) Add(c Change) error {
 	return nil
 }
 
+// Location returns the string "memory"
+func (cl memChangelist) Location() string {
+	return "memory"
+}
+
 // Remove deletes the changes found at the given indices
 func (cl *memChangelist) Remove(idxs []int) error {
 	remove := make(map[int]struct{})

client/changelist/file_changelist.go

@@ -137,6 +137,11 @@ func (cl FileChangelist) Close() error {
 	return nil
 }
 
+// Location returns the file path to the changelist
+func (cl FileChangelist) Location() string {
+	return cl.dir
+}
+
 // NewIterator creates an iterator from FileChangelist
 func (cl FileChangelist) NewIterator() (ChangeIterator, error) {
 	fileInfos, err := getFileNames(cl.dir)

client/changelist/interface.go

@@ -27,6 +27,9 @@ type Changelist interface {
 	// NewIterator returns an iterator for walking through the list
 	// of changes currently stored
 	NewIterator() (ChangeIterator, error)
+
+	// Location returns the place the changelist is stores
+	Location() string
 }
 
 const (

client/client.go

@@ -17,7 +17,6 @@ import (
 	"github.com/docker/notary/client/changelist"
 	"github.com/docker/notary/cryptoservice"
 	store "github.com/docker/notary/storage"
-	"github.com/docker/notary/trustmanager"
 	"github.com/docker/notary/trustpinning"
 	"github.com/docker/notary/tuf"
 	"github.com/docker/notary/tuf/data"
@@ -42,8 +41,9 @@ type NotaryRepository struct {
 	baseDir        string
 	gun            data.GUN
 	baseURL        string
-	tufRepoPath    string
+	changelist     changelist.Changelist
 	cache          store.MetadataStore
+	remoteStore    store.RemoteStore
 	CryptoService  signed.CryptoService
 	tufRepo        *tuf.Repo
 	invalid        *tuf.Repo // known data that was parsable but deemed invalid
@@ -53,7 +53,9 @@ type NotaryRepository struct {
 }
 
 // NewFileCachedNotaryRepository is a wrapper for NewNotaryRepository that initializes
-// a file cache from the provided repository and local config information
+// a file cache from the provided repository, local config information and a crypto service.
+// It also retrieves the remote store associated to the base directory under where all the
+// trust files will be stored and the specified GUN.
 func NewFileCachedNotaryRepository(baseDir string, gun data.GUN, baseURL string, rt http.RoundTripper,
 	retriever notary.PassRetriever, trustPinning trustpinning.TrustPinConfig) (
 	*NotaryRepository, error) {
@@ -65,47 +67,56 @@ func NewFileCachedNotaryRepository(baseDir string, gun data.GUN, baseURL string,
 	if err != nil {
 		return nil, err
 	}
-	return NewNotaryRepository(baseDir, gun, baseURL, rt, cache, retriever, trustPinning)
-}
-
-// NewNotaryRepository is a helper method that returns a new notary repository.
-// It takes the base directory under where all the trust files will be stored
-// (This is normally defaults to "~/.notary" or "~/.docker/trust" when enabling
-// docker content trust).
-func NewNotaryRepository(baseDir string, gun data.GUN, baseURL string, rt http.RoundTripper, cache store.MetadataStore,
-	retriever notary.PassRetriever, trustPinning trustpinning.TrustPinConfig) (
-	*NotaryRepository, error) {
 
 	keyStores, err := getKeyStores(baseDir, retriever)
 	if err != nil {
 		return nil, err
 	}
 
-	return repositoryFromKeystores(baseDir, gun, baseURL, rt, cache,
-		keyStores, trustPinning)
+	cryptoService := cryptoservice.NewCryptoService(keyStores...)
+
+	remoteStore, err := getRemoteStore(baseURL, gun, rt)
+	if err != nil {
+		// baseURL is syntactically invalid
+		return nil, err
+	}
+
+	cl, err := changelist.NewFileChangelist(filepath.Join(
+		filepath.Join(baseDir, tufDir, filepath.FromSlash(gun.String()), "changelist"),
+	))
+	if err != nil {
+		return nil, err
+	}
+
+	return NewNotaryRepository(baseDir, gun, baseURL, remoteStore, cache, trustPinning, cryptoService, cl)
 }
 
-// repositoryFromKeystores is a helper function for NewNotaryRepository that
-// takes some basic NotaryRepository parameters as well as keystores (in order
-// of usage preference), and returns a NotaryRepository.
-func repositoryFromKeystores(baseDir string, gun data.GUN, baseURL string, rt http.RoundTripper, cache store.MetadataStore,
-	keyStores []trustmanager.KeyStore, trustPin trustpinning.TrustPinConfig) (*NotaryRepository, error) {
+// NewNotaryRepository is the base method that returns a new notary repository.
+// It takes the base directory under where all the trust files will be stored
+// (This is normally defaults to "~/.notary" or "~/.docker/trust" when enabling
+// docker content trust).
+// It expects an initialized remote store and cache.
+func NewNotaryRepository(baseDir string, gun data.GUN, baseURL string, remoteStore store.RemoteStore, cache store.MetadataStore,
+	trustPinning trustpinning.TrustPinConfig, cryptoService signed.CryptoService, cl changelist.Changelist) (
+	*NotaryRepository, error) {
 
-	cryptoService := cryptoservice.NewCryptoService(keyStores...)
+	// Repo's remote store is either a valid remote store or an OfflineStore
+	if remoteStore == nil {
+		remoteStore = store.OfflineStore{}
+	}
 
 	nRepo := &NotaryRepository{
 		gun:            gun,
-		baseDir:        baseDir,
 		baseURL:        baseURL,
-		tufRepoPath:    filepath.Join(baseDir, tufDir, filepath.FromSlash(gun.String())),
+		baseDir:        baseDir,
+		changelist:     cl,
+		cache:          cache,
+		remoteStore:    remoteStore,
 		CryptoService:  cryptoService,
-		roundTrip:      rt,
-		trustPinning:   trustPin,
+		trustPinning:   trustPinning,
 		LegacyVersions: 0, // By default, don't sign with legacy roles
 	}
 
-	nRepo.cache = cache
-
 	return nRepo, nil
 }
 
@@ -274,10 +285,13 @@ func (r *NotaryRepository) initializeRoles(rootKeys []data.PublicKey, localRoles
 			)
 		}
 	}
+
+	remote := r.getRemoteStore()
+
 	for _, role := range remoteRoles {
 		// This key is generated by the remote server.
 		var key data.PublicKey
-		key, err = getRemoteKey(r.baseURL, r.gun, role, r.roundTrip)
+		key, err = getRemoteKey(role, remote)
 		if err != nil {
 			return
 		}
@@ -302,8 +316,7 @@ func (r *NotaryRepository) initializeRoles(rootKeys []data.PublicKey, localRoles
 }
 
 // adds a TUF Change template to the given roles
-func addChange(cl *changelist.FileChangelist, c changelist.Change, roles ...data.RoleName) error {
-
+func addChange(cl changelist.Changelist, c changelist.Change, roles ...data.RoleName) error {
 	if len(roles) == 0 {
 		roles = []data.RoleName{data.CanonicalTargetsRole}
 	}
@@ -344,11 +357,6 @@ func (r *NotaryRepository) AddTarget(target *Target, roles ...data.RoleName) err
 	if len(target.Hashes) == 0 {
 		return fmt.Errorf("no hashes specified for target \"%s\"", target.Name)
 	}
-	cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist"))
-	if err != nil {
-		return err
-	}
-	defer cl.Close()
 	logrus.Debugf("Adding target \"%s\" with sha256 \"%x\" and size %d bytes.\n", target.Name, target.Hashes["sha256"], target.Length)
 
 	meta := data.FileMeta{Length: target.Length, Hashes: target.Hashes}
@@ -360,22 +368,17 @@ func (r *NotaryRepository) AddTarget(target *Target, roles ...data.RoleName) err
 	template := changelist.NewTUFChange(
 		changelist.ActionCreate, "", changelist.TypeTargetsTarget,
 		target.Name, metaJSON)
-	return addChange(cl, template, roles...)
+	return addChange(r.changelist, template, roles...)
 }
 
 // RemoveTarget creates new changelist entries to remove a target from the given
 // roles in the repository when the changelist gets applied at publish time.
 // If roles are unspecified, the default role is "target".
 func (r *NotaryRepository) RemoveTarget(targetName string, roles ...data.RoleName) error {
-
-	cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist"))
-	if err != nil {
-		return err
-	}
 	logrus.Debugf("Removing target \"%s\"", targetName)
 	template := changelist.NewTUFChange(changelist.ActionDelete, "",
 		changelist.TypeTargetsTarget, targetName, nil)
-	return addChange(cl, template, roles...)
+	return addChange(r.changelist, template, roles...)
 }
 
 // ListTargets lists all targets for the current repository. The list of
@@ -533,13 +536,19 @@ func (r *NotaryRepository) GetAllTargetMetadataByName(name string) ([]TargetSign
 
 // GetChangelist returns the list of the repository's unpublished changes
 func (r *NotaryRepository) GetChangelist() (changelist.Changelist, error) {
-	changelistDir := filepath.Join(r.tufRepoPath, "changelist")
-	cl, err := changelist.NewFileChangelist(changelistDir)
-	if err != nil {
-		logrus.Debug("Error initializing changelist")
-		return nil, err
+	return r.changelist, nil
+}
+
+// getRemoteStore returns the remoteStore of a repository if valid or
+// or an OfflineStore otherwise
+func (r *NotaryRepository) getRemoteStore() store.RemoteStore {
+	if r.remoteStore != nil {
+		return r.remoteStore
 	}
-	return cl, nil
+
+	r.remoteStore = &store.OfflineStore{}
+
+	return r.remoteStore
 }
 
 // RoleWithSignatures is a Role with its associated signatures
@@ -590,18 +599,14 @@ func (r *NotaryRepository) ListRoles() ([]RoleWithSignatures, error) {
 // Publish pushes the local changes in signed material to the remote notary-server
 // Conceptually it performs an operation similar to a `git rebase`
 func (r *NotaryRepository) Publish() error {
-	cl, err := r.GetChangelist()
-	if err != nil {
+	if err := r.publish(r.changelist); err != nil {
 		return err
 	}
-	if err = r.publish(cl); err != nil {
-		return err
-	}
-	if err = cl.Clear(""); err != nil {
+	if err := r.changelist.Clear(""); err != nil {
 		// This is not a critical problem when only a single host is pushing
 		// but will cause weird behaviour if changelist cleanup is failing
 		// and there are multiple hosts writing to the repo.
-		logrus.Warn("Unable to clear changelist. You may want to manually delete the folder ", filepath.Join(r.tufRepoPath, "changelist"))
+		logrus.Warn("Unable to clear changelist. You may want to manually delete the folder ", r.changelist.Location())
 	}
 	return nil
 }
@@ -686,10 +691,7 @@ func (r *NotaryRepository) publish(cl changelist.Changelist) error {
 		return err
 	}
 
-	remote, err := getRemoteStore(r.baseURL, r.gun, r.roundTrip)
-	if err != nil {
-		return err
-	}
+	remote := r.getRemoteStore()
 
 	return remote.SetMulti(data.MetadataRoleMapToStringMap(updatedFiles))
 }
@@ -970,10 +972,9 @@ func (r *NotaryRepository) bootstrapClient(checkInitialized bool) (*TUFClient, e
 		}
 	}
 
-	remote, remoteErr := getRemoteStore(r.baseURL, r.gun, r.roundTrip)
-	if remoteErr != nil {
-		logrus.Error(remoteErr)
-	} else if !newBuilder.IsLoaded(data.CanonicalRootRole) || checkInitialized {
+	remote := r.getRemoteStore()
+
+	if !newBuilder.IsLoaded(data.CanonicalRootRole) || checkInitialized {
 		// remoteErr was nil and we were not able to load a root from cache or
 		// are specifically checking for initialization of the repo.
 
@@ -1037,7 +1038,8 @@ func (r *NotaryRepository) pubKeyListForRotation(role data.RoleName, serverManag
 
 	// If server manages the key being rotated, request a rotation and return the new key
 	if serverManaged {
-		pubKey, err = rotateRemoteKey(r.baseURL, r.gun, role, r.roundTrip)
+		remote := r.getRemoteStore()
+		pubKey, err = rotateRemoteKey(role, remote)
 		pubKeyList = make(data.KeyList, 0, 1)
 		pubKeyList = append(pubKeyList, pubKey)
 		if err != nil {
@@ -1062,7 +1064,7 @@ func (r *NotaryRepository) pubKeyListForRotation(role data.RoleName, serverManag
 		for _, keyID := range newKeys {
 			pubKey = r.CryptoService.GetKey(keyID)
 			if pubKey == nil {
-				return nil, fmt.Errorf("unable to find key: %s")
+				return nil, fmt.Errorf("unable to find key: %s", keyID)
 			}
 			pubKeyList = append(pubKeyList, pubKey)
 		}
@@ -1139,15 +1141,17 @@ func (r *NotaryRepository) rootFileKeyChange(cl changelist.Changelist, role data
 
 // DeleteTrustData removes the trust data stored for this repo in the TUF cache on the client side
 // Note that we will not delete any private key material from local storage
-func (r *NotaryRepository) DeleteTrustData(deleteRemote bool) error {
+func DeleteTrustData(baseDir string, gun data.GUN, URL string, rt http.RoundTripper, deleteRemote bool) error {
+	localRepo := filepath.Join(baseDir, tufDir, filepath.FromSlash(gun.String()))
 	// Remove the tufRepoPath directory, which includes local TUF metadata files and changelist information
-	if err := os.RemoveAll(r.tufRepoPath); err != nil {
+	if err := os.RemoveAll(localRepo); err != nil {
 		return fmt.Errorf("error clearing TUF repo data: %v", err)
 	}
 	// Note that this will require admin permission in this NotaryRepository's roundtripper
 	if deleteRemote {
-		remote, err := getRemoteStore(r.baseURL, r.gun, r.roundTrip)
+		remote, err := getRemoteStore(URL, gun, rt)
 		if err != nil {
+			logrus.Error("unable to instantiate a remote store: %v", err)
 			return err
 		}
 		if err := remote.RemoveAll(); err != nil {