Give useful error message if log config is missing

Today when starting Elasticsearch without a Log4j 2 configuration file,
we end up throwing an array index out of bounds exception. This is
because we are passing no configuration files to Log4j. Instead, we
should throw a useful error message to the user. This commit modifies
the Log4j configuration setup to throw a user exception if no Log4j
configuration files are present in the config directory.

Relates #20493
This commit is contained in:
Jason Tedor 2016-09-15 07:44:05 -04:00 committed by GitHub
parent a5f03b4bc5
commit 7132fcd7ac
6 changed files with 34 additions and 9 deletions

View File

@ -28,6 +28,7 @@ import org.apache.lucene.util.StringHelper;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.PidFile;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.inject.CreationException;
@ -233,7 +234,7 @@ final class Bootstrap {
final boolean foreground,
final Path pidFile,
final boolean quiet,
final Map<String, String> esSettings) throws BootstrapException, NodeValidationException {
final Map<String, String> esSettings) throws BootstrapException, NodeValidationException, UserException {
// Set the system property before anything has a chance to trigger its use
initLoggerPrefix();

View File

@ -107,7 +107,7 @@ class Elasticsearch extends SettingCommand {
}
void init(final boolean daemonize, final Path pidFile, final boolean quiet, final Map<String, String> esSettings)
throws NodeValidationException {
throws NodeValidationException, UserException {
try {
Bootstrap.init(!daemonize, pidFile, quiet, esSettings);
} catch (BootstrapException | RuntimeException e) {

View File

@ -30,6 +30,8 @@ import org.apache.logging.log4j.core.config.builder.impl.BuiltConfiguration;
import org.apache.logging.log4j.core.config.composite.CompositeConfiguration;
import org.apache.logging.log4j.core.config.properties.PropertiesConfiguration;
import org.apache.logging.log4j.core.config.properties.PropertiesConfigurationFactory;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.settings.Settings;
@ -50,7 +52,7 @@ import java.util.Set;
public class LogConfigurator {
public static void configure(final Environment environment, final boolean resolveConfig) throws IOException {
public static void configure(final Environment environment, final boolean resolveConfig) throws IOException, UserException {
final Settings settings = environment.settings();
setLogConfigurationSystemProperty(environment, settings);
@ -75,6 +77,13 @@ public class LogConfigurator {
return FileVisitResult.CONTINUE;
}
});
if (configurations.isEmpty()) {
throw new UserException(
ExitCodes.CONFIG,
"no log4j2.properties found; tried [" + environment.configFile() + "] and its subdirectories");
}
context.start(new CompositeConfiguration(configurations));
}

View File

@ -27,6 +27,7 @@ import org.apache.logging.log4j.core.LoggerContext;
import org.apache.logging.log4j.core.config.Configuration;
import org.apache.logging.log4j.core.config.Configurator;
import org.apache.logging.log4j.core.config.LoggerConfig;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ESTestCase;
@ -34,7 +35,9 @@ import org.elasticsearch.test.ESTestCase;
import java.io.IOException;
import java.nio.file.Path;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.Matchers.hasToString;
import static org.hamcrest.Matchers.notNullValue;
public class EvilLoggerConfigurationTests extends ESTestCase {
@ -85,7 +88,7 @@ public class EvilLoggerConfigurationTests extends ESTestCase {
}
}
public void testDefaults() throws IOException {
public void testDefaults() throws IOException, UserException {
final Path configDir = getDataPath("config");
final String level = randomFrom(Level.TRACE, Level.DEBUG, Level.INFO, Level.WARN, Level.ERROR).toString();
final Settings settings = Settings.builder()
@ -137,4 +140,15 @@ public class EvilLoggerConfigurationTests extends ESTestCase {
assertThat(ESLoggerFactory.getLogger("x.y").getLevel(), equalTo(level));
}
public void testMissingConfigFile() {
final Path configDir = getDataPath("does_not_exist");
final Settings settings = Settings.builder()
.put(Environment.PATH_CONF_SETTING.getKey(), configDir.toAbsolutePath())
.put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString())
.build();
final Environment environment = new Environment(settings);
UserException e = expectThrows(UserException.class, () -> LogConfigurator.configure(environment, true));
assertThat(e, hasToString(containsString("no log4j2.properties found; tried")));
}
}

View File

@ -28,6 +28,7 @@ import org.apache.logging.log4j.core.appender.ConsoleAppender;
import org.apache.logging.log4j.core.appender.CountingNoOpAppender;
import org.apache.logging.log4j.core.config.Configurator;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
@ -59,7 +60,7 @@ public class EvilLoggerTests extends ESTestCase {
super.tearDown();
}
public void testLocationInfoTest() throws IOException {
public void testLocationInfoTest() throws IOException, UserException {
setupLogging("location_info");
final Logger testLogger = ESLoggerFactory.getLogger("test");
@ -81,7 +82,7 @@ public class EvilLoggerTests extends ESTestCase {
assertLogLine(events.get(4), Level.TRACE, location, "This is a trace message");
}
public void testDeprecationLogger() throws IOException {
public void testDeprecationLogger() throws IOException, UserException {
setupLogging("deprecation");
final DeprecationLogger deprecationLogger = new DeprecationLogger(ESLoggerFactory.getLogger("deprecation"));
@ -97,7 +98,7 @@ public class EvilLoggerTests extends ESTestCase {
"This is a deprecation message");
}
public void testFindAppender() throws IOException {
public void testFindAppender() throws IOException, UserException {
setupLogging("find_appender");
final Logger hasConsoleAppender = ESLoggerFactory.getLogger("has_console_appender");
@ -111,7 +112,7 @@ public class EvilLoggerTests extends ESTestCase {
assertThat(countingNoOpAppender.getName(), equalTo("counting_no_op"));
}
public void testPrefixLogger() throws IOException, IllegalAccessException {
public void testPrefixLogger() throws IOException, IllegalAccessException, UserException {
setupLogging("prefix");
final String prefix = randomBoolean() ? null : randomAsciiOfLength(16);
@ -179,7 +180,7 @@ public class EvilLoggerTests extends ESTestCase {
}
}
private void setupLogging(final String config) throws IOException {
private void setupLogging(final String config) throws IOException, UserException {
final Path configDir = getDataPath(config);
// need to set custom path.conf so we can use a custom log4j2.properties file for the test
final Settings settings = Settings.builder()