Skip to content

Commit 8c3b83a

Browse files
talevytalevy
authored
Add rollup config validation (elastic#69582) (elastic#70118)
The RollupActionConfig objects had validation of mappings, but the TransportRollupAction never leveraged them to validate the action. This commit introduces a new first step for validation before continuing with the rollup. validation exception messages had "... across all indices" removed from the messages of RollupAction objects, but legacy objects were left untouched since they still interact with index patterns and RollupAction is always against one index.
1 parent 2695fc1 commit 8c3b83a

File tree

6 files changed

+206
-57
lines changed

6 files changed

+206
-57
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/RollupActionDateHistogramGroupConfig.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -247,21 +247,20 @@ public void validateMappings(Map<String, Map<String, FieldCapabilities>> fieldCa
247247
if (fieldCaps.get(dateType).isAggregatable()) {
248248
return;
249249
} else {
250-
validationException.addValidationError("The field [" + field + "] must be aggregatable across all indices, " +
250+
validationException.addValidationError("The field [" + field + "] must be aggregatable, " +
251251
"but is not.");
252252
}
253253
}
254254
}
255255
if (matchesDateType == false) {
256256
validationException.addValidationError("The field referenced by a date_histo group must be one of type [" +
257-
Strings.collectionToCommaDelimitedString(RollupField.DATE_FIELD_MAPPER_TYPES) + "] across all " +
258-
"indices in the index pattern. Found: " + fieldCaps.keySet().toString() + " for field [" + field + "]");
257+
Strings.collectionToCommaDelimitedString(RollupField.DATE_FIELD_MAPPER_TYPES) + "]." +
258+
" Found: " + fieldCaps.keySet().toString() + " for field [" + field + "]");
259259
}
260260
} else {
261261
validationException.addValidationError("Could not find one of [" +
262262
Strings.collectionToCommaDelimitedString(RollupField.DATE_FIELD_MAPPER_TYPES) + "] fields with name [" +
263-
field + "] in any of the indices matching " +
264-
"the index pattern.");
263+
field + "].");
265264
}
266265
}
267266

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionConfigTests.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,26 @@
66
*/
77
package org.elasticsearch.xpack.core.rollup;
88

9+
import org.elasticsearch.action.ActionRequestValidationException;
10+
import org.elasticsearch.action.fieldcaps.FieldCapabilities;
911
import org.elasticsearch.common.io.stream.Writeable;
1012
import org.elasticsearch.common.xcontent.XContentParser;
13+
import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval;
1114
import org.elasticsearch.test.AbstractSerializingTestCase;
1215
import org.elasticsearch.xpack.core.rollup.job.MetricConfig;
16+
import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig;
1317

1418
import java.io.IOException;
19+
import java.util.Collections;
20+
import java.util.HashMap;
1521
import java.util.List;
22+
import java.util.Map;
1623
import java.util.Random;
1724

1825
import static java.util.Collections.emptyList;
1926
import static org.hamcrest.Matchers.equalTo;
27+
import static org.mockito.Mockito.mock;
28+
import static org.mockito.Mockito.when;
2029

2130

