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
YARN-11126. ZKConfigurationStore Java deserialisation vulnerability #4265
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.
looks good, just need some error text on assertion failures.
every assert needs to have a meaningful error message so that a jenkins/yetus build helps diagnose problems. line numbers/stack traces aren't enough. sorry
Thread.sleep(100); | ||
} | ||
|
||
Assert.assertFalse(flagFile.exists()); |
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.
can you add the error text to raise in the assert, including the filename.
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.
Thanks for the review. I added an error message.
I see some test failures with errors Metrics source DelegationTokenSecretManagerMetrics already exists!
, that was introduced with this commit (git bisect):
d60262fe0092a9b45ee17830b98750fb00b856b1 is the first bad commit
commit d60262fe0092a9b45ee17830b98750fb00b856b1
Author: hchaverri <55413673+hchaverri@users.noreply.github.com>
Date: Tue Apr 26 09:20:11 2022 -0700
HADOOP-18167. Add metrics to track delegation token secret manager op… (#4092)
* HADOOP-18167. Add metrics to track delegation token secret manager operations
.../AbstractDelegationTokenSecretManager.java | 121 +++++++++++++++++-
.../token/delegation/TestDelegationToken.java | 136 +++++++++++++++++++++
2 files changed, 252 insertions(+), 5 deletions(-)
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.
This is fixed with HADOOP-18222.
Please rebase your branch so that UT failures can go away :)
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Thanks @tomicooler for working on this. |
Change-Id: I07162784851480b9b85c5faffa3e01f18b5dc4a7
Change-Id: I7c6f7668ca6be44f5ea38167d275f56b6b7cdfe5
🎊 +1 overall
This message was automatically generated. |
Thanks @tomicooler , |
No description provided.