Fix ignore_missing parameter has no effect in rename ingest processor (…

…opensearch-project#9725) * Fix ignore_missing parameter has no effect when using template snippet in rename ingest processor Signed-off-by: Gao Binlong <[email protected]> * Small change Co-authored-by: Andriy Redko <[email protected]> Signed-off-by: gaobinlong <[email protected]> --------- Signed-off-by: Gao Binlong <[email protected]> Signed-off-by: gaobinlong <[email protected]> Co-authored-by: Andriy Redko <[email protected]>
sachinpkale · Sep 14, 2023 · 921cd0c · 921cd0c
1 parent d2ca926
commit 921cd0c
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -98,6 +98,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Removed
 
 ### Fixed
+- Fix ignore_missing parameter has no effect when using template snippet in rename ingest processor ([#9725](https://github.com/opensearch-project/OpenSearch/pull/9725))
 
 ### 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
@@ -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) {
+        final boolean fieldPathIsNullOrEmpty = Strings.isNullOrEmpty(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
@@ -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/...gest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/280_rename_processor.yml b/...gest-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" }