From aaf45a9ccec53bbcfd9c88b5032f32bf8c64cfcd Mon Sep 17 00:00:00 2001 From: Ying Li Date: Thu, 10 Dec 2015 14:40:16 -0800 Subject: [PATCH] Refactor Initialize to be easier to read, and update comments per review. Signed-off-by: Ying Li --- client/client.go | 91 +++++++++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 43 deletions(-) diff --git a/client/client.go b/client/client.go index 672e1aa0fc..3e51d602d9 100644 --- a/client/client.go +++ b/client/client.go @@ -143,18 +143,28 @@ func (r *NotaryRepository) Initialize(rootKeyID string, serverManagedRoles ...st return err } - // currently we only support server managing snapshots, and nothing else - rolesAreManaged := map[string]bool{ - data.CanonicalTimestampRole: true, - data.CanonicalSnapshotRole: false, - data.CanonicalTargetsRole: false, + // currently we only support server managing timestamps and snapshots, and + // nothing else - timestamps are always managed by the server, and implicit + // (do not have to be passed in as part of `serverManagedRoles`, so that + // the API of Initialize doens't change). + var serverManagesSnapshot bool + locallyManagedKeys := []string{ + data.CanonicalTargetsRole, + data.CanonicalSnapshotRole, + // root is also locally managed, but that should have been created + // already } + remotelyManagedKeys := []string{data.CanonicalTimestampRole} for _, role := range serverManagedRoles { switch role { case data.CanonicalTimestampRole: - continue // always support timestamp + continue // timestamp is already in the right place case data.CanonicalSnapshotRole: - rolesAreManaged[data.CanonicalSnapshotRole] = true + // because we put Snapshot last + locallyManagedKeys = []string{data.CanonicalTargetsRole} + remotelyManagedKeys = append( + remotelyManagedKeys, data.CanonicalSnapshotRole) + serverManagesSnapshot = true default: return fmt.Errorf( "Notary does not support the server managing the %s key", role) @@ -194,31 +204,26 @@ func (r *NotaryRepository) Initialize(rootKeyID string, serverManagedRoles ...st // we want to create all the local keys first so we don't have to // make unnecessary network calls - for _, isManaged := range []bool{false, true} { - for role, shouldBeManaged := range rolesAreManaged { - if isManaged != shouldBeManaged { - continue - } - var key data.PublicKey - if isManaged { - // This key is generated by the remote server. - key, err = getRemoteKey(r.baseURL, r.gun, role, r.roundTrip) - if err != nil { - return err - } - logrus.Debugf("got remote %s %s key with keyID: %s", - role, key.Algorithm(), key.ID()) - } else { - // This is currently hardcoding the keys to ECDSA. - key, err = r.CryptoService.Create(role, data.ECDSAKey) - if err != nil { - return err - } - } - err = addKeyForRole(kdb, role, key) - if err != nil { - return err - } + for _, role := range locallyManagedKeys { + // This is currently hardcoding the keys to ECDSA. + key, err := r.CryptoService.Create(role, data.ECDSAKey) + if err != nil { + return err + } + if err := addKeyForRole(kdb, role, key); err != nil { + return err + } + } + for _, role := range remotelyManagedKeys { + // This key is generated by the remote server. + key, err := getRemoteKey(r.baseURL, r.gun, role, r.roundTrip) + if err != nil { + return err + } + logrus.Debugf("got remote %s %s key with keyID: %s", + role, key.Algorithm(), key.ID()) + if err := addKeyForRole(kdb, role, key); err != nil { + return err } } @@ -240,7 +245,7 @@ func (r *NotaryRepository) Initialize(rootKeyID string, serverManagedRoles ...st return err } - return r.saveMetadata(rolesAreManaged[data.CanonicalSnapshotRole]) + return r.saveMetadata(serverManagesSnapshot) } // AddTarget adds a new target to the repository, forcing a timestamps check from TUF @@ -346,7 +351,6 @@ func (r *NotaryRepository) GetChangelist() (changelist.Changelist, error) { // Conceptually it performs an operation similar to a `git rebase` func (r *NotaryRepository) Publish() error { var updateRoot bool - var root *data.Signed // attempt to initialize the repo from the remote store c, err := r.bootstrapClient() if err != nil { @@ -357,6 +361,8 @@ func (r *NotaryRepository) Publish() error { if err != nil { // There are lots of reasons there might be an error, such as // corrupt metadata. We need better errors from bootstrapRepo. + logrus.Debugf("Unable to load repository from local files: %s", + err.Error()) return err } // We had local data but the server doesn't know about the repo yet, @@ -395,25 +401,23 @@ func (r *NotaryRepository) Publish() error { // these are the tuf files we will need to update, serialized as JSON before // we send anything to remote - update := make(map[string][]byte) + updatedFiles := make(map[string][]byte) // check if our root file is nearing expiry. Resign if it is. if nearExpiry(r.tufRepo.Root) || r.tufRepo.Root.Dirty || updateRoot { - root, err = r.tufRepo.SignRoot(data.DefaultExpires("root")) - - rootJSON, err := json.Marshal(root) + rootJSON, err := serializeCanonicalRole(r.tufRepo, data.CanonicalRootRole) if err != nil { return err } - update[data.CanonicalRootRole] = rootJSON + updatedFiles[data.CanonicalRootRole] = rootJSON } - // we will always resign targets, and snapshots if we have the snapshots key + // we will always re-sign targets targetsJSON, err := serializeCanonicalRole(r.tufRepo, data.CanonicalTargetsRole) if err != nil { return err } - update[data.CanonicalTargetsRole] = targetsJSON + updatedFiles[data.CanonicalTargetsRole] = targetsJSON // do not update the snapshot role if we do not have the snapshot key or // any snapshot data. There might not be any snapshot data the repo was @@ -425,11 +429,12 @@ func (r *NotaryRepository) Publish() error { snapshotJSON, err := serializeCanonicalRole( r.tufRepo, data.CanonicalSnapshotRole) if err == nil { // we have the key - snapshot signed, let's update it - update[data.CanonicalSnapshotRole] = snapshotJSON + updatedFiles[data.CanonicalSnapshotRole] = snapshotJSON } else if _, ok := err.(signed.ErrNoKeys); ok { logrus.Debugf("Client does not have the key to sign snapshot. " + "Assuming that server should sign the snapshot.") } else { + logrus.Debugf("Client was unable to sign the snapshot: %s", err.Error()) return err } } @@ -439,7 +444,7 @@ func (r *NotaryRepository) Publish() error { return err } - err = remote.SetMultiMeta(update) + err = remote.SetMultiMeta(updatedFiles) if err != nil { return err }