From 9252d9d892d8abc8519f6e8296b564f246bb45cd Mon Sep 17 00:00:00 2001 From: Ying Li Date: Wed, 23 Dec 2015 15:08:27 -0800 Subject: [PATCH 1/4] Update client.Target to include a RoleName, so we know where the target is when listed. Signed-off-by: Ying Li --- client/client.go | 18 ++++++++-------- client/client_test.go | 48 +++++++++++++++++++++++++++++-------------- tuf/client/client.go | 6 +++--- 3 files changed, 45 insertions(+), 27 deletions(-) diff --git a/client/client.go b/client/client.go index 9c830ef2b8..48991bfd4c 100644 --- a/client/client.go +++ b/client/client.go @@ -127,9 +127,10 @@ func repositoryFromKeystores(baseDir, gun, baseURL string, rt http.RoundTripper, // Target represents a simplified version of the data TUF operates on, so external // applications don't have to depend on tuf data types. type Target struct { - Name string - Hashes data.Hashes - Length int64 + Name string // the name of the target + Hashes data.Hashes // the hash of the target + Length int64 // the size in bytes of the target + Role string // the role where the target was found - used for reading only } // NewTarget is a helper method that returns a Target @@ -368,6 +369,8 @@ func (r *NotaryRepository) RemoveDelegation(name string) error { // AddTarget creates new changelist entries to add a target to the given roles // in the repository when the changelist gets appied at publish time. // If roles are unspecified, the default role is "targets". +// AddTarget ignores the role in the `target` object, since passing a list of +// roles means the target can be added to more than one role at a time. func (r *NotaryRepository) AddTarget(target *Target, roles ...string) error { cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist")) @@ -460,7 +463,7 @@ func (r *NotaryRepository) listSubtree(targets map[string]*Target, role string, } for name, meta := range tgts.Signed.Targets { if _, ok := targets[name]; !ok { - targets[name] = &Target{Name: name, Hashes: meta.Hashes, Length: meta.Length} + targets[name] = &Target{Name: name, Hashes: meta.Hashes, Length: meta.Length, Role: role} } } for _, d := range tgts.Signed.Delegations.Roles { @@ -495,13 +498,10 @@ func (r *NotaryRepository) GetTargetByName(name string, roles ...string) (*Targe if len(roles) == 0 { roles = append(roles, data.CanonicalTargetsRole) } - var ( - meta *data.FileMeta - ) for _, role := range roles { - meta = c.TargetMeta(role, name, roles...) + meta, foundRole := c.TargetMeta(role, name, roles...) if meta != nil { - return &Target{Name: name, Hashes: meta.Hashes, Length: meta.Length}, nil + return &Target{Name: name, Hashes: meta.Hashes, Length: meta.Length, Role: foundRole}, nil } } return nil, fmt.Errorf("No trust data for %s", name) diff --git a/client/client_test.go b/client/client_test.go index 06773d32fa..27afeb1c48 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -761,14 +761,14 @@ func TestRemoveTargetErrorWritingChanges(t *testing.T) { // of listed targets. // We test this with both an RSA and ECDSA root key func TestListTarget(t *testing.T) { - testListEmptyTargets(t, data.ECDSAKey) - testListTarget(t, data.ECDSAKey) + // testListEmptyTargets(t, data.ECDSAKey) + // testListTarget(t, data.ECDSAKey) testListTargetWithDelegates(t, data.ECDSAKey) - if !testing.Short() { - testListEmptyTargets(t, data.RSAKey) - testListTarget(t, data.RSAKey) - testListTargetWithDelegates(t, data.RSAKey) - } + // if !testing.Short() { + // testListEmptyTargets(t, data.RSAKey) + // testListTarget(t, data.RSAKey) + // testListTargetWithDelegates(t, data.RSAKey) + // } } func testListEmptyTargets(t *testing.T, rootType string) { @@ -910,6 +910,10 @@ func testListTarget(t *testing.T, rootType string) { sort.Stable(targetSorter(targets)) + // the targets should both be find in the targets role + latestTarget.Role = data.CanonicalTargetsRole + currentTarget.Role = data.CanonicalTargetsRole + // current should be first assert.Equal(t, currentTarget, targets[0], "current target does not match") assert.Equal(t, latestTarget, targets[1], "latest target does not match") @@ -987,6 +991,12 @@ func testListTargetWithDelegates(t *testing.T, rootType string) { sort.Stable(targetSorter(targets)) + // specify where the targets were found: + latestTarget.Role = data.CanonicalTargetsRole + currentTarget.Role = data.CanonicalTargetsRole + level2Target.Role = "targets/level2" + otherTarget.Role = "targets/level1" + // current should be first. assert.Equal(t, currentTarget, targets[0], "current target does not match") assert.Equal(t, latestTarget, targets[1], "latest target does not match") @@ -1002,6 +1012,12 @@ func testListTargetWithDelegates(t *testing.T, rootType string) { sort.Stable(targetSorter(targets)) + // specify where the targets were found: + latestTarget.Role = data.CanonicalTargetsRole + delegatedTarget.Role = "targets/level1" + level2Target.Role = "targets/level2" + otherTarget.Role = "targets/level1" + // current should be first assert.Equal(t, delegatedTarget, targets[0], "current target does not match") assert.Equal(t, latestTarget, targets[1], "latest target does not match") @@ -1270,19 +1286,19 @@ func assertPublishToRolesSucceeds(t *testing.T, repo1 *NotaryRepository, sort.Stable(targetSorter(targets)) + currentTarget.Role = role + latestTarget.Role = role assert.Equal(t, currentTarget, targets[0], "current target does not match") assert.Equal(t, latestTarget, targets[1], "latest target does not match") // Also test GetTargetByName - if role == data.CanonicalTargetsRole { - newLatestTarget, err := repo.GetTargetByName("latest") - assert.NoError(t, err) - assert.Equal(t, latestTarget, newLatestTarget, "latest target does not match") + newLatestTarget, err := repo.GetTargetByName("latest", role) + assert.NoError(t, err) + assert.Equal(t, latestTarget, newLatestTarget, "latest target does not match") - newCurrentTarget, err := repo.GetTargetByName("current") - assert.NoError(t, err) - assert.Equal(t, currentTarget, newCurrentTarget, "current target does not match") - } + newCurrentTarget, err := repo.GetTargetByName("current", role) + assert.NoError(t, err) + assert.Equal(t, currentTarget, newCurrentTarget, "current target does not match") } } } @@ -1320,6 +1336,8 @@ func testPublishAfterPullServerHasSnapshotKey(t *testing.T, rootType string) { // list, so that the snapshot metadata is pulled from server targets, err := repo.ListTargets(data.CanonicalTargetsRole) assert.NoError(t, err) + // specify where target was found: + published.Role = data.CanonicalTargetsRole assert.Equal(t, []*Target{published}, targets) // listing downloaded the timestamp and snapshot metadata info assertRepoHasExpectedMetadata(t, repo, data.CanonicalTimestampRole, true) diff --git a/tuf/client/client.go b/tuf/client/client.go index 98b630d2d6..834f5c8def 100644 --- a/tuf/client/client.go +++ b/tuf/client/client.go @@ -525,7 +525,7 @@ func (c Client) RoleTargetsPath(role string, hashSha256 string, consistent bool) // TargetMeta ensures the repo is up to date. It assumes downloadTargets // has already downloaded all delegated roles -func (c Client) TargetMeta(role, path string, excludeRoles ...string) *data.FileMeta { +func (c Client) TargetMeta(role, path string, excludeRoles ...string) (*data.FileMeta, string) { excl := make(map[string]bool) for _, r := range excludeRoles { excl[r] = true @@ -548,7 +548,7 @@ func (c Client) TargetMeta(role, path string, excludeRoles ...string) *data.File meta = c.local.TargetMeta(curr, path) if meta != nil { // we found the target! - return meta + return meta, curr } delegations := c.local.TargetDelegations(role, path, pathHex) for _, d := range delegations { @@ -557,7 +557,7 @@ func (c Client) TargetMeta(role, path string, excludeRoles ...string) *data.File } } } - return meta + return meta, "" } // DownloadTarget downloads the target to dst from the remote From ecd96c82188f47c057836e29ee308af7d0627291 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Mon, 4 Jan 2016 10:32:00 -0800 Subject: [PATCH 2/4] Fix potential infinite loop in tuf/Client.TargetMeta Signed-off-by: Ying Li --- client/client_test.go | 46 ++++++++++++++++++++++++++------ tuf/client/client.go | 2 +- tuf/client/client_test.go | 56 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 9 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 27afeb1c48..1f29d0cce8 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -518,6 +518,35 @@ func TestAddTargetToTargetRoleByDefault(t *testing.T) { []string{data.CanonicalTargetsRole}) } +// Adding a target ignores the role of the target object, and goes by the +// roles passed to AddTarget (or the default targets role, if no roles are passed) +func TestAddTargetIgnoresTargetRole(t *testing.T) { + ts, _, _ := simpleTestServer(t) + defer ts.Close() + + repo, _ := initializeRepo(t, data.ECDSAKey, "docker.com/notary", ts.URL, false) + defer os.RemoveAll(repo.baseDir) + + targetObj, err := NewTarget("latest", "../fixtures/root-ca.crt") + assert.NoError(t, err, "error creating target") + targetObj.Role = "targets/whosits" + + // if no roles are passed, the scope is targets, not whatever targetObj has + // specified + assert.NoError(t, repo.AddTarget(targetObj)) + changes := getChanges(t, repo) + assert.Len(t, changes, 1) + assert.Equal(t, data.CanonicalTargetsRole, changes[0].Scope()) + + // if roles are passed, the scope is one of those roles, not whatever + // targetObj has specified + assert.NoError(t, repo.AddTarget(targetObj, "targets/one", "targets/two")) + changes = getChanges(t, repo) + assert.Len(t, changes, 3) + assert.Equal(t, "targets/one", changes[1].Scope()) + assert.Equal(t, "targets/two", changes[2].Scope()) +} + // Tests that adding a target to a repo or deleting a target from a repo, // with the given roles, makes a change to the expected scopes func testAddOrDeleteTarget(t *testing.T, repo *NotaryRepository, action string, @@ -593,7 +622,8 @@ func testAddOrDeleteTarget(t *testing.T, repo *NotaryRepository, action string, } } -// TestAddTargetToSpecifiedValidRoles adds a target to the specified roles. +// TestAddTargetToSpecifiedValidRoles adds a target to the specified roles, +// ignoring whatever role is in the target. // Confirms that the changelist is created correctly, one for each of the // the specified roles as scopes. func TestAddTargetToSpecifiedValidRoles(t *testing.T) { @@ -761,14 +791,14 @@ func TestRemoveTargetErrorWritingChanges(t *testing.T) { // of listed targets. // We test this with both an RSA and ECDSA root key func TestListTarget(t *testing.T) { - // testListEmptyTargets(t, data.ECDSAKey) - // testListTarget(t, data.ECDSAKey) + testListEmptyTargets(t, data.ECDSAKey) + testListTarget(t, data.ECDSAKey) testListTargetWithDelegates(t, data.ECDSAKey) - // if !testing.Short() { - // testListEmptyTargets(t, data.RSAKey) - // testListTarget(t, data.RSAKey) - // testListTargetWithDelegates(t, data.RSAKey) - // } + if !testing.Short() { + testListEmptyTargets(t, data.RSAKey) + testListTarget(t, data.RSAKey) + testListTargetWithDelegates(t, data.RSAKey) + } } func testListEmptyTargets(t *testing.T, rootType string) { diff --git a/tuf/client/client.go b/tuf/client/client.go index 834f5c8def..fbb19e9266 100644 --- a/tuf/client/client.go +++ b/tuf/client/client.go @@ -550,7 +550,7 @@ func (c Client) TargetMeta(role, path string, excludeRoles ...string) (*data.Fil // we found the target! return meta, curr } - delegations := c.local.TargetDelegations(role, path, pathHex) + delegations := c.local.TargetDelegations(curr, path, pathHex) for _, d := range delegations { if !excl[d.Name] { roles = append(roles, d.Name) diff --git a/tuf/client/client_test.go b/tuf/client/client_test.go index d62525cb17..57488631e6 100644 --- a/tuf/client/client_test.go +++ b/tuf/client/client_test.go @@ -3,6 +3,7 @@ package client import ( "crypto/sha256" "encoding/json" + "strconv" "testing" "time" @@ -660,3 +661,58 @@ func TestDownloadSnapshotBadChecksum(t *testing.T) { err = client.downloadSnapshot() assert.IsType(t, ErrChecksumMismatch{}, err) } + +// TargetMeta returns the file metadata for a file path in the role subtree, +// if it exists. It also returns the role in that subtree in which the target +// was found. If the path doesn't exist in that role subtree, returns +// nil and an empty string. +func TestTargetMeta(t *testing.T) { + kdb, repo, cs := testutils.EmptyRepo() + localStorage := store.NewMemoryStore(nil, nil) + client := NewClient(repo, nil, kdb, localStorage) + + delegations := []string{ + "targets/level1", + "targets/level1/a", + "targets/level1/a/i", + } + + k, err := cs.Create("", data.ED25519Key) + assert.NoError(t, err) + + hash := sha256.Sum256([]byte{}) + f := data.FileMeta{ + Length: 1, + Hashes: map[string][]byte{ + "sha256": hash[:], + }, + } + + for i, r := range delegations { + // create role + role, err := data.NewRole(r, 1, []string{k.ID()}, []string{""}, nil) + assert.NoError(t, err) + + // add role to repo + repo.UpdateDelegations(role, []data.PublicKey{k}) + repo.InitTargets(r) + + // add a target to the role + _, err = repo.AddTargets(r, data.Files{strconv.Itoa(i): f}) + assert.NoError(t, err) + } + + // returns the right level + fileMeta, role := client.TargetMeta("targets", "1") + assert.Equal(t, &f, fileMeta) + assert.Equal(t, "targets/level1/a", role) + + // looks only in subtree + fileMeta, role = client.TargetMeta("targets/level1/a", "0") + assert.Nil(t, fileMeta) + assert.Equal(t, "", role) + + fileMeta, role = client.TargetMeta("targets/level1/a", "2") + assert.Equal(t, &f, fileMeta) + assert.Equal(t, "targets/level1/a/i", role) +} From 2f2a0b9c9fde4d23859ba0e14f8a6f3d1f70a179 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Mon, 4 Jan 2016 10:48:03 -0800 Subject: [PATCH 3/4] Display the role when listing targets using the Notary CLI. Signed-off-by: Ying Li --- client/client_test.go | 8 ++++---- cmd/notary/prettyprint.go | 3 ++- cmd/notary/prettyprint_test.go | 16 ++++++++-------- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 1f29d0cce8..b8b0c3f263 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -529,7 +529,7 @@ func TestAddTargetIgnoresTargetRole(t *testing.T) { targetObj, err := NewTarget("latest", "../fixtures/root-ca.crt") assert.NoError(t, err, "error creating target") - targetObj.Role = "targets/whosits" + targetObj.Role = "targets/whoisit" // if no roles are passed, the scope is targets, not whatever targetObj has // specified @@ -940,7 +940,7 @@ func testListTarget(t *testing.T, rootType string) { sort.Stable(targetSorter(targets)) - // the targets should both be find in the targets role + // the targets should both be found in the targets role latestTarget.Role = data.CanonicalTargetsRole currentTarget.Role = data.CanonicalTargetsRole @@ -1016,7 +1016,7 @@ func testListTargetWithDelegates(t *testing.T, rootType string) { targets, err := repo.ListTargets() assert.NoError(t, err) - // Should be two targets + // Should be four targets assert.Len(t, targets, 4, "unexpected number of targets returned by ListTargets") sort.Stable(targetSorter(targets)) @@ -1037,7 +1037,7 @@ func testListTargetWithDelegates(t *testing.T, rootType string) { targets, err = repo.ListTargets("targets/level1", data.CanonicalTargetsRole) assert.NoError(t, err) - // Should be two targets + // Should be four targets assert.Len(t, targets, 4, "unexpected number of targets returned by ListTargets") sort.Stable(targetSorter(targets)) diff --git a/cmd/notary/prettyprint.go b/cmd/notary/prettyprint.go index 191220b7f7..cc8d362fed 100644 --- a/cmd/notary/prettyprint.go +++ b/cmd/notary/prettyprint.go @@ -148,13 +148,14 @@ func prettyPrintTargets(ts []*client.Target, writer io.Writer) { sort.Stable(targetsSorter(ts)) - table := getTable([]string{"Name", "Digest", "Size (bytes)"}, writer) + table := getTable([]string{"Name", "Digest", "Size (bytes)", "Role"}, writer) for _, t := range ts { table.Append([]string{ t.Name, hex.EncodeToString(t.Hashes["sha256"]), fmt.Sprintf("%d", t.Length), + t.Role, }) } table.Render() diff --git a/cmd/notary/prettyprint_test.go b/cmd/notary/prettyprint_test.go index 1231c57fde..3289bab431 100644 --- a/cmd/notary/prettyprint_test.go +++ b/cmd/notary/prettyprint_test.go @@ -157,7 +157,7 @@ func TestPrettyPrintZeroTargets(t *testing.T) { } -// Targets are sorted by name, and the name, SHA256 digest, and size are +// Targets are sorted by name, and the name, SHA256 digest, size, and role are // printed. func TestPrettyPrintSortedTargets(t *testing.T) { hashes := make([][]byte, 3) @@ -167,9 +167,9 @@ func TestPrettyPrintSortedTargets(t *testing.T) { assert.NoError(t, err) } unsorted := []*client.Target{ - {Name: "zebra", Hashes: data.Hashes{"sha256": hashes[0]}, Length: 8}, - {Name: "abracadabra", Hashes: data.Hashes{"sha256": hashes[1]}, Length: 1}, - {Name: "bee", Hashes: data.Hashes{"sha256": hashes[2]}, Length: 5}, + {Name: "zebra", Role: "targets/b", Hashes: data.Hashes{"sha256": hashes[0]}, Length: 8}, + {Name: "abracadabra", Role: "targets", Hashes: data.Hashes{"sha256": hashes[1]}, Length: 1}, + {Name: "bee", Role: "targets/a", Hashes: data.Hashes{"sha256": hashes[2]}, Length: 5}, } var b bytes.Buffer @@ -178,9 +178,9 @@ func TestPrettyPrintSortedTargets(t *testing.T) { assert.NoError(t, err) expected := [][]string{ - {"abracadabra", "b012", "1"}, - {"bee", "c012", "5"}, - {"zebra", "a012", "8"}, + {"abracadabra", "b012", "1", "targets"}, + {"bee", "c012", "5", "targets/a"}, + {"zebra", "a012", "8", "targets/b"}, } lines := strings.Split(strings.TrimSpace(string(text)), "\n") @@ -188,7 +188,7 @@ func TestPrettyPrintSortedTargets(t *testing.T) { // starts with headers assert.True(t, reflect.DeepEqual(strings.Fields(lines[0]), strings.Fields( - "NAME DIGEST SIZE (BYTES)"))) + "NAME DIGEST SIZE (BYTES) ROLE"))) assert.Equal(t, "----", lines[1][:4]) for i, line := range lines[2:] { From 61bbf7be491a52f12eb700d1dc125c715307a382 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Mon, 4 Jan 2016 16:59:51 -0800 Subject: [PATCH 4/4] Change ListTargetes and GetTargetsByName to return TargetWithRole. This object has both the target and the role in which the target was found. Signed-off-by: Ying Li --- client/client.go | 26 ++++--- client/client_test.go | 121 ++++++++++++++------------------- cmd/notary/prettyprint.go | 4 +- cmd/notary/prettyprint_test.go | 13 ++-- 4 files changed, 75 insertions(+), 89 deletions(-) diff --git a/client/client.go b/client/client.go index 48991bfd4c..26aeea8f1c 100644 --- a/client/client.go +++ b/client/client.go @@ -130,7 +130,13 @@ type Target struct { Name string // the name of the target Hashes data.Hashes // the hash of the target Length int64 // the size in bytes of the target - Role string // the role where the target was found - used for reading only +} + +// TargetWithRole represents a Target that exists in a particular role - this is +// produced by ListTargets and GetTargetByName +type TargetWithRole struct { + Target + Role string } // NewTarget is a helper method that returns a Target @@ -369,8 +375,6 @@ func (r *NotaryRepository) RemoveDelegation(name string) error { // AddTarget creates new changelist entries to add a target to the given roles // in the repository when the changelist gets appied at publish time. // If roles are unspecified, the default role is "targets". -// AddTarget ignores the role in the `target` object, since passing a list of -// roles means the target can be added to more than one role at a time. func (r *NotaryRepository) AddTarget(target *Target, roles ...string) error { cl, err := changelist.NewFileChangelist(filepath.Join(r.tufRepoPath, "changelist")) @@ -414,7 +418,7 @@ func (r *NotaryRepository) RemoveTarget(targetName string, roles ...string) erro // its entries will be strictly shadowed by those in other parts of the "targets/a" // 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) ([]*Target, error) { +func (r *NotaryRepository) ListTargets(roles ...string) ([]*TargetWithRole, error) { c, err := r.bootstrapClient() if err != nil { return nil, err @@ -431,7 +435,7 @@ func (r *NotaryRepository) ListTargets(roles ...string) ([]*Target, error) { if len(roles) == 0 { roles = []string{data.CanonicalTargetsRole} } - targets := make(map[string]*Target) + targets := make(map[string]*TargetWithRole) for _, role := range roles { // we don't need to do anything special with removing role from // roles because listSubtree always processes role and only excludes @@ -439,7 +443,7 @@ func (r *NotaryRepository) ListTargets(roles ...string) ([]*Target, error) { r.listSubtree(targets, role, roles...) } - var targetList []*Target + var targetList []*TargetWithRole for _, v := range targets { targetList = append(targetList, v) } @@ -447,7 +451,7 @@ func (r *NotaryRepository) ListTargets(roles ...string) ([]*Target, error) { return targetList, nil } -func (r *NotaryRepository) listSubtree(targets map[string]*Target, role string, exclude ...string) { +func (r *NotaryRepository) listSubtree(targets map[string]*TargetWithRole, role string, exclude ...string) { excl := make(map[string]bool) for _, r := range exclude { excl[r] = true @@ -463,7 +467,8 @@ func (r *NotaryRepository) listSubtree(targets map[string]*Target, role string, } for name, meta := range tgts.Signed.Targets { if _, ok := targets[name]; !ok { - targets[name] = &Target{Name: name, Hashes: meta.Hashes, Length: meta.Length, Role: role} + targets[name] = &TargetWithRole{ + Target: Target{Name: name, Hashes: meta.Hashes, Length: meta.Length}, Role: role} } } for _, d := range tgts.Signed.Delegations.Roles { @@ -481,7 +486,7 @@ func (r *NotaryRepository) listSubtree(targets map[string]*Target, role string, // the target entry found in the subtree of the highest priority role // will be returned // See the IMPORTANT section on ListTargets above. Those roles also apply here. -func (r *NotaryRepository) GetTargetByName(name string, roles ...string) (*Target, error) { +func (r *NotaryRepository) GetTargetByName(name string, roles ...string) (*TargetWithRole, error) { c, err := r.bootstrapClient() if err != nil { return nil, err @@ -501,7 +506,8 @@ func (r *NotaryRepository) GetTargetByName(name string, roles ...string) (*Targe for _, role := range roles { meta, foundRole := c.TargetMeta(role, name, roles...) if meta != nil { - return &Target{Name: name, Hashes: meta.Hashes, Length: meta.Length, Role: foundRole}, nil + return &TargetWithRole{ + Target: Target{Name: name, Hashes: meta.Hashes, Length: meta.Length}, Role: foundRole}, nil } } return nil, fmt.Errorf("No trust data for %s", name) diff --git a/client/client_test.go b/client/client_test.go index b8b0c3f263..d99e7750aa 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -518,35 +518,6 @@ func TestAddTargetToTargetRoleByDefault(t *testing.T) { []string{data.CanonicalTargetsRole}) } -// Adding a target ignores the role of the target object, and goes by the -// roles passed to AddTarget (or the default targets role, if no roles are passed) -func TestAddTargetIgnoresTargetRole(t *testing.T) { - ts, _, _ := simpleTestServer(t) - defer ts.Close() - - repo, _ := initializeRepo(t, data.ECDSAKey, "docker.com/notary", ts.URL, false) - defer os.RemoveAll(repo.baseDir) - - targetObj, err := NewTarget("latest", "../fixtures/root-ca.crt") - assert.NoError(t, err, "error creating target") - targetObj.Role = "targets/whoisit" - - // if no roles are passed, the scope is targets, not whatever targetObj has - // specified - assert.NoError(t, repo.AddTarget(targetObj)) - changes := getChanges(t, repo) - assert.Len(t, changes, 1) - assert.Equal(t, data.CanonicalTargetsRole, changes[0].Scope()) - - // if roles are passed, the scope is one of those roles, not whatever - // targetObj has specified - assert.NoError(t, repo.AddTarget(targetObj, "targets/one", "targets/two")) - changes = getChanges(t, repo) - assert.Len(t, changes, 3) - assert.Equal(t, "targets/one", changes[1].Scope()) - assert.Equal(t, "targets/two", changes[2].Scope()) -} - // Tests that adding a target to a repo or deleting a target from a repo, // with the given roles, makes a change to the expected scopes func testAddOrDeleteTarget(t *testing.T, repo *NotaryRepository, action string, @@ -622,8 +593,7 @@ func testAddOrDeleteTarget(t *testing.T, repo *NotaryRepository, action string, } } -// TestAddTargetToSpecifiedValidRoles adds a target to the specified roles, -// ignoring whatever role is in the target. +// TestAddTargetToSpecifiedValidRoles adds a target to the specified roles. // Confirms that the changelist is created correctly, one for each of the // the specified roles as scopes. func TestAddTargetToSpecifiedValidRoles(t *testing.T) { @@ -899,7 +869,7 @@ func fakeServerData(t *testing.T, repo *NotaryRepository, mux *http.ServeMux, } // We want to sort by name, so we can guarantee ordering. -type targetSorter []*Target +type targetSorter []*TargetWithRole func (k targetSorter) Len() int { return len(k) } func (k targetSorter) Swap(i, j int) { k[i], k[j] = k[j], k[i] } @@ -941,21 +911,24 @@ func testListTarget(t *testing.T, rootType string) { sort.Stable(targetSorter(targets)) // the targets should both be found in the targets role - latestTarget.Role = data.CanonicalTargetsRole - currentTarget.Role = data.CanonicalTargetsRole + for _, foundTarget := range targets { + assert.Equal(t, data.CanonicalTargetsRole, foundTarget.Role) + } // current should be first - assert.Equal(t, currentTarget, targets[0], "current target does not match") - assert.Equal(t, latestTarget, targets[1], "latest target does not match") + assert.True(t, reflect.DeepEqual(*currentTarget, targets[0].Target), "current target does not match") + assert.True(t, reflect.DeepEqual(*latestTarget, targets[1].Target), "latest target does not match") // Also test GetTargetByName newLatestTarget, err := repo.GetTargetByName("latest") assert.NoError(t, err) - assert.Equal(t, latestTarget, newLatestTarget, "latest target does not match") + assert.Equal(t, data.CanonicalTargetsRole, newLatestTarget.Role) + assert.True(t, reflect.DeepEqual(*latestTarget, newLatestTarget.Target), "latest target does not match") newCurrentTarget, err := repo.GetTargetByName("current") assert.NoError(t, err) - assert.Equal(t, currentTarget, newCurrentTarget, "current target does not match") + assert.Equal(t, data.CanonicalTargetsRole, newCurrentTarget.Role) + assert.True(t, reflect.DeepEqual(*currentTarget, newCurrentTarget.Target), "current target does not match") } func testListTargetWithDelegates(t *testing.T, rootType string) { @@ -1021,17 +994,18 @@ func testListTargetWithDelegates(t *testing.T, rootType string) { sort.Stable(targetSorter(targets)) - // specify where the targets were found: - latestTarget.Role = data.CanonicalTargetsRole - currentTarget.Role = data.CanonicalTargetsRole - level2Target.Role = "targets/level2" - otherTarget.Role = "targets/level1" - // current should be first. - assert.Equal(t, currentTarget, targets[0], "current target does not match") - assert.Equal(t, latestTarget, targets[1], "latest target does not match") - assert.Equal(t, level2Target, targets[2], "level2 target does not match") - assert.Equal(t, otherTarget, targets[3], "other target does not match") + assert.True(t, reflect.DeepEqual(*currentTarget, targets[0].Target), "current target does not match") + assert.Equal(t, data.CanonicalTargetsRole, targets[0].Role) + + assert.True(t, reflect.DeepEqual(*latestTarget, targets[1].Target), "latest target does not match") + assert.Equal(t, data.CanonicalTargetsRole, targets[1].Role) + + assert.True(t, reflect.DeepEqual(*level2Target, targets[2].Target), "level2 target does not match") + assert.Equal(t, "targets/level2", targets[2].Role) + + assert.True(t, reflect.DeepEqual(*otherTarget, targets[3].Target), "other target does not match") + assert.Equal(t, "targets/level1", targets[3].Role) // test listing with priority specified targets, err = repo.ListTargets("targets/level1", data.CanonicalTargetsRole) @@ -1042,34 +1016,39 @@ func testListTargetWithDelegates(t *testing.T, rootType string) { sort.Stable(targetSorter(targets)) - // specify where the targets were found: - latestTarget.Role = data.CanonicalTargetsRole - delegatedTarget.Role = "targets/level1" - level2Target.Role = "targets/level2" - otherTarget.Role = "targets/level1" + // current (in delegated role) should be first + assert.True(t, reflect.DeepEqual(*delegatedTarget, targets[0].Target), "current target does not match") + assert.Equal(t, "targets/level1", targets[0].Role) - // current should be first - assert.Equal(t, delegatedTarget, targets[0], "current target does not match") - assert.Equal(t, latestTarget, targets[1], "latest target does not match") - assert.Equal(t, level2Target, targets[2], "level2 target does not match") - assert.Equal(t, otherTarget, targets[3], "other target does not match") + assert.True(t, reflect.DeepEqual(*latestTarget, targets[1].Target), "latest target does not match") + assert.Equal(t, data.CanonicalTargetsRole, targets[1].Role) + + assert.True(t, reflect.DeepEqual(*level2Target, targets[2].Target), "level2 target does not match") + assert.Equal(t, "targets/level2", targets[2].Role) + + assert.True(t, reflect.DeepEqual(*otherTarget, targets[3].Target), "other target does not match") + assert.Equal(t, "targets/level1", targets[3].Role) // Also test GetTargetByName newLatestTarget, err := repo.GetTargetByName("latest") assert.NoError(t, err) - assert.Equal(t, latestTarget, newLatestTarget, "latest target does not match") + assert.True(t, reflect.DeepEqual(*latestTarget, newLatestTarget.Target), "latest target does not match") + assert.Equal(t, data.CanonicalTargetsRole, newLatestTarget.Role) newCurrentTarget, err := repo.GetTargetByName("current", "targets/level1", "targets") assert.NoError(t, err) - assert.Equal(t, delegatedTarget, newCurrentTarget, "current target does not match") + assert.True(t, reflect.DeepEqual(*delegatedTarget, newCurrentTarget.Target), "current target does not match") + assert.Equal(t, "targets/level1", newCurrentTarget.Role) newOtherTarget, err := repo.GetTargetByName("other") assert.NoError(t, err) - assert.True(t, reflect.DeepEqual(otherTarget, newOtherTarget), "other target does not match") + assert.True(t, reflect.DeepEqual(*otherTarget, newOtherTarget.Target), "other target does not match") + assert.Equal(t, "targets/level1", newOtherTarget.Role) newLevel2Target, err := repo.GetTargetByName("level2") assert.NoError(t, err) - assert.True(t, reflect.DeepEqual(level2Target, newLevel2Target), "level2 target does not match") + assert.True(t, reflect.DeepEqual(*level2Target, newLevel2Target.Target), "level2 target does not match") + assert.Equal(t, "targets/level2", newLevel2Target.Role) } // TestValidateRootKey verifies that the public data in root.json for the root @@ -1316,19 +1295,21 @@ func assertPublishToRolesSucceeds(t *testing.T, repo1 *NotaryRepository, sort.Stable(targetSorter(targets)) - currentTarget.Role = role - latestTarget.Role = role - assert.Equal(t, currentTarget, targets[0], "current target does not match") - assert.Equal(t, latestTarget, targets[1], "latest target does not match") + assert.True(t, reflect.DeepEqual(*currentTarget, targets[0].Target), "current target does not match") + assert.Equal(t, role, targets[0].Role) + assert.True(t, reflect.DeepEqual(*latestTarget, targets[1].Target), "latest target does not match") + assert.Equal(t, role, targets[1].Role) // Also test GetTargetByName newLatestTarget, err := repo.GetTargetByName("latest", role) assert.NoError(t, err) - assert.Equal(t, latestTarget, newLatestTarget, "latest target does not match") + assert.True(t, reflect.DeepEqual(*latestTarget, newLatestTarget.Target), "latest target does not match") + assert.Equal(t, role, newLatestTarget.Role) newCurrentTarget, err := repo.GetTargetByName("current", role) assert.NoError(t, err) - assert.Equal(t, currentTarget, newCurrentTarget, "current target does not match") + assert.True(t, reflect.DeepEqual(*currentTarget, newCurrentTarget.Target), "current target does not match") + assert.Equal(t, role, newCurrentTarget.Role) } } } @@ -1366,9 +1347,7 @@ func testPublishAfterPullServerHasSnapshotKey(t *testing.T, rootType string) { // list, so that the snapshot metadata is pulled from server targets, err := repo.ListTargets(data.CanonicalTargetsRole) assert.NoError(t, err) - // specify where target was found: - published.Role = data.CanonicalTargetsRole - assert.Equal(t, []*Target{published}, targets) + assert.Equal(t, []*TargetWithRole{{Target: *published, Role: data.CanonicalTargetsRole}}, targets) // listing downloaded the timestamp and snapshot metadata info assertRepoHasExpectedMetadata(t, repo, data.CanonicalTimestampRole, true) assertRepoHasExpectedMetadata(t, repo, data.CanonicalSnapshotRole, true) diff --git a/cmd/notary/prettyprint.go b/cmd/notary/prettyprint.go index cc8d362fed..96a3ef8225 100644 --- a/cmd/notary/prettyprint.go +++ b/cmd/notary/prettyprint.go @@ -130,7 +130,7 @@ func prettyPrintKeys(keyStores []trustmanager.KeyStore, writer io.Writer) { // --- pretty printing targets --- -type targetsSorter []*client.Target +type targetsSorter []*client.TargetWithRole func (t targetsSorter) Len() int { return len(t) } func (t targetsSorter) Swap(i, j int) { t[i], t[j] = t[j], t[i] } @@ -140,7 +140,7 @@ func (t targetsSorter) Less(i, j int) bool { // Given a list of KeyStores in order of listing preference, pretty-prints the // root keys and then the signing keys. -func prettyPrintTargets(ts []*client.Target, writer io.Writer) { +func prettyPrintTargets(ts []*client.TargetWithRole, writer io.Writer) { if len(ts) == 0 { writer.Write([]byte("\nNo targets present in this repository.\n\n")) return diff --git a/cmd/notary/prettyprint_test.go b/cmd/notary/prettyprint_test.go index 3289bab431..afb14dffcc 100644 --- a/cmd/notary/prettyprint_test.go +++ b/cmd/notary/prettyprint_test.go @@ -147,7 +147,7 @@ func TestPrettyPrintRootAndSigningKeys(t *testing.T) { // are no targets. func TestPrettyPrintZeroTargets(t *testing.T) { var b bytes.Buffer - prettyPrintTargets([]*client.Target{}, &b) + prettyPrintTargets([]*client.TargetWithRole{}, &b) text, err := ioutil.ReadAll(&b) assert.NoError(t, err) @@ -166,10 +166,11 @@ func TestPrettyPrintSortedTargets(t *testing.T) { hashes[i], err = hex.DecodeString(letter) assert.NoError(t, err) } - unsorted := []*client.Target{ - {Name: "zebra", Role: "targets/b", Hashes: data.Hashes{"sha256": hashes[0]}, Length: 8}, - {Name: "abracadabra", Role: "targets", Hashes: data.Hashes{"sha256": hashes[1]}, Length: 1}, - {Name: "bee", Role: "targets/a", Hashes: data.Hashes{"sha256": hashes[2]}, Length: 5}, + unsorted := []*client.TargetWithRole{ + {Target: client.Target{Name: "zebra", Hashes: data.Hashes{"sha256": hashes[0]}, Length: 8}, Role: "targets/b"}, + {Target: client.Target{Name: "aardvark", Hashes: data.Hashes{"sha256": hashes[1]}, Length: 1}, + Role: "targets"}, + {Target: client.Target{Name: "bee", Hashes: data.Hashes{"sha256": hashes[2]}, Length: 5}, Role: "targets/a"}, } var b bytes.Buffer @@ -178,7 +179,7 @@ func TestPrettyPrintSortedTargets(t *testing.T) { assert.NoError(t, err) expected := [][]string{ - {"abracadabra", "b012", "1", "targets"}, + {"aardvark", "b012", "1", "targets"}, {"bee", "c012", "5", "targets/a"}, {"zebra", "a012", "8", "targets/b"}, }