Fix supervisor tombstone auth handling (#5504)

This commit is contained in:
Jonathan Wei 2018-03-19 12:55:47 -07:00 committed by GitHub
parent 693e3575f9
commit b22455b924
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 238 additions and 6 deletions

View File

@ -71,6 +71,7 @@ public class SupervisorManager
Preconditions.checkState(started, "SupervisorManager not started");
Preconditions.checkNotNull(spec, "spec");
Preconditions.checkNotNull(spec.getId(), "spec.getId()");
Preconditions.checkNotNull(spec.getDataSources(), "spec.getDatasources()");
synchronized (lock) {
Preconditions.checkState(started, "SupervisorManager not started");
@ -197,7 +198,7 @@ public class SupervisorManager
}
if (writeTombstone) {
metadataSupervisorManager.insert(id, new NoopSupervisorSpec()); // where NoopSupervisorSpec is a tombstone
metadataSupervisorManager.insert(id, new NoopSupervisorSpec(null, pair.rhs.getDataSources())); // where NoopSupervisorSpec is a tombstone
}
pair.lhs.stop(true);
supervisors.remove(id);
@ -232,7 +233,7 @@ public class SupervisorManager
catch (Exception e) {
// Supervisor creation or start failed write tombstone only when trying to start a new supervisor
if (persistSpec) {
metadataSupervisorManager.insert(id, new NoopSupervisorSpec());
metadataSupervisorManager.insert(id, new NoopSupervisorSpec(null, spec.getDataSources()));
}
throw Throwables.propagate(e);
}

View File

@ -47,6 +47,7 @@ import javax.ws.rs.Produces;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
@ -65,7 +66,7 @@ public class SupervisorResource
return null;
}
if (supervisorSpec.getSpec().getDataSources() == null) {
return null;
return new ArrayList<>();
}
return Iterables.transform(
supervisorSpec.getSpec().getDataSources(),

View File

@ -35,6 +35,7 @@ import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@ -288,7 +289,7 @@ public class SupervisorManagerTest extends EasyMockSupport
@Override
public List<String> getDataSources()
{
return null;
return new ArrayList<>();
}
}

View File

