diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/TransformConfig.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/TransformConfig.java index 8989ca3594b..5970e33c212 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/TransformConfig.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/TransformConfig.java @@ -423,6 +423,41 @@ public class TransformConfig extends AbstractDiffable implement return lenient ? LENIENT_PARSER.apply(parser, optionalTransformId) : STRICT_PARSER.apply(parser, optionalTransformId); } + /** + * Rewrites the transform config according to the latest format, for example moving deprecated + * settings to its new place. + * + * @param transformConfig original config + * @return a rewritten transform config if a rewrite was necessary, otherwise the given transformConfig + */ + public static TransformConfig rewriteForUpdate(final TransformConfig transformConfig) { + + // quick checks for deprecated features, if none found just return the original + if (transformConfig.getPivotConfig() == null || transformConfig.getPivotConfig().getMaxPageSearchSize() == null) { + return transformConfig; + } + + Builder builder = new Builder(transformConfig); + + if (transformConfig.getPivotConfig() != null && transformConfig.getPivotConfig().getMaxPageSearchSize() != null) { + // create a new pivot config but set maxPageSearchSize to null + PivotConfig newPivotConfig = new PivotConfig( + transformConfig.getPivotConfig().getGroupConfig(), + transformConfig.getPivotConfig().getAggregationConfig(), + null + ); + builder.setPivotConfig(newPivotConfig); + + Integer maxPageSearchSizeDeprecated = transformConfig.getPivotConfig().getMaxPageSearchSize(); + Integer maxPageSearchSize = transformConfig.getSettings().getMaxPageSearchSize() != null + ? transformConfig.getSettings().getMaxPageSearchSize() + : maxPageSearchSizeDeprecated; + + builder.setSettings(new SettingsConfig(maxPageSearchSize, transformConfig.getSettings().getDocsPerSecond())); + } + return builder.setVersion(Version.CURRENT).build(); + } + public static class Builder { private String id; private SourceConfig source; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/persistence/TransformInternalIndexConstants.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/persistence/TransformInternalIndexConstants.java index 59a80f6c959..86115c06c37 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/persistence/TransformInternalIndexConstants.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/persistence/TransformInternalIndexConstants.java @@ -6,6 +6,8 @@ package org.elasticsearch.xpack.core.transform.transforms.persistence; +import org.elasticsearch.Version; + public final class TransformInternalIndexConstants { /* Constants for internal indexes of the transform plugin @@ -24,6 +26,7 @@ public final class TransformInternalIndexConstants { // internal index // version is not a rollover pattern, however padded because sort is string based + public static final Version INDEX_VERSION_LAST_CHANGED = Version.V_7_7_0; public static final String INDEX_VERSION = "005"; public static final String INDEX_PATTERN = ".transform-internal-"; public static final String LATEST_INDEX_VERSIONED_NAME = INDEX_PATTERN + INDEX_VERSION; @@ -42,7 +45,6 @@ public final class TransformInternalIndexConstants { public static final String AUDIT_INDEX_READ_ALIAS = ".transform-notifications-read"; public static final String AUDIT_INDEX = AUDIT_INDEX_PREFIX + AUDIT_TEMPLATE_VERSION; - private TransformInternalIndexConstants() { - } + private TransformInternalIndexConstants() {} } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/transforms/TransformConfigTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/transforms/TransformConfigTests.java index b1b25061edb..bb05c2f765d 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/transforms/TransformConfigTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/transform/transforms/TransformConfigTests.java @@ -313,9 +313,81 @@ public class TransformConfigTests extends AbstractSerializingTransformTestCase createTransformResponse = entityAsMap(client().performRequest(createTransformRequest)); + + assertThat(createTransformResponse.get("acknowledged"), equalTo(Boolean.TRUE)); + Request getRequest = createRequestWithAuth("GET", getTransformEndpoint() + transformId, BASIC_AUTH_VALUE_TRANSFORM_USER); + Map transforms = entityAsMap(client().performRequest(getRequest)); + assertEquals(1, XContentMapValues.extractValue("count", transforms)); + Map transform = ((List>) XContentMapValues.extractValue("transforms", transforms)).get(0); + assertThat(XContentMapValues.extractValue("pivot.max_page_search_size", transform), equalTo(555)); + + final Request updateRequest = createRequestWithAuth( + "POST", + getTransformEndpoint() + transformId + "/_update", + BASIC_AUTH_VALUE_TRANSFORM_ADMIN_1 + ); + updateRequest.setJsonEntity("{}"); + + Map updateResponse = entityAsMap(client().performRequest(updateRequest)); + + assertNull(XContentMapValues.extractValue("pivot.max_page_search_size", updateResponse)); + assertThat(XContentMapValues.extractValue("settings.max_page_search_size", updateResponse), equalTo(555)); + + getRequest = createRequestWithAuth("GET", getTransformEndpoint() + transformId, BASIC_AUTH_VALUE_TRANSFORM_USER); + transforms = entityAsMap(client().performRequest(getRequest)); + assertEquals(1, XContentMapValues.extractValue("count", transforms)); + transform = ((List>) XContentMapValues.extractValue("transforms", transforms)).get(0); + + assertNull(XContentMapValues.extractValue("pivot.max_page_search_size", transform)); + assertThat(XContentMapValues.extractValue("settings.max_page_search_size", transform), equalTo(555)); + } + + @SuppressWarnings("unchecked") + public void testUpdateTransferRights() throws Exception { + String transformId = "transform1"; + // Note: Due to a bug the transform does not fail to start after deleting the user and role, therefore invalidating + // the credentials stored with the config. As a workaround we use a 2nd transform that uses the same config + // once the bug is fixed, delete this 2nd transform + String transformIdCloned = "transform2"; + String transformDest = transformId + "_idx"; + setupDataAccessRole(DATA_ACCESS_ROLE, REVIEWS_INDEX_NAME, transformDest); + setupDataAccessRole(DATA_ACCESS_ROLE_2, REVIEWS_INDEX_NAME, transformDest); + + final Request createTransformRequest = createRequestWithAuth( + "PUT", + getTransformEndpoint() + transformId, + BASIC_AUTH_VALUE_TRANSFORM_ADMIN_2 + ); + + final Request createTransformRequest_2 = createRequestWithAuth( + "PUT", + getTransformEndpoint() + transformIdCloned, + BASIC_AUTH_VALUE_TRANSFORM_ADMIN_2 + ); + + String config = "{ \"dest\": {\"index\":\"" + + transformDest + + "\"}," + + " \"source\": {\"index\":\"" + + REVIEWS_INDEX_NAME + + "\"}," + + " \"pivot\": {" + + " \"group_by\": {" + + " \"reviewer\": {" + + " \"terms\": {" + + " \"field\": \"user_id\"" + + " } } }," + + " \"aggregations\": {" + + " \"avg_rating\": {" + + " \"avg\": {" + + " \"field\": \"stars\"" + + " } } }" + + " }" + + "}"; + + createTransformRequest.setJsonEntity(config); + Map createTransformResponse = entityAsMap(client().performRequest(createTransformRequest)); + + assertThat(createTransformResponse.get("acknowledged"), equalTo(Boolean.TRUE)); + Request getRequest = createRequestWithAuth("GET", getTransformEndpoint() + transformId, BASIC_AUTH_VALUE_TRANSFORM_ADMIN_2); + Map transforms = entityAsMap(client().performRequest(getRequest)); + assertEquals(1, XContentMapValues.extractValue("count", transforms)); + + // create a 2nd, identical one + createTransformRequest_2.setJsonEntity(config); + createTransformResponse = entityAsMap(client().performRequest(createTransformRequest_2)); + assertThat(createTransformResponse.get("acknowledged"), equalTo(Boolean.TRUE)); + + // delete the user _and_ the role to access the data + deleteUser(TEST_ADMIN_USER_NAME_2); + deleteDataAccessRole(DATA_ACCESS_ROLE_2); + + // getting the transform with the just deleted admin 2 user should fail + try { + client().performRequest(getRequest); + fail("request should have failed"); + } catch (ResponseException e) { + assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(401)); + } + + // get the transform with admin 1 + getRequest = createRequestWithAuth("GET", getTransformEndpoint() + transformId, BASIC_AUTH_VALUE_TRANSFORM_ADMIN_1); + transforms = entityAsMap(client().performRequest(getRequest)); + assertEquals(1, XContentMapValues.extractValue("count", transforms)); + + // start using admin 1, but as the header is still admin 2 + // BUG: this should fail, because the transform can not access the source index any longer + startAndWaitForTransform(transformId, transformDest, BASIC_AUTH_VALUE_TRANSFORM_ADMIN_1); + + assertBusy(() -> { + Map transformStatsAsMap = getTransformStateAndStats(transformId); + assertThat(XContentMapValues.extractValue("stats.documents_indexed", transformStatsAsMap), equalTo(0)); + }, 3, TimeUnit.SECONDS); + + // update the transform with an empty body, the credentials (headers) should change + final Request updateRequest = createRequestWithAuth( + "POST", + getTransformEndpoint() + transformIdCloned + "/_update", + BASIC_AUTH_VALUE_TRANSFORM_ADMIN_1 + ); + updateRequest.setJsonEntity("{}"); + assertOK(client().performRequest(updateRequest)); + + // get should still work + getRequest = createRequestWithAuth("GET", getTransformEndpoint() + transformIdCloned, BASIC_AUTH_VALUE_TRANSFORM_ADMIN_1); + transforms = entityAsMap(client().performRequest(getRequest)); + assertEquals(1, XContentMapValues.extractValue("count", transforms)); + + // start with updated configuration should succeed + startAndWaitForTransform(transformIdCloned, transformDest, BASIC_AUTH_VALUE_TRANSFORM_ADMIN_1); + + assertBusy(() -> { + Map transformStatsAsMap = getTransformStateAndStats(transformIdCloned); + assertThat(XContentMapValues.extractValue("stats.documents_indexed", transformStatsAsMap), equalTo(27)); + }, 15, TimeUnit.SECONDS); + } + + private void deleteUser(String user) throws IOException { + Request request = new Request("DELETE", "/_security/user/" + user); + client().performRequest(request); + } + + protected void deleteDataAccessRole(String role) throws IOException { + Request request = new Request("DELETE", "/_security/role/" + role); + client().performRequest(request); + } +} diff --git a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportUpdateTransformAction.java b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportUpdateTransformAction.java index cab9dd72e1e..174f21e0a05 100644 --- a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportUpdateTransformAction.java +++ b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/action/TransportUpdateTransformAction.java @@ -56,6 +56,7 @@ import org.elasticsearch.xpack.core.transform.transforms.TransformConfigUpdate; import org.elasticsearch.xpack.core.transform.transforms.TransformDestIndexSettings; import org.elasticsearch.xpack.core.transform.transforms.TransformState; import org.elasticsearch.xpack.core.transform.transforms.TransformTaskState; +import org.elasticsearch.xpack.core.transform.transforms.persistence.TransformInternalIndexConstants; import org.elasticsearch.xpack.transform.TransformServices; import org.elasticsearch.xpack.transform.notifications.TransformAuditor; import org.elasticsearch.xpack.transform.persistence.SeqNoPrimaryTermAndIndex; @@ -193,9 +194,18 @@ public class TransportUpdateTransformAction extends TransportTasksAction { - final TransformConfig config = configAndVersion.v1(); + final TransformConfig oldConfig = configAndVersion.v1(); + final TransformConfig config = TransformConfig.rewriteForUpdate(oldConfig); + // If it is a noop don't bother even writing the doc, save the cycles, just return here. - if (update.isNoop(config)) { + // skip when: + // - config is in the latest index + // - rewrite did not change the config + // - update is not making any changes + if (config.getVersion() != null + && config.getVersion().onOrAfter(TransformInternalIndexConstants.INDEX_VERSION_LAST_CHANGED) + && config.equals(oldConfig) + && update.isNoop(config)) { listener.onResponse(new Response(config)); return; } @@ -213,8 +223,7 @@ public class TransportUpdateTransformAction extends TransportTasksAction { request.setConfig(updateResponse.getConfig());