You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/10/19 13:12:47 UTC

[GitHub] [flink] mbalassi opened a new pull request, #21114: [FLINK-29622][runtime][security] Start kerberos delegation token prov…

mbalassi opened a new pull request, #21114:
URL: https://github.com/apache/flink/pull/21114

   Backport of #21098 that I have already merged to master.
   
   cc @gaborgsomogyi 


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] gaborgsomogyi commented on a diff in pull request #21114: [FLINK-29622][runtime][security] Start kerberos delegation token prov…

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on code in PR #21114:
URL: https://github.com/apache/flink/pull/21114#discussion_r999540330


##########
flink-runtime/src/main/java/org/apache/flink/runtime/security/token/KerberosDelegationTokenManagerFactory.java:
##########
@@ -40,12 +41,21 @@ public static DelegationTokenManager create(
             ClassLoader classLoader,
             Configuration configuration,
             @Nullable ScheduledExecutor scheduledExecutor,
-            @Nullable ExecutorService ioExecutor) {
+            @Nullable ExecutorService ioExecutor)
+            throws IOException {
 
         if (configuration.getBoolean(SecurityOptions.KERBEROS_FETCH_DELEGATION_TOKEN)) {
             if (HadoopDependency.isHadoopCommonOnClasspath(classLoader)) {
-                return new KerberosDelegationTokenManager(
-                        configuration, scheduledExecutor, ioExecutor);
+                KerberosLoginProvider kerberosLoginProvider =
+                        new KerberosLoginProvider(configuration);
+                if (kerberosLoginProvider.isLoginPossible()) {

Review Comment:
   Having a static function which is hardly testable would be more strange. From perf perspective this runs once per cluster start.



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] flinkbot commented on pull request #21114: [FLINK-29622][runtime][security] Start kerberos delegation token prov…

Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #21114:
URL: https://github.com/apache/flink/pull/21114#issuecomment-1284015191

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "8a6f140e25b93ffeb3b963b9686b3542a664558a",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "8a6f140e25b93ffeb3b963b9686b3542a664558a",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 8a6f140e25b93ffeb3b963b9686b3542a664558a UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] mbalassi commented on pull request #21114: [FLINK-29622][runtime][security] Start kerberos delegation token prov…

Posted by GitBox <gi...@apache.org>.
mbalassi commented on PR #21114:
URL: https://github.com/apache/flink/pull/21114#issuecomment-1284225036

   Thanks for taking a look, @zentol. Based on the urgency of the matter and @gaborgsomogyi's reply I am comfortable with merging this as soon as the CI passes. I have performed local tests which look good. Let me know if you have further input.


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] zentol commented on a diff in pull request #21114: [FLINK-29622][runtime][security] Start kerberos delegation token prov…

Posted by GitBox <gi...@apache.org>.
zentol commented on code in PR #21114:
URL: https://github.com/apache/flink/pull/21114#discussion_r999460321


##########
flink-runtime/src/main/java/org/apache/flink/runtime/security/token/KerberosDelegationTokenManagerFactory.java:
##########
@@ -40,12 +41,21 @@ public static DelegationTokenManager create(
             ClassLoader classLoader,
             Configuration configuration,
             @Nullable ScheduledExecutor scheduledExecutor,
-            @Nullable ExecutorService ioExecutor) {
+            @Nullable ExecutorService ioExecutor)
+            throws IOException {
 
         if (configuration.getBoolean(SecurityOptions.KERBEROS_FETCH_DELEGATION_TOKEN)) {
             if (HadoopDependency.isHadoopCommonOnClasspath(classLoader)) {
-                return new KerberosDelegationTokenManager(
-                        configuration, scheduledExecutor, ioExecutor);
+                KerberosLoginProvider kerberosLoginProvider =
+                        new KerberosLoginProvider(configuration);
+                if (kerberosLoginProvider.isLoginPossible()) {

Review Comment:
   its a bit strange that we create a provider, use it for one call, throw it away, only for it to be re-created in the delegation token manager.



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] mbalassi merged pull request #21114: [FLINK-29622][runtime][security] Start kerberos delegation token prov…

Posted by GitBox <gi...@apache.org>.
mbalassi merged PR #21114:
URL: https://github.com/apache/flink/pull/21114


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org