@ -19,6 +19,7 @@
package io.druid.indexing.overlord.supervisor;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@ -287,6 +288,10 @@ public class SupervisorResourceTest extends EasyMockSupport
new VersionedSupervisorSpec(
new TestSupervisorSpec("id1", null, Arrays.asList("datasource1")),
"v2"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, Arrays.asList("datasource1")),
"tombstone"
)
);
List<VersionedSupervisorSpec> versions2 = ImmutableList.of(
@ -297,11 +302,42 @@ public class SupervisorResourceTest extends EasyMockSupport
new VersionedSupervisorSpec(
new TestSupervisorSpec("id2", null, Arrays.asList("datasource2")),
"v2"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, Arrays.asList("datasource2")),
"tombstone"
),
new VersionedSupervisorSpec(
new TestSupervisorSpec("id2", null, Arrays.asList("datasource2")),
"v3"
)
);
List<VersionedSupervisorSpec> versions3 = ImmutableList.of(
new VersionedSupervisorSpec(
new TestSupervisorSpec("id2", null, Arrays.asList("datasource3")),
"v1"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, null),
"tombstone"
),
new VersionedSupervisorSpec(
new TestSupervisorSpec("id2", null, Arrays.asList("datasource3")),
"v2"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, null),
"tombstone"
),
new VersionedSupervisorSpec(
new TestSupervisorSpec("id2", null, Arrays.asList("datasource3")),
"v3"
)
);
Map<String, List<VersionedSupervisorSpec>> history = Maps.newHashMap();
history.put("id1", versions1);
history.put("id2", versions2);
history.put("id3", versions3);
EasyMock.expect(taskMaster.getSupervisorManager()).andReturn(Optional.of(supervisorManager)).times(2);
EasyMock.expect(supervisorManager.getSupervisorHistory()).andReturn(history);
@ -344,6 +380,10 @@ public class SupervisorResourceTest extends EasyMockSupport
new VersionedSupervisorSpec(
new TestSupervisorSpec("id1", null, Arrays.asList("datasource1")),
"v2"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, Arrays.asList("datasource1")),
"tombstone"
)
);
List<VersionedSupervisorSpec> versions2 = ImmutableList.of(
@ -354,12 +394,62 @@ public class SupervisorResourceTest extends EasyMockSupport
new VersionedSupervisorSpec(
new TestSupervisorSpec("id2", null, Arrays.asList("datasource2")),
"v2"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, Arrays.asList("datasource2")),
"tombstone"
)
);
List<VersionedSupervisorSpec> versions3 = ImmutableList.of(
new VersionedSupervisorSpec(
new TestSupervisorSpec("id2", null, Arrays.asList("datasource2")),
"v1"
),
new VersionedSupervisorSpec(
new TestSupervisorSpec("id2", null, Arrays.asList("datasource2")),
"v2"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, Arrays.asList("datasource2")),
"tombstone"
),
new VersionedSupervisorSpec(
new TestSupervisorSpec("id3", null, Arrays.asList("datasource3")),
"v1"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, Arrays.asList("datasource3")),
"tombstone"
)
);
List<VersionedSupervisorSpec> versions4 = ImmutableList.of(
new VersionedSupervisorSpec(
new TestSupervisorSpec("id2", null, Arrays.asList("datasource2")),
"v1"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, null),
"tombstone"
),
new VersionedSupervisorSpec(
new TestSupervisorSpec("id2", null, Arrays.asList("datasource2")),
"v2"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, null),
"tombstone"
),
new VersionedSupervisorSpec(
new TestSupervisorSpec("id2", null, Arrays.asList("datasource2")),
"v3"
)
);
Map<String, List<VersionedSupervisorSpec>> history = Maps.newHashMap();
history.put("id1", versions1);
history.put("id2", versions2);
history.put("id3", versions3);
history.put("id4", versions4);
EasyMock.expect(taskMaster.getSupervisorManager()).andReturn(Optional.of(supervisorManager)).times(2);
EasyMock.expect(supervisorManager.getSupervisorHistory()).andReturn(history);
@ -379,6 +469,32 @@ public class SupervisorResourceTest extends EasyMockSupport
Map<String, List<VersionedSupervisorSpec>> filteredHistory = Maps.newHashMap();
filteredHistory.put("id1", versions1);
filteredHistory.put(
"id3",
ImmutableList.of(
new VersionedSupervisorSpec(
new TestSupervisorSpec("id3", null, Arrays.asList("datasource3")),
"v1"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, Arrays.asList("datasource3")),
"tombstone"
)
)
);
filteredHistory.put(
"id4",
ImmutableList.of(
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, null),
"tombstone"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, null),
"tombstone"
)
)
);
Assert.assertEquals(200, response.getStatus());
Assert.assertEquals(filteredHistory, response.getEntity());
@ -402,6 +518,10 @@ public class SupervisorResourceTest extends EasyMockSupport
new TestSupervisorSpec("id1", null, Arrays.asList("datasource1")),
"v1"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, Arrays.asList("datasource1")),
"tombstone"
),
new VersionedSupervisorSpec(
new TestSupervisorSpec("id1", null, Arrays.asList("datasource1")),
"v2"
@ -412,6 +532,10 @@ public class SupervisorResourceTest extends EasyMockSupport
new TestSupervisorSpec("id2", null, Arrays.asList("datasource2")),
"v1"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, Arrays.asList("datasource2")),
"tombstone"
),
new VersionedSupervisorSpec(
new TestSupervisorSpec("id2", null, Arrays.asList("datasource2")),
"v2"
@ -464,6 +588,10 @@ public class SupervisorResourceTest extends EasyMockSupport
new TestSupervisorSpec("id1", null, Arrays.asList("datasource1")),
"v1"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, Arrays.asList("datasource3")),
"tombstone"
),
new VersionedSupervisorSpec(
new TestSupervisorSpec("id1", null, Arrays.asList("datasource1")),
"v2"
@ -474,6 +602,10 @@ public class SupervisorResourceTest extends EasyMockSupport
new TestSupervisorSpec("id2", null, Arrays.asList("datasource2")),
"v1"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, Arrays.asList("datasource2")),
"tombstone"
),
new VersionedSupervisorSpec(
new TestSupervisorSpec("id2", null, Arrays.asList("datasource2")),
"v2"
@ -484,9 +616,25 @@ public class SupervisorResourceTest extends EasyMockSupport
new TestSupervisorSpec("id3", null, Arrays.asList("datasource3")),
"v1"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, null),
"tombstone"
),
new VersionedSupervisorSpec(
new TestSupervisorSpec("id3", null, Arrays.asList("datasource2")),
"v2"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, null),
"tombstone"
),
new VersionedSupervisorSpec(
new TestSupervisorSpec("id3", null, Arrays.asList("datasource3")),
"v2"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, Arrays.asList("datasource3")),
"tombstone"
)
);
Map<String, List<VersionedSupervisorSpec>> history = Maps.newHashMap();
@ -521,6 +669,22 @@ public class SupervisorResourceTest extends EasyMockSupport
new VersionedSupervisorSpec(
new TestSupervisorSpec("id3", null, Arrays.asList("datasource3")),
"v1"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, null),
"tombstone"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, null),
"tombstone"
),
new VersionedSupervisorSpec(
new TestSupervisorSpec("id3", null, Arrays.asList("datasource3")),
"v2"
),
new VersionedSupervisorSpec(
new NoopSupervisorSpec(null, Arrays.asList("datasource3")),
"tombstone"
)
),
response.getEntity()
@ -574,6 +738,23 @@ public class SupervisorResourceTest extends EasyMockSupport
verifyAll();
}
@Test
public void testNoopSupervisorSpecSerde() throws Exception
{
ObjectMapper mapper = new ObjectMapper();
String oldSpec = "{\"type\":\"NoopSupervisorSpec\",\"id\":null,\"dataSources\":null}";
NoopSupervisorSpec expectedSpec = new NoopSupervisorSpec(null, null);
NoopSupervisorSpec deserializedSpec = mapper.readValue(oldSpec, NoopSupervisorSpec.class);
Assert.assertEquals(expectedSpec, deserializedSpec);
NoopSupervisorSpec spec1 = new NoopSupervisorSpec("abcd", Lists.newArrayList("defg"));
NoopSupervisorSpec spec2 = mapper.readValue(
mapper.writeValueAsBytes(spec1),
NoopSupervisorSpec.class
);
Assert.assertEquals(spec1, spec2);
}
private static class TestSupervisorSpec implements SupervisorSpec
{
private final String id;

View File

@ -19,20 +19,46 @@
package io.druid.indexing.overlord.supervisor;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import io.druid.indexing.overlord.DataSourceMetadata;
import javax.annotation.Nullable;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
/**
* Used as a tombstone marker in the supervisors metadata table to indicate that the supervisor has been removed.
*/
public class NoopSupervisorSpec implements SupervisorSpec
{
// NoopSupervisorSpec is used as a tombstone, added when a previously running supervisor is stopped.
// Inherit the datasources of the previous running spec, so that we can determine whether a user is authorized to see
// this tombstone (users can only see tombstones for datasources that they have access to).
@Nullable
@JsonProperty("dataSources")
private List<String> datasources;
@Nullable
@JsonProperty("id")
private String id;
@JsonCreator
public NoopSupervisorSpec(
@Nullable @JsonProperty("id") String id,
@Nullable @JsonProperty("dataSources") List<String> datasources
)
{
this.id = id;
this.datasources = datasources == null ? new ArrayList<>() : datasources;
}
@Override
@JsonProperty
public String getId()
{
return null;
return id;
}
@Override
@ -68,8 +94,30 @@ public class NoopSupervisorSpec implements SupervisorSpec
}
@Override
@Nullable
@JsonProperty("dataSources")
public List<String> getDataSources()
{
return null;
return datasources;
}
@Override
public boolean equals(Object o)
{
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
NoopSupervisorSpec spec = (NoopSupervisorSpec) o;
return Objects.equals(datasources, spec.datasources) &&
Objects.equals(getId(), spec.getId());
}
@Override
public int hashCode()
{
return Objects.hash(datasources, getId());
}
}