[
https://issues.apache.org/jira/browse/HADOOP-20022?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18127443#comment-18127443
]
ASF GitHub Bot commented on HADOOP-20022:
-----------------------------------------
joseluisll opened a new pull request, #8826:
URL: https://github.com/apache/hadoop/pull/8826
### Description of PR
JIRA: https://issues.apache.org/jira/browse/HADOOP-20022
`SecurityUtil.doAsLoginUserOrFatal()` called `Runtime.getRuntime().exit(-1)`
when `UserGroupInformation.getLoginUser()` threw an `IOException` with security
enabled. That bypasses `ExitUtil`, so tests that call
`ExitUtil.disableSystemExit()` cannot intercept it and the JVM is killed
instead. Callers include `ZKFailoverController`, `SecondaryNameNode`,
`BootstrapStandby`, `EditLogTailer` and `StandbyCheckpointer`.
Changes:
- Call `ExitUtil.terminate(-1, e)` instead of
`Runtime.getRuntime().exit(-1)`. Production behaviour is unchanged: with exits
enabled, `ExitUtil.terminate` ends in `System.exit(-1)`.
- Drop the redundant `e.printStackTrace()`; the exception is already logged.
- Throw after `terminate()`. SpotBugs cannot tell that
`ExitUtil.terminate()` never returns, and without this it reports
`NP_NULL_ON_SOME_PATH_EXCEPTION` on `ugi.doAs(action)`.
- Add `TestSecurityUtil#testDoAsLoginUserOrFatalTerminatesOnLoginFailure`,
which statically mocks `UserGroupInformation` so `isSecurityEnabled()` is true
and `getLoginUser()` throws, then checks for an `ExitException` with exit code
-1, the original `IOException` as its cause, and that the action never runs.
Note: `ZKFailoverController.run()` catches `RuntimeException` and rethrows
its cause, so a ZKFC test with exits disabled sees the original `IOException`
rather than the `ExitException`. `ExitUtil.terminateCalled()` still reports the
exit. That is existing ZKFC behaviour and is left unchanged here.
Related: HADOOP-20010 (same issue in
`AbstractDelegationTokenSecretManager.ExpiredTokenRemover`) and YARN-12028
(same issue in `DelegationTokenRenewer.DelegationTokenCancelThread`).
### How was this patch tested?
Run on the host (pure Java, no native profile), on the branch's previous
base `86e6c203` (YARN-12012):
- `mvn test -Dtest=TestSecurityUtil -Dmaven.test.failure.ignore=false` in
`hadoop-common`: 27 tests run, 0 failures, 0 errors.
- The new test run against the original code kills the surefire fork ("The
forked VM terminated without properly saying goodbye. VM crash or System.exit
called?"), so it reproduces the problem.
- SpotBugs on `hadoop-common`: 0 bugs (the first version of the fix reported
`NP_NULL_ON_SOME_PATH_EXCEPTION`, fixed in the second commit).
- Checkstyle: no violations on the changed lines.
The branch was then rebuilt on `apache/hadoop` trunk `df8e2a77`
(HADOOP-20020). None of the 5 commits in between touch `SecurityUtil.java` or
`TestSecurityUtil.java`, and the diff is identical, but the tests have not been
re-run on the new base.
### For code changes:
- [x] Does the title of this PR start with the corresponding JIRA issue id
(e.g. 'HADOOP-17799. Your PR title ...')?
- [ ] Object storage: Have the integration tests been executed and the
endpoint
declared according to the connector-specific documentation? *Note:
Automated CI
testing doesn't cover all cases so manual testing with cloud storage
is still
required.* (N/A: no object storage changes)
- [ ] If adding new dependencies to the code, are these dependencies
licensed in a way that is compatible for inclusion under [ASF
2.0](http://www.apache.org/legal/resolved.html#category-a)? (N/A: no new
dependencies)
- [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`,
`NOTICE-binary` files? (N/A)
### AI Tooling
If an AI tool was used:
- [x] The PR includes the phrase "Contains content generated by <tool>"
where <tool> is the name of the AI tool used.
- [ X] My use of AI contributions follows the ASF legal policy
https://www.apache.org/legal/generative-tooling.html
Contains content generated by Claude Code.
> SecurityUtil.doAsLoginUserOrFatal should use ExitUtil instead of Runtime.exit
> -----------------------------------------------------------------------------
>
> Key: HADOOP-20022
> URL: https://issues.apache.org/jira/browse/HADOOP-20022
> Project: Hadoop Common
> Issue Type: Improvement
> Components: security
> Reporter: Jose Luis López
> Priority: Minor
>
> {{SecurityUtil.doAsLoginUserOrFatal()}} calls
> {{Runtime.getRuntime().exit(-1)}} when
> {{UserGroupInformation.getLoginUser()}} throws an {{IOException}}:
> {code:java}
> try {
> ugi = UserGroupInformation.getLoginUser();
> } catch (IOException e) {
> LOG.error("Exception while getting login user", e);
> e.printStackTrace();
> Runtime.getRuntime().exit(-1);
> }
> {code}
> After HADOOP-20010 this is the only non-test caller of
> {{Runtime.getRuntime().exit(-1)}} in hadoop-common. It bypasses {{ExitUtil}},
> so tests that call {{ExitUtil.disableSystemExit()}} can't intercept it and
> the JVM is killed instead.
> Callers include {{ZKFailoverController}}, {{SecondaryNameNode}},
> {{BootstrapStandby}}, {{EditLogTailer}} and {{StandbyCheckpointer}}.
> *Proposed fix:*
> * Call {{ExitUtil.terminate(-1, e)}} instead of
> {{Runtime.getRuntime().exit(-1)}}.
> * Drop the redundant {{e.printStackTrace()}}; the exception is already logged.
> * Add a test that a {{getLoginUser()}} failure with security enabled calls
> {{ExitUtil.terminate(-1, ...)}}.
> Raised in review of HADOOP-20010 (https://github.com/apache/hadoop/pull/8805).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
