WalkTargets on tuf repo, use in getting targets/roles

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

client/client.go

@@ -390,10 +390,23 @@ func (r *NotaryRepository) ListTargets(roles ...string) ([]*TargetWithRole, erro
 	}
 	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
-		// descendant delegations that appear in roles.
-		r.listSubtree(targets, role, roles...)
+		// Define a visitor function to populate the targets map in priority order
+		listVisitorFunc := func(tgt *data.SignedTargets, roleName string) error {
+			if tgt == nil {
+				return tuf.ErrContinueWalk{}
+			}
+			// We found targets so we should try to add them to our targets map
+			for targetName, targetMeta := range tgt.Signed.Targets {
+				// Follow the priority by not overriding previously set targets
+				if _, ok := targets[targetName]; ok {
+					continue
+				}
+				targets[targetName] =
+					&TargetWithRole{Target: Target{Name: targetName, Hashes: targetMeta.Hashes, Length: targetMeta.Length}, Role: roleName}
+			}
+			return tuf.ErrContinueWalk{}
+		}
+		r.tufRepo.WalkTargets("", role, listVisitorFunc)
 	}
 
 	var targetList []*TargetWithRole
@@ -404,34 +417,6 @@ func (r *NotaryRepository) ListTargets(roles ...string) ([]*TargetWithRole, erro
 	return targetList, nil
 }
 
-func (r *NotaryRepository) listSubtree(targets map[string]*TargetWithRole, role string, exclude ...string) {
-	excl := make(map[string]bool)
-	for _, r := range exclude {
-		excl[r] = true
-	}
-	roles := []string{role}
-	for len(roles) > 0 {
-		role = roles[0]
-		roles = roles[1:]
-		tgts, ok := r.tufRepo.Targets[role]
-		if !ok {
-			// not every role has to exist
-			continue
-		}
-		for name, meta := range tgts.Signed.Targets {
-			if _, ok := targets[name]; !ok {
-				targets[name] = &TargetWithRole{
-					Target: Target{Name: name, Hashes: meta.Hashes, Length: meta.Length}, Role: role}
-			}
-		}
-		for _, d := range tgts.Signed.Delegations.Roles {
-			if !excl[d.Name] {
-				roles = append(roles, d.Name)
-			}
-		}
-	}
-}
-
 // GetTargetByName returns a target given a name. If no roles are passed
 // it uses the targets role and does a search of the entire delegation
 // graph, finding the first entry in a breadth first search of the delegations.
