-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Non-recursive DFS token tree revoke #2478
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking pretty good, i left just a few comments. I'd like to take another look after the code is formatted properly.
Also you have a merge conflict, so you'll need to merge master into this branch.
vault/token_store.go
Outdated
children, err := ts.view.List(path) | ||
if err != nil { | ||
return fmt.Errorf("failed to scan for children: %v", err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you run go fmt on your code? https://golang.org/cmd/gofmt/
vault/token_store.go
Outdated
dfs = dfs[1:l] | ||
} | ||
} else { | ||
// If we make it here, the there are children and they must |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo on "the there"
vault/token_store.go
Outdated
@@ -1120,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 + "/") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How come this changed?
LGTM! |
sweet! thanks for all the help! |
I'll try to get to a review soon but adding to 0.7.3. |
sounds great! thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
fixes #2348