Block storing invalid pipeline configs caused by nested processors #93425
Conversation
mattc58
added
Team:Data Management
| @@ -36,6 +36,8 @@ | |||
| import java.util.Objects; | |||
|
|
|||
| public class SimulatePipelineRequest extends ActionRequest implements ToXContentObject { | |||
| public static final String PROCESSORS_KEY = "processors"; | |||
There was a problem hiding this comment.
I think this variable is never used.
|
|
||
| import static org.elasticsearch.rest.RestRequest.Method.PUT; | ||
|
|
||
| public class RestPutPipelineAction extends BaseRestHandler { | ||
| public static final String PROCESSORS_KEY = "processors"; |
There was a problem hiding this comment.
I think this variable is never used.
| String fieldName = randomAlphaOfLengthBetween(1, 10); | ||
| String fieldValue = randomAlphaOfLengthBetween(1, 10); | ||
| Map<String, Object> doc = new HashMap<>(); | ||
| doc.put(Fields.SOURCE, Collections.singletonMap(fieldName, fieldValue)); |
There was a problem hiding this comment.
Prefer Map.of(...) rather than Collections.singletonMap(...) where possible -- there's this line and another a few lines below.
| Map<String, Object> requestContent = new HashMap<>(); | ||
| Map<String, Object> pipelineConfig = new HashMap<>(); | ||
| List<Map<String, Object>> processors = new ArrayList<>(); | ||
| Map<String, Object> processorConfig = new HashMap<>(); |
There was a problem hiding this comment.
I'd declare the processorConfig right before you use it, slightly clearer that way.
| @@ -53,6 +53,32 @@ public void testCreate() throws Exception { | |||
| assertThat(pipeline.getProcessors().get(1).getTag(), nullValue()); | |||
| } | |||
|
|
|||
| public void testContainsNestedProcessors() throws Exception { | |||
| // create a pipeline with non-nested processors | |||
| Map<String, Object> processorConfig0 = new HashMap<>(); | |||
There was a problem hiding this comment.
I'd drop processorConfig0 and processorConfig1, I don't think they carry their weight as variables. Probably just replace new HashMap<>() at each of their current uses.
|
Nits aside, mostly looks pretty good to me! What's the word on how we're going to treat this -- e.g. as a bugfix with no warnings, or as a breaking change with a deprecation period or the like? |
|
Closing this until it's prioritized again |
This fixes #41837 by blocking
PUT _ingest/pipelineorPOST _ingest/pipeline/_simulatecalls if there is are nested processors in any of the processor configs.It will not do anything to pipelines with nested processors which have already been stored in the server. The behavior of those pipelines will remain as they are before this change (which is to say, non-deterministic in their processing order based on the use of
HashMapfor their storage).