diff --git a/client/client.go b/client/client.go index 817161f45..44b2c957b 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" @@ -44,6 +43,7 @@ type NotaryRepository struct { baseURL string tufRepoPath string 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, gun, baseURL string, rt http.RoundTripper, retriever notary.PassRetriever, trustPinning trustpinning.TrustPinConfig) ( *NotaryRepository, error) { @@ -65,47 +67,44 @@ func NewFileCachedNotaryRepository(baseDir, gun, baseURL string, rt http.RoundTr 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, 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...) -// 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, gun, baseURL string, rt http.RoundTripper, cache store.MetadataStore, - keyStores []trustmanager.KeyStore, trustPin trustpinning.TrustPinConfig) (*NotaryRepository, error) { + remoteStore, err := getRemoteStore(baseURL, gun, rt) + if err != nil { + return nil, err + } - cryptoService := cryptoservice.NewCryptoService(keyStores...) + return NewNotaryRepository(baseDir, gun, baseURL, remoteStore, rt, cache, trustPinning, cryptoService) +} + +// 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 initiated remote store and cache. +func NewNotaryRepository(baseDir, gun, baseURL string, remoteStore store.RemoteStore, rt http.RoundTripper, cache store.MetadataStore, + trustPinning trustpinning.TrustPinConfig, cryptoService signed.CryptoService) ( + *NotaryRepository, error) { nRepo := &NotaryRepository{ gun: gun, baseDir: baseDir, baseURL: baseURL, tufRepoPath: filepath.Join(baseDir, tufDir, filepath.FromSlash(gun)), + 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 } @@ -686,10 +685,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.remoteStore return remote.SetMulti(updatedFiles) } @@ -970,10 +966,8 @@ 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.remoteStore + 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. @@ -1146,13 +1140,25 @@ func (r *NotaryRepository) DeleteTrustData(deleteRemote bool) error { } // Note that this will require admin permission in this NotaryRepository's roundtripper if deleteRemote { - remote, err := getRemoteStore(r.baseURL, r.gun, r.roundTrip) - if err != nil { - return err - } + remote := r.remoteStore if err := remote.RemoveAll(); err != nil { return err } } return nil } + +// SetBaseURL updates the repository's base URL and its associated remote store +func (r *NotaryRepository) setBaseURL(baseURL string) error { + // We try to retrieve the remote store before updating the repository + remoteStore, err := getRemoteStore(baseURL, r.gun, r.roundTrip) + if err != nil { + return err + } + + // Update the repository + r.baseURL = baseURL + r.remoteStore = remoteStore + + return nil +} diff --git a/client/client_test.go b/client/client_test.go index 9a31bf3c6..e16ead436 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -3509,10 +3509,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( @@ -3523,20 +3523,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) { @@ -3705,8 +3696,7 @@ func TestDeleteRemoteRepo(t *testing.T) { 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.remoteStore meta, err := remoteStore.GetSized(data.CanonicalRootRole, store.NoSizeLimit) require.Error(t, err) require.IsType(t, store.ErrMetaNotFound{}, err) @@ -3731,8 +3721,7 @@ 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.remoteStore meta, err = remoteStore.GetSized(data.CanonicalRootRole, store.NoSizeLimit) require.NoError(t, err) require.NotNil(t, meta) @@ -3746,9 +3735,11 @@ func TestDeleteRemoteRepo(t *testing.T) { 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)) + // Try setting an invalid server URL before deleting + require.Error(t, repo.setBaseURL("invalid")) + + // Try deleting again the remote + require.NoError(t, repo.DeleteTrustData(true)) } // Test that we get a correct list of roles with keys and signatures