Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2433#discussion_r173232056
--- Diff:
storm-server/src/main/java/org/apache/storm/daemon/supervisor/Supervisor.java
---
@@ -234,6 +295,60 @@ public void launchDaemon() {
}
}
+ private void launchSupervisorThriftServer(Map conf) throws IOException
{
+ // validate port
+ int port = getThriftServerPort();
+ try {
+ ServerSocket socket = new ServerSocket(port);
+ socket.close();
+ } catch (BindException e) {
+ LOG.error("{} is not available. Check if another process is
already listening on {}", port, port);
+ throw new RuntimeException(e);
+ }
+
+ TProcessor processor = new
org.apache.storm.generated.Supervisor.Processor(
+ new org.apache.storm.generated.Supervisor.Iface() {
+ @Override
+ public void
sendSupervisorAssignments(SupervisorAssignments assignments)
+ throws AuthorizationException, TException {
+ LOG.info("Got an assignments from master, will
start to sync with assignments: {}", assignments);
+ SynchronizeAssignments syn = new
SynchronizeAssignments(getSupervisor(), assignments, getReadClusterState());
+ getEventManger().add(syn);
+ }
+
+ @Override
+ public Assignment getLocalAssignmentForStorm(String id)
+ throws NotAliveException,
AuthorizationException, TException {
+ Assignment assignment =
getStormClusterState().assignmentInfo(id, null);
+ if (null == assignment) {
+ throw new NotAliveException("No local
assignment assigned for storm: " + id + " for node: " + getHostName());
+ }
+ return assignment;
+ }
+
+ @Override
+ public void
sendSupervisorWorkerHeartbeat(SupervisorWorkerHeartbeat heartbeat)
+ throws AuthorizationException, TException {
+ // do nothing now
+ }
--- End diff --
Sorry I forgot this before. All of these must have some kind of
authorization checks. We have authenticated the user connecting, but right now
anyone with valid Kerberos credentials or a valid WorkerToken can call these
APIs. We need something that can block users that should not be calling them,
and with the ability to turn it off for a non-secure cluster.
`sendSupervisorAssignments` is the biggest security problem. It needs to
be restricted to only nimbus making that call. `getLocalAssignmentForStrom` is
probably okay to be totally open, but it might be good to restrict it to just
the owner of that topology + nimbus. Similar for
`sendSupervisorWorkerHeartbeat`. It is a noop right now so not that big of a
deal, but in the future I would expect us to want to restrict it.
Please take a look at how nimbus is doing these checks.
---
