HDFS-16686. GetJournalEditServlet fails to authorize valid Kerberos request #4724
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
ndimiduk
approved these changes
Aug 12, 2022
|
This looks like an example of a flaky test, where a previous test execution affects tests that follow. Specifically the reported failed test is |
added 7 commits
August 17, 2022 09:28
…equest Use DfsServlet to obtain the UGI.
Changed "conf" and "servlet" to uppercase since they are static (the servlet is initialized only once).
Update the tests to ensure that the mini clusters are shutdown properly.
This update allows MiniQJMHACluster to be used in try-with-resources, ensuring that tests are cleaned-up correctly.
This update ensures that tests are cleaned-up correctly.
Create a default HDFS configuration which has test-specific data directories. This is intended to protect against interactions between test runs that might corrupt results. Each test run's data is automatically cleaned-up by JUnit.
snmvaughan
force-pushed
the
feature/HDFS-16686
branch
from
e1039f4
to
1c8f552
Compare
|
🎊 +1 overall
This message was automatically generated. |
sunchao
reviewed
Aug 19, 2022
...hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/MiniQJMHACluster.java
Show resolved
Hide resolved
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestRollingUpgrade.java
Outdated
Show resolved
Hide resolved
| @@ -80,16 +82,38 @@ public static void runCmd(DFSAdmin dfsadmin, boolean success, | |||
| } | |||
| } | |||
|
|
|||
| @Rule | |||
| public TemporaryFolder folder= new TemporaryFolder(); | |||
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestRollingUpgrade.java
Show resolved
Hide resolved
|
|
||
| @BeforeClass | ||
| public static void setUp() throws ServletException { | ||
| LogManager.getLogger(GetJournalEditServlet.class).setLevel(Level.DEBUG); |
There was a problem hiding this comment.
why do we want to set the logger level to DEBUG here?
There was a problem hiding this comment.
That was a hold over from debug testing. I'll remove it.
|
🎊 +1 overall
This message was automatically generated. |
The signature for close() can't throw an exception for AutoCloseable, but the definition of shutdown() didn't have to change. Move the try-catch into close().
sunchao
approved these changes
Aug 23, 2022
|
🎊 +1 overall
This message was automatically generated. |
|
Merged, thanks |
snmvaughan
added a commit
to snmvaughan/hadoop
that referenced
this pull request
Aug 23, 2022
sunchao
pushed a commit
that referenced
this pull request
Sep 13, 2022
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
LiuGuH
pushed a commit
to LiuGuH/hadoop
that referenced
this pull request
Mar 29, 2024
LiuGuH
pushed a commit
to LiuGuH/hadoop
that referenced
this pull request
Jul 4, 2024
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.
Description of PR
GetJournalEditServlet uses request.getRemoteuser() to determine the remoteShortName for Kerberos authorization, which fails to match when the JournalNode uses its own Kerberos principal (e.g. jn/@).
This can be fixed by using the UserGroupInformation provided by the base DfsServlet class using the getUGI(request, conf) call.
How was this patch tested?
Integration tests were performed against an HA configuration running in Kubernetes, running Java 11. With the patch, exceptions which had previously reported expected Kerberos principals which included an IP address string were eliminated.
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?