Guice: Ensure no exceptions are thrown in constructor

Because this leads to endless loops when starting elasticsearch
some components have been refactored to AbstractLifecycleComponents
so that the exception throwing logic can executed in the
`doStart()` method.

Closes elastic/elasticsearch#505

Original commit: elastic/x-pack-elasticsearch@75d1fd358a
This commit is contained in:
Alexander Reelsen 2014-12-18 12:33:39 +01:00
parent 9d5dc3552b
commit 38a0ec9c3e
7 changed files with 47 additions and 22 deletions

View File

@ -14,8 +14,10 @@ import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.plugins.AbstractPlugin;
import org.elasticsearch.shield.authc.Realms;
import org.elasticsearch.shield.authc.support.SecuredString;
import org.elasticsearch.shield.authc.support.UsernamePasswordToken;
import org.elasticsearch.shield.authz.store.FileRolesStore;
import org.elasticsearch.shield.license.LicenseService;
import java.io.File;
@ -59,7 +61,7 @@ public class ShieldPlugin extends AbstractPlugin {
@Override
public Collection<Class<? extends LifecycleComponent>> services() {
return enabled && !clientMode ?
ImmutableList.<Class<? extends LifecycleComponent>>of(LicenseService.class) :
ImmutableList.<Class<? extends LifecycleComponent>>of(LicenseService.class, FileRolesStore.class, Realms.class) :
ImmutableList.<Class<? extends LifecycleComponent>>of();
}

View File

@ -6,37 +6,45 @@
package org.elasticsearch.shield.authc;
import org.apache.lucene.util.CollectionUtil;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.collect.Lists;
import org.elasticsearch.common.collect.Sets;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.component.AbstractLifecycleComponent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.shield.ShieldSettingsException;
import org.elasticsearch.shield.authc.esusers.ESUsersRealm;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.*;
import java.util.concurrent.CopyOnWriteArrayList;
/**
* Serves as a realms registry (also responsible for ordering the realms appropriately)
*/
public class Realms extends AbstractComponent implements Iterable<Realm> {
public class Realms extends AbstractLifecycleComponent<Realms> implements Iterable<Realm> {
private final Map<String, Realm.Factory> factories;
private final List<Realm> realms;
private List<Realm> realms = Collections.EMPTY_LIST;
@Inject
public Realms(Settings settings, Map<String, Realm.Factory> factories) {
super(settings);
this.factories = factories;
}
@Override
protected void doStart() throws ElasticsearchException {
realms = new CopyOnWriteArrayList<>(initRealms());
}
@Override
protected void doStop() throws ElasticsearchException {}
@Override
protected void doClose() throws ElasticsearchException {}
@Override
public Iterator<Realm> iterator() {
return realms.iterator();
@ -112,4 +120,5 @@ public class Realms extends AbstractComponent implements Iterable<Realm> {
}
return result != null ? result : ImmutableSettings.EMPTY;
}
}

View File

@ -21,7 +21,9 @@ public class AuthorizationModule extends AbstractShieldModule.Node {
@Override
protected void configureNode() {
bind(FileRolesStore.class).asEagerSingleton();
bind(RolesStore.class).to(FileRolesStore.class).asEagerSingleton();
bind(AuthorizationService.class).to(InternalAuthorizationService.class).asEagerSingleton();
}
}

View File

@ -8,13 +8,11 @@ package org.elasticsearch.shield.authz.store;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.ImmutableMap;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.component.AbstractLifecycleComponent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.jackson.dataformat.yaml.snakeyaml.error.YAMLException;
import org.elasticsearch.common.logging.ESLogger;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.yaml.YamlXContent;
import org.elasticsearch.env.Environment;
@ -30,20 +28,18 @@ import org.elasticsearch.watcher.ResourceWatcherService;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;
/**
*
*/
public class FileRolesStore extends AbstractComponent implements RolesStore {
public class FileRolesStore extends AbstractLifecycleComponent<RolesStore> implements RolesStore {
private static final Pattern COMMA_DELIM = Pattern.compile("\\s*,\\s*");
@ -59,12 +55,26 @@ public class FileRolesStore extends AbstractComponent implements RolesStore {
public FileRolesStore(Settings settings, Environment env, ResourceWatcherService watcherService, Listener listener) {
super(settings);
file = resolveFile(settings, env);
permissions = parseFile(file, logger);
this.file = resolveFile(settings, env);
this.listener = listener;
permissions = ImmutableMap.of();
FileWatcher watcher = new FileWatcher(file.getParent().toFile());
watcher.addListener(new FileListener());
watcherService.add(watcher, ResourceWatcherService.Frequency.HIGH);
this.listener = listener;
}
@Override
protected void doStart() throws ElasticsearchException {
permissions = parseFile(file, logger);
}
@Override
protected void doStop() throws ElasticsearchException {
}
@Override
protected void doClose() throws ElasticsearchException {
}
@Override

View File

@ -65,6 +65,7 @@ public class InternalAuthenticationServiceTests extends ElasticsearchTestCase {
return ImmutableList.of(firstRealm, secondRealm);
}
};
realms.start();
signatureService = mock(SignatureService.class);
auditTrail = mock(AuditTrail.class);

View File

@ -5,7 +5,6 @@
*/
package org.elasticsearch.shield.authc;
import com.carrotsearch.randomizedtesting.annotations.Repeat;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.rest.RestRequest;
@ -19,9 +18,7 @@ import org.junit.Test;
import java.util.*;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.*;
/**
*
@ -55,6 +52,7 @@ public class RealmsTests extends ElasticsearchTestCase {
orderToIndex.put(orders.get(i), i);
}
Realms realms = new Realms(builder.build(), factories);
realms.start();
int i = 0;
for (Realm realm : realms) {
assertThat(realm.order(), equalTo(i));
@ -72,12 +70,13 @@ public class RealmsTests extends ElasticsearchTestCase {
builder.put("shield.authc.realms.realm_1.order", 0);
builder.put("shield.authc.realms.realm_2.type", ESUsersRealm.TYPE);
builder.put("shield.authc.realms.realm_2.order", 1);
new Realms(builder.build(), factories);
new Realms(builder.build(), factories).start();
}
@Test
public void testWithEmptySettings() throws Exception {
Realms realms = new Realms(ImmutableSettings.EMPTY, factories);
realms.start();
Iterator<Realm> iter = realms.iterator();
assertThat(iter.hasNext(), is(true));
Realm realm = iter.next();
@ -109,6 +108,7 @@ public class RealmsTests extends ElasticsearchTestCase {
Settings settings = builder.build();
Realms realms = new Realms(settings, factories);
realms.start();
Iterator<Realm> iterator = realms.iterator();
int count = 0;

View File

@ -151,6 +151,7 @@ public class FileRolesStoreTests extends ElasticsearchTestCase {
latch.countDown();
}
});
store.start();
Permission.Global.Role role = store.role("role1");
assertThat(role, notNullValue());