From 92951f52340a55c91e49101cf3c4774908b70374 Mon Sep 17 00:00:00 2001 From: Gao Binlong Date: Mon, 4 Sep 2023 16:45:51 +0800 Subject: [PATCH 1/2] Fix ignore_missing parameter has no effect when using template snippet in rename ingest processor Signed-off-by: Gao Binlong --- CHANGELOG.md | 1 + .../ingest/common/RenameProcessor.java | 6 +- .../ingest/common/RenameProcessorTests.java | 14 ++++ .../test/ingest/280_rename_processor.yml | 66 +++++++++++++++++++ 4 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/280_rename_processor.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 529278c188f46..b3a88542dd2e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -191,6 +191,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Segment Replication] Fixed bug where replica shard temporarily serves stale data during an engine reset ([#9495](https://github.com/opensearch-project/OpenSearch/pull/9495)) - Disable shard/segment level search_after short cutting if track_total_hits != false ([#9683](https://github.com/opensearch-project/OpenSearch/pull/9683)) - [Segment Replication] Fixed bug where bytes behind metric is not accurate ([#9686](https://github.com/opensearch-project/OpenSearch/pull/9686)) +- Fix ignore_missing parameter has no effect when using template snippet in rename ingest processor ### Security diff --git a/modules/ingest-common/src/main/java/org/opensearch/ingest/common/RenameProcessor.java b/modules/ingest-common/src/main/java/org/opensearch/ingest/common/RenameProcessor.java index af356eb10d79c..4d9796181ddd4 100644 --- a/modules/ingest-common/src/main/java/org/opensearch/ingest/common/RenameProcessor.java +++ b/modules/ingest-common/src/main/java/org/opensearch/ingest/common/RenameProcessor.java @@ -32,6 +32,7 @@ package org.opensearch.ingest.common; +import org.opensearch.core.common.Strings; import org.opensearch.ingest.AbstractProcessor; import org.opensearch.ingest.ConfigurationUtils; import org.opensearch.ingest.IngestDocument; @@ -80,9 +81,12 @@ boolean isIgnoreMissing() { @Override public IngestDocument execute(IngestDocument document) { String path = document.renderTemplate(field); - if (document.hasField(path, true) == false) { + boolean fieldPathIsNullOrEmpty = Strings.isEmpty(path); + if (fieldPathIsNullOrEmpty || document.hasField(path, true) == false) { if (ignoreMissing) { return document; + } else if (fieldPathIsNullOrEmpty) { + throw new IllegalArgumentException("field path cannot be null nor empty"); } else { throw new IllegalArgumentException("field [" + path + "] doesn't exist"); } diff --git a/modules/ingest-common/src/test/java/org/opensearch/ingest/common/RenameProcessorTests.java b/modules/ingest-common/src/test/java/org/opensearch/ingest/common/RenameProcessorTests.java index fc95693024cb0..a600464371af8 100644 --- a/modules/ingest-common/src/test/java/org/opensearch/ingest/common/RenameProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/opensearch/ingest/common/RenameProcessorTests.java @@ -112,6 +112,15 @@ public void testRenameNonExistingField() throws Exception { } catch (IllegalArgumentException e) { assertThat(e.getMessage(), equalTo("field [" + fieldName + "] doesn't exist")); } + + // when using template snippet, the resolved field path maybe empty + processor = createRenameProcessor("", RandomDocumentPicks.randomFieldName(random()), false); + try { + processor.execute(ingestDocument); + fail("processor execute should have failed"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), equalTo("field path cannot be null nor empty")); + } } public void testRenameNonExistingFieldWithIgnoreMissing() throws Exception { @@ -121,6 +130,11 @@ public void testRenameNonExistingFieldWithIgnoreMissing() throws Exception { Processor processor = createRenameProcessor(fieldName, RandomDocumentPicks.randomFieldName(random()), true); processor.execute(ingestDocument); assertIngestDocument(originalIngestDocument, ingestDocument); + + // when using template snippet, the resolved field path maybe empty + processor = createRenameProcessor("", RandomDocumentPicks.randomFieldName(random()), true); + processor.execute(ingestDocument); + assertIngestDocument(originalIngestDocument, ingestDocument); } public void testRenameNewFieldAlreadyExists() throws Exception { diff --git a/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/280_rename_processor.yml b/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/280_rename_processor.yml new file mode 100644 index 0000000000000..96b2256bcc1dc --- /dev/null +++ b/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/280_rename_processor.yml @@ -0,0 +1,66 @@ +--- +teardown: + - do: + ingest.delete_pipeline: + id: "my_pipeline" + ignore: 404 + +--- +"Test rename processor with non-existing field and without ignore_missing": + - do: + ingest.put_pipeline: + id: "my_pipeline" + body: > + { + "description": "_description", + "processors": [ + { + "rename" : { + "field" : "{{field_foo}}", + "target_field" : "bar" + } + } + ] + } + - match: { acknowledged: true } + + - do: + catch: '/field path cannot be null nor empty/' + index: + index: test + id: 1 + pipeline: "my_pipeline" + body: { message: "foo bar baz" } + +--- +"Test rename processor with non-existing field and ignore_missing": + - do: + ingest.put_pipeline: + id: "my_pipeline" + body: > + { + "description": "_description", + "processors": [ + { + "rename" : { + "field" : "{{field_foo}}", + "target_field" : "bar", + "ignore_missing" : true + } + } + ] + } + - match: { acknowledged: true } + + - do: + index: + index: test + id: 1 + pipeline: "my_pipeline" + body: { message: "foo bar baz" } + + - do: + get: + index: test + id: 1 + - match: { _source.message: "foo bar baz" } From b19ed233305717449edd0e6c5837169757109a3f Mon Sep 17 00:00:00 2001 From: gaobinlong Date: Tue, 12 Sep 2023 13:48:09 +0800 Subject: [PATCH 2/2] Small change Co-authored-by: Andriy Redko Signed-off-by: gaobinlong --- .../main/java/org/opensearch/ingest/common/RenameProcessor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/ingest-common/src/main/java/org/opensearch/ingest/common/RenameProcessor.java b/modules/ingest-common/src/main/java/org/opensearch/ingest/common/RenameProcessor.java index 4d9796181ddd4..7564bbdf95f45 100644 --- a/modules/ingest-common/src/main/java/org/opensearch/ingest/common/RenameProcessor.java +++ b/modules/ingest-common/src/main/java/org/opensearch/ingest/common/RenameProcessor.java @@ -81,7 +81,7 @@ boolean isIgnoreMissing() { @Override public IngestDocument execute(IngestDocument document) { String path = document.renderTemplate(field); - boolean fieldPathIsNullOrEmpty = Strings.isEmpty(path); + final boolean fieldPathIsNullOrEmpty = Strings.isNullOrEmpty(path); if (fieldPathIsNullOrEmpty || document.hasField(path, true) == false) { if (ignoreMissing) { return document;