NotaryRepository.Update now just returns an error, rather than a client

an error, because we don't actually use the client anymore.

Signed-off-by: Ying Li <ying.li@docker.com>
This commit is contained in:
Ying Li 2016-04-06 14:08:08 -07:00
parent 819499d6d7
commit f8c42e4cbf
5 changed files with 35 additions and 41 deletions

View File

@ -130,7 +130,7 @@ func Test0Dot1RepoFormat(t *testing.T) {
// and we can download the snapshot
require.NoError(t, repo.RotateKey(data.CanonicalSnapshotRole, true))
require.NoError(t, repo.Publish())
_, err = repo.Update(false)
err = repo.Update(false)
require.NoError(t, err)
}
@ -155,6 +155,6 @@ func TestDownloading0Dot1RepoFormat(t *testing.T) {
passphrase.ConstantRetriever(passwd))
require.NoError(t, err, "error creating repo: %s", err)
_, err = repo.Update(true)
err = repo.Update(true)
require.NoError(t, err, "error updating repo: %s", err)
}

View File

@ -386,7 +386,7 @@ func (r *NotaryRepository) RemoveTarget(targetName string, roles ...string) erro
// subtree and also the "targets/x" subtree, as we will defer parsing it until
// we explicitly reach it in our iteration of the provided list of roles.
func (r *NotaryRepository) ListTargets(roles ...string) ([]*TargetWithRole, error) {
_, err := r.Update(false)
err := r.Update(false)
if err != nil {
return nil, err
}
@ -432,8 +432,7 @@ func (r *NotaryRepository) ListTargets(roles ...string) ([]*TargetWithRole, erro
// will be returned
// See the IMPORTANT section on ListTargets above. Those roles also apply here.
func (r *NotaryRepository) GetTargetByName(name string, roles ...string) (*TargetWithRole, error) {
_, err := r.Update(false)
if err != nil {
if err := r.Update(false); err != nil {
return nil, err
}
@ -460,9 +459,8 @@ func (r *NotaryRepository) GetTargetByName(name string, roles ...string) (*Targe
}
return nil
}
err = r.tufRepo.WalkTargets(name, role, getTargetVisitorFunc, skipRoles...)
// Check that we didn't error, and that we assigned to our target
if err == nil && foundTarget {
if err := r.tufRepo.WalkTargets(name, role, getTargetVisitorFunc, skipRoles...); err == nil && foundTarget {
return &TargetWithRole{Target: Target{Name: name, Hashes: resultMeta.Hashes, Length: resultMeta.Length}, Role: resultRoleName}, nil
}
}
@ -491,8 +489,7 @@ type RoleWithSignatures struct {
// This represents the latest metadata for each role in this repo
func (r *NotaryRepository) ListRoles() ([]RoleWithSignatures, error) {
// Update to latest repo state
_, err := r.Update(false)
if err != nil {
if err := r.Update(false); err != nil {
return nil, err
}
@ -552,8 +549,7 @@ func (r *NotaryRepository) Publish() error {
func (r *NotaryRepository) publish(cl changelist.Changelist) error {
var initialPublish bool
// update first before publishing
_, err := r.Update(true)
if err != nil {
if err := r.Update(true); err != nil {
// If the remote is not aware of the repo, then this is being published
// for the first time. Try to load from disk instead for publishing.
if _, ok := err.(ErrRepositoryNotExist); ok {
@ -578,8 +574,7 @@ func (r *NotaryRepository) publish(cl changelist.Changelist) error {
}
}
// apply the changelist to the repo
err = applyChangelist(r.tufRepo, cl)
if err != nil {
if err := applyChangelist(r.tufRepo, cl); err != nil {
logrus.Debug("Error applying changelist")
return err
}
@ -756,25 +751,24 @@ func (r *NotaryRepository) errRepositoryNotExist() error {
// Update bootstraps a trust anchor (root.json) before updating all the
// metadata from the repo.
func (r *NotaryRepository) Update(forWrite bool) (*tufclient.Client, error) {
func (r *NotaryRepository) Update(forWrite bool) error {
c, err := r.bootstrapClient(forWrite)
if err != nil {
if _, ok := err.(store.ErrMetaNotFound); ok {
return nil, r.errRepositoryNotExist()
return r.errRepositoryNotExist()
}
return nil, err
return err
}
err = c.Update()
if err != nil {
if err := c.Update(); err != nil {
// notFound.Resource may include a checksum so when the role is root,
// it will be root.json or root.<checksum>.json. Therefore best we can
// do it match a "root." prefix
if notFound, ok := err.(store.ErrMetaNotFound); ok && strings.HasPrefix(notFound.Resource, data.CanonicalRootRole+".") {
return nil, r.errRepositoryNotExist()
return r.errRepositoryNotExist()
}
return nil, err
return err
}
return c, nil
return nil
}
// bootstrapClient attempts to bootstrap a root.json to be used as the trust

View File

@ -2588,7 +2588,7 @@ func requireRotationSuccessful(t *testing.T, repo1 *NotaryRepository, keysToRota
// Download data from remote and check that keys have changed
for _, repo := range repos {
_, err := repo.Update(true)
err := repo.Update(true)
require.NoError(t, err)
for role, isRemoteKey := range keysToRotate {

View File

@ -106,7 +106,7 @@ func TestUpdateSucceedsEvenIfCannotWriteNewRepo(t *testing.T) {
for role := range serverMeta {
repo := newBlankRepo(t, ts.URL)
repo.fileStore = &unwritableStore{MetadataStore: repo.fileStore, roleToNotWrite: role}
_, err := repo.Update(false)
err := repo.Update(false)
if role == data.CanonicalRootRole {
require.Error(t, err) // because checkRoot loads root from cache to check hashes
@ -146,7 +146,7 @@ func TestUpdateSucceedsEvenIfCannotWriteExistingRepo(t *testing.T) {
repo := newBlankRepo(t, ts.URL)
defer os.RemoveAll(repo.baseDir)
_, err := repo.Update(false)
err := repo.Update(false)
require.NoError(t, err)
origFileStore := repo.fileStore
@ -158,7 +158,7 @@ func TestUpdateSucceedsEvenIfCannotWriteExistingRepo(t *testing.T) {
// update fileStore
repo.fileStore = &unwritableStore{MetadataStore: origFileStore, roleToNotWrite: role}
_, err := repo.Update(forWrite)
err := repo.Update(forWrite)
if role == data.CanonicalRootRole {
require.Error(t, err) // because checkRoot loads root from cache to check hashes
@ -222,7 +222,7 @@ func TestUpdateReplacesCorruptOrMissingMetadata(t *testing.T) {
repo := newBlankRepo(t, ts.URL)
defer os.RemoveAll(repo.baseDir)
_, err = repo.Update(false) // ensure we have all metadata to start with
err = repo.Update(false) // ensure we have all metadata to start with
require.NoError(t, err)
// we want to swizzle the local cache, not the server, so create a new one
@ -234,7 +234,7 @@ func TestUpdateReplacesCorruptOrMissingMetadata(t *testing.T) {
text, messItUp := expt.desc, expt.swizzle
for _, forWrite := range []bool{true, false} {
require.NoError(t, messItUp(repoSwizzler, role), "could not fuzz %s (%s)", role, text)
_, err := repo.Update(forWrite)
err := repo.Update(forWrite)
require.NoError(t, err)
for r, expected := range serverMeta {
actual, err := repo.fileStore.GetMeta(r, -1)
@ -266,7 +266,7 @@ func TestUpdateFailsIfServerRootKeyChangedWithoutMultiSign(t *testing.T) {
repo := newBlankRepo(t, ts.URL)
defer os.RemoveAll(repo.baseDir)
_, err := repo.Update(false) // ensure we have all metadata to start with
err := repo.Update(false) // ensure we have all metadata to start with
require.NoError(t, err)
// rotate the server's root.json root key so that they no longer match trust anchors
@ -289,14 +289,14 @@ func TestUpdateFailsIfServerRootKeyChangedWithoutMultiSign(t *testing.T) {
if _, ok := err.(store.ErrMetaNotFound); ok { // one of the ways to mess up is to delete metadata
_, err = repo.Update(forWrite)
err = repo.Update(forWrite)
require.Error(t, err) // the new server has a different root key, update fails
} else {
require.NoError(t, err)
_, err = repo.Update(forWrite)
err = repo.Update(forWrite)
require.Error(t, err) // the new server has a different root, update fails
// we can't test that all the metadata is the same, because we probably would
@ -615,7 +615,7 @@ func testUpdateRemoteNon200Error(t *testing.T, opts updateOpts, errExpected inte
defer os.RemoveAll(repo.baseDir)
if opts.localCache {
_, err := repo.Update(false) // acquire local cache
err := repo.Update(false) // acquire local cache
require.NoError(t, err)
}
@ -625,7 +625,7 @@ func testUpdateRemoteNon200Error(t *testing.T, opts updateOpts, errExpected inte
require.NoError(t, serverSwizzler.RemoveMetadata(opts.role), "failed to remove %s", opts.role)
_, err := repo.Update(opts.forWrite)
err := repo.Update(opts.forWrite)
if errExpected == nil {
require.NoError(t, err, "expected no failure updating when %s is %v (forWrite: %v)",
opts.role, opts.notFoundCode, opts.forWrite)
@ -727,7 +727,7 @@ func testUpdateRemoteFileChecksumWrong(t *testing.T, opts updateOpts, errExpecte
defer os.RemoveAll(repo.baseDir)
if opts.localCache {
_, err := repo.Update(false) // acquire local cache
err := repo.Update(false) // acquire local cache
require.NoError(t, err)
}
@ -737,7 +737,7 @@ func testUpdateRemoteFileChecksumWrong(t *testing.T, opts updateOpts, errExpecte
require.NoError(t, serverSwizzler.AddExtraSpace(opts.role), "failed to checksum-corrupt to %s", opts.role)
_, err := repo.Update(opts.forWrite)
err := repo.Update(opts.forWrite)
if !errExpected {
require.NoError(t, err, "expected no failure updating when %s has the wrong checksum (forWrite: %v)",
opts.role, opts.forWrite)
@ -1141,7 +1141,7 @@ func testUpdateRemoteCorruptValidChecksum(t *testing.T, opts updateOpts, expt sw
defer os.RemoveAll(repo.baseDir)
if opts.localCache {
_, err := repo.Update(false)
err := repo.Update(false)
require.NoError(t, err)
}
@ -1175,7 +1175,7 @@ func testUpdateRemoteCorruptValidChecksum(t *testing.T, opts updateOpts, expt sw
}
}
}
_, err := repo.Update(opts.forWrite)
err := repo.Update(opts.forWrite)
if shouldErr {
require.Error(t, err, "expected failure updating when %s", msg)
@ -1230,7 +1230,7 @@ func testUpdateLocalAndRemoteRootCorrupt(t *testing.T, forWrite bool, localExpt,
defer os.RemoveAll(repo.baseDir)
// get local cache
_, err := repo.Update(false)
err := repo.Update(false)
require.NoError(t, err)
repoSwizzler := &testutils.MetadataSwizzler{
Gun: serverSwizzler.Gun,
@ -1253,7 +1253,7 @@ func testUpdateLocalAndRemoteRootCorrupt(t *testing.T, forWrite bool, localExpt,
msg := fmt.Sprintf("swizzling root locally to return <%v> and remotely to return: <%v> (forWrite: %v)",
localExpt.desc, serverExpt.desc, forWrite)
_, err = repo.Update(forWrite)
err = repo.Update(forWrite)
require.Error(t, err, "expected failure updating when %s", msg)
errType := reflect.TypeOf(err)
@ -1292,7 +1292,7 @@ func testUpdateRemoteKeyRotated(t *testing.T, targetsRole string) {
defer os.RemoveAll(repo.baseDir)
// get local cache
_, err := repo.Update(false)
err := repo.Update(false)
require.NoError(t, err)
cs := signed.NewEd25519()
@ -1316,7 +1316,7 @@ func testUpdateRemoteKeyRotated(t *testing.T, targetsRole string) {
msg := fmt.Sprintf("swizzling %s remotely to rotate key (forWrite: false)", targetsRole)
_, err = repo.Update(false)
err = repo.Update(false)
require.Error(t, err, "expected failure updating when %s", msg)
require.IsType(t, signed.ErrRoleThreshold{}, err, "expected ErrRoleThreshold when %s: got %s",
msg, reflect.TypeOf(err))

View File

@ -240,7 +240,7 @@ func newDeleteDelegationChange(name string, content []byte) *changelist.TufChang
// Also converts key IDs to canonical key IDs to keep consistent with signing prompts
func (r *NotaryRepository) GetDelegationRoles() ([]*data.Role, error) {
// Update state of the repo to latest
if _, err := r.Update(false); err != nil {
if err := r.Update(false); err != nil {
return nil, err
}