From 93fc947b7877df531a4b1d504d46b94c85277e99 Mon Sep 17 00:00:00 2001 From: lemondrank Date: Mon, 13 Mar 2017 13:13:03 -0700 Subject: [PATCH 01/23] first commit --- vault/token_store.go | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index 1cd611ca0a29..c7f5c292044f 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -6,6 +6,7 @@ import ( "regexp" "strings" "time" + "container/list" "github.com/armon/go-metrics" "github.com/hashicorp/go-multierror" @@ -1026,26 +1027,24 @@ func (ts *TokenStore) RevokeTree(id string) error { // revokeTreeSalted is used to invalide a given token and all // child tokens using a saltedID. func (ts *TokenStore) revokeTreeSalted(saltedId string) error { - // Scan for child tokens - path := parentPrefix + saltedId + "/" - children, err := ts.view.List(path) - if err != nil { - return fmt.Errorf("failed to scan for children: %v", err) - } - - // Recursively nuke the children. The subtle nuance here is that - // we don't have the acutal ID of the child, but we have the salted - // value. Turns out, this is good enough! - for _, child := range children { - if err := ts.revokeTreeSalted(child); err != nil { - return err + //store the Id's in a list + dfs := list.New() + dfs.PushBack(saltedId) + for dfs.Front() != nil { + id := dfs.Front() + path := parentPrefix + id + "/" + children, no_child := ts.view.List(path) + if no_child != nil { + /* we have reached a leaf node, so we need to delete this + * , before the parent, i think */ + if err := ts.revokeSalted(id); err != nil { + return fmt.Errorf("failed to revoke entry: %v", err) + } + dfs.Remove(id) + } else { //there are children and we need to prepend them to the list + dfs.PushFront(children) } } - - // Revoke this entry - if err := ts.revokeSalted(saltedId); err != nil { - return fmt.Errorf("failed to revoke entry: %v", err) - } return nil } From 16eadf4e1d4c2affdd3507119bb9fb161a9ba4af Mon Sep 17 00:00:00 2001 From: lemondrank Date: Mon, 13 Mar 2017 14:52:54 -0700 Subject: [PATCH 02/23] changed a few things, still doesnt work D: --- vault/token_store.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index c7f5c292044f..79daa9428c64 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1029,18 +1029,18 @@ func (ts *TokenStore) RevokeTree(id string) error { func (ts *TokenStore) revokeTreeSalted(saltedId string) error { //store the Id's in a list dfs := list.New() - dfs.PushBack(saltedId) + dfs.PushFront(saltedId) for dfs.Front() != nil { id := dfs.Front() - path := parentPrefix + id + "/" + path := parentPrefix + id.Value + "/" children, no_child := ts.view.List(path) if no_child != nil { /* we have reached a leaf node, so we need to delete this * , before the parent, i think */ - if err := ts.revokeSalted(id); err != nil { + if err := ts.revokeSalted((string)id.Value); err != nil { return fmt.Errorf("failed to revoke entry: %v", err) } - dfs.Remove(id) + dfs.Remove(id.Value) } else { //there are children and we need to prepend them to the list dfs.PushFront(children) } From c856b8fd8e5b61b775927007ea1a2ab152bf9b26 Mon Sep 17 00:00:00 2001 From: lemondrank Date: Mon, 13 Mar 2017 14:58:44 -0700 Subject: [PATCH 03/23] fixed list type problems i think --- vault/token_store.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index 79daa9428c64..b0a8919a2fd8 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1032,15 +1032,15 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { dfs.PushFront(saltedId) for dfs.Front() != nil { id := dfs.Front() - path := parentPrefix + id.Value + "/" + path := parentPrefix + id.Value.(string) + "/" children, no_child := ts.view.List(path) if no_child != nil { /* we have reached a leaf node, so we need to delete this * , before the parent, i think */ - if err := ts.revokeSalted((string)id.Value); err != nil { + if err := ts.revokeSalted(id.Value.(string)); err != nil { return fmt.Errorf("failed to revoke entry: %v", err) } - dfs.Remove(id.Value) + dfs.Remove(id) } else { //there are children and we need to prepend them to the list dfs.PushFront(children) } From 2337ebc4862924491ebf3f746b4b6a649aa3a0de Mon Sep 17 00:00:00 2001 From: lemondrank Date: Thu, 6 Apr 2017 13:01:34 -0700 Subject: [PATCH 04/23] properly prepends children array to dfs List --- vault/token_store.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index 2bbf845adf4d..9f3ad669a520 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1,12 +1,12 @@ package vault import ( + "container/list" "encoding/json" "fmt" "regexp" "strings" "time" - "container/list" "github.com/armon/go-metrics" "github.com/hashicorp/go-multierror" @@ -1034,15 +1034,17 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { id := dfs.Front() path := parentPrefix + id.Value.(string) + "/" children, no_child := ts.view.List(path) - if no_child != nil { + if no_child != nil { /* we have reached a leaf node, so we need to delete this * , before the parent, i think */ - if err := ts.revokeSalted(id.Value.(string)); err != nil { - return fmt.Errorf("failed to revoke entry: %v", err) - } - dfs.Remove(id) + if err := ts.revokeSalted(id.Value.(string)); err != nil { + return fmt.Errorf("failed to revoke entry: %v", err) + } + dfs.Remove(id) } else { //there are children and we need to prepend them to the list - dfs.PushFront(children) + for i := 0; i < len(children); i++ { + dfs.PushFront(children[i]) + } } } return nil From 5721a6e36fda3c292e8994a0ea5d259dc7ad857b Mon Sep 17 00:00:00 2001 From: lemondrank Date: Thu, 13 Apr 2017 22:24:20 -0700 Subject: [PATCH 05/23] implemented dfs with slice instead of list --- vault/token_store.go | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index 9f3ad669a520..25cd40b04c09 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1,7 +1,6 @@ package vault import ( - "container/list" "encoding/json" "fmt" "regexp" @@ -1027,24 +1026,26 @@ func (ts *TokenStore) RevokeTree(id string) error { // revokeTreeSalted is used to invalide a given token and all // child tokens using a saltedID. func (ts *TokenStore) revokeTreeSalted(saltedId string) error { - //store the Id's in a list - dfs := list.New() - dfs.PushFront(saltedId) - for dfs.Front() != nil { - id := dfs.Front() - path := parentPrefix + id.Value.(string) + "/" - children, no_child := ts.view.List(path) - if no_child != nil { - /* we have reached a leaf node, so we need to delete this - * , before the parent, i think */ - if err := ts.revokeSalted(id.Value.(string)); err != nil { + dfs := make([]string, 0, 64) + dfs = append(dfs, saltedId) + + for l := len(dfs); l > 0; l = len(dfs) { + id := dfs[0] + path := parentPrefix + id + "/" + children, leaf := ts.view.List(path) + if leaf != nil { + /* we have reached a leaf node, so we need to delete it + * before the parent */ + if err := ts.revokeSalted(id); err != nil { return fmt.Errorf("failed to revoke entry: %v", err) } - dfs.Remove(id) - } else { //there are children and we need to prepend them to the list - for i := 0; i < len(children); i++ { - dfs.PushFront(children[i]) + if l == 1 { //we deleted last node, then return + return nil + } else { + dfs = dfs[1:l] //get rid of first } + } else { //there are children and we need to prepend them to the slice + dfs = append(children, dfs...) //append used as prepend in this way } } return nil From 5985f6b6330a719094d632c5a9a3e4c6d61c5b74 Mon Sep 17 00:00:00 2001 From: lemondrank Date: Fri, 14 Apr 2017 11:44:57 -0700 Subject: [PATCH 06/23] implemented dadgars testing for revokeTree --- vault/token_store_test.go | 79 ++++++++++++++++++++++++++------------- 1 file changed, 54 insertions(+), 25 deletions(-) diff --git a/vault/token_store_test.go b/vault/token_store_test.go index 69e5b7ca4512..c578ff33c486 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -677,41 +677,27 @@ func TestTokenStore_Revoke_Orphan(t *testing.T) { } } -func TestTokenStore_RevokeTree(t *testing.T) { - _, ts, _, _ := TestCoreWithTokenStore(t) - - ent1 := &TokenEntry{} - if err := ts.create(ent1); err != nil { - t.Fatalf("err: %v", err) - } +func TestTokenStore_RevokeTree_nonRecursive(t testing.TB, depth uint64) { - ent2 := &TokenEntry{Parent: ent1.ID} - if err := ts.create(ent2); err != nil { - t.Fatalf("err: %v", err) - } + _, ts, _, _ := TestCoreWithTokenStore(t) + root, children := buildTokenTree(t, ts, depth) + err := ts.RevokeTree("") - ent3 := &TokenEntry{Parent: ent2.ID} - if err := ts.create(ent3); err != nil { + if err.Error() != "cannot revoke blank token" { t.Fatalf("err: %v", err) } - - ent4 := &TokenEntry{Parent: ent2.ID} - if err := ts.create(ent4); err != nil { + if err != nil { t.Fatalf("err: %v", err) } - err := ts.RevokeTree("") - if err.Error() != "cannot revoke blank token" { - t.Fatalf("err: %v", err) - } - err = ts.RevokeTree(ent1.ID) + err = ts.RevokeTree(root.ID) //nuke the tree, non recursively if err != nil { t.Fatalf("err: %v", err) } - - lookup := []string{ent1.ID, ent2.ID, ent3.ID, ent4.ID} - for _, id := range lookup { - out, err := ts.Lookup(id) + //way to check if child was indeed revoked before parent?? + children = append(children, root) //append the root, so we can verify that it was revoked + for _, entry := range children { + out, err := ts.Lookup(entry.Id) if err != nil { t.Fatalf("err: %v", err) } @@ -720,6 +706,49 @@ func TestTokenStore_RevokeTree(t *testing.T) { } } } +//taken from dadgar's pull request +func BenchmarkTokenStore_RevokeTree(b *testing.B) { + benchmarks := []uint64{0, 1, 2, 4, 8, 16, 20} + for _, depth := range benchmarks { + b.Run(fmt.Sprintf("Tree of Depth %d", depth), func(b *testing.B) { + for i := 0; i < b.N; i++ { + testTokenStore_RevokeTree_Impl(b, depth) + } + }) + } +} +//taken from dadgar's pull request +func buildTokenTree(t testing.TB, ts *TokenStore, depth uint64) (root *TokenEntry, children []*TokenEntry) { + root = &TokenEntry{} + if err := ts.create(root); err != nil { + t.Fatalf("err: %v", err) + } + + frontier := []*TokenEntry{root} + current := uint64(0) + for current < depth { + next := make([]*TokenEntry, 0, 2*len(frontier)) + for _, node := range frontier { + left := &TokenEntry{Parent: node.ID} + if err := ts.create(left); err != nill { + t.Fatalf("err: %v", err) + } + + right := &TokenEntry{Parent: node.ID} + if err := ts.create(right); err != nill { + t.Fatalf("err: %v", err) + } + + children = append(children, left, right) + next = append(next, left, right) + } + frontier = next + current++ + } + + return root, children + +} func TestTokenStore_RevokeSelf(t *testing.T) { _, ts, _, _ := TestCoreWithTokenStore(t) From b6b763234b529291a7414491fae8d0ec9b04fd8b Mon Sep 17 00:00:00 2001 From: lemondrank Date: Fri, 14 Apr 2017 11:48:53 -0700 Subject: [PATCH 07/23] Fixed improper function name --- vault/token_store_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vault/token_store_test.go b/vault/token_store_test.go index c578ff33c486..ee7e1fa6990c 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -677,7 +677,7 @@ func TestTokenStore_Revoke_Orphan(t *testing.T) { } } -func TestTokenStore_RevokeTree_nonRecursive(t testing.TB, depth uint64) { +func TestTokenStore_RevokeTree_NonRecursive(t testing.TB, depth uint64) { _, ts, _, _ := TestCoreWithTokenStore(t) root, children := buildTokenTree(t, ts, depth) @@ -712,7 +712,7 @@ func BenchmarkTokenStore_RevokeTree(b *testing.B) { for _, depth := range benchmarks { b.Run(fmt.Sprintf("Tree of Depth %d", depth), func(b *testing.B) { for i := 0; i < b.N; i++ { - testTokenStore_RevokeTree_Impl(b, depth) + testTokenStore_RevokeTree_NonRecursive(b, depth) } }) } From a43756234398b3bf664ab2dedf944d61b54927f1 Mon Sep 17 00:00:00 2001 From: lemondrank Date: Fri, 14 Apr 2017 11:50:45 -0700 Subject: [PATCH 08/23] Fixed improper function name for real this time --- vault/token_store_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/token_store_test.go b/vault/token_store_test.go index ee7e1fa6990c..5bb27e88a3a2 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -712,7 +712,7 @@ func BenchmarkTokenStore_RevokeTree(b *testing.B) { for _, depth := range benchmarks { b.Run(fmt.Sprintf("Tree of Depth %d", depth), func(b *testing.B) { for i := 0; i < b.N; i++ { - testTokenStore_RevokeTree_NonRecursive(b, depth) + TestTokenStore_RevokeTree_NonRecursive(b, depth) } }) } From 352b15fa5383d5f6abd32d4616563d0deac14814 Mon Sep 17 00:00:00 2001 From: lemondrank Date: Fri, 21 Apr 2017 00:35:41 -0700 Subject: [PATCH 09/23] fixed some errors, but tests are hanging --- vault/token_store.go | 3 +-- vault/token_store_test.go | 21 +++++++++++++++------ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index fbbc576a2eae..69f97d782195 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1034,8 +1034,7 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { path := parentPrefix + id + "/" children, leaf := ts.view.List(path) if leaf != nil { - /* we have reached a leaf node, so we need to delete it - * before the parent */ + /* we have reached a leaf node, so we need to delete it */ if err := ts.revokeSalted(id); err != nil { return fmt.Errorf("failed to revoke entry: %v", err) } diff --git a/vault/token_store_test.go b/vault/token_store_test.go index 5bb27e88a3a2..edea418b0926 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -677,7 +677,11 @@ func TestTokenStore_Revoke_Orphan(t *testing.T) { } } -func TestTokenStore_RevokeTree_NonRecursive(t testing.TB, depth uint64) { +func TestTokenStore_RevokeTree(t *testing.T) { + testTokenStore_RevokeTree_NonRecursive(t ,2) +} + +func testTokenStore_RevokeTree_NonRecursive(t testing.TB, depth uint64) { _, ts, _, _ := TestCoreWithTokenStore(t) root, children := buildTokenTree(t, ts, depth) @@ -686,18 +690,21 @@ func TestTokenStore_RevokeTree_NonRecursive(t testing.TB, depth uint64) { if err.Error() != "cannot revoke blank token" { t.Fatalf("err: %v", err) } + + err = ts.RevokeTree(root.ID) + if err != nil { t.Fatalf("err: %v", err) } - err = ts.RevokeTree(root.ID) //nuke the tree, non recursively + //nuke the tree, non recursively if err != nil { t.Fatalf("err: %v", err) } //way to check if child was indeed revoked before parent?? children = append(children, root) //append the root, so we can verify that it was revoked for _, entry := range children { - out, err := ts.Lookup(entry.Id) + out, err := ts.Lookup(entry.ID) if err != nil { t.Fatalf("err: %v", err) } @@ -706,17 +713,19 @@ func TestTokenStore_RevokeTree_NonRecursive(t testing.TB, depth uint64) { } } } + //taken from dadgar's pull request func BenchmarkTokenStore_RevokeTree(b *testing.B) { benchmarks := []uint64{0, 1, 2, 4, 8, 16, 20} for _, depth := range benchmarks { b.Run(fmt.Sprintf("Tree of Depth %d", depth), func(b *testing.B) { for i := 0; i < b.N; i++ { - TestTokenStore_RevokeTree_NonRecursive(b, depth) + testTokenStore_RevokeTree_NonRecursive(b, depth) } }) } } + //taken from dadgar's pull request func buildTokenTree(t testing.TB, ts *TokenStore, depth uint64) (root *TokenEntry, children []*TokenEntry) { root = &TokenEntry{} @@ -730,12 +739,12 @@ func buildTokenTree(t testing.TB, ts *TokenStore, depth uint64) (root *TokenEntr next := make([]*TokenEntry, 0, 2*len(frontier)) for _, node := range frontier { left := &TokenEntry{Parent: node.ID} - if err := ts.create(left); err != nill { + if err := ts.create(left); err != nil { t.Fatalf("err: %v", err) } right := &TokenEntry{Parent: node.ID} - if err := ts.create(right); err != nill { + if err := ts.create(right); err != nil { t.Fatalf("err: %v", err) } From 5ff2a92694a4e65013e7029ffa453f278ef66854 Mon Sep 17 00:00:00 2001 From: lemondrank Date: Fri, 21 Apr 2017 12:28:54 -0700 Subject: [PATCH 10/23] fixed hanging problem, still need to clean up comments --- vault/token_store.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index 69f97d782195..19f3217f5ab6 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1032,8 +1032,11 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { for l := len(dfs); l > 0; l = len(dfs) { id := dfs[0] path := parentPrefix + id + "/" - children, leaf := ts.view.List(path) - if leaf != nil { + children, err := ts.view.List(path) + if err != nil { + return fmt.Errorf("failed to scan for children: %v", err) + } + if len(children) == 0 { //len(chil) == 0 /* we have reached a leaf node, so we need to delete it */ if err := ts.revokeSalted(id); err != nil { return fmt.Errorf("failed to revoke entry: %v", err) From 5b5fef5aedecf6cf4e3bddf62f57676862c095aa Mon Sep 17 00:00:00 2001 From: lemondrank Date: Sun, 23 Apr 2017 14:52:40 -0700 Subject: [PATCH 11/23] cleaned up comments, should be ready for a review. --- vault/token_store.go | 14 ++++++++++---- vault/token_store_test.go | 23 +++++++++++++---------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index 19f3217f5ab6..44cc1711437c 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1025,7 +1025,10 @@ func (ts *TokenStore) RevokeTree(id string) error { // revokeTreeSalted is used to invalide a given token and all // child tokens using a saltedID. +// Updated to be non-recursive and revoke child tokens +// before parent tokens(DFS). func (ts *TokenStore) revokeTreeSalted(saltedId string) error { + //Initialize slice to be arbitraty capacity of 64 dfs := make([]string, 0, 64) dfs = append(dfs, saltedId) @@ -1036,8 +1039,9 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { if err != nil { return fmt.Errorf("failed to scan for children: %v", err) } - if len(children) == 0 { //len(chil) == 0 - /* we have reached a leaf node, so we need to delete it */ + //If the length of the children is equal to zero, + //then we are at a leaf node. + if len(children) == 0 { if err := ts.revokeSalted(id); err != nil { return fmt.Errorf("failed to revoke entry: %v", err) } @@ -1046,8 +1050,10 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { } else { dfs = dfs[1:l] //get rid of first } - } else { //there are children and we need to prepend them to the slice - dfs = append(children, dfs...) //append used as prepend in this way + } else { + //If we make it here, the there are children and they must + // be prepended. + dfs = append(children, dfs...) } } return nil diff --git a/vault/token_store_test.go b/vault/token_store_test.go index edea418b0926..1ec431153abe 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -677,10 +677,14 @@ func TestTokenStore_Revoke_Orphan(t *testing.T) { } } +// This was the original function name, and now it just calls +// the non recursive version. func TestTokenStore_RevokeTree(t *testing.T) { testTokenStore_RevokeTree_NonRecursive(t ,2) } - + +// Revokes a given Token Store tree non recursively. +// The second parameter refers to the depth of the tree. func testTokenStore_RevokeTree_NonRecursive(t testing.TB, depth uint64) { _, ts, _, _ := TestCoreWithTokenStore(t) @@ -691,18 +695,15 @@ func testTokenStore_RevokeTree_NonRecursive(t testing.TB, depth uint64) { t.Fatalf("err: %v", err) } + // Nuke tree non recursively. err = ts.RevokeTree(root.ID) if err != nil { t.Fatalf("err: %v", err) } - - //nuke the tree, non recursively - if err != nil { - t.Fatalf("err: %v", err) - } - //way to check if child was indeed revoked before parent?? - children = append(children, root) //append the root, so we can verify that it was revoked + // Append the root to ensure it was successfully + // deleted. + children = append(children, root) for _, entry := range children { out, err := ts.Lookup(entry.ID) if err != nil { @@ -714,7 +715,8 @@ func testTokenStore_RevokeTree_NonRecursive(t testing.TB, depth uint64) { } } -//taken from dadgar's pull request +// A benchmark function that tests testTokenStore_RevokeTree_NonRecursive +// for a variety of different depths. func BenchmarkTokenStore_RevokeTree(b *testing.B) { benchmarks := []uint64{0, 1, 2, 4, 8, 16, 20} for _, depth := range benchmarks { @@ -726,7 +728,8 @@ func BenchmarkTokenStore_RevokeTree(b *testing.B) { } } -//taken from dadgar's pull request +// Builds a TokenTree of a specified depth, so that +// we may run revoke tests on it. func buildTokenTree(t testing.TB, ts *TokenStore, depth uint64) (root *TokenEntry, children []*TokenEntry) { root = &TokenEntry{} if err := ts.create(root); err != nil { From 0ad78e1f275f3c185f29d0726240da541d225b72 Mon Sep 17 00:00:00 2001 From: lemondrank Date: Sun, 23 Apr 2017 14:55:12 -0700 Subject: [PATCH 12/23] cleaned up comments for real this time --- vault/token_store.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index 44cc1711437c..e990e9faa979 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1028,7 +1028,7 @@ func (ts *TokenStore) RevokeTree(id string) error { // Updated to be non-recursive and revoke child tokens // before parent tokens(DFS). func (ts *TokenStore) revokeTreeSalted(saltedId string) error { - //Initialize slice to be arbitraty capacity of 64 + // Initialize slice to be arbitraty capacity of 64 dfs := make([]string, 0, 64) dfs = append(dfs, saltedId) @@ -1039,8 +1039,8 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { if err != nil { return fmt.Errorf("failed to scan for children: %v", err) } - //If the length of the children is equal to zero, - //then we are at a leaf node. + // If the length of the children is equal to zero, + // then we are at a leaf node. if len(children) == 0 { if err := ts.revokeSalted(id); err != nil { return fmt.Errorf("failed to revoke entry: %v", err) @@ -1051,7 +1051,7 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { dfs = dfs[1:l] //get rid of first } } else { - //If we make it here, the there are children and they must + // If we make it here, the there are children and they must // be prepended. dfs = append(children, dfs...) } From 87fc97b8d3e57c0a082fabfb8f3573674885f56d Mon Sep 17 00:00:00 2001 From: lemondrank Date: Sun, 23 Apr 2017 22:42:44 -0700 Subject: [PATCH 13/23] clean up --- vault/token_store.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index e990e9faa979..dd9651b4bc3b 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1023,7 +1023,7 @@ func (ts *TokenStore) RevokeTree(id string) error { return nil } -// revokeTreeSalted is used to invalide a given token and all +// revokeTreeSalted is used to invalidate a given token and all // child tokens using a saltedID. // Updated to be non-recursive and revoke child tokens // before parent tokens(DFS). @@ -1045,10 +1045,11 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { if err := ts.revokeSalted(id); err != nil { return fmt.Errorf("failed to revoke entry: %v", err) } - if l == 1 { //we deleted last node, then return + // If the length of l is equal to 1, then the last token has been deleted + if l == 1 { return nil } else { - dfs = dfs[1:l] //get rid of first + dfs = dfs[1:l] } } else { // If we make it here, the there are children and they must From 5157969b09a31bf06699751f4c606218f162541c Mon Sep 17 00:00:00 2001 From: lemondrank Date: Thu, 27 Apr 2017 20:57:29 -0700 Subject: [PATCH 14/23] commit --- vault/token_store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/token_store.go b/vault/token_store.go index f7c0a61eead1..dd9651b4bc3b 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1131,7 +1131,7 @@ func (ts *TokenStore) handleTidy(req *logical.Request, data *framework.FieldData // Scan through the secondary index entries; if there is an entry // with the token's salt ID at the end, remove it for _, parent := range parentList { - children, err := ts.view.List(parentPrefix + parent + "/") + children, err := ts.view.List(parentPrefix + parent) if err != nil { tidyErrors = multierror.Append(tidyErrors, fmt.Errorf("failed to read child index entry: %v", err)) continue From 2b0cd93402f25f635d169b4bbf93049294f706e4 Mon Sep 17 00:00:00 2001 From: lemondrank Date: Thu, 27 Apr 2017 21:06:07 -0700 Subject: [PATCH 15/23] ran gofmt, not sure if it worked --- vault/token_store.go | 2 +- vault/token_store_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index edc96fb677fb..3fe0acd8aaf1 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1052,7 +1052,7 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { dfs = dfs[1:l] } } else { - // If we make it here, the there are children and they must + // If we make it here, there are children and they must // be prepended. dfs = append(children, dfs...) } diff --git a/vault/token_store_test.go b/vault/token_store_test.go index 5db0d4b557e9..7adaa0e48730 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -686,7 +686,6 @@ func TestTokenStore_RevokeTree(t *testing.T) { // Revokes a given Token Store tree non recursively. // The second parameter refers to the depth of the tree. func testTokenStore_RevokeTree_NonRecursive(t testing.TB, depth uint64) { - _, ts, _, _ := TestCoreWithTokenStore(t) root, children := buildTokenTree(t, ts, depth) err := ts.RevokeTree("") From 947b9b99ca3f024d1976a21458db5e5f2c89dcc7 Mon Sep 17 00:00:00 2001 From: lemondrank Date: Thu, 27 Apr 2017 21:12:09 -0700 Subject: [PATCH 16/23] hopefully formatted --- vault/token_store.go | 20 ++++++++++---------- vault/token_store_test.go | 20 ++++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index 3fe0acd8aaf1..a4391522c8b0 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1028,7 +1028,7 @@ func (ts *TokenStore) RevokeTree(id string) error { // Updated to be non-recursive and revoke child tokens // before parent tokens(DFS). func (ts *TokenStore) revokeTreeSalted(saltedId string) error { - // Initialize slice to be arbitraty capacity of 64 + // Initialize slice to be arbitraty capacity of 64 dfs := make([]string, 0, 64) dfs = append(dfs, saltedId) @@ -1037,23 +1037,23 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { path := parentPrefix + id + "/" children, err := ts.view.List(path) if err != nil { - return fmt.Errorf("failed to scan for children: %v", err) - } - // If the length of the children is equal to zero, - // then we are at a leaf node. + return fmt.Errorf("failed to scan for children: %v", err) + } + // If the length of the children is equal to zero, + // then we are at a leaf node. if len(children) == 0 { if err := ts.revokeSalted(id); err != nil { return fmt.Errorf("failed to revoke entry: %v", err) } // If the length of l is equal to 1, then the last token has been deleted - if l == 1 { + if l == 1 { return nil } else { - dfs = dfs[1:l] + dfs = dfs[1:l] } - } else { - // If we make it here, there are children and they must - // be prepended. + } else { + // If we make it here, there are children and they must + // be prepended. dfs = append(children, dfs...) } } diff --git a/vault/token_store_test.go b/vault/token_store_test.go index 7adaa0e48730..c69a91b943de 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -677,10 +677,10 @@ func TestTokenStore_Revoke_Orphan(t *testing.T) { } } -// This was the original function name, and now it just calls +// This was the original function name, and now it just calls // the non recursive version. func TestTokenStore_RevokeTree(t *testing.T) { - testTokenStore_RevokeTree_NonRecursive(t ,2) + testTokenStore_RevokeTree_NonRecursive(t, 2) } // Revokes a given Token Store tree non recursively. @@ -693,15 +693,15 @@ func testTokenStore_RevokeTree_NonRecursive(t testing.TB, depth uint64) { if err.Error() != "cannot tree-revoke blank token" { t.Fatalf("err: %v", err) } - - // Nuke tree non recursively. + + // Nuke tree non recursively. err = ts.RevokeTree(root.ID) - + if err != nil { t.Fatalf("err: %v", err) } - // Append the root to ensure it was successfully - // deleted. + // Append the root to ensure it was successfully + // deleted. children = append(children, root) for _, entry := range children { out, err := ts.Lookup(entry.ID) @@ -715,7 +715,7 @@ func testTokenStore_RevokeTree_NonRecursive(t testing.TB, depth uint64) { } // A benchmark function that tests testTokenStore_RevokeTree_NonRecursive -// for a variety of different depths. +// for a variety of different depths. func BenchmarkTokenStore_RevokeTree(b *testing.B) { benchmarks := []uint64{0, 1, 2, 4, 8, 16, 20} for _, depth := range benchmarks { @@ -727,8 +727,8 @@ func BenchmarkTokenStore_RevokeTree(b *testing.B) { } } -// Builds a TokenTree of a specified depth, so that -// we may run revoke tests on it. +// Builds a TokenTree of a specified depth, so that +// we may run revoke tests on it. func buildTokenTree(t testing.TB, ts *TokenStore, depth uint64) (root *TokenEntry, children []*TokenEntry) { root = &TokenEntry{} if err := ts.create(root); err != nil { From bebb56b20c4b85bbd0dd83f47049a9f472b04bed Mon Sep 17 00:00:00 2001 From: lemondrank Date: Fri, 28 Apr 2017 12:12:22 -0700 Subject: [PATCH 17/23] fixed handleTidy issue --- vault/token_store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/token_store.go b/vault/token_store.go index a4391522c8b0..17c674acae86 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1131,7 +1131,7 @@ func (ts *TokenStore) handleTidy(req *logical.Request, data *framework.FieldData // Scan through the secondary index entries; if there is an entry // with the token's salt ID at the end, remove it for _, parent := range parentList { - children, err := ts.view.List(parentPrefix + parent) + children, err := ts.view.List(parentPrefix + parent + "/") if err != nil { tidyErrors = multierror.Append(tidyErrors, fmt.Errorf("failed to read child index entry: %v", err)) continue From d30a433836e5e0af480ace16b53851dea69d98bd Mon Sep 17 00:00:00 2001 From: lemondrank Date: Fri, 5 May 2017 02:00:25 -0700 Subject: [PATCH 18/23] added variety of depths for testing --- vault/token_store.go | 3 ++- vault/token_store_test.go | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index 17c674acae86..7b20b3e9228c 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1039,7 +1039,7 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { if err != nil { return fmt.Errorf("failed to scan for children: %v", err) } - // If the length of the children is equal to zero, + // If the length of the children array is zero, // then we are at a leaf node. if len(children) == 0 { if err := ts.revokeSalted(id); err != nil { @@ -1057,6 +1057,7 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { dfs = append(children, dfs...) } } + return nil } diff --git a/vault/token_store_test.go b/vault/token_store_test.go index c69a91b943de..f2fce44a0c41 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -678,9 +678,11 @@ func TestTokenStore_Revoke_Orphan(t *testing.T) { } // This was the original function name, and now it just calls -// the non recursive version. +// the non recursive version for a variety of depths. func TestTokenStore_RevokeTree(t *testing.T) { + testTokenStore_RevokeTree_NonRecursive(t, 1) testTokenStore_RevokeTree_NonRecursive(t, 2) + testTokenStore_RevokeTree_NonRecursive(t, 20) } // Revokes a given Token Store tree non recursively. @@ -758,7 +760,6 @@ func buildTokenTree(t testing.TB, ts *TokenStore, depth uint64) (root *TokenEntr } return root, children - } func TestTokenStore_RevokeSelf(t *testing.T) { From becaacb4c32ba3011823e8c71f44b34437aebfee Mon Sep 17 00:00:00 2001 From: lemondrank Date: Fri, 5 May 2017 11:24:19 -0700 Subject: [PATCH 19/23] formatting --- vault/token_store.go | 2 +- vault/token_store_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index 4f220d6fb845..56de40ba92ae 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1057,7 +1057,7 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { dfs = append(children, dfs...) } } - + return nil } diff --git a/vault/token_store_test.go b/vault/token_store_test.go index 8277ccbf88e1..d1b7f157d560 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -680,7 +680,7 @@ func TestTokenStore_Revoke_Orphan(t *testing.T) { // This was the original function name, and now it just calls // the non recursive version for a variety of depths. func TestTokenStore_RevokeTree(t *testing.T) { - testTokenStore_RevokeTree_NonRecursive(t, 1) + testTokenStore_RevokeTree_NonRecursive(t, 1) testTokenStore_RevokeTree_NonRecursive(t, 2) testTokenStore_RevokeTree_NonRecursive(t, 20) } From 4e6a83080241b8e4fef459bebb4e432891645b19 Mon Sep 17 00:00:00 2001 From: lemondrank Date: Sat, 6 May 2017 11:38:39 -0700 Subject: [PATCH 20/23] changed max testing depth to 10 --- vault/token_store_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/token_store_test.go b/vault/token_store_test.go index d1b7f157d560..9b467e54b157 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -682,7 +682,7 @@ func TestTokenStore_Revoke_Orphan(t *testing.T) { func TestTokenStore_RevokeTree(t *testing.T) { testTokenStore_RevokeTree_NonRecursive(t, 1) testTokenStore_RevokeTree_NonRecursive(t, 2) - testTokenStore_RevokeTree_NonRecursive(t, 20) + testTokenStore_RevokeTree_NonRecursive(t, 10) } // Revokes a given Token Store tree non recursively. From c9f7d82465053b84a8f0d1c3c1109f231e50b80d Mon Sep 17 00:00:00 2001 From: lemondrank Date: Tue, 9 May 2017 12:27:25 -0700 Subject: [PATCH 21/23] checking if build will pass for depth 2 --- vault/token_store_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/token_store_test.go b/vault/token_store_test.go index 6a140a6cca66..a364369f2e34 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -682,7 +682,7 @@ func TestTokenStore_Revoke_Orphan(t *testing.T) { func TestTokenStore_RevokeTree(t *testing.T) { testTokenStore_RevokeTree_NonRecursive(t, 1) testTokenStore_RevokeTree_NonRecursive(t, 2) - testTokenStore_RevokeTree_NonRecursive(t, 10) + //testTokenStore_RevokeTree_NonRecursive(t, 10) } // Revokes a given Token Store tree non recursively. From 87c943b0bcdda1778e85ce645e6ddada077d43e8 Mon Sep 17 00:00:00 2001 From: lemondrank Date: Wed, 10 May 2017 11:12:24 -0700 Subject: [PATCH 22/23] trying again with depth 10 --- vault/token_store_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/token_store_test.go b/vault/token_store_test.go index a364369f2e34..6a140a6cca66 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -682,7 +682,7 @@ func TestTokenStore_Revoke_Orphan(t *testing.T) { func TestTokenStore_RevokeTree(t *testing.T) { testTokenStore_RevokeTree_NonRecursive(t, 1) testTokenStore_RevokeTree_NonRecursive(t, 2) - //testTokenStore_RevokeTree_NonRecursive(t, 10) + testTokenStore_RevokeTree_NonRecursive(t, 10) } // Revokes a given Token Store tree non recursively. From b121dfb237eae177716510c90a7c09d6798507d1 Mon Sep 17 00:00:00 2001 From: lemondrank Date: Fri, 12 May 2017 13:49:04 -0700 Subject: [PATCH 23/23] made requested changes, should be good to go --- vault/token_store.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/vault/token_store.go b/vault/token_store.go index 809819669e9f..fac329472882 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1038,8 +1038,7 @@ func (ts *TokenStore) RevokeTree(id string) error { // Updated to be non-recursive and revoke child tokens // before parent tokens(DFS). func (ts *TokenStore) revokeTreeSalted(saltedId string) error { - // Initialize slice to be arbitraty capacity of 64 - dfs := make([]string, 0, 64) + var dfs []string dfs = append(dfs, saltedId) for l := len(dfs); l > 0; l = len(dfs) { @@ -1058,9 +1057,8 @@ func (ts *TokenStore) revokeTreeSalted(saltedId string) error { // If the length of l is equal to 1, then the last token has been deleted if l == 1 { return nil - } else { - dfs = dfs[1:l] } + dfs = dfs[1:] } else { // If we make it here, there are children and they must // be prepended.