diff --git a/server/handlers/default.go b/server/handlers/default.go index a46371a8f1..96a601412c 100644 --- a/server/handlers/default.go +++ b/server/handlers/default.go @@ -122,9 +122,7 @@ func getHandler(ctx context.Context, w http.ResponseWriter, r *http.Request, var return errors.ErrNoStorage.WithDetail(nil) } - logger := ctxu.GetLoggerWithFields(ctx, map[string]interface{}{"gun": gun, "tufRole": tufRole}) - - return getRole(ctx, logger, w, store, gun, tufRole, checksum) + return getRole(ctx, w, store, gun, tufRole, checksum) } // DeleteHandler deletes all data for a GUN. A 200 responses indicates success. diff --git a/server/handlers/logic.go b/server/handlers/roles.go similarity index 70% rename from server/handlers/logic.go rename to server/handlers/roles.go index 42377cdbc4..15a1b3fbbe 100644 --- a/server/handlers/logic.go +++ b/server/handlers/roles.go @@ -3,8 +3,6 @@ package handlers import ( "io" - "github.com/Sirupsen/logrus" - ctxu "github.com/docker/distribution/context" "golang.org/x/net/context" "github.com/docker/notary/server/errors" @@ -15,7 +13,7 @@ import ( "github.com/docker/notary/tuf/signed" ) -func getRole(ctx context.Context, logger ctxu.Logger, w io.Writer, store storage.MetaStore, gun, role, checksum string) error { +func getRole(ctx context.Context, w io.Writer, store storage.MetaStore, gun, role, checksum string) error { var ( out []byte err error @@ -25,7 +23,7 @@ func getRole(ctx context.Context, logger ctxu.Logger, w io.Writer, store storage // handled specially switch role { case data.CanonicalTimestampRole, data.CanonicalSnapshotRole: - return getMaybeServerSigned(ctx, w, logger, store, gun, role) + return getMaybeServerSigned(ctx, w, store, gun, role) } out, err = store.GetCurrent(gun, role) } else { @@ -34,18 +32,14 @@ func getRole(ctx context.Context, logger ctxu.Logger, w io.Writer, store storage if err != nil { if _, ok := err.(storage.ErrNotFound); ok { - logrus.Errorf("404 GET %s:%s@%s, error: %v", gun, role, checksum, err) - return errors.ErrMetadataNotFound.WithDetail(nil) + return errors.ErrMetadataNotFound.WithDetail(err) } - logger.Error("500 GET") return errors.ErrUnknown.WithDetail(err) } if out == nil { - logger.Error("404 GET") return errors.ErrMetadataNotFound.WithDetail(nil) } w.Write(out) - logger.Debug("200 GET") return nil } @@ -55,7 +49,7 @@ func getRole(ctx context.Context, logger ctxu.Logger, w io.Writer, store storage // the timestamp and snapshot, based on the keys held by the server, a new one // might be generated and signed due to expiry of the previous one or updates // to other roles. -func getMaybeServerSigned(ctx context.Context, w io.Writer, logger ctxu.Logger, store storage.MetaStore, gun, role string) error { +func getMaybeServerSigned(ctx context.Context, w io.Writer, store storage.MetaStore, gun, role string) error { cryptoServiceVal := ctx.Value("cryptoService") cryptoService, ok := cryptoServiceVal.(signed.CryptoService) if !ok { @@ -75,15 +69,12 @@ func getMaybeServerSigned(ctx context.Context, w io.Writer, logger ctxu.Logger, if err != nil { switch err.(type) { case *storage.ErrNoKey, storage.ErrNotFound: - logger.Errorf("404 GET %s", role) - return errors.ErrMetadataNotFound.WithDetail(nil) + return errors.ErrMetadataNotFound.WithDetail(err) default: - logger.Errorf("500 GET %s", role) return errors.ErrUnknown.WithDetail(err) } } - logger.Debugf("200 GET %s", role) w.Write(out) return nil } diff --git a/server/handlers/logic_test.go b/server/handlers/roles_test.go similarity index 93% rename from server/handlers/logic_test.go rename to server/handlers/roles_test.go index 00b87195ea..ce6df0d4c5 100644 --- a/server/handlers/logic_test.go +++ b/server/handlers/roles_test.go @@ -3,7 +3,6 @@ package handlers import ( "testing" - "github.com/Sirupsen/logrus" "github.com/docker/distribution/registry/api/errcode" "github.com/stretchr/testify/require" "golang.org/x/net/context" @@ -17,7 +16,7 @@ import ( func TestGetMaybeServerSignedNoCrypto(t *testing.T) { err := getMaybeServerSigned( context.Background(), - nil, nil, nil, "", "", + nil, nil, "", "", ) require.Error(t, err) require.IsType(t, errcode.Error{}, err) @@ -37,7 +36,6 @@ func TestGetMaybeServerSignedNoKey(t *testing.T) { err := getMaybeServerSigned( ctx, nil, - logrus.StandardLogger(), store, "gun", data.CanonicalTimestampRole, diff --git a/server/server_test.go b/server/server_test.go index 427f2fddb0..caab4d3a8a 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -100,7 +100,7 @@ func TestGetRoleByHash(t *testing.T) { Signatures: make([]data.Signature, 0), Signed: data.Timestamp{ Type: data.TUFTypes["timestamp"], - Version: 0, + Version: 1, Expires: data.DefaultExpires("timestamp"), }, } @@ -116,6 +116,25 @@ func TestGetRoleByHash(t *testing.T) { store.UpdateCurrent("gun", update) + // create and add a newer timestamp. We're going to try and request + // the older version we created above. + ts = data.SignedTimestamp{ + Signatures: make([]data.Signature, 0), + Signed: data.Timestamp{ + Type: data.TUFTypes["timestamp"], + Version: 2, + Expires: data.DefaultExpires("timestamp"), + }, + } + newJ, err := json.Marshal(&ts) + assert.NoError(t, err) + update = storage.MetaUpdate{ + Role: data.CanonicalTimestampRole, + Version: 2, + Data: newJ, + } + store.UpdateCurrent("gun", update) + ctx := context.WithValue( context.Background(), "metaStore", store) diff --git a/server/storage/database_test.go b/server/storage/database_test.go index 1bae517673..37bd2bdc4d 100644 --- a/server/storage/database_test.go +++ b/server/storage/database_test.go @@ -451,7 +451,7 @@ func TestDBGetChecksum(t *testing.T) { Signatures: make([]data.Signature, 0), Signed: data.Timestamp{ Type: data.TUFTypes["timestamp"], - Version: 0, + Version: 1, Expires: data.DefaultExpires("timestamp"), }, } @@ -467,6 +467,26 @@ func TestDBGetChecksum(t *testing.T) { store.UpdateCurrent("gun", update) + // create and add a newer timestamp. We're going to try and get the one + // created above by checksum + ts = data.SignedTimestamp{ + Signatures: make([]data.Signature, 0), + Signed: data.Timestamp{ + Type: data.TUFTypes["timestamp"], + Version: 2, + Expires: data.DefaultExpires("timestamp"), + }, + } + newJ, err := json.Marshal(&ts) + require.NoError(t, err) + update = MetaUpdate{ + Role: data.CanonicalTimestampRole, + Version: 2, + Data: newJ, + } + + store.UpdateCurrent("gun", update) + data, err := store.GetChecksum("gun", data.CanonicalTimestampRole, checksum) require.NoError(t, err) require.EqualValues(t, j, data) diff --git a/server/storage/memory.go b/server/storage/memory.go index 55cd063d44..b46bd02302 100644 --- a/server/storage/memory.go +++ b/server/storage/memory.go @@ -24,7 +24,7 @@ type MemStorage struct { lock sync.Mutex tufMeta map[string][]*ver keys map[string]map[string]*key - checksums map[string][]byte + checksums map[string]map[string][]byte } // NewMemStorage instantiates a memStorage instance @@ -32,7 +32,7 @@ func NewMemStorage() *MemStorage { return &MemStorage{ tufMeta: make(map[string][]*ver), keys: make(map[string]map[string]*key), - checksums: make(map[string][]byte), + checksums: make(map[string]map[string][]byte), } } @@ -51,7 +51,12 @@ func (st *MemStorage) UpdateCurrent(gun string, update MetaUpdate) error { st.tufMeta[id] = append(st.tufMeta[id], &ver{version: update.Version, data: update.Data}) checksumBytes := sha256.Sum256(update.Data) checksum := hex.EncodeToString(checksumBytes[:]) - st.checksums[checksum] = update.Data + + _, ok := st.checksums[gun] + if !ok { + st.checksums[gun] = make(map[string][]byte) + } + st.checksums[gun][checksum] = update.Data return nil } @@ -63,7 +68,7 @@ func (st *MemStorage) UpdateMany(gun string, updates []MetaUpdate) error { return nil } -// GetCurrent returns the metadada for a given role, under a GUN +// GetCurrent returns the metadata for a given role, under a GUN func (st *MemStorage) GetCurrent(gun, role string) (data []byte, err error) { id := entryKey(gun, role) st.lock.Lock() @@ -75,18 +80,18 @@ func (st *MemStorage) GetCurrent(gun, role string) (data []byte, err error) { return space[len(space)-1].data, nil } -// GetChecksum returns the metadada for a given role, under a GUN +// GetChecksum returns the metadata for a given role, under a GUN func (st *MemStorage) GetChecksum(gun, role, checksum string) (data []byte, err error) { st.lock.Lock() defer st.lock.Unlock() - data, ok := st.checksums[checksum] + data, ok := st.checksums[gun][checksum] if !ok || len(data) == 0 { return nil, ErrNotFound{} } return data, nil } -// Delete delets all the metadata for a given GUN +// Delete deletes all the metadata for a given GUN func (st *MemStorage) Delete(gun string) error { st.lock.Lock() defer st.lock.Unlock() @@ -95,6 +100,7 @@ func (st *MemStorage) Delete(gun string) error { delete(st.tufMeta, k) } } + delete(st.checksums, gun) return nil }