Skip to content

Commit 211d647

Browse files
martijnvgmartijnvg
committed
Backport: also validate source index at put enrich policy time (#48311)
Backport of: #48254 This changes tests to create a valid source index prior to creating the enrich policy.
1 parent f4cb76d commit 211d647

File tree

17 files changed

+178
-28
lines changed

17 files changed

+178
-28
lines changed

‎client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/EnrichDocumentationIT.java‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@
4444
import java.util.concurrent.CountDownLatch;
4545
import java.util.concurrent.TimeUnit;
4646

47+
import static java.util.Collections.singletonMap;
48+
4749
public class EnrichDocumentationIT extends ESRestHighLevelClientTestCase {
4850

4951
@After
@@ -59,6 +61,10 @@ public void cleanup() {
5961

6062
public void testPutPolicy() throws Exception {
6163
RestHighLevelClient client = highLevelClient();
64+
CreateIndexRequest createIndexRequest = new CreateIndexRequest("users")
65+
.mapping(singletonMap("properties", singletonMap("email", singletonMap("type", "keyword"))));
66+
client.indices().create(createIndexRequest, RequestOptions.DEFAULT);
67+
6268
// tag::enrich-put-policy-request
6369
PutPolicyRequest putPolicyRequest = new PutPolicyRequest(
6470
"users-policy", "match", Arrays.asList("users"),
@@ -106,6 +112,10 @@ public void testDeletePolicy() throws Exception {
106112
RestHighLevelClient client = highLevelClient();
107113

108114
{
115+
CreateIndexRequest createIndexRequest = new CreateIndexRequest("users")
116+
.mapping(singletonMap("properties", singletonMap("email", singletonMap("type", "keyword"))));
117+
client.indices().create(createIndexRequest, RequestOptions.DEFAULT);
118+
109119
// Add a policy, so that it can be deleted:
110120
PutPolicyRequest putPolicyRequest = new PutPolicyRequest(
111121
"users-policy", "match", Arrays.asList("users"),
@@ -158,6 +168,10 @@ public void onFailure(Exception e) {
158168
public void testGetPolicy() throws Exception {
159169
RestHighLevelClient client = highLevelClient();
160170

171+
CreateIndexRequest createIndexRequest = new CreateIndexRequest("users")
172+
.mapping(singletonMap("properties", singletonMap("email", singletonMap("type", "keyword"))));
173+
client.indices().create(createIndexRequest, RequestOptions.DEFAULT);
174+
161175
PutPolicyRequest putPolicyRequest = new PutPolicyRequest(
162176
"users-policy", "match", Collections.singletonList("users"),
163177
"email", Arrays.asList("address", "zip", "city", "state"));
@@ -259,8 +273,8 @@ public void testExecutePolicy() throws Exception {
259273

260274
{
261275
CreateIndexRequest createIndexRequest = new CreateIndexRequest("users")
262-
.mapping(Collections.singletonMap("properties", Collections.singletonMap("email",
263-
Collections.singletonMap("type", "keyword"))));
276+
.mapping(singletonMap("properties", singletonMap("email",
277+
singletonMap("type", "keyword"))));
264278
client.indices().create(createIndexRequest, RequestOptions.DEFAULT);
265279
PutPolicyRequest putPolicyRequest = new PutPolicyRequest(
266280
"users-policy", "match", Collections.singletonList("users"),

‎docs/reference/ingest/apis/enrich/delete-enrich-policy.asciidoc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ Deletes an existing enrich policy and its enrich index.
1212
[source,console]
1313
----
1414
PUT /users
15+
{
16+
"mappings" : {
17+
"properties" : {
18+
"email" : { "type" : "keyword" }
19+
}
20+
}
21+
}
1522
1623
PUT /_enrich/policy/my-policy
1724
{

‎docs/reference/ingest/apis/enrich/get-enrich-policy.asciidoc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ Returns information about an enrich policy.
1212
[source,console]
1313
----
1414
PUT /users
15+
{
16+
"mappings" : {
17+
"properties" : {
18+
"email" : { "type" : "keyword" }
19+
}
20+
}
21+
}
1522
1623
PUT /_enrich/policy/my-policy
1724
{

‎docs/reference/ingest/apis/enrich/put-enrich-policy.asciidoc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ Creates an enrich policy.
1212
[source,console]
1313
----
1414
PUT /users
15+
{
16+
"mappings" : {
17+
"properties" : {
18+
"email" : { "type" : "keyword" }
19+
}
20+
}
21+
}
1522
----
1623
////
1724

‎x-pack/plugin/enrich/qa/common/src/main/java/org/elasticsearch/test/enrich/CommonEnrichRestTestCase.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.elasticsearch.client.Response;
1111
import org.elasticsearch.client.ResponseException;
1212
import org.elasticsearch.common.Strings;
13+
import org.elasticsearch.common.settings.Settings;
1314
import org.elasticsearch.common.xcontent.XContentBuilder;
1415
import org.elasticsearch.common.xcontent.XContentHelper;
1516
import org.elasticsearch.common.xcontent.json.JsonXContent;
@@ -38,6 +39,15 @@ public void deletePolicies() throws Exception {
3839
for (Map<?, ?> entry: policies) {
3940
client().performRequest(new Request("DELETE", "/_enrich/policy/" +
4041
XContentMapValues.extractValue("config.match.name", entry)));
42+
43+
List<?> sourceIndices = (List<?>) XContentMapValues.extractValue("config.match.indices", entry);
44+
for (Object sourceIndex : sourceIndices) {
45+
try {
46+
client().performRequest(new Request("DELETE", "/" + sourceIndex));
47+
} catch (ResponseException e) {
48+
// and that is ok
49+
}
50+
}
4151
}
4252
}
4353

@@ -48,6 +58,8 @@ protected boolean preserveIndicesUponCompletion() {
4858
}
4959

5060
private void setupGenericLifecycleTest(boolean deletePipeilne) throws Exception {
61+
// Create source index:
62+
createSourceIndex("my-source-index");
5163
// Create the policy:
5264
Request putPolicyRequest = new Request("PUT", "/_enrich/policy/my_policy");
5365
putPolicyRequest.setJsonEntity(generatePolicySource("my-source-index"));
@@ -99,6 +111,7 @@ public void testBasicFlow() throws Exception {
99111
}
100112

101113
public void testImmutablePolicy() throws IOException {
114+
createSourceIndex("my-source-index");
102115
Request putPolicyRequest = new Request("PUT", "/_enrich/policy/my_policy");
103116
putPolicyRequest.setJsonEntity(generatePolicySource("my-source-index"));
104117
assertOK(client().performRequest(putPolicyRequest));
@@ -108,6 +121,7 @@ public void testImmutablePolicy() throws IOException {
108121
}
109122

110123
public void testDeleteIsCaseSensitive() throws Exception {
124+
createSourceIndex("my-source-index");
111125
Request putPolicyRequest = new Request("PUT", "/_enrich/policy/my_policy");
112126
putPolicyRequest.setJsonEntity(generatePolicySource("my-source-index"));
113127
assertOK(client().performRequest(putPolicyRequest));
@@ -155,6 +169,20 @@ public static String generatePolicySource(String index) throws IOException {
155169
return Strings.toString(source);
156170
}
157171

172+
public static void createSourceIndex(String index) throws IOException {
173+
String mapping = createSourceIndexMapping();
174+
createIndex(index, Settings.EMPTY, mapping);
175+
}
176+
177+
public static String createSourceIndexMapping() {
178+
return "\"properties\":" +
179+
"{\"host\": {\"type\":\"keyword\"}," +
180+
"\"globalRank\":{\"type\":\"keyword\"}," +
181+
"\"tldRank\":{\"type\":\"keyword\"}," +
182+
"\"tld\":{\"type\":\"keyword\"}" +
183+
"}";
184+
}
185+
158186
private static Map<String, Object> toMap(Response response) throws IOException {
159187
return toMap(EntityUtils.toString(response.getEntity()));
160188
}

‎x-pack/plugin/enrich/qa/rest-with-security/src/test/java/org/elasticsearch/xpack/enrich/EnrichSecurityIT.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ protected Settings restAdminSettings() {
3636
public void testInsufficientPermissionsOnNonExistentIndex() throws Exception {
3737
// This test is here because it requires a valid user that has permission to execute policy PUTs but should fail if the user
3838
// does not have access to read the backing indices used to enrich the data.
39+
Request request = new Request("PUT", "/some-other-index");
40+
request.setJsonEntity("{\n \"mappings\" : {" + createSourceIndexMapping() + "} }");
41+
adminClient().performRequest(request);
3942
Request putPolicyRequest = new Request("PUT", "/_enrich/policy/my_policy");
4043
putPolicyRequest.setJsonEntity(generatePolicySource("some-other-index"));
4144
ResponseException exc = expectThrows(ResponseException.class, () -> client().performRequest(putPolicyRequest));

‎x-pack/plugin/enrich/qa/rest/src/test/resources/rest-api-spec/test/enrich/10_basic.yml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,20 @@
11
---
22
"Test enrich crud apis":
33

4+
- do:
5+
indices.create:
6+
index: bar
7+
body:
8+
mappings:
9+
properties:
10+
baz:
11+
type: keyword
12+
a:
13+
type: keyword
14+
b:
15+
type: keyword
16+
- is_true: acknowledged
17+
418
- do:
519
enrich.put_policy:
620
name: policy-crud

‎x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichPolicyRunner.java‎

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -136,27 +136,34 @@ private void validateMappings(final GetIndexResponse getIndexResponse) {
136136
logger.debug("Policy [{}]: Validating [{}] source mappings", policyName, sourceIndices);
137137
for (String sourceIndex : sourceIndices) {
138138
Map<String, Object> mapping = getMappings(getIndexResponse, sourceIndex);
139-
// First ensure mapping is set
140-
if (mapping.get("properties") == null) {
141-
throw new ElasticsearchException(
142-
"Enrich policy execution for [{}] failed. Could not read mapping for source [{}] included by pattern [{}]",
143-
policyName, sourceIndex, policy.getIndices());
144-
}
145-
// Validate the key and values
146-
try {
147-
validateField(mapping, policy.getMatchField(), true);
148-
for (String valueFieldName : policy.getEnrichFields()) {
149-
validateField(mapping, valueFieldName, false);
150-
}
151-
} catch (ElasticsearchException e) {
152-
throw new ElasticsearchException(
153-
"Enrich policy execution for [{}] failed while validating field mappings for index [{}]",
154-
e, policyName, sourceIndex);
139+
validateMappings(policyName, policy, sourceIndex, mapping);
140+
}
141+
}
142+
143+
static void validateMappings(final String policyName,
144+
final EnrichPolicy policy,
145+
final String sourceIndex,
146+
final Map<String, Object> mapping) {
147+
// First ensure mapping is set
148+
if (mapping.get("properties") == null) {
149+
throw new ElasticsearchException(
150+
"Enrich policy execution for [{}] failed. Could not read mapping for source [{}] included by pattern [{}]",
151+
policyName, sourceIndex, policy.getIndices());
152+
}
153+
// Validate the key and values
154+
try {
155+
validateField(mapping, policy.getMatchField(), true);
156+
for (String valueFieldName : policy.getEnrichFields()) {
157+
validateField(mapping, valueFieldName, false);
155158
}
159+
} catch (ElasticsearchException e) {
160+
throw new ElasticsearchException(
161+
"Enrich policy execution for [{}] failed while validating field mappings for index [{}]",
162+
e, policyName, sourceIndex);
156163
}
157164
}
158165

159-
private void validateField(Map<?, ?> properties, String fieldName, boolean fieldRequired) {
166+
private static void validateField(Map<?, ?> properties, String fieldName, boolean fieldRequired) {
160167
assert Strings.isEmpty(fieldName) == false: "Field name cannot be null or empty";
161168
String[] fieldParts = fieldName.split("\\.");
162169
StringBuilder parent = new StringBuilder();

‎x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichStore.java‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,12 @@
88
import org.elasticsearch.ResourceAlreadyExistsException;
99
import org.elasticsearch.ResourceNotFoundException;
1010
import org.elasticsearch.Version;
11+
import org.elasticsearch.action.support.IndicesOptions;
1112
import org.elasticsearch.cluster.ClusterState;
1213
import org.elasticsearch.cluster.ClusterStateUpdateTask;
14+
import org.elasticsearch.cluster.metadata.IndexMetaData;
15+
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
16+
import org.elasticsearch.cluster.metadata.MappingMetaData;
1317
import org.elasticsearch.cluster.metadata.MetaData;
1418
import org.elasticsearch.cluster.metadata.MetaDataCreateIndexService;
1519
import org.elasticsearch.cluster.service.ClusterService;
@@ -40,7 +44,11 @@ private EnrichStore() {}
4044
* @param policy The policy to store
4145
* @param handler The handler that gets invoked if policy has been stored or a failure has occurred.
4246
*/
43-
public static void putPolicy(String name, EnrichPolicy policy, ClusterService clusterService, Consumer<Exception> handler) {
47+
public static void putPolicy(final String name,
48+
final EnrichPolicy policy,
49+
final ClusterService clusterService,
50+
final IndexNameExpressionResolver indexNameExpressionResolver,
51+
final Consumer<Exception> handler) {
4452
assert clusterService.localNode().isMasterNode();
4553

4654
if (Strings.isNullOrEmpty(name)) {
@@ -76,6 +84,22 @@ public static void putPolicy(String name, EnrichPolicy policy, ClusterService cl
7684
finalPolicy = policy;
7785
}
7886
updateClusterState(clusterService, handler, current -> {
87+
for (String indexExpression : finalPolicy.getIndices()) {
88+
// indices field in policy can contain wildcards, aliases etc.
89+
String[] concreteIndices =
90+
indexNameExpressionResolver.concreteIndexNames(current, IndicesOptions.strictExpandOpen(), indexExpression);
91+
for (String concreteIndex : concreteIndices) {
92+
IndexMetaData imd = current.getMetaData().index(concreteIndex);
93+
assert imd != null;
94+
MappingMetaData mapping = imd.mapping();
95+
if (mapping == null) {
96+
throw new IllegalArgumentException("source index [" + concreteIndex + "] has no mapping");
97+
}
98+
Map<String, Object> mappingSource = mapping.getSourceAsMap();
99+
EnrichPolicyRunner.validateMappings(name, finalPolicy, concreteIndex, mappingSource);
100+
}
101+
}
102+
79103
final Map<String, EnrichPolicy> policies = getPolicies(current);
80104
if (policies.get(name) != null) {
81105
throw new ResourceAlreadyExistsException("policy [{}] already exists", name);

‎x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/action/TransportPutEnrichPolicyAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ protected void masterOperation(PutEnrichPolicyAction.Request request, ClusterSta
102102
}
103103

104104
private void putPolicy(PutEnrichPolicyAction.Request request, ActionListener<AcknowledgedResponse> listener ) {
105-
EnrichStore.putPolicy(request.getName(), request.getPolicy(), clusterService, e -> {
105+
EnrichStore.putPolicy(request.getName(), request.getPolicy(), clusterService, indexNameExpressionResolver, e -> {
106106
if (e == null) {
107107
listener.onResponse(new AcknowledgedResponse(true));
108108
} else {

0 commit comments

Comments
 (0)