review feedback

Signed-off-by: Nathan McCauley <nathan.mccauley@docker.com>

client/client.go

@@ -83,7 +83,9 @@ func NewTarget(targetName string, targetPath string) (*Target, error) {
 // 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
 // (usually ~/.docker/trust/).
-func NewNotaryRepository(baseDir, gun, baseURL string, rt http.RoundTripper, passphraseRetriever trustmanager.PassphraseRetriever) (*NotaryRepository, error) {
+func NewNotaryRepository(baseDir, gun, baseURL string, rt http.RoundTripper,
+	passphraseRetriever trustmanager.PassphraseRetriever) (*NotaryRepository, error) {
+
 	keyStoreManager, err := keystoremanager.NewKeyStoreManager(baseDir, passphraseRetriever)
 	if err != nil {
 		return nil, err
@@ -298,7 +300,7 @@ func (r *NotaryRepository) GetTargetByName(name string) (*Target, 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(getPass trustmanager.PassphraseRetriever) error {
+func (r *NotaryRepository) Publish() error {
 	var updateRoot bool
 	var root *data.Signed
 	// attempt to initialize the repo from the remote store

client/client_test.go

@@ -563,7 +563,7 @@ func testPublish(t *testing.T, rootType data.KeyAlgorithm) {
 	changelistDir.Close()
 
 	// Now test Publish
-	err = repo.Publish(passphraseRetriever)
+	err = repo.Publish()
 	assert.NoError(t, err)
 
 	changelistDir, err = os.Open(changelistDirPath)

cmd/notary-server/config.json

@@ -3,8 +3,10 @@
 		"addr": ":4443"
 	},
 	"trust_service": {
-		"type": "local"
-	},
+	  "type": "remote",
+	  "hostname": "notarysigner",
+	  "port": "7899",
+	  "tls_ca_file": "./fixtures/root-ca.crt"			},
 	"logging": {
 		"level": 5
 	},

cmd/notary-signer/main.go

@@ -22,6 +22,7 @@ import (
 	"github.com/miekg/pkcs11"
 
 	pb "github.com/docker/notary/proto"
+	"fmt"
 )
 
 const (

cmd/notary/tuf.go

@@ -20,7 +20,7 @@ import (
 )
 
 // FIXME: This should not be hardcoded
-const hardcodedBaseURL = "http://notary-server:4443"
+const hardcodedBaseURL = "https://notary-server:4443"
 
 var retriever trustmanager.PassphraseRetriever
 
@@ -207,7 +207,7 @@ func tufPublish(cmd *cobra.Command, args []string) {
 		fatalf(err.Error())
 	}
 
-	err = repo.Publish(retriever)
+	err = repo.Publish()
 	if err != nil {
 		fatalf(err.Error())
 	}

cryptoservice/crypto_service_test.go

@@ -17,12 +17,12 @@ func TestCryptoService(t *testing.T) {
 	}
 }
 
-var passphraseretriever = func(string, string, bool, int) (string, bool, error) { return "", false, nil }
+var passphraseRetriever = func(string, string, bool, int) (string, bool, error) { return "", false, nil }
 
 func testCryptoService(t *testing.T, keyAlgo data.KeyAlgorithm, verifier signed.Verifier) {
 	content := []byte("this is a secret")
 
-	keyStore := trustmanager.NewKeyMemoryStore(passphraseretriever)
+	keyStore := trustmanager.NewKeyMemoryStore(passphraseRetriever)
 	cryptoService := NewCryptoService("", keyStore)
 
 	// Test Create

cryptoservice/unlocked_crypto_service_test.go

@@ -9,8 +9,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-var passphraseRetriever = func(string, string, bool, int) (string, bool, error) { return "passphrase", false, nil }
-
 func TestUnlockedSigner(t *testing.T) {
 	privKey, err := trustmanager.GenerateECDSAKey(rand.Reader)
 	assert.NoError(t, err, "could not generate key")

docker-compose.yml

@@ -6,7 +6,7 @@ notaryserver:
    - notarysigner
   ports:
    - "8080"
-   - "4443:4443"
+   - "4443"
   environment:
     SERVICE_NAME: notary
 notarysigner:

keystoremanager/import_export.go

@@ -11,7 +11,6 @@ import (
 	"path/filepath"
 	"strings"
 
-	"fmt"
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/notary/trustmanager"
 )
@@ -169,11 +168,9 @@ func (km *KeyStoreManager) ExportAllKeys(dest io.Writer, newPassphraseRetriever
 
 	zipWriter := zip.NewWriter(dest)
 
-	fmt.Println("moving keys to archive: root")
 	if err := addKeysToArchive(zipWriter, tempRootKeyStore, privRootKeysSubdir); err != nil {
 		return err
 	}
-	fmt.Println("moving keys to archive: nonroot")
 	if err := addKeysToArchive(zipWriter, tempNonRootKeyStore, privNonRootKeysSubdir); err != nil {
 
 		return err

keystoremanager/import_export_test.go

@@ -86,9 +86,8 @@ func TestImportExportZip(t *testing.T) {
 	privKeyList := repo.KeyStoreManager.NonRootKeyStore().ListKeys()
 	for _, privKeyName := range privKeyList {
 		alias, err := repo.KeyStoreManager.NonRootKeyStore().GetKeyAlias(privKeyName)
-		if err != nil {
-			t.Fatalf("privKey %s has no alias", privKeyName)
-		}
+		assert.NoError(t, err, "privKey %s has no alias", privKeyName)
+
 		relKeyPath := filepath.Join("private", "tuf_keys", privKeyName+"_"+alias+".key")
 		passphraseByFile[relKeyPath] = exportPassphrase
 	}
@@ -158,9 +157,7 @@ func TestImportExportZip(t *testing.T) {
 	// in the repo's private key store.
 	for _, privKeyName := range privKeyList {
 		alias, err := repo.KeyStoreManager.NonRootKeyStore().GetKeyAlias(privKeyName)
-		if err != nil {
-			t.Fatalf("privKey %s has no alias", privKeyName)
-		}
+		assert.NoError(t, err, "privKey %s has no alias", privKeyName)
 
 		relKeyPath := filepath.Join("private", "tuf_keys", privKeyName+"_"+alias+".key")
 		privKeyFileName := filepath.Join(tempBaseDir2, relKeyPath)

trustmanager/filestore.go

@@ -172,6 +172,7 @@ func (f *SimpleFileStore) list(path string, symlinks bool) []string {
 		matched, _ := filepath.Match("*"+f.fileExt, fi.Name())
 
 		if matched {
+			// Find the relative path for this file relative to the base path.
 			fp, err = filepath.Rel(path, fp)
 			if err != nil {
 				return err

trustmanager/keyfilestore.go

@@ -11,7 +11,6 @@ import (
 
 const (
 	keyExtension   = "key"
-	aliasExtension = "alias"
 )
 
 // KeyStore is a generic interface for private key storage
@@ -134,7 +133,7 @@ func addKey(s LimitedFileStore, passphraseRetriever PassphraseRetriever, name, a
 			return errors.New("obtaining passphrase failed")
 		}
 		if attempts > 10 {
-			return errors.New("sanity check on number of passphrase attempts exceeded")
+			return errors.New("maximum number of passphrase attempts exceeded")
 		}
 		break
 	}
@@ -191,7 +190,7 @@ func getKey(s LimitedFileStore, passphraseRetriever PassphraseRetriever, name st
 				return nil, errors.New("obtaining passphrase failed")
 			}
 			if attempts > 10 {
-				return nil, errors.New("sanity check on number of passphrase attempts exceeded")
+				return nil, errors.New("maximum number of passphrase attempts exceeded")
 			}
 
 			// Try to convert PEM encoded bytes back to a PrivateKey using the passphrase