fixing up ListTargets and GetTargetByName to process prioritized roles more efficiently

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

client/client.go

@@ -419,8 +419,34 @@ func (r *NotaryRepository) ListTargets(roles ...string) ([]*Target, error) {
 		return nil, err
 	}
 
+	if len(roles) == 0 {
+		roles = []string{data.CanonicalTargetsRole}
+	}
 	targets := make(map[string]*Target)
 	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
+		// descendent delegations that appear in roles.
+		r.listSubtree(targets, role, roles...)
+	}
+
+	var targetList []*Target
+	for _, v := range targets {
+		targetList = append(targetList, v)
+	}
+
+	return targetList, nil
+}
+
+func (r *NotaryRepository) listSubtree(targets map[string]*Target, 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
@@ -430,14 +456,12 @@ func (r *NotaryRepository) ListTargets(roles ...string) ([]*Target, error) {
 			target := &Target{Name: name, Hashes: meta.Hashes, Length: meta.Length}
 			targets[name] = target
 		}
+		for _, d := range tgts.Signed.Delegations.Roles {
+			if !excl[d.Name] {
+				roles = append(roles, d.Name)
+			}
+		}
 	}
-
-	var targetList []*Target
-	for _, v := range targets {
-		targetList = append(targetList, v)
-	}
-
-	return targetList, nil
 }
 
 // GetTargetByName returns a target given a name. If no roles are passed
@@ -467,7 +491,7 @@ func (r *NotaryRepository) GetTargetByName(name string, roles ...string) (*Targe
 		meta *data.FileMeta
 	)
 	for i := len(roles) - 1; i >= 0; i-- {
-		meta, err = c.TargetMeta(roles[i], name)
+		meta, err = c.TargetMeta(roles[i], name, roles...)
 		if err != nil {
 			// important to error here otherwise there might be malicious
 			// behaviour that prevents a legitimate version of a target

server/server.go

@@ -89,7 +89,7 @@ func RootHandler(ac auth.AccessController, ctx context.Context, trust signed.Cry
 		prometheus.InstrumentHandlerWithOpts(
 			prometheusOpts("UpdateTuf"),
 			hand(handlers.AtomicUpdateHandler, "push", "pull")))
-	r.Methods("GET").Path("/v2/{imageName:.*}/_trust/tuf/{tufRole:(root|targets(?:/.+)?|snapshot|timestamp)}.json").Handler(
+	r.Methods("GET").Path("/v2/{imageName:.*}/_trust/tuf/{tufRole:(root|targets(?:/[^/\\s]+)*|snapshot|timestamp)}.json").Handler(
 		prometheus.InstrumentHandlerWithOpts(
 			prometheusOpts("GetRole"),
 			hand(handlers.GetHandler, "pull")))

tuf/client/client.go

@@ -520,10 +520,15 @@ 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) (*data.FileMeta, error) {
+func (c Client) TargetMeta(role, path string, excludeRoles ...string) (*data.FileMeta, error) {
 	c.Update()
 	var meta *data.FileMeta
 
+	excl := make(map[string]bool)
+	for _, r := range excludeRoles {
+		excl[r] = true
+	}
+
 	pathDigest := sha256.Sum256([]byte(path))
 	pathHex := hex.EncodeToString(pathDigest[:])
 
@@ -542,7 +547,9 @@ func (c Client) TargetMeta(role, path string) (*data.FileMeta, error) {
 		}
 		delegations := c.local.TargetDelegations(role, path, pathHex)
 		for _, d := range delegations {
-			roles = append(roles, d.Name)
+			if !excl[d.Name] {
+				roles = append(roles, d.Name)
+			}
 		}
 	}
 	return meta, nil