becketqin commented on code in PR #22509:
URL: https://github.com/apache/flink/pull/22509#discussion_r1223687864
##########
flink-yarn/src/main/java/org/apache/flink/yarn/Utils.java:
##########
@@ -620,4 +626,54 @@ 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. If the viewAcls or modifyAcls string contains the
WILDCARD_ACL constant, it
+ * will replace the entire string with the WILDCARD_ACL. The resulting map
is then set as the
+ * application acls for the given container launch context.
+ *
+ * @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) {
+ if (viewAcls.contains(WILDCARD_ACL)) {
+ if (!isValidWildcardAcl(viewAcls)) {
+ throw new IllegalArgumentException("Invalid wildcard ACL:
" + viewAcls);
+ }
+ viewAcls = WILDCARD_ACL;
+ }
+ acls.put(ApplicationAccessType.VIEW_APP, viewAcls);
+ }
+ if (modifyAcls != null) {
+ if (modifyAcls.contains(WILDCARD_ACL)) {
+ if (!isValidWildcardAcl(modifyAcls)) {
+ throw new IllegalArgumentException("Invalid wildcard ACL:
" + modifyAcls);
+ }
+ modifyAcls = WILDCARD_ACL;
+ }
+ acls.put(ApplicationAccessType.MODIFY_APP, modifyAcls);
+ }
+ if (!acls.isEmpty()) {
+ amContainer.setApplicationACLs(acls);
+ }
+ }
+
+ private static boolean isValidWildcardAcl(String acl) {
Review Comment:
Can we replace this with the following?
```
void validateAclString(String acl) {
if (acl != null && acl.contains("*") && !acl.equals("*") {
throw new IllegalArgumentExpception(String.format("Invalid wildcard ACL
%s. the ACL wildcard does not support regex. The only valid wildcard ACL is
'*'.", acl).
}
}
```
The main logic can be just:
```
...
validateAclString(viewAcls);
validateAclString(modifyAcls);
if (viewAcls != null && !viewAcls.isEmpty()) {
acls.put(ApplicationAccessType.VIEW_APP, viewAcls)
}
if (modifyAcls != null && !modifyAcls.isEmpty()) {
acls.put(ApplicationAccessType.MODIFY_APP, modifyAcls)
}
...
```
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YARNSessionFIFOITCase.java:
##########
@@ -60,6 +60,12 @@
class YARNSessionFIFOITCase extends YarnTestBase {
private static final Logger log =
LoggerFactory.getLogger(YARNSessionFIFOITCase.class);
+ protected static final String VIEW_ACLS = "user group";
+ protected static final String MODIFY_ACLS = "admin groupAdmin";
+ protected static final String VIEW_ACLS_WITH_WILDCARD = "user,* group";
+ protected static final String MODIFY_ACLS_WITH_WILDCARD = "admin,*
groupAdmin";
Review Comment:
Should this be changed as well according to the new wildcard validation?
Also, I am wondering if the wildcard is applied to user and group
independently or it applies to the entire ACL as a whole. In another word, does
something like `user1,user2 *` makes sense or the only valid wildcard is `*`?
--
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]