2231
public class RollupActionConfigTests extends AbstractSerializingTestCase<RollupActionConfig> {
@@ -54,4 +63,49 @@ public void testEmptyMetrics() {
5463
new RollupActionConfig(groupConfig, randomBoolean() ? null : emptyList()));
5564
assertThat(e.getMessage(), equalTo("At least one metric must be configured"));
5665
}
66+
67+
public void testValidateMapping() {
68+
ActionRequestValidationException e = new ActionRequestValidationException();
69+
Map<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
70+
String type = getRandomType();
71+
72+
// Have to mock fieldcaps because the ctor's aren't public...
73+
FieldCapabilities myFieldCaps = mock(FieldCapabilities.class);
74+
when(myFieldCaps.isAggregatable()).thenReturn(true);
75+
responseMap.put("my_field", Collections.singletonMap(type, myFieldCaps));
76+
responseMap.put("date_field", Collections.singletonMap("date", myFieldCaps));
77+
responseMap.put("group_field", Collections.singletonMap("keyword", myFieldCaps));
78+
responseMap.put("metric_field", Collections.singletonMap("short", myFieldCaps));
79+
80+
RollupActionConfig config = new RollupActionConfig(new RollupActionGroupConfig(
81+
new RollupActionDateHistogramGroupConfig.FixedInterval("date_field", DateHistogramInterval.DAY),
82+
null, new TermsGroupConfig("group_field")),
83+
Collections.singletonList(new MetricConfig("metric_field", Collections.singletonList("max"))));
84+
config.validateMappings(responseMap, e);
85+
assertThat(e.validationErrors().size(), equalTo(0));
86+
}
87+
88+
private String getRandomType() {
89+
int n = randomIntBetween(0,8);
90+
if (n == 0) {
91+
return "keyword";
92+
} else if (n == 1) {
93+
return "text";
94+
} else if (n == 2) {
95+
return "long";
96+
} else if (n == 3) {
97+
return "integer";
98+
} else if (n == 4) {
99+
return "short";
100+
} else if (n == 5) {
101+
return "float";
102+
} else if (n == 6) {
103+
return "double";
104+
} else if (n == 7) {
105+
return "scaled_float";
106+
} else if (n == 8) {
107+
return "half_float";
108+
}
109+
return "long";
110+
}
57111
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionDateHistogramGroupConfigSerializingTests.java‎

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import org.elasticsearch.common.xcontent.XContentParser;
1313
import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval;
1414
import org.elasticsearch.test.AbstractSerializingTestCase;
15-
import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig;
1615

1716
import java.io.IOException;
1817
import java.util.Collections;
@@ -46,11 +45,10 @@ public void testValidateNoMapping() {
4645
ActionRequestValidationException e = new ActionRequestValidationException();
4746
Map<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
4847

49-
DateHistogramGroupConfig config = new DateHistogramGroupConfig.CalendarInterval("my_field",
50-
new DateHistogramInterval("1d"), null, null);
48+
RollupActionDateHistogramGroupConfig config = new RollupActionDateHistogramGroupConfig.CalendarInterval("my_field",
49+
new DateHistogramInterval("1d"));
5150
config.validateMappings(responseMap, e);
52-
assertThat(e.validationErrors().get(0), equalTo("Could not find one of [date,date_nanos] fields with name [my_field] in " +
53-
"any of the indices matching the index pattern."));
51+
assertThat(e.validationErrors().get(0), equalTo("Could not find one of [date,date_nanos] fields with name [my_field]."));
5452
}
5553

5654
public void testValidateNomatchingField() {
@@ -61,11 +59,10 @@ public void testValidateNomatchingField() {
6159
FieldCapabilities fieldCaps = mock(FieldCapabilities.class);
6260
responseMap.put("some_other_field", Collections.singletonMap("date", fieldCaps));
6361

64-
DateHistogramGroupConfig config = new DateHistogramGroupConfig.CalendarInterval("my_field",
65-
new DateHistogramInterval("1d"), null, null);
62+
RollupActionDateHistogramGroupConfig config = new RollupActionDateHistogramGroupConfig.CalendarInterval("my_field",
63+
new DateHistogramInterval("1d"));
6664
config.validateMappings(responseMap, e);
67-
assertThat(e.validationErrors().get(0), equalTo("Could not find one of [date,date_nanos] fields with name [my_field] in " +
68-
"any of the indices matching the index pattern."));
65+
assertThat(e.validationErrors().get(0), equalTo("Could not find one of [date,date_nanos] fields with name [my_field]."));
6966
}
7067

7168
public void testValidateFieldWrongType() {
@@ -76,11 +73,11 @@ public void testValidateFieldWrongType() {
7673
FieldCapabilities fieldCaps = mock(FieldCapabilities.class);
7774
responseMap.put("my_field", Collections.singletonMap("keyword", fieldCaps));
7875

79-
DateHistogramGroupConfig config = new DateHistogramGroupConfig.CalendarInterval("my_field",
80-
new DateHistogramInterval("1d"), null, null);
76+
RollupActionDateHistogramGroupConfig config = new RollupActionDateHistogramGroupConfig.CalendarInterval("my_field",
77+
new DateHistogramInterval("1d"));
8178
config.validateMappings(responseMap, e);
8279
assertThat(e.validationErrors().get(0), equalTo("The field referenced by a date_histo group must be one of type " +
83-
"[date,date_nanos] across all indices in the index pattern. Found: [keyword] for field [my_field]"));
80+
"[date,date_nanos]. Found: [keyword] for field [my_field]"));
8481
}
8582

