pithecuse527 commented on code in PR #13455:
URL: https://github.com/apache/gravitino/pull/13455#discussion_r4232630412
##########
clients/filesystem-hadoop3/src/main/java/org/apache/gravitino/filesystem/hadoop/BaseGVFSOperations.java:
##########
@@ -453,23 +455,72 @@ public abstract FSDataOutputStream append(Path gvfsPath,
int bufferSize, Progres
*/
public abstract Token<?>[] addDelegationTokens(String renewer, Credentials
credentials);
+ /**
+ * Resolve the filesets listed in {@link
+ *
GravitinoVirtualFileSystemConfiguration#FS_GRAVITINO_DELEGATION_TOKEN_FILESETS},
so that their
+ * actual file systems exist before delegation tokens are collected. Token
requests carry no path,
+ * so the filesets cannot be learned from the caller.
+ */
+ public void prepareDelegationTokens() {
+ for (String path :
+ conf.getTrimmedStrings(
+
GravitinoVirtualFileSystemConfiguration.FS_GRAVITINO_DELEGATION_TOKEN_FILESETS))
{
+ try {
+ getActualFileSystem(new Path(path), currentLocationName());
Review Comment:
- Resolved with skipping location creation during token collection
- Added a regression test
##########
clients/filesystem-hadoop3/src/main/java/org/apache/gravitino/filesystem/hadoop/BaseGVFSOperations.java:
##########
@@ -453,23 +455,72 @@ public abstract FSDataOutputStream append(Path gvfsPath,
int bufferSize, Progres
*/
public abstract Token<?>[] addDelegationTokens(String renewer, Credentials
credentials);
+ /**
+ * Resolve the filesets listed in {@link
+ *
GravitinoVirtualFileSystemConfiguration#FS_GRAVITINO_DELEGATION_TOKEN_FILESETS},
so that their
+ * actual file systems exist before delegation tokens are collected. Token
requests carry no path,
+ * so the filesets cannot be learned from the caller.
+ */
+ public void prepareDelegationTokens() {
+ for (String path :
+ conf.getTrimmedStrings(
+
GravitinoVirtualFileSystemConfiguration.FS_GRAVITINO_DELEGATION_TOKEN_FILESETS))
{
+ try {
+ getActualFileSystem(new Path(path), currentLocationName());
+ } catch (Exception e) {
+ // Never fail token collection: an unresolvable fileset simply yields
no token.
+ LOG.warn("Failed to resolve {} while preparing delegation tokens",
path, e);
+ }
+ }
+ }
+
/**
* Add delegation tokens for all file systems in the cache.
*
+ * <p>Only tokens owned by the current user are added. A fileset configured
with its own Kerberos
+ * principal is accessed as that principal, and its token must not be handed
to the caller's job.
+ *
* @param renewer the renewer.
* @param credentials the credentials.
* @return the array of tokens.
*/
protected Token<?>[] addDelegationTokensForAllFS(String renewer, Credentials
credentials) {
+ String currentUser;
+ try {
+ currentUser = UserGroupInformation.getCurrentUser().getShortUserName();
+ } catch (IOException e) {
+ LOG.warn("Failed to get the current user, skipping delegation tokens",
e);
+ return new Token<?>[0];
+ }
+
List<Token<?>> tokenList = Lists.newArrayList();
for (FileSystem fileSystem : fileSystemCache.asMap().values()) {
+ // Collect into a copy, so a token owned by another user never reaches
the caller.
+ Credentials fetched = new Credentials(credentials);
Review Comment:
- Resolved with normalizing null credentials to an empty `Credentials` at
entry
- Added a regression test
##########
clients/filesystem-hadoop3/src/main/java/org/apache/gravitino/filesystem/hadoop/BaseGVFSOperations.java:
##########
@@ -453,23 +455,72 @@ public abstract FSDataOutputStream append(Path gvfsPath,
int bufferSize, Progres
*/
public abstract Token<?>[] addDelegationTokens(String renewer, Credentials
credentials);
+ /**
+ * Resolve the filesets listed in {@link
+ *
GravitinoVirtualFileSystemConfiguration#FS_GRAVITINO_DELEGATION_TOKEN_FILESETS},
so that their
+ * actual file systems exist before delegation tokens are collected. Token
requests carry no path,
+ * so the filesets cannot be learned from the caller.
+ */
+ public void prepareDelegationTokens() {
+ for (String path :
+ conf.getTrimmedStrings(
+
GravitinoVirtualFileSystemConfiguration.FS_GRAVITINO_DELEGATION_TOKEN_FILESETS))
{
+ try {
+ getActualFileSystem(new Path(path), currentLocationName());
+ } catch (Exception e) {
+ // Never fail token collection: an unresolvable fileset simply yields
no token.
+ LOG.warn("Failed to resolve {} while preparing delegation tokens",
path, e);
+ }
+ }
+ }
+
/**
* Add delegation tokens for all file systems in the cache.
*
+ * <p>Only tokens owned by the current user are added. A fileset configured
with its own Kerberos
+ * principal is accessed as that principal, and its token must not be handed
to the caller's job.
+ *
* @param renewer the renewer.
* @param credentials the credentials.
* @return the array of tokens.
*/
protected Token<?>[] addDelegationTokensForAllFS(String renewer, Credentials
credentials) {
+ String currentUser;
+ try {
+ currentUser = UserGroupInformation.getCurrentUser().getShortUserName();
+ } catch (IOException e) {
+ LOG.warn("Failed to get the current user, skipping delegation tokens",
e);
+ return new Token<?>[0];
+ }
+
List<Token<?>> tokenList = Lists.newArrayList();
for (FileSystem fileSystem : fileSystemCache.asMap().values()) {
Review Comment:
- Resolved with collecting tokens right after each fileset is resolved, not
from the cache
- Added a regression test
--
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]
