YARN-11190. CS Mapping rule bug: User matcher does not work correctly for usernames with dot (#4471)
This commit is contained in:
parent
e3b09b7512
commit
7f6cc196f8
|
@ -228,7 +228,11 @@ public class CSMappingPlacementRule extends PlacementRule {
|
||||||
ApplicationSubmissionContext asc, String user) {
|
ApplicationSubmissionContext asc, String user) {
|
||||||
VariableContext vctx = new VariableContext();
|
VariableContext vctx = new VariableContext();
|
||||||
|
|
||||||
vctx.put("%user", cleanName(user));
|
String cleanedName = cleanName(user);
|
||||||
|
if (!user.equals(cleanedName)) {
|
||||||
|
vctx.putOriginal("%user", user);
|
||||||
|
}
|
||||||
|
vctx.put("%user", cleanedName);
|
||||||
//If the specified matches the default it means NO queue have been specified
|
//If the specified matches the default it means NO queue have been specified
|
||||||
//as per ClientRMService#submitApplication which sets the queue to default
|
//as per ClientRMService#submitApplication which sets the queue to default
|
||||||
//when no queue is provided.
|
//when no queue is provided.
|
||||||
|
@ -239,7 +243,7 @@ public class CSMappingPlacementRule extends PlacementRule {
|
||||||
//Adding specified as empty will prevent it to be undefined and it won't
|
//Adding specified as empty will prevent it to be undefined and it won't
|
||||||
//try to place the application to a queue named '%specified', queue path
|
//try to place the application to a queue named '%specified', queue path
|
||||||
//validation will reject the empty path or the path with empty parts,
|
//validation will reject the empty path or the path with empty parts,
|
||||||
//so we sill still hit the fallback action of this rule if no queue
|
//so we still hit the fallback action of this rule if no queue
|
||||||
//is specified
|
//is specified
|
||||||
vctx.put("%specified", "");
|
vctx.put("%specified", "");
|
||||||
}
|
}
|
||||||
|
|
|
@ -39,6 +39,7 @@ public class VariableContext {
|
||||||
* This is our actual variable store.
|
* This is our actual variable store.
|
||||||
*/
|
*/
|
||||||
private Map<String, String> variables = new HashMap<>();
|
private Map<String, String> variables = new HashMap<>();
|
||||||
|
private Map<String, String> originalVariables = new HashMap<>();
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* This is our conditional variable store.
|
* This is our conditional variable store.
|
||||||
|
@ -124,6 +125,10 @@ public class VariableContext {
|
||||||
return this;
|
return this;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public void putOriginal(String name, String value) {
|
||||||
|
originalVariables.put(name, value);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* This method is used to add a conditional variable to the variable context.
|
* This method is used to add a conditional variable to the variable context.
|
||||||
* @param name Name of the variable
|
* @param name Name of the variable
|
||||||
|
@ -150,6 +155,10 @@ public class VariableContext {
|
||||||
return ret == null ? "" : ret;
|
return ret == null ? "" : ret;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public String getOriginal(String name) {
|
||||||
|
return originalVariables.get(name);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Adds a set to the context, each name can only be added once. The extra
|
* Adds a set to the context, each name can only be added once. The extra
|
||||||
* dataset is different from the regular variables because it cannot be
|
* dataset is different from the regular variables because it cannot be
|
||||||
|
|
|
@ -87,6 +87,12 @@ public class MappingRuleMatchers {
|
||||||
}
|
}
|
||||||
|
|
||||||
String substituted = variables.replaceVariables(value);
|
String substituted = variables.replaceVariables(value);
|
||||||
|
|
||||||
|
String originalVariableValue = variables.getOriginal(variable);
|
||||||
|
if (originalVariableValue != null) {
|
||||||
|
return substituted.equals(originalVariableValue);
|
||||||
|
}
|
||||||
|
|
||||||
return substituted.equals(variables.get(variable));
|
return substituted.equals(variables.get(variable));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -85,6 +85,7 @@ public class TestCSMappingPlacementRule {
|
||||||
.withQueue("root.user.alice")
|
.withQueue("root.user.alice")
|
||||||
.withQueue("root.user.bob")
|
.withQueue("root.user.bob")
|
||||||
.withQueue("root.user.test_dot_user")
|
.withQueue("root.user.test_dot_user")
|
||||||
|
.withQueue("root.user.testuser")
|
||||||
.withQueue("root.groups.main_dot_grp")
|
.withQueue("root.groups.main_dot_grp")
|
||||||
.withQueue("root.groups.sec_dot_test_dot_grp")
|
.withQueue("root.groups.sec_dot_test_dot_grp")
|
||||||
.withQueue("root.secondaryTests.unique")
|
.withQueue("root.secondaryTests.unique")
|
||||||
|
@ -857,6 +858,46 @@ public class TestCSMappingPlacementRule {
|
||||||
assertPlace(engine, app, user, "root.man.testGroup0");
|
assertPlace(engine, app, user, "root.man.testGroup0");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testOriginalUserNameWithDotCanBeUsedInMatchExpression() throws IOException {
|
||||||
|
List<MappingRule> rules = new ArrayList<>();
|
||||||
|
rules.add(
|
||||||
|
new MappingRule(
|
||||||
|
MappingRuleMatchers.createUserMatcher("test.user"),
|
||||||
|
(MappingRuleActions.createUpdateDefaultAction("root.user.testuser"))
|
||||||
|
.setFallbackSkip()));
|
||||||
|
rules.add(new MappingRule(
|
||||||
|
MappingRuleMatchers.createUserMatcher("test.user"),
|
||||||
|
(MappingRuleActions.createPlaceToDefaultAction())
|
||||||
|
.setFallbackReject()));
|
||||||
|
|
||||||
|
CSMappingPlacementRule engine = setupEngine(true, rules);
|
||||||
|
ApplicationSubmissionContext app = createApp("app");
|
||||||
|
assertPlace(
|
||||||
|
"test.user should be placed to root.user",
|
||||||
|
engine, app, "test.user", "root.user.testuser");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testOriginalGroupNameWithDotCanBeUsedInMatchExpression() throws IOException {
|
||||||
|
List<MappingRule> rules = new ArrayList<>();
|
||||||
|
rules.add(
|
||||||
|
new MappingRule(
|
||||||
|
MappingRuleMatchers.createUserGroupMatcher("sec.test.grp"),
|
||||||
|
(MappingRuleActions.createUpdateDefaultAction("root.user.testuser"))
|
||||||
|
.setFallbackSkip()));
|
||||||
|
rules.add(new MappingRule(
|
||||||
|
MappingRuleMatchers.createUserMatcher("test.user"),
|
||||||
|
(MappingRuleActions.createPlaceToDefaultAction())
|
||||||
|
.setFallbackReject()));
|
||||||
|
|
||||||
|
CSMappingPlacementRule engine = setupEngine(true, rules);
|
||||||
|
ApplicationSubmissionContext app = createApp("app");
|
||||||
|
assertPlace(
|
||||||
|
"test.user should be placed to root.user",
|
||||||
|
engine, app, "test.user", "root.user.testuser");
|
||||||
|
}
|
||||||
|
|
||||||
private CSMappingPlacementRule initPlacementEngine(CapacityScheduler cs) throws IOException {
|
private CSMappingPlacementRule initPlacementEngine(CapacityScheduler cs) throws IOException {
|
||||||
CSMappingPlacementRule engine = new CSMappingPlacementRule();
|
CSMappingPlacementRule engine = new CSMappingPlacementRule();
|
||||||
engine.setFailOnConfigError(true);
|
engine.setFailOnConfigError(true);
|
||||||
|
|
Loading…
Reference in New Issue