ChengbingLiu commented on a change in pull request #15810:
URL: https://github.com/apache/flink/pull/15810#discussion_r630024881
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/security/modules/HadoopModule.java
##########
@@ -97,8 +97,7 @@ public void install() throws SecurityInstallException {
// If UGI use keytab for login, do not load HDFS
delegation token.
for (Token<? extends TokenIdentifier> token : usrTok) {
if (!token.getKind().equals(hdfsDelegationTokenKind)) {
- final Text id = new Text(token.getIdentifier());
- credentialsToBeAdded.addToken(id, token);
+ credentialsToBeAdded.addToken(token.getService(),
token);
Review comment:
@lirui-apache Let me explain this a little bit.
1) HDFS-9276 made a defensive copy because a `Token` instance is passed to
the constructor of `PrivateToken`. In the view of the `PrivateToken` class, a
defensive copy is certainly needed.
2) For this change, `token`, along with its referencing object `usrTok` and
`credentialsFromTokenStorageFile`, are all local variables.
`credentialsFromTokenStorageFile` comes from reading a file. Therefore, a deep
copy is not as necessary here as it is in HDFS-9276.
3) `token.getIdentifier()` returns a `byte[]`, while `token.getService()`
returns a `Text`. This is probably why a deep copy looks like removed in this
change. The fact is that previously to this change, it didn't make a deep copy
either.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
