-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
HADOOP-17476. ITestAssumeRole.testAssumeRoleBadInnerAuth failure. #2777
HADOOP-17476. ITestAssumeRole.testAssumeRoleBadInnerAuth failure. #2777
Conversation
Change-Id: Ia4daa2cd0d552979d96b1475e01fb83ea9145e25
ran test case against S3 london; all good
|
🎊 +1 overall
This message was automatically generated. |
This is ready to be reviewed @mukund-thakur |
@@ -255,8 +255,7 @@ public void testAssumeRoleBadInnerAuth() throws Exception { | |||
conf.set(SECRET_KEY, "not secret"); | |||
expectFileSystemCreateFailure(conf, | |||
AWSBadRequestException.class, | |||
"not a valid " + | |||
"key=value pair (missing equal-sign) in Authorization header"); | |||
"IncompleteSignature"); |
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.
As per the commit message "Removes string match so change in AWS S3 error message doesn't cause the test to fail", the complete string matching is to be removed right?
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.
yes. They've changed the string, and rather than try and keep up with whatever error message comes back, I'll just remove the match. We had to do the same with some of the assumed role tests a few releases back when they changed the max life of a role token from 60 minutes to 12h.
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.
I though you said you will completely remove the string matching. But we are still trying to match "IncompleteSignature"
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.
sorry, I was confused. it's looking for the structured bit of the error message
@mukund-thakur any further comments? |
* Select is spanned properly * HttpReferrer adds dynamic eval of current thread. * Every entry point in S3A code which creates a span is tagged @AuditSpan.AuditEntryPoint. This is to hint what is/isn't spanned. Public code MUST go through an AuditEntryPoint; internal code within an existing span MUST NOT -doing so will lose the first span. +revert test fix of ITestAssumeRole as it is covered in HADOOP-17476/apache#2777 Change-Id: I2cfdba907df0dcc76629e5af8effeed8d25d5933
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 +1
thx |
…ache#2777) Contributed by Steve Loughran.
…failure. (apache#2777) Contributed by Steve Loughran. (cherry picked from commit bf2b431d15c84a62bfb6322f1c8702804d58ce28) Signed-off-by: Arpit Agarwal <[email protected]> Change-Id: I49f779ff3599104c920bc560e2c68d1bfed7ec6d
Removes string match so change in AWS S3 error message doesn't cause the test to fail