authz: Stdout logger #6230
authz: Stdout logger #6230
Conversation
gtcooke94
left a comment
There was a problem hiding this comment.
Good start, I think we definitely want to split this into a PR for the stdout logger and a PR for XDS registry and parsing, then title them accordingly
rockspore
left a comment
There was a problem hiding this comment.
Thanks for the PR. Left some comments. Also suggest simply some variable names following Go's style. Please follow existing code in the repo to do logging instead of fmt.Println
gtcooke94
left a comment
There was a problem hiding this comment.
Looks good! Have a few comments and some golang convention items to change
gtcooke94
left a comment
There was a problem hiding this comment.
Looks good! Have a few comments and some golang convention items to change
|
@erm-g Please don't mark comment threads as resolved. The policy we follow is that the reviewer marks them as resolved, once they feel that the comment has been appropriately addressed. It's a pain to unresolve every comment and see if they have been addressed appropriately. Thanks. |
…ger struct func. Add timestamp testing logic. Add registry presense test.
|
@erm-g : Looks like there are more comments from the security team here. Please let me know once you have addressed all of them, and want me to take another pass. Thanks. |
rockspore
left a comment
There was a problem hiding this comment.
One nit was not resolved yet. The rest LGTM! Thanks.
| l := log.New(&buf, "", 0) | ||
| builder := &loggerBuilder{goLogger: l} |
There was a problem hiding this comment.
Nit: inline l into the next line.
| builder := &loggerBuilder{goLogger: l} | ||
| auditLogger := builder.Build(nil) | ||
| auditLogger.Log(test.event) | ||
| var container map[string]interface{} |
There was a problem hiding this comment.
Nit: Consider adding newlines to logically separate the steps of the test. At the very least, it would be good to separate out the following steps using newlines: setup, actual test logic, verification.
|
LGTM. Some very minor nits. Feel free to merge without another pass from me. |
Part of RBAC audit logging effort for authz. The functionality in this PR includes a built in StdOut logger (prints the whole Event to stdout in json format)
RELEASE NOTES: