Skip to content

Commit cda2669

Browse files
authored
Truncate step_info and error reason in ILM execution state and history (#125054)
This commit adds a limit to the `step_info` contained in `LifecycleExcutionState` so that large step info messages will not be stored in the cluster state. Additionally, when generating an ILM history failure, the full exception that is "stringified" is truncated to the same limit, ensuring that we do not accidentally index gigantic documents in the history store. The default limit is 1024 characters. Resolves #124181
1 parent fa46b87 commit cda2669

File tree

5 files changed

+81
-2
lines changed

5 files changed

+81
-2
lines changed

docs/changelog/125054.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 125054
2+
summary: Truncate `step_info` and error reason in ILM execution state and history
3+
area: ILM+SLM
4+
type: enhancement
5+
issues:
6+
- 124181

server/src/main/java/org/elasticsearch/cluster/metadata/LifecycleExecutionState.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
package org.elasticsearch.cluster.metadata;
1111

1212
import org.elasticsearch.ElasticsearchException;
13+
import org.elasticsearch.common.Strings;
1314

1415
import java.util.Collections;
1516
import java.util.HashMap;
@@ -42,6 +43,7 @@ public record LifecycleExecutionState(
4243
) {
4344

4445
public static final String ILM_CUSTOM_METADATA_KEY = "ilm";
46+
public static final int MAXIMUM_STEP_INFO_STRING_LENGTH = 1024;
4547

4648
private static final String PHASE = "phase";
4749
private static final String ACTION = "action";
@@ -267,6 +269,17 @@ public Map<String, String> asMap() {
267269
return Collections.unmodifiableMap(result);
268270
}
269271

272+
public static String truncateWithExplanation(String input) {
273+
if (input != null && input.length() > MAXIMUM_STEP_INFO_STRING_LENGTH) {
274+
return Strings.cleanTruncate(input, MAXIMUM_STEP_INFO_STRING_LENGTH)
275+
+ "... ("
276+
+ (input.length() - MAXIMUM_STEP_INFO_STRING_LENGTH)
277+
+ " chars truncated)";
278+
} else {
279+
return input;
280+
}
281+
}
282+
270283
public static class Builder {
271284
private String phase;
272285
private String action;
@@ -308,7 +321,7 @@ public Builder setFailedStep(String failedStep) {
308321
}
309322

310323
public Builder setStepInfo(String stepInfo) {
311-
this.stepInfo = stepInfo;
324+
this.stepInfo = truncateWithExplanation(stepInfo);
312325
return this;
313326
}
314327

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecycleExecutionStateTests.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
import java.util.HashMap;
1515
import java.util.Map;
1616

17+
import static org.hamcrest.Matchers.equalTo;
18+
1719
public class LifecycleExecutionStateTests extends ESTestCase {
1820

1921
public void testConversion() {
@@ -22,6 +24,20 @@ public void testConversion() {
2224
assertEquals(customMetadata, parsed.asMap());
2325
}
2426

27+
public void testTruncatingStepInfo() {
28+
Map<String, String> custom = createCustomMetadata();
29+
LifecycleExecutionState state = LifecycleExecutionState.fromCustomMetadata(custom);
30+
assertThat(custom.get("step_info"), equalTo(state.stepInfo()));
31+
String longStepInfo = randomAlphanumericOfLength(LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH + 100);
32+
LifecycleExecutionState newState = LifecycleExecutionState.builder(state).setStepInfo(longStepInfo).build();
33+
// Length includes the post suffix
34+
assertThat(newState.stepInfo().length(), equalTo(LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH + 25));
35+
assertThat(
36+
newState.stepInfo().substring(LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH, 1049),
37+
equalTo("... (100 chars truncated)")
38+
);
39+
}
40+
2541
public void testEmptyValuesAreNotSerialized() {
2642
LifecycleExecutionState empty = LifecycleExecutionState.builder().build();
2743
assertEquals(new HashMap<String, String>().entrySet(), empty.asMap().entrySet());

x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/history/ILMHistoryItem.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,15 @@ public static ILMHistoryItem failure(
8686
Exception error
8787
) {
8888
Objects.requireNonNull(error, "ILM failures require an attached exception");
89-
return new ILMHistoryItem(index, policyId, timestamp, indexAge, false, executionState, exceptionToString(error));
89+
String fullErrorString = exceptionToString(error);
90+
String truncatedErrorString = LifecycleExecutionState.truncateWithExplanation(fullErrorString);
91+
if (truncatedErrorString.equals(fullErrorString) == false) {
92+
// Append a closing quote and closing brace to attempt to make it valid JSON.
93+
// There is no requirement that it actually be valid JSON, so this is
94+
// best-effort, but does not cause problems if it is still invalid.
95+
truncatedErrorString += "\"}";
96+
}
97+
return new ILMHistoryItem(index, policyId, timestamp, indexAge, false, executionState, truncatedErrorString);
9098
}
9199

92100
@Override

x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/history/ILMHistoryItemTests.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,13 @@
1313
import org.elasticsearch.test.ESTestCase;
1414
import org.elasticsearch.xcontent.ToXContent;
1515
import org.elasticsearch.xcontent.XContentBuilder;
16+
import org.elasticsearch.xcontent.XContentFactory;
17+
import org.elasticsearch.xcontent.XContentParser;
18+
import org.elasticsearch.xcontent.XContentParserConfiguration;
19+
import org.elasticsearch.xcontent.XContentType;
1620

1721
import java.io.IOException;
22+
import java.util.Map;
1823

1924
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
2025
import static org.hamcrest.Matchers.equalTo;
@@ -111,4 +116,35 @@ public void testToXContent() throws IOException {
111116
\\"stack_trace\\":\\"java.lang.IllegalArgumentException: failure""".replaceAll("\\s", "")));
112117
}
113118
}
119+
120+
public void testTruncateLongError() throws IOException {
121+
String longError = randomAlphaOfLength(LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH + 20);
122+
123+
ILMHistoryItem failure = ILMHistoryItem.failure(
124+
"index",
125+
"policy",
126+
1234L,
127+
100L,
128+
LifecycleExecutionState.EMPTY_STATE,
129+
new IllegalArgumentException(longError)
130+
);
131+
132+
try (XContentBuilder builder = jsonBuilder()) {
133+
failure.toXContent(builder, ToXContent.EMPTY_PARAMS);
134+
String json = Strings.toString(builder);
135+
try (XContentParser p = XContentFactory.xContent(XContentType.JSON).createParser(XContentParserConfiguration.EMPTY, json)) {
136+
Map<String, Object> item = p.map();
137+
assertThat(
138+
item.get("error_details"),
139+
equalTo(
140+
"{\"type\":\"illegal_argument_exception\",\"reason\":\""
141+
// We subtract a number of characters here due to the truncation being based
142+
// on the length of the whole string, not just the "reason" part.
143+
+ longError.substring(0, LifecycleExecutionState.MAXIMUM_STEP_INFO_STRING_LENGTH - 47)
144+
+ "... (5126 chars truncated)\"}"
145+
)
146+
);
147+
}
148+
}
149+
}
114150
}

0 commit comments

Comments
 (0)