honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Watcher: Fix possible chained input NPE for execution failures (elastic/elasticsearch#3490) 

Due to untested code there was an NPE happening in production,
when a chained input execution failed, but the chained input
tried to access the resulting payload (which is never set on
failures). This payload now defaults to being empty.

This commit also drive-by fixes a broken logging statement, that
on the one side returned not the watch id, but a useless watch
toString() representation, and on the other hand only logs an error
message, but not a stack trace into the log, as this is what the
history is for.

Original commit: elastic/x-pack-elasticsearch@7dbe1afd90
This commit is contained in:
Alexander Reelsen 2016-09-19 10:06:02 +02:00 committed by GitHub
parent 5b265ea569
commit 273a9fb46f
6 changed files with 80 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/input/Input.java
View File

@ -43,7 +43,7 @@ public interface Input extends ToXContent {
this.status = Status.FAILURE;
this.type = type;
this.reason = ExceptionsHelper.detailedMessage(e);
this.payload = null;
this.payload = Payload.EMPTY;
}
public String type() {
@ -55,7 +55,6 @@ public interface Input extends ToXContent {
}
public Payload payload() {
assert status == Status.SUCCESS;
return payload;
}

4
elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/input/chain/ExecutableChainInput.java
View File

@ -6,8 +6,6 @@
package org.elasticsearch.xpack.watcher.input.chain;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.xpack.watcher.execution.WatchExecutionContext;
import org.elasticsearch.xpack.watcher.input.ExecutableInput;
@ -44,7 +42,7 @@ public class ExecutableChainInput extends ExecutableInput<ChainInput,ChainInput.
return new ChainInput.Result(results, new Payload.Simple(payloads));
} catch (Exception e) {
logger.error((Supplier<?>) () -> new ParameterizedMessage("failed to execute [{}] input for [{}]", TYPE, ctx.watch().id()), e);
logger.error("failed to execute [{}] input for watch [{}], reason [{}]", TYPE, ctx.watch().id(), e.getMessage());
return new ChainInput.Result(e);
}
}

4
elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/input/http/ExecutableHttpInput.java
View File

@ -7,8 +7,6 @@ package org.elasticsearch.xpack.watcher.input.http;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
@ -49,7 +47,7 @@ public class ExecutableHttpInput extends ExecutableInput<HttpInput, HttpInput.Re
request = input.getRequest().render(templateEngine, model);
return doExecute(ctx, request);
} catch (Exception e) {
logger.error((Supplier<?>) () -> new ParameterizedMessage("failed to execute [{}] input for [{}]", TYPE, ctx.watch()), e);
logger.error("failed to execute [{}] input for watch [{}], reason [{}]", TYPE, ctx.watch().id(), e.getMessage());
return new HttpInput.Result(request, e);
}
}

4
elasticsearch/x-pack/watcher/src/main/java/org/elasticsearch/xpack/watcher/input/search/ExecutableSearchInput.java
View File

@ -6,8 +6,6 @@
package org.elasticsearch.xpack.watcher.input.search;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.SearchType;
import org.elasticsearch.common.Nullable;
@ -60,7 +58,7 @@ public class ExecutableSearchInput extends ExecutableInput<SearchInput, SearchIn
request = new WatcherSearchTemplateRequest(input.getRequest(), renderedTemplate);
return doExecute(ctx, request);
} catch (Exception e) {
logger.error((Supplier<?>) () -> new ParameterizedMessage("failed to execute [{}] input for [{}]", TYPE, ctx.watch()), e);
logger.error("failed to execute [{}] input for watch [{}], reason [{}]", TYPE, ctx.watch().id(), e.getMessage());
return new SearchInput.Result(request, e);
}
}

9
elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/input/chain/ChainIntegrationTests.java
View File

@ -12,9 +12,9 @@ import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.transport.Netty3Plugin;
import org.elasticsearch.transport.Netty4Plugin;
import org.elasticsearch.xpack.watcher.input.http.HttpInput;
import org.elasticsearch.xpack.common.http.HttpRequestTemplate;
import org.elasticsearch.xpack.common.http.auth.basic.BasicAuth;
import org.elasticsearch.xpack.watcher.input.http.HttpInput;
import org.elasticsearch.xpack.watcher.test.AbstractWatcherIntegrationTestCase;
import java.net.InetSocketAddress;
@ -79,12 +79,7 @@ public class ChainIntegrationTests extends AbstractWatcherIntegrationTestCase {
timeWarp().scheduler().trigger("_name");
refresh();
} else {
assertBusy(new Runnable() {
@Override
public void run() {
assertWatchExecuted();
}
}, 9, TimeUnit.SECONDS);
assertBusy(() -> assertWatchExecuted(), 9, TimeUnit.SECONDS);
}
assertWatchWithMinimumPerformedActionsCount("_name", 1, false);

74
elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/input/chain/ExecutableChainInputTests.java Normal file
View File

@ -0,0 +1,74 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.watcher.input.chain;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.watcher.execution.WatchExecutionContext;
import org.elasticsearch.xpack.watcher.execution.Wid;
import org.elasticsearch.xpack.watcher.input.ExecutableInput;
import org.elasticsearch.xpack.watcher.input.Input;
import org.elasticsearch.xpack.watcher.input.simple.SimpleInput;
import org.elasticsearch.xpack.watcher.watch.Payload;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import java.io.IOException;
import java.util.Arrays;
import static org.elasticsearch.xpack.watcher.input.Input.Result.Status;
import static org.elasticsearch.xpack.watcher.test.WatcherTestUtils.mockExecutionContextBuilder;
import static org.hamcrest.Matchers.is;
public class ExecutableChainInputTests extends ESTestCase {
public void testFailedResultHandling() throws Exception {
WatchExecutionContext ctx = createWatchExecutionContext();
ChainInput chainInput = new ChainInput(Arrays.asList(new Tuple<>("whatever", new SimpleInput(Payload.EMPTY))));
Tuple<String, ExecutableInput> tuple = new Tuple<>("whatever", new FailingExecutableInput());
ExecutableChainInput executableChainInput = new ExecutableChainInput(chainInput, Arrays.asList(tuple), logger);
ChainInput.Result result = executableChainInput.execute(ctx, Payload.EMPTY);
assertThat(result.status(), is(Status.SUCCESS));
}
private class FailingExecutableInput extends ExecutableInput<SimpleInput, Input.Result> {
protected FailingExecutableInput() {
super(new SimpleInput(Payload.EMPTY), ExecutableChainInputTests.this.logger);
}
@Override
public Input.Result execute(WatchExecutionContext ctx, @Nullable Payload payload) {
return new FailingExecutableInputResult(new RuntimeException("foo"));
}
}
private static class FailingExecutableInputResult extends Input.Result {
protected FailingExecutableInputResult(Exception e) {
super("failing", e);
}
@Override
protected XContentBuilder typeXContent(XContentBuilder builder, Params params) throws IOException {
return builder;
}
}
private WatchExecutionContext createWatchExecutionContext() {
DateTime now = DateTime.now(DateTimeZone.UTC);
Wid wid = new Wid(randomAsciiOfLength(5), randomLong(), now);
return mockExecutionContextBuilder(wid.watchId())
.wid(wid)
.payload(new Payload.Simple())
.time(wid.watchId(), now)
.buildMock();
}
}