docker/docs
1
0
Fork 0
mirror of https://github.com/docker/docs.git synced 2026-11-05 00:14:17 +03:00
Code Activity

More robust errors for visitor and walker, improve comments

Browse Source 
Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
This commit is contained in:
Riyaz Faizullabhoy
2016-02-19 12:01:11 -08:00
parent 22c220d4d2
commit fe1c8ea1d8
7 changed files with 56 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
client/client.go
View File

@@ -391,18 +391,19 @@ func (r *NotaryRepository) ListTargets(roles ...string) ([]*TargetWithRole, erro
targets := make(map[string]*TargetWithRole)
for _, role := range roles {
// Define a visitor function to populate the targets map in priority order
listVisitorFunc := func(tgt *data.SignedTargets, roleName string) error {
listVisitorFunc := func(tgt *data.SignedTargets, validRole data.DelegationRole) 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 {
// and check that this path is valid with this role
if _, ok := targets[targetName]; ok || !validRole.CheckPaths(targetName) {
continue
}
targets[targetName] =
&TargetWithRole{Target: Target{Name: targetName, Hashes: targetMeta.Hashes, Length: targetMeta.Length}, Role: roleName}
&TargetWithRole{Target: Target{Name: targetName, Hashes: targetMeta.Hashes, Length: targetMeta.Length}, Role: validRole.Name}
}
return tuf.ErrContinueWalk{}
}
@@ -437,15 +438,15 @@ func (r *NotaryRepository) GetTargetByName(name string, roles ...string) (*Targe
var resultRoleName string
for _, role := range roles {
// Define a visitor function to find the specified target
getTargetVisitorFunc := func(tgt *data.SignedTargets, roleName string) error {
getTargetVisitorFunc := func(tgt *data.SignedTargets, validRole data.DelegationRole) 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
// We found the target and validated path compatiblity in our walk,
// so we should stop our walk and set the resultMeta and resultRoleName variables
if resultMeta, ok = tgt.Signed.Targets[name]; ok {
resultRoleName = roleName
resultRoleName = validRole.Name
return tuf.ErrStopWalk{}
}
return tuf.ErrContinueWalk{}

8
client/delegations.go
View File

@@ -255,11 +255,12 @@ func (r *NotaryRepository) GetDelegationRoles() ([]*data.Role, error) {
allDelegations := []*data.Role{}
// 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 {
delegationCanonicalListVisitor := func(tgt *data.SignedTargets, validRole data.DelegationRole) error {
if tgt == nil {
return tuf.ErrContinueWalk{}
}
// For the return list, update with a copy that includes canonicalKeyIDs
// These aren't validated by the validRole
canonicalDelegations, err := translateDelegationsToCanonicalIDs(tgt.Signed.Delegations)
if err != nil {
return err
@@ -267,7 +268,10 @@ func (r *NotaryRepository) GetDelegationRoles() ([]*data.Role, error) {
allDelegations = append(allDelegations, canonicalDelegations...)
return tuf.ErrContinueWalk{}
}
r.tufRepo.WalkTargets("", "", delegationCanonicalListVisitor)
err := r.tufRepo.WalkTargets("", "", delegationCanonicalListVisitor)
if err != nil {
return nil, err
}
return allDelegations, nil
}

17
client/helpers.go
View File

@@ -79,11 +79,13 @@ func changeTargetsDelegation(repo *tuf.Repo, c changelist.Change) error {
if err != nil {
return err
}
r, _, err := repo.GetDelegation(c.Scope())
if _, ok := err.(data.ErrNoSuchRole); err != nil && !ok {
// error that wasn't ErrNoSuchRole
_, err = repo.GetDelegationRole(c.Scope())
if _, ok := err.(data.ErrInvalidRole); err != nil && !ok {
// error that wasn't ErrInvalidRole
return err
}
r, _, _ := repo.GetDelegation(c.Scope())
if err == nil {
// role existed, attempt to merge paths and keys
if err := r.AddPaths(td.AddPaths); err != nil {
@@ -103,10 +105,11 @@ func changeTargetsDelegation(repo *tuf.Repo, c changelist.Change) error {
if err != nil {
return err
}
r, keys, err := repo.GetDelegation(c.Scope())
_, err = repo.GetDelegationRole(c.Scope())
if err != nil {
return err
}
r, keys, _ := repo.GetDelegation(c.Scope())
// We need to translate the keys from canonical ID to TUF ID for compatibility
canonicalToTUFID := make(map[string]string)
@@ -125,8 +128,7 @@ func changeTargetsDelegation(repo *tuf.Repo, c changelist.Change) error {
// If we specify the only keys left delete the role, else just delete specified keys
if strings.Join(r.KeyIDs, ";") == strings.Join(removeTUFKeyIDs, ";") && len(td.AddKeys) == 0 {
r := data.Role{Name: c.Scope()}
return repo.DeleteDelegation(r)
return repo.DeleteDelegation(c.Scope())
}
// if we aren't deleting and the role exists, merge
if err := r.AddPaths(td.AddPaths); err != nil {
@@ -142,8 +144,7 @@ func changeTargetsDelegation(repo *tuf.Repo, c changelist.Change) error {
r.RemoveKeys(removeTUFKeyIDs)
return repo.UpdateDelegations(r, td.AddKeys)
case changelist.ActionDelete:
r := data.Role{Name: c.Scope()}
return repo.DeleteDelegation(r)
return repo.DeleteDelegation(c.Scope())
default:
return fmt.Errorf("unsupported action against delegations: %s", c.Action())
}

2
client/helpers_test.go
View File

@@ -445,7 +445,7 @@ func TestApplyTargetsDelegationEditNonExisting(t *testing.T) {
err = applyTargetsChange(repo, ch)
assert.Error(t, err)
assert.IsType(t, data.ErrNoSuchRole{}, err)
assert.IsType(t, data.ErrInvalidRole{}, err)
}
func TestApplyTargetsDelegationCreateAlreadyExisting(t *testing.T) {

3
tuf/data/targets.go
View File

@@ -93,7 +93,8 @@ func (t SignedTargets) GetValidDelegations(parent DelegationRole) []DelegationRo
return result
}
// helper function to create DelegationRole structures from all delegations in a SignedTargets
// helper function to create DelegationRole structures from all delegations in a SignedTargets,
// these delegations are read directly from the SignedTargets and not modified or validated
func (t SignedTargets) buildDelegationRoles() []DelegationRole {
var roles []DelegationRole
for _, roleData := range t.Signed.Delegations.Roles {

43
tuf/tuf.go
View File

@@ -230,7 +230,7 @@ func (tr *Repo) GetDelegationRole(name string) (data.DelegationRole, error) {
// Start with top level roles in targets. Walk the chain of ancestors
// until finding the desired role, or we run out of targets files to search.
var foundRole *data.DelegationRole
buildDelegationRoleVisitor := func(tgt *data.SignedTargets, roleName string) error {
buildDelegationRoleVisitor := func(tgt *data.SignedTargets, validRole data.DelegationRole) error {
if tgt == nil {
return ErrContinueWalk{}
}
@@ -242,8 +242,8 @@ func (tr *Repo) GetDelegationRole(name string) (data.DelegationRole, error) {
for _, keyID := range role.KeyIDs {
pubKey, ok := tgt.Signed.Delegations.Keys[keyID]
if !ok {
// Couldn't retrieve all keys, so stop walking
return ErrStopWalk{}
// Couldn't retrieve all keys, so stop walking and return invalid role
return data.ErrInvalidRole{Role: name, Reason: "delegation does not exist"}
}
pubKeys[keyID] = pubKey
}
@@ -262,7 +262,10 @@ func (tr *Repo) GetDelegationRole(name string) (data.DelegationRole, error) {
}
// Walk to the parent of this delegation, since that is where its role metadata exists
tr.WalkTargets("", path.Dir(name), buildDelegationRoleVisitor)
err := tr.WalkTargets("", path.Dir(name), buildDelegationRoleVisitor)
if err != nil {
return data.DelegationRole{}, err
}
// We never found the delegation. In the context of this repo it is considered
// invalid. N.B. it may be that it existed at one point but an ancestor has since
@@ -388,22 +391,20 @@ func (tr *Repo) UpdateDelegations(role *data.Role, keys []data.PublicKey) error
// DeleteDelegation removes a delegated targets role from its parent
// targets object. It also deletes the delegation from the snapshot.
// DeleteDelegation will only make use of the role Name field.
func (tr *Repo) DeleteDelegation(role data.Role) error {
if !data.IsDelegation(role.Name) {
return data.ErrInvalidRole{Role: role.Name, Reason: "not a valid delegated role"}
func (tr *Repo) DeleteDelegation(roleName string) error {
if !data.IsDelegation(roleName) {
return data.ErrInvalidRole{Role: roleName, Reason: "not a valid delegated role"}
}
// the role variable must not be used past this assignment for safety
name := role.Name
parent := path.Dir(name)
parent := path.Dir(roleName)
if err := tr.VerifyCanSign(parent); err != nil {
return err
}
// delete delegated data from Targets map and Snapshot - if they don't
// exist, these are no-op
delete(tr.Targets, name)
tr.Snapshot.DeleteMeta(name)
delete(tr.Targets, roleName)
tr.Snapshot.DeleteMeta(roleName)
p, ok := tr.Targets[parent]
if !ok {
@@ -412,7 +413,7 @@ func (tr *Repo) DeleteDelegation(role data.Role) error {
return nil
}
foundAt := utils.FindRoleIndex(p.Signed.Delegations.Roles, name)
foundAt := utils.FindRoleIndex(p.Signed.Delegations.Roles, roleName)
if foundAt >= 0 {
var roles []*data.Role
@@ -601,7 +602,7 @@ func (tr *Repo) VerifyCanSign(roleName string) error {
}
// used for walking the targets/delegations tree, potentially modifying the underlying SignedTargets for the repo
type walkVisitorFunc func(*data.SignedTargets, string) error
type walkVisitorFunc func(*data.SignedTargets, data.DelegationRole) 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
@@ -641,7 +642,7 @@ func (tr *Repo) WalkTargets(targetPath, rolePath string, visitTarget walkVisitor
// If the paths validate against the specified targetPath and the rolePath is empty or is in the subtree
if isValidPath(targetPath, role) && 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)
err := visitTarget(signedTgt, role)
switch err.(type) {
case ErrStopWalk:
// If the visitor function signalled a stop, return nil to finish the walk
@@ -691,8 +692,8 @@ func (tr *Repo) AddTargets(role string, targets data.Files) (data.Files, error)
}
addedTargets := make(data.Files)
addTargetVisitor := func(targetPath string, targetMeta data.FileMeta) func(*data.SignedTargets, string) error {
return func(tgt *data.SignedTargets, roleName string) error {
addTargetVisitor := func(targetPath string, targetMeta data.FileMeta) func(*data.SignedTargets, data.DelegationRole) error {
return func(tgt *data.SignedTargets, validRole data.DelegationRole) error {
if tgt == nil {
return ErrContinueWalk{}
}
@@ -722,13 +723,15 @@ func (tr *Repo) RemoveTargets(role string, targets ...string) error {
return err
}
removeTargetVisitor := func(targetPath string) func(*data.SignedTargets, string) error {
return func(tgt *data.SignedTargets, roleName string) error {
removeTargetVisitor := func(targetPath string) func(*data.SignedTargets, data.DelegationRole) error {
return func(tgt *data.SignedTargets, validRole data.DelegationRole) error {
if tgt == nil {
return ErrContinueWalk{}
}
// We've already validated the role's target path in our walk, so just modify the metadata
// We've already validated the role path in our walk, so just modify the metadata
// We don't check against the target path against the valid role paths because it's
// possible we got into an invalid state and are trying to fix it
delete(tgt.Signed.Targets, targetPath)
tgt.Dirty = true
return ErrStopWalk{}

14
tuf/tuf_test.go
View File

@@ -390,7 +390,7 @@ func TestDeleteDelegations(t *testing.T) {
_, ok = repo.Snapshot.Signed.Meta["targets/test"]
assert.True(t, ok)
assert.NoError(t, repo.DeleteDelegation(*role))
assert.NoError(t, repo.DeleteDelegation(role.Name))
assert.Len(t, r.Signed.Delegations.Roles, 0)
assert.Len(t, r.Signed.Delegations.Keys, 0)
assert.True(t, r.Dirty)
@@ -420,7 +420,7 @@ func TestDeleteDelegationsRoleNotExistBecauseNoParentMeta(t *testing.T) {
delRole, err := data.NewRole("targets/test/a", 1, []string{testKey.ID()}, []string{"test"})
err = repo.DeleteDelegation(*delRole)
err = repo.DeleteDelegation(delRole.Name)
assert.NoError(t, err)
// still no metadata
_, ok = repo.Targets["targets/test"]
@@ -439,7 +439,7 @@ func TestDeleteDelegationsRoleNotExist(t *testing.T) {
role, err := data.NewRole("targets/test", 1, []string{}, []string{""})
assert.NoError(t, err)
err = repo.DeleteDelegation(*role)
err = repo.DeleteDelegation(role.Name)
assert.NoError(t, err)
r, ok := repo.Targets[data.CanonicalTargetsRole]
assert.True(t, ok)
@@ -457,7 +457,7 @@ func TestDeleteDelegationsInvalidRole(t *testing.T) {
invalidRole, err := data.NewRole("root", 1, []string{}, []string{""})
assert.NoError(t, err)
err = repo.DeleteDelegation(*invalidRole)
err = repo.DeleteDelegation(invalidRole.Name)
assert.Error(t, err)
assert.IsType(t, data.ErrInvalidRole{}, err)
@@ -473,7 +473,7 @@ func TestDeleteDelegationsParentMissing(t *testing.T) {
testRole, err := data.NewRole("targets/test/deep", 1, []string{}, []string{""})
assert.NoError(t, err)
err = repo.DeleteDelegation(*testRole)
err = repo.DeleteDelegation(testRole.Name)
assert.Error(t, err)
assert.IsType(t, data.ErrInvalidRole{}, err)
@@ -514,7 +514,7 @@ func TestDeleteDelegationsMissingParentSigningKey(t *testing.T) {
// delete all signing keys
repo.cryptoService = signed.NewEd25519()
err = repo.DeleteDelegation(*role)
err = repo.DeleteDelegation(role.Name)
assert.Error(t, err)
assert.IsType(t, signed.ErrNoKeys{}, err)
@@ -553,7 +553,7 @@ func TestDeleteDelegationsMidSliceRole(t *testing.T) {
err = repo.UpdateDelegations(role3, data.KeyList{testKey})
assert.NoError(t, err)
err = repo.DeleteDelegation(*role2)
err = repo.DeleteDelegation(role2.Name)
assert.NoError(t, err)
r, ok := repo.Targets[data.CanonicalTargetsRole]