From 7f622d4a3322d6812000463e26f61835f6fb99b4 Mon Sep 17 00:00:00 2001 From: Jedr Blaszyk Date: Wed, 15 Jan 2025 14:47:59 +0100 Subject: [PATCH 1/8] [Connector API] Add hard delete support --- .../apis/delete-connector-api.asciidoc | 5 +- muted-tests.yml | 2 - .../rest-api-spec/api/connector.delete.json | 5 ++ x-pack/plugin/ent-search/build.gradle | 3 +- .../entsearch/connector/20_connector_list.yml | 88 +++++++++++++++++++ .../connector/30_connector_delete.yml | 26 +++++- .../connector/ConnectorIndexService.java | 87 ++++++++++++------ .../action/DeleteConnectorAction.java | 31 ++++--- .../action/RestDeleteConnectorAction.java | 3 +- .../TransportDeleteConnectorAction.java | 15 ++-- .../connector/ConnectorIndexServiceTests.java | 74 ++++++++++++---- ...ectorActionRequestBWCSerializingTests.java | 43 --------- 12 files changed, 265 insertions(+), 117 deletions(-) delete mode 100644 x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/action/DeleteConnectorActionRequestBWCSerializingTests.java diff --git a/docs/reference/connector/apis/delete-connector-api.asciidoc b/docs/reference/connector/apis/delete-connector-api.asciidoc index f161a3c3b5933..c326bf747deb3 100644 --- a/docs/reference/connector/apis/delete-connector-api.asciidoc +++ b/docs/reference/connector/apis/delete-connector-api.asciidoc @@ -13,7 +13,7 @@ beta::[] For the most up-to-date API details, refer to {api-es}/group/endpoint-connector[Connector APIs]. -- -Soft-deletes a connector and removes associated sync jobs. +Deletes a connector and optionally removes associated sync jobs. Note: this action doesn't delete any API key, ingest pipeline or data index associated with the connector. These need to be removed manually. @@ -37,6 +37,9 @@ To get started with Connector APIs, check out <`:: (Required, string) +``:: +(Optional, boolean) If `true`, the connector doc is deleted. If `false`, connector doc is marked as deleted but doc stays in index (soft deletion). Defaults to `false`. + `delete_sync_jobs`:: (Optional, boolean) A flag indicating if associated sync jobs should be also removed. Defaults to `false`. diff --git a/muted-tests.yml b/muted-tests.yml index 9766d3ed35f18..5c110f7dc492b 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -79,8 +79,6 @@ tests: - class: org.elasticsearch.search.StressSearchServiceReaperIT method: testStressReaper issue: https://github.com/elastic/elasticsearch/issues/115816 -- class: org.elasticsearch.xpack.application.connector.ConnectorIndexServiceTests - issue: https://github.com/elastic/elasticsearch/issues/116087 - class: org.elasticsearch.xpack.test.rest.XPackRestIT method: test {p0=transform/transforms_start_stop/Test start already started transform} issue: https://github.com/elastic/elasticsearch/issues/98802 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/connector.delete.json b/rest-api-spec/src/main/resources/rest-api-spec/api/connector.delete.json index 96d477160b277..2693f2668f602 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/connector.delete.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/connector.delete.json @@ -28,6 +28,11 @@ ] }, "params": { + "hard": { + "type": "boolean", + "default": false, + "description": "If true, the connector doc is deleted. If false, connector doc is marked as deleted but doc stays in index (soft deleted)." + }, "delete_sync_jobs": { "type": "boolean", "default": false, diff --git a/x-pack/plugin/ent-search/build.gradle b/x-pack/plugin/ent-search/build.gradle index 52634ad788d97..d6572e8c92a90 100644 --- a/x-pack/plugin/ent-search/build.gradle +++ b/x-pack/plugin/ent-search/build.gradle @@ -14,7 +14,8 @@ base { } dependencies { - compileOnly project(path: xpackModule('core')) + implementation project(path: ':server') + compileOnly project(path: xpackModule('core')) implementation project(xpackModule('search-business-rules')) api project(':modules:lang-mustache') diff --git a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml index c54d764fc4761..7b13a3f641ecc 100644 --- a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml +++ b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml @@ -312,6 +312,94 @@ setup: - match: { count: 3 } + +--- +"List Connectors - Single hard deleted connector": + - requires: + cluster_features: ["connector_soft_deletes"] + reason: Soft deletes were introduced in 9.0 release + + - do: + connector.delete: + connector_id: connector-a + hard: true + + - do: + connector.list: {} + + - match: { count: 2 } + + - do: + connector.list: + include_deleted: true + + - match: { count: 2 } + + +--- +"List Connectors - All hard deleted connector": + - requires: + cluster_features: ["connector_soft_deletes"] + reason: Soft deletes were introduced in 9.0 release + + - do: + connector.delete: + connector_id: connector-a + hard: true + + - do: + connector.delete: + connector_id: connector-b + hard: true + + - do: + connector.delete: + connector_id: connector-b + hard: true + + - do: + connector.list: {} + + - match: { count: 0 } + + - do: + connector.list: + include_deleted: true + + - match: { count: 3 } + +--- +"List Connectors - 2 hard deleted connector, 1 soft deleted": + - requires: + cluster_features: ["connector_soft_deletes"] + reason: Soft deletes were introduced in 9.0 release + + - do: + connector.delete: + connector_id: connector-a + hard: false + + - do: + connector.delete: + connector_id: connector-b + hard: true + + - do: + connector.delete: + connector_id: connector-b + hard: true + + - do: + connector.list: {} + + - match: { count: 0 } + + - do: + connector.list: + include_deleted: true + + - match: { count: 1 } + --- "List Connectors - Soft deleted connectors": - requires: diff --git a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/30_connector_delete.yml b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/30_connector_delete.yml index aa820732be2e2..cabda7e0a1a0f 100644 --- a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/30_connector_delete.yml +++ b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/30_connector_delete.yml @@ -27,6 +27,31 @@ setup: connector_id: test-connector-to-delete +--- +"Delete Connector - Hard Delete": + - do: + connector.put: + connector_id: test-connector-hard-delete + body: + index_name: search-2-test + name: my-hard-delete-connector + language: en + is_native: false + service_type: super-connector + + - do: + connector.delete: + connector_id: test-connector-hard-delete + hard: true + + - match: { acknowledged: true } + + - do: + catch: "missing" + connector.get: + connector_id: test-connector-hard-delete + include_deleted: true + --- "Delete Connector - deletes associated sync jobs": @@ -107,7 +132,6 @@ setup: connector.delete: connector_id: test-nonexistent-connector - --- "Delete Connector - Supports soft deletes": - requires: diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java index bb80f5fee4ec9..8f71540b78ee9 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java @@ -13,6 +13,8 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.DelegatingActionListener; import org.elasticsearch.action.DocWriteRequest; +import org.elasticsearch.action.DocWriteResponse; +import org.elasticsearch.action.delete.DeleteRequest; import org.elasticsearch.action.get.GetRequest; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.search.SearchRequest; @@ -232,40 +234,75 @@ public void getConnector(String connectorId, boolean includeDeleted, ActionListe } /** - * Soft deletes the {@link Connector} and optionally removes the related instances of {@link ConnectorSyncJob} in the underlying index. + * Deletes the {@link Connector} and optionally removes the related instances of {@link ConnectorSyncJob} in the underlying index. * * @param connectorId The id of the {@link Connector}. + * @param hardDelete If set to true, the {@link Connector} is permanently deleted; otherwise, it is soft-deleted. * @param shouldDeleteSyncJobs The flag indicating if {@link ConnectorSyncJob} should also be deleted. * @param listener The action listener to invoke on response/failure. */ - public void deleteConnector(String connectorId, boolean shouldDeleteSyncJobs, ActionListener listener) { + public void deleteConnector( + String connectorId, + boolean hardDelete, + boolean shouldDeleteSyncJobs, + ActionListener listener + ) { try { - // ensure that if connector is soft-deleted, deleting it again results in 404 - getConnector(connectorId, false, listener.delegateFailure((l, connector) -> { - final UpdateRequest updateRequest = new UpdateRequest(CONNECTOR_INDEX_NAME, connectorId).setRefreshPolicy( - WriteRequest.RefreshPolicy.IMMEDIATE - ) - .doc( - new IndexRequest(CONNECTOR_INDEX_NAME).opType(DocWriteRequest.OpType.INDEX) - .id(connectorId) - .source(Map.of(Connector.IS_DELETED_FIELD.getPreferredName(), true)) - ); - clientWithOrigin.update(updateRequest, new DelegatingIndexNotFoundActionListener<>(connectorId, l, (ll, updateResponse) -> { - if (updateResponse.getResult() == UpdateResponse.Result.NOT_FOUND) { - ll.onFailure(new ResourceNotFoundException(connectorNotFoundErrorMsg(connectorId))); - return; - } - if (shouldDeleteSyncJobs) { - new ConnectorSyncJobIndexService(clientWithOrigin).deleteAllSyncJobsByConnectorId( - connectorId, - ll.map(r -> updateResponse) + if (hardDelete) { + // Hard-delete-case + final DeleteRequest deleteRequest = new DeleteRequest(CONNECTOR_INDEX_NAME).id(connectorId) + .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); + + clientWithOrigin.delete( + deleteRequest, + new DelegatingIndexNotFoundActionListener<>(connectorId, listener, (l, deleteResponse) -> { + if (deleteResponse.getResult() == DocWriteResponse.Result.NOT_FOUND) { + l.onFailure(new ResourceNotFoundException(connectorNotFoundErrorMsg(connectorId))); + return; + } + if (shouldDeleteSyncJobs) { + new ConnectorSyncJobIndexService(clientWithOrigin).deleteAllSyncJobsByConnectorId( + connectorId, + l.map(r -> deleteResponse) + ); + } else { + l.onResponse(deleteResponse); + } + }) + ); + } + + else { + // Soft-delete case + getConnector(connectorId, false, listener.delegateFailure((l, connector) -> { + final UpdateRequest updateRequest = new UpdateRequest(CONNECTOR_INDEX_NAME, connectorId).setRefreshPolicy( + WriteRequest.RefreshPolicy.IMMEDIATE + ) + .doc( + new IndexRequest(CONNECTOR_INDEX_NAME).opType(DocWriteRequest.OpType.INDEX) + .id(connectorId) + .source(Map.of(Connector.IS_DELETED_FIELD.getPreferredName(), true)) ); - } else { - ll.onResponse(updateResponse); - } + clientWithOrigin.update( + updateRequest, + new DelegatingIndexNotFoundActionListener<>(connectorId, l, (ll, updateResponse) -> { + if (updateResponse.getResult() == UpdateResponse.Result.NOT_FOUND) { + ll.onFailure(new ResourceNotFoundException(connectorNotFoundErrorMsg(connectorId))); + return; + } + if (shouldDeleteSyncJobs) { + new ConnectorSyncJobIndexService(clientWithOrigin).deleteAllSyncJobsByConnectorId( + connectorId, + ll.map(r -> updateResponse) + ); + } else { + ll.onResponse(updateResponse); + } + }) + ); })); - })); + } } catch (Exception e) { listener.onFailure(e); } diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/action/DeleteConnectorAction.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/action/DeleteConnectorAction.java index 5d98f9703ecea..57d940537d176 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/action/DeleteConnectorAction.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/action/DeleteConnectorAction.java @@ -9,9 +9,9 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionType; +import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.xcontent.ConstructingObjectParser; import org.elasticsearch.xcontent.ParseField; @@ -35,19 +35,16 @@ private DeleteConnectorAction() {/* no instances */} public static class Request extends ConnectorActionRequest implements ToXContentObject { private final String connectorId; + private final boolean hardDelete; private final boolean deleteSyncJobs; private static final ParseField CONNECTOR_ID_FIELD = new ParseField("connector_id"); + private static final ParseField HARD_DELETE_FIELD = new ParseField("hard"); private static final ParseField DELETE_SYNC_JOB_FIELD = new ParseField("delete_sync_jobs"); - public Request(StreamInput in) throws IOException { - super(in); - this.connectorId = in.readString(); - this.deleteSyncJobs = in.readBoolean(); - } - - public Request(String connectorId, boolean deleteSyncJobs) { + public Request(String connectorId, boolean hardDelete, boolean deleteSyncJobs) { this.connectorId = connectorId; + this.hardDelete = hardDelete; this.deleteSyncJobs = deleteSyncJobs; } @@ -66,15 +63,17 @@ public String getConnectorId() { return connectorId; } + public boolean isHardDelete() { + return hardDelete; + } + public boolean shouldDeleteSyncJobs() { return deleteSyncJobs; } @Override public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - out.writeString(connectorId); - out.writeBoolean(deleteSyncJobs); + TransportAction.localOnly(); } @Override @@ -82,18 +81,21 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; Request request = (Request) o; - return deleteSyncJobs == request.deleteSyncJobs && Objects.equals(connectorId, request.connectorId); + return hardDelete == request.hardDelete + && deleteSyncJobs == request.deleteSyncJobs + && Objects.equals(connectorId, request.connectorId); } @Override public int hashCode() { - return Objects.hash(connectorId, deleteSyncJobs); + return Objects.hash(connectorId, hardDelete, deleteSyncJobs); } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); builder.field(CONNECTOR_ID_FIELD.getPreferredName(), connectorId); + builder.field(HARD_DELETE_FIELD.getPreferredName(), hardDelete); builder.field(DELETE_SYNC_JOB_FIELD.getPreferredName(), deleteSyncJobs); builder.endObject(); return builder; @@ -102,10 +104,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "delete_connector_request", false, - (p) -> new Request((String) p[0], (boolean) p[1]) + (p) -> new Request((String) p[0], (boolean) p[1], (boolean) p[2]) ); static { PARSER.declareString(constructorArg(), CONNECTOR_ID_FIELD); + PARSER.declareBoolean(constructorArg(), HARD_DELETE_FIELD); PARSER.declareBoolean(constructorArg(), DELETE_SYNC_JOB_FIELD); } diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/action/RestDeleteConnectorAction.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/action/RestDeleteConnectorAction.java index 8d4a6dccd95fe..2adcc1ccafbcd 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/action/RestDeleteConnectorAction.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/action/RestDeleteConnectorAction.java @@ -40,8 +40,9 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient String connectorId = restRequest.param(CONNECTOR_ID_PARAM); boolean shouldDeleteSyncJobs = restRequest.paramAsBoolean("delete_sync_jobs", false); + boolean hardDelete = restRequest.paramAsBoolean("hard", false); - DeleteConnectorAction.Request request = new DeleteConnectorAction.Request(connectorId, shouldDeleteSyncJobs); + DeleteConnectorAction.Request request = new DeleteConnectorAction.Request(connectorId, hardDelete, shouldDeleteSyncJobs); return channel -> client.execute(DeleteConnectorAction.INSTANCE, request, new RestToXContentListener<>(channel)); } } diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/action/TransportDeleteConnectorAction.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/action/TransportDeleteConnectorAction.java index e534d969fdaaa..5e0e94ece2ae8 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/action/TransportDeleteConnectorAction.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/action/TransportDeleteConnectorAction.java @@ -9,7 +9,7 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.HandledTransportAction; +import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.internal.Client; import org.elasticsearch.common.util.concurrent.EsExecutors; @@ -18,26 +18,21 @@ import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.application.connector.ConnectorIndexService; -public class TransportDeleteConnectorAction extends HandledTransportAction { +public class TransportDeleteConnectorAction extends TransportAction { protected final ConnectorIndexService connectorIndexService; @Inject public TransportDeleteConnectorAction(TransportService transportService, ActionFilters actionFilters, Client client) { - super( - DeleteConnectorAction.NAME, - transportService, - actionFilters, - DeleteConnectorAction.Request::new, - EsExecutors.DIRECT_EXECUTOR_SERVICE - ); + super(DeleteConnectorAction.NAME, actionFilters, transportService.getTaskManager(), EsExecutors.DIRECT_EXECUTOR_SERVICE); this.connectorIndexService = new ConnectorIndexService(client); } @Override protected void doExecute(Task task, DeleteConnectorAction.Request request, ActionListener listener) { String connectorId = request.getConnectorId(); + boolean hardDelete = request.isHardDelete(); boolean shouldDeleteSyncJobs = request.shouldDeleteSyncJobs(); - connectorIndexService.deleteConnector(connectorId, shouldDeleteSyncJobs, listener.map(v -> AcknowledgedResponse.TRUE)); + connectorIndexService.deleteConnector(connectorId, hardDelete, shouldDeleteSyncJobs, listener.map(v -> AcknowledgedResponse.TRUE)); } } diff --git a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorIndexServiceTests.java b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorIndexServiceTests.java index 6189db51df183..7b6d9c9b14df9 100644 --- a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorIndexServiceTests.java +++ b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorIndexServiceTests.java @@ -101,7 +101,7 @@ public void testPostConnector() throws Exception { assertThat(resp.getId(), equalTo(indexedConnector.getConnectorId())); } - public void testDeleteConnector() throws Exception { + public void testDeleteConnector_expectSoftDeletionSingle() throws Exception { int numConnectors = 5; List connectorIds = new ArrayList<>(); for (int i = 0; i < numConnectors; i++) { @@ -111,11 +111,10 @@ public void testDeleteConnector() throws Exception { } String connectorIdToDelete = connectorIds.get(0); - UpdateResponse resp = awaitDeleteConnector(connectorIdToDelete, false); + DocWriteResponse resp = awaitDeleteConnector(connectorIdToDelete, false, false); assertThat(resp.status(), equalTo(RestStatus.OK)); expectThrows(ResourceNotFoundException.class, () -> awaitGetConnector(connectorIdToDelete)); - - expectThrows(ResourceNotFoundException.class, () -> awaitDeleteConnector(connectorIdToDelete, false)); + expectThrows(ResourceNotFoundException.class, () -> awaitDeleteConnector(connectorIdToDelete, false, false)); } public void testDeleteConnector_expectSoftDeletion() throws Exception { @@ -130,13 +129,11 @@ public void testDeleteConnector_expectSoftDeletion() throws Exception { } String connectorIdToDelete = connectorIds.get(0); - UpdateResponse resp = awaitDeleteConnector(connectorIdToDelete, false); + DocWriteResponse resp = awaitDeleteConnector(connectorIdToDelete, false, false); assertThat(resp.status(), equalTo(RestStatus.OK)); expectThrows(ResourceNotFoundException.class, () -> awaitGetConnector(connectorIdToDelete)); - - expectThrows(ResourceNotFoundException.class, () -> awaitDeleteConnector(connectorIdToDelete, false)); - - Connector softDeletedConnector = awaitGetSoftDeletedConnector(connectorIdToDelete); + expectThrows(ResourceNotFoundException.class, () -> awaitDeleteConnector(connectorIdToDelete, false, false)); + Connector softDeletedConnector = awaitGetConnectorIncludeDeleted(connectorIdToDelete); assertThat(softDeletedConnector.getConnectorId(), equalTo(connectorIdToDelete)); assertThat(softDeletedConnector.getServiceType(), equalTo(connectors.get(0).getServiceType())); } @@ -150,27 +147,65 @@ public void testDeleteConnector_expectSoftDeletionMultipleConnectors() throws Ex connectorIds.add(resp.getId()); } - // Delete all of them for (int i = 0; i < numConnectors; i++) { String connectorIdToDelete = connectorIds.get(i); - UpdateResponse resp = awaitDeleteConnector(connectorIdToDelete, false); + DocWriteResponse resp = awaitDeleteConnector(connectorIdToDelete, false, false); assertThat(resp.status(), equalTo(RestStatus.OK)); } - // Connectors were deleted from main index for (int i = 0; i < numConnectors; i++) { String connectorId = connectorIds.get(i); expectThrows(ResourceNotFoundException.class, () -> awaitGetConnector(connectorId)); } - // Soft deleted connectors available in system index for (int i = 0; i < numConnectors; i++) { String connectorId = connectorIds.get(i); - Connector softDeletedConnector = awaitGetSoftDeletedConnector(connectorId); + Connector softDeletedConnector = awaitGetConnectorIncludeDeleted(connectorId); assertThat(softDeletedConnector.getConnectorId(), equalTo(connectorId)); } } + public void testDeleteConnector_expectHardDeletionSingle() throws Exception { + int numConnectors = 3; + List connectorIds = new ArrayList<>(); + for (int i = 0; i < numConnectors; i++) { + Connector connector = ConnectorTestUtils.getRandomConnector(); + ConnectorCreateActionResponse resp = awaitCreateConnector(null, connector); + connectorIds.add(resp.getId()); + } + + String connectorIdToDelete = connectorIds.get(0); + DocWriteResponse resp = awaitDeleteConnector(connectorIdToDelete, true, false); + assertThat(resp.status(), equalTo(RestStatus.OK)); + expectThrows(ResourceNotFoundException.class, () -> awaitGetConnector(connectorIdToDelete)); + expectThrows(ResourceNotFoundException.class, () -> awaitGetConnectorIncludeDeleted(connectorIdToDelete)); + expectThrows(ResourceNotFoundException.class, () -> awaitDeleteConnector(connectorIdToDelete, true, false)); + } + + public void testDeleteConnector_expectHardDeletionMultipleConnectors() throws Exception { + int numConnectors = 5; + List connectorIds = new ArrayList<>(); + for (int i = 0; i < numConnectors; i++) { + Connector connector = ConnectorTestUtils.getRandomConnector(); + ConnectorCreateActionResponse resp = awaitCreateConnector(null, connector); + connectorIds.add(resp.getId()); + } + + for (int i = 0; i < numConnectors; i++) { + String connectorIdToDelete = connectorIds.get(i); + DocWriteResponse resp = awaitDeleteConnector(connectorIdToDelete, true, false); + assertThat(resp.status(), equalTo(RestStatus.OK)); + } + + for (String connectorId : connectorIds) { + expectThrows(ResourceNotFoundException.class, () -> awaitGetConnector(connectorId)); + } + + for (String connectorId : connectorIds) { + expectThrows(ResourceNotFoundException.class, () -> awaitGetConnectorIncludeDeleted(connectorId)); + } + } + public void testUpdateConnectorConfiguration_FullConfiguration() throws Exception { Connector connector = ConnectorTestUtils.getRandomConnector(); String connectorId = randomUUID(); @@ -949,13 +984,14 @@ public void testUpdateConnectorApiKeyIdOrApiKeySecretId() throws Exception { assertThat(updateApiKeyIdRequest.getApiKeySecretId(), equalTo(indexedConnector.getApiKeySecretId())); } - private UpdateResponse awaitDeleteConnector(String connectorId, boolean deleteConnectorSyncJobs) throws Exception { + private DocWriteResponse awaitDeleteConnector(String connectorId, boolean hardDelete, boolean deleteConnectorSyncJobs) + throws Exception { CountDownLatch latch = new CountDownLatch(1); - final AtomicReference resp = new AtomicReference<>(null); + final AtomicReference resp = new AtomicReference<>(null); final AtomicReference exc = new AtomicReference<>(null); - connectorIndexService.deleteConnector(connectorId, deleteConnectorSyncJobs, new ActionListener<>() { + connectorIndexService.deleteConnector(connectorId, hardDelete, deleteConnectorSyncJobs, new ActionListener<>() { @Override - public void onResponse(UpdateResponse deleteResponse) { + public void onResponse(DocWriteResponse deleteResponse) { resp.set(deleteResponse); latch.countDown(); } @@ -1008,7 +1044,7 @@ public void onFailure(Exception e) { return resp.get(); } - private Connector awaitGetSoftDeletedConnector(String connectorId) throws Exception { + private Connector awaitGetConnectorIncludeDeleted(String connectorId) throws Exception { return awaitGetConnector(connectorId, true); } diff --git a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/action/DeleteConnectorActionRequestBWCSerializingTests.java b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/action/DeleteConnectorActionRequestBWCSerializingTests.java deleted file mode 100644 index 5ad7109a6b7c1..0000000000000 --- a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/action/DeleteConnectorActionRequestBWCSerializingTests.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -package org.elasticsearch.xpack.application.connector.action; - -import org.elasticsearch.TransportVersion; -import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.test.AbstractBWCSerializationTestCase; -import org.elasticsearch.xcontent.XContentParser; - -import java.io.IOException; - -public class DeleteConnectorActionRequestBWCSerializingTests extends AbstractBWCSerializationTestCase { - - @Override - protected Writeable.Reader instanceReader() { - return DeleteConnectorAction.Request::new; - } - - @Override - protected DeleteConnectorAction.Request createTestInstance() { - return new DeleteConnectorAction.Request(randomAlphaOfLengthBetween(1, 10), false); - } - - @Override - protected DeleteConnectorAction.Request mutateInstance(DeleteConnectorAction.Request instance) throws IOException { - return randomValueOtherThan(instance, this::createTestInstance); - } - - @Override - protected DeleteConnectorAction.Request doParseInstance(XContentParser parser) throws IOException { - return DeleteConnectorAction.Request.parse(parser); - } - - @Override - protected DeleteConnectorAction.Request mutateInstanceForVersion(DeleteConnectorAction.Request instance, TransportVersion version) { - return new DeleteConnectorAction.Request(instance.getConnectorId(), instance.shouldDeleteSyncJobs()); - } -} From d37266ddbfec0f53530af388904bf0bdb78b3a8f Mon Sep 17 00:00:00 2001 From: Jedr Blaszyk Date: Wed, 15 Jan 2025 14:48:28 +0100 Subject: [PATCH 2/8] Undo accidental change --- muted-tests.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 5c110f7dc492b..9766d3ed35f18 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -79,6 +79,8 @@ tests: - class: org.elasticsearch.search.StressSearchServiceReaperIT method: testStressReaper issue: https://github.com/elastic/elasticsearch/issues/115816 +- class: org.elasticsearch.xpack.application.connector.ConnectorIndexServiceTests + issue: https://github.com/elastic/elasticsearch/issues/116087 - class: org.elasticsearch.xpack.test.rest.XPackRestIT method: test {p0=transform/transforms_start_stop/Test start already started transform} issue: https://github.com/elastic/elasticsearch/issues/98802 From 1381401aadbca50fef70579e00f4358642d6ad28 Mon Sep 17 00:00:00 2001 From: Jedr Blaszyk Date: Wed, 15 Jan 2025 14:51:54 +0100 Subject: [PATCH 3/8] undo accidental build gradle change --- x-pack/plugin/ent-search/build.gradle | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugin/ent-search/build.gradle b/x-pack/plugin/ent-search/build.gradle index d6572e8c92a90..52634ad788d97 100644 --- a/x-pack/plugin/ent-search/build.gradle +++ b/x-pack/plugin/ent-search/build.gradle @@ -14,8 +14,7 @@ base { } dependencies { - implementation project(path: ':server') - compileOnly project(path: xpackModule('core')) + compileOnly project(path: xpackModule('core')) implementation project(xpackModule('search-business-rules')) api project(':modules:lang-mustache') From 8aeeb207e4daddad58fe942b721e7e751ba87099 Mon Sep 17 00:00:00 2001 From: Jedr Blaszyk Date: Wed, 15 Jan 2025 14:56:10 +0100 Subject: [PATCH 4/8] Tweak typos --- docs/reference/connector/apis/delete-connector-api.asciidoc | 2 +- .../main/resources/rest-api-spec/api/connector.delete.json | 2 +- .../test/entsearch/connector/20_connector_list.yml | 4 ++-- .../xpack/application/connector/ConnectorIndexService.java | 3 --- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/docs/reference/connector/apis/delete-connector-api.asciidoc b/docs/reference/connector/apis/delete-connector-api.asciidoc index c326bf747deb3..a324630cc8a52 100644 --- a/docs/reference/connector/apis/delete-connector-api.asciidoc +++ b/docs/reference/connector/apis/delete-connector-api.asciidoc @@ -38,7 +38,7 @@ To get started with Connector APIs, check out <`:: -(Optional, boolean) If `true`, the connector doc is deleted. If `false`, connector doc is marked as deleted but doc stays in index (soft deletion). Defaults to `false`. +(Optional, boolean) If `true`, the connector doc is deleted. If `false`, connector doc is marked as deleted (soft deletion). Defaults to `false`. `delete_sync_jobs`:: (Optional, boolean) A flag indicating if associated sync jobs should be also removed. Defaults to `false`. diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/connector.delete.json b/rest-api-spec/src/main/resources/rest-api-spec/api/connector.delete.json index 2693f2668f602..aa5a3dc0a791f 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/connector.delete.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/connector.delete.json @@ -31,7 +31,7 @@ "hard": { "type": "boolean", "default": false, - "description": "If true, the connector doc is deleted. If false, connector doc is marked as deleted but doc stays in index (soft deleted)." + "description": "If true, the connector doc is deleted. If false, connector doc is marked as deleted (soft-deleted)." }, "delete_sync_jobs": { "type": "boolean", diff --git a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml index 7b13a3f641ecc..fe9527d837f16 100644 --- a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml +++ b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml @@ -337,7 +337,7 @@ setup: --- -"List Connectors - All hard deleted connector": +"List Connectors - All hard deleted connectors": - requires: cluster_features: ["connector_soft_deletes"] reason: Soft deletes were introduced in 9.0 release @@ -369,7 +369,7 @@ setup: - match: { count: 3 } --- -"List Connectors - 2 hard deleted connector, 1 soft deleted": +"List Connectors - 2 hard deleted connectors, 1 soft deleted": - requires: cluster_features: ["connector_soft_deletes"] reason: Soft deletes were introduced in 9.0 release diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java index 8f71540b78ee9..f16ecefaa695d 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java @@ -250,7 +250,6 @@ public void deleteConnector( try { if (hardDelete) { - // Hard-delete-case final DeleteRequest deleteRequest = new DeleteRequest(CONNECTOR_INDEX_NAME).id(connectorId) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); @@ -272,9 +271,7 @@ public void deleteConnector( }) ); } - else { - // Soft-delete case getConnector(connectorId, false, listener.delegateFailure((l, connector) -> { final UpdateRequest updateRequest = new UpdateRequest(CONNECTOR_INDEX_NAME, connectorId).setRefreshPolicy( WriteRequest.RefreshPolicy.IMMEDIATE From 076f7e68e4f994add01e1917183076c95a8db5c6 Mon Sep 17 00:00:00 2001 From: Jedr Blaszyk Date: Wed, 15 Jan 2025 14:58:49 +0100 Subject: [PATCH 5/8] Update docs/changelog/120200.yaml --- docs/changelog/120200.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/120200.yaml diff --git a/docs/changelog/120200.yaml b/docs/changelog/120200.yaml new file mode 100644 index 0000000000000..abde91aec0dff --- /dev/null +++ b/docs/changelog/120200.yaml @@ -0,0 +1,5 @@ +pr: 120200 +summary: "[Connector API] Support hard deletes with new URL param in delete endpoint" +area: Extract&Transform +type: feature +issues: [] From 1a26f67b57ad20e7282156dd24bca5ae99213e98 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 15 Jan 2025 14:05:54 +0000 Subject: [PATCH 6/8] [CI] Auto commit changes from spotless --- .../xpack/application/connector/ConnectorIndexService.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java index f16ecefaa695d..3120124c17523 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java @@ -270,8 +270,7 @@ public void deleteConnector( } }) ); - } - else { + } else { getConnector(connectorId, false, listener.delegateFailure((l, connector) -> { final UpdateRequest updateRequest = new UpdateRequest(CONNECTOR_INDEX_NAME, connectorId).setRefreshPolicy( WriteRequest.RefreshPolicy.IMMEDIATE From 846ba46c26b106330ec5aa2c4023ce6a7f48817c Mon Sep 17 00:00:00 2001 From: Jedr Blaszyk Date: Wed, 15 Jan 2025 15:23:07 +0100 Subject: [PATCH 7/8] Fix yaml test --- .../test/entsearch/connector/20_connector_list.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml index fe9527d837f16..0fa50807c694f 100644 --- a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml +++ b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml @@ -366,7 +366,7 @@ setup: connector.list: include_deleted: true - - match: { count: 3 } + - match: { count: 0 } --- "List Connectors - 2 hard deleted connectors, 1 soft deleted": From 21f750629e8203e022d69cb469ae6a458bbdaaa2 Mon Sep 17 00:00:00 2001 From: Jedr Blaszyk Date: Wed, 15 Jan 2025 16:28:11 +0100 Subject: [PATCH 8/8] Actually skip the feature check since we don't have the feature anyway --- .../entsearch/connector/20_connector_list.yml | 36 ++++++------------- .../connector/30_connector_delete.yml | 4 +-- 2 files changed, 11 insertions(+), 29 deletions(-) diff --git a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml index 0fa50807c694f..3a84bda7042a2 100644 --- a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml +++ b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/20_connector_list.yml @@ -280,9 +280,7 @@ setup: --- "List Connectors - Soft deleted connectors / no deleted": - - requires: - cluster_features: ["connector_soft_deletes"] - reason: Soft deletes were introduced in 9.0 release + - do: connector.list: @@ -293,9 +291,7 @@ setup: --- "List Connectors - Single soft deleted connector": - - requires: - cluster_features: ["connector_soft_deletes"] - reason: Soft deletes were introduced in 9.0 release + - do: connector.delete: @@ -315,9 +311,7 @@ setup: --- "List Connectors - Single hard deleted connector": - - requires: - cluster_features: ["connector_soft_deletes"] - reason: Soft deletes were introduced in 9.0 release + - do: connector.delete: @@ -338,9 +332,7 @@ setup: --- "List Connectors - All hard deleted connectors": - - requires: - cluster_features: ["connector_soft_deletes"] - reason: Soft deletes were introduced in 9.0 release + - do: connector.delete: @@ -354,7 +346,7 @@ setup: - do: connector.delete: - connector_id: connector-b + connector_id: connector-c hard: true - do: @@ -370,9 +362,7 @@ setup: --- "List Connectors - 2 hard deleted connectors, 1 soft deleted": - - requires: - cluster_features: ["connector_soft_deletes"] - reason: Soft deletes were introduced in 9.0 release + - do: connector.delete: @@ -386,7 +376,7 @@ setup: - do: connector.delete: - connector_id: connector-b + connector_id: connector-c hard: true - do: @@ -402,9 +392,7 @@ setup: --- "List Connectors - Soft deleted connectors": - - requires: - cluster_features: ["connector_soft_deletes"] - reason: Soft deletes were introduced in 9.0 release + - do: connector.delete: @@ -441,9 +429,7 @@ setup: --- "List Connectors - Soft deleted with from": - - requires: - cluster_features: ["connector_soft_deletes"] - reason: Soft deletes were introduced in 9.0 release + - do: connector.delete: @@ -475,9 +461,7 @@ setup: --- "List Connector - Soft deleted with size": - - requires: - cluster_features: ["connector_soft_deletes"] - reason: Soft deletes were introduced in 9.0 release + - do: connector.delete: diff --git a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/30_connector_delete.yml b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/30_connector_delete.yml index cabda7e0a1a0f..6cb0be2a737ef 100644 --- a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/30_connector_delete.yml +++ b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/connector/30_connector_delete.yml @@ -134,9 +134,7 @@ setup: --- "Delete Connector - Supports soft deletes": - - requires: - cluster_features: ["connector_soft_deletes"] - reason: Soft deletes were introduced in 9.0 release + - do: connector.delete: