diff --git a/client/changelist/changelist.go b/client/changelist/changelist.go index 9b52981ad..30cf69459 100644 --- a/client/changelist/changelist.go +++ b/client/changelist/changelist.go @@ -21,6 +21,11 @@ func (cl *memChangelist) Add(c Change) error { return nil } +// Location returns the string "memory" +func (cl memChangelist) Location() string { + return "memory" +} + // Remove deletes the changes found at the given indices func (cl *memChangelist) Remove(idxs []int) error { remove := make(map[int]struct{}) diff --git a/client/changelist/file_changelist.go b/client/changelist/file_changelist.go index a25215482..7e128a194 100644 --- a/client/changelist/file_changelist.go +++ b/client/changelist/file_changelist.go @@ -137,6 +137,11 @@ func (cl FileChangelist) Close() error { return nil } +// Location returns the file path to the changelist +func (cl FileChangelist) Location() string { + return cl.dir +} + // NewIterator creates an iterator from FileChangelist func (cl FileChangelist) NewIterator() (ChangeIterator, error) { fileInfos, err := getFileNames(cl.dir) diff --git a/client/changelist/interface.go b/client/changelist/interface.go index d1af57445..70dc0a2d0 100644 --- a/client/changelist/interface.go +++ b/client/changelist/interface.go @@ -27,6 +27,9 @@ type Changelist interface { // NewIterator returns an iterator for walking through the list // of changes currently stored NewIterator() (ChangeIterator, error) + + // Location returns the place the changelist is stores + Location() string } const ( diff --git a/client/client.go b/client/client.go index b74f62496..2a27e32a0 100644 --- a/client/client.go +++ b/client/client.go @@ -17,7 +17,6 @@ import ( "github.com/docker/notary/client/changelist" "github.com/docker/notary/cryptoservice" store "github.com/docker/notary/storage" - "github.com/docker/notary/trustmanager" "github.com/docker/notary/trustpinning" "github.com/docker/notary/tuf" "github.com/docker/notary/tuf/data" @@ -42,8 +41,9 @@ type NotaryRepository struct { baseDir string gun data.GUN baseURL string - tufRepoPath string + changelist changelist.Changelist cache store.MetadataStore + remoteStore store.RemoteStore CryptoService signed.CryptoService tufRepo *tuf.Repo invalid *tuf.Repo // known data that was parsable but deemed invalid @@ -53,7 +53,9 @@ type NotaryRepository struct { } // NewFileCachedNotaryRepository is a wrapper for NewNotaryRepository that initializes -// a file cache from the provided repository and local config information +// a file cache from the provided repository, local config information and a crypto service. +// It also retrieves the remote store associated to the base directory under where all the +// trust files will be stored and the specified GUN. func NewFileCachedNotaryRepository(baseDir string, gun data.GUN, baseURL string, rt http.RoundTripper, retriever notary.PassRetriever, trustPinning trustpinning.TrustPinConfig) ( *NotaryRepository, error) { @@ -65,47 +67,56 @@ func NewFileCachedNotaryRepository(baseDir string, gun data.GUN, baseURL string, if err != nil { return nil, err } - return NewNotaryRepository(baseDir, gun, baseURL, rt, cache, retriever, trustPinning) -} - -// 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 -// (This is normally defaults to "~/.notary" or "~/.docker/trust" when enabling -// docker content trust). -func NewNotaryRepository(baseDir string, gun data.GUN, baseURL string, rt http.RoundTripper, cache store.MetadataStore, - retriever notary.PassRetriever, trustPinning trustpinning.TrustPinConfig) ( - *NotaryRepository, error) { keyStores, err := getKeyStores(baseDir, retriever) if err != nil { return nil, err } - return repositoryFromKeystores(baseDir, gun, baseURL, rt, cache, - keyStores, trustPinning) + cryptoService := cryptoservice.NewCryptoService(keyStores...) + + remoteStore, err := getRemoteStore(baseURL, gun, rt) + if err != nil { + // baseURL is syntactically invalid + return nil, err + } + + cl, err := changelist.NewFileChangelist(filepath.Join( + filepath.Join(baseDir, tufDir, filepath.FromSlash(gun.String()), "changelist"), + )) + if err != nil { + return nil, err + } + + return NewNotaryRepository(baseDir, gun, baseURL, remoteStore, cache, trustPinning, cryptoService, cl) } -// repositoryFromKeystores is a helper function for NewNotaryRepository that -// takes some basic NotaryRepository parameters as well as keystores (in order -// of usage preference), and returns a NotaryRepository. -func repositoryFromKeystores(baseDir string, gun data.GUN, baseURL string, rt http.RoundTripper, cache store.MetadataStore, - keyStores []trustmanager.KeyStore, trustPin trustpinning.TrustPinConfig) (*NotaryRepository, error) { +// NewNotaryRepository is the base method that returns a new notary repository. +// It takes the base directory under where all the trust files will be stored +// (This is normally defaults to "~/.notary" or "~/.docker/trust" when enabling +// docker content trust). +// It expects an initialized remote store and cache. +func NewNotaryRepository(baseDir string, gun data.GUN, baseURL string, remoteStore store.RemoteStore, cache store.MetadataStore, + trustPinning trustpinning.TrustPinConfig, cryptoService signed.CryptoService, cl changelist.Changelist) ( + *NotaryRepository, error) { - cryptoService := cryptoservice.NewCryptoService(keyStores...) + // Repo's remote store is either a valid remote store or an OfflineStore + if remoteStore == nil { + remoteStore = store.OfflineStore{} + } nRepo := &NotaryRepository{ gun: gun, - baseDir: baseDir, baseURL: baseURL, - tufRepoPath: filepath.Join(baseDir, tufDir, filepath.FromSlash(gun.String())), + baseDir: baseDir, + changelist: cl, + cache: cache, + remoteStore: remoteStore, CryptoService: cryptoService, - roundTrip: rt, - trustPinning: trustPin, + trustPinning: trustPinning, LegacyVersions: 0, // By default, don't sign with legacy roles } - nRepo.cache = cache - return nRepo, nil } @@ -274,10 +285,13 @@ func (r *NotaryRepository) initializeRoles(rootKeys []data.PublicKey, localRoles ) } } + + remote := r.getRemoteStore() + for _, role := range remoteRoles { // This key is generated by the remote server. var key data.PublicKey - key, err = getRemoteKey(r.baseURL, r.gun, role, r.roundTrip) + key, err = getRemoteKey(role, remote) if err != nil { return } @@ -302,8 +316,7 @@ func (r *NotaryRepository) initializeRoles(rootKeys []data.PublicKey, localRoles } // adds a TUF Change template to the given roles -func addChange(cl *changelist.FileChangelist, c changelist.Change, roles ...data.RoleName) error { - +func addChange(cl changelist.Changelist, c changelist.Change, roles ...data.RoleName) error { if len(roles) == 0 { roles = []data.RoleName{data.CanonicalTargetsRole} } @@ -344,11 +357,6 @@ func (r *NotaryRepository) AddTarget(target *Target, roles ...data.RoleName) err if len(target.Hashes) == 0 { return fmt.Errorf("no hashes specified for target \"%s\"", target.Name) } - cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist")) - if err != nil { - return err - } - defer cl.Close() 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} @@ -360,22 +368,17 @@ func (r *NotaryRepository) AddTarget(target *Target, roles ...data.RoleName) err template := changelist.NewTUFChange( changelist.ActionCreate, "", changelist.TypeTargetsTarget, target.Name, metaJSON) - return addChange(cl, template, roles...) + return addChange(r.changelist, template, roles...) } // RemoveTarget creates new changelist entries to remove a target from the given // roles in the repository when the changelist gets applied at publish time. // If roles are unspecified, the default role is "target". func (r *NotaryRepository) RemoveTarget(targetName string, roles ...data.RoleName) error { - - cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist")) - if err != nil { - return err - } logrus.Debugf("Removing target \"%s\"", targetName) template := changelist.NewTUFChange(changelist.ActionDelete, "", changelist.TypeTargetsTarget, targetName, nil) - return addChange(cl, template, roles...) + return addChange(r.changelist, template, roles...) } // ListTargets lists all targets for the current repository. The list of @@ -533,13 +536,19 @@ func (r *NotaryRepository) GetAllTargetMetadataByName(name string) ([]TargetSign // GetChangelist returns the list of the repository's unpublished changes func (r *NotaryRepository) GetChangelist() (changelist.Changelist, error) { - changelistDir := filepath.Join(r.tufRepoPath, "changelist") - cl, err := changelist.NewFileChangelist(changelistDir) - if err != nil { - logrus.Debug("Error initializing changelist") - return nil, err + return r.changelist, nil +} + +// getRemoteStore returns the remoteStore of a repository if valid or +// or an OfflineStore otherwise +func (r *NotaryRepository) getRemoteStore() store.RemoteStore { + if r.remoteStore != nil { + return r.remoteStore } - return cl, nil + + r.remoteStore = &store.OfflineStore{} + + return r.remoteStore } // RoleWithSignatures is a Role with its associated signatures @@ -590,18 +599,14 @@ func (r *NotaryRepository) ListRoles() ([]RoleWithSignatures, 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() error { - cl, err := r.GetChangelist() - if err != nil { + if err := r.publish(r.changelist); err != nil { return err } - if err = r.publish(cl); err != nil { - return err - } - if err = cl.Clear(""); err != nil { + if err := r.changelist.Clear(""); 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 ", filepath.Join(r.tufRepoPath, "changelist")) + logrus.Warn("Unable to clear changelist. You may want to manually delete the folder ", r.changelist.Location()) } return nil } @@ -686,10 +691,7 @@ func (r *NotaryRepository) publish(cl changelist.Changelist) error { return err } - remote, err := getRemoteStore(r.baseURL, r.gun, r.roundTrip) - if err != nil { - return err - } + remote := r.getRemoteStore() return remote.SetMulti(data.MetadataRoleMapToStringMap(updatedFiles)) } @@ -970,10 +972,9 @@ func (r *NotaryRepository) bootstrapClient(checkInitialized bool) (*TUFClient, e } } - remote, remoteErr := getRemoteStore(r.baseURL, r.gun, r.roundTrip) - if remoteErr != nil { - logrus.Error(remoteErr) - } else if !newBuilder.IsLoaded(data.CanonicalRootRole) || checkInitialized { + remote := r.getRemoteStore() + + if !newBuilder.IsLoaded(data.CanonicalRootRole) || checkInitialized { // remoteErr was nil and we were not able to load a root from cache or // are specifically checking for initialization of the repo. @@ -1037,7 +1038,8 @@ func (r *NotaryRepository) pubKeyListForRotation(role data.RoleName, serverManag // If server manages the key being rotated, request a rotation and return the new key if serverManaged { - pubKey, err = rotateRemoteKey(r.baseURL, r.gun, role, r.roundTrip) + remote := r.getRemoteStore() + pubKey, err = rotateRemoteKey(role, remote) pubKeyList = make(data.KeyList, 0, 1) pubKeyList = append(pubKeyList, pubKey) if err != nil { @@ -1062,7 +1064,7 @@ func (r *NotaryRepository) pubKeyListForRotation(role data.RoleName, serverManag for _, keyID := range newKeys { pubKey = r.CryptoService.GetKey(keyID) if pubKey == nil { - return nil, fmt.Errorf("unable to find key: %s") + return nil, fmt.Errorf("unable to find key: %s", keyID) } pubKeyList = append(pubKeyList, pubKey) } @@ -1139,15 +1141,17 @@ func (r *NotaryRepository) rootFileKeyChange(cl changelist.Changelist, role data // DeleteTrustData removes the trust data stored for this repo in the TUF cache on the client side // Note that we will not delete any private key material from local storage -func (r *NotaryRepository) DeleteTrustData(deleteRemote bool) error { +func DeleteTrustData(baseDir string, gun data.GUN, URL string, rt http.RoundTripper, deleteRemote bool) error { + localRepo := filepath.Join(baseDir, tufDir, filepath.FromSlash(gun.String())) // Remove the tufRepoPath directory, which includes local TUF metadata files and changelist information - if err := os.RemoveAll(r.tufRepoPath); err != nil { + if err := os.RemoveAll(localRepo); err != nil { return fmt.Errorf("error clearing TUF repo data: %v", err) } // Note that this will require admin permission in this NotaryRepository's roundtripper if deleteRemote { - remote, err := getRemoteStore(r.baseURL, r.gun, r.roundTrip) + remote, err := getRemoteStore(URL, gun, rt) if err != nil { + logrus.Error("unable to instantiate a remote store: %v", err) return err } if err := remote.RemoveAll(); err != nil { diff --git a/client/client_test.go b/client/client_test.go index 081ec3b0d..557639070 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -837,13 +837,15 @@ func testAddTargetToSpecifiedInvalidRoles(t *testing.T, clearCache bool) { func testErrorWritingChangefiles(t *testing.T, writeChangeFile func(*NotaryRepository) error) { ts, _, _ := simpleTestServer(t) defer ts.Close() - - repo, _ := initializeRepo(t, data.ECDSAKey, "docker.com/notary", ts.URL, false) + gun := "docker.com/notary" + repo, _ := initializeRepo(t, data.ECDSAKey, gun, ts.URL, false) defer os.RemoveAll(repo.baseDir) // first, make the actual changefile unwritable by making the changelist // directory unwritable - changelistPath := filepath.Join(repo.tufRepoPath, "changelist") + changelistPath := filepath.Join( + filepath.Join(repo.baseDir, tufDir, filepath.FromSlash(gun)), "changelist", + ) err := os.MkdirAll(changelistPath, 0744) require.NoError(t, err, "could not create changelist dir") err = os.Chmod(changelistPath, 0600) @@ -2463,7 +2465,12 @@ func TestPublishRemoveDelegationKeyFromDelegationRole(t *testing.T) { }) require.NoError(t, err) - cl, err := changelist.NewFileChangelist(filepath.Join(ownerRepo.tufRepoPath, "changelist")) + cl, err := changelist.NewFileChangelist( + filepath.Join( + filepath.Join(ownerRepo.baseDir, tufDir, filepath.FromSlash(gun)), + "changelist", + ), + ) require.NoError(t, err) require.NoError(t, cl.Add(changelist.NewTUFChange( changelist.ActionUpdate, @@ -3404,7 +3411,9 @@ func TestFullAddDelegationChangefileApplicable(t *testing.T) { }) require.NoError(t, err) change := newCreateDelegationChange(delegationName, tdJSON) - cl, err := changelist.NewFileChangelist(filepath.Join(repo.tufRepoPath, "changelist")) + cl, err := changelist.NewFileChangelist( + filepath.Join(repo.baseDir, tufDir, filepath.FromSlash(gun), "changelist"), + ) require.NoError(t, err) addChange(cl, change, delegationName) @@ -3455,7 +3464,9 @@ func TestFullRemoveDelegationChangefileApplicable(t *testing.T) { }) require.NoError(t, err) change := newUpdateDelegationChange(delegationName, tdJSON) - cl, err := changelist.NewFileChangelist(filepath.Join(repo.tufRepoPath, "changelist")) + cl, err := changelist.NewFileChangelist( + filepath.Join(repo.baseDir, tufDir, filepath.FromSlash(gun), "changelist"), + ) require.NoError(t, err) addChange(cl, change, delegationName) @@ -3507,10 +3518,10 @@ func TestBootstrapClientBadURL(t *testing.T) { require.EqualError(t, err, err2.Error()) } -// TestBootstrapClientInvalidURL checks that bootstrapClient correctly -// returns an error when the URL is valid but does not point to +// TestClientInvalidURL checks that instantiating a new NotaryRepository +// correctly returns an error when the URL is valid but does not point to // a TUF server -func TestBootstrapClientInvalidURL(t *testing.T) { +func TestClientInvalidURL(t *testing.T) { tempBaseDir, err := ioutil.TempDir("", "notary-test-") require.NoError(t, err, "failed to create a temporary directory: %s", err) repo, err := NewFileCachedNotaryRepository( @@ -3521,20 +3532,11 @@ func TestBootstrapClientInvalidURL(t *testing.T) { passphraseRetriever, trustpinning.TrustPinConfig{}, ) - require.NoError(t, err, "error creating repo: %s", err) - - c, err := repo.bootstrapClient(false) - require.Nil(t, c) + // NewFileCachedNotaryRepository should fail and return an error + // since it initializes the cache but also the remote repository + // from the baseURL and the GUN + require.Nil(t, repo) require.Error(t, err) - - c, err2 := repo.bootstrapClient(true) - require.Nil(t, c) - require.Error(t, err2) - - // same error should be returned because we don't have local data - // and are requesting remote root regardless of checkInitialized - // value - require.EqualError(t, err, err2.Error()) } func TestPublishTargetsDelegationCanUseUserKeyWithArbitraryRole(t *testing.T) { @@ -3595,12 +3597,12 @@ func testPublishTargetsDelegationCanUseUserKeyWithArbitraryRole(t *testing.T, x5 // TestDeleteRepo tests that local repo data is deleted from the client library call func TestDeleteRepo(t *testing.T) { - gun := "docker.com/notary" + var gun data.GUN = "docker.com/notary" ts, _, _ := simpleTestServer(t) defer ts.Close() - repo, rootKeyID := initializeRepo(t, data.ECDSAKey, gun, ts.URL, false) + repo, rootKeyID := initializeRepo(t, data.ECDSAKey, gun.String(), ts.URL, false) defer os.RemoveAll(repo.baseDir) // Assert initialization was successful before we delete @@ -3618,7 +3620,7 @@ func TestDeleteRepo(t *testing.T) { require.Len(t, cl.List(), 1) // Delete all local trust data for repo - err = repo.DeleteTrustData(false) + err = DeleteTrustData(repo.baseDir, gun, "", nil, false) require.NoError(t, err) // Assert no metadata for this repo exists locally @@ -3631,7 +3633,7 @@ func TestDeleteRepo(t *testing.T) { require.Len(t, cl.List(), 0) // Check that the tuf/ directory itself is gone - _, err = os.Stat(repo.tufRepoPath) + _, err = os.Stat(filepath.Join(repo.baseDir, tufDir, filepath.FromSlash(gun.String()))) require.Error(t, err) // Assert keys for this repo exist locally @@ -3640,13 +3642,13 @@ func TestDeleteRepo(t *testing.T) { // TestDeleteRemoteRepo tests that local and remote repo data is deleted from the client library call func TestDeleteRemoteRepo(t *testing.T) { - gun := "docker.com/notary" + var gun data.GUN = "docker.com/notary" ts := fullTestServer(t) defer ts.Close() // Create and publish a repo to delete - repo, rootKeyID := initializeRepo(t, data.ECDSAKey, gun, ts.URL, false) + repo, rootKeyID := initializeRepo(t, data.ECDSAKey, gun.String(), ts.URL, false) defer os.RemoveAll(repo.baseDir) require.NoError(t, repo.Publish()) @@ -3682,7 +3684,7 @@ func TestDeleteRemoteRepo(t *testing.T) { require.Len(t, repoCl.List(), 1) // Delete all local and remote trust data for one repo - err = repo.DeleteTrustData(true) + err = DeleteTrustData(repo.baseDir, gun, ts.URL, http.DefaultTransport, true) require.NoError(t, err) // Assert no metadata for that repo exists locally @@ -3695,15 +3697,15 @@ func TestDeleteRemoteRepo(t *testing.T) { require.Len(t, repoCl.List(), 0) // Check that the tuf/ directory itself is gone - _, err = os.Stat(repo.tufRepoPath) + _, err = os.Stat(filepath.Join(repo.baseDir, tufDir, filepath.FromSlash(gun.String()))) require.Error(t, err) // Assert keys for this repo still exist locally requireRepoHasExpectedKeys(t, repo, rootKeyID, true) // Try connecting to the remote store directly and make sure that no metadata exists for this gun - remoteStore, err := getRemoteStore(repo.baseURL, repo.gun, repo.roundTrip) - require.NoError(t, err) + remoteStore := repo.getRemoteStore() + require.NotNil(t, remoteStore) meta, err := remoteStore.GetSized(data.CanonicalRootRole.String(), store.NoSizeLimit) require.Error(t, err) require.IsType(t, store.ErrMetaNotFound{}, err) @@ -3728,8 +3730,8 @@ func TestDeleteRemoteRepo(t *testing.T) { require.Len(t, longLivingCl.List(), 1) // Check that the other repo's remote data is unaffected - remoteStore, err = getRemoteStore(longLivingRepo.baseURL, longLivingRepo.gun, longLivingRepo.roundTrip) - require.NoError(t, err) + remoteStore = longLivingRepo.getRemoteStore() + require.NotNil(t, remoteStore) meta, err = remoteStore.GetSized(data.CanonicalRootRole.String(), store.NoSizeLimit) require.NoError(t, err) require.NotNil(t, meta) @@ -3742,10 +3744,6 @@ func TestDeleteRemoteRepo(t *testing.T) { meta, err = remoteStore.GetSized(data.CanonicalTimestampRole.String(), store.NoSizeLimit) require.NoError(t, err) require.NotNil(t, meta) - - // Try deleting again with an invalid server URL - repo.baseURL = "invalid" - require.Error(t, repo.DeleteTrustData(true)) } // Test that we get a correct list of roles with keys and signatures diff --git a/client/delegations.go b/client/delegations.go index 605af93e3..d32c558fc 100644 --- a/client/delegations.go +++ b/client/delegations.go @@ -3,7 +3,6 @@ package client import ( "encoding/json" "fmt" - "path/filepath" "github.com/Sirupsen/logrus" "github.com/docker/notary" @@ -40,12 +39,6 @@ func (r *NotaryRepository) AddDelegationRoleAndKeys(name data.RoleName, delegati return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"} } - cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist")) - if err != nil { - return err - } - defer cl.Close() - logrus.Debugf(`Adding delegation "%s" with threshold %d, and %d keys\n`, name, notary.MinThreshold, len(delegationKeys)) @@ -59,7 +52,7 @@ func (r *NotaryRepository) AddDelegationRoleAndKeys(name data.RoleName, delegati } template := newCreateDelegationChange(name, tdJSON) - return addChange(cl, template, name) + return addChange(r.changelist, template, name) } // AddDelegationPaths creates a changelist entry to add provided paths to an existing delegation. @@ -70,12 +63,6 @@ func (r *NotaryRepository) AddDelegationPaths(name data.RoleName, paths []string return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"} } - cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist")) - if err != nil { - return err - } - defer cl.Close() - logrus.Debugf(`Adding %s paths to delegation %s\n`, paths, name) tdJSON, err := json.Marshal(&changelist.TUFDelegation{ @@ -86,7 +73,7 @@ func (r *NotaryRepository) AddDelegationPaths(name data.RoleName, paths []string } template := newCreateDelegationChange(name, tdJSON) - return addChange(cl, template, name) + return addChange(r.changelist, template, name) } // RemoveDelegationKeysAndPaths creates changelist entries to remove provided delegation key IDs and paths. @@ -114,16 +101,10 @@ func (r *NotaryRepository) RemoveDelegationRole(name data.RoleName) error { return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"} } - cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist")) - if err != nil { - return err - } - defer cl.Close() - logrus.Debugf(`Removing delegation "%s"\n`, name) template := newDeleteDelegationChange(name, nil) - return addChange(cl, template, name) + return addChange(r.changelist, template, name) } // RemoveDelegationPaths creates a changelist entry to remove provided paths from an existing delegation. @@ -133,12 +114,6 @@ func (r *NotaryRepository) RemoveDelegationPaths(name data.RoleName, paths []str return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"} } - cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist")) - if err != nil { - return err - } - defer cl.Close() - logrus.Debugf(`Removing %s paths from delegation "%s"\n`, paths, name) tdJSON, err := json.Marshal(&changelist.TUFDelegation{ @@ -149,7 +124,7 @@ func (r *NotaryRepository) RemoveDelegationPaths(name data.RoleName, paths []str } template := newUpdateDelegationChange(name, tdJSON) - return addChange(cl, template, name) + return addChange(r.changelist, template, name) } // RemoveDelegationKeys creates a changelist entry to remove provided keys from an existing delegation. @@ -163,12 +138,6 @@ func (r *NotaryRepository) RemoveDelegationKeys(name data.RoleName, keyIDs []str return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"} } - cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist")) - if err != nil { - return err - } - defer cl.Close() - logrus.Debugf(`Removing %s keys from delegation "%s"\n`, keyIDs, name) tdJSON, err := json.Marshal(&changelist.TUFDelegation{ @@ -179,7 +148,7 @@ func (r *NotaryRepository) RemoveDelegationKeys(name data.RoleName, keyIDs []str } template := newUpdateDelegationChange(name, tdJSON) - return addChange(cl, template, name) + return addChange(r.changelist, template, name) } // ClearDelegationPaths creates a changelist entry to remove all paths from an existing delegation. @@ -189,12 +158,6 @@ func (r *NotaryRepository) ClearDelegationPaths(name data.RoleName) error { return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"} } - cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist")) - if err != nil { - return err - } - defer cl.Close() - logrus.Debugf(`Removing all paths from delegation "%s"\n`, name) tdJSON, err := json.Marshal(&changelist.TUFDelegation{ @@ -205,7 +168,7 @@ func (r *NotaryRepository) ClearDelegationPaths(name data.RoleName) error { } template := newUpdateDelegationChange(name, tdJSON) - return addChange(cl, template, name) + return addChange(r.changelist, template, name) } func newUpdateDelegationChange(name data.RoleName, content []byte) *changelist.TUFChange { diff --git a/client/helpers.go b/client/helpers.go index a473cd7f6..5ec0384e2 100644 --- a/client/helpers.go +++ b/client/helpers.go @@ -26,7 +26,7 @@ func getRemoteStore(baseURL string, gun data.GUN, rt http.RoundTripper) (store.R if err != nil { return store.OfflineStore{}, err } - return s, err + return s, nil } func applyChangelist(repo *tuf.Repo, invalid *tuf.Repo, cl changelist.Changelist) error { @@ -218,11 +218,7 @@ func warnRolesNearExpiry(r *tuf.Repo) { } // Fetches a public key from a remote store, given a gun and role -func getRemoteKey(url string, gun data.GUN, role data.RoleName, rt http.RoundTripper) (data.PublicKey, error) { - remote, err := getRemoteStore(url, gun, rt) - if err != nil { - return nil, err - } +func getRemoteKey(role data.RoleName, remote store.RemoteStore) (data.PublicKey, error) { rawPubKey, err := remote.GetKey(role) if err != nil { return nil, err @@ -237,11 +233,7 @@ func getRemoteKey(url string, gun data.GUN, role data.RoleName, rt http.RoundTri } // Rotates a private key in a remote store and returns the public key component -func rotateRemoteKey(url string, gun data.GUN, role data.RoleName, rt http.RoundTripper) (data.PublicKey, error) { - remote, err := getRemoteStore(url, gun, rt) - if err != nil { - return nil, err - } +func rotateRemoteKey(role data.RoleName, remote store.RemoteStore) (data.PublicKey, error) { rawPubKey, err := remote.RotateKey(role) if err != nil { return nil, err diff --git a/client/helpers_test.go b/client/helpers_test.go index c0e12151b..42d30d856 100644 --- a/client/helpers_test.go +++ b/client/helpers_test.go @@ -10,6 +10,7 @@ import ( log "github.com/Sirupsen/logrus" "github.com/docker/notary/client/changelist" + "github.com/docker/notary/storage" "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/testutils" "github.com/stretchr/testify/require" @@ -1023,13 +1024,20 @@ func TestAllNotNearExpiry(t *testing.T) { } func TestRotateRemoteKeyOffline(t *testing.T) { + // http store requires an absolute baseURL + _, err := getRemoteStore("invalidURL", "gun", nil) + require.Error(t, err) + // without a valid roundtripper, rotation should fail since we cannot initialize a HTTPStore - key, err := rotateRemoteKey("invalidURL", "gun", data.CanonicalSnapshotRole, nil) + var remote storage.RemoteStore = storage.OfflineStore{} + key, err := rotateRemoteKey(data.CanonicalSnapshotRole, remote) require.Error(t, err) require.Nil(t, key) // if the underlying remote store is faulty and cannot rotate keys, we should get back the error - key, err = rotateRemoteKey("https://notary-server", "gun", data.CanonicalSnapshotRole, http.DefaultTransport) + remote, err = getRemoteStore("https://notary-server", "gun", http.DefaultTransport) + require.NoError(t, err) + key, err = rotateRemoteKey(data.CanonicalSnapshotRole, remote) require.Error(t, err) require.Nil(t, key) } diff --git a/client/witness.go b/client/witness.go index 636d4f01f..72aed031c 100644 --- a/client/witness.go +++ b/client/witness.go @@ -1,8 +1,6 @@ package client import ( - "path/filepath" - "github.com/docker/notary/client/changelist" "github.com/docker/notary/tuf" "github.com/docker/notary/tuf/data" @@ -11,12 +9,7 @@ import ( // Witness creates change objects to witness (i.e. re-sign) the given // roles on the next publish. One change is created per role func (r *NotaryRepository) Witness(roles ...data.RoleName) ([]data.RoleName, error) { - cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist")) - if err != nil { - return nil, err - } - defer cl.Close() - + var err error successful := make([]data.RoleName, 0, len(roles)) for _, role := range roles { // scope is role @@ -27,7 +20,7 @@ func (r *NotaryRepository) Witness(roles ...data.RoleName) ([]data.RoleName, err "", nil, ) - err = cl.Add(c) + err = r.changelist.Add(c) if err != nil { break } diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index 54bb06cab..c80509a71 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -359,11 +359,6 @@ func (t *tufCommander) tufDeleteGUN(cmd *cobra.Command, args []string) error { gun := data.GUN(args[0]) - trustPin, err := getTrustPinning(config) - if err != nil { - return err - } - // Only initialize a roundtripper if we get the remote flag var rt http.RoundTripper var remoteDeleteInfo string @@ -375,16 +370,15 @@ func (t *tufCommander) tufDeleteGUN(cmd *cobra.Command, args []string) error { remoteDeleteInfo = " and remote" } - nRepo, err := notaryclient.NewFileCachedNotaryRepository( - config.GetString("trust_dir"), gun, getRemoteTrustServer(config), rt, t.retriever, trustPin) - - if err != nil { - return err - } - cmd.Printf("Deleting trust data for repository %s\n", gun) - if err := nRepo.DeleteTrustData(t.deleteRemote); err != nil { + if err := notaryclient.DeleteTrustData( + config.GetString("trust_dir"), + gun, + getRemoteTrustServer(config), + rt, + t.deleteRemote, + ); err != nil { return err } cmd.Printf("Successfully deleted local%s trust data for repository %s\n", remoteDeleteInfo, gun) diff --git a/cmd/notary/tuf_test.go b/cmd/notary/tuf_test.go index 6e32051c3..90c7c9a00 100644 --- a/cmd/notary/tuf_test.go +++ b/cmd/notary/tuf_test.go @@ -206,7 +206,6 @@ func TestGetTrustPinningErrors(t *testing.T) { require.Error(t, tc.tufStatus(&cobra.Command{}, []string{"gun"})) tc.resetAll = true require.Error(t, tc.tufReset(&cobra.Command{}, []string{"gun"})) - require.Error(t, tc.tufDeleteGUN(&cobra.Command{}, []string{"gun"})) require.Error(t, tc.tufInit(&cobra.Command{}, []string{"gun"})) require.Error(t, tc.tufPublish(&cobra.Command{}, []string{"gun"})) require.Error(t, tc.tufVerify(&cobra.Command{}, []string{"gun", "target", "file"}))