8683
public void testValidateFieldMixtureTypes() {
@@ -94,11 +91,11 @@ public void testValidateFieldMixtureTypes() {
9491
types.put("keyword", fieldCaps);
9592
responseMap.put("my_field", types);
9693

97-
DateHistogramGroupConfig config = new DateHistogramGroupConfig.CalendarInterval("my_field",
98-
new DateHistogramInterval("1d"), null, null);
94+
RollupActionDateHistogramGroupConfig config = new RollupActionDateHistogramGroupConfig.CalendarInterval("my_field",
95+
new DateHistogramInterval("1d"));
9996
config.validateMappings(responseMap, e);
10097
assertThat(e.validationErrors().get(0), equalTo("The field referenced by a date_histo group must be one of type " +
101-
"[date,date_nanos] across all indices in the index pattern. Found: [date, keyword] for field [my_field]"));
98+
"[date,date_nanos]. Found: [date, keyword] for field [my_field]"));
10299
}
103100

104101
public void testValidateFieldMatchingNotAggregatable() {
@@ -110,10 +107,10 @@ public void testValidateFieldMatchingNotAggregatable() {
110107
when(fieldCaps.isAggregatable()).thenReturn(false);
111108
responseMap.put("my_field", Collections.singletonMap("date", fieldCaps));
112109

113-
DateHistogramGroupConfig config =new DateHistogramGroupConfig.CalendarInterval("my_field",
114-
new DateHistogramInterval("1d"), null, null);
110+
RollupActionDateHistogramGroupConfig config =new RollupActionDateHistogramGroupConfig.CalendarInterval("my_field",
111+
new DateHistogramInterval("1d"));
115112
config.validateMappings(responseMap, e);
116-
assertThat(e.validationErrors().get(0), equalTo("The field [my_field] must be aggregatable across all indices, but is not."));
113+
assertThat(e.validationErrors().get(0), equalTo("The field [my_field] must be aggregatable, but is not."));
117114
}
118115

119116
public void testValidateMatchingField() {
@@ -125,8 +122,8 @@ public void testValidateMatchingField() {
125122
when(fieldCaps.isAggregatable()).thenReturn(true);
126123
responseMap.put("my_field", Collections.singletonMap("date", fieldCaps));
127124

128-
DateHistogramGroupConfig config = new DateHistogramGroupConfig.CalendarInterval("my_field",
129-
new DateHistogramInterval("1d"), null, null);
125+
RollupActionDateHistogramGroupConfig config = new RollupActionDateHistogramGroupConfig.CalendarInterval("my_field",
126+
new DateHistogramInterval("1d"));
130127
config.validateMappings(responseMap, e);
131128
assertThat(e.validationErrors().size(), equalTo(0));
132129
}
@@ -140,8 +137,8 @@ public void testValidateWeek() {
140137
when(fieldCaps.isAggregatable()).thenReturn(true);
141138
responseMap.put("my_field", Collections.singletonMap("date", fieldCaps));
142139

143-
DateHistogramGroupConfig config = new DateHistogramGroupConfig.CalendarInterval("my_field",
144-
new DateHistogramInterval("1w"), null, null);
140+
RollupActionDateHistogramGroupConfig config = new RollupActionDateHistogramGroupConfig.CalendarInterval("my_field",
141+
new DateHistogramInterval("1w"));
145142
config.validateMappings(responseMap, e);
146143
assertThat(e.validationErrors().size(), equalTo(0));
147144
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/RollupActionGroupConfigSerializingTests.java‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,24 @@
66
*/
77
package org.elasticsearch.xpack.core.rollup;
88

9+
import org.elasticsearch.action.ActionRequestValidationException;
10+
import org.elasticsearch.action.fieldcaps.FieldCapabilities;
911
import org.elasticsearch.common.io.stream.Writeable;
1012
import org.elasticsearch.common.xcontent.XContentParser;
13+
import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval;
1114
import org.elasticsearch.test.AbstractSerializingTestCase;
15+
import org.elasticsearch.xpack.core.rollup.job.HistogramGroupConfig;
16+
import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig;
1217

1318
import java.io.IOException;
19+
import java.util.Collections;
20+
import java.util.HashMap;
21+
import java.util.Map;
1422

1523
import static org.elasticsearch.xpack.core.rollup.ConfigTestHelpers.randomRollupActionGroupConfig;
24+
import static org.hamcrest.Matchers.equalTo;
25+
import static org.mockito.Mockito.mock;
26+
import static org.mockito.Mockito.when;
1627

1728
public class RollupActionGroupConfigSerializingTests extends AbstractSerializingTestCase<RollupActionGroupConfig> {
1829

@@ -30,4 +41,41 @@ protected Writeable.Reader<RollupActionGroupConfig> instanceReader() {
3041
protected RollupActionGroupConfig createTestInstance() {
3142
return randomRollupActionGroupConfig(random());
3243
}
44+
45+
public void testValidatesDateHistogramConfig() {
46+
ActionRequestValidationException e = new ActionRequestValidationException();
47+
Map<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
48+
// Have to mock fieldcaps because the ctor's aren't public...
49+
FieldCapabilities fieldCaps = mock(FieldCapabilities.class);
50+
when(fieldCaps.isAggregatable()).thenReturn(true);
51+
responseMap.put("date_field", Collections.singletonMap("not_date", fieldCaps));
52+
RollupActionGroupConfig config = new RollupActionGroupConfig(
53+
new RollupActionDateHistogramGroupConfig.FixedInterval("date_field", DateHistogramInterval.DAY));
54+
config.validateMappings(responseMap, e);
55+
assertThat(e.validationErrors().size(), equalTo(1));
56+
}
57+
58+
public void testValidatesAllSubConfigs() {
59+
ActionRequestValidationException e = new ActionRequestValidationException();
60+
Map<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
61+
// Have to mock fieldcaps because the ctor's aren't public...
62+
FieldCapabilities fieldCaps = mock(FieldCapabilities.class);
63+
when(fieldCaps.isAggregatable()).thenReturn(false);
64+
responseMap.put("date_field", Collections.singletonMap("date", fieldCaps));
65+
responseMap.put("terms_field", Collections.singletonMap("keyword", fieldCaps));
66+
responseMap.put("histogram_field", Collections.singletonMap("keyword", fieldCaps));
67+
RollupActionGroupConfig config = new RollupActionGroupConfig(
68+
new RollupActionDateHistogramGroupConfig.FixedInterval("date_field", DateHistogramInterval.DAY),
69+
new HistogramGroupConfig(132, "histogram_field"), new TermsGroupConfig("terms_field"));
70+
config.validateMappings(responseMap, e);
71+
// all fields are non-aggregatable
72+
assertThat(e.validationErrors().size(), equalTo(3));
73+
assertThat(e.validationErrors().get(0),
74+
equalTo("The field [date_field] must be aggregatable, but is not."));
75+
assertThat(e.validationErrors().get(1),
76+
equalTo("The field referenced by a histo group must be a [numeric] type, " +
77+
"but found [keyword] for field [histogram_field]"));
78+
assertThat(e.validationErrors().get(2),
79+
equalTo("The field [terms_field] must be aggregatable across all indices, but is not."));
80+
}
3381
}

0 commit comments

Comments
 (0)