mbaurin opened a new pull request, #8187:
URL: https://github.com/apache/gravitino/pull/8187
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
Examples:
- "[#123] feat(operator): support xxx"
- "[#233] fix: check null before access result in xxx"
- "[MINOR] refactor: fix typo in variable name"
- "[MINOR] docs: fix typo in README"
- "[#255] test: fix flaky test NameOfTheTest"
Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->
### What changes were proposed in this pull request ?
This PR fixes the OAuth/Kerberos authentication detection logic in
`GravitinoConfig.java`. The changes include:
1. **Fixed authentication type comparisons**: Replace
`authKey.equals("oauth")` with `"oauth".equals(authType)` and similar for
Kerberos
2. **Removed duplicate OAuth initialization**: Eliminated redundant OAuth
configuration block that was overwriting Kerberos settings
3. **Consolidated control flow**: Created clean if-else if structure for
proper authentication type handling
The root issue was that the code was comparing against the constant
`authKey` ("auth") instead of the parsed `authType` value from the
configuration file.
### Why are the changes needed?
The current implementation has critical bugs that prevent OAuth and Kerberos
authentication from working correctly:
1. **Wrong variable comparison**: Code used `authKey` (constant "auth")
instead of `authType` (parsed value like "oauth" or "kerberos")
2. **Configuration overwriting**: Duplicate OAuth logic was overwriting any
Kerberos configuration
3. **Unreachable code paths**: Kerberos authentication could never be
properly initialized
These bugs prevent the CLI from correctly detecting and configuring
authentication when `auth=oauth` or `auth=kerberos` is set in the configuration
file.
Fix: https://github.com/apache/gravitino/issues/8162
### Does this PR introduce _any_ user-facing change?
**No user-facing API changes**, but fixes critical authentication
functionality:
- When `auth=oauth` is configured, `config.getOAuth()` now correctly returns
valid OAuth configuration
- When `auth=kerberos` is configured, `config.getKerberos()` now correctly
returns valid Kerberos configuration
- No changes to configuration file format or property keys
### How was this patch tested?
### How was this patch tested?
1. **Code review**: Verified the logic fixes address all identified issues
2. **Integration verification**: Confirmed the changes work with existing
`Command.java` usage patterns at lines 202-223
3. **Style compliance**: Ensured code follows Apache Gravitino conventions
using defensive null-safe comparisons
The existing unit tests in `TestGravitinoConfig.java` should pass without
modification
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
