Fix audit_test regex for iso8601 timestamps #32593

johnbieren · 2016-09-13T19:07:37Z

What this PR does / why we need it: The audit_test unit test fails as some iso8601 timestamps are of the form 2016-09-13T10:32:50.823081217Z and the current regex doesn't allow T's or Z's.

Which issue this PR fixes (optional, in fixes #<issue number>(, #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release note:NONE

Fix audit_test regex for iso8601 timestamps

Signed-off-by: Johnny Bieren jbieren@redhat.com

This change is

Signed-off-by: Johnny Bieren <jbieren@redhat.com>

k8s-ci-robot · 2016-09-13T19:07:38Z

Can a kubernetes member verify that this patch is reasonable to test? If so, please reply with "ok to test" on its own line.

Regular contributors should join the org to skip this step.

While we transition away from the Jenkins GitHub PR Builder plugin, "ok to test" commenters will need to be on the admin list defined in this file.

googlebot · 2016-09-13T19:07:39Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

johnbieren · 2016-09-13T19:30:45Z

I signed it!

googlebot · 2016-09-14T11:49:11Z

CLAs look good, thanks!

yoojinl · 2016-09-14T15:35:40Z

pkg/apiserver/audit/audit_test.go

@@ -95,14 +95,14 @@ func TestAudit(t *testing.T) {
 	if len(line) != 2 {
 		t.Fatalf("Unexpected amount of lines in audit log: %d", len(line))
 	}
-	match, err := regexp.MatchString(`[\d\:\-\.\+]+ AUDIT: id="[\w-]+" ip="127.0.0.1" method="GET" user="admin" as="<self>" namespace="default" uri="/api/v1/namespaces/default/pods"`, line[0])
+	match, err := regexp.MatchString(`[\d\:\-\.\+,T,Z]+ AUDIT: id="[\w-]+" ip="127.0.0.1" method="GET" user="admin" as="<self>" namespace="default" uri="/api/v1/namespaces/default/pods"`, line[0])


I think it should be just [\d\:\-\.\+TZ], otherwise it also matches , symbol.

Nice catch, fixed it

Signed-off-by: Johnny Bieren <jbieren@redhat.com>

nikhiljindal · 2016-09-14T22:50:34Z

@k8s-bot ok to test

Is there an issue filed for failing test?

k8s-bot · 2016-09-14T23:30:19Z

GCE e2e build/test passed for commit df6299d.

k8s-github-robot · 2016-09-15T06:47:22Z

Automatic merge from submit-queue

johnbieren · 2016-09-15T11:08:09Z

@nikhiljindal No, I did not open an issue for the failing test, just this PR

ivan4th · 2016-09-15T11:38:28Z

Can we have this cherry-picked into release-1.4 branch? We're having some problems with our CI due to the bug this PR fixes

pigmej · 2016-09-15T11:41:47Z

@ivan4th it's not only about CI, 1.4 release is affected with this bug :)

cc @nikhiljindal @johnbieren

pigmej · 2016-09-21T12:20:50Z

Pinging again, can we cherry-pick it for 1.4?

@nikhiljindal @johnbieren

zen · 2016-09-22T09:29:08Z

This bug affects almost anyone running custom CI for k8s, it would be very good to have it merged to 1.4. I'd like to understand what needs to happen to have it picked for the release.

idvoretskyi · 2016-09-23T09:17:03Z

@pwittrock this issue looks critical for some instances, while it hasn't been added to 1.4 branch yet (while it is important).

nikhiljindal · 2016-09-23T17:42:29Z

Sorry for not replying earlier.
Yes @pwittrock is the right person to make that call.

I'd like to understand what needs to happen to have it picked for the release.

The process is: add the PR to be cherrypicked to the relevant milestone and add the "cherrypick-candidate" label.
Release czar will then either remove the "cherrypick-candidate" label or add a "cherrypick-approved" label.
Adding the milestone and label for you guys.

pwittrock · 2016-09-23T17:48:51Z

Sounds like the ball got dropped here. I have repeatedly communicated both at the community meeting and to kubernetes-dev@googlegroups.com that:

Last call for cherrypick changes to be merged was on Wednesday
If this is a surprise or issue for anyone to reach out to me immediately on slack

Why can't this wait for 1.4.1 and we add an item to known issues?

pwittrock · 2016-09-23T17:50:45Z

Looking at the code, this only updates a test file. How does the impact CI?

pwittrock · 2016-09-23T18:02:08Z

@nikhiljindal We are past the point of doing batch cherrypicks. You will need to do this one yourself.

pwittrock · 2016-09-23T20:36:16Z

Need more context on this if it is going to go in. Seems like the regex adds a piece that isn't optional. How do we know this won't break anything else?

nikhiljindal · 2016-09-23T20:55:23Z

@nikhiljindal We are past the point of doing batch cherrypicks. You will need to do this one yourself.

Sorry I dont want this to be blocked on me. I have some other 1.4 work am involved in. Anyone else willing to step up? You will have to convince pwittrock and then send a PR cherrypicking this to release-1.4 branch.

idvoretskyi · 2016-09-23T23:30:47Z

@pwittrock I'm not pushing this item to be cherrypicked urgently, I'm notifying about this issue that is critical for some users and we have to solve it in the best possible way.
1.4.1 branch cherrypicking SGTM.

pwittrock · 2016-09-23T23:35:52Z

Alright sounds good.

jessfraz · 2016-10-06T00:50:31Z

@nikhiljindal @idvoretskyi can one of you please open the PR to cherry-pick this into release-1.4, thanks!

pigmej · 2016-10-06T12:46:55Z

@jessfraz I submitted cherry pick #34228

…-upstream-release-1.4 Automatic merge from submit-queue Automated cherry pick of #32593 Cherry pick of #32593 on release-1.4. #32593: Subject: [PATCH 1/2] Fix audit_test regex for iso8601 timestamps Subject: [PATCH 2/2] Fixed edited regex in audit_test unit test

k8s-cherrypick-bot · 2016-10-07T02:38:25Z

Commit found in the "release-1.4" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

Automatic merge from submit-queue hack: update cherry-pick script to show subject when patch is split  **What this PR does / why we need it**: This is a continuation of #34049, which worked, but then I realized some patch files have more than one subject, see example: #34228 This cleans the output so it looks like this: ``` Automated cherry pick of #32593 Cherry pick of #32593 on release-1.4. #32593: Fix audit_test regex for iso8601 timestamps ``` pretty!!! **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes # **Special notes for your reviewer**: **Release note**:  Signed-off-by: Jess Frazelle <acidburn@google.com>

…-of-#32593-upstream-release-1.4 Automatic merge from submit-queue Automated cherry pick of kubernetes#32593 Cherry pick of kubernetes#32593 on release-1.4. kubernetes#32593: Subject: [PATCH 1/2] Fix audit_test regex for iso8601 timestamps Subject: [PATCH 2/2] Fixed edited regex in audit_test unit test

Fix audit_test regex for iso8601 timestamps

6194bfb

Signed-off-by: Johnny Bieren <jbieren@redhat.com>

googlebot added the cla: no label Sep 13, 2016

k8s-github-robot assigned nikhiljindal Sep 13, 2016

k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed labels Sep 13, 2016

ncdc added the release-note-none Denotes a PR that doesn't merit a release note. label Sep 13, 2016

k8s-github-robot removed the release-note-label-needed label Sep 13, 2016

googlebot added cla: yes and removed cla: no labels Sep 14, 2016

yoojinl reviewed Sep 14, 2016
View reviewed changes

Fixed edited regex in audit_test unit test

df6299d

Signed-off-by: Johnny Bieren <jbieren@redhat.com>

nikhiljindal added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 14, 2016

wojtek-t added the retest-not-required label Sep 15, 2016

k8s-github-robot merged commit 1ad5c5e into kubernetes:master Sep 15, 2016

nikhiljindal added the cherrypick-candidate label Sep 23, 2016

nikhiljindal added this to the v1.4 milestone Sep 23, 2016

pwittrock added the 1.4.1 label Sep 23, 2016

pwittrock added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Sep 23, 2016

pwittrock removed the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Sep 23, 2016

jessfraz added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Oct 6, 2016

pigmej mentioned this pull request Oct 6, 2016

Automated cherry pick of #32593 #34228

Merged

jessfraz added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Oct 6, 2016

k8s-cherrypick-bot removed the cherrypick-candidate label Oct 7, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix audit_test regex for iso8601 timestamps #32593

Fix audit_test regex for iso8601 timestamps #32593

johnbieren commented Sep 13, 2016 •

edited by jessfraz

Loading

k8s-ci-robot commented Sep 13, 2016

googlebot commented Sep 13, 2016

johnbieren commented Sep 13, 2016 •

edited

Loading

googlebot commented Sep 14, 2016

yoojinl Sep 14, 2016

johnbieren Sep 14, 2016

nikhiljindal commented Sep 14, 2016

k8s-bot commented Sep 14, 2016

k8s-github-robot commented Sep 15, 2016

johnbieren commented Sep 15, 2016

ivan4th commented Sep 15, 2016

pigmej commented Sep 15, 2016

pigmej commented Sep 21, 2016

zen commented Sep 22, 2016

idvoretskyi commented Sep 23, 2016

nikhiljindal commented Sep 23, 2016

pwittrock commented Sep 23, 2016

pwittrock commented Sep 23, 2016

pwittrock commented Sep 23, 2016

pwittrock commented Sep 23, 2016

nikhiljindal commented Sep 23, 2016

idvoretskyi commented Sep 23, 2016

pwittrock commented Sep 23, 2016

jessfraz commented Oct 6, 2016

pigmej commented Oct 6, 2016

k8s-cherrypick-bot commented Oct 7, 2016

Fix audit_test regex for iso8601 timestamps #32593

Fix audit_test regex for iso8601 timestamps #32593

Conversation

johnbieren commented Sep 13, 2016 • edited by jessfraz Loading

k8s-ci-robot commented Sep 13, 2016

googlebot commented Sep 13, 2016

johnbieren commented Sep 13, 2016 • edited Loading

googlebot commented Sep 14, 2016

yoojinl Sep 14, 2016

Choose a reason for hiding this comment

johnbieren Sep 14, 2016

Choose a reason for hiding this comment

nikhiljindal commented Sep 14, 2016

k8s-bot commented Sep 14, 2016

k8s-github-robot commented Sep 15, 2016

johnbieren commented Sep 15, 2016

ivan4th commented Sep 15, 2016

pigmej commented Sep 15, 2016

pigmej commented Sep 21, 2016

zen commented Sep 22, 2016

idvoretskyi commented Sep 23, 2016

nikhiljindal commented Sep 23, 2016

pwittrock commented Sep 23, 2016

pwittrock commented Sep 23, 2016

pwittrock commented Sep 23, 2016

pwittrock commented Sep 23, 2016

nikhiljindal commented Sep 23, 2016

idvoretskyi commented Sep 23, 2016

pwittrock commented Sep 23, 2016

jessfraz commented Oct 6, 2016

pigmej commented Oct 6, 2016

k8s-cherrypick-bot commented Oct 7, 2016

johnbieren commented Sep 13, 2016 •

edited by jessfraz

Loading

johnbieren commented Sep 13, 2016 •

edited

Loading