This is an automated email from the ASF dual-hosted git repository. rzo1 pushed a commit to branch fix/scheduler-strategy-allowlist-default in repository https://gitbox.apache.org/repos/asf/storm.git
commit 0bd6458d19a4916af2ca775ddac24202babb021d Author: Richard Zowalla <[email protected]> AuthorDate: Fri Aug 21 14:25:34 2026 +0200 Restrict topology scheduler strategies to the shipped strategies when no allowlist is configured --- conf/defaults.yaml | 9 +++ docs/Resource_Aware_Scheduler_overview.md | 2 + storm-client/src/jvm/org/apache/storm/Config.java | 3 +- .../storm/utils/DisallowedStrategyException.java | 4 ++ .../org/apache/storm/utils/ReflectionUtils.java | 20 +++++- .../apache/storm/utils/ReflectionUtilsTest.java | 75 ++++++++++++++++++++++ .../scheduler/resource/ResourceAwareScheduler.java | 4 +- 7 files changed, 114 insertions(+), 3 deletions(-) diff --git a/conf/defaults.yaml b/conf/defaults.yaml index 9682cf8bc..57b1d7c94 100644 --- a/conf/defaults.yaml +++ b/conf/defaults.yaml @@ -89,6 +89,15 @@ nimbus.code.sync.freq.secs: 120 nimbus.task.launch.secs: 120 nimbus.file.copy.expiration.secs: 600 nimbus.topology.validator: "org.apache.storm.nimbus.DefaultTopologyValidator" +# The scheduler strategies that a topology is allowed to select through topology.scheduler.strategy. +# Defaults to the strategies shipped with Storm; a custom strategy must be added to this list. +nimbus.scheduler.strategy.class.whitelist: + - "org.apache.storm.scheduler.resource.strategies.scheduling.DefaultResourceAwareStrategy" + - "org.apache.storm.scheduler.resource.strategies.scheduling.DefaultResourceAwareStrategyOld" + - "org.apache.storm.scheduler.resource.strategies.scheduling.GenericResourceAwareStrategy" + - "org.apache.storm.scheduler.resource.strategies.scheduling.GenericResourceAwareStrategyOld" + - "org.apache.storm.scheduler.resource.strategies.scheduling.RoundRobinResourceAwareStrategy" + - "org.apache.storm.scheduler.resource.strategies.scheduling.ConstraintSolverStrategy" topology.min.replication.count: 1 topology.max.replication.wait.time.sec: 60 nimbus.credential.renewers.freq.secs: 600 diff --git a/docs/Resource_Aware_Scheduler_overview.md b/docs/Resource_Aware_Scheduler_overview.md index 7fb0e31e2..353cc0f3e 100644 --- a/docs/Resource_Aware_Scheduler_overview.md +++ b/docs/Resource_Aware_Scheduler_overview.md @@ -251,6 +251,8 @@ A user can specify on a per topology basis what scheduling strategy to use. Use Parameters: * clazz – The strategy class that implements the IStrategy interface +Only the strategies that ship with Storm may be selected by default. A strategy of your own has to be added to `nimbus.scheduler.strategy.class.whitelist` in the nimbus configuration, otherwise nimbus refuses to schedule the topology and reports which class was refused. + Example Usage: ``` conf.setTopologyStrategy(org.apache.storm.scheduler.resource.strategies.scheduling.DefaultResourceAwareStrategy.class); diff --git a/storm-client/src/jvm/org/apache/storm/Config.java b/storm-client/src/jvm/org/apache/storm/Config.java index f9c09b897..f583e1e44 100644 --- a/storm-client/src/jvm/org/apache/storm/Config.java +++ b/storm-client/src/jvm/org/apache/storm/Config.java @@ -2069,7 +2069,8 @@ public class Config extends HashMap<String, Object> { valueValidatorClasses = {ConfigValidation.ImpersonationAclUserEntryValidator.class}) public static final String NIMBUS_IMPERSONATION_ACL = "nimbus.impersonation.acl"; /** - * A whitelist of the RAS scheduler strategies allowed by nimbus. Should be a list of fully-qualified class names or null to allow all. + * A whitelist of the RAS scheduler strategies allowed by nimbus. Should be a list of fully-qualified class names. When it is not + * set only the scheduler strategies shipped with Storm are allowed, so a custom strategy has to be listed here explicitly. */ @IsStringList public static final String NIMBUS_SCHEDULER_STRATEGY_CLASS_WHITELIST = "nimbus.scheduler.strategy.class.whitelist"; diff --git a/storm-client/src/jvm/org/apache/storm/utils/DisallowedStrategyException.java b/storm-client/src/jvm/org/apache/storm/utils/DisallowedStrategyException.java index 0b50a3748..fd93bfd6b 100644 --- a/storm-client/src/jvm/org/apache/storm/utils/DisallowedStrategyException.java +++ b/storm-client/src/jvm/org/apache/storm/utils/DisallowedStrategyException.java @@ -19,6 +19,7 @@ package org.apache.storm.utils; import java.util.List; +import org.apache.storm.Config; public class DisallowedStrategyException extends RuntimeException { private String attemptedClass; @@ -26,6 +27,9 @@ public class DisallowedStrategyException extends RuntimeException { public DisallowedStrategyException(String attemptedClass, List<String> allowedStrategies) { + super(attemptedClass + " is not an allowed scheduler strategy. Either pick one of the allowed strategies " + + allowedStrategies + " or add " + attemptedClass + " to the nimbus config " + + Config.NIMBUS_SCHEDULER_STRATEGY_CLASS_WHITELIST); this.attemptedClass = attemptedClass; this.allowedStrategies = allowedStrategies; } diff --git a/storm-client/src/jvm/org/apache/storm/utils/ReflectionUtils.java b/storm-client/src/jvm/org/apache/storm/utils/ReflectionUtils.java index 5a3ef12a9..4e41dda55 100644 --- a/storm-client/src/jvm/org/apache/storm/utils/ReflectionUtils.java +++ b/storm-client/src/jvm/org/apache/storm/utils/ReflectionUtils.java @@ -18,11 +18,26 @@ package org.apache.storm.utils; +import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; import org.apache.storm.Config; public class ReflectionUtils { + /** + * The scheduler strategies shipped with Storm. They are used when {@link Config#NIMBUS_SCHEDULER_STRATEGY_CLASS_WHITELIST} is not + * set at all, so that an unset config does not mean that any class on the nimbus classpath may be instantiated. The names are + * spelled out because the strategies live in storm-server. Keep this list in sync with conf/defaults.yaml. + */ + public static final List<String> DEFAULT_SCHEDULER_STRATEGIES = Collections.unmodifiableList(Arrays.asList( + "org.apache.storm.scheduler.resource.strategies.scheduling.DefaultResourceAwareStrategy", + "org.apache.storm.scheduler.resource.strategies.scheduling.DefaultResourceAwareStrategyOld", + "org.apache.storm.scheduler.resource.strategies.scheduling.GenericResourceAwareStrategy", + "org.apache.storm.scheduler.resource.strategies.scheduling.GenericResourceAwareStrategyOld", + "org.apache.storm.scheduler.resource.strategies.scheduling.RoundRobinResourceAwareStrategy", + "org.apache.storm.scheduler.resource.strategies.scheduling.ConstraintSolverStrategy")); + // A singleton instance allows us to mock delegated static methods in our // tests by subclassing. private static ReflectionUtils _instance = new ReflectionUtils(); @@ -76,7 +91,10 @@ public class ReflectionUtils { public static <T> T newSchedulerStrategyInstance(String klass, Map<String, Object> conf) { List<String> allowedSchedulerStrategies = (List<String>) conf.get(Config.NIMBUS_SCHEDULER_STRATEGY_CLASS_WHITELIST); - if (allowedSchedulerStrategies == null || allowedSchedulerStrategies.contains(klass)) { + if (allowedSchedulerStrategies == null) { + allowedSchedulerStrategies = DEFAULT_SCHEDULER_STRATEGIES; + } + if (allowedSchedulerStrategies.contains(klass)) { return newInstance(klass); } else { throw new DisallowedStrategyException(klass, allowedSchedulerStrategies); diff --git a/storm-client/test/jvm/org/apache/storm/utils/ReflectionUtilsTest.java b/storm-client/test/jvm/org/apache/storm/utils/ReflectionUtilsTest.java new file mode 100644 index 000000000..71ebc71e4 --- /dev/null +++ b/storm-client/test/jvm/org/apache/storm/utils/ReflectionUtilsTest.java @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.storm.utils; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.apache.storm.Config; +import org.junit.jupiter.api.Test; + +public class ReflectionUtilsTest { + + @Test + public void testSchedulerStrategyWithoutWhitelistOnlyAllowsShippedStrategies() { + Map<String, Object> conf = new HashMap<>(); + DisallowedStrategyException e = assertThrows(DisallowedStrategyException.class, + () -> ReflectionUtils.newSchedulerStrategyInstance("java.util.HashMap", conf)); + assertEquals("java.util.HashMap", e.getAttemptedClass()); + assertEquals(ReflectionUtils.DEFAULT_SCHEDULER_STRATEGIES, e.getAllowedStrategies()); + + for (String strategy : ReflectionUtils.DEFAULT_SCHEDULER_STRATEGIES) { + // The strategies themselves live in storm-server, so they cannot be loaded here, but they must pass the whitelist check. + RuntimeException notFound = assertThrows(RuntimeException.class, + () -> ReflectionUtils.newSchedulerStrategyInstance(strategy, conf)); + assertTrue(notFound.getCause() instanceof ClassNotFoundException, "unexpected failure for " + strategy); + } + } + + @Test + public void testSchedulerStrategyMessageNamesTheClassAndTheConfigToChange() { + DisallowedStrategyException e = assertThrows(DisallowedStrategyException.class, + () -> ReflectionUtils.newSchedulerStrategyInstance("com.example.CustomStrategy", new HashMap<>())); + assertTrue(e.getMessage().contains("com.example.CustomStrategy"), e.getMessage()); + assertTrue(e.getMessage().contains(Config.NIMBUS_SCHEDULER_STRATEGY_CLASS_WHITELIST), e.getMessage()); + } + + @Test + public void testSchedulerStrategyWithWhitelist() { + Map<String, Object> conf = new HashMap<>(); + conf.put(Config.NIMBUS_SCHEDULER_STRATEGY_CLASS_WHITELIST, Collections.singletonList("java.util.HashMap")); + Object instance = ReflectionUtils.newSchedulerStrategyInstance("java.util.HashMap", conf); + assertEquals(HashMap.class, instance.getClass()); + + conf.put(Config.NIMBUS_SCHEDULER_STRATEGY_CLASS_WHITELIST, Collections.emptyList()); + assertThrows(DisallowedStrategyException.class, () -> ReflectionUtils.newSchedulerStrategyInstance("java.util.HashMap", conf)); + } + + @Test + public void testDefaultsYamlMatchesTheShippedStrategies() { + List<String> fromDefaults = + (List<String>) Utils.readDefaultConfig().get(Config.NIMBUS_SCHEDULER_STRATEGY_CLASS_WHITELIST); + assertEquals(ReflectionUtils.DEFAULT_SCHEDULER_STRATEGIES, fromDefaults); + } +} diff --git a/storm-server/src/main/java/org/apache/storm/scheduler/resource/ResourceAwareScheduler.java b/storm-server/src/main/java/org/apache/storm/scheduler/resource/ResourceAwareScheduler.java index f2580b805..a113a425d 100644 --- a/storm-server/src/main/java/org/apache/storm/scheduler/resource/ResourceAwareScheduler.java +++ b/storm-server/src/main/java/org/apache/storm/scheduler/resource/ResourceAwareScheduler.java @@ -158,7 +158,9 @@ public class ResourceAwareScheduler implements IScheduler { + " is not an allowed strategy. Please make sure your " + Config.TOPOLOGY_SCHEDULER_STRATEGY + " config is one of the allowed strategies: " - + e.getAllowedStrategies(), e); + + e.getAllowedStrategies() + + ", or ask your administrator to add " + e.getAttemptedClass() + + " to the nimbus config " + Config.NIMBUS_SCHEDULER_STRATEGY_CLASS_WHITELIST, e); return; } catch (RuntimeException e) { markFailedTopology(topologySubmitter, cluster, td,
