steveloughran commented on code in PR #8196:
URL: https://github.com/apache/hadoop/pull/8196#discussion_r2736711994
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/planner/NodePlan.java:
##########
@@ -43,6 +49,10 @@ public class NodePlan {
private static final ObjectReader READER = MAPPER.readerFor(NodePlan.class);
private static final ObjectWriter WRITER = MAPPER.writerFor(
MAPPER.constructType(NodePlan.class));
+ private static final String SUPPORTED_PACKAGES_CONFIG_NAME =
"dfs.nodeplan.steps.supported.packages";
Review Comment:
move to org.apache.hadoop.hdfs.DFSConfigKeys ; add javadoc with @value tag
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml:
##########
@@ -6712,4 +6712,12 @@
Enables observer reads for clients. This should only be enabled when
clients are using routers.
</description>
</property>
+ <property>
+ <name>dfs.nodeplan.steps.supported.packages</name>
+ <value>org.apache.hadoop.hdfs.server</value>
Review Comment:
should we restrict to org.apache.hadoop.hdfs.server.diskbalancer.planner for
even more lockdown?
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/planner/NodePlan.java:
##########
@@ -188,4 +233,32 @@ public String getNodeUUID() {
public void setNodeUUID(String nodeUUID) {
this.nodeUUID = nodeUUID;
}
+
+ private static boolean stepClassIsAllowed(String className) {
+ for (String pkg : SUPPORTED_PACKAGES) {
+ if (className.startsWith(pkg)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static List<String> getAllowedPackages() {
+ String packages = CONFIGURATION.get(SUPPORTED_PACKAGES_CONFIG_NAME);
+ return parseCsvLine(packages);
Review Comment:
Configuration.getStrings() does this
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/planner/NodePlan.java:
##########
@@ -188,4 +233,32 @@ public String getNodeUUID() {
public void setNodeUUID(String nodeUUID) {
this.nodeUUID = nodeUUID;
}
+
+ private static boolean stepClassIsAllowed(String className) {
+ for (String pkg : SUPPORTED_PACKAGES) {
+ if (className.startsWith(pkg)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static List<String> getAllowedPackages() {
+ String packages = CONFIGURATION.get(SUPPORTED_PACKAGES_CONFIG_NAME);
+ return parseCsvLine(packages);
+ }
+
+ static List<String> parseCsvLine(final String line) {
Review Comment:
not needed. which is good as I'd be asking for tests for it.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
