test/e2e/auth: fix audit log test format parsing #51560
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
size/S
k8s-github-robot
added
approved
liggitt
reviewed
Aug 29, 2017
test/e2e/auth/audit.go
Outdated
| @@ -138,6 +139,26 @@ func expectAuditLines(f *framework.Framework, expected []auditEvent) { | |||
| } | |||
|
|
|||
| func parseAuditLine(line string) (auditEvent, error) { | |||
| var e struct { | |||
There was a problem hiding this comment.
Is this struct not already defined in API types we can reference?
There was a problem hiding this comment.
Updated to use the audit Event type.
ericchiang
force-pushed
the
fix-audit-log-test
branch
from
087b3da
to
77de774
Compare
ericchiang
commented
Aug 30, 2017
| @@ -126,9 +124,6 @@ func expectAuditLines(f *framework.Framework, expected []auditEvent) { | |||
| if _, found := expectations[event]; found { | |||
| expectations[event] = true | |||
| } | |||
|
|
|||
| // /version should not be audited (filtered in the policy). | |||
| Expect(event.uri).NotTo(HavePrefix("/version")) | |||
There was a problem hiding this comment.
Removed since non-resource URL requests are now tracked.
|
Managed to run this locally with some local-up modifications and modifying the test to read a file on disk |
|
LGTM. @sttts? |
ericchiang
force-pushed
the
fix-audit-log-test
branch
from
77de774
to
2166668
Compare
|
If we are in a hurry to fix this we can add |
|
I think this fix is fine. It supports both formats now, so it'll work regardless of the setup, which seems more ideal. |
Agree! Thanks. |
|
/test pull-kubernetes-e2e-gce-bazel |
ericchiang
added
the
kind/failing-test
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ericchiang, sttts Associated issue: 51556 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
Automatic merge from submit-queue (batch tested with PRs 51632, 51055, 51676, 51560, 50007) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #51556
cc @CaoShuFeng
Still need to figure out how to run this test locally.