Skip to content

Commit

Permalink
Remove Notary from Repository constructor functions
Browse files Browse the repository at this point in the history
Also fixes a small bug with SetLegacyVersions.

Signed-off-by: Tibor Vass <teabee89@gmail.com>
  • Loading branch information
tiborvass committed Sep 7, 2017
1 parent e69f125 commit c848281
Show file tree
Hide file tree
Showing 18 changed files with 145 additions and 127 deletions.
18 changes: 11 additions & 7 deletions client/backwards_compatibility_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (

// Once a fixture is read in, ensure that it's valid by making sure the expiry
// times of all the metadata and certificates is > 10 years ahead
func requireValidFixture(t *testing.T, notaryRepo *NotaryRepository) {
func requireValidFixture(t *testing.T, notaryRepo *repository) {
tenYearsInFuture := time.Now().AddDate(10, 0, 0)
require.True(t, notaryRepo.tufRepo.Root.Signed.Expires.After(tenYearsInFuture))
require.True(t, notaryRepo.tufRepo.Snapshot.Signed.Expires.After(tenYearsInFuture))
Expand Down Expand Up @@ -90,7 +90,7 @@ func Test0Dot1Migration(t *testing.T) {
ts := fullTestServer(t)
defer ts.Close()

_, err = NewFileCachedNotaryRepository(tmpDir, gun, ts.URL, http.DefaultTransport,
_, err = NewFileCachedRepository(tmpDir, gun, ts.URL, http.DefaultTransport,
passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{})
require.NoError(t, err, "error creating repo: %s", err)

Expand Down Expand Up @@ -138,7 +138,7 @@ func Test0Dot3Migration(t *testing.T) {
ts := fullTestServer(t)
defer ts.Close()

_, err = NewFileCachedNotaryRepository(tmpDir, gun, ts.URL, http.DefaultTransport,
_, err = NewFileCachedRepository(tmpDir, gun, ts.URL, http.DefaultTransport,
passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{})
require.NoError(t, err, "error creating repo: %s", err)

Expand Down Expand Up @@ -197,9 +197,10 @@ func Test0Dot1RepoFormat(t *testing.T) {
ts := fullTestServer(t)
defer ts.Close()

repo, err := NewFileCachedNotaryRepository(tmpDir, gun, ts.URL, http.DefaultTransport,
r, err := NewFileCachedRepository(tmpDir, gun, ts.URL, http.DefaultTransport,
passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{})
require.NoError(t, err, "error creating repo: %s", err)
repo := r.(*repository)

// targets should have 1 target, and it should be readable offline
targets, err := repo.ListTargets()
Expand Down Expand Up @@ -260,9 +261,10 @@ func Test0Dot3RepoFormat(t *testing.T) {
ts := fullTestServer(t)
defer ts.Close()

repo, err := NewFileCachedNotaryRepository(tmpDir, gun, ts.URL, http.DefaultTransport,
r, err := NewFileCachedRepository(tmpDir, gun, ts.URL, http.DefaultTransport,
passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{})
require.NoError(t, err, "error creating repo: %s", err)
repo := r.(*repository)

// targets should have 1 target, and it should be readable offline
targets, err := repo.ListTargets()
Expand Down Expand Up @@ -326,9 +328,10 @@ func TestDownloading0Dot1RepoFormat(t *testing.T) {
require.NoError(t, err)
defer os.RemoveAll(repoDir)

repo, err := NewFileCachedNotaryRepository(repoDir, gun, ts.URL, http.DefaultTransport,
r, err := NewFileCachedRepository(repoDir, gun, ts.URL, http.DefaultTransport,
passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{})
require.NoError(t, err, "error creating repo: %s", err)
repo := r.(*repository)

err = repo.Update(true)
require.NoError(t, err, "error updating repo: %s", err)
Expand All @@ -351,9 +354,10 @@ func TestDownloading0Dot3RepoFormat(t *testing.T) {
require.NoError(t, err)
defer os.RemoveAll(repoDir)

repo, err := NewFileCachedNotaryRepository(repoDir, gun, ts.URL, http.DefaultTransport,
r, err := NewFileCachedRepository(repoDir, gun, ts.URL, http.DefaultTransport,
passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{})
require.NoError(t, err, "error creating repo: %s", err)
repo := r.(*repository)

err = repo.Update(true)
require.NoError(t, err, "error updating repo: %s", err)
Expand Down
2 changes: 1 addition & 1 deletion client/changelist/file_changelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ import (
"fmt"
"io/ioutil"
"os"
"path/filepath"
"sort"
"time"

"github.com/Sirupsen/logrus"
"github.com/docker/distribution/uuid"
"path/filepath"
)

// FileChangelist stores all the changes as files
Expand Down
87 changes: 42 additions & 45 deletions client/client.go

Large diffs are not rendered by default.

90 changes: 50 additions & 40 deletions client/client_test.go

Large diffs are not rendered by default.

15 changes: 8 additions & 7 deletions client/client_update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ import (
"github.com/stretchr/testify/require"
)

func newBlankRepo(t *testing.T, url string) *NotaryRepository {
func newBlankRepo(t *testing.T, url string) *repository {
// Temporary directory where test files will be created
tempBaseDir, err := ioutil.TempDir("", "notary-test-")
require.NoError(t, err, "failed to create a temporary directory: %s", err)

repo, err := NewFileCachedNotaryRepository(tempBaseDir, "docker.com/notary", url,
r, err := NewFileCachedRepository(tempBaseDir, "docker.com/notary", url,
http.DefaultTransport, passphrase.ConstantRetriever("pass"), trustpinning.TrustPinConfig{})
require.NoError(t, err)
return repo
return r.(*repository)
}

var metadataDelegations = []data.RoleName{"targets/a", "targets/a/b", "targets/b", "targets/a/b/c", "targets/b/c"}
Expand Down Expand Up @@ -207,9 +207,10 @@ func TestUpdateInOfflineMode(t *testing.T) {
require.NoError(t, err, "failed to create a temporary directory: %s", err)
defer os.RemoveAll(tempBaseDir)

offlineRepo, err := NewFileCachedNotaryRepository(tempBaseDir, "docker.com/notary", "https://nope",
or, err := NewFileCachedRepository(tempBaseDir, "docker.com/notary", "https://nope",
nil, passphrase.ConstantRetriever("pass"), trustpinning.TrustPinConfig{})
require.NoError(t, err)
offlineRepo := or.(*repository)
err = offlineRepo.Update(false)
require.Error(t, err)
require.IsType(t, store.ErrOffline{}, err)
Expand Down Expand Up @@ -403,7 +404,7 @@ type updateOpts struct {
forWrite bool // whether the update is for writing or not (force check remote root.json)
role data.RoleName // the role to mess up on the server

checkRepo func(*NotaryRepository, *testutils.MetadataSwizzler) // a callback that can examine the repo at the end
checkRepo func(*repository, *testutils.MetadataSwizzler) // a callback that can examine the repo at the end
}

// If there's no local cache, we go immediately to check the remote server for
Expand Down Expand Up @@ -1171,8 +1172,8 @@ func TestUpdateNonRootRemoteCorruptedCanUseLocalCache(t *testing.T) {

// requires that a delegation role and its descendants were not accepted as a valid part of the
// TUF repo, but everything else was
func checkBadDelegationRoleSkipped(t *testing.T, delgRoleName string) func(*NotaryRepository, *testutils.MetadataSwizzler) {
return func(repo *NotaryRepository, s *testutils.MetadataSwizzler) {
func checkBadDelegationRoleSkipped(t *testing.T, delgRoleName string) func(*repository, *testutils.MetadataSwizzler) {
return func(repo *repository, s *testutils.MetadataSwizzler) {
for _, roleName := range s.Roles {
if roleName != data.CanonicalTargetsRole && !data.IsDelegation(roleName) {
continue
Expand Down
18 changes: 9 additions & 9 deletions client/delegations.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (

// AddDelegation creates changelist entries to add provided delegation public keys and paths.
// This method composes AddDelegationRoleAndKeys and AddDelegationPaths (each creates one changelist if called).
func (r *NotaryRepository) AddDelegation(name data.RoleName, delegationKeys []data.PublicKey, paths []string) error {
func (r *repository) AddDelegation(name data.RoleName, delegationKeys []data.PublicKey, paths []string) error {
if len(delegationKeys) > 0 {
err := r.AddDelegationRoleAndKeys(name, delegationKeys)
if err != nil {
Expand All @@ -33,7 +33,7 @@ func (r *NotaryRepository) AddDelegation(name data.RoleName, delegationKeys []da
// AddDelegationRoleAndKeys creates a changelist entry to add provided delegation public keys.
// This method is the simplest way to create a new delegation, because the delegation must have at least
// one key upon creation to be valid since we will reject the changelist while validating the threshold.
func (r *NotaryRepository) AddDelegationRoleAndKeys(name data.RoleName, delegationKeys []data.PublicKey) error {
func (r *repository) AddDelegationRoleAndKeys(name data.RoleName, delegationKeys []data.PublicKey) error {

if !data.IsDelegation(name) {
return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"}
Expand All @@ -57,7 +57,7 @@ func (r *NotaryRepository) AddDelegationRoleAndKeys(name data.RoleName, delegati

// AddDelegationPaths creates a changelist entry to add provided paths to an existing delegation.
// This method cannot create a new delegation itself because the role must meet the key threshold upon creation.
func (r *NotaryRepository) AddDelegationPaths(name data.RoleName, paths []string) error {
func (r *repository) AddDelegationPaths(name data.RoleName, paths []string) error {

if !data.IsDelegation(name) {
return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"}
Expand All @@ -78,7 +78,7 @@ func (r *NotaryRepository) AddDelegationPaths(name data.RoleName, paths []string

// RemoveDelegationKeysAndPaths creates changelist entries to remove provided delegation key IDs and paths.
// This method composes RemoveDelegationPaths and RemoveDelegationKeys (each creates one changelist if called).
func (r *NotaryRepository) RemoveDelegationKeysAndPaths(name data.RoleName, keyIDs, paths []string) error {
func (r *repository) RemoveDelegationKeysAndPaths(name data.RoleName, keyIDs, paths []string) error {
if len(paths) > 0 {
err := r.RemoveDelegationPaths(name, paths)
if err != nil {
Expand All @@ -95,7 +95,7 @@ func (r *NotaryRepository) RemoveDelegationKeysAndPaths(name data.RoleName, keyI
}

// RemoveDelegationRole creates a changelist to remove all paths and keys from a role, and delete the role in its entirety.
func (r *NotaryRepository) RemoveDelegationRole(name data.RoleName) error {
func (r *repository) RemoveDelegationRole(name data.RoleName) error {

if !data.IsDelegation(name) {
return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"}
Expand All @@ -108,7 +108,7 @@ func (r *NotaryRepository) RemoveDelegationRole(name data.RoleName) error {
}

// RemoveDelegationPaths creates a changelist entry to remove provided paths from an existing delegation.
func (r *NotaryRepository) RemoveDelegationPaths(name data.RoleName, paths []string) error {
func (r *repository) RemoveDelegationPaths(name data.RoleName, paths []string) error {

if !data.IsDelegation(name) {
return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"}
Expand All @@ -132,7 +132,7 @@ func (r *NotaryRepository) RemoveDelegationPaths(name data.RoleName, paths []str
// the role itself will be deleted in its entirety.
// It can also delete a key from all delegations under a parent using a name
// with a wildcard at the end.
func (r *NotaryRepository) RemoveDelegationKeys(name data.RoleName, keyIDs []string) error {
func (r *repository) RemoveDelegationKeys(name data.RoleName, keyIDs []string) error {

if !data.IsDelegation(name) && !data.IsWildDelegation(name) {
return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"}
Expand All @@ -152,7 +152,7 @@ func (r *NotaryRepository) RemoveDelegationKeys(name data.RoleName, keyIDs []str
}

// ClearDelegationPaths creates a changelist entry to remove all paths from an existing delegation.
func (r *NotaryRepository) ClearDelegationPaths(name data.RoleName) error {
func (r *repository) ClearDelegationPaths(name data.RoleName) error {

if !data.IsDelegation(name) {
return data.ErrInvalidRole{Role: name, Reason: "invalid delegation role name"}
Expand Down Expand Up @@ -203,7 +203,7 @@ func newDeleteDelegationChange(name data.RoleName, content []byte) *changelist.T

// GetDelegationRoles returns the keys and roles of the repository's delegations
// Also converts key IDs to canonical key IDs to keep consistent with signing prompts
func (r *NotaryRepository) GetDelegationRoles() ([]data.Role, error) {
func (r *repository) GetDelegationRoles() ([]data.Role, error) {
// Update state of the repo to latest
if err := r.Update(false); err != nil {
return nil, err
Expand Down
1 change: 1 addition & 0 deletions client/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package client

import (
"fmt"

"github.com/docker/notary/tuf/data"
)

Expand Down
2 changes: 1 addition & 1 deletion client/witness.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,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) {
func (r *repository) Witness(roles ...data.RoleName) ([]data.RoleName, error) {
var err error
successful := make([]data.RoleName, 0, len(roles))
for _, role := range roles {
Expand Down
8 changes: 4 additions & 4 deletions cmd/notary/delegations.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (d *delegationCommander) delegationPurgeKeys(cmd *cobra.Command, args []str
return err
}

nRepo, err := notaryclient.NewFileCachedNotaryRepository(
nRepo, err := notaryclient.NewFileCachedRepository(
config.GetString("trust_dir"),
gun,
getRemoteTrustServer(config),
Expand Down Expand Up @@ -153,7 +153,7 @@ func (d *delegationCommander) delegationsList(cmd *cobra.Command, args []string)
}

// initialize repo with transport to get latest state of the world before listing delegations
nRepo, err := notaryclient.NewFileCachedNotaryRepository(
nRepo, err := notaryclient.NewFileCachedRepository(
config.GetString("trust_dir"), gun, getRemoteTrustServer(config), rt, d.retriever, trustPin)
if err != nil {
return err
Expand Down Expand Up @@ -184,7 +184,7 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string

// no online operations are performed by add so the transport argument
// should be nil
nRepo, err := notaryclient.NewFileCachedNotaryRepository(
nRepo, err := notaryclient.NewFileCachedRepository(
config.GetString("trust_dir"), gun, getRemoteTrustServer(config), nil, d.retriever, trustPin)
if err != nil {
return err
Expand Down Expand Up @@ -314,7 +314,7 @@ func (d *delegationCommander) delegationAdd(cmd *cobra.Command, args []string) e

// no online operations are performed by add so the transport argument
// should be nil
nRepo, err := notaryclient.NewFileCachedNotaryRepository(
nRepo, err := notaryclient.NewFileCachedRepository(
config.GetString("trust_dir"), gun, getRemoteTrustServer(config), nil, d.retriever, trustPin)
if err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions cmd/notary/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ func (k *keyCommander) keysRotate(cmd *cobra.Command, args []string) error {
return err
}

nRepo, err := notaryclient.NewFileCachedNotaryRepository(
nRepo, err := notaryclient.NewFileCachedRepository(
config.GetString("trust_dir"), gun, getRemoteTrustServer(config),
rt, k.getRetriever(), trustPin)
if err != nil {
Expand Down Expand Up @@ -341,7 +341,7 @@ func (k *keyCommander) keysRotate(cmd *cobra.Command, args []string) error {
return nil
}
}
nRepo.LegacyVersions = k.legacyVersions
nRepo.SetLegacyVersions(k.legacyVersions)
if err := nRepo.RotateKey(rotateKeyRole, k.rotateKeyServerManaged, keyList); err != nil {
return err
}
Expand Down
8 changes: 4 additions & 4 deletions cmd/notary/keys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ func setUpRepo(t *testing.T, tempBaseDir string, gun data.GUN, ret notary.PassRe
cryptoService := cryptoservice.NewCryptoService(trustmanager.NewKeyMemoryStore(ret))
ts := httptest.NewServer(server.RootHandler(ctx, nil, cryptoService, nil, nil, nil))

repo, err := client.NewFileCachedNotaryRepository(
repo, err := client.NewFileCachedRepository(
tempBaseDir, gun, ts.URL, http.DefaultTransport, ret, trustpinning.TrustPinConfig{})
require.NoError(t, err, "error creating repo: %s", err)

Expand Down Expand Up @@ -376,7 +376,7 @@ func TestRotateKeyRemoteServerManagesKey(t *testing.T) {
}
require.NoError(t, k.keysRotate(&cobra.Command{}, []string{gun.String(), role, "-r"}))

repo, err := client.NewFileCachedNotaryRepository(tempBaseDir, data.GUN(gun), ts.URL, http.DefaultTransport, ret, trustpinning.TrustPinConfig{})
repo, err := client.NewFileCachedRepository(tempBaseDir, data.GUN(gun), ts.URL, http.DefaultTransport, ret, trustpinning.TrustPinConfig{})
require.NoError(t, err, "error creating repo: %s", err)

cl, err := repo.GetChangelist()
Expand Down Expand Up @@ -430,7 +430,7 @@ func TestRotateKeyBothKeys(t *testing.T) {
require.NoError(t, k.keysRotate(&cobra.Command{}, []string{gun.String(), data.CanonicalTargetsRole.String()}))
require.NoError(t, k.keysRotate(&cobra.Command{}, []string{gun.String(), data.CanonicalSnapshotRole.String()}))

repo, err := client.NewFileCachedNotaryRepository(tempBaseDir, data.GUN(gun), ts.URL, nil, ret, trustpinning.TrustPinConfig{})
repo, err := client.NewFileCachedRepository(tempBaseDir, data.GUN(gun), ts.URL, nil, ret, trustpinning.TrustPinConfig{})
require.NoError(t, err, "error creating repo: %s", err)

cl, err := repo.GetChangelist()
Expand Down Expand Up @@ -495,7 +495,7 @@ func TestRotateKeyRootIsInteractive(t *testing.T) {

require.Contains(t, out.String(), "Aborting action")

repo, err := client.NewFileCachedNotaryRepository(tempBaseDir, gun, ts.URL, nil, ret, trustpinning.TrustPinConfig{})
repo, err := client.NewFileCachedRepository(tempBaseDir, gun, ts.URL, nil, ret, trustpinning.TrustPinConfig{})
require.NoError(t, err, "error creating repo: %s", err)

// There should still just be one root key (and one targets and one snapshot)
Expand Down
2 changes: 1 addition & 1 deletion cmd/notary/repo_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func ConfigureRepo(v *viper.Viper, retriever notary.PassRetriever, onlineOperati
return nil, err
}
}
return client.NewFileCachedNotaryRepository(
return client.NewFileCachedRepository(
v.GetString("trust_dir"),
gun,
getRemoteTrustServer(v),
Expand Down
2 changes: 1 addition & 1 deletion cmd/notary/tuf.go
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,7 @@ func maybeAutoPublish(cmd *cobra.Command, doPublish bool, gun data.GUN, config *
return err
}

nRepo, err := notaryclient.NewFileCachedNotaryRepository(
nRepo, err := notaryclient.NewFileCachedRepository(
config.GetString("trust_dir"), gun, getRemoteTrustServer(config), rt, passRetriever, trustPin)
if err != nil {
return err
Expand Down
7 changes: 4 additions & 3 deletions trustmanager/remoteks/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ import (

"crypto/tls"
"crypto/x509"
"github.com/docker/notary/storage"
"github.com/docker/notary/trustmanager"
"google.golang.org/grpc/credentials"
"io/ioutil"
"path/filepath"
"runtime"

"github.com/docker/notary/storage"
"github.com/docker/notary/trustmanager"
"google.golang.org/grpc/credentials"
)

type TestError struct{}
Expand Down
3 changes: 2 additions & 1 deletion trustmanager/remoteks/server_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package remoteks

import (
"testing"

"github.com/stretchr/testify/require"
"golang.org/x/net/context"
"testing"

"github.com/docker/notary/storage"
)
Expand Down
1 change: 1 addition & 0 deletions trustmanager/yubikey/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package yubikey
import (
"encoding/pem"
"errors"

"github.com/docker/notary"
"github.com/docker/notary/trustmanager"
"github.com/docker/notary/tuf/data"
Expand Down
3 changes: 2 additions & 1 deletion trustpinning/trustpin_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package trustpinning

import (
"github.com/stretchr/testify/require"
"testing"

"github.com/stretchr/testify/require"
)

func TestWildcardMatch(t *testing.T) {
Expand Down
Loading

0 comments on commit c848281

Please sign in to comment.