@@ -440,7 +425,7 @@ func (r *NotaryRepository) listSubtree(targets map[string]*TargetWithRole, role
 // will be returned
 // See the IMPORTANT section on ListTargets above. Those roles also apply here.
 func (r *NotaryRepository) GetTargetByName(name string, roles ...string) (*TargetWithRole, error) {
-	c, err := r.Update(false)
+	_, err := r.Update(false)
 	if err != nil {
 		return nil, err
 	}
@@ -448,11 +433,26 @@ func (r *NotaryRepository) GetTargetByName(name string, roles ...string) (*Targe
 	if len(roles) == 0 {
 		roles = append(roles, data.CanonicalTargetsRole)
 	}
+	var resultMeta data.FileMeta
+	var resultRoleName string
 	for _, role := range roles {
-		meta, foundRole := c.TargetMeta(role, name, roles...)
-		if meta != nil {
-			return &TargetWithRole{
-				Target: Target{Name: name, Hashes: meta.Hashes, Length: meta.Length}, Role: foundRole}, nil
+		// Define a visitor function to find the specified target
+		getTargetVisitorFunc := func(tgt *data.SignedTargets, roleName string) error {
+			var ok bool
+			if tgt == nil {
+				return tuf.ErrContinueWalk{}
+			}
+			// We found the target so we should stop our walk
+			// and set the resultMeta and resultRoleName variables
+			if resultMeta, ok = tgt.Signed.Targets[name]; ok {
+				resultRoleName = roleName
+				return tuf.ErrStopWalk{}
+			}
+			return tuf.ErrContinueWalk{}
+		}
+		err = r.tufRepo.WalkTargets(name, role, getTargetVisitorFunc)
+		if err == nil {
+			return &TargetWithRole{Target: Target{Name: name, Hashes: resultMeta.Hashes, Length: resultMeta.Length}, Role: resultRoleName}, nil
 		}
 	}
 	return nil, fmt.Errorf("No trust data for %s", name)

client/delegations.go

@@ -8,6 +8,7 @@ import (
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/notary"
 	"github.com/docker/notary/client/changelist"
+	"github.com/docker/notary/tuf"
 	"github.com/docker/notary/tuf/data"
 	"github.com/docker/notary/tuf/store"
 	"github.com/docker/notary/tuf/utils"
@@ -245,45 +246,28 @@ func (r *NotaryRepository) GetDelegationRoles() ([]*data.Role, error) {
 	}
 
 	// All top level delegations (ex: targets/level1) are stored exclusively in targets.json
-	targets, ok := r.tufRepo.Targets[data.CanonicalTargetsRole]
+	_, ok := r.tufRepo.Targets[data.CanonicalTargetsRole]
 	if !ok {
 		return nil, store.ErrMetaNotFound{Resource: data.CanonicalTargetsRole}
 	}
 
-	// make a copy of top-level Delegations and only show canonical key IDs
-	allDelegations, err := translateDelegationsToCanonicalIDs(targets.Signed.Delegations)
-	if err != nil {
-		return nil, err
-	}
-
 	// make a copy for traversing nested delegations
-	delegationsList := make([]*data.Role, len(allDelegations))
-	copy(delegationsList, allDelegations)
+	allDelegations := []*data.Role{}
 
-	// Now traverse to lower level delegations (ex: targets/level1/level2)
-	for len(delegationsList) > 0 {
-		// Pop off first delegation to traverse
-		delegation := delegationsList[0]
-		delegationsList = delegationsList[1:]
-
-		// Get metadata
-		delegationMeta, ok := r.tufRepo.Targets[delegation.Name]
-		// If we get an error, don't try to traverse further into this subtree because it doesn't exist or is malformed
-		if !ok {
-			continue
+	// Define a visitor function to populate the delegations list and translate their key IDs to canonical IDs
+	delegationCanonicalListVisitor := func(tgt *data.SignedTargets, roleName string) error {
+		if tgt == nil {
+			return tuf.ErrContinueWalk{}
 		}
-
 		// For the return list, update with a copy that includes canonicalKeyIDs
-		canonicalDelegations, err := translateDelegationsToCanonicalIDs(delegationMeta.Signed.Delegations)
+		canonicalDelegations, err := translateDelegationsToCanonicalIDs(tgt.Signed.Delegations)
 		if err != nil {
-			return nil, err
+			return err
 		}
 		allDelegations = append(allDelegations, canonicalDelegations...)
-		// Add nested delegations to the exploration list
-		delegationsList = append(delegationsList, delegationMeta.Signed.Delegations.Roles...)
+		return tuf.ErrContinueWalk{}
 	}
-
-	// Convert all key IDs to canonical IDs:
+	r.tufRepo.WalkTargets("", "", delegationCanonicalListVisitor)
 	return allDelegations, nil
 }
 

tuf/client/client.go

@@ -536,37 +536,3 @@ func (c Client) getTargetsFile(role string, snapshotMeta data.Files, consistent
 	}
 	return s, nil
 }
-
-// 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, string) {
-	excl := make(map[string]bool)
-	for _, r := range excludeRoles {
-		excl[r] = true
-	}
-
-	// FIFO list of targets delegations to inspect for target
-	roles := []string{role}
-	var (
-		meta *data.FileMeta
-		curr string
-	)
-	for len(roles) > 0 {
-		// have to do these lines here because of order of execution in for statement
-		curr = roles[0]
-		roles = roles[1:]
-
-		meta = c.local.TargetMeta(curr, path)
-		if meta != nil {
-			// we found the target!
-			return meta, curr
-		}
-		delegations := c.local.TargetDelegations(curr, path)
-		for _, d := range delegations {
-			if !excl[d.Name] {
-				roles = append(roles, d.Name)
-			}
-		}
-	}
-	return meta, ""
-}

tuf/client/client_test.go

@@ -789,62 +789,6 @@ func TestDownloadSnapshotChecksumNotFound(t *testing.T) {
 	assert.IsType(t, store.ErrMetaNotFound{}, 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) {
-	repo, cs, err := testutils.EmptyRepo("docker.com/notary")
-	assert.NoError(t, err)
-	localStorage := store.NewMemoryStore(nil)
-	client := NewClient(repo, nil, 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{""})
-		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)
-}
-
 // If there is no local cache and also no remote timestamp, downloading the timestamp
 // fails with a store.ErrMetaNotFound
 func TestDownloadTimestampNoTimestamps(t *testing.T) {

tuf/tuf.go

@@ -47,6 +47,20 @@ func (err ErrNotLoaded) Error() string {
 	return fmt.Sprintf("%s role has not been loaded", err.Role)
 }
 
+// ErrStopWalk - used by visitor functions to signal WalkTargets to stop walking
+type ErrStopWalk struct{}
+
+func (e ErrStopWalk) Error() string {
+	return "Error: signalled to stop the walk"
+}
+
+// ErrContinueWalk - used by visitor functions to signal WalkTargets to continue walking
+type ErrContinueWalk struct{}
+
+func (e ErrContinueWalk) Error() string {
+	return "Error: signalled to continue the walk"
+}
+
 // Repo is an in memory representation of the TUF Repo.
 // It operates at the data.Signed level, accepting and producing
 // data.Signed objects. Users of a Repo are responsible for
@@ -558,32 +572,6 @@ func (tr Repo) TargetDelegations(role, path string) []*data.Role {
 	return roles
 }
 
-// FindTarget attempts to find the target represented by the given
-// path by starting at the top targets file and traversing
-// appropriate delegations until the first entry is found or it
-// runs out of locations to search.
-// N.B. Multiple entries may exist in different delegated roles
-//      for the same target. Only the first one encountered is returned.
-func (tr Repo) FindTarget(path string) *data.FileMeta {
-
-	var walkTargets func(role string) *data.FileMeta
-	walkTargets = func(role string) *data.FileMeta {
-		if m := tr.TargetMeta(role, path); m != nil {
-			return m
-		}
-		// Depth first search of delegations based on order
-		// as presented in current targets file for role:
-		for _, r := range tr.TargetDelegations(role, path) {
-			if m := walkTargets(r.Name); m != nil {
-				return m
-			}
-		}
-		return nil
-	}
-
-	return walkTargets("targets")
-}
-
 // VerifyCanSign returns nil if the role exists and we have at least one
 // signing key for the role, false otherwise.  This does not check that we have
 // enough signing keys to meet the threshold, since we want to support the use
@@ -624,6 +612,83 @@ func (tr *Repo) VerifyCanSign(roleName string) error {
 	return signed.ErrNoKeys{KeyIDs: role.ListKeyIDs()}
 }
 
+// used for walking the targets/delegations tree, potentially modifying the underlying SignedTargets for the repo
+type walkVisitorFunc func(*data.SignedTargets, string) error
+
+// WalkTargets will apply the specified visitor function to iteratively walk the targets/delegation metadata tree,
+// until receiving a ErrStopWalk.  The walk starts from the base "targets" role, and searches for the correct targetPath and/or rolePath
+// to call the visitor function on.
+func (tr *Repo) WalkTargets(targetPath, rolePath string, visitTarget walkVisitorFunc) error {
+	// Start with the base targets role, which implicitly has the "" targets path
+	targetsRole, err := tr.GetBaseRole(data.CanonicalTargetsRole)
+	if err != nil {
+		return err
+	}
+	// Make the targets role have the empty path, when we treat it as a delegation role
+	roles := []data.DelegationRole{
+		{
+			BaseRole: targetsRole,
+			Paths:    []string{""},
+		},
+	}
+
+	for len(roles) > 0 {
+		role := roles[0]
+		roles = roles[1:]
+
+		// Check the role metadata
+		signedTgt, ok := tr.Targets[role.Name]
+		if !ok {
+			// The role meta doesn't exist in the repo so continue onward
+			continue
+		}
+
+		// We're at a prefix of the desired role subtree, so add its delegation role children and continue walking
+		if strings.HasPrefix(rolePath, role.Name+"/") {
+			for _, delgRole := range tr.Targets[role.Name].Signed.Delegations.Roles {
+				delegationRole, err := tr.GetDelegationRole(delgRole.Name)
+				if err != nil {
+					return err
+				}
+				roles = append(roles, delegationRole)
+			}
+			continue
+		}
+
+		// Determine whether to visit this role or not:
+		// If the paths validate against the specified targetPath and the rolePath is empty or is in the subtree
+		if role.CheckPaths(targetPath) && isAncestorRole(role.Name, rolePath) {
+			// If we had matching path or role name, visit this target and determine whether or not to keep walking
+			err = visitTarget(signedTgt, role.Name)
+			switch err.(type) {
+			case ErrStopWalk:
+				// If the visitor function signalled a stop, return nil to finish the walk
+				return nil
+			case ErrContinueWalk:
+				// If the visitor function signalled to continue, add this role's delegation to the walk
+				for _, delgRole := range tr.Targets[role.Name].Signed.Delegations.Roles {
+					delegationRole, err := tr.GetDelegationRole(delgRole.Name)
+					if err != nil {
+						return err
+					}
+					roles = append(roles, delegationRole)
+				}
+			default:
+				// Return out if we got a different error or nil
+				return err
+			}
+
+		}
+	}
+	return nil
+}
+
+// helper function that returns whether the candidateChild role name is an ancestor or equal to the candidateAncestor role name
+// Will return true if given an empty candidateAncestor role name
+func isAncestorRole(candidateChild, candidateAncestor string) bool {
+	return candidateAncestor == "" || candidateAncestor == candidateChild || strings.HasPrefix(candidateChild, candidateAncestor+"/")
+}
+
 // AddTargets will attempt to add the given targets specifically to
 // the directed role. If the metadata for the role doesn't exist yet,
 // AddTargets will create one.