venkata91 commented on code in PR #22509:
URL: https://github.com/apache/flink/pull/22509#discussion_r1186730308


##########
flink-yarn/src/main/java/org/apache/flink/yarn/Utils.java:
##########
@@ -620,4 +623,30 @@ public static YarnConfiguration getYarnConfiguration(
 
         return yarnConfig;
     }
+
+    /**
+     * Sets the application ACLs for the given ContainerLaunchContext based on 
the values specified
+     * in the given Flink configuration. Only ApplicationAccessType.VIEW_APP 
and
+     * ApplicationAccessType.MODIFY_APP ACLs are set, and only if they are 
configured in the Flink
+     * configuration.
+     *
+     * @param amContainer the ContainerLaunchContext to set the ACLs for
+     * @param flinkConfig the Flink configuration to read the ACL values from
+     */
+    public static void setAclsFor(
+            ContainerLaunchContext amContainer,
+            org.apache.flink.configuration.Configuration flinkConfig) {
+        Map<ApplicationAccessType, String> acls = new HashMap<>();
+        String viewAcls = 
flinkConfig.getString(YarnConfigOptions.APPLICATION_VIEW_ACLS, null);
+        String modifyAcls = 
flinkConfig.getString(YarnConfigOptions.APPLICATION_MODIFY_ACLS, null);
+        if (viewAcls != null) {
+            acls.put(ApplicationAccessType.VIEW_APP, viewAcls);
+        }
+        if (modifyAcls != null) {
+            acls.put(ApplicationAccessType.MODIFY_APP, modifyAcls);
+        }
+        if (!acls.isEmpty()) {
+            amContainer.setApplicationACLs(acls);

Review Comment:
   What happens if wildcard (`*`) is passed in the config value? Should we only 
set the `*` instead of `user1,*`. Looks like YARN cannot recognize it properly 
based on the comment 
[here](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/SecurityManager.scala#L134)?



##########
flink-yarn/src/main/java/org/apache/flink/yarn/Utils.java:
##########
@@ -620,4 +623,30 @@ public static YarnConfiguration getYarnConfiguration(
 
         return yarnConfig;
     }
+
+    /**
+     * Sets the application ACLs for the given ContainerLaunchContext based on 
the values specified
+     * in the given Flink configuration. Only ApplicationAccessType.VIEW_APP 
and
+     * ApplicationAccessType.MODIFY_APP ACLs are set, and only if they are 
configured in the Flink
+     * configuration.
+     *
+     * @param amContainer the ContainerLaunchContext to set the ACLs for
+     * @param flinkConfig the Flink configuration to read the ACL values from
+     */
+    public static void setAclsFor(
+            ContainerLaunchContext amContainer,
+            org.apache.flink.configuration.Configuration flinkConfig) {
+        Map<ApplicationAccessType, String> acls = new HashMap<>();
+        String viewAcls = 
flinkConfig.getString(YarnConfigOptions.APPLICATION_VIEW_ACLS, null);
+        String modifyAcls = 
flinkConfig.getString(YarnConfigOptions.APPLICATION_MODIFY_ACLS, null);
+        if (viewAcls != null) {
+            acls.put(ApplicationAccessType.VIEW_APP, viewAcls);
+        }
+        if (modifyAcls != null) {
+            acls.put(ApplicationAccessType.MODIFY_APP, modifyAcls);
+        }

Review Comment:
   Looks like Spark is explicitly adding `user.name and SPARK_USER` both to the 
acls by default. Although from this 
[doc](https://docs.cloudera.com/runtime/7.2.7/yarn-security/yarn-security.pdf), 
looks like the `Users who start an application (the owners) always have access 
to the application they start, which includes the application logs, job 
statistics, and ACLs`



-- 
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-unsubscr...@flink.apache.org

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

Reply via email to