fixing timestamps, clearing changelists, and the Adding target byte log

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

client/client.go

@@ -238,7 +238,7 @@ func (r *NotaryRepository) AddTarget(target *Target) error {
 	if err != nil {
 		return err
 	}
-	fmt.Printf("Adding target \"%s\" with sha256 \"%s\" and size %d bytes.\n", target.Name, target.Hashes["sha256"], target.Length)
+	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}
 	metaJSON, err := json.Marshal(meta)
@@ -340,9 +340,9 @@ func (r *NotaryRepository) Publish(getPass passwordRetriever) error {
 			return err
 		}
 	}
-
 	// load the changelist for this repo
-	cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist"))
+	changelistDir := filepath.Join(r.tufRepoPath, "changelist")
+	cl, err := changelist.NewFileChangelist(changelistDir)
 	if err != nil {
 		logrus.Debug("Error initializing changelist")
 		return err
@@ -406,7 +406,18 @@ func (r *NotaryRepository) Publish(getPass passwordRetriever) error {
 	}
 	update["targets"] = targetsJSON
 	update["snapshot"] = snapshotJSON
-	return remote.SetMultiMeta(update)
+	err = remote.SetMultiMeta(update)
+	if err != nil {
+		return err
+	}
+	err = cl.Clear("")
+	if 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 ", changelistDir)
+	}
+	return nil
 }
 
 func (r *NotaryRepository) bootstrapRepo() error {

client/client_test.go

@@ -570,6 +570,13 @@ func testPublish(t *testing.T, rootType data.KeyAlgorithm) {
 	})
 	assert.NoError(t, err)
 
+	changelistDir, err = os.Open(changelistDirPath)
+	assert.NoError(t, err, "could not open changelist directory")
+	fileInfos, err = changelistDir.Readdir(0)
+	assert.NoError(t, err, "could not read changelist directory")
+	// Should only be one file in the directory
+	assert.Len(t, fileInfos, 0, "wrong number of changelist files found")
+
 	// Create a new repo and pull from the server
 	tempBaseDir2, err := ioutil.TempDir("", "notary-test-")
 	defer os.RemoveAll(tempBaseDir2)

server/timestamp/timestamp.go

@@ -1,6 +1,7 @@
 package timestamp
 
 import (
+	"bytes"
 	"encoding/json"
 	"time"
 
@@ -49,6 +50,10 @@ func GetOrCreateTimestampKey(gun string, store storage.MetaStore, crypto signed.
 // a new timestamp is generated either because none exists, or because the current
 // one has expired. Once generated, the timestamp is saved in the store.
 func GetOrCreateTimestamp(gun string, store storage.MetaStore, cryptoService signed.CryptoService) ([]byte, error) {
+	snapshot, err := store.GetCurrent(gun, "snapshot")
+	if err != nil {
+		return nil, err
+	}
 	d, err := store.GetCurrent(gun, "timestamp")
 	if err != nil {
 		if _, ok := err.(*storage.ErrNotFound); !ok {
@@ -65,11 +70,11 @@ func GetOrCreateTimestamp(gun string, store storage.MetaStore, cryptoService sig
 			logrus.Error("Failed to unmarshal existing timestamp")
 			return nil, err
 		}
-		if !timestampExpired(ts) {
+		if !timestampExpired(ts) && !snapshotExpired(ts, snapshot) {
 			return d, nil
 		}
 	}
-	sgnd, version, err := createTimestamp(gun, ts, store, cryptoService)
+	sgnd, version, err := CreateTimestamp(gun, ts, snapshot, store, cryptoService)
 	if err != nil {
 		logrus.Error("Failed to create a new timestamp")
 		return nil, err
@@ -91,11 +96,23 @@ func timestampExpired(ts *data.SignedTimestamp) bool {
 	return time.Now().After(ts.Signed.Expires)
 }
 
-// createTimestamp creates a new timestamp. If a prev timestamp is provided, it
+func snapshotExpired(ts *data.SignedTimestamp, snapshot []byte) bool {
+	meta, err := data.NewFileMeta(bytes.NewReader(snapshot), "sha256")
+	if err != nil {
+		// if we can't generate FileMeta from the current snapshot, we should
+		// continue to serve the old timestamp if it isn't time expired
+		// because we won't be able to generate a new one.
+		return false
+	}
+	hash := meta.Hashes["sha256"]
+	return !bytes.Equal(hash, ts.Signed.Meta["snapshot"].Hashes["sha256"])
+}
+
+// CreateTimestamp creates a new timestamp. If a prev timestamp is provided, it
 // is assumed this is the immediately previous one, and the new one will have a
 // version number one higher than prev. The store is used to lookup the current
 // snapshot, this function does not save the newly generated timestamp.
-func createTimestamp(gun string, prev *data.SignedTimestamp, store storage.MetaStore, cryptoService signed.CryptoService) (*data.Signed, int, error) {
+func CreateTimestamp(gun string, prev *data.SignedTimestamp, snapshot []byte, store storage.MetaStore, cryptoService signed.CryptoService) (*data.Signed, int, error) {
 	algorithm, public, err := store.GetTimestampKey(gun)
 	if err != nil {
 		// owner of gun must have generated a timestamp key otherwise
@@ -103,10 +120,6 @@ func createTimestamp(gun string, prev *data.SignedTimestamp, store storage.MetaS
 		return nil, 0, err
 	}
 	key := data.NewPublicKey(algorithm, public)
-	snapshot, err := store.GetCurrent(gun, "snapshot")
-	if err != nil {
-		return nil, 0, err
-	}
 	sn := &data.Signed{}
 	err = json.Unmarshal(snapshot, sn)
 	if err != nil {

server/timestamp/timestamp_test.go

@@ -60,3 +60,31 @@ func TestGetTimestamp(t *testing.T) {
 	_, err = GetOrCreateTimestamp("gun", store, crypto)
 	assert.Nil(t, err, "GetTimestamp errored")
 }
+
+func TestGetTimestampNewSnapshot(t *testing.T) {
+	store := storage.NewMemStorage()
+	crypto := signed.NewEd25519()
+
+	snapshot := data.SignedSnapshot{}
+	snapshot.Signed.Version = 0
+	snapJSON, _ := json.Marshal(snapshot)
+
+	store.UpdateCurrent("gun", storage.MetaUpdate{Role: "snapshot", Version: 0, Data: snapJSON})
+	// create a key to be used by GetTimestamp
+	_, err := GetOrCreateTimestampKey("gun", store, crypto, data.ED25519Key)
+	assert.Nil(t, err, "GetTimestampKey errored")
+
+	ts1, err := GetOrCreateTimestamp("gun", store, crypto)
+	assert.Nil(t, err, "GetTimestamp errored")
+
+	snapshot = data.SignedSnapshot{}
+	snapshot.Signed.Version = 1
+	snapJSON, _ = json.Marshal(snapshot)
+
+	store.UpdateCurrent("gun", storage.MetaUpdate{Role: "snapshot", Version: 1, Data: snapJSON})
+
+	ts2, err := GetOrCreateTimestamp("gun", store, crypto)
+	assert.Nil(t, err, "GetTimestamp errored")
+
+	assert.NotEqual(t, ts1, ts2, "Timestamp was not regenerated when snapshot changed")
+}