From d440108aa86e2c22c8a84dc6ef00c014919eec46 Mon Sep 17 00:00:00 2001 From: Benjamin Trent Date: Thu, 9 Jan 2020 09:31:44 -0500 Subject: [PATCH] [Transform] fail to start/put on missing pipeline (#50701) If a pipeline referenced by a transform does not exist, we should not allow the transform to be created. We do allow the pipeline existence check to be skipped with defer_validations, but if the pipeline still does not exist on `_start`, the pipeline will fail to start. relates: #50135 --- .../TransformDocumentationIT.java | 1 + docs/build.gradle | 18 +++++- .../transform/apis/put-transform.asciidoc | 2 +- .../core/transform/TransformMessages.java | 1 + .../test/transform/transforms_crud.yml | 49 ++++++++++++++++ .../test/transform/transforms_start_stop.yml | 56 +++++++++++++++++++ .../action/TransportPutTransformAction.java | 22 +++++++- .../action/TransportStartTransformAction.java | 22 +++++++- ...TransportPutTransformActionDeprecated.java | 7 ++- ...ansportStartTransformActionDeprecated.java | 7 ++- 10 files changed, 173 insertions(+), 12 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/TransformDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/TransformDocumentationIT.java index 5438d5dd554c5..861cdef8e5ac6 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/TransformDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/TransformDocumentationIT.java @@ -134,6 +134,7 @@ public void testPutTransform() throws IOException, InterruptedException { .setIndex("pivot-destination") .setPipeline("my-pipeline").build(); // end::put-transform-dest-config + destConfig = DestConfig.builder().setIndex("pivot-destination").build(); // tag::put-transform-group-config GroupConfig groupConfig = GroupConfig.builder() .groupBy("reviewer", // <1> diff --git a/docs/build.gradle b/docs/build.gradle index 2477733fca435..9000868cac536 100644 --- a/docs/build.gradle +++ b/docs/build.gradle @@ -1226,7 +1226,23 @@ buildRestTests.setups['kibana_sample_data_ecommerce'] = ''' number_of_shards: 1 number_of_replicas: 0 ''' -buildRestTests.setups['simple_kibana_continuous_pivot'] = buildRestTests.setups['kibana_sample_data_ecommerce'] + ''' +buildRestTests.setups['add_timestamp_pipeline'] = ''' + - do: + ingest.put_pipeline: + id: "add_timestamp_pipeline" + body: > + { + "processors": [ + { + "set" : { + "field" : "@timestamp", + "value" : "{{_ingest.timestamp}}" + } + } + ] + } +''' +buildRestTests.setups['simple_kibana_continuous_pivot'] = buildRestTests.setups['kibana_sample_data_ecommerce'] + buildRestTests.setups['add_timestamp_pipeline'] + ''' - do: raw: method: PUT diff --git a/docs/reference/transform/apis/put-transform.asciidoc b/docs/reference/transform/apis/put-transform.asciidoc index dfe11c96f1d93..ff55af4addb0a 100644 --- a/docs/reference/transform/apis/put-transform.asciidoc +++ b/docs/reference/transform/apis/put-transform.asciidoc @@ -195,7 +195,7 @@ PUT _transform/ecommerce_transform } } -------------------------------------------------- -// TEST[setup:kibana_sample_data_ecommerce] +// TEST[setup:kibana_sample_data_ecommerce,add_timestamp_pipeline] When the {transform} is created, you receive the following results: diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/TransformMessages.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/TransformMessages.java index b1e9dffea5027..d0279d9aebd38 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/TransformMessages.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/TransformMessages.java @@ -27,6 +27,7 @@ public class TransformMessages { public static final String REST_FAILED_TO_SERIALIZE_TRANSFORM = "Failed to serialise transform [{0}]"; public static final String TRANSFORM_FAILED_TO_PERSIST_STATS = "Failed to persist transform statistics for transform [{0}]"; public static final String UNKNOWN_TRANSFORM_STATS = "Statistics for transform [{0}] could not be found"; + public static final String PIPELINE_MISSING = "Pipeline with id [{0}] could not be found"; public static final String REST_DEPRECATED_ENDPOINT = "[_data_frame/transforms/] is deprecated, use [_transform/] in the future."; diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/transform/transforms_crud.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/transform/transforms_crud.yml index 1968e1fa431cc..b8dc88c3ae0c1 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/transform/transforms_crud.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/transform/transforms_crud.yml @@ -122,6 +122,22 @@ setup: } - match: { acknowledged: true } + - do: + ingest.put_pipeline: + id: "airline-pipeline" + body: > + { + "processors": [ + { + "set" : { + "field" : "some_field", + "value" : 42 + } + } + ] + } + - match: { acknowledged: true } + - do: transform.put_transform: transform_id: "airline-transform-dos" @@ -631,3 +647,36 @@ setup: transform_id: "airline-transform-start-delete" force: true - match: { acknowledged: true } +--- +"Test put transform with missing pipeline": + - do: + catch: /Pipeline with id \[missing-transform-pipeline\] could not be found/ + transform.put_transform: + transform_id: "airline-transform-with-missing-pipeline-crud" + body: > + { + "source": { "index": "airline-data" }, + "dest": { "index": "airline-data-by-airline-with-pipeline", "pipeline": "missing-transform-pipeline" }, + "pivot": { + "group_by": { "airline": {"terms": {"field": "airline"}}}, + "aggs": {"avg_response": {"avg": {"field": "responsetime"}}} + }, + "description": "yaml test transform on airline-data" + } +--- +"Test put transform with missing pipeline and defer validations": + - do: + transform.put_transform: + defer_validation: true + transform_id: "airline-transform-with-missing-pipeline-crud-defer" + body: > + { + "source": { "index": "airline-data" }, + "dest": { "index": "airline-data-by-airline", "pipeline": "missing-transform-pipeline" }, + "pivot": { + "group_by": { "airline": {"terms": {"field": "airline"}}}, + "aggs": {"avg_response": {"avg": {"field": "responsetime"}}} + }, + "description": "yaml test transform on airline-data" + } + - match: {acknowledged: true} diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/transform/transforms_start_stop.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/transform/transforms_start_stop.yml index f3ed8351803b9..38268e499246e 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/transform/transforms_start_stop.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/transform/transforms_start_stop.yml @@ -376,3 +376,59 @@ teardown: index: airline-data-time-alias - match: { airline-data-time-alias.mappings.properties.time.type: date } - match: { airline-data-time-alias.mappings.properties.avg_response.type: double } +--- +"Test start transform with missing pipeline": + - do: + transform.put_transform: + defer_validation: true + transform_id: "airline-transform-with-missing-pipeline" + body: > + { + "source": { "index": "airline-data" }, + "dest": { "index": "airline-data-by-airline-pipeline", "pipeline": "missing-transform-pipeline" }, + "pivot": { + "group_by": { "airline": {"terms": {"field": "airline"}}}, + "aggs": {"avg_response": {"avg": {"field": "responsetime"}}} + }, + "description": "yaml test transform on airline-data" + } + - match: {acknowledged: true} + - do: + catch: /Pipeline with id \[missing-transform-pipeline\] could not be found/ + transform.start_transform: + transform_id: "airline-transform-with-missing-pipeline" +--- +"Test start transform with pipeline": + - do: + ingest.put_pipeline: + id: "transform-pipeline" + body: > + { + "processors": [ + { + "set" : { + "field" : "some_field", + "value" : 42 + } + } + ] + } + - match: { acknowledged: true } + + - do: + transform.put_transform: + transform_id: "airline-transform-with-pipeline" + body: > + { + "source": { "index": "airline-data" }, + "dest": { "index": "airline-data-by-airline-pipeline", "pipeline": "transform-pipeline" }, + "pivot": { + "group_by": { "airline": {"terms": {"field": "airline"}}}, + "aggs": {"avg_response": {"avg": {"field": "responsetime"}}} + }, + "description": "yaml test transform on airline-data" + } + - match: {acknowledged: true} + - do: + transform.start_transform: + transform_id: "airline-transform-with-pipeline" diff --git a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportPutTransformAction.java b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportPutTransformAction.java index 505d4ffe5fde9..0cfbafa3285d1 100644 --- a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportPutTransformAction.java +++ b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportPutTransformAction.java @@ -26,6 +26,7 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.ingest.IngestService; import org.elasticsearch.license.License; import org.elasticsearch.license.LicenseUtils; import org.elasticsearch.license.RemoteClusterLicenseChecker; @@ -73,6 +74,7 @@ public class TransportPutTransformAction extends TransportMasterNodeAction if (request.isDeferValidation()) { pivotValidationListener.onResponse(true); } else { + if (config.getDestination().getPipeline() != null) { + if (ingestService.getPipeline(config.getDestination().getPipeline()) == null) { + listener.onFailure(new ElasticsearchStatusException( + TransformMessages.getMessage(TransformMessages.PIPELINE_MISSING, config.getDestination().getPipeline()), + RestStatus.BAD_REQUEST + ) + ); + return; + } + } pivot.validateQuery(client, config.getSource(), pivotValidationListener); } } diff --git a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportStartTransformAction.java b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportStartTransformAction.java index cc55a3705ca10..3de0b74830955 100644 --- a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportStartTransformAction.java +++ b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportStartTransformAction.java @@ -26,6 +26,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.ingest.IngestService; import org.elasticsearch.license.License; import org.elasticsearch.license.LicenseUtils; import org.elasticsearch.license.RemoteClusterLicenseChecker; @@ -70,6 +71,7 @@ public class TransportStartTransformAction extends TransportMasterNodeAction