[#5157]fix: User can specify the krb5 conf file path for Kerberos enabled Hadoop catalog #5165
Conversation
| KRB5_CONF_KEY, | ||
| "The Kerberos krb file for the catalog", | ||
| false /* immutable */, | ||
| "/etc/krb5.conf" /* defaultValue */, |
There was a problem hiding this comment.
As the default value is /etc/krb5.conf, what if the file is not found?
There was a problem hiding this comment.
If the file is not exist, "Can't get Kerberos realm" error message will be shown. This behavior is same as before.
| String krbFilePath = kerberosConfig.getKrb5Conf(); | ||
| Preconditions.checkArgument( | ||
| StringUtils.isNotBlank(krbFilePath), "The Kerberos krb file can't be blank"); | ||
| System.setProperty("java.security.krb5.conf", krbFilePath); |
There was a problem hiding this comment.
Is this method class loader isolated? I mean what if I set java.security.krb5.conf in catalogA, does it take effect in catalogB
There was a problem hiding this comment.
The configuration is separated for each catalog, this mean the setting of catalogA can not affect catalogB.
There was a problem hiding this comment.
I mean if we set the value in catalogA, and in catalogB, could we still get the value if catalogB does not set it?
| @@ -32,6 +32,7 @@ Besides the [common catalog properties](./gravitino-server-config.md#gravitino-c | |||
| | `default-filesystem-provider` | The name default filesystem providers of this Hadoop catalog if users do not specify the scheme in the URI. Default value is `builtin-local` | `builtin-local` | No | 0.7.0-incubating | | |||
| | `authentication.impersonation-enable` | Whether to enable impersonation for the Hadoop catalog. | `false` | No | 0.5.1 | | |||
| | `authentication.type` | The type of authentication for Hadoop catalog, currently we only support `kerberos`, `simple`. | `simple` | No | 0.5.1 | | |||
| | `java.security.krb5.conf` | Kerberos krb file for configuration of Kerberos. | /etc/krb5.conf | No | 0.7.0-incubating | | |||
There was a problem hiding this comment.
Let me think if there is a better name for it, java.security.krb5.conf is so long.
There was a problem hiding this comment.
what about krb5.conf.path?
There was a problem hiding this comment.
LGTM about the name changing. That mean I should create a new PR for it, is it correct?
There was a problem hiding this comment.
That mean I should create a new PR for it?
No, if it's okay, please do it in this PR.
|
|
What changes were proposed in this pull request?
User can specify the krb5 conf file path for Kerberos enabled Hadoop catalog.
Why are the changes needed?
Fix: #5157
Does this PR introduce any user-facing change?
Add a property key java.security.krb5.conf for Kerberos enabled Hadoop catalog.
How was this patch tested?
Create a Hadoop catalog for Kerberos enabled HDFS, you can put krb5.conf in any readable path of Gravatino host, and add the config java.security.krb5.conf=/X/krb5.conf , then the Hadoop catalog can be created succeed and all works well.