From 3bcbf88a4980b909c9af9ff9a04ebef12fc8ed35 Mon Sep 17 00:00:00 2001 From: Patrick Mann Date: Wed, 3 Jun 2026 10:40:38 +0200 Subject: [PATCH 1/2] Fix extractors being deleted when an input is started or stopped (#26198) * Fix extractors being deleted when an input is started or stopped The PersistedImpl -> AutoValue migration of InputImpl (#24057) dropped the embedded extractors array from the entity model: the builder ignores unknown properties, so loading an input discards its extractors, and saving uses replaceOne, which then persists the loss by replacing the whole document. Starting or stopping an input persists the desired state on the input document since #25338, so every start/stop deleted all extractors of the input. Updating an input through the REST API had the same effect via the getFields() merge in InputsResource, which also omitted extractors. Fix this by modeling the embedded extractor documents on InputImpl (like the embedded static fields) so they survive full document round-trips, and by carrying them through getFields() and buildFromMap(). Also turn persistDesiredState() into a targeted update of the desired_state field so state changes no longer rewrite the whole document. Fixes #26009 Co-Authored-By: Claude Opus 4.8 (1M context) * CL --------- Co-authored-by: Claude Opus 4.8 (1M context) (cherry picked from commit dbc747f8c5b638c5ea7221722d654d64c060a933) --- changelog/unreleased/issue-26009.toml | 5 ++ .../java/org/graylog2/inputs/InputImpl.java | 17 ++++++ .../org/graylog2/inputs/InputServiceImpl.java | 13 ++++- .../graylog2/inputs/InputServiceImplTest.java | 53 +++++++++++++++++++ 4 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 changelog/unreleased/issue-26009.toml diff --git a/changelog/unreleased/issue-26009.toml b/changelog/unreleased/issue-26009.toml new file mode 100644 index 000000000000..71411c891a65 --- /dev/null +++ b/changelog/unreleased/issue-26009.toml @@ -0,0 +1,5 @@ +type = "fixed" +message = "Fix extractors being deleted when an input is started, stopped, or updated." + +issues = ["26009"] +pulls = ["26198"] diff --git a/graylog2-server/src/main/java/org/graylog2/inputs/InputImpl.java b/graylog2-server/src/main/java/org/graylog2/inputs/InputImpl.java index b832da15275b..de4be9ff5db8 100644 --- a/graylog2-server/src/main/java/org/graylog2/inputs/InputImpl.java +++ b/graylog2-server/src/main/java/org/graylog2/inputs/InputImpl.java @@ -120,6 +120,15 @@ public Map getStaticFields() { return result; } + /** + * The embedded extractor documents. They are modified through targeted update operations (see + * {@code InputServiceImpl#addExtractor} etc.) and only modeled here so that they survive full document + * replacements when saving an input. + */ + @Nullable + @JsonProperty(EMBEDDED_EXTRACTORS) + public abstract List> getEmbeddedExtractors(); + @NotNull @JsonProperty(FIELD_TYPE) public abstract String getType(); @@ -179,6 +188,9 @@ public static Builder create() { @JsonProperty(EMBEDDED_STATIC_FIELDS) public abstract Builder setEmbeddedStaticFields(List> staticFields); + @JsonProperty(EMBEDDED_EXTRACTORS) + public abstract Builder setEmbeddedExtractors(List> extractors); + @JsonProperty(FIELD_TYPE) public abstract Builder setType(String type); @@ -226,6 +238,11 @@ public Map getFields() { doc.put(EMBEDDED_STATIC_FIELDS, getEmbeddedStaticFields()); } + final List> extractors = getEmbeddedExtractors(); + if (extractors != null && !extractors.isEmpty()) { + doc.put(EMBEDDED_EXTRACTORS, extractors); + } + if (getContentPack() != null) { doc.put(FIELD_CONTENT_PACK, getContentPack()); } diff --git a/graylog2-server/src/main/java/org/graylog2/inputs/InputServiceImpl.java b/graylog2-server/src/main/java/org/graylog2/inputs/InputServiceImpl.java index 63e7b9a868fa..a2d6c643e49d 100644 --- a/graylog2-server/src/main/java/org/graylog2/inputs/InputServiceImpl.java +++ b/graylog2-server/src/main/java/org/graylog2/inputs/InputServiceImpl.java @@ -296,6 +296,11 @@ private InputImpl.Builder buildFromMap(Map fields) { builder.setEmbeddedStaticFields(staticFields); } + final List> extractors = (List>) fields.get(InputImpl.EMBEDDED_EXTRACTORS); + if (extractors != null && !extractors.isEmpty()) { + builder.setEmbeddedExtractors(extractors); + } + if (!isGlobal) { builder.setNodeId((String) fields.get(MessageInput.FIELD_NODE_ID)); } @@ -749,8 +754,12 @@ private InputImpl withEncryptedFields(InputImpl input) { @Override public void persistDesiredState(Input input, IOState.Type desiredState) throws ValidationException { - final Input updatedInput = input.withDesiredState(desiredState); - saveWithoutEvents(updatedInput); + // Use a targeted update instead of saving the whole input to avoid overwriting concurrent changes + // to other parts of the input document. + collection.updateOne( + MongoUtils.idEq(input.getId()), + Updates.set(InputImpl.FIELD_DESIRED_STATE, desiredState.name()) + ); } @Override diff --git a/graylog2-server/src/test/java/org/graylog2/inputs/InputServiceImplTest.java b/graylog2-server/src/test/java/org/graylog2/inputs/InputServiceImplTest.java index 3aad16cc37e8..cf138bf9a1ce 100644 --- a/graylog2-server/src/test/java/org/graylog2/inputs/InputServiceImplTest.java +++ b/graylog2-server/src/test/java/org/graylog2/inputs/InputServiceImplTest.java @@ -448,6 +448,7 @@ public void persistedDocumentContainsOnlyExpectedFields(MongoCollections mongoCo .setEmbeddedStaticFields(List.of( Map.of(InputImpl.FIELD_STATIC_FIELD_KEY, "static_key", InputImpl.FIELD_STATIC_FIELD_VALUE, "static_value"))) + .setEmbeddedExtractors(List.of(createCopyInputExtractor().getPersistedFields())) .build(); final String id = inputService.save(input); @@ -466,12 +467,64 @@ public void persistedDocumentContainsOnlyExpectedFields(MongoCollections mongoCo InputImpl.FIELD_GLOBAL, InputImpl.FIELD_CONFIGURATION, InputImpl.EMBEDDED_STATIC_FIELDS, + InputImpl.EMBEDDED_EXTRACTORS, InputImpl.FIELD_DESIRED_STATE, InputImpl.FIELD_CONTENT_PACK, InputImpl.FIELD_NODE_ID ); } + /** + * Regression test for #26009: + * persisting the desired state when starting or stopping an input must not delete its extractors. + */ + @Test + void persistDesiredStateKeepsExtractors() throws Exception { + final Input input = inputService.find(inputService.save(createTestInput())); + inputService.addExtractor(input, createCopyInputExtractor()); + + inputService.persistDesiredState(input, IOState.Type.STOPPED); + + assertThat(inputService.extractorCountByInputId(List.of(input.getId()))) + .containsEntry(input.getId(), 1); + assertThat(inputService.find(input.getId()).getDesiredState()).isEqualTo(IOState.Type.STOPPED); + } + + /** + * Regression test for #26009: + * saving an input that was loaded from the database must round-trip the embedded extractors. + */ + @Test + void saveKeepsExtractors() throws Exception { + final Input input = inputService.find(inputService.save(createTestInput())); + inputService.addExtractor(input, createCopyInputExtractor()); + + inputService.save(inputService.find(input.getId())); + + assertThat(inputService.extractorCountByInputId(List.of(input.getId()))) + .containsEntry(input.getId(), 1); + } + + /** + * Regression test for #26009: + * the input update flow in {@code InputsResource} merges via the {@code getFields()} Map view and + * re-creates the input from the merged map. Extractors must survive this round-trip, too. + */ + @Test + void updateViaFieldsMapKeepsExtractors() throws Exception { + final Input input = inputService.find(inputService.save(createTestInput())); + inputService.addExtractor(input, createCopyInputExtractor()); + + final Input reloaded = inputService.find(input.getId()); + final Map mergedFields = new HashMap<>(reloaded.getFields()); + mergedFields.put(MessageInput.FIELD_TITLE, "updated title"); + inputService.update(inputService.create(reloaded.getId(), mergedFields)); + + assertThat(inputService.extractorCountByInputId(List.of(input.getId()))) + .containsEntry(input.getId(), 1); + assertThat(inputService.find(input.getId()).getTitle()).isEqualTo("updated title"); + } + @Test void totalExtractorCountByTypeCountsExtractorsAcrossInputs() throws Exception { final Input input1 = inputService.find(inputService.save(createTestInput())); From fa7f12d76855b623d66e220ac1354ccfcb17072c Mon Sep 17 00:00:00 2001 From: Patrick Mann Date: Wed, 3 Jun 2026 11:02:29 +0200 Subject: [PATCH 2/2] Remove backported extractor regression tests from 7.1 branch --- .../graylog2/inputs/InputServiceImplTest.java | 51 ------------------- 1 file changed, 51 deletions(-) diff --git a/graylog2-server/src/test/java/org/graylog2/inputs/InputServiceImplTest.java b/graylog2-server/src/test/java/org/graylog2/inputs/InputServiceImplTest.java index cf138bf9a1ce..11ab7abd15d7 100644 --- a/graylog2-server/src/test/java/org/graylog2/inputs/InputServiceImplTest.java +++ b/graylog2-server/src/test/java/org/graylog2/inputs/InputServiceImplTest.java @@ -474,57 +474,6 @@ public void persistedDocumentContainsOnlyExpectedFields(MongoCollections mongoCo ); } - /** - * Regression test for #26009: - * persisting the desired state when starting or stopping an input must not delete its extractors. - */ - @Test - void persistDesiredStateKeepsExtractors() throws Exception { - final Input input = inputService.find(inputService.save(createTestInput())); - inputService.addExtractor(input, createCopyInputExtractor()); - - inputService.persistDesiredState(input, IOState.Type.STOPPED); - - assertThat(inputService.extractorCountByInputId(List.of(input.getId()))) - .containsEntry(input.getId(), 1); - assertThat(inputService.find(input.getId()).getDesiredState()).isEqualTo(IOState.Type.STOPPED); - } - - /** - * Regression test for #26009: - * saving an input that was loaded from the database must round-trip the embedded extractors. - */ - @Test - void saveKeepsExtractors() throws Exception { - final Input input = inputService.find(inputService.save(createTestInput())); - inputService.addExtractor(input, createCopyInputExtractor()); - - inputService.save(inputService.find(input.getId())); - - assertThat(inputService.extractorCountByInputId(List.of(input.getId()))) - .containsEntry(input.getId(), 1); - } - - /** - * Regression test for #26009: - * the input update flow in {@code InputsResource} merges via the {@code getFields()} Map view and - * re-creates the input from the merged map. Extractors must survive this round-trip, too. - */ - @Test - void updateViaFieldsMapKeepsExtractors() throws Exception { - final Input input = inputService.find(inputService.save(createTestInput())); - inputService.addExtractor(input, createCopyInputExtractor()); - - final Input reloaded = inputService.find(input.getId()); - final Map mergedFields = new HashMap<>(reloaded.getFields()); - mergedFields.put(MessageInput.FIELD_TITLE, "updated title"); - inputService.update(inputService.create(reloaded.getId(), mergedFields)); - - assertThat(inputService.extractorCountByInputId(List.of(input.getId()))) - .containsEntry(input.getId(), 1); - assertThat(inputService.find(input.getId()).getTitle()).isEqualTo("updated title"); - } - @Test void totalExtractorCountByTypeCountsExtractorsAcrossInputs() throws Exception { final Input input1 = inputService.find(inputService.save(createTestInput()));