diff --git a/docs/changelog/101487.yaml b/docs/changelog/101487.yaml new file mode 100644 index 0000000000000..b4531f7fd6f75 --- /dev/null +++ b/docs/changelog/101487.yaml @@ -0,0 +1,5 @@ +pr: 101487 +summary: Wait for async searches to finish when shutting down +area: Infra/Node Lifecycle +type: enhancement +issues: [] diff --git a/docs/changelog/102207.yaml b/docs/changelog/102207.yaml new file mode 100644 index 0000000000000..8b247828845f4 --- /dev/null +++ b/docs/changelog/102207.yaml @@ -0,0 +1,6 @@ +pr: 102207 +summary: Fix disk computation when initializing unassigned shards in desired balance + computation +area: Allocation +type: bug +issues: [] diff --git a/docs/changelog/103232.yaml b/docs/changelog/103232.yaml new file mode 100644 index 0000000000000..b955e7abb7683 --- /dev/null +++ b/docs/changelog/103232.yaml @@ -0,0 +1,5 @@ +pr: 103232 +summary: "Remove leniency in msearch parsing" +area: Search +type: enhancement +issues: [] diff --git a/docs/changelog/103453.yaml b/docs/changelog/103453.yaml new file mode 100644 index 0000000000000..4b7dab77c8b23 --- /dev/null +++ b/docs/changelog/103453.yaml @@ -0,0 +1,5 @@ +pr: 103453 +summary: Add expiration time to update api key api +area: Security +type: enhancement +issues: [] diff --git a/docs/changelog/103873.yaml b/docs/changelog/103873.yaml new file mode 100644 index 0000000000000..937106043ecf4 --- /dev/null +++ b/docs/changelog/103873.yaml @@ -0,0 +1,5 @@ +pr: 103873 +summary: Catch exceptions during `pytorch_inference` startup +area: Machine Learning +type: bug +issues: [] diff --git a/docs/reference/rest-api/security/bulk-update-api-keys.asciidoc b/docs/reference/rest-api/security/bulk-update-api-keys.asciidoc index 2e7034caaae89..faf87c67d1ccc 100644 --- a/docs/reference/rest-api/security/bulk-update-api-keys.asciidoc +++ b/docs/reference/rest-api/security/bulk-update-api-keys.asciidoc @@ -30,7 +30,7 @@ This operation can greatly improve performance over making individual updates. It's not possible to update expired or <> API keys. -This API supports updates to API key access scope and metadata. +This API supports updates to API key access scope, metadata and expiration. The access scope of each API key is derived from the <> you specify in the request, and a snapshot of the owner user's permissions at the time of the request. The snapshot of the owner's permissions is updated automatically on every call. @@ -63,6 +63,9 @@ The structure of a role descriptor is the same as the request for the <>. -This API supports updates to an API key's access scope and metadata. +This API supports updates to an API key's access scope, metadata and expiration. The access scope of an API key is derived from the <> you specify in the request, and a snapshot of the owner user's permissions at the time of the request. The snapshot of the owner's permissions is updated automatically on every call. @@ -67,6 +67,9 @@ It supports nested data structure. Within the `metadata` object, top-level keys beginning with `_` are reserved for system usage. When specified, this fully replaces metadata previously associated with the API key. +`expiration`:: +(Optional, string) Expiration time for the API key. By default, API keys never expire. Can be omitted to leave unchanged. + [[security-api-update-api-key-response-body]] ==== {api-response-body-title} diff --git a/docs/reference/rest-api/security/update-cross-cluster-api-key.asciidoc b/docs/reference/rest-api/security/update-cross-cluster-api-key.asciidoc index f0dfb11f1c98b..c22a1347c8262 100644 --- a/docs/reference/rest-api/security/update-cross-cluster-api-key.asciidoc +++ b/docs/reference/rest-api/security/update-cross-cluster-api-key.asciidoc @@ -34,7 +34,7 @@ Use this API to update cross-cluster API keys created by the <>. -This API supports updates to an API key's access scope and metadata. +This API supports updates to an API key's access scope, metadata and expiration. The owner user's information, e.g. `username`, `realm`, is also updated automatically on every call. NOTE: This API cannot update <>, which should be updated by @@ -66,6 +66,9 @@ It supports nested data structure. Within the `metadata` object, top-level keys beginning with `_` are reserved for system usage. When specified, this fully replaces metadata previously associated with the API key. +`expiration`:: +(Optional, string) Expiration time for the API key. By default, API keys never expire. Can be omitted to leave unchanged. + [[security-api-update-cross-cluster-api-key-response-body]] ==== {api-response-body-title} diff --git a/docs/reference/tab-widgets/esql/esql-getting-started-enrich-policy.asciidoc b/docs/reference/tab-widgets/esql/esql-getting-started-enrich-policy.asciidoc index c51a46bdef3b3..3e9796f89e94d 100644 --- a/docs/reference/tab-widgets/esql/esql-getting-started-enrich-policy.asciidoc +++ b/docs/reference/tab-widgets/esql/esql-getting-started-enrich-policy.asciidoc @@ -58,9 +58,9 @@ DELETE /_enrich/policy/clientip_policy // tag::demo-env[] -On the demo environment at https://esql.demo.elastic.co/[esql.demo.elastic.co], +On the demo environment at https://ela.st/ql/[ela.st/ql], an enrich policy called `clientip_policy` has already been created an executed. The policy links an IP address to an environment ("Development", "QA", or -"Production") +"Production"). // end::demo-env[] diff --git a/docs/reference/tab-widgets/esql/esql-getting-started-sample-data.asciidoc b/docs/reference/tab-widgets/esql/esql-getting-started-sample-data.asciidoc index 2a899a9f1ea33..d9b08b7281f77 100644 --- a/docs/reference/tab-widgets/esql/esql-getting-started-sample-data.asciidoc +++ b/docs/reference/tab-widgets/esql/esql-getting-started-sample-data.asciidoc @@ -43,6 +43,6 @@ PUT sample_data/_bulk The data set used in this guide has been preloaded into the Elastic {esql} public demo environment. Visit -https://esql.demo.elastic.co/[esql.demo.elastic.co] to start using it. +https://ela.st/ql[ela.st/ql] to start using it. // end::demo-env[] diff --git a/modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/transport/netty4/Netty4TransportMultiPortIntegrationIT.java b/modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/transport/netty4/Netty4TransportMultiPortIntegrationIT.java index 14952a2d37860..104677b64351f 100644 --- a/modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/transport/netty4/Netty4TransportMultiPortIntegrationIT.java +++ b/modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/transport/netty4/Netty4TransportMultiPortIntegrationIT.java @@ -7,6 +7,7 @@ */ package org.elasticsearch.transport.netty4; +import org.apache.lucene.util.Constants; import org.elasticsearch.ESNetty4IntegTestCase; import org.elasticsearch.action.admin.cluster.node.info.NodeInfo; import org.elasticsearch.action.admin.cluster.node.info.NodesInfoResponse; @@ -31,14 +32,16 @@ @ClusterScope(scope = Scope.SUITE, supportsDedicatedMasters = false, numDataNodes = 1, numClientNodes = 0) public class Netty4TransportMultiPortIntegrationIT extends ESNetty4IntegTestCase { + private static final int NUMBER_OF_CLIENT_PORTS = Constants.WINDOWS ? 300 : 10; + private static int randomPort = -1; private static String randomPortRange; @Override protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) { if (randomPort == -1) { - randomPort = randomIntBetween(49152, 65525); - randomPortRange = Strings.format("%s-%s", randomPort, randomPort + 10); + randomPort = randomIntBetween(49152, 65535 - NUMBER_OF_CLIENT_PORTS); + randomPortRange = Strings.format("%s-%s", randomPort, randomPort + NUMBER_OF_CLIENT_PORTS); } Settings.Builder builder = Settings.builder() .put(super.nodeSettings(nodeOrdinal, otherSettings)) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/indices/recovery/IndexRecoveryIT.java b/server/src/internalClusterTest/java/org/elasticsearch/indices/recovery/IndexRecoveryIT.java index 5dcd8b5b0e34f..bd400f9f0f6a1 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/indices/recovery/IndexRecoveryIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/indices/recovery/IndexRecoveryIT.java @@ -782,10 +782,13 @@ public Settings onNodeStopped(String nodeName) { * Tests shard recovery throttling on the target node. Node statistics should show throttling time on the target node, while no * throttling should be shown on the source node because the target will accept data more slowly than the source's throttling threshold. */ - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/103204") public void testTargetThrottling() throws Exception { logger.info("--> starting node A with default settings"); - final String nodeA = internalCluster().startNode(); + final String nodeA = internalCluster().startNode( + Settings.builder() + // Use a high value so that when unthrottling recoveries we do not cause accidental throttling on the source node. + .put(RecoverySettings.INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING.getKey(), "200mb") + ); logger.info("--> creating index on node A"); ByteSizeValue shardSize = createAndPopulateIndex(INDEX_NAME, 1, SHARD_COUNT_1, REPLICA_COUNT_0).getShards()[0].getStats() diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SnapshotShutdownIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SnapshotShutdownIT.java index 746d2907f47dc..922fd2ce4cb21 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SnapshotShutdownIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SnapshotShutdownIT.java @@ -28,6 +28,7 @@ import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.NodesShutdownMetadata; import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata; +import org.elasticsearch.cluster.routing.ShardRoutingState; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Priority; import org.elasticsearch.common.Strings; @@ -46,6 +47,7 @@ import java.util.stream.Stream; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.oneOf; @@ -84,6 +86,7 @@ public void testRestartNodeDuringSnapshot() throws Exception { ); return false; }); + addUnassignedShardsWatcher(clusterService, indexName); PlainActionFuture.get( fut -> putShutdownMetadata( @@ -117,6 +120,7 @@ public void testRemoveNodeDuringSnapshot() throws Exception { final var clusterService = internalCluster().getCurrentMasterNodeInstance(ClusterService.class); final var snapshotFuture = startFullSnapshotBlockedOnDataNode(randomIdentifier(), repoName, originalNode); final var snapshotPausedListener = createSnapshotPausedListener(clusterService, repoName, indexName); + addUnassignedShardsWatcher(clusterService, indexName); updateIndexSettings(Settings.builder().putNull(REQUIRE_NODE_NAME_SETTING), indexName); putShutdownForRemovalMetadata(originalNode, clusterService); @@ -128,7 +132,6 @@ public void testRemoveNodeDuringSnapshot() throws Exception { if (randomBoolean()) { internalCluster().stopNode(originalNode); - ensureGreen(indexName); } clearShutdownMetadata(clusterService); @@ -146,6 +149,7 @@ public void testRemoveNodeAndFailoverMasterDuringSnapshot() throws Exception { final var clusterService = internalCluster().getCurrentMasterNodeInstance(ClusterService.class); final var snapshotFuture = startFullSnapshotBlockedOnDataNode(randomIdentifier(), repoName, originalNode); final var snapshotPausedListener = createSnapshotPausedListener(clusterService, repoName, indexName); + addUnassignedShardsWatcher(clusterService, indexName); final var snapshotStatusUpdateBarrier = new CyclicBarrier(2); final var masterName = internalCluster().getMasterName(); @@ -258,6 +262,8 @@ public void testRemoveNodeDuringSnapshotWithOtherRunningShardSnapshots() throws final var clusterService = internalCluster().getCurrentMasterNodeInstance(ClusterService.class); final var snapshotFuture = startFullSnapshotBlockedOnDataNode(randomIdentifier(), repoName, nodeForRemoval); final var snapshotPausedListener = createSnapshotPausedListener(clusterService, repoName, indexName); + addUnassignedShardsWatcher(clusterService, indexName); + waitForBlock(otherNode, repoName); putShutdownForRemovalMetadata(nodeForRemoval, clusterService); @@ -312,6 +318,7 @@ public void testStartRemoveNodeButDoNotComplete() throws Exception { final var clusterService = internalCluster().getCurrentMasterNodeInstance(ClusterService.class); final var snapshotFuture = startFullSnapshotBlockedOnDataNode(randomIdentifier(), repoName, primaryNode); final var snapshotPausedListener = createSnapshotPausedListener(clusterService, repoName, indexName); + addUnassignedShardsWatcher(clusterService, indexName); putShutdownForRemovalMetadata(primaryNode, clusterService); unblockAllDataNodes(repoName); // lets the shard snapshot abort, but allocation filtering stops it from moving @@ -351,6 +358,7 @@ public void testAbortSnapshotWhileRemovingNode() throws Exception { ); final var clusterService = internalCluster().getCurrentMasterNodeInstance(ClusterService.class); + addUnassignedShardsWatcher(clusterService, indexName); putShutdownForRemovalMetadata(primaryNode, clusterService); unblockAllDataNodes(repoName); // lets the shard snapshot abort, but allocation filtering stops it from moving safeAwait(updateSnapshotStatusBarrier); // wait for data node to notify master that the shard snapshot is paused @@ -395,6 +403,7 @@ public void testShutdownWhileSuccessInFlight() throws Exception { ) ); + addUnassignedShardsWatcher(clusterService, indexName); assertEquals( SnapshotState.SUCCESS, startFullSnapshot(repoName, randomIdentifier()).get(10, TimeUnit.SECONDS).getSnapshotInfo().state() @@ -424,6 +433,18 @@ private static SubscribableListener createSnapshotPausedListener( }); } + private static void addUnassignedShardsWatcher(ClusterService clusterService, String indexName) { + ClusterServiceUtils.addTemporaryStateListener(clusterService, state -> { + final var indexRoutingTable = state.routingTable().index(indexName); + if (indexRoutingTable == null) { + // index was deleted, can remove this listener now + return true; + } + assertThat(indexRoutingTable.shardsWithState(ShardRoutingState.UNASSIGNED), empty()); + return false; + }); + } + private static void putShutdownForRemovalMetadata(String nodeName, ClusterService clusterService) { PlainActionFuture.get( fut -> putShutdownForRemovalMetadata(clusterService, nodeName, fut), diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index cfbc1eb8225ea..ce1f6d9ecaad5 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -177,6 +177,7 @@ static TransportVersion def(int id) { public static final TransportVersion ESQL_CLUSTER_ALIAS = def(8_565_00_0); public static final TransportVersion SNAPSHOTS_IN_PROGRESS_TRACKING_REMOVING_NODES_ADDED = def(8_566_00_0); public static final TransportVersion SMALLER_RELOAD_SECURE_SETTINGS_REQUEST = def(8_567_00_0); + public static final TransportVersion UPDATE_API_KEY_EXPIRATION_TIME_ADDED = def(8_568_00_0); /* * STOP! READ THIS FIRST! No, really, diff --git a/server/src/main/java/org/elasticsearch/action/search/MultiSearchRequest.java b/server/src/main/java/org/elasticsearch/action/search/MultiSearchRequest.java index 2e50667fc02b1..f33f13dd7741f 100644 --- a/server/src/main/java/org/elasticsearch/action/search/MultiSearchRequest.java +++ b/server/src/main/java/org/elasticsearch/action/search/MultiSearchRequest.java @@ -29,6 +29,7 @@ import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.XContent; import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.XContentParseException; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.XContentParserConfiguration; @@ -246,6 +247,9 @@ public static void readMultiLineFormat( ) ) { Map source = parser.map(); + if (parser.nextToken() != null) { + throw new XContentParseException(parser.getTokenLocation(), "Unexpected token after end of object"); + } Object expandWildcards = null; Object ignoreUnavailable = null; Object ignoreThrottled = null; @@ -312,6 +316,9 @@ public static void readMultiLineFormat( ) ) { consumer.accept(searchRequest, parser); + if (parser.nextToken() != null) { + throw new XContentParseException(parser.getTokenLocation(), "Unexpected token after end of object"); + } } // move pointers from = nextMarker + 1; diff --git a/server/src/main/java/org/elasticsearch/cluster/ClusterInfoSimulator.java b/server/src/main/java/org/elasticsearch/cluster/ClusterInfoSimulator.java index f9e56b5b2ff2d..9019ee465c936 100644 --- a/server/src/main/java/org/elasticsearch/cluster/ClusterInfoSimulator.java +++ b/server/src/main/java/org/elasticsearch/cluster/ClusterInfoSimulator.java @@ -8,7 +8,9 @@ package org.elasticsearch.cluster; +import org.elasticsearch.cluster.ClusterInfo.NodeAndShard; import org.elasticsearch.cluster.routing.ShardRouting; +import org.elasticsearch.cluster.routing.allocation.RoutingAllocation; import org.elasticsearch.common.util.CopyOnFirstWriteMap; import org.elasticsearch.index.shard.ShardId; @@ -16,20 +18,28 @@ import java.util.Map; import java.util.Objects; +import static org.elasticsearch.cluster.ClusterInfo.shardIdentifierFromRouting; +import static org.elasticsearch.cluster.routing.ExpectedShardSizeEstimator.getExpectedShardSize; +import static org.elasticsearch.cluster.routing.ExpectedShardSizeEstimator.shouldReserveSpaceForInitializingShard; +import static org.elasticsearch.cluster.routing.ShardRouting.UNAVAILABLE_EXPECTED_SHARD_SIZE; + public class ClusterInfoSimulator { + private final RoutingAllocation allocation; + private final Map leastAvailableSpaceUsage; private final Map mostAvailableSpaceUsage; private final CopyOnFirstWriteMap shardSizes; private final Map shardDataSetSizes; - private final Map dataPath; + private final Map dataPath; - public ClusterInfoSimulator(ClusterInfo clusterInfo) { - this.leastAvailableSpaceUsage = new HashMap<>(clusterInfo.getNodeLeastAvailableDiskUsages()); - this.mostAvailableSpaceUsage = new HashMap<>(clusterInfo.getNodeMostAvailableDiskUsages()); - this.shardSizes = new CopyOnFirstWriteMap<>(clusterInfo.shardSizes); - this.shardDataSetSizes = Map.copyOf(clusterInfo.shardDataSetSizes); - this.dataPath = Map.copyOf(clusterInfo.dataPath); + public ClusterInfoSimulator(RoutingAllocation allocation) { + this.allocation = allocation; + this.leastAvailableSpaceUsage = new HashMap<>(allocation.clusterInfo().getNodeLeastAvailableDiskUsages()); + this.mostAvailableSpaceUsage = new HashMap<>(allocation.clusterInfo().getNodeMostAvailableDiskUsages()); + this.shardSizes = new CopyOnFirstWriteMap<>(allocation.clusterInfo().shardSizes); + this.shardDataSetSizes = Map.copyOf(allocation.clusterInfo().shardDataSetSizes); + this.dataPath = Map.copyOf(allocation.clusterInfo().dataPath); } /** @@ -43,49 +53,50 @@ public ClusterInfoSimulator(ClusterInfo clusterInfo) { public void simulateShardStarted(ShardRouting shard) { assert shard.initializing(); - var size = getEstimatedShardSize(shard); - if (size != null && size > 0) { + var size = getExpectedShardSize( + shard, + UNAVAILABLE_EXPECTED_SHARD_SIZE, + getClusterInfo(), + allocation.snapshotShardSizeInfo(), + allocation.metadata(), + allocation.routingTable() + ); + if (size != UNAVAILABLE_EXPECTED_SHARD_SIZE) { if (shard.relocatingNodeId() != null) { // relocation modifyDiskUsage(shard.relocatingNodeId(), size); modifyDiskUsage(shard.currentNodeId(), -size); } else { // new shard - modifyDiskUsage(shard.currentNodeId(), -size); - shardSizes.put(ClusterInfo.shardIdentifierFromRouting(shard), size); + if (shouldReserveSpaceForInitializingShard(shard, allocation.metadata())) { + modifyDiskUsage(shard.currentNodeId(), -size); + } + shardSizes.put( + shardIdentifierFromRouting(shard), + allocation.metadata().getIndexSafe(shard.index()).ignoreDiskWatermarks() ? 0 : size + ); } } } - private Long getEstimatedShardSize(ShardRouting shard) { - if (shard.relocatingNodeId() != null) { - // relocation existing shard, get size of the source shard - return shardSizes.get(ClusterInfo.shardIdentifierFromRouting(shard)); - } else if (shard.primary() == false) { - // initializing new replica, get size of the source primary shard - return shardSizes.get(ClusterInfo.shardIdentifierFromRouting(shard.shardId(), true)); - } else { - // initializing new (empty?) primary - return shard.getExpectedShardSize(); + private void modifyDiskUsage(String nodeId, long freeDelta) { + if (freeDelta == 0) { + return; } - } - - private void modifyDiskUsage(String nodeId, long delta) { var diskUsage = mostAvailableSpaceUsage.get(nodeId); if (diskUsage == null) { return; } var path = diskUsage.getPath(); + updateDiskUsage(leastAvailableSpaceUsage, nodeId, path, freeDelta); + updateDiskUsage(mostAvailableSpaceUsage, nodeId, path, freeDelta); + } - var leastUsage = leastAvailableSpaceUsage.get(nodeId); - if (leastUsage != null && Objects.equals(leastUsage.getPath(), path)) { - // ensure new value is within bounds - leastAvailableSpaceUsage.put(nodeId, updateWithFreeBytes(leastUsage, delta)); - } - var mostUsage = mostAvailableSpaceUsage.get(nodeId); - if (mostUsage != null && Objects.equals(mostUsage.getPath(), path)) { + private void updateDiskUsage(Map availableSpaceUsage, String nodeId, String path, long freeDelta) { + var usage = availableSpaceUsage.get(nodeId); + if (usage != null && Objects.equals(usage.getPath(), path)) { // ensure new value is within bounds - mostAvailableSpaceUsage.put(nodeId, updateWithFreeBytes(mostUsage, delta)); + availableSpaceUsage.put(nodeId, updateWithFreeBytes(usage, freeDelta)); } } diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/ExpectedShardSizeEstimator.java b/server/src/main/java/org/elasticsearch/cluster/routing/ExpectedShardSizeEstimator.java index 1f364e1ace6e4..46a45a058f123 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/ExpectedShardSizeEstimator.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/ExpectedShardSizeEstimator.java @@ -48,6 +48,8 @@ public static boolean shouldReserveSpaceForInitializingShard(ShardRouting shard, case PEER -> true; // Snapshot restore (unless it is partial) require downloading all segments locally from the blobstore to start the shard. + // See org.elasticsearch.xpack.searchablesnapshots.action.TransportMountSearchableSnapshotAction.buildIndexSettings + // and DiskThresholdDecider.SETTING_IGNORE_DISK_WATERMARKS case SNAPSHOT -> metadata.getIndexSafe(shard.index()).isPartialSearchableSnapshot() == false; // shrink/split/clone operation is going to clone existing locally placed shards using file system hard links diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java index 60a6ec2e49899..effd5ec110c44 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java @@ -84,7 +84,7 @@ public DesiredBalance compute( final var knownNodeIds = routingNodes.getAllNodeIds(); final var changes = routingAllocation.changes(); final var ignoredShards = getIgnoredShardsWithDiscardedAllocationStatus(desiredBalanceInput.ignoredShards()); - final var clusterInfoSimulator = new ClusterInfoSimulator(routingAllocation.clusterInfo()); + final var clusterInfoSimulator = new ClusterInfoSimulator(routingAllocation); if (routingNodes.size() == 0) { return new DesiredBalance(desiredBalanceInput.index(), Map.of()); @@ -274,7 +274,7 @@ public DesiredBalance compute( routingAllocation.setSimulatedClusterInfo(clusterInfoSimulator.getClusterInfo()); logger.trace("running delegate allocator"); delegateAllocator.allocate(routingAllocation); - assert routingNodes.unassigned().size() == 0; // any unassigned shards should now be ignored + assert routingNodes.unassigned().isEmpty(); // any unassigned shards should now be ignored hasChanges = false; for (final var routingNode : routingNodes) { diff --git a/server/src/main/java/org/elasticsearch/common/util/CopyOnFirstWriteMap.java b/server/src/main/java/org/elasticsearch/common/util/CopyOnFirstWriteMap.java index 08d86c143fc1b..51491df02c37c 100644 --- a/server/src/main/java/org/elasticsearch/common/util/CopyOnFirstWriteMap.java +++ b/server/src/main/java/org/elasticsearch/common/util/CopyOnFirstWriteMap.java @@ -12,6 +12,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.Set; /** @@ -70,7 +71,11 @@ public V get(Object key) { @Override public V put(K key, V value) { - return getForUpdate().put(key, value); + if (Objects.equals(get(key), value)) { + return value; + } else { + return getForUpdate().put(key, value); + } } @Override diff --git a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index fd288df9b377f..08fc9e55fd408 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -2177,8 +2177,7 @@ private boolean shouldPeriodicallyFlush(long flushThresholdSizeInBytes, long flu @Override protected void flushHoldingLock(boolean force, boolean waitIfOngoing, ActionListener listener) throws EngineException { - assert isClosed.get() == false; // might be closing, but not closed yet - ensureOpen(); + ensureOpen(); // best-effort, a concurrent failEngine() can still happen but that's ok if (force && waitIfOngoing == false) { assert false : "wait_if_ongoing must be true for a force flush: force=" + force + " wait_if_ongoing=" + waitIfOngoing; throw new IllegalArgumentException( diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 8f493977484a8..6da1dbc3e5c52 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -13,6 +13,7 @@ import org.apache.lucene.util.SetOnce; import org.elasticsearch.ElasticsearchTimeoutException; import org.elasticsearch.Version; +import org.elasticsearch.action.search.TransportSearchAction; import org.elasticsearch.bootstrap.BootstrapCheck; import org.elasticsearch.bootstrap.BootstrapContext; import org.elasticsearch.client.internal.Client; @@ -40,6 +41,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.common.transport.TransportAddress; +import org.elasticsearch.common.util.concurrent.FutureUtils; import org.elasticsearch.core.Assertions; import org.elasticsearch.core.IOUtils; import org.elasticsearch.core.PathUtils; @@ -75,6 +77,7 @@ import org.elasticsearch.snapshots.SnapshotShardsService; import org.elasticsearch.snapshots.SnapshotsService; import org.elasticsearch.tasks.TaskCancellationService; +import org.elasticsearch.tasks.TaskManager; import org.elasticsearch.tasks.TaskResultsService; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.RemoteClusterPortSettings; @@ -97,14 +100,16 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.FutureTask; import java.util.concurrent.TimeUnit; import java.util.function.BiConsumer; import java.util.function.Function; import javax.net.ssl.SNIHostName; +import static org.elasticsearch.core.Strings.format; + /** * A node represent a node within a cluster ({@code cluster.name}). The {@link #client()} can be used * in order to use a {@link Client} to perform actions/operations against the cluster. @@ -148,6 +153,12 @@ public class Node implements Closeable { Property.NodeScope ); + public static final Setting MAXIMUM_SHUTDOWN_TIMEOUT_SETTING = Setting.positiveTimeSetting( + "node.maximum_shutdown_grace_period", + TimeValue.timeValueMillis(0), + Setting.Property.NodeScope + ); + private final Lifecycle lifecycle = new Lifecycle(); /** @@ -580,17 +591,92 @@ public synchronized void close() throws IOException { */ public void prepareForClose() { HttpServerTransport httpServerTransport = injector.getInstance(HttpServerTransport.class); - FutureTask stopper = new FutureTask<>(httpServerTransport::close, null); - new Thread(stopper, "http-server-transport-stop").start(); - + Map stoppers = new HashMap<>(); + TimeValue maxTimeout = MAXIMUM_SHUTDOWN_TIMEOUT_SETTING.get(this.settings()); + stoppers.put("http-server-transport-stop", httpServerTransport::close); + stoppers.put("async-search-stop", () -> this.awaitSearchTasksComplete(maxTimeout)); if (terminationHandler != null) { - terminationHandler.handleTermination(); + stoppers.put("termination-handler-stop", terminationHandler::handleTermination); + } + + Map> futures = new HashMap<>(stoppers.size()); + for (var stopperEntry : stoppers.entrySet()) { + var future = new CompletableFuture(); + new Thread(() -> { + try { + stopperEntry.getValue().run(); + } catch (Exception ex) { + logger.warn("unexpected exception in shutdown task [" + stopperEntry.getKey() + "]", ex); + } finally { + future.complete(null); + } + }, stopperEntry.getKey()).start(); + futures.put(stopperEntry.getKey(), future); } + @SuppressWarnings(value = "rawtypes") // Can't make an array of parameterized types, but it complains if you leave the type out + CompletableFuture allStoppers = CompletableFuture.allOf(futures.values().toArray(new CompletableFuture[stoppers.size()])); + try { - stopper.get(); - } catch (Exception e) { - logger.warn("unexpected exception while waiting for http server to close", e); + if (maxTimeout.millis() == 0) { + FutureUtils.get(allStoppers); + } else { + FutureUtils.get(allStoppers, maxTimeout.millis(), TimeUnit.MILLISECONDS); + } + + } catch (ElasticsearchTimeoutException t) { + var unfinishedTasks = futures.entrySet() + .stream() + .filter(entry -> entry.getValue().isDone() == false) + .map(Map.Entry::getKey) + .toList(); + logger.warn("timed out while waiting for graceful shutdown tasks: " + unfinishedTasks); + } + } + + private void awaitSearchTasksComplete(TimeValue asyncSearchTimeout) { + TaskManager taskManager = injector.getInstance(TransportService.class).getTaskManager(); + long millisWaited = 0; + while (true) { + long searchTasksRemaining = taskManager.getTasks() + .values() + .stream() + .filter(task -> TransportSearchAction.TYPE.name().equals(task.getAction())) + .count(); + if (searchTasksRemaining == 0) { + logger.debug("all search tasks complete"); + return; + } else { + // Let the system work on those searches for a while. We're on a dedicated thread to manage app shutdown, so we + // literally just want to wait and not take up resources on this thread for now. Poll period chosen to allow short + // response times, but checking the tasks list is relatively expensive, and we don't want to waste CPU time we could + // be spending on finishing those searches. + final TimeValue pollPeriod = TimeValue.timeValueMillis(500); + millisWaited += pollPeriod.millis(); + if (millisWaited >= asyncSearchTimeout.millis()) { + logger.warn( + format( + "timed out after waiting [%s] for [%d] search tasks to finish", + asyncSearchTimeout.toString(), + searchTasksRemaining + ) + ); + return; + } + logger.debug(format("waiting for [%s] search tasks to finish, next poll in [%s]", searchTasksRemaining, pollPeriod)); + try { + Thread.sleep(pollPeriod.millis()); + } catch (InterruptedException ex) { + logger.warn( + format( + "interrupted while waiting [%s] for [%d] search tasks to finish", + asyncSearchTimeout.toString(), + searchTasksRemaining + ) + ); + return; + } + } } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/stats/SearchUsageStatsTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/stats/SearchUsageStatsTests.java index a5704748ea242..a5bf7b39669e7 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/stats/SearchUsageStatsTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/stats/SearchUsageStatsTests.java @@ -8,7 +8,6 @@ package org.elasticsearch.action.admin.cluster.stats; -import org.apache.lucene.tests.util.LuceneTestCase; import org.elasticsearch.TransportVersion; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.Writeable.Reader; @@ -20,7 +19,6 @@ import java.util.List; import java.util.Map; -@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/102920") // failing test is final, mute whole suite public class SearchUsageStatsTests extends AbstractWireSerializingTestCase { private static final List QUERY_TYPES = List.of( @@ -108,7 +106,7 @@ protected SearchUsageStats mutateInstance(SearchUsageStats instance) { case 2 -> new SearchUsageStats( instance.getQueryUsage(), instance.getRescorerUsage(), - randomValueOtherThan(instance.getRescorerUsage(), () -> randomSectionsUsage(randomIntBetween(0, SECTIONS.size()))), + randomValueOtherThan(instance.getSectionsUsage(), () -> randomSectionsUsage(randomIntBetween(0, SECTIONS.size()))), instance.getTotalSearchCount() ); default -> new SearchUsageStats( diff --git a/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java b/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java index 7ba320d84f91e..e9330c9643318 100644 --- a/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java @@ -30,6 +30,7 @@ import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xcontent.ParseField; import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.XContentParseException; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xcontent.json.JsonXContent; @@ -87,7 +88,7 @@ public void testSimpleAdd() throws Exception { public void testFailWithUnknownKey() { final String requestContent = """ - {"index":"test", "ignore_unavailable" : true, "unknown_key" : "open,closed"}} + {"index":"test", "ignore_unavailable" : true, "unknown_key" : "open,closed"} {"query" : {"match_all" :{}}} """; FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry()).withContent( @@ -103,7 +104,7 @@ public void testFailWithUnknownKey() { public void testSimpleAddWithCarriageReturn() throws Exception { final String requestContent = """ - {"index":"test", "ignore_unavailable" : true, "expand_wildcards" : "open,closed"}} + {"index":"test", "ignore_unavailable" : true, "expand_wildcards" : "open,closed"} {"query" : {"match_all" :{}}} """; FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry()).withContent( @@ -121,7 +122,7 @@ public void testSimpleAddWithCarriageReturn() throws Exception { public void testDefaultIndicesOptions() throws IOException { final String requestContent = """ - {"index":"test", "expand_wildcards" : "open,closed"}} + {"index":"test", "expand_wildcards" : "open,closed"} {"query" : {"match_all" :{}}} """; FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry()).withContent( @@ -542,6 +543,41 @@ public void testEqualsAndHashcode() { checkEqualsAndHashCode(createMultiSearchRequest(), MultiSearchRequestTests::copyRequest, MultiSearchRequestTests::mutate); } + public void testFailOnExtraCharacters() throws IOException { + try { + parseMultiSearchRequestFromString(""" + {"index": "test"}{{{{{extra chars that shouldn't be here + { "query": {"match_all": {}}} + """, null); + fail("should have caught first line; extra open brackets"); + } catch (XContentParseException e) { + assertEquals("[1:18] Unexpected token after end of object", e.getMessage()); + } + try { + parseMultiSearchRequestFromString(""" + {"index": "test"} + { "query": {"match_all": {}}}{{{{even more chars + """, null); + fail("should have caught second line"); + } catch (XContentParseException e) { + assertEquals("[1:30] Unexpected token after end of object", e.getMessage()); + } + try { + parseMultiSearchRequestFromString(""" + {} + { "query": {"match_all": {}}}}}}different error message + """, null); + fail("should have caught second line; extra closing brackets"); + } catch (XContentParseException e) { + assertEquals( + "[1:31] Unexpected close marker '}': expected ']' (for root starting at " + + "[Source: (byte[])\"{ \"query\": {\"match_all\": {}}}}}}different error message\"; line: 1, column: 0])\n " + + "at [Source: (byte[])\"{ \"query\": {\"match_all\": {}}}}}}different error message\"; line: 1, column: 31]", + e.getMessage() + ); + } + } + private static MultiSearchRequest mutate(MultiSearchRequest searchRequest) throws IOException { MultiSearchRequest mutation = copyRequest(searchRequest); List> mutators = new ArrayList<>(); diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/ExpectedShardSizeEstimatorTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/ExpectedShardSizeEstimatorTests.java index f81d99c55e84e..dd92589f0af89 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/ExpectedShardSizeEstimatorTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/ExpectedShardSizeEstimatorTests.java @@ -158,7 +158,7 @@ public void testShouldReadSizeFromClonedShard() { var state = ClusterState.builder(ClusterName.DEFAULT) .metadata( metadata( - IndexMetadata.builder("source").settings(indexSettings(IndexVersion.current(), 2, 0)), + IndexMetadata.builder("source").settings(indexSettings(IndexVersion.current(), 1, 0)), IndexMetadata.builder("target") .settings( indexSettings(IndexVersion.current(), 1, 0) // diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/ClusterInfoSimulatorTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/ClusterInfoSimulatorTests.java index 1d2a7f05ff1f2..7aecd611b931b 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/ClusterInfoSimulatorTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/ClusterInfoSimulatorTests.java @@ -9,54 +9,73 @@ package org.elasticsearch.cluster.routing.allocation.allocator; import org.elasticsearch.cluster.ClusterInfo; +import org.elasticsearch.cluster.ClusterInfo.NodeAndPath; +import org.elasticsearch.cluster.ClusterInfo.ReservedSpace; import org.elasticsearch.cluster.ClusterInfoSimulator; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.DiskUsage; +import org.elasticsearch.cluster.ESAllocationTestCase; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.Metadata; -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.cluster.node.DiscoveryNodeRole; -import org.elasticsearch.cluster.node.DiscoveryNodeUtils; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.routing.IndexRoutingTable; +import org.elasticsearch.cluster.routing.RecoverySource; import org.elasticsearch.cluster.routing.RoutingTable; import org.elasticsearch.cluster.routing.ShardRouting; +import org.elasticsearch.cluster.routing.ShardRoutingState; import org.elasticsearch.cluster.routing.allocation.RoutingAllocation; +import org.elasticsearch.cluster.routing.allocation.decider.AllocationDecider; import org.elasticsearch.cluster.routing.allocation.decider.AllocationDeciders; import org.elasticsearch.cluster.routing.allocation.decider.Decision; import org.elasticsearch.cluster.routing.allocation.decider.DiskThresholdDecider; -import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexVersion; +import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.repositories.IndexId; +import org.elasticsearch.snapshots.InternalSnapshotsInfoService; +import org.elasticsearch.snapshots.Snapshot; +import org.elasticsearch.snapshots.SnapshotId; import org.elasticsearch.snapshots.SnapshotShardSizeInfo; -import org.elasticsearch.test.ESTestCase; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Set; +import static org.elasticsearch.cluster.ClusterInfo.shardIdentifierFromRouting; +import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_RESIZE_SOURCE_NAME_KEY; +import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_RESIZE_SOURCE_UUID_KEY; import static org.elasticsearch.cluster.routing.ShardRoutingState.INITIALIZING; import static org.elasticsearch.cluster.routing.ShardRoutingState.STARTED; -import static org.elasticsearch.cluster.routing.ShardRoutingState.UNASSIGNED; import static org.elasticsearch.cluster.routing.TestShardRouting.newShardRouting; +import static org.elasticsearch.cluster.routing.allocation.decider.DiskThresholdDecider.SETTING_IGNORE_DISK_WATERMARKS; +import static org.elasticsearch.index.IndexModule.INDEX_STORE_TYPE_SETTING; +import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOT_STORE_TYPE; +import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SNAPSHOT_PARTIAL_SETTING; import static org.hamcrest.Matchers.equalTo; -public class ClusterInfoSimulatorTests extends ESTestCase { +public class ClusterInfoSimulatorTests extends ESAllocationTestCase { public void testInitializeNewPrimary() { - var newPrimary = newShardRouting("index-1", 0, "node-0", true, INITIALIZING); - - var simulator = new ClusterInfoSimulator( - new ClusterInfoTestBuilder() // - .withNode("node-0", new DiskUsageBuilder(1000, 1000)) - .withNode("node-1", new DiskUsageBuilder(1000, 1000)) - .withShard(newPrimary, 0) - .build() + var newPrimary = newShardRouting( + new ShardId("my-index", "_na_", 0), + "node-0", + true, + ShardRoutingState.INITIALIZING, + RecoverySource.EmptyStoreRecoverySource.INSTANCE ); + + var initialClusterInfo = new ClusterInfoTestBuilder() // + .withNode("node-0", new DiskUsageBuilder(1000, 1000)) + .build(); + + var state = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().put(IndexMetadata.builder("my-index").settings(indexSettings(IndexVersion.current(), 1, 0)))) + .build(); + var allocation = createRoutingAllocation(state, initialClusterInfo, SnapshotShardSizeInfo.EMPTY); + var simulator = new ClusterInfoSimulator(allocation); simulator.simulateShardStarted(newPrimary); assertThat( @@ -64,32 +83,39 @@ public void testInitializeNewPrimary() { equalTo( new ClusterInfoTestBuilder() // .withNode("node-0", new DiskUsageBuilder(1000, 1000)) - .withNode("node-1", new DiskUsageBuilder(1000, 1000)) - .withShard(newPrimary, 0) .build() ) ); } - public void testInitializeNewPrimaryWithKnownExpectedSize() { - - var newPrimary = newShardRouting("index-1", 0, null, true, UNASSIGNED).initialize("node-0", null, 100); + public void testInitializePreviouslyExistingPrimary() { - var simulator = new ClusterInfoSimulator( - new ClusterInfoTestBuilder() // - .withNode("node-0", new DiskUsageBuilder(1000, 1000)) - .withNode("node-1", new DiskUsageBuilder(1000, 1000)) - .build() + var existingPrimary = newShardRouting( + new ShardId("my-index", "_na_", 0), + "node-0", + true, + ShardRoutingState.INITIALIZING, + RecoverySource.ExistingStoreRecoverySource.INSTANCE ); - simulator.simulateShardStarted(newPrimary); + + var initialClusterInfo = new ClusterInfoTestBuilder() // + .withNode("node-0", new DiskUsageBuilder(1000, 900)) + .withShard(existingPrimary, 100) + .build(); + + var state = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().put(IndexMetadata.builder("my-index").settings(indexSettings(IndexVersion.current(), 1, 0)))) + .build(); + var allocation = createRoutingAllocation(state, initialClusterInfo, SnapshotShardSizeInfo.EMPTY); + var simulator = new ClusterInfoSimulator(allocation); + simulator.simulateShardStarted(existingPrimary); assertThat( simulator.getClusterInfo(), equalTo( new ClusterInfoTestBuilder() // .withNode("node-0", new DiskUsageBuilder(1000, 900)) - .withNode("node-1", new DiskUsageBuilder(1000, 1000)) - .withShard(newPrimary, 100) + .withShard(existingPrimary, 100) .build() ) ); @@ -97,17 +123,26 @@ public void testInitializeNewPrimaryWithKnownExpectedSize() { public void testInitializeNewReplica() { - var existingPrimary = newShardRouting("index-1", 0, "node-0", true, STARTED); - var newReplica = newShardRouting("index-1", 0, "node-1", false, INITIALIZING); - - var simulator = new ClusterInfoSimulator( - new ClusterInfoTestBuilder() // - .withNode("node-0", new DiskUsageBuilder(1000, 900)) - .withNode("node-1", new DiskUsageBuilder(1000, 1000)) - .withShard(existingPrimary, 100) - .withShard(newReplica, 0) - .build() + var existingPrimary = newShardRouting(new ShardId("my-index", "_na_", 0), "node-0", true, STARTED); + var newReplica = newShardRouting( + new ShardId("my-index", "_na_", 0), + "node-1", + false, + INITIALIZING, + RecoverySource.PeerRecoverySource.INSTANCE ); + + var initialClusterInfo = new ClusterInfoTestBuilder() // + .withNode("node-0", new DiskUsageBuilder(1000, 900)) + .withNode("node-1", new DiskUsageBuilder(1000, 1000)) + .withShard(existingPrimary, 100) + .build(); + + var state = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().put(IndexMetadata.builder("my-index").settings(indexSettings(IndexVersion.current(), 1, 1)))) + .build(); + var allocation = createRoutingAllocation(state, initialClusterInfo, SnapshotShardSizeInfo.EMPTY); + var simulator = new ClusterInfoSimulator(allocation); simulator.simulateShardStarted(newReplica); assertThat( @@ -128,15 +163,26 @@ public void testRelocateShard() { var fromNodeId = "node-0"; var toNodeId = "node-1"; - var shard = newShardRouting("index-1", 0, toNodeId, fromNodeId, true, INITIALIZING); - - var simulator = new ClusterInfoSimulator( - new ClusterInfoTestBuilder() // - .withNode(fromNodeId, new DiskUsageBuilder(1000, 900)) - .withNode(toNodeId, new DiskUsageBuilder(1000, 1000)) - .withShard(shard, 100) - .build() + var shard = newShardRouting( + new ShardId("my-index", "_na_", 0), + toNodeId, + fromNodeId, + true, + INITIALIZING, + RecoverySource.PeerRecoverySource.INSTANCE ); + + var initialClusterInfo = new ClusterInfoTestBuilder() // + .withNode(fromNodeId, new DiskUsageBuilder(1000, 900)) + .withNode(toNodeId, new DiskUsageBuilder(1000, 1000)) + .withShard(shard, 100) + .build(); + + var state = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().put(IndexMetadata.builder("my-index").settings(indexSettings(IndexVersion.current(), 1, 0)))) + .build(); + var allocation = createRoutingAllocation(state, initialClusterInfo, SnapshotShardSizeInfo.EMPTY); + var simulator = new ClusterInfoSimulator(allocation); simulator.simulateShardStarted(shard); assertThat( @@ -151,20 +197,31 @@ public void testRelocateShard() { ); } - public void testRelocateShardWithMultipleDataPath1() { + public void testRelocateShardWithMultipleDataPath() { var fromNodeId = "node-0"; var toNodeId = "node-1"; - var shard = newShardRouting("index-1", 0, toNodeId, fromNodeId, true, INITIALIZING); - - var simulator = new ClusterInfoSimulator( - new ClusterInfoTestBuilder() // - .withNode(fromNodeId, new DiskUsageBuilder("/data-1", 1000, 500), new DiskUsageBuilder("/data-2", 1000, 750)) - .withNode(toNodeId, new DiskUsageBuilder("/data-1", 1000, 750), new DiskUsageBuilder("/data-2", 1000, 900)) - .withShard(shard, 100) - .build() + var shard = newShardRouting( + new ShardId("my-index", "_na_", 0), + toNodeId, + fromNodeId, + true, + INITIALIZING, + RecoverySource.PeerRecoverySource.INSTANCE ); + + var initialClusterInfo = new ClusterInfoTestBuilder() // + .withNode(fromNodeId, new DiskUsageBuilder("/data-1", 1000, 500), new DiskUsageBuilder("/data-2", 1000, 750)) + .withNode(toNodeId, new DiskUsageBuilder("/data-1", 1000, 750), new DiskUsageBuilder("/data-2", 1000, 900)) + .withShard(shard, 100) + .build(); + + var state = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().put(IndexMetadata.builder("my-index").settings(indexSettings(IndexVersion.current(), 1, 0)))) + .build(); + var allocation = createRoutingAllocation(state, initialClusterInfo, SnapshotShardSizeInfo.EMPTY); + var simulator = new ClusterInfoSimulator(allocation); simulator.simulateShardStarted(shard); assertThat( @@ -179,12 +236,201 @@ public void testRelocateShardWithMultipleDataPath1() { ); } + public void testInitializeShardFromSnapshot() { + + var shardSize = 100; + var indexSettings = indexSettings(IndexVersion.current(), 1, 0); + if (randomBoolean()) { + indexSettings.put(INDEX_STORE_TYPE_SETTING.getKey(), SEARCHABLE_SNAPSHOT_STORE_TYPE); + } + + var state = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().put(IndexMetadata.builder("my-index").settings(indexSettings))) + .build(); + + var snapshot = new Snapshot("repository", new SnapshotId("snapshot-1", "na")); + var indexId = new IndexId("my-index", "_na_"); + var shard = newShardRouting( + new ShardId(state.metadata().index("my-index").getIndex(), 0), + "node-0", + true, + ShardRoutingState.INITIALIZING, + new RecoverySource.SnapshotRecoverySource(randomUUID(), snapshot, IndexVersion.current(), indexId) + ); + + var initialClusterInfo = new ClusterInfoTestBuilder() // + .withNode("node-0", new DiskUsageBuilder(1000, 1000)) + .withNode("node-1", new DiskUsageBuilder(1000, 1000)) + .build(); + var snapshotShardSizeInfo = new SnapshotShardSizeInfoTestBuilder() // + .withShard(snapshot, indexId, shard.shardId(), shardSize) + .build(); + + var allocation = createRoutingAllocation(state, initialClusterInfo, snapshotShardSizeInfo); + var simulator = new ClusterInfoSimulator(allocation); + simulator.simulateShardStarted(shard); + + assertThat( + simulator.getClusterInfo(), + equalTo( + new ClusterInfoTestBuilder() // + .withNode("node-0", new DiskUsageBuilder(1000, 1000 - shardSize)) + .withNode("node-1", new DiskUsageBuilder(1000, 1000)) + .withShard(shard, shardSize) + .build() + ) + ); + } + + public void testInitializeShardFromPartialSearchableSnapshot() { + + var shardSize = 100; + var indexSettings = indexSettings(IndexVersion.current(), 1, 0) // + .put(INDEX_STORE_TYPE_SETTING.getKey(), SEARCHABLE_SNAPSHOT_STORE_TYPE) + .put(SNAPSHOT_PARTIAL_SETTING.getKey(), true) + .put(SETTING_IGNORE_DISK_WATERMARKS.getKey(), true); + + var state = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().put(IndexMetadata.builder("my-index").settings(indexSettings))) + .build(); + + var snapshot = new Snapshot("repository", new SnapshotId("snapshot-1", "na")); + var indexId = new IndexId("my-index", "_na_"); + var shard = newShardRouting( + new ShardId(state.metadata().index("my-index").getIndex(), 0), + "node-0", + true, + ShardRoutingState.INITIALIZING, + new RecoverySource.SnapshotRecoverySource(randomUUID(), snapshot, IndexVersion.current(), indexId) + ); + + var initialClusterInfo = new ClusterInfoTestBuilder() // + .withNode("node-0", new DiskUsageBuilder(1000, 1000)) + .withNode("node-1", new DiskUsageBuilder(1000, 1000)) + .build(); + var snapshotShardSizeInfo = new SnapshotShardSizeInfoTestBuilder() // + .withShard(snapshot, indexId, shard.shardId(), shardSize) + .build(); + + var allocation = createRoutingAllocation(state, initialClusterInfo, snapshotShardSizeInfo); + var simulator = new ClusterInfoSimulator(allocation); + simulator.simulateShardStarted(shard); + + assertThat( + simulator.getClusterInfo(), + equalTo( + new ClusterInfoTestBuilder() // + .withNode("node-0", new DiskUsageBuilder(1000, 1000)) + .withNode("node-1", new DiskUsageBuilder(1000, 1000)) + .withShard(shard, 0) // partial searchable snapshot always reports 0 size + .build() + ) + ); + } + + public void testRelocatePartialSearchableSnapshotShard() { + + var shardSize = 100; + var indexSettings = indexSettings(IndexVersion.current(), 1, 0) // + .put(INDEX_STORE_TYPE_SETTING.getKey(), SEARCHABLE_SNAPSHOT_STORE_TYPE) + .put(SNAPSHOT_PARTIAL_SETTING.getKey(), true) + .put(SETTING_IGNORE_DISK_WATERMARKS.getKey(), true); + + var state = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().put(IndexMetadata.builder("my-index").settings(indexSettings))) + .build(); + + var snapshot = new Snapshot("repository", new SnapshotId("snapshot-1", "na")); + var indexId = new IndexId("my-index", "_na_"); + + var fromNodeId = "node-0"; + var toNodeId = "node-1"; + + var shard = newShardRouting( + new ShardId("my-index", "_na_", 0), + toNodeId, + fromNodeId, + true, + INITIALIZING, + RecoverySource.PeerRecoverySource.INSTANCE + ); + + var initialClusterInfo = new ClusterInfoTestBuilder() // + .withNode(fromNodeId, new DiskUsageBuilder(1000, 1000)) + .withNode(toNodeId, new DiskUsageBuilder(1000, 1000)) + .withShard(shard, 0) + .build(); + var snapshotShardSizeInfo = new SnapshotShardSizeInfoTestBuilder() // + .withShard(snapshot, indexId, shard.shardId(), shardSize) + .build(); + + var allocation = createRoutingAllocation(state, initialClusterInfo, snapshotShardSizeInfo); + var simulator = new ClusterInfoSimulator(allocation); + simulator.simulateShardStarted(shard); + + assertThat( + simulator.getClusterInfo(), + equalTo( + new ClusterInfoTestBuilder() // + .withNode(fromNodeId, new DiskUsageBuilder(1000, 1000)) + .withNode(toNodeId, new DiskUsageBuilder(1000, 1000)) + .withShard(shard, 0) // partial searchable snapshot always reports 0 size + .build() + ) + ); + } + + public void testInitializeShardFromClone() { + + var sourceShardSize = randomLongBetween(100, 1000); + var source = newShardRouting(new ShardId("source", "_na_", 0), randomIdentifier(), true, ShardRoutingState.STARTED); + var target = newShardRouting( + new ShardId("target", "_na_", 0), + randomIdentifier(), + true, + ShardRoutingState.INITIALIZING, + RecoverySource.LocalShardsRecoverySource.INSTANCE + ); + + var state = ClusterState.builder(ClusterName.DEFAULT) + .metadata( + Metadata.builder() + .put(IndexMetadata.builder("source").settings(indexSettings(IndexVersion.current(), 1, 0))) + .put( + IndexMetadata.builder("target") + .settings( + indexSettings(IndexVersion.current(), 1, 0) // + .put(INDEX_RESIZE_SOURCE_NAME_KEY, "source") // + .put(INDEX_RESIZE_SOURCE_UUID_KEY, "_na_") + ) + ) + ) + .routingTable(RoutingTable.builder().add(IndexRoutingTable.builder(source.index()).addShard(source))) + .build(); + + var initialClusterInfo = new ClusterInfoTestBuilder().withNode("node-0", new DiskUsageBuilder(1000, 1000 - sourceShardSize)) + .withShard(source, sourceShardSize) + .build(); + + var allocation = createRoutingAllocation(state, initialClusterInfo, SnapshotShardSizeInfo.EMPTY); + var simulator = new ClusterInfoSimulator(allocation); + simulator.simulateShardStarted(target); + + assertThat( + simulator.getClusterInfo(), + equalTo( + new ClusterInfoTestBuilder() // + .withNode("node-0", new DiskUsageBuilder(1000, 1000 - sourceShardSize)) + .withShard(source, sourceShardSize) + .withShard(target, sourceShardSize) + .build() + ) + ); + } + public void testDiskUsageSimulationWithSingleDataPathAndDiskThresholdDecider() { - var discoveryNodesBuilder = DiscoveryNodes.builder() - .add(createDiscoveryNode("node-0", DiscoveryNodeRole.roles())) - .add(createDiscoveryNode("node-1", DiscoveryNodeRole.roles())) - .add(createDiscoveryNode("node-2", DiscoveryNodeRole.roles())); + var discoveryNodesBuilder = DiscoveryNodes.builder().add(newNode("node-0")).add(newNode("node-1")).add(newNode("node-2")); var metadataBuilder = Metadata.builder(); var routingTableBuilder = RoutingTable.builder(); @@ -192,28 +438,35 @@ public void testDiskUsageSimulationWithSingleDataPathAndDiskThresholdDecider() { var shard1 = newShardRouting("index-1", 0, "node-0", null, true, STARTED); addIndex(metadataBuilder, routingTableBuilder, shard1); - var shard2 = newShardRouting("index-2", 0, "node-0", "node-1", true, INITIALIZING); + var shard2 = newShardRouting( + new ShardId("index-2", "_na_", 0), + "node-0", + "node-1", + true, + INITIALIZING, + RecoverySource.PeerRecoverySource.INSTANCE + ); addIndex(metadataBuilder, routingTableBuilder, shard2); var shard3 = newShardRouting("index-3", 0, "node-1", null, true, STARTED); addIndex(metadataBuilder, routingTableBuilder, shard3); - var clusterState = ClusterState.builder(ClusterName.DEFAULT) + var state = ClusterState.builder(ClusterName.DEFAULT) .nodes(discoveryNodesBuilder) .metadata(metadataBuilder) .routingTable(routingTableBuilder) .build(); - var simulator = new ClusterInfoSimulator( - new ClusterInfoTestBuilder() // - .withNode("node-0", new DiskUsageBuilder("/data-1", 1000, 500)) - .withNode("node-1", new DiskUsageBuilder("/data-1", 1000, 300)) - .withShard(shard1, 500) - .withShard(shard2, 400) - .withShard(shard3, 300) - .build() - ); + var initialClusterInfo = new ClusterInfoTestBuilder() // + .withNode("node-0", new DiskUsageBuilder("/data-1", 1000, 500)) + .withNode("node-1", new DiskUsageBuilder("/data-1", 1000, 300)) + .withShard(shard1, 500) + .withShard(shard2, 400) + .withShard(shard3, 300) + .build(); + var allocation = createRoutingAllocation(state, initialClusterInfo, SnapshotShardSizeInfo.EMPTY); + var simulator = new ClusterInfoSimulator(allocation); simulator.simulateShardStarted(shard2); assertThat( @@ -229,42 +482,29 @@ public void testDiskUsageSimulationWithSingleDataPathAndDiskThresholdDecider() { ) ); - var decider = new DiskThresholdDecider( - Settings.EMPTY, - new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) - ); - var allocation = new RoutingAllocation( - new AllocationDeciders(List.of(decider)), - clusterState, - simulator.getClusterInfo(), - SnapshotShardSizeInfo.EMPTY, - 0L - ); - var routingNodes = allocation.routingNodes(); + var decider = new DiskThresholdDecider(Settings.EMPTY, ClusterSettings.createBuiltInClusterSettings(Settings.EMPTY)); + allocation = createRoutingAllocation(state, simulator.getClusterInfo(), SnapshotShardSizeInfo.EMPTY, decider); assertThat( "Should keep index-1 on node-0", - decider.canRemain(clusterState.metadata().index("index-1"), shard1, routingNodes.node("node-0"), allocation).type(), + decider.canRemain(state.metadata().index("index-1"), shard1, allocation.routingNodes().node("node-0"), allocation).type(), equalTo(Decision.Type.YES) ); assertThat( "Should keep index-2 on node-0", - decider.canRemain(clusterState.metadata().index("index-2"), shard2, routingNodes.node("node-0"), allocation).type(), + decider.canRemain(state.metadata().index("index-2"), shard2, allocation.routingNodes().node("node-0"), allocation).type(), equalTo(Decision.Type.YES) ); assertThat( "Should not allocate index-3 on node-0 (not enough space)", - decider.canAllocate(shard3, routingNodes.node("node-0"), allocation).type(), + decider.canAllocate(shard3, allocation.routingNodes().node("node-0"), allocation).type(), equalTo(Decision.Type.NO) ); } public void testDiskUsageSimulationWithMultipleDataPathAndDiskThresholdDecider() { - var discoveryNodesBuilder = DiscoveryNodes.builder() - .add(createDiscoveryNode("node-0", DiscoveryNodeRole.roles())) - .add(createDiscoveryNode("node-1", DiscoveryNodeRole.roles())) - .add(createDiscoveryNode("node-2", DiscoveryNodeRole.roles())); + var discoveryNodesBuilder = DiscoveryNodes.builder().add(newNode("node-0")).add(newNode("node-1")).add(newNode("node-2")); var metadataBuilder = Metadata.builder(); var routingTableBuilder = RoutingTable.builder(); @@ -272,28 +512,35 @@ public void testDiskUsageSimulationWithMultipleDataPathAndDiskThresholdDecider() var shard1 = newShardRouting("index-1", 0, "node-0", null, true, STARTED); addIndex(metadataBuilder, routingTableBuilder, shard1); - var shard2 = newShardRouting("index-2", 0, "node-0", "node-1", true, INITIALIZING); + var shard2 = newShardRouting( + new ShardId("index-2", "_na_", 0), + "node-0", + "node-1", + true, + INITIALIZING, + RecoverySource.PeerRecoverySource.INSTANCE + ); addIndex(metadataBuilder, routingTableBuilder, shard2); var shard3 = newShardRouting("index-3", 0, "node-1", null, true, STARTED); addIndex(metadataBuilder, routingTableBuilder, shard3); - var clusterState = ClusterState.builder(ClusterName.DEFAULT) + var state = ClusterState.builder(ClusterName.DEFAULT) .nodes(discoveryNodesBuilder) .metadata(metadataBuilder) .routingTable(routingTableBuilder) .build(); - var simulator = new ClusterInfoSimulator( - new ClusterInfoTestBuilder() // - .withNode("node-0", new DiskUsageBuilder("/data-1", 1000, 100), new DiskUsageBuilder("/data-2", 1000, 500)) - .withNode("node-1", new DiskUsageBuilder("/data-1", 1000, 100), new DiskUsageBuilder("/data-2", 1000, 300)) - .withShard(shard1, 500) - .withShard(shard2, 400) - .withShard(shard3, 300) - .build() - ); + var initialClusterInfo = new ClusterInfoTestBuilder() // + .withNode("node-0", new DiskUsageBuilder("/data-1", 1000, 100), new DiskUsageBuilder("/data-2", 1000, 500)) + .withNode("node-1", new DiskUsageBuilder("/data-1", 1000, 100), new DiskUsageBuilder("/data-2", 1000, 300)) + .withShard(shard1, 500) + .withShard(shard2, 400) + .withShard(shard3, 300) + .build(); + var allocation = createRoutingAllocation(state, initialClusterInfo, SnapshotShardSizeInfo.EMPTY); + var simulator = new ClusterInfoSimulator(allocation); simulator.simulateShardStarted(shard2); assertThat( @@ -309,53 +556,63 @@ public void testDiskUsageSimulationWithMultipleDataPathAndDiskThresholdDecider() ) ); - var decider = new DiskThresholdDecider( - Settings.EMPTY, - new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) - ); - var allocation = new RoutingAllocation( - new AllocationDeciders(List.of(decider)), - clusterState, - simulator.getClusterInfo(), - SnapshotShardSizeInfo.EMPTY, - 0L - ); - var routingNodes = allocation.routingNodes(); + var decider = new DiskThresholdDecider(Settings.EMPTY, ClusterSettings.createBuiltInClusterSettings(Settings.EMPTY)); + allocation = createRoutingAllocation(state, simulator.getClusterInfo(), SnapshotShardSizeInfo.EMPTY, decider); assertThat( "Should keep index-1 on node-0", - decider.canRemain(clusterState.metadata().index("index-1"), shard1, routingNodes.node("node-0"), allocation).type(), + decider.canRemain(state.metadata().index("index-1"), shard1, allocation.routingNodes().node("node-0"), allocation).type(), equalTo(Decision.Type.YES) ); assertThat( "Should keep index-2 on node-0", - decider.canRemain(clusterState.metadata().index("index-2"), shard2, routingNodes.node("node-0"), allocation).type(), + decider.canRemain(state.metadata().index("index-2"), shard2, allocation.routingNodes().node("node-0"), allocation).type(), equalTo(Decision.Type.YES) ); assertThat( "Should not allocate index-3 on node-0 (not enough space)", - decider.canAllocate(shard3, routingNodes.node("node-0"), allocation).type(), + decider.canAllocate(shard3, allocation.routingNodes().node("node-0"), allocation).type(), equalTo(Decision.Type.NO) ); } - private static DiscoveryNode createDiscoveryNode(String id, Set roles) { - return DiscoveryNodeUtils.builder(id).name(id).externalId(UUIDs.randomBase64UUID(random())).roles(roles).build(); - } - private static void addIndex(Metadata.Builder metadataBuilder, RoutingTable.Builder routingTableBuilder, ShardRouting shardRouting) { var name = shardRouting.getIndexName(); metadataBuilder.put(IndexMetadata.builder(name).settings(indexSettings(IndexVersion.current(), 1, 0))); routingTableBuilder.add(IndexRoutingTable.builder(metadataBuilder.get(name).getIndex()).addShard(shardRouting)); } + private static RoutingAllocation createRoutingAllocation( + ClusterState state, + ClusterInfo clusterInfo, + SnapshotShardSizeInfo snapshotShardSizeInfo, + AllocationDecider... deciders + ) { + return new RoutingAllocation(new AllocationDeciders(List.of(deciders)), state, clusterInfo, snapshotShardSizeInfo, 0); + } + + private static class SnapshotShardSizeInfoTestBuilder { + + private final Map snapshotShardSizes = new HashMap<>(); + + public SnapshotShardSizeInfoTestBuilder withShard(Snapshot snapshot, IndexId indexId, ShardId shardId, long size) { + snapshotShardSizes.put(new InternalSnapshotsInfoService.SnapshotShard(snapshot, indexId, shardId), size); + return this; + } + + public SnapshotShardSizeInfo build() { + return new SnapshotShardSizeInfo(snapshotShardSizes); + } + } + private static class ClusterInfoTestBuilder { private final Map leastAvailableSpaceUsage = new HashMap<>(); private final Map mostAvailableSpaceUsage = new HashMap<>(); private final Map shardSizes = new HashMap<>(); + private final Map reservedSpace = new HashMap<>(); public ClusterInfoTestBuilder withNode(String name, DiskUsageBuilder diskUsageBuilderBuilder) { leastAvailableSpaceUsage.put(name, diskUsageBuilderBuilder.toDiskUsage(name)); @@ -370,12 +627,12 @@ public ClusterInfoTestBuilder withNode(String name, DiskUsageBuilder leastAvaila } public ClusterInfoTestBuilder withShard(ShardRouting shard, long size) { - shardSizes.put(ClusterInfo.shardIdentifierFromRouting(shard), size); + shardSizes.put(shardIdentifierFromRouting(shard), size); return this; } public ClusterInfo build() { - return new ClusterInfo(leastAvailableSpaceUsage, mostAvailableSpaceUsage, shardSizes, Map.of(), Map.of(), Map.of()); + return new ClusterInfo(leastAvailableSpaceUsage, mostAvailableSpaceUsage, shardSizes, Map.of(), Map.of(), reservedSpace); } } @@ -385,8 +642,8 @@ private DiskUsageBuilder(long total, long free) { this("/data", total, free); } - public DiskUsage toDiskUsage(String name) { - return new DiskUsage(name, name, name + path, total, free); + public DiskUsage toDiskUsage(String nodeId) { + return new DiskUsage(nodeId, nodeId, nodeId + path, total, free); } } } diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java index 5e3b6cd02f830..0396024156cb7 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputerTests.java @@ -10,8 +10,9 @@ import org.apache.logging.log4j.Level; import org.elasticsearch.cluster.ClusterInfo; +import org.elasticsearch.cluster.ClusterInfo.NodeAndPath; import org.elasticsearch.cluster.ClusterInfo.NodeAndShard; -import org.elasticsearch.cluster.ClusterModule; +import org.elasticsearch.cluster.ClusterInfo.ReservedSpace; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.DiskUsage; @@ -39,13 +40,13 @@ import org.elasticsearch.cluster.routing.allocation.decider.ThrottlingAllocationDecider; import org.elasticsearch.common.Randomness; import org.elasticsearch.common.UUIDs; -import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.util.Maps; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.repositories.IndexId; +import org.elasticsearch.snapshots.InternalSnapshotsInfoService; import org.elasticsearch.snapshots.InternalSnapshotsInfoService.SnapshotShard; import org.elasticsearch.snapshots.Snapshot; import org.elasticsearch.snapshots.SnapshotId; @@ -82,6 +83,7 @@ import static org.hamcrest.Matchers.everyItem; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.Matchers.notNullValue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -804,29 +806,25 @@ public void testComputeConsideringShardSizes() { var node0RemainingBytes = (index0ReplicaShard.started() || index0ReplicaShard.relocating()) && Objects.equals(index0ReplicaShard.currentNodeId(), "node-0") ? 100 : 600; - var node0Usage = new DiskUsage("node-0", "node-0", "/data", 1000, node0RemainingBytes); - var node1Usage = new DiskUsage("node-1", "node-1", "/data", 1000, 100); - var node2Usage = new DiskUsage("node-2", "node-2", "/data", 1000, 1000); - var clusterInfo = createClusterInfo( - List.of(node0Usage, node1Usage, node2Usage), - Map.ofEntries( - // node-0 & node-1 - indexSize(clusterState, "index-0", 500, true), - indexSize(clusterState, "index-0", 500, false), - // node-0 - indexSize(clusterState, "index-1", 400, true), - // node-1 - indexSize(clusterState, "index-2", 50, true), - indexSize(clusterState, "index-3", 50, true), - indexSize(clusterState, "index-4", 50, true), - indexSize(clusterState, "index-5", 50, true), - indexSize(clusterState, "index-6", 50, true), - indexSize(clusterState, "index-7", 50, true), - indexSize(clusterState, "index-8", 50, true), - indexSize(clusterState, "index-9", 50, true) - ) - ); + var clusterInfo = new ClusterInfoTestBuilder().withNode("node-0", 1000, node0RemainingBytes) + .withNode("node-1", 1000, 100) + .withNode("node-2", 1000, 1000) + // node-0 & node-1 + .withShard(findShardId(clusterState, "index-0"), true, 500) + .withShard(findShardId(clusterState, "index-0"), false, 500) + // node-0 + .withShard(findShardId(clusterState, "index-1"), true, 400) + // node-1 + .withShard(findShardId(clusterState, "index-2"), true, 50) + .withShard(findShardId(clusterState, "index-3"), true, 50) + .withShard(findShardId(clusterState, "index-4"), true, 50) + .withShard(findShardId(clusterState, "index-5"), true, 50) + .withShard(findShardId(clusterState, "index-6"), true, 50) + .withShard(findShardId(clusterState, "index-7"), true, 50) + .withShard(findShardId(clusterState, "index-8"), true, 50) + .withShard(findShardId(clusterState, "index-9"), true, 50) + .build(); var settings = Settings.builder() // force as many iterations as possible to accumulate the diff @@ -839,11 +837,9 @@ public void testComputeConsideringShardSizes() { var initial = new DesiredBalance( 1, - Map.of( - findShardId(clusterState, "index-0"), - new ShardAssignment(Set.of("node-0", "node-1"), 2, 0, 0), - findShardId(clusterState, "index-1"), - new ShardAssignment(Set.of("node-0"), 1, 0, 0) + Map.ofEntries( + Map.entry(findShardId(clusterState, "index-0"), new ShardAssignment(Set.of("node-0", "node-1"), 2, 0, 0)), + Map.entry(findShardId(clusterState, "index-1"), new ShardAssignment(Set.of("node-0"), 1, 0, 0)) ) ); @@ -865,16 +861,183 @@ public void testComputeConsideringShardSizes() { assertThat(resultDiskUsage, allOf(aMapWithSize(2), hasEntry("node-0", 950L), hasEntry("node-1", 850L))); } - private static ClusterInfo createClusterInfo(List diskUsages, Map shardSizes) { - var diskUsage = diskUsages.stream().collect(toMap(DiskUsage::getNodeId, Function.identity())); - return new ClusterInfo(diskUsage, diskUsage, shardSizes, Map.of(), Map.of(), Map.of()); + @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/103907") + public void testAccountForSizeOfMisplacedShardsDuringNewComputation() { + + var snapshot = new Snapshot("repository", new SnapshotId("snapshot", randomUUID())); + + var clusterInfoBuilder = new ClusterInfoTestBuilder().withNode( + "node-1", + ByteSizeValue.ofGb(10).getBytes(), + ByteSizeValue.ofGb(2).getBytes() + ).withNode("node-2", ByteSizeValue.ofGb(10).getBytes(), ByteSizeValue.ofGb(2).getBytes()); + var snapshotShardSizes = Maps.newHashMapWithExpectedSize(5); + + var routingTableBuilder = RoutingTable.builder(TestShardRoutingRoleStrategies.DEFAULT_ROLE_ONLY); + // index-1 is allocated according to the desired balance + var indexMetadata1 = IndexMetadata.builder("index-1").settings(indexSettings(IndexVersion.current(), 2, 0)).build(); + routingTableBuilder.add( + IndexRoutingTable.builder(indexMetadata1.getIndex()) + .addShard(newShardRouting(shardIdFrom(indexMetadata1, 0), "node-1", true, STARTED)) + .addShard(newShardRouting(shardIdFrom(indexMetadata1, 1), "node-2", true, STARTED)) + ); + clusterInfoBuilder.withShard(shardIdFrom(indexMetadata1, 0), true, ByteSizeValue.ofGb(8).getBytes()) + .withShard(shardIdFrom(indexMetadata1, 1), true, ByteSizeValue.ofGb(8).getBytes()); + + // index-2 is restored earlier but is not started on the desired node yet + var indexMetadata2 = IndexMetadata.builder("index-2").settings(indexSettings(IndexVersion.current(), 1, 0)).build(); + snapshotShardSizes.put( + new SnapshotShard(snapshot, indexIdFrom(indexMetadata2), shardIdFrom(indexMetadata2, 0)), + ByteSizeValue.ofGb(1).getBytes() + ); + var index2SnapshotRecoverySource = new RecoverySource.SnapshotRecoverySource( + "restore", + snapshot, + IndexVersion.current(), + indexIdFrom(indexMetadata2) + ); + switch (randomInt(3)) { + // index is still unassigned + case 0 -> routingTableBuilder.addAsNewRestore(indexMetadata2, index2SnapshotRecoverySource, Set.of()); + // index is initializing on desired node + case 1 -> { + ShardId index2ShardId = shardIdFrom(indexMetadata2, 0); + routingTableBuilder.add( + IndexRoutingTable.builder(indexMetadata2.getIndex()) + .addShard(newShardRouting(index2ShardId, "node-1", true, INITIALIZING, index2SnapshotRecoverySource)) + ); + if (randomBoolean()) { + // Shard is 75% downloaded + clusterInfoBuilder // + .withNodeUsedSpace("node-1", ByteSizeValue.ofMb(768).getBytes()) + .withReservedSpace("node-1", ByteSizeValue.ofMb(256).getBytes(), index2ShardId); + } + } + // index is initializing on undesired node + case 2 -> { + ShardId index2ShardId = shardIdFrom(indexMetadata2, 0); + routingTableBuilder.add( + IndexRoutingTable.builder(indexMetadata2.getIndex()) + .addShard(newShardRouting(index2ShardId, "node-2", true, INITIALIZING, index2SnapshotRecoverySource)) + ); + if (randomBoolean()) { + // Shard is 75% downloaded + clusterInfoBuilder // + .withNodeUsedSpace("node-2", ByteSizeValue.ofMb(768).getBytes()) + .withReservedSpace("node-2", ByteSizeValue.ofMb(256).getBytes(), index2ShardId); + } + } + // index is started on undesired node + case 3 -> { + routingTableBuilder.add( + IndexRoutingTable.builder(indexMetadata2.getIndex()) + .addShard(newShardRouting(shardIdFrom(indexMetadata2, 0), "node-2", true, STARTED)) + ); + clusterInfoBuilder.withNodeUsedSpace("node-2", ByteSizeValue.ofGb(1).getBytes()) + .withShard(shardIdFrom(indexMetadata2, 0), true, ByteSizeValue.ofGb(1).getBytes()); + } + default -> throw new AssertionError("unexpected randomization"); + } + + // index-3 is restored as new from snapshot + var indexMetadata3 = IndexMetadata.builder("index-3").settings(indexSettings(IndexVersion.current(), 2, 0)).build(); + routingTableBuilder.addAsNewRestore( + indexMetadata3, + new RecoverySource.SnapshotRecoverySource("restore", snapshot, IndexVersion.current(), indexIdFrom(indexMetadata3)), + Set.of() + ); + snapshotShardSizes.put( + new SnapshotShard(snapshot, indexIdFrom(indexMetadata3), shardIdFrom(indexMetadata3, 0)), + ByteSizeValue.ofMb(512).getBytes() + ); + snapshotShardSizes.put( + new SnapshotShard(snapshot, indexIdFrom(indexMetadata3), shardIdFrom(indexMetadata3, 1)), + ByteSizeValue.ofMb(512).getBytes() + ); + + var clusterState = ClusterState.builder(ClusterName.DEFAULT) + .nodes(DiscoveryNodes.builder().add(newNode("node-1")).add(newNode("node-2"))) + .metadata(Metadata.builder().put(indexMetadata1, false).put(indexMetadata2, false).put(indexMetadata3, false).build()) + .routingTable(routingTableBuilder) + .customs( + Map.of( + RestoreInProgress.TYPE, + new RestoreInProgress.Builder().add( + new RestoreInProgress.Entry( + "restore", + snapshot, + RestoreInProgress.State.STARTED, + randomBoolean(), + List.of(indexMetadata2.getIndex().getName(), indexMetadata3.getIndex().getName()), + Map.ofEntries( + Map.entry(shardIdFrom(indexMetadata2, 0), new RestoreInProgress.ShardRestoreStatus(randomUUID())), + Map.entry(shardIdFrom(indexMetadata3, 0), new RestoreInProgress.ShardRestoreStatus(randomUUID())), + Map.entry(shardIdFrom(indexMetadata3, 1), new RestoreInProgress.ShardRestoreStatus(randomUUID())) + ) + ) + ).build() + ) + ) + .build(); + + var settings = Settings.EMPTY; + var allocation = new RoutingAllocation( + randomAllocationDeciders(settings, createBuiltInClusterSettings(settings)), + clusterState, + clusterInfoBuilder.build(), + new SnapshotShardSizeInfo(snapshotShardSizes), + 0L + ); + var initialDesiredBalance = new DesiredBalance( + 1, + Map.ofEntries( + Map.entry(shardIdFrom(indexMetadata1, 0), new ShardAssignment(Set.of("node-1"), 1, 0, 0)), + Map.entry(shardIdFrom(indexMetadata1, 1), new ShardAssignment(Set.of("node-2"), 1, 0, 0)), + Map.entry(shardIdFrom(indexMetadata2, 0), new ShardAssignment(Set.of("node-1"), 1, 0, 0)) + ) + ); + var nextDesiredBalance = createDesiredBalanceComputer(new BalancedShardsAllocator()).compute( + initialDesiredBalance, + new DesiredBalanceInput(2, allocation, List.of()), + queue(), + input -> true + ); + + // both node-1 and node-2 has enough space to allocate either only [index-2] shard or both [index-3] shards + assertThat( + nextDesiredBalance.assignments(), + anyOf( + equalTo( + Map.ofEntries( + Map.entry(shardIdFrom(indexMetadata1, 0), new ShardAssignment(Set.of("node-1"), 1, 0, 0)), + Map.entry(shardIdFrom(indexMetadata1, 1), new ShardAssignment(Set.of("node-2"), 1, 0, 0)), + Map.entry(shardIdFrom(indexMetadata2, 0), new ShardAssignment(Set.of("node-1"), 1, 0, 0)), + Map.entry(shardIdFrom(indexMetadata3, 0), new ShardAssignment(Set.of("node-2"), 1, 0, 0)), + Map.entry(shardIdFrom(indexMetadata3, 1), new ShardAssignment(Set.of("node-2"), 1, 0, 0)) + ) + ), + equalTo( + Map.ofEntries( + Map.entry(shardIdFrom(indexMetadata1, 0), new ShardAssignment(Set.of("node-1"), 1, 0, 0)), + Map.entry(shardIdFrom(indexMetadata1, 1), new ShardAssignment(Set.of("node-2"), 1, 0, 0)), + Map.entry(shardIdFrom(indexMetadata2, 0), new ShardAssignment(Set.of("node-2"), 1, 0, 0)), + Map.entry(shardIdFrom(indexMetadata3, 0), new ShardAssignment(Set.of("node-1"), 1, 0, 0)), + Map.entry(shardIdFrom(indexMetadata3, 1), new ShardAssignment(Set.of("node-1"), 1, 0, 0)) + ) + ) + ) + ); } public void testAccountForSizeOfAllInitializingShardsDuringAllocation() { var snapshot = new Snapshot("repository", new SnapshotId("snapshot", randomUUID())); - var shardSizeInfo = Maps.newHashMapWithExpectedSize(5); + var clusterInfoBuilder = new ClusterInfoTestBuilder().withNode( + "node-1", + ByteSizeValue.ofGb(10).getBytes(), + ByteSizeValue.ofGb(2).getBytes() + ).withNode("node-2", ByteSizeValue.ofGb(10).getBytes(), ByteSizeValue.ofGb(2).getBytes()); var snapshotShardSizes = Maps.newHashMapWithExpectedSize(5); var routingTableBuilder = RoutingTable.builder(TestShardRoutingRoleStrategies.DEFAULT_ROLE_ONLY); @@ -885,8 +1048,8 @@ public void testAccountForSizeOfAllInitializingShardsDuringAllocation() { .addShard(newShardRouting(shardIdFrom(indexMetadata1, 0), "node-1", true, STARTED)) .addShard(newShardRouting(shardIdFrom(indexMetadata1, 1), "node-2", true, STARTED)) ); - shardSizeInfo.put(shardIdentifierFromRouting(shardIdFrom(indexMetadata1, 0), true), ByteSizeValue.ofGb(8).getBytes()); - shardSizeInfo.put(shardIdentifierFromRouting(shardIdFrom(indexMetadata1, 1), true), ByteSizeValue.ofGb(8).getBytes()); + clusterInfoBuilder.withShard(shardIdFrom(indexMetadata1, 0), true, ByteSizeValue.ofGb(8).getBytes()) + .withShard(shardIdFrom(indexMetadata1, 1), true, ByteSizeValue.ofGb(8).getBytes()); // index-2 & index-3 are restored as new from snapshot var indexMetadata2 = IndexMetadata.builder("index-2") @@ -944,23 +1107,12 @@ public void testAccountForSizeOfAllInitializingShardsDuringAllocation() { ) .build(); - var clusterInfo = createClusterInfo( - List.of( - // node-1 has enough space to only allocate the only [index-2] shard - new DiskUsage("node-1", "data-1", "/data", ByteSizeValue.ofGb(10).getBytes(), ByteSizeValue.ofGb(2).getBytes()), - // node-2 has enough space to only allocate both shards of [index-3] - new DiskUsage("node-2", "data-2", "/data", ByteSizeValue.ofGb(10).getBytes(), ByteSizeValue.ofGb(2).getBytes()) - ), - shardSizeInfo - ); - var snapshotShardSizeInfo = new SnapshotShardSizeInfo(snapshotShardSizes); - var settings = Settings.EMPTY; var allocation = new RoutingAllocation( randomAllocationDeciders(settings, createBuiltInClusterSettings(settings)), clusterState, - clusterInfo, - snapshotShardSizeInfo, + clusterInfoBuilder.build(), + new SnapshotShardSizeInfo(snapshotShardSizes), 0L ); var initialDesiredBalance = new DesiredBalance( @@ -977,6 +1129,7 @@ public void testAccountForSizeOfAllInitializingShardsDuringAllocation() { input -> true ); + // both node-1 and node-2 has enough space to allocate either only [index-2] shard or both [index-3] shards assertThat( nextDesiredBalance.assignments(), anyOf( @@ -1002,6 +1155,46 @@ public void testAccountForSizeOfAllInitializingShardsDuringAllocation() { ); } + @Deprecated + private static ClusterInfo createClusterInfo(List diskUsages, Map shardSizes) { + var diskUsage = diskUsages.stream().collect(toMap(DiskUsage::getNodeId, Function.identity())); + return new ClusterInfo(diskUsage, diskUsage, shardSizes, Map.of(), Map.of(), Map.of()); + } + + private static class ClusterInfoTestBuilder { + + private final Map diskUsage = new HashMap<>(); + private final Map shardSizes = new HashMap<>(); + private final Map reservedSpace = new HashMap<>(); + + public ClusterInfoTestBuilder withNode(String nodeId, long totalBytes, long freeBytes) { + diskUsage.put(nodeId, new DiskUsage(nodeId, nodeId, "/path", totalBytes, freeBytes)); + return this; + } + + public ClusterInfoTestBuilder withNodeUsedSpace(String nodeId, long usedBytes) { + diskUsage.compute(nodeId, (key, usage) -> { + assertThat(usage, notNullValue()); + return new DiskUsage(usage.nodeId(), usage.nodeName(), usage.path(), usage.totalBytes(), usage.freeBytes() - usedBytes); + }); + return this; + } + + public ClusterInfoTestBuilder withShard(ShardId shardId, boolean primary, long size) { + shardSizes.put(shardIdentifierFromRouting(shardId, primary), size); + return this; + } + + public ClusterInfoTestBuilder withReservedSpace(String nodeId, long size, ShardId... shardIds) { + reservedSpace.put(new NodeAndPath(nodeId, "/path"), new ReservedSpace(size, Set.of(shardIds))); + return this; + } + + public ClusterInfo build() { + return new ClusterInfo(diskUsage, diskUsage, shardSizes, Map.of(), Map.of(), reservedSpace); + } + } + private static IndexId indexIdFrom(IndexMetadata indexMetadata) { return new IndexId(indexMetadata.getIndex().getName(), indexMetadata.getIndex().getUUID()); } @@ -1188,8 +1381,8 @@ public ShardAllocationDecision decideShardAllocation(ShardRouting shard, Routing }); } - private static DesiredBalanceComputer createDesiredBalanceComputer(ShardsAllocator shardsAllocator) { - return new DesiredBalanceComputer(createBuiltInClusterSettings(), mock(ThreadPool.class), shardsAllocator); + private static DesiredBalanceComputer createDesiredBalanceComputer(ShardsAllocator allocator) { + return new DesiredBalanceComputer(createBuiltInClusterSettings(), mock(ThreadPool.class), allocator); } private static void assertDesiredAssignments(DesiredBalance desiredBalance, Map expected) { @@ -1210,13 +1403,7 @@ private static RoutingAllocation routingAllocationWithDecidersOf( Settings settings ) { return new RoutingAllocation( - new AllocationDeciders( - ClusterModule.createAllocationDeciders( - settings, - new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), - List.of() - ) - ), + randomAllocationDeciders(settings, createBuiltInClusterSettings(settings)), clusterState, clusterInfo, SnapshotShardSizeInfo.EMPTY, diff --git a/server/src/test/java/org/elasticsearch/common/util/CopyOnFirstWriteMapTests.java b/server/src/test/java/org/elasticsearch/common/util/CopyOnFirstWriteMapTests.java index 41870862c437d..0c34bfa66524b 100644 --- a/server/src/test/java/org/elasticsearch/common/util/CopyOnFirstWriteMapTests.java +++ b/server/src/test/java/org/elasticsearch/common/util/CopyOnFirstWriteMapTests.java @@ -21,7 +21,16 @@ public class CopyOnFirstWriteMapTests extends ESTestCase { public void testShouldNotCopyIfThereWereNoUpdates() { var source = Map.of("key", "value"); var copyOnFirstWrite = new CopyOnFirstWriteMap<>(source); - source.get("key"); + var copy = copyOnFirstWrite.toImmutableMap(); + + assertThat(copy, sameInstance(source)); + assertThat(copy, equalTo(source)); + } + + public void testShouldNotCopyWhenPuttingTheSameValue() { + var source = Map.of("key", "value"); + var copyOnFirstWrite = new CopyOnFirstWriteMap<>(source); + copyOnFirstWrite.put("key", "value"); var copy = copyOnFirstWrite.toImmutableMap(); assertThat(copy, sameInstance(source)); diff --git a/test/framework/src/main/java/org/elasticsearch/cluster/routing/TestShardRouting.java b/test/framework/src/main/java/org/elasticsearch/cluster/routing/TestShardRouting.java index 1158e805ba3c1..1810b5cee76ec 100644 --- a/test/framework/src/main/java/org/elasticsearch/cluster/routing/TestShardRouting.java +++ b/test/framework/src/main/java/org/elasticsearch/cluster/routing/TestShardRouting.java @@ -84,6 +84,29 @@ public static ShardRouting newShardRouting( ); } + public static ShardRouting newShardRouting( + ShardId shardId, + String currentNodeId, + String relocatingNodeId, + boolean primary, + ShardRoutingState state, + RecoverySource recoverySource + ) { + return new ShardRouting( + shardId, + currentNodeId, + relocatingNodeId, + primary, + state, + recoverySource, + buildUnassignedInfo(state), + buildRelocationFailureInfo(state), + buildAllocationId(state), + -1, + ShardRouting.Role.DEFAULT + ); + } + public static ShardRouting newShardRouting( String index, int shardId, diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java index 4fe85b2875ec6..eb255be092157 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java @@ -2376,7 +2376,7 @@ public static void setIgnoredErrorResponseCodes(Request request, RestStatus... r } public static void addXContentBody(Request request, ToXContent body) throws IOException { - final var xContentType = randomFrom(XContentType.JSON, XContentType.CBOR, XContentType.YAML, XContentType.SMILE); + final var xContentType = XContentType.JSON; final var bodyBytes = XContentHelper.toXContent(body, xContentType, EMPTY_PARAMS, randomBoolean()); request.setEntity( new InputStreamEntity( diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java index ad05a81cc2574..049102f87a544 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java @@ -15,7 +15,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.tests.util.TimeUnits; -import org.elasticsearch.Version; import org.elasticsearch.client.Node; import org.elasticsearch.client.Request; import org.elasticsearch.client.RequestOptions; @@ -26,7 +25,6 @@ import org.elasticsearch.client.sniff.ElasticsearchNodesSniffer; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.VersionId; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.core.IOUtils; @@ -447,49 +445,35 @@ public void test() throws IOException { ); } - // Try to extract the minimum node version. Assume CURRENT if nodes have non-semantic versions - // TODO: after https://github.com/elastic/elasticsearch/pull/103404 is merged, we can push this logic into SkipVersionContext. - // This way will have version parsing only when we actually have to skip on a version, we can remove the default and throw an - // IllegalArgumentException instead (attempting to skip on version where version is not semantic) - var oldestNodeVersion = restTestExecutionContext.nodesVersions() - .stream() - .map(ESRestTestCase::parseLegacyVersion) - .flatMap(Optional::stream) - .min(VersionId::compareTo) - .orElse(Version.CURRENT); - // skip test if the whole suite (yaml file) is disabled assumeFalse( testCandidate.getSetupSection().getSkipSection().getSkipMessage(testCandidate.getSuitePath()), - testCandidate.getSetupSection().getSkipSection().skip(oldestNodeVersion) + testCandidate.getSetupSection().getSkipSection().skip(restTestExecutionContext) ); // skip test if the whole suite (yaml file) is disabled assumeFalse( testCandidate.getTeardownSection().getSkipSection().getSkipMessage(testCandidate.getSuitePath()), - testCandidate.getTeardownSection().getSkipSection().skip(oldestNodeVersion) + testCandidate.getTeardownSection().getSkipSection().skip(restTestExecutionContext) ); // skip test if test section is disabled assumeFalse( testCandidate.getTestSection().getSkipSection().getSkipMessage(testCandidate.getTestPath()), - testCandidate.getTestSection().getSkipSection().skip(oldestNodeVersion) - ); - // skip test if os is excluded - assumeFalse( - testCandidate.getTestSection().getSkipSection().getSkipMessage(testCandidate.getTestPath()), - testCandidate.getTestSection().getSkipSection().skip(restTestExecutionContext.os()) + testCandidate.getTestSection().getSkipSection().skip(restTestExecutionContext) ); // let's check that there is something to run, otherwise there might be a problem with the test section - if (testCandidate.getTestSection().getExecutableSections().size() == 0) { + if (testCandidate.getTestSection().getExecutableSections().isEmpty()) { throw new IllegalArgumentException("No executable sections loaded for [" + testCandidate.getTestPath() + "]"); } assumeFalse( "[" + testCandidate.getTestPath() + "] skipped, reason: in fips 140 mode", - inFipsJvm() && testCandidate.getTestSection().getSkipSection().getFeatures().contains("fips_140") + inFipsJvm() && testCandidate.getTestSection().getSkipSection().yamlRunnerHasFeature("fips_140") ); - final Settings globalTemplateSettings = getGlobalTemplateSettings(testCandidate.getTestSection().getSkipSection().getFeatures()); + final Settings globalTemplateSettings = getGlobalTemplateSettings( + testCandidate.getTestSection().getSkipSection().yamlRunnerHasFeature("default_shards") + ); if (globalTemplateSettings.isEmpty() == false && ESRestTestCase.has(ProductFeature.LEGACY_TEMPLATES)) { final XContentBuilder template = jsonBuilder(); @@ -538,6 +522,7 @@ public void test() throws IOException { } } + @Deprecated protected Settings getGlobalTemplateSettings(List features) { if (features.contains("default_shards")) { return Settings.EMPTY; @@ -546,6 +531,14 @@ protected Settings getGlobalTemplateSettings(List features) { } } + protected Settings getGlobalTemplateSettings(boolean defaultShardsFeature) { + if (defaultShardsFeature) { + return Settings.EMPTY; + } else { + return globalTemplateIndexSettings; + } + } + protected boolean skipSetupSections() { return false; } diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/Features.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/Features.java index 8b9aafff5bded..d32b5684e19a9 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/Features.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/Features.java @@ -8,8 +8,6 @@ package org.elasticsearch.test.rest.yaml; -import org.elasticsearch.test.rest.ESRestTestCase; - import java.util.List; /** @@ -52,15 +50,7 @@ private Features() { */ public static boolean areAllSupported(List features) { for (String feature : features) { - if (feature.equals("xpack")) { - if (false == ESRestTestCase.hasXPack()) { - return false; - } - } else if (feature.equals("no_xpack")) { - if (ESRestTestCase.hasXPack()) { - return false; - } - } else if (false == SUPPORTED.contains(feature)) { + if (false == SUPPORTED.contains(feature)) { return false; } } diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/ClientYamlTestSuite.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/ClientYamlTestSuite.java index 4fee01e71d881..48f24d3a935af 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/ClientYamlTestSuite.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/ClientYamlTestSuite.java @@ -298,7 +298,7 @@ private static boolean hasSkipFeature( } private static boolean hasSkipFeature(String feature, SkipSection skipSection) { - return skipSection != null && skipSection.getFeatures().contains(feature); + return skipSection != null && skipSection.yamlRunnerHasFeature(feature); } public List getTestSections() { diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java index f57d90e911ea2..26158451755fd 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java @@ -677,7 +677,7 @@ private static NodeSelector parseVersionSelector(XContentParser parser) throws I nodeMatcher = nodeVersion -> Build.current().version().equals(nodeVersion); versionSelectorString = "version is " + Build.current().version() + " (current)"; } else { - var acceptedVersionRange = SkipSection.parseVersionRanges(parser.text()); + var acceptedVersionRange = VersionRange.parseVersionRanges(parser.text()); nodeMatcher = nodeVersion -> matchWithRange(nodeVersion, acceptedVersionRange, parser.getTokenLocation()); versionSelectorString = "version ranges " + acceptedVersionRange; } diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/SkipCriteria.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/SkipCriteria.java new file mode 100644 index 0000000000000..398b402748506 --- /dev/null +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/SkipCriteria.java @@ -0,0 +1,58 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.test.rest.yaml.section; + +import org.elasticsearch.Version; +import org.elasticsearch.common.VersionId; +import org.elasticsearch.test.rest.ESRestTestCase; +import org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext; + +import java.util.List; +import java.util.Optional; +import java.util.function.Predicate; + +public class SkipCriteria { + + public static final Predicate SKIP_ALWAYS = context -> true; + + private SkipCriteria() {} + + static Predicate fromVersionRange(String versionRange) { + final List versionRanges = VersionRange.parseVersionRanges(versionRange); + assert versionRanges.isEmpty() == false; + return context -> { + // Try to extract the minimum node version. Assume CURRENT if nodes have non-semantic versions + // TODO: push this logic down to VersionRange. + // This way will have version parsing only when we actually have to skip on a version, we can remove the default and throw + // IllegalArgumentException instead (attempting to skip on version where version is not semantic) + var oldestNodeVersion = context.nodesVersions() + .stream() + .map(ESRestTestCase::parseLegacyVersion) + .flatMap(Optional::stream) + .min(VersionId::compareTo) + .orElse(Version.CURRENT); + + return versionRanges.stream().anyMatch(range -> range.contains(oldestNodeVersion)); + }; + } + + static Predicate fromOsList(List operatingSystems) { + return context -> operatingSystems.stream().anyMatch(osName -> osName.equals(context.os())); + } + + static Predicate fromClusterModules(boolean xpackRequired) { + // TODO: change ESRestTestCase.hasXPack() to be context-specific + return context -> { + if (xpackRequired) { + return ESRestTestCase.hasXPack() == false; + } + return ESRestTestCase.hasXPack(); + }; + } +} diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/SkipSection.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/SkipSection.java index 74d6e8284b438..4bd80fa4d9f13 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/SkipSection.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/SkipSection.java @@ -7,17 +7,17 @@ */ package org.elasticsearch.test.rest.yaml.section; -import org.elasticsearch.Version; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; -import org.elasticsearch.test.VersionUtils; +import org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext; import org.elasticsearch.test.rest.yaml.Features; +import org.elasticsearch.xcontent.XContentLocation; import org.elasticsearch.xcontent.XContentParser; import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; import java.util.List; +import java.util.function.Predicate; /** * Represents a skip section that tells whether a specific test section or suite needs to be skipped @@ -27,6 +27,98 @@ * - an operating system (full name, including specific Linux distributions) that might show a certain behavior */ public class SkipSection { + + static class SkipSectionBuilder { + String version = null; + String reason = null; + List testFeatures = new ArrayList<>(); + List operatingSystems = new ArrayList<>(); + + enum XPackRequested { + NOT_SPECIFIED, + YES, + NO, + MISMATCHED + } + + XPackRequested xpackRequested = XPackRequested.NOT_SPECIFIED; + + public SkipSectionBuilder withVersion(String version) { + this.version = version; + return this; + } + + public SkipSectionBuilder withReason(String reason) { + this.reason = reason; + return this; + } + + public SkipSectionBuilder withTestFeature(String featureName) { + this.testFeatures.add(featureName); + return this; + } + + public void withXPack(boolean xpackRequired) { + if (xpackRequired && xpackRequested == XPackRequested.NO || xpackRequired == false && xpackRequested == XPackRequested.YES) { + xpackRequested = XPackRequested.MISMATCHED; + } else { + xpackRequested = xpackRequired ? XPackRequested.YES : XPackRequested.NO; + } + } + + public SkipSectionBuilder withOs(String osName) { + this.operatingSystems.add(osName); + return this; + } + + void validate(XContentLocation contentLocation) { + if ((Strings.hasLength(version) == false) + && testFeatures.isEmpty() + && operatingSystems.isEmpty() + && xpackRequested == XPackRequested.NOT_SPECIFIED) { + throw new ParsingException( + contentLocation, + "at least one criteria (version, test features, os) is mandatory within a skip section" + ); + } + if (Strings.hasLength(version) && Strings.hasLength(reason) == false) { + throw new ParsingException(contentLocation, "reason is mandatory within skip version section"); + } + if (operatingSystems.isEmpty() == false && Strings.hasLength(reason) == false) { + throw new ParsingException(contentLocation, "reason is mandatory within skip version section"); + } + // make feature "skip_os" mandatory if os is given, this is a temporary solution until language client tests know about os + if (operatingSystems.isEmpty() == false && testFeatures.contains("skip_os") == false) { + throw new ParsingException(contentLocation, "if os is specified, feature skip_os must be set"); + } + if (xpackRequested == XPackRequested.MISMATCHED) { + throw new ParsingException(contentLocation, "either `xpack` or `no_xpack` can be present, not both"); + } + } + + public SkipSection build() { + final List> skipCriteriaList; + + // Check if the test runner supports all YAML framework features (see {@link Features}). If not, default to always skip this + // section. + if (Features.areAllSupported(testFeatures) == false) { + skipCriteriaList = List.of(SkipCriteria.SKIP_ALWAYS); + } else { + skipCriteriaList = new ArrayList<>(); + if (xpackRequested == XPackRequested.YES || xpackRequested == XPackRequested.NO) { + skipCriteriaList.add(SkipCriteria.fromClusterModules(xpackRequested == XPackRequested.YES)); + } + if (Strings.hasLength(version)) { + skipCriteriaList.add(SkipCriteria.fromVersionRange(version)); + } + if (operatingSystems.isEmpty() == false) { + skipCriteriaList.add(SkipCriteria.fromOsList(operatingSystems)); + } + } + return new SkipSection(skipCriteriaList, testFeatures, reason); + } + } + /** * Parse a {@link SkipSection} if the next field is {@code skip}, otherwise returns {@link SkipSection#EMPTY}. */ @@ -43,6 +135,24 @@ public static SkipSection parseIfNext(XContentParser parser) throws IOException } public static SkipSection parse(XContentParser parser) throws IOException { + return parseInternal(parser).build(); + } + + private static void parseFeature(String feature, SkipSectionBuilder builder) { + // #31403 introduced YAML test "features" to indicate if the cluster being tested has xpack installed (`xpack`) + // or if it does *not* have xpack installed (`no_xpack`). These are not test runner features, so now that we have + // "modular" skip criteria let's separate them. Eventually, these should move to their own skip section. + if (feature.equals("xpack")) { + builder.withXPack(true); + } else if (feature.equals("no_xpack")) { + builder.withXPack(false); + } else { + builder.withTestFeature(feature); + } + } + + // package private for tests + static SkipSectionBuilder parseInternal(XContentParser parser) throws IOException { if (parser.nextToken() != XContentParser.Token.START_OBJECT) { throw new IllegalArgumentException( "Expected [" @@ -54,22 +164,21 @@ public static SkipSection parse(XContentParser parser) throws IOException { } String currentFieldName = null; XContentParser.Token token; - String version = null; - String reason = null; - List features = new ArrayList<>(); - List operatingSystems = new ArrayList<>(); + + var builder = new SkipSectionBuilder(); + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); } else if (token.isValue()) { if ("version".equals(currentFieldName)) { - version = parser.text(); + builder.withVersion(parser.text()); } else if ("reason".equals(currentFieldName)) { - reason = parser.text(); + builder.withReason(parser.text()); } else if ("features".equals(currentFieldName)) { - features.add(parser.text()); + parseFeature(parser.text(), builder); } else if ("os".equals(currentFieldName)) { - operatingSystems.add(parser.text()); + builder.withOs(parser.text()); } else { throw new ParsingException( parser.getTokenLocation(), @@ -79,131 +188,67 @@ public static SkipSection parse(XContentParser parser) throws IOException { } else if (token == XContentParser.Token.START_ARRAY) { if ("features".equals(currentFieldName)) { while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - features.add(parser.text()); + parseFeature(parser.text(), builder); } } else if ("os".equals(currentFieldName)) { while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - operatingSystems.add(parser.text()); + builder.withOs(parser.text()); } } } } parser.nextToken(); - - if ((Strings.hasLength(version) == false) && features.isEmpty() && operatingSystems.isEmpty()) { - throw new ParsingException(parser.getTokenLocation(), "version, features or os is mandatory within skip section"); - } - if (Strings.hasLength(version) && Strings.hasLength(reason) == false) { - throw new ParsingException(parser.getTokenLocation(), "reason is mandatory within skip version section"); - } - if (operatingSystems.isEmpty() == false && Strings.hasLength(reason) == false) { - throw new ParsingException(parser.getTokenLocation(), "reason is mandatory within skip version section"); - } - // make feature "skip_os" mandatory if os is given, this is a temporary solution until language client tests know about os - if (operatingSystems.isEmpty() == false && features.contains("skip_os") == false) { - throw new ParsingException(parser.getTokenLocation(), "if os is specified, feature skip_os must be set"); - } - return new SkipSection(version, features, operatingSystems, reason); + builder.validate(parser.getTokenLocation()); + return builder; } public static final SkipSection EMPTY = new SkipSection(); - private final List versionRanges; - private final List features; - private final List operatingSystems; + private final List> skipCriteriaList; + private final List yamlRunnerFeatures; private final String reason; private SkipSection() { - this.versionRanges = new ArrayList<>(); - this.features = new ArrayList<>(); - this.operatingSystems = new ArrayList<>(); + this.skipCriteriaList = new ArrayList<>(); + this.yamlRunnerFeatures = new ArrayList<>(); this.reason = null; } - public SkipSection(String versionRange, List features, List operatingSystems, String reason) { - assert features != null; - this.versionRanges = parseVersionRanges(versionRange); - assert versionRanges.isEmpty() == false; - this.features = features; - this.operatingSystems = operatingSystems; + SkipSection(List> skipCriteriaList, List yamlRunnerFeatures, String reason) { + this.skipCriteriaList = skipCriteriaList; + this.yamlRunnerFeatures = yamlRunnerFeatures; this.reason = reason; } - public Version getLowerVersion() { - return versionRanges.get(0).lower(); - } - - public Version getUpperVersion() { - return versionRanges.get(versionRanges.size() - 1).upper(); - } - - public List getFeatures() { - return features; - } - - public List getOperatingSystems() { - return operatingSystems; + public boolean yamlRunnerHasFeature(String feature) { + return yamlRunnerFeatures.contains(feature); } public String getReason() { return reason; } - public boolean skip(Version currentVersion) { + public boolean skip(ClientYamlTestExecutionContext context) { if (isEmpty()) { return false; } - boolean skip = versionRanges.stream().anyMatch(range -> range.contains(currentVersion)); - return skip || Features.areAllSupported(features) == false; - } - - public boolean skip(String os) { - return this.operatingSystems.contains(os); - } - public boolean isVersionCheck() { - return features.isEmpty() && operatingSystems.isEmpty(); + return skipCriteriaList.stream().anyMatch(c -> c.test(context)); } public boolean isEmpty() { return EMPTY.equals(this); } - static List parseVersionRanges(String rawRanges) { - if (rawRanges == null) { - return Collections.singletonList(new VersionRange(null, null)); - } - String[] ranges = rawRanges.split(","); - List versionRanges = new ArrayList<>(); - for (String rawRange : ranges) { - if (rawRange.trim().equals("all")) { - return Collections.singletonList(new VersionRange(VersionUtils.getFirstVersion(), Version.CURRENT)); - } - String[] skipVersions = rawRange.split("-", -1); - if (skipVersions.length > 2) { - throw new IllegalArgumentException("version range malformed: " + rawRanges); - } - - String lower = skipVersions[0].trim(); - String upper = skipVersions[1].trim(); - VersionRange versionRange = new VersionRange( - lower.isEmpty() ? VersionUtils.getFirstVersion() : Version.fromString(lower), - upper.isEmpty() ? Version.CURRENT : Version.fromString(upper) - ); - versionRanges.add(versionRange); - } - return versionRanges; - } - public String getSkipMessage(String description) { StringBuilder messageBuilder = new StringBuilder(); messageBuilder.append("[").append(description).append("] skipped,"); if (reason != null) { messageBuilder.append(" reason: [").append(getReason()).append("]"); } - if (features.isEmpty() == false) { - messageBuilder.append(" unsupported features ").append(getFeatures()); + if (yamlRunnerFeatures.isEmpty() == false) { + messageBuilder.append(" unsupported features ").append(yamlRunnerFeatures); } return messageBuilder.toString(); } diff --git a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/VersionRange.java b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/VersionRange.java index f4b9a3d4aef1a..4feb1ed609fae 100644 --- a/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/VersionRange.java +++ b/test/yaml-rest-runner/src/main/java/org/elasticsearch/test/rest/yaml/section/VersionRange.java @@ -8,6 +8,11 @@ package org.elasticsearch.test.rest.yaml.section; import org.elasticsearch.Version; +import org.elasticsearch.test.VersionUtils; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; public record VersionRange(Version lower, Version upper) { @@ -19,4 +24,30 @@ public boolean contains(Version currentVersion) { public String toString() { return "[" + lower + " - " + upper + "]"; } + + static List parseVersionRanges(String rawRanges) { + if (rawRanges == null) { + return Collections.singletonList(new VersionRange(null, null)); + } + String[] ranges = rawRanges.split(","); + List versionRanges = new ArrayList<>(); + for (String rawRange : ranges) { + if (rawRange.trim().equals("all")) { + return Collections.singletonList(new VersionRange(VersionUtils.getFirstVersion(), Version.CURRENT)); + } + String[] skipVersions = rawRange.split("-", -1); + if (skipVersions.length > 2) { + throw new IllegalArgumentException("version range malformed: " + rawRanges); + } + + String lower = skipVersions[0].trim(); + String upper = skipVersions[1].trim(); + VersionRange versionRange = new VersionRange( + lower.isEmpty() ? VersionUtils.getFirstVersion() : Version.fromString(lower), + upper.isEmpty() ? Version.CURRENT : Version.fromString(upper) + ); + versionRanges.add(versionRange); + } + return versionRanges; + } } diff --git a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/ClientYamlTestSectionTests.java b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/ClientYamlTestSectionTests.java index 6a692e29926de..0ee275fc89c15 100644 --- a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/ClientYamlTestSectionTests.java +++ b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/ClientYamlTestSectionTests.java @@ -8,7 +8,6 @@ package org.elasticsearch.test.rest.yaml.section; -import org.elasticsearch.Version; import org.elasticsearch.common.ParsingException; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.yaml.YamlXContent; @@ -98,8 +97,6 @@ public void testParseTestSectionWithDoSetAndSkipSectionsNoSkip() throws Exceptio assertThat(testSection, notNullValue()); assertThat(testSection.getName(), equalTo("First test section")); assertThat(testSection.getSkipSection(), notNullValue()); - assertThat(testSection.getSkipSection().getLowerVersion(), equalTo(Version.fromString("6.0.0"))); - assertThat(testSection.getSkipSection().getUpperVersion(), equalTo(Version.fromString("6.2.0"))); assertThat(testSection.getSkipSection().getReason(), equalTo("Update doesn't return metadata fields, waiting for #3259")); assertThat(testSection.getExecutableSections().size(), equalTo(2)); DoSection doSection = (DoSection) testSection.getExecutableSections().get(0); diff --git a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/ClientYamlTestSuiteTests.java b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/ClientYamlTestSuiteTests.java index 3f8cc298c5c36..c64a30378e9d6 100644 --- a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/ClientYamlTestSuiteTests.java +++ b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/ClientYamlTestSuiteTests.java @@ -8,7 +8,6 @@ package org.elasticsearch.test.rest.yaml.section; -import org.elasticsearch.Version; import org.elasticsearch.client.NodeSelector; import org.elasticsearch.common.ParsingException; import org.elasticsearch.core.Strings; @@ -28,7 +27,6 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; -import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; @@ -152,8 +150,7 @@ public void testParseTestSetupTeardownAndSections() throws Exception { restTestSuite.getTestSections().get(1).getSkipSection().getReason(), equalTo("for newer versions the index name is always returned") ); - assertThat(restTestSuite.getTestSections().get(1).getSkipSection().getLowerVersion(), equalTo(Version.fromString("6.0.0"))); - assertThat(restTestSuite.getTestSections().get(1).getSkipSection().getUpperVersion(), equalTo(Version.CURRENT)); + assertThat(restTestSuite.getTestSections().get(1).getExecutableSections().size(), equalTo(3)); assertThat(restTestSuite.getTestSections().get(1).getExecutableSections().get(0), instanceOf(DoSection.class)); doSection = (DoSection) restTestSuite.getTestSections().get(1).getExecutableSections().get(0); @@ -423,11 +420,7 @@ public void testParseSkipOs() throws Exception { assertThat(restTestSuite.getTestSections().get(0).getName(), equalTo("Broken on some os")); assertThat(restTestSuite.getTestSections().get(0).getSkipSection().isEmpty(), equalTo(false)); assertThat(restTestSuite.getTestSections().get(0).getSkipSection().getReason(), equalTo("not supported")); - assertThat( - restTestSuite.getTestSections().get(0).getSkipSection().getOperatingSystems(), - containsInAnyOrder("windows95", "debian-5") - ); - assertThat(restTestSuite.getTestSections().get(0).getSkipSection().getFeatures(), containsInAnyOrder("skip_os")); + assertThat(restTestSuite.getTestSections().get(0).getSkipSection().yamlRunnerHasFeature("skip_os"), equalTo(true)); } public void testParseFileWithSingleTestSection() throws Exception { @@ -660,7 +653,7 @@ public void testAddingDoWithWarningWithSkip() { DoSection doSection = new DoSection(new XContentLocation(lineNumber, 0)); doSection.setExpectedWarningHeaders(singletonList("foo")); doSection.setApiCallSection(new ApiCallSection("test")); - SkipSection skipSection = new SkipSection(null, singletonList("warnings"), emptyList(), null); + SkipSection skipSection = new SkipSection(emptyList(), singletonList("warnings"), null); createTestSuite(skipSection, doSection).validate(); } @@ -669,13 +662,13 @@ public void testAddingDoWithWarningRegexWithSkip() { DoSection doSection = new DoSection(new XContentLocation(lineNumber, 0)); doSection.setExpectedWarningHeadersRegex(singletonList(Pattern.compile("foo"))); doSection.setApiCallSection(new ApiCallSection("test")); - SkipSection skipSection = new SkipSection(null, singletonList("warnings_regex"), emptyList(), null); + SkipSection skipSection = new SkipSection(emptyList(), singletonList("warnings_regex"), null); createTestSuite(skipSection, doSection).validate(); } public void testAddingDoWithNodeSelectorWithSkip() { int lineNumber = between(1, 10000); - SkipSection skipSection = new SkipSection(null, singletonList("node_selector"), emptyList(), null); + SkipSection skipSection = new SkipSection(emptyList(), singletonList("node_selector"), null); DoSection doSection = new DoSection(new XContentLocation(lineNumber, 0)); ApiCallSection apiCall = new ApiCallSection("test"); apiCall.setNodeSelector(NodeSelector.SKIP_DEDICATED_MASTERS); @@ -685,7 +678,7 @@ public void testAddingDoWithNodeSelectorWithSkip() { public void testAddingDoWithHeadersWithSkip() { int lineNumber = between(1, 10000); - SkipSection skipSection = new SkipSection(null, singletonList("headers"), emptyList(), null); + SkipSection skipSection = new SkipSection(emptyList(), singletonList("headers"), null); DoSection doSection = new DoSection(new XContentLocation(lineNumber, 0)); ApiCallSection apiCallSection = new ApiCallSection("test"); apiCallSection.addHeaders(singletonMap("foo", "bar")); @@ -695,7 +688,7 @@ public void testAddingDoWithHeadersWithSkip() { public void testAddingContainsWithSkip() { int lineNumber = between(1, 10000); - SkipSection skipSection = new SkipSection(null, singletonList("contains"), emptyList(), null); + SkipSection skipSection = new SkipSection(emptyList(), singletonList("contains"), null); ContainsAssertion containsAssertion = new ContainsAssertion( new XContentLocation(lineNumber, 0), randomAlphaOfLength(randomIntBetween(3, 30)), @@ -706,7 +699,7 @@ public void testAddingContainsWithSkip() { public void testAddingCloseToWithSkip() { int lineNumber = between(1, 10000); - SkipSection skipSection = new SkipSection(null, singletonList("close_to"), emptyList(), null); + SkipSection skipSection = new SkipSection(emptyList(), singletonList("close_to"), null); CloseToAssertion closeToAssertion = new CloseToAssertion( new XContentLocation(lineNumber, 0), randomAlphaOfLength(randomIntBetween(3, 30)), @@ -718,7 +711,7 @@ public void testAddingCloseToWithSkip() { public void testAddingIsAfterWithSkip() { int lineNumber = between(1, 10000); - SkipSection skipSection = new SkipSection(null, singletonList("is_after"), emptyList(), null); + SkipSection skipSection = new SkipSection(emptyList(), singletonList("is_after"), null); IsAfterAssertion isAfterAssertion = new IsAfterAssertion( new XContentLocation(lineNumber, 0), randomAlphaOfLength(randomIntBetween(3, 30)), diff --git a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/SetupSectionTests.java b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/SetupSectionTests.java index fa91726ca73e6..53aaf99d7e272 100644 --- a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/SetupSectionTests.java +++ b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/SetupSectionTests.java @@ -7,7 +7,6 @@ */ package org.elasticsearch.test.rest.yaml.section; -import org.elasticsearch.Version; import org.elasticsearch.xcontent.yaml.YamlXContent; import java.io.IOException; @@ -108,8 +107,6 @@ public void testParseSetupAndSkipSectionNoSkip() throws Exception { assertThat(setupSection, notNullValue()); assertThat(setupSection.getSkipSection().isEmpty(), equalTo(false)); assertThat(setupSection.getSkipSection(), notNullValue()); - assertThat(setupSection.getSkipSection().getLowerVersion(), equalTo(Version.fromString("6.0.0"))); - assertThat(setupSection.getSkipSection().getUpperVersion(), equalTo(Version.fromString("6.3.0"))); assertThat(setupSection.getSkipSection().getReason(), equalTo("Update doesn't return metadata fields, waiting for #3259")); assertThat(setupSection.getExecutableSections().size(), equalTo(2)); assertThat(setupSection.getExecutableSections().get(0), instanceOf(DoSection.class)); diff --git a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/SkipSectionTests.java b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/SkipSectionTests.java index 5e4db061dbaaa..bd1f8fa758499 100644 --- a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/SkipSectionTests.java +++ b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/SkipSectionTests.java @@ -12,61 +12,171 @@ import org.elasticsearch.common.ParsingException; import org.elasticsearch.core.Strings; import org.elasticsearch.test.VersionUtils; +import org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext; import org.elasticsearch.xcontent.yaml.YamlXContent; +import java.io.IOException; import java.util.Collections; +import java.util.List; +import java.util.Set; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.emptyOrNullString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class SkipSectionTests extends AbstractClientYamlTestFragmentParserTestCase { - public void testSkipMultiRange() { - SkipSection section = new SkipSection("6.0.0 - 6.1.0, 7.1.0 - 7.5.0", Collections.emptyList(), Collections.emptyList(), "foobar"); - - assertFalse(section.skip(Version.CURRENT)); - assertFalse(section.skip(Version.fromString("6.2.0"))); - assertFalse(section.skip(Version.fromString("7.0.0"))); - assertFalse(section.skip(Version.fromString("7.6.0"))); - - assertTrue(section.skip(Version.fromString("6.0.0"))); - assertTrue(section.skip(Version.fromString("6.1.0"))); - assertTrue(section.skip(Version.fromString("7.1.0"))); - assertTrue(section.skip(Version.fromString("7.5.0"))); + public void testSkipVersionMultiRange() { + SkipSection section = new SkipSection( + List.of(SkipCriteria.fromVersionRange("6.0.0 - 6.1.0, 7.1.0 - 7.5.0")), + Collections.emptyList(), + "foobar" + ); - section = new SkipSection("- 7.1.0, 7.2.0 - 7.5.0, 8.0.0 -", Collections.emptyList(), Collections.emptyList(), "foobar"); - assertTrue(section.skip(Version.fromString("7.0.0"))); - assertTrue(section.skip(Version.fromString("7.3.0"))); - assertTrue(section.skip(Version.fromString("8.0.0"))); + var outOfRangeMockContext = mock(ClientYamlTestExecutionContext.class); + when(outOfRangeMockContext.nodesVersions()).thenReturn(Set.of(Version.CURRENT.toString())) + .thenReturn(Set.of("6.2.0")) + .thenReturn(Set.of("7.0.0")) + .thenReturn(Set.of("7.6.0")); + + assertFalse(section.skip(outOfRangeMockContext)); + assertFalse(section.skip(outOfRangeMockContext)); + assertFalse(section.skip(outOfRangeMockContext)); + assertFalse(section.skip(outOfRangeMockContext)); + + var inRangeMockContext = mock(ClientYamlTestExecutionContext.class); + when(inRangeMockContext.nodesVersions()).thenReturn(Set.of("6.0.0")) + .thenReturn(Set.of("6.1.0")) + .thenReturn(Set.of("7.1.0")) + .thenReturn(Set.of("7.5.0")); + + assertTrue(section.skip(inRangeMockContext)); + assertTrue(section.skip(inRangeMockContext)); + assertTrue(section.skip(inRangeMockContext)); + assertTrue(section.skip(inRangeMockContext)); } - public void testSkip() { - SkipSection section = new SkipSection( - "6.0.0 - 6.1.0", - randomBoolean() ? Collections.emptyList() : Collections.singletonList("warnings"), + public void testSkipVersionMultiOpenRange() { + var section = new SkipSection( + List.of(SkipCriteria.fromVersionRange("- 7.1.0, 7.2.0 - 7.5.0, 8.0.0 -")), Collections.emptyList(), "foobar" ); - assertFalse(section.skip(Version.CURRENT)); - assertTrue(section.skip(Version.fromString("6.0.0"))); - section = new SkipSection( - randomBoolean() ? null : "6.0.0 - 6.1.0", - Collections.singletonList("boom"), - Collections.emptyList(), + + var outOfRangeMockContext = mock(ClientYamlTestExecutionContext.class); + when(outOfRangeMockContext.nodesVersions()).thenReturn(Set.of("7.1.1")).thenReturn(Set.of("7.6.0")); + + assertFalse(section.skip(outOfRangeMockContext)); + assertFalse(section.skip(outOfRangeMockContext)); + + var inRangeMockContext = mock(ClientYamlTestExecutionContext.class); + when(inRangeMockContext.nodesVersions()).thenReturn(Set.of("7.0.0")) + .thenReturn(Set.of("7.3.0")) + .thenReturn(Set.of("8.0.0")) + .thenReturn(Set.of(Version.CURRENT.toString())); + + assertTrue(section.skip(inRangeMockContext)); + assertTrue(section.skip(inRangeMockContext)); + assertTrue(section.skip(inRangeMockContext)); + assertTrue(section.skip(inRangeMockContext)); + } + + public void testSkipVersion() { + SkipSection section = new SkipSection(List.of(SkipCriteria.fromVersionRange("6.0.0 - 6.1.0")), Collections.emptyList(), "foobar"); + + var mockContext = mock(ClientYamlTestExecutionContext.class); + when(mockContext.nodesVersions()).thenReturn(Set.of(Version.CURRENT.toString())) + .thenReturn(Set.of("6.0.0")) + .thenReturn(Set.of("6.0.0", "6.1.0")) + .thenReturn(Set.of("6.0.0", "5.2.0")); + + assertFalse(section.skip(mockContext)); + assertTrue(section.skip(mockContext)); + assertTrue(section.skip(mockContext)); + assertFalse(section.skip(mockContext)); + } + + public void testSkipVersionWithTestFeatures() { + SkipSection section = new SkipSection( + List.of(SkipCriteria.fromVersionRange("6.0.0 - 6.1.0")), + Collections.singletonList("warnings"), "foobar" ); - assertTrue(section.skip(Version.CURRENT)); + + var mockContext = mock(ClientYamlTestExecutionContext.class); + when(mockContext.nodesVersions()).thenReturn(Set.of(Version.CURRENT.toString())).thenReturn(Set.of("6.0.0")); + + assertFalse(section.skip(mockContext)); + assertTrue(section.skip(mockContext)); + } + + public void testSkipTestFeatures() { + var section = new SkipSection.SkipSectionBuilder().withTestFeature("boom").build(); + assertTrue(section.skip(mock(ClientYamlTestExecutionContext.class))); + } + + public void testSkipTestFeaturesOverridesAnySkipCriteria() { + var section = new SkipSection.SkipSectionBuilder().withOs("test-os").withTestFeature("boom").build(); + + var mockContext = mock(ClientYamlTestExecutionContext.class); + when(mockContext.os()).thenReturn("test-os"); + + // Skip even if OS is matching + assertTrue(section.skip(mockContext)); + } + + public void testSkipOs() { + SkipSection section = new SkipSection.SkipSectionBuilder().withOs("windows95").withOs("debian-5").build(); + + var mockContext = mock(ClientYamlTestExecutionContext.class); + + when(mockContext.os()).thenReturn("debian-5"); + assertTrue(section.skip(mockContext)); + + when(mockContext.os()).thenReturn("windows95"); + assertTrue(section.skip(mockContext)); + + when(mockContext.os()).thenReturn("ms-dos"); + assertFalse(section.skip(mockContext)); + } + + public void testSkipOsWithTestFeatures() { + SkipSection section = new SkipSection.SkipSectionBuilder().withTestFeature("warnings") + .withOs("windows95") + .withOs("debian-5") + .build(); + + var mockContext = mock(ClientYamlTestExecutionContext.class); + when(mockContext.os()).thenReturn("debian-5"); + assertTrue(section.skip(mockContext)); + + when(mockContext.os()).thenReturn("windows95"); + assertTrue(section.skip(mockContext)); + + when(mockContext.os()).thenReturn("ms-dos"); + assertFalse(section.skip(mockContext)); } public void testMessage() { - SkipSection section = new SkipSection("6.0.0 - 6.1.0", Collections.singletonList("warnings"), Collections.emptyList(), "foobar"); + SkipSection section = new SkipSection( + List.of(SkipCriteria.fromVersionRange("6.0.0 - 6.1.0")), + Collections.singletonList("warnings"), + "foobar" + ); assertEquals("[FOOBAR] skipped, reason: [foobar] unsupported features [warnings]", section.getSkipMessage("FOOBAR")); - section = new SkipSection(null, Collections.singletonList("warnings"), Collections.emptyList(), "foobar"); + section = new SkipSection(List.of(), Collections.singletonList("warnings"), "foobar"); assertEquals("[FOOBAR] skipped, reason: [foobar] unsupported features [warnings]", section.getSkipMessage("FOOBAR")); - section = new SkipSection(null, Collections.singletonList("warnings"), Collections.emptyList(), null); + section = new SkipSection(List.of(), Collections.singletonList("warnings"), null); assertEquals("[FOOBAR] skipped, unsupported features [warnings]", section.getSkipMessage("FOOBAR")); } @@ -76,49 +186,61 @@ public void testParseSkipSectionVersionNoFeature() throws Exception { version: " - %s" reason: Delete ignores the parent param""", version)); - SkipSection skipSection = SkipSection.parse(parser); - assertThat(skipSection, notNullValue()); - assertThat(skipSection.getLowerVersion(), equalTo(VersionUtils.getFirstVersion())); - assertThat(skipSection.getUpperVersion(), equalTo(version)); - assertThat(skipSection.getFeatures().size(), equalTo(0)); - assertThat(skipSection.getReason(), equalTo("Delete ignores the parent param")); + var skipSectionBuilder = SkipSection.parseInternal(parser); + assertThat(skipSectionBuilder, notNullValue()); + assertThat(skipSectionBuilder.version, not(emptyOrNullString())); + assertThat(skipSectionBuilder.testFeatures.size(), equalTo(0)); + assertThat(skipSectionBuilder.reason, equalTo("Delete ignores the parent param")); } - public void testParseSkipSectionAllVersions() throws Exception { - parser = createParser(YamlXContent.yamlXContent, """ - version: " all " - reason: Delete ignores the parent param"""); + public void testParseSkipSectionFeatureNoVersion() throws Exception { + parser = createParser(YamlXContent.yamlXContent, "features: regex"); - SkipSection skipSection = SkipSection.parse(parser); - assertThat(skipSection, notNullValue()); - assertThat(skipSection.getLowerVersion(), equalTo(VersionUtils.getFirstVersion())); - assertThat(skipSection.getUpperVersion(), equalTo(Version.CURRENT)); - assertThat(skipSection.getFeatures().size(), equalTo(0)); - assertThat(skipSection.getReason(), equalTo("Delete ignores the parent param")); + var skipSectionBuilder = SkipSection.parseInternal(parser); + assertThat(skipSectionBuilder, notNullValue()); + assertThat(skipSectionBuilder.version, emptyOrNullString()); + assertThat(skipSectionBuilder.testFeatures, contains("regex")); + assertThat(skipSectionBuilder.reason, nullValue()); + assertThat(skipSectionBuilder.xpackRequested, is(SkipSection.SkipSectionBuilder.XPackRequested.NOT_SPECIFIED)); } - public void testParseSkipSectionFeatureNoVersion() throws Exception { - parser = createParser(YamlXContent.yamlXContent, "features: regex"); + public void testParseXPackFeature() throws IOException { + parser = createParser(YamlXContent.yamlXContent, "features: xpack"); + + var skipSectionBuilder = SkipSection.parseInternal(parser); + assertThat(skipSectionBuilder, notNullValue()); + assertThat(skipSectionBuilder.version, emptyOrNullString()); + assertThat(skipSectionBuilder.testFeatures, empty()); + assertThat(skipSectionBuilder.reason, nullValue()); + assertThat(skipSectionBuilder.xpackRequested, is(SkipSection.SkipSectionBuilder.XPackRequested.YES)); + } + + public void testParseNoXPackFeature() throws IOException { + parser = createParser(YamlXContent.yamlXContent, "features: no_xpack"); + + var skipSectionBuilder = SkipSection.parseInternal(parser); + assertThat(skipSectionBuilder, notNullValue()); + assertThat(skipSectionBuilder.version, emptyOrNullString()); + assertThat(skipSectionBuilder.testFeatures, empty()); + assertThat(skipSectionBuilder.reason, nullValue()); + assertThat(skipSectionBuilder.xpackRequested, is(SkipSection.SkipSectionBuilder.XPackRequested.NO)); + } - SkipSection skipSection = SkipSection.parse(parser); - assertThat(skipSection, notNullValue()); - assertThat(skipSection.isVersionCheck(), equalTo(false)); - assertThat(skipSection.getFeatures().size(), equalTo(1)); - assertThat(skipSection.getFeatures().get(0), equalTo("regex")); - assertThat(skipSection.getReason(), nullValue()); + public void testParseBothXPackFeatures() throws IOException { + parser = createParser(YamlXContent.yamlXContent, "features: [xpack, no_xpack]"); + + var e = expectThrows(ParsingException.class, () -> SkipSection.parseInternal(parser)); + assertThat(e.getMessage(), containsString("either `xpack` or `no_xpack` can be present, not both")); } public void testParseSkipSectionFeaturesNoVersion() throws Exception { parser = createParser(YamlXContent.yamlXContent, "features: [regex1,regex2,regex3]"); - SkipSection skipSection = SkipSection.parse(parser); - assertThat(skipSection, notNullValue()); - assertThat(skipSection.isVersionCheck(), equalTo(false)); - assertThat(skipSection.getFeatures().size(), equalTo(3)); - assertThat(skipSection.getFeatures().get(0), equalTo("regex1")); - assertThat(skipSection.getFeatures().get(1), equalTo("regex2")); - assertThat(skipSection.getFeatures().get(2), equalTo("regex3")); - assertThat(skipSection.getReason(), nullValue()); + var skipSectionBuilder = SkipSection.parseInternal(parser); + assertThat(skipSectionBuilder, notNullValue()); + assertThat(skipSectionBuilder.version, emptyOrNullString()); + assertThat(skipSectionBuilder.testFeatures, contains("regex1", "regex2", "regex3")); + assertThat(skipSectionBuilder.reason, nullValue()); } public void testParseSkipSectionBothFeatureAndVersion() throws Exception { @@ -127,25 +249,24 @@ public void testParseSkipSectionBothFeatureAndVersion() throws Exception { features: regex reason: Delete ignores the parent param"""); - SkipSection skipSection = SkipSection.parse(parser); - assertEquals(VersionUtils.getFirstVersion(), skipSection.getLowerVersion()); - assertEquals(Version.fromString("0.90.2"), skipSection.getUpperVersion()); - assertEquals(Collections.singletonList("regex"), skipSection.getFeatures()); - assertEquals("Delete ignores the parent param", skipSection.getReason()); + var skipSectionBuilder = SkipSection.parseInternal(parser); + assertThat(skipSectionBuilder.version, not(emptyOrNullString())); + assertThat(skipSectionBuilder.testFeatures, contains("regex")); + assertThat(skipSectionBuilder.reason, equalTo("Delete ignores the parent param")); } public void testParseSkipSectionNoReason() throws Exception { parser = createParser(YamlXContent.yamlXContent, "version: \" - 0.90.2\"\n"); - Exception e = expectThrows(ParsingException.class, () -> SkipSection.parse(parser)); + Exception e = expectThrows(ParsingException.class, () -> SkipSection.parseInternal(parser)); assertThat(e.getMessage(), is("reason is mandatory within skip version section")); } public void testParseSkipSectionNoVersionNorFeature() throws Exception { parser = createParser(YamlXContent.yamlXContent, "reason: Delete ignores the parent param\n"); - Exception e = expectThrows(ParsingException.class, () -> SkipSection.parse(parser)); - assertThat(e.getMessage(), is("version, features or os is mandatory within skip section")); + Exception e = expectThrows(ParsingException.class, () -> SkipSection.parseInternal(parser)); + assertThat(e.getMessage(), is("at least one criteria (version, test features, os) is mandatory within a skip section")); } public void testParseSkipSectionOsNoVersion() throws Exception { @@ -155,13 +276,12 @@ public void testParseSkipSectionOsNoVersion() throws Exception { reason: memory accounting broken, see gh#xyz """); - SkipSection skipSection = SkipSection.parse(parser); - assertThat(skipSection, notNullValue()); - assertThat(skipSection.isVersionCheck(), equalTo(false)); - assertThat(skipSection.getFeatures().size(), equalTo(2)); - assertThat(skipSection.getOperatingSystems().size(), equalTo(1)); - assertThat(skipSection.getOperatingSystems().get(0), equalTo("debian-9")); - assertThat(skipSection.getReason(), is("memory accounting broken, see gh#xyz")); + var skipSectionBuilder = SkipSection.parseInternal(parser); + assertThat(skipSectionBuilder, notNullValue()); + assertThat(skipSectionBuilder.version, emptyOrNullString()); + assertThat(skipSectionBuilder.testFeatures, hasSize(2)); + assertThat(skipSectionBuilder.operatingSystems, contains("debian-9")); + assertThat(skipSectionBuilder.reason, is("memory accounting broken, see gh#xyz")); } public void testParseSkipSectionOsListNoVersion() throws Exception { @@ -171,14 +291,12 @@ public void testParseSkipSectionOsListNoVersion() throws Exception { reason: see gh#xyz """); - SkipSection skipSection = SkipSection.parse(parser); - assertThat(skipSection, notNullValue()); - assertThat(skipSection.isVersionCheck(), equalTo(false)); - assertThat(skipSection.getOperatingSystems().size(), equalTo(3)); - assertThat(skipSection.getOperatingSystems().get(0), equalTo("debian-9")); - assertThat(skipSection.getOperatingSystems().get(1), equalTo("windows-95")); - assertThat(skipSection.getOperatingSystems().get(2), equalTo("ms-dos")); - assertThat(skipSection.getReason(), is("see gh#xyz")); + var skipSectionBuilder = SkipSection.parseInternal(parser); + assertThat(skipSectionBuilder, notNullValue()); + assertThat(skipSectionBuilder.version, emptyOrNullString()); + assertThat(skipSectionBuilder.testFeatures, hasSize(1)); + assertThat(skipSectionBuilder.operatingSystems, containsInAnyOrder("debian-9", "windows-95", "ms-dos")); + assertThat(skipSectionBuilder.reason, is("see gh#xyz")); } public void testParseSkipSectionOsNoFeatureNoVersion() throws Exception { @@ -187,20 +305,7 @@ public void testParseSkipSectionOsNoFeatureNoVersion() throws Exception { reason: memory accounting broken, see gh#xyz """); - Exception e = expectThrows(ParsingException.class, () -> SkipSection.parse(parser)); + Exception e = expectThrows(ParsingException.class, () -> SkipSection.parseInternal(parser)); assertThat(e.getMessage(), is("if os is specified, feature skip_os must be set")); } - - public void testParseSkipSectionWithThreeDigitVersion() throws Exception { - parser = createParser(YamlXContent.yamlXContent, """ - version: " - 8.2.999" - features: regex - reason: Now you have two problems"""); - - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> SkipSection.parse(parser)); - assertThat( - e.getMessage(), - containsString("illegal revision version format - only one or two digit numbers are supported but found 999") - ); - } } diff --git a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/TeardownSectionTests.java b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/TeardownSectionTests.java index 52fbb3838ac9e..2c6b4f5be12de 100644 --- a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/TeardownSectionTests.java +++ b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/TeardownSectionTests.java @@ -8,7 +8,6 @@ package org.elasticsearch.test.rest.yaml.section; -import org.elasticsearch.Version; import org.elasticsearch.xcontent.yaml.YamlXContent; import static org.hamcrest.Matchers.equalTo; @@ -64,8 +63,6 @@ public void testParseWithSkip() throws Exception { TeardownSection section = TeardownSection.parse(parser); assertThat(section, notNullValue()); assertThat(section.getSkipSection().isEmpty(), equalTo(false)); - assertThat(section.getSkipSection().getLowerVersion(), equalTo(Version.fromString("6.0.0"))); - assertThat(section.getSkipSection().getUpperVersion(), equalTo(Version.fromString("6.3.0"))); assertThat(section.getSkipSection().getReason(), equalTo("there is a reason")); assertThat(section.getDoSections().size(), equalTo(2)); assertThat(((DoSection) section.getDoSections().get(0)).getApiCallSection().getApi(), equalTo("delete")); diff --git a/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/VersionRangeTests.java b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/VersionRangeTests.java new file mode 100644 index 0000000000000..ba20cf51b8299 --- /dev/null +++ b/test/yaml-rest-runner/src/test/java/org/elasticsearch/test/rest/yaml/section/VersionRangeTests.java @@ -0,0 +1,73 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.test.rest.yaml.section; + +import org.elasticsearch.Version; +import org.elasticsearch.core.Strings; +import org.elasticsearch.test.VersionUtils; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.notNullValue; + +public class VersionRangeTests extends AbstractClientYamlTestFragmentParserTestCase { + + public void testParseVersionNoLowerBound() { + Version version = VersionUtils.randomVersion(random()); + String versionRangeString = Strings.format(" - %s", version); + + var versionRange = VersionRange.parseVersionRanges(versionRangeString); + assertThat(versionRange, notNullValue()); + assertThat(versionRange, hasSize(1)); + assertThat(versionRange.get(0).lower(), equalTo(VersionUtils.getFirstVersion())); + assertThat(versionRange.get(0).upper(), equalTo(version)); + } + + public void testParseVersionNoUpperBound() { + Version version = VersionUtils.randomVersion(random()); + String versionRangeString = Strings.format("%s - ", version); + + var versionRange = VersionRange.parseVersionRanges(versionRangeString); + assertThat(versionRange, notNullValue()); + assertThat(versionRange, hasSize(1)); + assertThat(versionRange.get(0).lower(), equalTo(version)); + assertThat(versionRange.get(0).upper(), equalTo(Version.CURRENT)); + } + + public void testParseAllVersions() { + String versionRangeString = " all "; + + var versionRange = VersionRange.parseVersionRanges(versionRangeString); + assertThat(versionRange, notNullValue()); + assertThat(versionRange, hasSize(1)); + assertThat(versionRange.get(0).lower(), equalTo(VersionUtils.getFirstVersion())); + assertThat(versionRange.get(0).upper(), equalTo(Version.CURRENT)); + } + + public void testParseMultipleRanges() { + String versionRangeString = "6.0.0 - 6.1.0, 7.1.0 - 7.5.0"; + + var versionRange = VersionRange.parseVersionRanges(versionRangeString); + assertThat(versionRange, notNullValue()); + assertThat(versionRange, hasSize(2)); + assertThat(versionRange.get(0).lower(), equalTo(Version.fromString("6.0.0"))); + assertThat(versionRange.get(0).upper(), equalTo(Version.fromString("6.1.0"))); + assertThat(versionRange.get(1).lower(), equalTo(Version.fromString("7.1.0"))); + assertThat(versionRange.get(1).upper(), equalTo(Version.fromString("7.5.0"))); + } + + public void testParseWithThreeDigitVersion() { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> VersionRange.parseVersionRanges(" - 8.2.999")); + assertThat( + e.getMessage(), + containsString("illegal revision version format - only one or two digit numbers are supported but found 999") + ); + } +} diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsAggregationBuilders.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsAggregationBuilders.java deleted file mode 100644 index 123ed1a2f8a10..0000000000000 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsAggregationBuilders.java +++ /dev/null @@ -1,21 +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.analytics; - -import org.elasticsearch.xpack.analytics.cumulativecardinality.CumulativeCardinalityPipelineAggregationBuilder; -import org.elasticsearch.xpack.analytics.stringstats.StringStatsAggregationBuilder; - -public class AnalyticsAggregationBuilders { - - public static CumulativeCardinalityPipelineAggregationBuilder cumulativeCardinality(String name, String bucketsPath) { - return new CumulativeCardinalityPipelineAggregationBuilder(name, bucketsPath); - } - - public static StringStatsAggregationBuilder stringStats(String name) { - return new StringStatsAggregationBuilder(name); - } -} diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/AbstractHistoBackedHDRPercentilesAggregator.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/AbstractHistoBackedHDRPercentilesAggregator.java index c79ef9a344b67..295f44cd3c523 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/AbstractHistoBackedHDRPercentilesAggregator.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/AbstractHistoBackedHDRPercentilesAggregator.java @@ -109,8 +109,7 @@ protected DoubleHistogram getState(long bucketOrd) { if (bucketOrd >= states.size()) { return null; } - final DoubleHistogram state = states.get(bucketOrd); - return state; + return states.get(bucketOrd); } @Override diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedHDRPercentileRanksAggregator.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedHDRPercentileRanksAggregator.java index 209a06f1c155c..d50ebc6a46a09 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedHDRPercentileRanksAggregator.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedHDRPercentileRanksAggregator.java @@ -58,7 +58,7 @@ public double metric(String name, long bucketOrd) { if (state == null) { return Double.NaN; } else { - return InternalHDRPercentileRanks.percentileRank(state, Double.valueOf(name)); + return InternalHDRPercentileRanks.percentileRank(state, Double.parseDouble(name)); } } } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedTDigestPercentileRanksAggregator.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedTDigestPercentileRanksAggregator.java index c20c166bd552b..4ffb3c0c95e91 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedTDigestPercentileRanksAggregator.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedTDigestPercentileRanksAggregator.java @@ -57,7 +57,7 @@ public double metric(String name, long bucketOrd) { if (state == null) { return Double.NaN; } else { - return InternalTDigestPercentileRanks.percentileRank(state, Double.valueOf(name)); + return InternalTDigestPercentileRanks.percentileRank(state, Double.parseDouble(name)); } } } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/support/AnalyticsValuesSourceType.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/support/AnalyticsValuesSourceType.java index f66008bcc932c..4279911771dee 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/support/AnalyticsValuesSourceType.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/support/AnalyticsValuesSourceType.java @@ -55,10 +55,6 @@ public ValuesSource replaceMissing( } }; - public static ValuesSourceType fromString(String name) { - return valueOf(name.trim().toUpperCase(Locale.ROOT)); - } - public String value() { return name().toLowerCase(Locale.ROOT); } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityPipelineAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityPipelineAggregationBuilder.java index 5845da9f6e6c5..a4b978e64da9f 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityPipelineAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/cumulativecardinality/CumulativeCardinalityPipelineAggregationBuilder.java @@ -29,9 +29,11 @@ public class CumulativeCardinalityPipelineAggregationBuilder extends AbstractPip public static final String NAME = "cumulative_cardinality"; public static final ConstructingObjectParser PARSER = - new ConstructingObjectParser<>(NAME, false, (args, name) -> { - return new CumulativeCardinalityPipelineAggregationBuilder(name, (String) args[0]); - }); + new ConstructingObjectParser<>( + NAME, + false, + (args, name) -> new CumulativeCardinalityPipelineAggregationBuilder(name, (String) args[0]) + ); static { PARSER.declareString(constructorArg(), BUCKETS_PATH_FIELD); PARSER.declareString(CumulativeCardinalityPipelineAggregationBuilder::format, FORMAT); diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/cumulativecardinality/InternalSimpleLongValue.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/cumulativecardinality/InternalSimpleLongValue.java index 0b15184e54ba7..a1c775c29da0e 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/cumulativecardinality/InternalSimpleLongValue.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/cumulativecardinality/InternalSimpleLongValue.java @@ -53,14 +53,6 @@ public double value() { return value; } - public long getValue() { - return value; - } - - DocValueFormat formatter() { - return format; - } - @Override public InternalSimpleLongValue reduce(List aggregations, AggregationReduceContext reduceContext) { throw new UnsupportedOperationException("Not supported"); diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/movingPercentiles/MovingPercentilesPipelineAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/movingPercentiles/MovingPercentilesPipelineAggregationBuilder.java index 72688dafe3721..76ee8272fe345 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/movingPercentiles/MovingPercentilesPipelineAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/movingPercentiles/MovingPercentilesPipelineAggregationBuilder.java @@ -29,9 +29,11 @@ public class MovingPercentilesPipelineAggregationBuilder extends AbstractPipelin private static final ParseField SHIFT = new ParseField("shift"); public static final ConstructingObjectParser PARSER = - new ConstructingObjectParser<>(NAME, false, (args, name) -> { - return new MovingPercentilesPipelineAggregationBuilder(name, (String) args[0], (int) args[1]); - }); + new ConstructingObjectParser<>( + NAME, + false, + (args, name) -> new MovingPercentilesPipelineAggregationBuilder(name, (String) args[0], (int) args[1]) + ); static { PARSER.declareString(constructorArg(), BUCKETS_PATH_FIELD); PARSER.declareInt(constructorArg(), WINDOW); diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/movingPercentiles/MovingPercentilesPipelineAggregator.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/movingPercentiles/MovingPercentilesPipelineAggregator.java index 90f22340edb0d..3dc364b1ec131 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/movingPercentiles/MovingPercentilesPipelineAggregator.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/movingPercentiles/MovingPercentilesPipelineAggregator.java @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; public class MovingPercentilesPipelineAggregator extends PipelineAggregator { @@ -41,7 +42,6 @@ public class MovingPercentilesPipelineAggregator extends PipelineAggregator { @Override public InternalAggregation reduce(InternalAggregation aggregation, AggregationReduceContext reduceContext) { - @SuppressWarnings("unchecked") InternalMultiBucketAggregation histo = (InternalMultiBucketAggregation) aggregation; List buckets = histo.getBuckets(); HistogramFactory factory = (HistogramFactory) histo; @@ -74,7 +74,7 @@ private void reduceTDigest( List values = buckets.stream() .map(b -> resolveTDigestBucketValue(histo, b, bucketsPaths()[0])) - .filter(v -> v != null) + .filter(Objects::nonNull) .toList(); int index = 0; @@ -124,7 +124,7 @@ private void reduceHDR( List values = buckets.stream() .map(b -> resolveHDRBucketValue(histo, b, bucketsPaths()[0])) - .filter(v -> v != null) + .filter(Objects::nonNull) .toList(); int index = 0; @@ -257,10 +257,7 @@ private static int clamp(int index, int length) { if (index < 0) { return 0; } - if (index > length) { - return length; - } - return index; + return Math.min(index, length); } // TODO: replace this with the PercentilesConfig that's used by the percentiles builder. diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java index 0d3931e9eb8c2..6307cfa5b3674 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java @@ -189,13 +189,6 @@ public MultiTermsAggregationBuilder terms(List ter return this; } - /** - * Gets the field to use for this aggregation. - */ - public List terms() { - return terms; - } - @Override protected AggregationBuilder shallowCopy(AggregatorFactories.Builder factoriesBuilder, Map metadata) { return new MultiTermsAggregationBuilder(this, factoriesBuilder, metadata); @@ -215,13 +208,6 @@ protected final void doWriteTo(StreamOutput out) throws IOException { out.writeBoolean(showTermDocCountError); } - /** - * Get whether doc count error will be return for individual terms - */ - public boolean showTermDocCountError() { - return showTermDocCountError; - } - /** * Set whether doc count error will be return for individual terms */ @@ -242,13 +228,6 @@ public MultiTermsAggregationBuilder size(int size) { return this; } - /** - * Returns the number of term buckets currently configured - */ - public int size() { - return bucketCountThresholds.getRequiredSize(); - } - /** * Sets the shard_size - indicating the number of term buckets each shard * will return to the coordinating node (the node that coordinates the @@ -263,13 +242,6 @@ public MultiTermsAggregationBuilder shardSize(int shardSize) { return this; } - /** - * Returns the number of term buckets per shard that are currently configured - */ - public int shardSize() { - return bucketCountThresholds.getShardSize(); - } - /** * Set the minimum document count terms should have in order to appear in * the response. @@ -284,13 +256,6 @@ public MultiTermsAggregationBuilder minDocCount(long minDocCount) { return this; } - /** - * Returns the minimum document count required per term - */ - public long minDocCount() { - return bucketCountThresholds.getMinDocCount(); - } - /** * Set the minimum document count terms should have on the shard in order to * appear in the response. @@ -305,13 +270,6 @@ public MultiTermsAggregationBuilder shardMinDocCount(long shardMinDocCount) { return this; } - /** - * Returns the minimum document count required per term, per shard - */ - public long shardMinDocCount() { - return bucketCountThresholds.getShardMinDocCount(); - } - /** * Set a new order on this builder and return the builder so that calls * can be chained. A tie-breaker may be added to avoid non-deterministic ordering. @@ -341,13 +299,6 @@ public MultiTermsAggregationBuilder order(List orders) { return this; } - /** - * Gets the order in which the buckets will be returned. - */ - public BucketOrder order() { - return order; - } - /** * Expert: set the collection mode. */ @@ -359,13 +310,6 @@ public MultiTermsAggregationBuilder collectMode(Aggregator.SubAggCollectionMode return this; } - /** - * Expert: get the collection mode. - */ - public Aggregator.SubAggCollectionMode collectMode() { - return collectMode; - } - @Override protected final MultiTermsAggregationFactory doBuild( AggregationContext context, diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/stringstats/InternalStringStats.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/stringstats/InternalStringStats.java index 1663a93a52235..20bd4b3380180 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/stringstats/InternalStringStats.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/stringstats/InternalStringStats.java @@ -214,7 +214,7 @@ public InternalStringStats reduce(List aggregations, Aggreg minLength = Math.min(minLength, stats.getMinLength()); maxLength = Math.max(maxLength, stats.getMaxLength()); totalLength += stats.totalLength; - stats.charOccurrences.forEach((k, v) -> occurs.merge(k, v, (oldValue, newValue) -> oldValue + newValue)); + stats.charOccurrences.forEach((k, v) -> occurs.merge(k, v, Long::sum)); } return new InternalStringStats(name, count, totalLength, minLength, maxLength, occurs, showDistribution, format, getMetadata()); diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/InternalTopMetrics.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/InternalTopMetrics.java index 0194e39f47d44..4135d713f91ad 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/InternalTopMetrics.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/InternalTopMetrics.java @@ -116,7 +116,7 @@ public InternalTopMetrics reduce(List aggregations, Aggrega return this; } List merged = new ArrayList<>(size); - PriorityQueue queue = new PriorityQueue(aggregations.size()) { + PriorityQueue queue = new PriorityQueue<>(aggregations.size()) { @Override protected boolean lessThan(ReduceState lhs, ReduceState rhs) { return sortOrder.reverseMul() * lhs.sortValue().compareTo(rhs.sortValue()) < 0; diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregationBuilder.java index 15838a77a96ad..ba9dc7ab7eed9 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregationBuilder.java @@ -233,7 +233,7 @@ List getMetricFields() { @Override public Optional> getOutputFieldNames() { - return Optional.of(metricFields.stream().map(mf -> mf.getFieldName()).collect(Collectors.toSet())); + return Optional.of(metricFields.stream().map(MultiValuesSourceFieldConfig::getFieldName).collect(Collectors.toSet())); } @Override diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregator.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregator.java index adfb41f83cc6c..3337b6d239413 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregator.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregator.java @@ -154,8 +154,8 @@ static class Metrics implements BucketedSort.ExtraData, Releasable { } boolean needsScores() { - for (int i = 0; i < values.length; i++) { - if (values[i].needsScores()) { + for (MetricValues value : values) { + if (value.needsScores()) { return true; } } @@ -174,8 +174,8 @@ boolean needsScores() { BucketedSort.ResultBuilder resultBuilder(DocValueFormat sortFormat) { return (index, sortValue) -> { List result = new ArrayList<>(values.length); - for (int i = 0; i < values.length; i++) { - result.add(values[i].metricValue(index)); + for (MetricValues value : values) { + result.add(value.metricValue(index)); } return new InternalTopMetrics.TopMetric(sortFormat, sortValue, result); }; @@ -187,8 +187,8 @@ List names() { @Override public void swap(long lhs, long rhs) { - for (int i = 0; i < values.length; i++) { - values[i].swap(lhs, rhs); + for (MetricValues value : values) { + value.swap(lhs, rhs); } } @@ -199,8 +199,8 @@ public Loader loader(LeafReaderContext ctx) throws IOException { loaders[i] = values[i].loader(ctx); } return (index, doc) -> { - for (int i = 0; i < loaders.length; i++) { - loaders[i].loadFromDoc(index, doc); + for (Loader loader : loaders) { + loader.loadFromDoc(index, doc); } }; } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/ttest/TTestStatsBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/ttest/TTestStatsBuilder.java index 8e85d16e01f9f..9c4fcb3e675c3 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/ttest/TTestStatsBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/ttest/TTestStatsBuilder.java @@ -42,10 +42,6 @@ public TTestStats get(long bucket) { return new TTestStats(counts.get(bucket), sums.get(bucket), sumOfSqrs.get(bucket)); } - public long build(long bucket) { - return counts.get(bucket); - } - public long getSize() { return counts.size(); } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/ttest/TTestType.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/ttest/TTestType.java index 7b901ea792601..0f718d982b545 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/ttest/TTestType.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/ttest/TTestType.java @@ -21,8 +21,4 @@ public static TTestType resolve(String name) { return TTestType.valueOf(name.toUpperCase(Locale.ROOT)); } - public String value() { - return name().toLowerCase(Locale.ROOT); - } - } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BaseBulkUpdateApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BaseBulkUpdateApiKeyRequest.java index 81c8479c47285..34b249d7a8233 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BaseBulkUpdateApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BaseBulkUpdateApiKeyRequest.java @@ -11,6 +11,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import java.io.IOException; @@ -27,9 +28,10 @@ public abstract class BaseBulkUpdateApiKeyRequest extends BaseUpdateApiKeyReques public BaseBulkUpdateApiKeyRequest( final List ids, @Nullable final List roleDescriptors, - @Nullable final Map metadata + @Nullable final Map metadata, + @Nullable final TimeValue expiration ) { - super(roleDescriptors, metadata); + super(roleDescriptors, metadata, expiration); this.ids = Objects.requireNonNull(ids, "API key IDs must not be null"); } @@ -56,4 +58,21 @@ public void writeTo(StreamOutput out) throws IOException { public List getIds() { return ids; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass() || super.equals(o)) return false; + + BaseBulkUpdateApiKeyRequest that = (BaseBulkUpdateApiKeyRequest) o; + return Objects.equals(getIds(), that.getIds()) + && Objects.equals(metadata, that.metadata) + && Objects.equals(expiration, that.expiration) + && Objects.equals(roleDescriptors, that.roleDescriptors); + } + + @Override + public int hashCode() { + return Objects.hash(getIds(), expiration, metadata, roleDescriptors); + } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BaseSingleUpdateApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BaseSingleUpdateApiKeyRequest.java index 7d89c0dd39b0c..725a9fb197b07 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BaseSingleUpdateApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BaseSingleUpdateApiKeyRequest.java @@ -10,6 +10,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import java.io.IOException; @@ -24,9 +25,10 @@ public abstract class BaseSingleUpdateApiKeyRequest extends BaseUpdateApiKeyRequ public BaseSingleUpdateApiKeyRequest( @Nullable final List roleDescriptors, @Nullable final Map metadata, + @Nullable final TimeValue expiration, String id ) { - super(roleDescriptors, metadata); + super(roleDescriptors, metadata, expiration); this.id = Objects.requireNonNull(id, "API key ID must not be null"); } @@ -44,4 +46,21 @@ public void writeTo(StreamOutput out) throws IOException { public String getId() { return id; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass() || super.equals(o)) return false; + + BaseSingleUpdateApiKeyRequest that = (BaseSingleUpdateApiKeyRequest) o; + return Objects.equals(getId(), that.getId()) + && Objects.equals(metadata, that.metadata) + && Objects.equals(expiration, that.expiration) + && Objects.equals(roleDescriptors, that.roleDescriptors); + } + + @Override + public int hashCode() { + return Objects.hash(getId(), expiration, metadata, roleDescriptors); + } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BaseUpdateApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BaseUpdateApiKeyRequest.java index b06f8868c53d1..a1acb6c9581f5 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BaseUpdateApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BaseUpdateApiKeyRequest.java @@ -7,11 +7,13 @@ package org.elasticsearch.xpack.core.security.action.apikey; +import org.elasticsearch.TransportVersions; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.xpack.core.security.action.role.RoleDescriptorRequestValidator; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.support.MetadataUtils; @@ -28,16 +30,28 @@ public abstract class BaseUpdateApiKeyRequest extends ActionRequest { protected final List roleDescriptors; @Nullable protected final Map metadata; + @Nullable + protected final TimeValue expiration; - public BaseUpdateApiKeyRequest(@Nullable final List roleDescriptors, @Nullable final Map metadata) { + public BaseUpdateApiKeyRequest( + @Nullable final List roleDescriptors, + @Nullable final Map metadata, + @Nullable final TimeValue expiration + ) { this.roleDescriptors = roleDescriptors; this.metadata = metadata; + this.expiration = expiration; } public BaseUpdateApiKeyRequest(StreamInput in) throws IOException { super(in); this.roleDescriptors = in.readOptionalCollectionAsList(RoleDescriptor::new); this.metadata = in.readMap(); + if (in.getTransportVersion().onOrAfter(TransportVersions.UPDATE_API_KEY_EXPIRATION_TIME_ADDED)) { + expiration = in.readOptionalTimeValue(); + } else { + expiration = null; + } } public Map getMetadata() { @@ -48,6 +62,10 @@ public List getRoleDescriptors() { return roleDescriptors; } + public TimeValue getExpiration() { + return expiration; + } + public abstract ApiKey.Type getType(); @Override @@ -72,5 +90,8 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeOptionalCollection(roleDescriptors); out.writeGenericMap(metadata); + if (out.getTransportVersion().onOrAfter(TransportVersions.UPDATE_API_KEY_EXPIRATION_TIME_ADDED)) { + out.writeOptionalTimeValue(expiration); + } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BulkUpdateApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BulkUpdateApiKeyRequest.java index d4712abd2cfe2..f915781c6211a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BulkUpdateApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/BulkUpdateApiKeyRequest.java @@ -9,6 +9,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import java.io.IOException; @@ -19,19 +20,25 @@ public final class BulkUpdateApiKeyRequest extends BaseBulkUpdateApiKeyRequest { public static BulkUpdateApiKeyRequest usingApiKeyIds(String... ids) { - return new BulkUpdateApiKeyRequest(Arrays.stream(ids).toList(), null, null); + return new BulkUpdateApiKeyRequest(Arrays.stream(ids).toList(), null, null, null); } public static BulkUpdateApiKeyRequest wrap(final UpdateApiKeyRequest request) { - return new BulkUpdateApiKeyRequest(List.of(request.getId()), request.getRoleDescriptors(), request.getMetadata()); + return new BulkUpdateApiKeyRequest( + List.of(request.getId()), + request.getRoleDescriptors(), + request.getMetadata(), + request.getExpiration() + ); } public BulkUpdateApiKeyRequest( final List ids, @Nullable final List roleDescriptors, - @Nullable final Map metadata + @Nullable final Map metadata, + @Nullable final TimeValue expiration ) { - super(ids, roleDescriptors, metadata); + super(ids, roleDescriptors, metadata, expiration); } public BulkUpdateApiKeyRequest(StreamInput in) throws IOException { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateApiKeyRequest.java index 456aa4d636335..c5c8bcc4fc87a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateApiKeyRequest.java @@ -9,6 +9,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import java.io.IOException; @@ -17,15 +18,16 @@ public final class UpdateApiKeyRequest extends BaseSingleUpdateApiKeyRequest { public static UpdateApiKeyRequest usingApiKeyId(final String id) { - return new UpdateApiKeyRequest(id, null, null); + return new UpdateApiKeyRequest(id, null, null, null); } public UpdateApiKeyRequest( final String id, @Nullable final List roleDescriptors, - @Nullable final Map metadata + @Nullable final Map metadata, + @Nullable final TimeValue expiration ) { - super(roleDescriptors, metadata, id); + super(roleDescriptors, metadata, expiration, id); } public UpdateApiKeyRequest(StreamInput in) throws IOException { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateCrossClusterApiKeyRequest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateCrossClusterApiKeyRequest.java index ff6bb7da13660..184ce2c521ce0 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateCrossClusterApiKeyRequest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateCrossClusterApiKeyRequest.java @@ -10,6 +10,7 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.TimeValue; import java.io.IOException; import java.util.List; @@ -22,9 +23,10 @@ public final class UpdateCrossClusterApiKeyRequest extends BaseSingleUpdateApiKe public UpdateCrossClusterApiKeyRequest( final String id, @Nullable CrossClusterApiKeyRoleDescriptorBuilder roleDescriptorBuilder, - @Nullable final Map metadata + @Nullable final Map metadata, + @Nullable TimeValue expiration ) { - super(roleDescriptorBuilder == null ? null : List.of(roleDescriptorBuilder.build()), metadata, id); + super(roleDescriptorBuilder == null ? null : List.of(roleDescriptorBuilder.build()), metadata, expiration, id); } public UpdateCrossClusterApiKeyRequest(StreamInput in) throws IOException { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/ApiKeyTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/ApiKeyTests.java index 9e357915186a5..02bce50ed3483 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/ApiKeyTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/ApiKeyTests.java @@ -9,6 +9,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.XContentTestUtils; import org.elasticsearch.xcontent.ToXContent; @@ -152,6 +153,10 @@ public static Map randomMetadata() { return randomMetadata == null ? new HashMap<>() : new HashMap<>(randomMetadata); } + public static TimeValue randomFutureExpirationTime() { + return TimeValue.parseTimeValue(randomTimeValue(10, 20, "d", "h", "s", "m"), "expiration"); + } + public static ApiKey randomApiKeyInstance() { final String name = randomAlphaOfLengthBetween(4, 10); final String id = randomAlphaOfLength(20); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/BulkUpdateApiKeyRequestSerializationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/BulkUpdateApiKeyRequestSerializationTests.java new file mode 100644 index 0000000000000..08e5266181298 --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/BulkUpdateApiKeyRequestSerializationTests.java @@ -0,0 +1,71 @@ +/* + * 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.core.security.action.apikey; + +import org.elasticsearch.TransportVersions; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.core.TimeValue; +import org.elasticsearch.test.AbstractWireSerializingTestCase; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.nullValue; + +public class BulkUpdateApiKeyRequestSerializationTests extends AbstractWireSerializingTestCase { + public void testSerializationBackwardsCompatibility() throws IOException { + BulkUpdateApiKeyRequest testInstance = createTestInstance(); + BulkUpdateApiKeyRequest deserializedInstance = copyInstance(testInstance, TransportVersions.V_8_500_064); + try { + // Transport is on a version before expiration was introduced, so should always be null + assertThat(deserializedInstance.getExpiration(), nullValue()); + } finally { + dispose(deserializedInstance); + } + } + + @Override + protected BulkUpdateApiKeyRequest createTestInstance() { + final boolean roleDescriptorsPresent = randomBoolean(); + final List descriptorList; + if (roleDescriptorsPresent == false) { + descriptorList = null; + } else { + final int numDescriptors = randomIntBetween(0, 4); + descriptorList = new ArrayList<>(); + for (int i = 0; i < numDescriptors; i++) { + descriptorList.add(new RoleDescriptor("role_" + i, new String[] { "all" }, null, null)); + } + } + + final var ids = randomList(randomInt(5), () -> randomAlphaOfLength(10)); + final var metadata = ApiKeyTests.randomMetadata(); + final TimeValue expiration = ApiKeyTests.randomFutureExpirationTime(); + return new BulkUpdateApiKeyRequest(ids, descriptorList, metadata, expiration); + } + + @Override + protected Writeable.Reader instanceReader() { + return BulkUpdateApiKeyRequest::new; + } + + @Override + protected BulkUpdateApiKeyRequest mutateInstance(BulkUpdateApiKeyRequest instance) throws IOException { + Map metadata = ApiKeyTests.randomMetadata(); + long days = randomValueOtherThan(instance.getExpiration().days(), () -> ApiKeyTests.randomFutureExpirationTime().getDays()); + return new BulkUpdateApiKeyRequest( + instance.getIds(), + instance.getRoleDescriptors(), + metadata, + TimeValue.parseTimeValue(days + "d", null, "expiration") + ); + } +} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/BulkUpdateApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/BulkUpdateApiKeyRequestTests.java index 8a0f384daedaa..583b336b3f6eb 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/BulkUpdateApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/BulkUpdateApiKeyRequestTests.java @@ -8,13 +8,10 @@ package org.elasticsearch.xpack.core.security.action.apikey; import org.elasticsearch.action.ActionRequestValidationException; -import org.elasticsearch.common.io.stream.BytesStreamOutput; -import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; -import java.io.IOException; -import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -23,42 +20,13 @@ import static org.hamcrest.Matchers.equalTo; public class BulkUpdateApiKeyRequestTests extends ESTestCase { - - public void testSerialization() throws IOException { - final boolean roleDescriptorsPresent = randomBoolean(); - final List descriptorList; - if (roleDescriptorsPresent == false) { - descriptorList = null; - } else { - final int numDescriptors = randomIntBetween(0, 4); - descriptorList = new ArrayList<>(); - for (int i = 0; i < numDescriptors; i++) { - descriptorList.add(new RoleDescriptor("role_" + i, new String[] { "all" }, null, null)); - } - } - - final List ids = randomList(1, 5, () -> randomAlphaOfLength(10)); - final Map metadata = ApiKeyTests.randomMetadata(); - final var request = new BulkUpdateApiKeyRequest(ids, descriptorList, metadata); - - try (BytesStreamOutput out = new BytesStreamOutput()) { - request.writeTo(out); - try (StreamInput in = out.bytes().streamInput()) { - final var serialized = new BulkUpdateApiKeyRequest(in); - assertEquals(ids, serialized.getIds()); - assertEquals(descriptorList, serialized.getRoleDescriptors()); - assertEquals(metadata, request.getMetadata()); - } - } - } - public void testNullValuesValidForNonIds() { final var request = BulkUpdateApiKeyRequest.usingApiKeyIds("id"); assertNull(request.validate()); } public void testEmptyIdsNotValid() { - final var request = new BulkUpdateApiKeyRequest(List.of(), null, null); + final var request = new BulkUpdateApiKeyRequest(List.of(), null, null, null); final ActionRequestValidationException ve = request.validate(); assertNotNull(ve); assertThat(ve.validationErrors().size(), equalTo(1)); @@ -68,10 +36,12 @@ public void testEmptyIdsNotValid() { public void testMetadataKeyValidation() { final var reservedKey = "_" + randomAlphaOfLengthBetween(0, 10); final var metadataValue = randomAlphaOfLengthBetween(1, 10); + final TimeValue expiration = ApiKeyTests.randomFutureExpirationTime(); final var request = new BulkUpdateApiKeyRequest( randomList(1, 5, () -> randomAlphaOfLength(10)), null, - Map.of(reservedKey, metadataValue) + Map.of(reservedKey, metadataValue), + expiration ); final ActionRequestValidationException ve = request.validate(); assertNotNull(ve); @@ -103,6 +73,7 @@ public void testRoleDescriptorValidation() { new RoleDescriptor.Restriction(unknownWorkflows) ) ), + null, null ); final ActionRequestValidationException ve = request.validate(); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateApiKeyRequestSerializationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateApiKeyRequestSerializationTests.java new file mode 100644 index 0000000000000..be1e69d4d30e8 --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateApiKeyRequestSerializationTests.java @@ -0,0 +1,72 @@ +/* + * 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.core.security.action.apikey; + +import org.elasticsearch.TransportVersions; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.core.TimeValue; +import org.elasticsearch.test.AbstractWireSerializingTestCase; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.nullValue; + +public class UpdateApiKeyRequestSerializationTests extends AbstractWireSerializingTestCase { + public void testSerializationBackwardsCompatibility() throws IOException { + UpdateApiKeyRequest testInstance = createTestInstance(); + UpdateApiKeyRequest deserializedInstance = copyInstance(testInstance, TransportVersions.V_8_500_064); + try { + // Transport is on a version before expiration was introduced, so should always be null + assertThat(deserializedInstance.getExpiration(), nullValue()); + } finally { + dispose(deserializedInstance); + } + } + + @Override + protected UpdateApiKeyRequest createTestInstance() { + final boolean roleDescriptorsPresent = randomBoolean(); + final List descriptorList; + if (roleDescriptorsPresent == false) { + descriptorList = null; + } else { + final int numDescriptors = randomIntBetween(0, 4); + descriptorList = new ArrayList<>(); + for (int i = 0; i < numDescriptors; i++) { + descriptorList.add(new RoleDescriptor("role_" + i, new String[] { "all" }, null, null)); + } + } + + final var id = randomAlphaOfLength(10); + final var metadata = ApiKeyTests.randomMetadata(); + final TimeValue expiration = ApiKeyTests.randomFutureExpirationTime(); + return new UpdateApiKeyRequest(id, descriptorList, metadata, expiration); + } + + @Override + protected Writeable.Reader instanceReader() { + return UpdateApiKeyRequest::new; + } + + @Override + protected UpdateApiKeyRequest mutateInstance(UpdateApiKeyRequest instance) throws IOException { + Map metadata = ApiKeyTests.randomMetadata(); + long days = randomValueOtherThan(instance.getExpiration().days(), () -> ApiKeyTests.randomFutureExpirationTime().getDays()); + return new UpdateApiKeyRequest( + instance.getId(), + instance.getRoleDescriptors(), + metadata, + TimeValue.parseTimeValue(days + "d", null, "expiration") + ); + } + +} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateApiKeyRequestTests.java index cf4015f6fd4cc..7b85c71c7519f 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateApiKeyRequestTests.java @@ -8,13 +8,10 @@ package org.elasticsearch.xpack.core.security.action.apikey; import org.elasticsearch.action.ActionRequestValidationException; -import org.elasticsearch.common.io.stream.BytesStreamOutput; -import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.restriction.WorkflowResolver; -import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -22,49 +19,19 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.containsStringIgnoringCase; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; public class UpdateApiKeyRequestTests extends ESTestCase { public void testNullValuesValidForNonIds() { - final var request = new UpdateApiKeyRequest("id", null, null); + final var request = new UpdateApiKeyRequest("id", null, null, null); assertNull(request.validate()); } - public void testSerialization() throws IOException { - final boolean roleDescriptorsPresent = randomBoolean(); - final List descriptorList; - if (roleDescriptorsPresent == false) { - descriptorList = null; - } else { - final int numDescriptors = randomIntBetween(0, 4); - descriptorList = new ArrayList<>(); - for (int i = 0; i < numDescriptors; i++) { - descriptorList.add(new RoleDescriptor("role_" + i, new String[] { "all" }, null, null)); - } - } - - final var id = randomAlphaOfLength(10); - final var metadata = ApiKeyTests.randomMetadata(); - final var request = new UpdateApiKeyRequest(id, descriptorList, metadata); - assertThat(request.getType(), is(ApiKey.Type.REST)); - - try (BytesStreamOutput out = new BytesStreamOutput()) { - request.writeTo(out); - try (StreamInput in = out.bytes().streamInput()) { - final var serialized = new UpdateApiKeyRequest(in); - assertEquals(id, serialized.getId()); - assertEquals(descriptorList, serialized.getRoleDescriptors()); - assertEquals(metadata, serialized.getMetadata()); - assertEquals(request.getType(), serialized.getType()); - } - } - } - public void testMetadataKeyValidation() { final var reservedKey = "_" + randomAlphaOfLengthBetween(0, 10); final var metadataValue = randomAlphaOfLengthBetween(1, 10); - UpdateApiKeyRequest request = new UpdateApiKeyRequest(randomAlphaOfLength(10), null, Map.of(reservedKey, metadataValue)); + + UpdateApiKeyRequest request = new UpdateApiKeyRequest(randomAlphaOfLength(10), null, Map.of(reservedKey, metadataValue), null); final ActionRequestValidationException ve = request.validate(); assertNotNull(ve); assertThat(ve.validationErrors().size(), equalTo(1)); @@ -98,6 +65,7 @@ public void testRoleDescriptorValidation() { new RoleDescriptor.Restriction(workflows.toArray(String[]::new)) ) ), + null, null ); final ActionRequestValidationException ve1 = request1.validate(); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateCrossClusterApiKeyRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateCrossClusterApiKeyRequestTests.java index 89a6f5b650b5a..f9faa2731dcc0 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateCrossClusterApiKeyRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/UpdateCrossClusterApiKeyRequestTests.java @@ -10,6 +10,7 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.test.ESTestCase; import java.io.IOException; @@ -25,15 +26,15 @@ public class UpdateCrossClusterApiKeyRequestTests extends ESTestCase { public void testSerialization() throws IOException { final var metadata = ApiKeyTests.randomMetadata(); - + final TimeValue expiration = ApiKeyTests.randomFutureExpirationTime(); final CrossClusterApiKeyRoleDescriptorBuilder roleDescriptorBuilder; - if (metadata == null || randomBoolean()) { + if (randomBoolean()) { roleDescriptorBuilder = CrossClusterApiKeyRoleDescriptorBuilder.parse(randomCrossClusterApiKeyAccessField()); } else { roleDescriptorBuilder = null; } - final var request = new UpdateCrossClusterApiKeyRequest(randomAlphaOfLength(10), roleDescriptorBuilder, metadata); + final var request = new UpdateCrossClusterApiKeyRequest(randomAlphaOfLength(10), roleDescriptorBuilder, metadata, expiration); assertThat(request.getType(), is(ApiKey.Type.CROSS_CLUSTER)); assertThat(request.validate(), nullValue()); @@ -44,13 +45,14 @@ public void testSerialization() throws IOException { assertEquals(request.getId(), serialized.getId()); assertEquals(request.getRoleDescriptors(), serialized.getRoleDescriptors()); assertEquals(metadata, serialized.getMetadata()); + assertEquals(expiration, serialized.getExpiration()); assertEquals(request.getType(), serialized.getType()); } } } public void testNotEmptyUpdateValidation() { - final var request = new UpdateCrossClusterApiKeyRequest(randomAlphaOfLength(10), null, null); + final var request = new UpdateCrossClusterApiKeyRequest(randomAlphaOfLength(10), null, null, null); final ActionRequestValidationException ve = request.validate(); assertThat(ve, notNullValue()); assertThat(ve.validationErrors(), contains("must update either [access] or [metadata] for cross-cluster API keys")); @@ -59,7 +61,7 @@ public void testNotEmptyUpdateValidation() { public void testMetadataKeyValidation() { final var reservedKey = "_" + randomAlphaOfLengthBetween(0, 10); final var metadataValue = randomAlphaOfLengthBetween(1, 10); - final var request = new UpdateCrossClusterApiKeyRequest(randomAlphaOfLength(10), null, Map.of(reservedKey, metadataValue)); + final var request = new UpdateCrossClusterApiKeyRequest(randomAlphaOfLength(10), null, Map.of(reservedKey, metadataValue), null); final ActionRequestValidationException ve = request.validate(); assertThat(ve, notNullValue()); assertThat(ve.validationErrors(), contains("API key metadata keys may not start with [_]")); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java index 22e6a6f005919..66ed55fadb734 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java @@ -11,6 +11,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.transport.TransportRequest; +import org.elasticsearch.xpack.core.security.action.apikey.ApiKeyTests; import org.elasticsearch.xpack.core.security.action.apikey.BulkUpdateApiKeyRequest; import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyRequest; import org.elasticsearch.xpack.core.security.action.apikey.CreateCrossClusterApiKeyAction; @@ -88,7 +89,7 @@ public void testAuthenticationForBulkUpdateApiKeyAllowsAll() { .build(); final List apiKeyIds = randomList(1, 5, () -> randomAlphaOfLengthBetween(4, 7)); final Authentication authentication = AuthenticationTestHelper.builder().build(); - final TransportRequest bulkUpdateApiKeyRequest = new BulkUpdateApiKeyRequest(apiKeyIds, null, null); + final TransportRequest bulkUpdateApiKeyRequest = new BulkUpdateApiKeyRequest(apiKeyIds, null, null, null); assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/update", bulkUpdateApiKeyRequest, authentication)); } @@ -315,7 +316,8 @@ public void testCheckUpdateCrossClusterApiKeyRequestDenied() { final UpdateCrossClusterApiKeyRequest request = new UpdateCrossClusterApiKeyRequest( randomAlphaOfLengthBetween(4, 7), null, - Map.of() + Map.of(), + ApiKeyTests.randomFutureExpirationTime() ); assertFalse(clusterPermission.check(UpdateCrossClusterApiKeyAction.NAME, request, AuthenticationTestHelper.builder().build())); } diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanArrayBlock.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanArrayBlock.java index d7d0856963019..2cdb7f616e684 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanArrayBlock.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanArrayBlock.java @@ -50,6 +50,9 @@ private BooleanArrayBlock( ) { super(positionCount, firstValueIndexes, nulls, mvOrdering, blockFactory); this.vector = vector; + assert firstValueIndexes == null + ? vector.getPositionCount() == getPositionCount() + : firstValueIndexes[getPositionCount()] == vector.getPositionCount(); } @Override @@ -105,7 +108,7 @@ public BooleanBlock expand() { // The following line is correct because positions with multi-values are never null. int expandedPositionCount = vector.getPositionCount(); - long bitSetRamUsedEstimate = BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount); + long bitSetRamUsedEstimate = Math.max(nullsMask.size(), BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount)); blockFactory().adjustBreaker(bitSetRamUsedEstimate, false); BooleanArrayBlock expanded = new BooleanArrayBlock( diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanBigArrayBlock.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanBigArrayBlock.java index d75e988fe3a84..57863d2d89dd0 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanBigArrayBlock.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanBigArrayBlock.java @@ -51,6 +51,9 @@ private BooleanBigArrayBlock( ) { super(positionCount, firstValueIndexes, nulls, mvOrdering, blockFactory); this.vector = vector; + assert firstValueIndexes == null + ? vector.getPositionCount() == getPositionCount() + : firstValueIndexes[getPositionCount()] == vector.getPositionCount(); } @Override @@ -106,7 +109,7 @@ public BooleanBlock expand() { // The following line is correct because positions with multi-values are never null. int expandedPositionCount = vector.getPositionCount(); - long bitSetRamUsedEstimate = BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount); + long bitSetRamUsedEstimate = Math.max(nullsMask.size(), BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount)); blockFactory().adjustBreaker(bitSetRamUsedEstimate, false); BooleanBigArrayBlock expanded = new BooleanBigArrayBlock( diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BytesRefArrayBlock.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BytesRefArrayBlock.java index b236949e92018..492f99015ff03 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BytesRefArrayBlock.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BytesRefArrayBlock.java @@ -53,6 +53,9 @@ private BytesRefArrayBlock( ) { super(positionCount, firstValueIndexes, nulls, mvOrdering, blockFactory); this.vector = vector; + assert firstValueIndexes == null + ? vector.getPositionCount() == getPositionCount() + : firstValueIndexes[getPositionCount()] == vector.getPositionCount(); } @Override @@ -109,7 +112,7 @@ public BytesRefBlock expand() { // The following line is correct because positions with multi-values are never null. int expandedPositionCount = vector.getPositionCount(); - long bitSetRamUsedEstimate = BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount); + long bitSetRamUsedEstimate = Math.max(nullsMask.size(), BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount)); blockFactory().adjustBreaker(bitSetRamUsedEstimate, false); BytesRefArrayBlock expanded = new BytesRefArrayBlock( diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleArrayBlock.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleArrayBlock.java index 6089679904e48..58e85b4c05015 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleArrayBlock.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleArrayBlock.java @@ -50,6 +50,9 @@ private DoubleArrayBlock( ) { super(positionCount, firstValueIndexes, nulls, mvOrdering, blockFactory); this.vector = vector; + assert firstValueIndexes == null + ? vector.getPositionCount() == getPositionCount() + : firstValueIndexes[getPositionCount()] == vector.getPositionCount(); } @Override @@ -105,7 +108,7 @@ public DoubleBlock expand() { // The following line is correct because positions with multi-values are never null. int expandedPositionCount = vector.getPositionCount(); - long bitSetRamUsedEstimate = BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount); + long bitSetRamUsedEstimate = Math.max(nullsMask.size(), BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount)); blockFactory().adjustBreaker(bitSetRamUsedEstimate, false); DoubleArrayBlock expanded = new DoubleArrayBlock( diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleBigArrayBlock.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleBigArrayBlock.java index adc09d5755b53..d7b5b644b721a 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleBigArrayBlock.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleBigArrayBlock.java @@ -51,6 +51,9 @@ private DoubleBigArrayBlock( ) { super(positionCount, firstValueIndexes, nulls, mvOrdering, blockFactory); this.vector = vector; + assert firstValueIndexes == null + ? vector.getPositionCount() == getPositionCount() + : firstValueIndexes[getPositionCount()] == vector.getPositionCount(); } @Override @@ -106,7 +109,7 @@ public DoubleBlock expand() { // The following line is correct because positions with multi-values are never null. int expandedPositionCount = vector.getPositionCount(); - long bitSetRamUsedEstimate = BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount); + long bitSetRamUsedEstimate = Math.max(nullsMask.size(), BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount)); blockFactory().adjustBreaker(bitSetRamUsedEstimate, false); DoubleBigArrayBlock expanded = new DoubleBigArrayBlock( diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntArrayBlock.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntArrayBlock.java index 1e5ae7a3de448..b33a70525b8f7 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntArrayBlock.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntArrayBlock.java @@ -50,6 +50,9 @@ private IntArrayBlock( ) { super(positionCount, firstValueIndexes, nulls, mvOrdering, blockFactory); this.vector = vector; + assert firstValueIndexes == null + ? vector.getPositionCount() == getPositionCount() + : firstValueIndexes[getPositionCount()] == vector.getPositionCount(); } @Override @@ -105,7 +108,7 @@ public IntBlock expand() { // The following line is correct because positions with multi-values are never null. int expandedPositionCount = vector.getPositionCount(); - long bitSetRamUsedEstimate = BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount); + long bitSetRamUsedEstimate = Math.max(nullsMask.size(), BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount)); blockFactory().adjustBreaker(bitSetRamUsedEstimate, false); IntArrayBlock expanded = new IntArrayBlock( diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntBigArrayBlock.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntBigArrayBlock.java index 068f550a56d5b..ebb3f470bdcee 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntBigArrayBlock.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntBigArrayBlock.java @@ -51,6 +51,9 @@ private IntBigArrayBlock( ) { super(positionCount, firstValueIndexes, nulls, mvOrdering, blockFactory); this.vector = vector; + assert firstValueIndexes == null + ? vector.getPositionCount() == getPositionCount() + : firstValueIndexes[getPositionCount()] == vector.getPositionCount(); } @Override @@ -106,7 +109,7 @@ public IntBlock expand() { // The following line is correct because positions with multi-values are never null. int expandedPositionCount = vector.getPositionCount(); - long bitSetRamUsedEstimate = BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount); + long bitSetRamUsedEstimate = Math.max(nullsMask.size(), BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount)); blockFactory().adjustBreaker(bitSetRamUsedEstimate, false); IntBigArrayBlock expanded = new IntBigArrayBlock( diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongArrayBlock.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongArrayBlock.java index 0d8464a95ed5b..5236bf32a5a10 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongArrayBlock.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongArrayBlock.java @@ -50,6 +50,9 @@ private LongArrayBlock( ) { super(positionCount, firstValueIndexes, nulls, mvOrdering, blockFactory); this.vector = vector; + assert firstValueIndexes == null + ? vector.getPositionCount() == getPositionCount() + : firstValueIndexes[getPositionCount()] == vector.getPositionCount(); } @Override @@ -105,7 +108,7 @@ public LongBlock expand() { // The following line is correct because positions with multi-values are never null. int expandedPositionCount = vector.getPositionCount(); - long bitSetRamUsedEstimate = BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount); + long bitSetRamUsedEstimate = Math.max(nullsMask.size(), BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount)); blockFactory().adjustBreaker(bitSetRamUsedEstimate, false); LongArrayBlock expanded = new LongArrayBlock( diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongBigArrayBlock.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongBigArrayBlock.java index eb1a353352ac9..1d3dde8357fa9 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongBigArrayBlock.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongBigArrayBlock.java @@ -51,6 +51,9 @@ private LongBigArrayBlock( ) { super(positionCount, firstValueIndexes, nulls, mvOrdering, blockFactory); this.vector = vector; + assert firstValueIndexes == null + ? vector.getPositionCount() == getPositionCount() + : firstValueIndexes[getPositionCount()] == vector.getPositionCount(); } @Override @@ -106,7 +109,7 @@ public LongBlock expand() { // The following line is correct because positions with multi-values are never null. int expandedPositionCount = vector.getPositionCount(); - long bitSetRamUsedEstimate = BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount); + long bitSetRamUsedEstimate = Math.max(nullsMask.size(), BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount)); blockFactory().adjustBreaker(bitSetRamUsedEstimate, false); LongBigArrayBlock expanded = new LongBigArrayBlock( diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/AbstractArrayBlock.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/AbstractArrayBlock.java index fe1ecbec92e5b..d6046f0bda085 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/AbstractArrayBlock.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/AbstractArrayBlock.java @@ -52,7 +52,7 @@ public final MvOrdering mvOrdering() { } protected BitSet shiftNullsToExpandedPositions() { - BitSet expanded = new BitSet(getTotalValueCount()); + BitSet expanded = new BitSet(nullsMask.size()); int next = -1; while ((next = nullsMask.nextSetBit(next + 1)) != -1) { expanded.set(getFirstValueIndex(next)); diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/AbstractBlock.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/AbstractBlock.java index 8ce6ef9ab78ab..0c5207133f71d 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/AbstractBlock.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/AbstractBlock.java @@ -31,6 +31,7 @@ protected AbstractBlock(int positionCount, BlockFactory blockFactory) { this.blockFactory = blockFactory; this.firstValueIndexes = null; this.nullsMask = null; + assert assertInvariants(); } /** @@ -43,6 +44,26 @@ protected AbstractBlock(int positionCount, @Nullable int[] firstValueIndexes, @N this.firstValueIndexes = firstValueIndexes; this.nullsMask = nullsMask == null || nullsMask.isEmpty() ? null : nullsMask; assert nullsMask != null || firstValueIndexes != null : "Create VectorBlock instead"; + assert assertInvariants(); + } + + private boolean assertInvariants() { + if (firstValueIndexes != null) { + assert firstValueIndexes.length == getPositionCount() + 1; + for (int i = 0; i < getPositionCount(); i++) { + assert (firstValueIndexes[i + 1] - firstValueIndexes[i]) >= 0; + } + } + if (nullsMask != null) { + assert nullsMask.nextSetBit(getPositionCount() + 1) == -1; + } + if (firstValueIndexes != null && nullsMask != null) { + for (int i = 0; i < getPositionCount(); i++) { + // Either we have multi-values or a null but never both. + assert ((nullsMask.get(i) == false) || (firstValueIndexes[i + 1] - firstValueIndexes[i]) == 1); + } + } + return true; } @Override diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BlockRamUsageEstimator.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BlockRamUsageEstimator.java index d1f1bac940714..93fd02c3cd879 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BlockRamUsageEstimator.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BlockRamUsageEstimator.java @@ -27,6 +27,8 @@ public static long sizeOfBitSet(@Nullable BitSet bitset) { } public static long sizeOfBitSet(long size) { - return BITSET_BASE_RAM_USAGE + (size / Byte.SIZE); + // BitSet is normally made up of words, represented by longs. So we need to divide and round up. + long wordCount = (size + Long.SIZE - 1) / Long.SIZE; + return BITSET_BASE_RAM_USAGE + wordCount * Long.BYTES; } } diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-ArrayBlock.java.st b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-ArrayBlock.java.st index 245089ff2a83e..e710f51ff939f 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-ArrayBlock.java.st +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-ArrayBlock.java.st @@ -61,6 +61,9 @@ final class $Type$ArrayBlock extends AbstractArrayBlock implements $Type$Block { ) { super(positionCount, firstValueIndexes, nulls, mvOrdering, blockFactory); this.vector = vector; + assert firstValueIndexes == null + ? vector.getPositionCount() == getPositionCount() + : firstValueIndexes[getPositionCount()] == vector.getPositionCount(); } @Override @@ -124,7 +127,7 @@ $endif$ // The following line is correct because positions with multi-values are never null. int expandedPositionCount = vector.getPositionCount(); - long bitSetRamUsedEstimate = BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount); + long bitSetRamUsedEstimate = Math.max(nullsMask.size(), BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount)); blockFactory().adjustBreaker(bitSetRamUsedEstimate, false); $Type$ArrayBlock expanded = new $Type$ArrayBlock( diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-BigArrayBlock.java.st b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-BigArrayBlock.java.st index a5f5001802cb7..1fb9e4ec27b2e 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-BigArrayBlock.java.st +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-BigArrayBlock.java.st @@ -51,6 +51,9 @@ public final class $Type$BigArrayBlock extends AbstractArrayBlock implements $Ty ) { super(positionCount, firstValueIndexes, nulls, mvOrdering, blockFactory); this.vector = vector; + assert firstValueIndexes == null + ? vector.getPositionCount() == getPositionCount() + : firstValueIndexes[getPositionCount()] == vector.getPositionCount(); } @Override @@ -106,7 +109,7 @@ public final class $Type$BigArrayBlock extends AbstractArrayBlock implements $Ty // The following line is correct because positions with multi-values are never null. int expandedPositionCount = vector.getPositionCount(); - long bitSetRamUsedEstimate = BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount); + long bitSetRamUsedEstimate = Math.max(nullsMask.size(), BlockRamUsageEstimator.sizeOfBitSet(expandedPositionCount)); blockFactory().adjustBreaker(bitSetRamUsedEstimate, false); $Type$BigArrayBlock expanded = new $Type$BigArrayBlock( diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TimeBasedIndicesIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TimeBasedIndicesIT.java index a1fbee17ef8ec..c953a9cbdc2f4 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TimeBasedIndicesIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/TimeBasedIndicesIT.java @@ -24,34 +24,35 @@ public class TimeBasedIndicesIT extends AbstractEsqlIntegTestCase { public void testFilter() { long epoch = System.currentTimeMillis(); assertAcked(client().admin().indices().prepareCreate("test").setMapping("@timestamp", "type=date", "value", "type=long")); - BulkRequestBuilder bulk = client().prepareBulk("test").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); - int oldDocs = between(10, 100); - for (int i = 0; i < oldDocs; i++) { - long timestamp = epoch - TimeValue.timeValueHours(between(1, 2)).millis(); - bulk.add(new IndexRequest().source("@timestamp", timestamp, "value", -i)); - } - int newDocs = between(10, 100); - for (int i = 0; i < newDocs; i++) { - long timestamp = epoch + TimeValue.timeValueHours(between(1, 2)).millis(); - bulk.add(new IndexRequest().source("@timestamp", timestamp, "value", i)); - } - bulk.get(); - { - EsqlQueryRequest request = new EsqlQueryRequest(); - request.query("FROM test | limit 1000"); - request.filter(new RangeQueryBuilder("@timestamp").from(epoch - TimeValue.timeValueHours(3).millis()).to("now")); - try (var resp = run(request)) { - List> values = getValuesList(resp); - assertThat(values, hasSize(oldDocs)); + try (BulkRequestBuilder bulk = client().prepareBulk("test").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)) { + int oldDocs = between(10, 100); + for (int i = 0; i < oldDocs; i++) { + long timestamp = epoch - TimeValue.timeValueHours(between(1, 2)).millis(); + bulk.add(new IndexRequest().source("@timestamp", timestamp, "value", -i)); } - } - { - EsqlQueryRequest request = new EsqlQueryRequest(); - request.query("FROM test | limit 1000"); - request.filter(new RangeQueryBuilder("@timestamp").from("now").to(epoch + TimeValue.timeValueHours(3).millis())); - try (var resp = run(request)) { - List> values = getValuesList(resp); - assertThat(values, hasSize(newDocs)); + int newDocs = between(10, 100); + for (int i = 0; i < newDocs; i++) { + long timestamp = epoch + TimeValue.timeValueHours(between(1, 2)).millis(); + bulk.add(new IndexRequest().source("@timestamp", timestamp, "value", i)); + } + bulk.get(); + { + EsqlQueryRequest request = new EsqlQueryRequest(); + request.query("FROM test | limit 1000"); + request.filter(new RangeQueryBuilder("@timestamp").from(epoch - TimeValue.timeValueHours(3).millis()).to("now")); + try (var resp = run(request)) { + List> values = getValuesList(resp); + assertThat(values, hasSize(oldDocs)); + } + } + { + EsqlQueryRequest request = new EsqlQueryRequest(); + request.query("FROM test | limit 1000"); + request.filter(new RangeQueryBuilder("@timestamp").from("now").to(epoch + TimeValue.timeValueHours(3).millis())); + try (var resp = run(request)) { + List> values = getValuesList(resp); + assertThat(values, hasSize(newDocs)); + } } } } diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/DownsampleActionIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/DownsampleActionIT.java index 23ec1c0262dd4..4e84830b4395a 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/DownsampleActionIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/DownsampleActionIT.java @@ -338,9 +338,6 @@ public void testTsdbDataStreams() throws Exception { rolloverMaxOneDocCondition(client(), dataStream); String rollupIndex = waitAndGetRollupIndexName(client(), backingIndexName, fixedInterval); - if (rollupIndex == null) { - logger.warn("explain:" + explainIndex(client(), backingIndexName)); - } assertNotNull(String.format(Locale.ROOT, "Cannot retrieve rollup index [%s]", rollupIndex), rollupIndex); assertBusy(() -> assertTrue("Rollup index does not exist", indexExists(rollupIndex)), 30, TimeUnit.SECONDS); assertBusy(() -> assertFalse("Source index should have been deleted", indexExists(backingIndexName)), 30, TimeUnit.SECONDS); @@ -607,7 +604,7 @@ public void testDownsampleTwiceSameInterval() throws Exception { * @return the name of the rollup index for a given index, null if none exist */ public String waitAndGetRollupIndexName(RestClient client, String originalIndexName, DateHistogramInterval fixedInterval) - throws InterruptedException { + throws InterruptedException, IOException { final String[] rollupIndexName = new String[1]; waitUntil(() -> { try { @@ -617,7 +614,15 @@ public String waitAndGetRollupIndexName(RestClient client, String originalIndexN return false; } }, 120, TimeUnit.SECONDS); // High timeout in case we're unlucky and end_time has been increased. - logger.info("--> original index name is [{}], rollup index name is [{}]", originalIndexName, rollupIndexName[0]); + if (rollupIndexName[0] == null) { + logger.warn( + "--> original index name is [{}], rollup index name is NULL, possible explanation: {}", + originalIndexName, + explainIndex(client(), originalIndexName) + ); + } else { + logger.info("--> original index name is [{}], rollup index name is [{}]", originalIndexName, rollupIndexName[0]); + } return rollupIndexName[0]; } diff --git a/x-pack/plugin/ilm/src/main/java/module-info.java b/x-pack/plugin/ilm/src/main/java/module-info.java index aa24c2d6f333c..591c9786247e6 100644 --- a/x-pack/plugin/ilm/src/main/java/module-info.java +++ b/x-pack/plugin/ilm/src/main/java/module-info.java @@ -18,4 +18,6 @@ provides org.elasticsearch.reservedstate.ReservedClusterStateHandlerProvider with org.elasticsearch.xpack.ilm.ReservedLifecycleStateHandlerProvider; + + provides org.elasticsearch.features.FeatureSpecification with org.elasticsearch.xpack.ilm.IndexLifecycleFeatures; } diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycle.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycle.java index 53e4d3de463fd..288306286882e 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycle.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycle.java @@ -147,6 +147,7 @@ public Collection createComponents(PluginServices services) { ILMHistoryTemplateRegistry ilmTemplateRegistry = new ILMHistoryTemplateRegistry( settings, services.clusterService(), + services.featureService(), services.threadPool(), services.client(), services.xContentRegistry() diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleFeatures.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleFeatures.java new file mode 100644 index 0000000000000..cc78271e2d878 --- /dev/null +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleFeatures.java @@ -0,0 +1,22 @@ +/* + * 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.ilm; + +import org.elasticsearch.Version; +import org.elasticsearch.features.FeatureSpecification; +import org.elasticsearch.features.NodeFeature; +import org.elasticsearch.xpack.ilm.history.ILMHistoryTemplateRegistry; + +import java.util.Map; + +public class IndexLifecycleFeatures implements FeatureSpecification { + @Override + public Map getHistoricalFeatures() { + return Map.of(ILMHistoryTemplateRegistry.MANAGED_BY_DATA_STREAM_LIFECYCLE, Version.V_8_12_0); + } +} diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/history/ILMHistoryTemplateRegistry.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/history/ILMHistoryTemplateRegistry.java index 5633033e6faa1..28c28ef6e4c55 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/history/ILMHistoryTemplateRegistry.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/history/ILMHistoryTemplateRegistry.java @@ -8,9 +8,12 @@ package org.elasticsearch.xpack.ilm.history; import org.elasticsearch.client.internal.Client; +import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.metadata.ComposableIndexTemplate; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.features.FeatureService; +import org.elasticsearch.features.NodeFeature; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xpack.core.ClientHelper; @@ -37,11 +40,13 @@ public class ILMHistoryTemplateRegistry extends IndexTemplateRegistry { // version 6: manage by data stream lifecycle // version 7: version the index template name so we can upgrade existing deployments public static final int INDEX_TEMPLATE_VERSION = 7; + public static final NodeFeature MANAGED_BY_DATA_STREAM_LIFECYCLE = new NodeFeature("ilm-history-managed-by-dsl"); public static final String ILM_TEMPLATE_VERSION_VARIABLE = "xpack.ilm_history.template.version"; public static final String ILM_TEMPLATE_NAME = "ilm-history-" + INDEX_TEMPLATE_VERSION; public static final String ILM_POLICY_NAME = "ilm-history-ilm-policy"; + private final FeatureService featureService; @Override protected boolean requiresMasterNode() { @@ -53,11 +58,13 @@ protected boolean requiresMasterNode() { public ILMHistoryTemplateRegistry( Settings nodeSettings, ClusterService clusterService, + FeatureService featureService, ThreadPool threadPool, Client client, NamedXContentRegistry xContentRegistry ) { super(nodeSettings, clusterService, threadPool, client, xContentRegistry); + this.featureService = featureService; this.ilmHistoryEnabled = LifecycleSettings.LIFECYCLE_HISTORY_INDEX_ENABLED_SETTING.get(nodeSettings); } @@ -97,4 +104,9 @@ protected List getLifecyclePolicies() { protected String getOrigin() { return ClientHelper.INDEX_LIFECYCLE_ORIGIN; } + + @Override + protected boolean isClusterReady(ClusterChangedEvent event) { + return featureService.clusterHasFeature(event.state(), MANAGED_BY_DATA_STREAM_LIFECYCLE); + } } diff --git a/x-pack/plugin/ilm/src/main/resources/META-INF/services/org.elasticsearch.features.FeatureSpecification b/x-pack/plugin/ilm/src/main/resources/META-INF/services/org.elasticsearch.features.FeatureSpecification new file mode 100644 index 0000000000000..1bf03ae25edd2 --- /dev/null +++ b/x-pack/plugin/ilm/src/main/resources/META-INF/services/org.elasticsearch.features.FeatureSpecification @@ -0,0 +1,8 @@ +# +# 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. +# + +org.elasticsearch.xpack.ilm.IndexLifecycleFeatures diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/history/ILMHistoryStoreTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/history/ILMHistoryStoreTests.java index 0eece33e2e581..8675c27325b4b 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/history/ILMHistoryStoreTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/history/ILMHistoryStoreTests.java @@ -31,6 +31,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.core.TimeValue; +import org.elasticsearch.features.FeatureService; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ClusterServiceUtils; @@ -39,6 +40,7 @@ import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xcontent.NamedXContentRegistry; +import org.elasticsearch.xpack.ilm.IndexLifecycleFeatures; import org.hamcrest.Matchers; import org.junit.After; import org.junit.Before; @@ -78,6 +80,7 @@ public void setup() { ILMHistoryTemplateRegistry registry = new ILMHistoryTemplateRegistry( clusterService.getSettings(), clusterService, + new FeatureService(List.of(new IndexLifecycleFeatures())), threadPool, client, NamedXContentRegistry.EMPTY diff --git a/x-pack/plugin/mapper-version/src/test/java/org/elasticsearch/xpack/versionfield/VersionStringFieldTests.java b/x-pack/plugin/mapper-version/src/test/java/org/elasticsearch/xpack/versionfield/VersionStringFieldTests.java index f41cc145831cf..94d8a144b0bd6 100644 --- a/x-pack/plugin/mapper-version/src/test/java/org/elasticsearch/xpack/versionfield/VersionStringFieldTests.java +++ b/x-pack/plugin/mapper-version/src/test/java/org/elasticsearch/xpack/versionfield/VersionStringFieldTests.java @@ -17,9 +17,9 @@ import org.elasticsearch.search.aggregations.metrics.Cardinality; import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.test.ESSingleNodeTestCase; -import org.elasticsearch.xpack.analytics.AnalyticsAggregationBuilders; import org.elasticsearch.xpack.analytics.AnalyticsPlugin; import org.elasticsearch.xpack.analytics.stringstats.InternalStringStats; +import org.elasticsearch.xpack.analytics.stringstats.StringStatsAggregationBuilder; import org.elasticsearch.xpack.core.LocalStateCompositeXPackPlugin; import java.io.IOException; @@ -415,7 +415,7 @@ public void testAggs() throws Exception { // string stats assertResponse( - client().prepareSearch(indexName).addAggregation(AnalyticsAggregationBuilders.stringStats("stats").field("version")), + client().prepareSearch(indexName).addAggregation(new StringStatsAggregationBuilder("stats").field("version")), response -> { InternalStringStats stats = response.getAggregations().get("stats"); assertEquals(3, stats.getMinLength()); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportCloseJobAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportCloseJobAction.java index 7512aa2b42acf..02801864a3e78 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportCloseJobAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportCloseJobAction.java @@ -244,7 +244,7 @@ static class OpenAndClosingIds { } List openJobIds; - List closingJobIds; + final List closingJobIds; } /** @@ -616,8 +616,8 @@ private void normalCloseJob( } static class WaitForCloseRequest { - List> persistentTasks = new ArrayList<>(); - List jobsToFinalize = new ArrayList<>(); + final List> persistentTasks = new ArrayList<>(); + final List jobsToFinalize = new ArrayList<>(); public boolean hasJobsToWaitFor() { return persistentTasks.isEmpty() == false; diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteTrainedModelAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteTrainedModelAction.java index 49f73056cd8bd..d19871d0e1b2f 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteTrainedModelAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteTrainedModelAction.java @@ -231,7 +231,6 @@ private void deleteModel(DeleteTrainedModelAction.Request request, ClusterState id ) ); - return; } } else { deleteAliasesAndModel(request, modelAliases, listener); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetOverallBucketsAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetOverallBucketsAction.java index 10fb54ee8ae4c..d9dfd0fb23eeb 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetOverallBucketsAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportGetOverallBucketsAction.java @@ -14,7 +14,6 @@ import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.client.internal.Client; -import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.core.TimeValue; @@ -66,7 +65,6 @@ public class TransportGetOverallBucketsAction extends HandledTransportAction< private final ThreadPool threadPool; private final Client client; - private final ClusterService clusterService; private final JobManager jobManager; @Inject @@ -74,7 +72,6 @@ public TransportGetOverallBucketsAction( ThreadPool threadPool, TransportService transportService, ActionFilters actionFilters, - ClusterService clusterService, JobManager jobManager, Client client ) { @@ -86,7 +83,6 @@ public TransportGetOverallBucketsAction( EsExecutors.DIRECT_EXECUTOR_SERVICE ); this.threadPool = threadPool; - this.clusterService = clusterService; this.client = client; this.jobManager = jobManager; } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutDatafeedAction.java index d71e99040177f..8cdb8050bd257 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutDatafeedAction.java @@ -71,7 +71,7 @@ protected void masterOperation( ClusterState state, ActionListener listener ) { - datafeedManager.putDatafeed(request, state, licenseState, securityContext, threadPool, listener); + datafeedManager.putDatafeed(request, state, securityContext, threadPool, listener); } @Override diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutJobAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutJobAction.java index 767ec08078b42..55f89a993ce61 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutJobAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutJobAction.java @@ -99,7 +99,6 @@ protected void masterOperation( new PutDatafeedAction.Request(jobCreated.getResponse().getDatafeedConfig().get()), // Use newer state from cluster service as the job creation may have created shared indexes clusterService.state(), - licenseState, securityContext, threadPool, ActionListener.wrap(createdDatafeed -> { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutTrainedModelAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutTrainedModelAction.java index 5206799735c52..195cc44dcaa65 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutTrainedModelAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutTrainedModelAction.java @@ -6,8 +6,6 @@ */ package org.elasticsearch.xpack.ml.action; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.ResourceNotFoundException; @@ -29,7 +27,6 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.logging.HeaderWarning; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; @@ -42,10 +39,8 @@ import org.elasticsearch.rest.RestStatus; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.tasks.Task; -import org.elasticsearch.tasks.TaskInfo; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; -import org.elasticsearch.xcontent.DeprecationHandler; import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentFactory; @@ -55,7 +50,6 @@ import org.elasticsearch.xpack.core.XPackField; import org.elasticsearch.xpack.core.ml.MachineLearningField; import org.elasticsearch.xpack.core.ml.MlConfigVersion; -import org.elasticsearch.xpack.core.ml.MlTasks; import org.elasticsearch.xpack.core.ml.action.GetTrainedModelsAction; import org.elasticsearch.xpack.core.ml.action.PutTrainedModelAction; import org.elasticsearch.xpack.core.ml.action.PutTrainedModelAction.Request; @@ -87,22 +81,18 @@ import static org.elasticsearch.xpack.core.ClientHelper.ML_ORIGIN; import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin; -import static org.elasticsearch.xpack.core.ml.MlTasks.downloadModelTaskDescription; public class TransportPutTrainedModelAction extends TransportMasterNodeAction { private static final ByteSizeValue MAX_NATIVE_DEFINITION_INDEX_SIZE = ByteSizeValue.ofGb(50); - private static final Logger logger = LogManager.getLogger(TransportPutTrainedModelAction.class); private final TrainedModelProvider trainedModelProvider; private final XPackLicenseState licenseState; private final NamedXContentRegistry xContentRegistry; private final OriginSettingClient client; - private final Settings settings; @Inject public TransportPutTrainedModelAction( - Settings settings, TransportService transportService, ClusterService clusterService, ThreadPool threadPool, @@ -128,7 +118,6 @@ public TransportPutTrainedModelAction( this.trainedModelProvider = trainedModelProvider; this.xContentRegistry = xContentRegistry; this.client = new OriginSettingClient(client, ML_ORIGIN); - this.settings = settings; } @Override @@ -248,13 +237,15 @@ protected void masterOperation( return; } - ActionListener finalResponseAction = ActionListener.wrap((configToReturn) -> { - finalResponseListener.onResponse(new PutTrainedModelAction.Response(configToReturn)); - }, finalResponseListener::onFailure); + ActionListener finalResponseAction = ActionListener.wrap( + (configToReturn) -> finalResponseListener.onResponse(new Response(configToReturn)), + finalResponseListener::onFailure + ); - ActionListener verifyClusterAndModelArchitectures = ActionListener.wrap((configToReturn) -> { - verifyMlNodesAndModelArchitectures(configToReturn, client, threadPool, finalResponseAction); - }, finalResponseListener::onFailure); + ActionListener verifyClusterAndModelArchitectures = ActionListener.wrap( + (configToReturn) -> verifyMlNodesAndModelArchitectures(configToReturn, client, threadPool, finalResponseAction), + finalResponseListener::onFailure + ); ActionListener finishedStoringListener = ActionListener.wrap(bool -> { TrainedModelConfig configToReturn = trainedModelConfig.clearDefinition().build(); @@ -361,7 +352,7 @@ void verifyMlNodesAndModelArchitectures( ThreadPool threadPool, ActionListener configToReturnListener ) { - ActionListener addWarningHeaderOnFailureListener = new ActionListener() { + ActionListener addWarningHeaderOnFailureListener = new ActionListener<>() { @Override public void onResponse(TrainedModelConfig config) { assert Objects.equals(config, configToReturn); @@ -413,38 +404,9 @@ static void checkForExistingTask( }, sendResponseListener::onFailure), timeout); } - private static void getExistingTaskInfo(Client client, String modelId, boolean waitForCompletion, ActionListener listener) { - client.admin() - .cluster() - .prepareListTasks() - .setActions(MlTasks.MODEL_IMPORT_TASK_ACTION) - .setDetailed(true) - .setWaitForCompletion(waitForCompletion) - .setDescriptions(downloadModelTaskDescription(modelId)) - .execute(ActionListener.wrap((response) -> { - var tasks = response.getTasks(); - - if (tasks.size() > 0) { - // there really shouldn't be more than a single task but if there is we'll just use the first one - listener.onResponse(tasks.get(0)); - } else { - listener.onResponse(null); - } - }, e -> { - listener.onFailure( - new ElasticsearchStatusException( - "Unable to retrieve task information for model id [{}]", - RestStatus.INTERNAL_SERVER_ERROR, - e, - modelId - ) - ); - })); - } - private static void getModelInformation(Client client, String modelId, ActionListener listener) { client.execute(GetTrainedModelsAction.INSTANCE, new GetTrainedModelsAction.Request(modelId), ActionListener.wrap(models -> { - if (models.getResources().results().size() == 0) { + if (models.getResources().results().isEmpty()) { listener.onFailure( new ElasticsearchStatusException( "No model information found for a concurrent create model execution for model id [{}]", @@ -563,11 +525,7 @@ static void setTrainedModelConfigFieldsFromPackagedModel( trainedModelConfig.setPlatformArchitecture(resolvedModelPackageConfig.getPlatformArchitecture()); trainedModelConfig.setMetadata(resolvedModelPackageConfig.getMetadata()); trainedModelConfig.setInferenceConfig( - parseInferenceConfigFromModelPackage( - resolvedModelPackageConfig.getInferenceConfigSource(), - xContentRegistry, - LoggingDeprecationHandler.INSTANCE - ) + parseInferenceConfigFromModelPackage(resolvedModelPackageConfig.getInferenceConfigSource(), xContentRegistry) ); trainedModelConfig.setTags(resolvedModelPackageConfig.getTags()); trainedModelConfig.setPrefixStrings(resolvedModelPackageConfig.getPrefixStrings()); @@ -578,16 +536,14 @@ static void setTrainedModelConfigFieldsFromPackagedModel( trainedModelConfig.setLocation(trainedModelConfig.getModelType().getDefaultLocation(trainedModelConfig.getModelId())); } - static InferenceConfig parseInferenceConfigFromModelPackage( - Map source, - NamedXContentRegistry namedXContentRegistry, - DeprecationHandler deprecationHandler - ) throws IOException { + static InferenceConfig parseInferenceConfigFromModelPackage(Map source, NamedXContentRegistry namedXContentRegistry) + throws IOException { try ( XContentBuilder xContentBuilder = XContentFactory.jsonBuilder().map(source); XContentParser sourceParser = XContentType.JSON.xContent() .createParser( - XContentParserConfiguration.EMPTY.withRegistry(namedXContentRegistry).withDeprecationHandler(deprecationHandler), + XContentParserConfiguration.EMPTY.withRegistry(namedXContentRegistry) + .withDeprecationHandler(LoggingDeprecationHandler.INSTANCE), BytesReference.bytes(xContentBuilder).streamInput() ) ) { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartTrainedModelDeploymentAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartTrainedModelDeploymentAction.java index ecfe4c8aac6c6..28475dc70569f 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartTrainedModelDeploymentAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStartTrainedModelDeploymentAction.java @@ -43,7 +43,6 @@ import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; -import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xpack.core.XPackField; import org.elasticsearch.xpack.core.ml.MachineLearningField; import org.elasticsearch.xpack.core.ml.action.CreateTrainedModelAssignmentAction; @@ -108,7 +107,6 @@ public TransportStartTrainedModelDeploymentAction( XPackLicenseState licenseState, IndexNameExpressionResolver indexNameExpressionResolver, TrainedModelAssignmentService trainedModelAssignmentService, - NamedXContentRegistry xContentRegistry, MlMemoryTracker memoryTracker, InferenceAuditor auditor ) { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStopTrainedModelDeploymentAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStopTrainedModelDeploymentAction.java index 5b2c3fdeddf43..a3eb15a372d2a 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStopTrainedModelDeploymentAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportStopTrainedModelDeploymentAction.java @@ -17,8 +17,6 @@ import org.elasticsearch.action.TaskOperationFailure; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.tasks.TransportTasksAction; -import org.elasticsearch.client.internal.Client; -import org.elasticsearch.client.internal.OriginSettingClient; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; @@ -48,7 +46,6 @@ import java.util.Set; import static org.elasticsearch.core.Strings.format; -import static org.elasticsearch.xpack.core.ClientHelper.ML_ORIGIN; import static org.elasticsearch.xpack.ml.action.TransportDeleteTrainedModelAction.getModelAliases; import static org.elasticsearch.xpack.ml.action.TransportDeleteTrainedModelAction.getReferencedModelKeys; @@ -66,7 +63,6 @@ public class TransportStopTrainedModelDeploymentAction extends TransportTasksAct private static final Logger logger = LogManager.getLogger(TransportStopTrainedModelDeploymentAction.class); - private final Client client; private final IngestService ingestService; private final TrainedModelAssignmentClusterService trainedModelAssignmentClusterService; private final InferenceAuditor auditor; @@ -76,7 +72,6 @@ public TransportStopTrainedModelDeploymentAction( ClusterService clusterService, TransportService transportService, ActionFilters actionFilters, - Client client, IngestService ingestService, TrainedModelAssignmentClusterService trainedModelAssignmentClusterService, InferenceAuditor auditor @@ -91,7 +86,6 @@ public TransportStopTrainedModelDeploymentAction( StopTrainedModelDeploymentAction.Response::new, EsExecutors.DIRECT_EXECUTOR_SERVICE ); - this.client = new OriginSettingClient(client, ML_ORIGIN); this.ingestService = ingestService; this.trainedModelAssignmentClusterService = trainedModelAssignmentClusterService; this.auditor = Objects.requireNonNull(auditor); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java index c1e77d953ab54..dc53c02389184 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java @@ -18,7 +18,6 @@ import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.client.internal.Client; -import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.util.concurrent.EsExecutors; @@ -60,8 +59,7 @@ public TransportUpdateFilterAction( TransportService transportService, ActionFilters actionFilters, Client client, - JobManager jobManager, - ClusterService clusterService + JobManager jobManager ) { super( UpdateFilterAction.NAME, diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/categorization/CategorizeTextAggregationBuilder.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/categorization/CategorizeTextAggregationBuilder.java index 6fce8aa20ed16..9262ac65b5cfd 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/categorization/CategorizeTextAggregationBuilder.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/categorization/CategorizeTextAggregationBuilder.java @@ -23,7 +23,6 @@ import org.elasticsearch.xcontent.ObjectParser; import org.elasticsearch.xcontent.ParseField; import org.elasticsearch.xcontent.XContentBuilder; -import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xpack.core.ml.job.config.CategorizationAnalyzerConfig; import org.elasticsearch.xpack.core.ml.job.messages.Messages; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; @@ -85,10 +84,6 @@ public class CategorizeTextAggregationBuilder extends AbstractAggregationBuilder PARSER.declareInt(CategorizeTextAggregationBuilder::size, REQUIRED_SIZE_FIELD_NAME); } - public static CategorizeTextAggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException { - return PARSER.parse(parser, new CategorizeTextAggregationBuilder(aggregationName), null); - } - private TermsAggregator.BucketCountThresholds bucketCountThresholds = new TermsAggregator.BucketCountThresholds( DEFAULT_BUCKET_COUNT_THRESHOLDS ); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/CountingItemSetTraverser.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/CountingItemSetTraverser.java index 35a0bd9e4c43f..a651b0c85eb40 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/CountingItemSetTraverser.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/frequentitemsets/CountingItemSetTraverser.java @@ -7,8 +7,6 @@ package org.elasticsearch.xpack.ml.aggs.frequentitemsets; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.elasticsearch.core.Releasable; import org.elasticsearch.core.Releasables; import org.elasticsearch.xpack.ml.aggs.frequentitemsets.TransactionStore.TopItemIds; @@ -31,7 +29,6 @@ * if [a, b] is not in T, [a, b, c] can not be in T either */ final class CountingItemSetTraverser implements Releasable { - private static final Logger logger = LogManager.getLogger(CountingItemSetTraverser.class); // start size and size increment for the occurences stack private static final int OCCURENCES_SIZE_INCREMENT = 10; diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/heuristic/PValueScore.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/heuristic/PValueScore.java index 324da870b1a40..8fe9f1ccd5415 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/heuristic/PValueScore.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/heuristic/PValueScore.java @@ -11,11 +11,9 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.search.aggregations.bucket.terms.heuristic.NXYSignificanceHeuristic; -import org.elasticsearch.search.aggregations.bucket.terms.heuristic.SignificanceHeuristic; import org.elasticsearch.xcontent.ConstructingObjectParser; import org.elasticsearch.xcontent.ParseField; import org.elasticsearch.xcontent.XContentBuilder; -import org.elasticsearch.xcontent.XContentParser; import java.io.IOException; import java.util.Objects; @@ -104,10 +102,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } - public static SignificanceHeuristic parse(XContentParser parser) throws IOException { - return PARSER.apply(parser, null); - } - /** * This finds the p-value that the frequency of a category is unchanged on set subset assuming * we observe subsetFreq out of subset values in total relative to set supersetFreq where it accounts @@ -200,28 +194,4 @@ private static double eps(double value) { return Math.max(0.05 * value + 0.5, 1.0); } - public static class PValueScoreBuilder extends NXYBuilder { - private final long normalizeAbove; - - public PValueScoreBuilder(boolean backgroundIsSuperset, Long normalizeAbove) { - super(true, backgroundIsSuperset); - this.normalizeAbove = normalizeAbove == null ? 0L : normalizeAbove; - if (normalizeAbove != null && normalizeAbove <= 0) { - throw new IllegalArgumentException( - "[" + NORMALIZE_ABOVE.getPreferredName() + "] must be a positive value, provided [" + normalizeAbove + "]" - ); - } - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(NAME); - builder.field(BACKGROUND_IS_SUPERSET.getPreferredName(), backgroundIsSuperset); - if (normalizeAbove > 0) { - builder.field(NORMALIZE_ABOVE.getPreferredName(), normalizeAbove); - } - builder.endObject(); - return builder; - } - } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/NodeFakeAvailabilityZoneMapper.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/NodeFakeAvailabilityZoneMapper.java index df2f66f6c5a42..1eb6fe3d03cd2 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/NodeFakeAvailabilityZoneMapper.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/autoscaling/NodeFakeAvailabilityZoneMapper.java @@ -7,8 +7,6 @@ package org.elasticsearch.xpack.ml.autoscaling; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodeRole; @@ -27,8 +25,6 @@ */ public class NodeFakeAvailabilityZoneMapper extends AbstractNodeAvailabilityZoneMapper { - private static final Logger logger = LogManager.getLogger(NodeFakeAvailabilityZoneMapper.class); - public NodeFakeAvailabilityZoneMapper(Settings settings, ClusterSettings clusterSettings) { this(settings, clusterSettings, null); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedManager.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedManager.java index 999d85b6dd549..ede57764a0813 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedManager.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedManager.java @@ -20,7 +20,6 @@ import org.elasticsearch.core.Nullable; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.license.RemoteClusterLicenseChecker; -import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.persistent.PersistentTasksCustomMetadata; import org.elasticsearch.tasks.TaskId; import org.elasticsearch.threadpool.ThreadPool; @@ -100,7 +99,6 @@ public DatafeedManager( public void putDatafeed( PutDatafeedAction.Request request, ClusterState state, - XPackLicenseState licenseState, SecurityContext securityContext, ThreadPool threadPool, ActionListener listener diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/assignment/TrainedModelAssignmentClusterService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/assignment/TrainedModelAssignmentClusterService.java index 471615e8bbd6a..637ad9d7bbbb2 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/assignment/TrainedModelAssignmentClusterService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/assignment/TrainedModelAssignmentClusterService.java @@ -78,8 +78,6 @@ public class TrainedModelAssignmentClusterService implements ClusterStateListene private static final TransportVersion RENAME_ALLOCATION_TO_ASSIGNMENT_TRANSPORT_VERSION = TransportVersions.V_8_3_0; public static final TransportVersion DISTRIBUTED_MODEL_ALLOCATION_TRANSPORT_VERSION = TransportVersions.V_8_4_0; - private static final TransportVersion NEW_ALLOCATION_MEMORY_VERSION = TransportVersions.V_8_500_064; - private final ClusterService clusterService; private final ThreadPool threadPool; private final NodeLoadDetector nodeLoadDetector; diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/assignment/planning/AssignmentPlan.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/assignment/planning/AssignmentPlan.java index 1dce7f0bb46ba..d9cb0f08a6cd0 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/assignment/planning/AssignmentPlan.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/assignment/planning/AssignmentPlan.java @@ -437,10 +437,6 @@ public Builder assignModelToNode(Deployment deployment, Node node, int allocatio return this; } - private boolean isAlreadyAssigned(Deployment deployment, Node node) { - return deployment.currentAllocationsByNodeId().containsKey(node.id()) || assignments.get(deployment).get(node) > 0; - } - private int getAssignedAllocations(Deployment deployment, Node node) { int currentAllocations = getCurrentAllocations(deployment, node); int assignmentAllocations = assignments.get(deployment).get(node); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/deployment/DeploymentManager.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/deployment/DeploymentManager.java index ef5de2718e702..18e89732daf21 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/deployment/DeploymentManager.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/deployment/DeploymentManager.java @@ -17,6 +17,7 @@ import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.internal.Client; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.core.TimeValue; @@ -200,9 +201,18 @@ public void startDeployment(TrainedModelDeploymentTask task, ActionListener processContext.startAndLoad(modelConfig.getLocation(), modelLoadedListener) - ); + executorServiceForDeployment.execute(new AbstractRunnable() { + + @Override + public void onFailure(Exception e) { + failedDeploymentListener.onFailure(e); + } + + @Override + protected void doRun() { + processContext.startAndLoad(modelConfig.getLocation(), modelLoadedListener); + } + }); }, failedDeploymentListener::onFailure) ); } else { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/pytorch/process/NativePyTorchProcessFactory.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/pytorch/process/NativePyTorchProcessFactory.java index b26c6720ed179..e538a6c686881 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/pytorch/process/NativePyTorchProcessFactory.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/pytorch/process/NativePyTorchProcessFactory.java @@ -89,7 +89,7 @@ public NativePyTorchProcess createProcess( process.start(executorService); } catch (IOException | EsRejectedExecutionException e) { String msg = "Failed to connect to pytorch process for job " + task.getDeploymentId(); - logger.error(msg); + logger.error(msg, e); try { IOUtils.close(process); } catch (IOException ioe) { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/pytorch/process/PyTorchProcessManager.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/pytorch/process/PyTorchProcessManager.java deleted file mode 100644 index c812e490217ed..0000000000000 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/pytorch/process/PyTorchProcessManager.java +++ /dev/null @@ -1,24 +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.ml.inference.pytorch.process; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; - -public class PyTorchProcessManager { - - private static final Logger logger = LogManager.getLogger(PyTorchProcessManager.class); - - public PyTorchProcessManager() { - - } - - public void start(String taskId) { - - } -} diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/queries/TextExpansionQueryBuilder.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/queries/TextExpansionQueryBuilder.java index 24383e51b0ed2..a392996fbb448 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/queries/TextExpansionQueryBuilder.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/queries/TextExpansionQueryBuilder.java @@ -144,7 +144,7 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws if (weightedTokensSupplier.get() == null) { return this; } - return weightedTokensToQuery(fieldName, weightedTokensSupplier.get(), queryRewriteContext); + return weightedTokensToQuery(fieldName, weightedTokensSupplier.get()); } CoordinatedInferenceAction.Request inferRequest = CoordinatedInferenceAction.Request.forTextInput( @@ -196,11 +196,7 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws return new TextExpansionQueryBuilder(this, textExpansionResultsSupplier); } - private QueryBuilder weightedTokensToQuery( - String fieldName, - TextExpansionResults textExpansionResults, - QueryRewriteContext queryRewriteContext - ) { + private QueryBuilder weightedTokensToQuery(String fieldName, TextExpansionResults textExpansionResults) { if (tokenPruningConfig != null) { WeightedTokensQueryBuilder weightedTokensQueryBuilder = new WeightedTokensQueryBuilder( fieldName, diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/action/TransportPutTrainedModelActionTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/action/TransportPutTrainedModelActionTests.java index 84c49ba95b522..6ed7a3311c94a 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/action/TransportPutTrainedModelActionTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/action/TransportPutTrainedModelActionTests.java @@ -16,8 +16,6 @@ import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.core.TimeValue; import org.elasticsearch.rest.RestStatus; @@ -115,8 +113,7 @@ public void testParseInferenceConfigFromModelPackage() throws IOException { assertNotNull(inferenceConfigMap); InferenceConfig parsedInferenceConfig = TransportPutTrainedModelAction.parseInferenceConfigFromModelPackage( Collections.singletonMap(inferenceConfig.getWriteableName(), inferenceConfigMap), - xContentRegistry(), - LoggingDeprecationHandler.INSTANCE + xContentRegistry() ); assertEquals(inferenceConfig, parsedInferenceConfig); @@ -278,7 +275,6 @@ private TransportPutTrainedModelAction createTransportPutTrainedModelAction() { doReturn(threadPool).when(mockClient).threadPool(); return new TransportPutTrainedModelAction( - Settings.EMPTY, mockTransportService, mockClusterService, threadPool, diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java index 3c3b06c84da2a..6c4aaeada74c7 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java @@ -15,6 +15,7 @@ import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.core.Strings; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.core.Tuple; import org.elasticsearch.test.XContentTestUtils; import org.elasticsearch.test.rest.ObjectPath; @@ -22,6 +23,7 @@ import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xcontent.json.JsonXContent; import org.elasticsearch.xpack.core.security.action.apikey.ApiKey; +import org.elasticsearch.xpack.core.security.action.apikey.ApiKeyTests; import org.elasticsearch.xpack.core.security.action.apikey.GetApiKeyResponse; import org.elasticsearch.xpack.core.security.action.apikey.GrantApiKeyAction; import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken; @@ -34,6 +36,7 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Collection; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -436,6 +439,23 @@ public void testBulkUpdateApiKey() throws IOException { doTestAuthenticationWithApiKey(apiKeyExpectingNoop.name, apiKeyExpectingNoop.id, apiKeyExpectingNoop.encoded); } + public void testBulkUpdateExpirationTimeApiKey() throws IOException { + final EncodedApiKey apiKey1 = createApiKey("my-api-key-name", Map.of()); + final EncodedApiKey apiKey2 = createApiKey("my-other-api-key-name", Map.of()); + final var bulkUpdateApiKeyRequest = new Request("POST", "_security/api_key/_bulk_update"); + final TimeValue expiration = ApiKeyTests.randomFutureExpirationTime(); + bulkUpdateApiKeyRequest.setJsonEntity( + XContentTestUtils.convertToXContent(Map.of("ids", List.of(apiKey1.id, apiKey2.id), "expiration", expiration), XContentType.JSON) + .utf8ToString() + ); + final Response bulkUpdateApiKeyResponse = performRequestUsingRandomAuthMethod(bulkUpdateApiKeyRequest); + assertOK(bulkUpdateApiKeyResponse); + final Map response = responseAsMap(bulkUpdateApiKeyResponse); + assertEquals(List.of(apiKey1.id(), apiKey2.id()), response.get("updated")); + assertNull(response.get("errors")); + assertEquals(List.of(), response.get("noops")); + } + public void testGrantTargetCanUpdateApiKey() throws IOException { final var request = new Request("POST", "_security/api_key/grant"); request.setOptions( @@ -923,7 +943,7 @@ public void testUpdateCrossClusterApiKey() throws IOException { final ObjectPath createResponse = assertOKAndCreateObjectPath(client().performRequest(createRequest)); final String apiKeyId = createResponse.evaluate("id"); - // Update both access and metadata + // Update access, metadata and expiration final Request updateRequest1 = new Request("PUT", "/_security/cross_cluster/api_key/" + apiKeyId); updateRequest1.setJsonEntity(""" { @@ -940,7 +960,8 @@ public void testUpdateCrossClusterApiKey() throws IOException { } ] }, - "metadata": { "tag": "shared", "points": 0 } + "metadata": { "tag": "shared", "points": 0 }, + "expiration": "30d" }"""); setUserForRequest(updateRequest1, MANAGE_SECURITY_USER, END_USER_PASSWORD); final ObjectPath updateResponse1 = assertOKAndCreateObjectPath(client().performRequest(updateRequest1)); @@ -966,6 +987,7 @@ public void testUpdateCrossClusterApiKey() throws IOException { fetchResponse1.evaluate("api_keys.0.role_descriptors"), equalTo(Map.of("cross_cluster", XContentTestUtils.convertToMap(updatedRoleDescriptor1))) ); + assertThat(fetchResponse1.evaluate("api_keys.0.expiration"), notNullValue()); assertThat(fetchResponse1.evaluate("api_keys.0.access"), equalTo(XContentHelper.convertToMap(JsonXContent.jsonXContent, """ { "search": [ @@ -1465,6 +1487,40 @@ private void doTestAuthenticationWithApiKey(final String apiKeyName, final Strin assertThat(authenticate, hasEntry("api_key", Map.of("id", apiKeyId, "name", apiKeyName))); } + private static Map getRandomUpdateApiKeyRequestBody( + final Map oldMetadata, + boolean updateExpiration, + boolean updateMetadata + ) { + return getRandomUpdateApiKeyRequestBody(oldMetadata, updateExpiration, updateMetadata, List.of()); + } + + private static Map getRandomUpdateApiKeyRequestBody( + final Map oldMetadata, + boolean updateExpiration, + boolean updateMetadata, + List ids + ) { + Map updateRequestBody = new HashMap<>(); + + if (updateMetadata) { + updateRequestBody.put("metadata", Map.of("not", "returned (changed)", "foo", "bar")); + } else if (oldMetadata != null) { + updateRequestBody.put("metadata", oldMetadata); + } + + if (updateExpiration) { + updateRequestBody.put("expiration", ApiKeyTests.randomFutureExpirationTime()); + } + + if (ids.isEmpty() == false) { + updateRequestBody.put("ids", ids); + } + + return updateRequestBody; + } + + @SuppressWarnings({ "unchecked" }) private void doTestUpdateApiKey( final String apiKeyName, final String apiKeyId, @@ -1472,19 +1528,17 @@ private void doTestUpdateApiKey( final Map oldMetadata ) throws IOException { final var updateApiKeyRequest = new Request("PUT", "_security/api_key/" + apiKeyId); - final boolean updated = randomBoolean(); - final Map expectedApiKeyMetadata = updated ? Map.of("not", "returned (changed)", "foo", "bar") : oldMetadata; - final Map updateApiKeyRequestBody = expectedApiKeyMetadata == null - ? Map.of() - : Map.of("metadata", expectedApiKeyMetadata); - updateApiKeyRequest.setJsonEntity(XContentTestUtils.convertToXContent(updateApiKeyRequestBody, XContentType.JSON).utf8ToString()); + final boolean updateExpiration = randomBoolean(); + final boolean updateMetadata = randomBoolean(); + final Map updateRequestBody = getRandomUpdateApiKeyRequestBody(oldMetadata, updateExpiration, updateMetadata); + updateApiKeyRequest.setJsonEntity(XContentTestUtils.convertToXContent(updateRequestBody, XContentType.JSON).utf8ToString()); final Response updateApiKeyResponse = performRequestUsingRandomAuthMethod(updateApiKeyRequest); assertOK(updateApiKeyResponse); final Map updateApiKeyResponseMap = responseAsMap(updateApiKeyResponse); - assertEquals(updated, updateApiKeyResponseMap.get("updated")); - expectMetadata(apiKeyId, expectedApiKeyMetadata == null ? Map.of() : expectedApiKeyMetadata); + assertEquals(updateMetadata || updateExpiration, updateApiKeyResponseMap.get("updated")); + expectMetadata(apiKeyId, (Map) updateRequestBody.get("metadata")); // validate authentication still works after update doTestAuthenticationWithApiKey(apiKeyName, apiKeyId, apiKeyEncoded); } @@ -1497,28 +1551,29 @@ private void doTestUpdateApiKeyUsingBulkAction( final Map oldMetadata ) throws IOException { final var bulkUpdateApiKeyRequest = new Request("POST", "_security/api_key/_bulk_update"); - final boolean updated = randomBoolean(); - final Map expectedApiKeyMetadata = updated ? Map.of("not", "returned (changed)", "foo", "bar") : oldMetadata; - final Map bulkUpdateApiKeyRequestBody = expectedApiKeyMetadata == null - ? Map.of("ids", List.of(apiKeyId)) - : Map.of("ids", List.of(apiKeyId), "metadata", expectedApiKeyMetadata); - bulkUpdateApiKeyRequest.setJsonEntity( - XContentTestUtils.convertToXContent(bulkUpdateApiKeyRequestBody, XContentType.JSON).utf8ToString() + boolean updateMetadata = randomBoolean(); + boolean updateExpiration = randomBoolean(); + Map updateRequestBody = getRandomUpdateApiKeyRequestBody( + oldMetadata, + updateExpiration, + updateMetadata, + List.of(apiKeyId) ); + bulkUpdateApiKeyRequest.setJsonEntity(XContentTestUtils.convertToXContent(updateRequestBody, XContentType.JSON).utf8ToString()); final Response bulkUpdateApiKeyResponse = performRequestUsingRandomAuthMethod(bulkUpdateApiKeyRequest); assertOK(bulkUpdateApiKeyResponse); final Map bulkUpdateApiKeyResponseMap = responseAsMap(bulkUpdateApiKeyResponse); assertThat(bulkUpdateApiKeyResponseMap, not(hasKey("errors"))); - if (updated) { + if (updateMetadata || updateExpiration) { assertThat((List) bulkUpdateApiKeyResponseMap.get("noops"), empty()); assertThat((List) bulkUpdateApiKeyResponseMap.get("updated"), contains(apiKeyId)); } else { assertThat((List) bulkUpdateApiKeyResponseMap.get("updated"), empty()); assertThat((List) bulkUpdateApiKeyResponseMap.get("noops"), contains(apiKeyId)); } - expectMetadata(apiKeyId, expectedApiKeyMetadata == null ? Map.of() : expectedApiKeyMetadata); + expectMetadata(apiKeyId, (Map) updateRequestBody.get("metadata")); // validate authentication still works after update doTestAuthenticationWithApiKey(apiKeyName, apiKeyId, apiKeyEncoded); } @@ -1604,7 +1659,6 @@ private String headerFromRandomAuthMethod(final String username, final SecureStr } } - @SuppressWarnings({ "unchecked" }) private void expectMetadata(final String apiKeyId, final Map expectedMetadata) throws IOException { final var request = new Request("GET", "_security/api_key/"); request.addParameter("id", apiKeyId); @@ -1613,7 +1667,8 @@ private void expectMetadata(final String apiKeyId, final Map exp try (XContentParser parser = responseAsParser(response)) { final var apiKeyResponse = GetApiKeyResponse.fromXContent(parser); assertThat(apiKeyResponse.getApiKeyInfos().length, equalTo(1)); - assertThat(apiKeyResponse.getApiKeyInfos()[0].getMetadata(), equalTo(expectedMetadata)); + // ApiKey metadata is set to empty Map if null + assertThat(apiKeyResponse.getApiKeyInfos()[0].getMetadata(), equalTo(expectedMetadata == null ? Map.of() : expectedMetadata)); } } diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java index 72a6b6049932c..1329158f57d4d 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java @@ -1959,7 +1959,7 @@ public void testUpdateApiKeysForSingleKey() throws Exception { null ) ); - final var request = new UpdateApiKeyRequest(apiKeyId, newRoleDescriptors, ApiKeyTests.randomMetadata()); + final var request = new UpdateApiKeyRequest(apiKeyId, newRoleDescriptors, ApiKeyTests.randomMetadata(), null); final UpdateApiKeyResponse response = updateSingleApiKeyMaybeUsingBulkAction(TEST_USER_NAME, request); @@ -2030,7 +2030,7 @@ public void testBulkUpdateApiKeysForMultipleKeys() throws ExecutionException, In BulkUpdateApiKeyResponse response = executeBulkUpdateApiKey( TEST_USER_NAME, - new BulkUpdateApiKeyRequest(apiKeyIds, newRoleDescriptors, newMetadata) + new BulkUpdateApiKeyRequest(apiKeyIds, newRoleDescriptors, newMetadata, ApiKeyTests.randomFutureExpirationTime()) ); assertNotNull(response); @@ -2070,7 +2070,7 @@ public void testBulkUpdateApiKeysForMultipleKeys() throws ExecutionException, In () -> randomValueOtherThanMany(apiKeyIds::contains, () -> randomAlphaOfLength(10)) ); newIds.addAll(notFoundIds); - final BulkUpdateApiKeyRequest request = new BulkUpdateApiKeyRequest(shuffledList(newIds), newRoleDescriptors, newMetadata); + final BulkUpdateApiKeyRequest request = new BulkUpdateApiKeyRequest(shuffledList(newIds), newRoleDescriptors, newMetadata, null); response = executeBulkUpdateApiKey(TEST_USER_NAME, request); @@ -2100,7 +2100,8 @@ public void testBulkUpdateApiKeysForMultipleKeys() throws ExecutionException, In final BulkUpdateApiKeyRequest requestWithSomeErrors = new BulkUpdateApiKeyRequest( shuffledList(apiKeyIds), randomValueOtherThan(null, this::randomRoleDescriptors), - randomValueOtherThan(null, ApiKeyTests::randomMetadata) + randomValueOtherThan(null, ApiKeyTests::randomMetadata), + ApiKeyTests.randomFutureExpirationTime() ); response = executeBulkUpdateApiKey(TEST_USER_NAME, requestWithSomeErrors); @@ -2124,7 +2125,7 @@ public void testBulkUpdateApiKeysWithDuplicates() throws ExecutionException, Int BulkUpdateApiKeyResponse response = executeBulkUpdateApiKey( TEST_USER_NAME, - new BulkUpdateApiKeyRequest(idsWithDuplicates, newRoleDescriptors, newMetadata) + new BulkUpdateApiKeyRequest(idsWithDuplicates, newRoleDescriptors, newMetadata, ApiKeyTests.randomFutureExpirationTime()) ); assertNotNull(response); @@ -2142,7 +2143,12 @@ public void testBulkUpdateApiKeysWithDuplicates() throws ExecutionException, Int response = executeBulkUpdateApiKey( TEST_USER_NAME, - new BulkUpdateApiKeyRequest(notFoundIdsWithDuplicates, newRoleDescriptors, newMetadata) + new BulkUpdateApiKeyRequest( + notFoundIdsWithDuplicates, + newRoleDescriptors, + newMetadata, + ApiKeyTests.randomFutureExpirationTime() + ) ); assertNotNull(response); @@ -2317,7 +2323,12 @@ public void testUpdateApiKeysNotFoundScenarios() throws Exception { final Tuple> createdApiKey = createApiKey(TEST_USER_NAME, null); final var apiKeyId = createdApiKey.v1().getId(); final var expectedRoleDescriptor = new RoleDescriptor(randomAlphaOfLength(10), new String[] { "all" }, null, null); - final var request = new UpdateApiKeyRequest(apiKeyId, List.of(expectedRoleDescriptor), ApiKeyTests.randomMetadata()); + final var request = new UpdateApiKeyRequest( + apiKeyId, + List.of(expectedRoleDescriptor), + ApiKeyTests.randomMetadata(), + ApiKeyTests.randomFutureExpirationTime() + ); // Validate can update own API key final UpdateApiKeyResponse response = updateSingleApiKeyMaybeUsingBulkAction(TEST_USER_NAME, request); @@ -2326,12 +2337,24 @@ public void testUpdateApiKeysNotFoundScenarios() throws Exception { // Test not found exception on non-existent API key final var otherApiKeyId = randomValueOtherThan(apiKeyId, () -> randomAlphaOfLength(20)); - doTestUpdateApiKeysNotFound(new UpdateApiKeyRequest(otherApiKeyId, request.getRoleDescriptors(), request.getMetadata())); + doTestUpdateApiKeysNotFound( + new UpdateApiKeyRequest( + otherApiKeyId, + request.getRoleDescriptors(), + request.getMetadata(), + ApiKeyTests.randomFutureExpirationTime() + ) + ); // Test not found exception on other user's API key final Tuple> otherUsersApiKey = createApiKey("user_with_manage_api_key_role", null); doTestUpdateApiKeysNotFound( - new UpdateApiKeyRequest(otherUsersApiKey.v1().getId(), request.getRoleDescriptors(), request.getMetadata()) + new UpdateApiKeyRequest( + otherUsersApiKey.v1().getId(), + request.getRoleDescriptors(), + request.getMetadata(), + ApiKeyTests.randomFutureExpirationTime() + ) ); // Test not found exception on API key of user with the same username but from a different realm @@ -2351,7 +2374,12 @@ public void testUpdateApiKeysNotFoundScenarios() throws Exception { "all" ).v1().get(0); doTestUpdateApiKeysNotFound( - new UpdateApiKeyRequest(apiKeyForNativeRealmUser.getId(), request.getRoleDescriptors(), request.getMetadata()) + new UpdateApiKeyRequest( + apiKeyForNativeRealmUser.getId(), + request.getRoleDescriptors(), + request.getMetadata(), + ApiKeyTests.randomFutureExpirationTime() + ) ); } @@ -2364,7 +2392,12 @@ public void testInvalidUpdateApiKeysScenarios() throws ExecutionException, Inter final var apiKeyId = createdApiKey.getId(); final var roleDescriptor = new RoleDescriptor(randomAlphaOfLength(10), new String[] { "manage_own_api_key" }, null, null); - final var request = new UpdateApiKeyRequest(apiKeyId, List.of(roleDescriptor), ApiKeyTests.randomMetadata()); + final var request = new UpdateApiKeyRequest( + apiKeyId, + List.of(roleDescriptor), + ApiKeyTests.randomMetadata(), + ApiKeyTests.randomFutureExpirationTime() + ); final PlainActionFuture updateListener = new PlainActionFuture<>(); client().filterWithHeader( Collections.singletonMap( @@ -2465,7 +2498,8 @@ public void testUpdateApiKeysNoopScenarios() throws Exception { List.of(new RoleDescriptor(randomAlphaOfLength(10), new String[] { "all" }, null, null)), // Ensure not `null` to set metadata since we use the initialRequest further down in the test to ensure that // metadata updates are non-noops - randomValueOtherThanMany(Objects::isNull, ApiKeyTests::randomMetadata) + randomValueOtherThanMany(Objects::isNull, ApiKeyTests::randomMetadata), + null // Expiration is relative current time, so must be null to cause noop ); UpdateApiKeyResponse response = updateSingleApiKeyMaybeUsingBulkAction(TEST_USER_NAME, initialRequest); assertNotNull(response); @@ -2501,14 +2535,17 @@ public void testUpdateApiKeysNoopScenarios() throws Exception { () -> RoleDescriptorTests.randomRoleDescriptor(false) ) ); - response = updateSingleApiKeyMaybeUsingBulkAction(TEST_USER_NAME, new UpdateApiKeyRequest(apiKeyId, newRoleDescriptors, null)); + response = updateSingleApiKeyMaybeUsingBulkAction( + TEST_USER_NAME, + new UpdateApiKeyRequest(apiKeyId, newRoleDescriptors, null, null) + ); assertNotNull(response); assertTrue(response.isUpdated()); // Update with re-ordered role descriptors is a noop response = updateSingleApiKeyMaybeUsingBulkAction( TEST_USER_NAME, - new UpdateApiKeyRequest(apiKeyId, List.of(newRoleDescriptors.get(1), newRoleDescriptors.get(0)), null) + new UpdateApiKeyRequest(apiKeyId, List.of(newRoleDescriptors.get(1), newRoleDescriptors.get(0)), null, null) ); assertNotNull(response); assertFalse(response.isUpdated()); @@ -2519,7 +2556,8 @@ public void testUpdateApiKeysNoopScenarios() throws Exception { new UpdateApiKeyRequest( apiKeyId, null, - randomValueOtherThanMany(md -> md == null || md.equals(initialRequest.getMetadata()), ApiKeyTests::randomMetadata) + randomValueOtherThanMany(md -> md == null || md.equals(initialRequest.getMetadata()), ApiKeyTests::randomMetadata), + null ) ); assertNotNull(response); @@ -2677,7 +2715,8 @@ public void testUpdateApiKeysClearsApiKeyDocCache() throws Exception { apiKey1.v1(), List.of(), // Set metadata to ensure update - Map.of(randomAlphaOfLength(5), randomAlphaOfLength(10)) + Map.of(randomAlphaOfLength(5), randomAlphaOfLength(10)), + ApiKeyTests.randomFutureExpirationTime() ) ); @@ -3251,7 +3290,12 @@ private UpdateApiKeyResponse updateSingleApiKeyMaybeUsingBulkAction(final String if (useBulkAction) { final BulkUpdateApiKeyResponse response = executeBulkUpdateApiKey( username, - new BulkUpdateApiKeyRequest(List.of(request.getId()), request.getRoleDescriptors(), request.getMetadata()) + new BulkUpdateApiKeyRequest( + List.of(request.getId()), + request.getRoleDescriptors(), + request.getMetadata(), + request.getExpiration() + ) ); return toUpdateApiKeyResponse(request.getId(), response); } else { diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/apikey/ApiKeySingleNodeTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/apikey/ApiKeySingleNodeTests.java index 91884086af959..33cd3de9e0685 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/apikey/ApiKeySingleNodeTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/apikey/ApiKeySingleNodeTests.java @@ -37,6 +37,7 @@ import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.security.action.Grant; import org.elasticsearch.xpack.core.security.action.apikey.ApiKey; +import org.elasticsearch.xpack.core.security.action.apikey.ApiKeyTests; import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyAction; import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyRequest; import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyRequestBuilder; @@ -706,7 +707,13 @@ public void testUpdateCrossClusterApiKey() throws IOException { updateMetadata = null; } - final var updateApiKeyRequest = new UpdateCrossClusterApiKeyRequest(apiKeyId, roleDescriptorBuilder, updateMetadata); + final boolean shouldUpdateExpiration = randomBoolean(); + TimeValue expiration = null; + if (shouldUpdateExpiration) { + ApiKeyTests.randomFutureExpirationTime(); + } + + final var updateApiKeyRequest = new UpdateCrossClusterApiKeyRequest(apiKeyId, roleDescriptorBuilder, updateMetadata, expiration); final UpdateApiKeyResponse updateApiKeyResponse = client().execute(UpdateCrossClusterApiKeyAction.INSTANCE, updateApiKeyRequest) .actionGet(); diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/pki/PkiOptionalClientAuthTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/pki/PkiOptionalClientAuthTests.java index 07f35e499d43a..fa38ce7314494 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/pki/PkiOptionalClientAuthTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/pki/PkiOptionalClientAuthTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.security.authc.pki; import org.apache.http.nio.conn.ssl.SSLIOSessionStrategy; +import org.apache.lucene.util.Constants; import org.elasticsearch.client.Request; import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.Response; @@ -34,11 +35,13 @@ public class PkiOptionalClientAuthTests extends SecuritySingleNodeTestCase { + private static final int NUMBER_OF_CLIENT_PORTS = Constants.WINDOWS ? 300 : 100; + private static int randomClientPort; @BeforeClass public static void initPort() { - randomClientPort = randomIntBetween(49000, 65500); + randomClientPort = randomIntBetween(49152, 65535 - NUMBER_OF_CLIENT_PORTS); } @Override @@ -47,7 +50,7 @@ protected boolean addMockHttpTransport() { } protected Settings nodeSettings() { - String randomClientPortRange = randomClientPort + "-" + (randomClientPort + 100); + String randomClientPortRange = randomClientPort + "-" + (randomClientPort + NUMBER_OF_CLIENT_PORTS); Settings.Builder builder = Settings.builder() .put(super.nodeSettings()) diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/transport/filter/IpFilteringIntegrationTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/transport/filter/IpFilteringIntegrationTests.java index 16e0b322efcac..f479c4703194b 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/transport/filter/IpFilteringIntegrationTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/transport/filter/IpFilteringIntegrationTests.java @@ -6,6 +6,7 @@ */ package org.elasticsearch.xpack.security.transport.filter; +import org.apache.lucene.util.Constants; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.core.SuppressForbidden; @@ -28,11 +29,14 @@ // no client nodes as they all get rejected on network connections @ClusterScope(scope = Scope.SUITE, numDataNodes = 0, numClientNodes = 0) public class IpFilteringIntegrationTests extends SecurityIntegTestCase { + + private static final int NUMBER_OF_CLIENT_PORTS = Constants.WINDOWS ? 300 : 100; + private static int randomClientPort; @BeforeClass public static void getRandomPort() { - randomClientPort = randomIntBetween(49000, 65500); // ephemeral port + randomClientPort = randomIntBetween(49152, 65535 - NUMBER_OF_CLIENT_PORTS); // ephemeral port } @Override @@ -42,7 +46,7 @@ protected boolean addMockHttpTransport() { @Override protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) { - String randomClientPortRange = randomClientPort + "-" + (randomClientPort + 100); + String randomClientPortRange = randomClientPort + "-" + (randomClientPort + NUMBER_OF_CLIENT_PORTS); return Settings.builder() .put(super.nodeSettings(nodeOrdinal, otherSettings)) .put("transport.profiles.client.port", randomClientPortRange) diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/transport/filter/IpFilteringUpdateTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/transport/filter/IpFilteringUpdateTests.java index d5cb0f165b89d..0b1d33cb35c97 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/transport/filter/IpFilteringUpdateTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/transport/filter/IpFilteringUpdateTests.java @@ -6,6 +6,7 @@ */ package org.elasticsearch.xpack.security.transport.filter; +import org.apache.lucene.util.Constants; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.Strings; @@ -26,13 +27,15 @@ @ClusterScope(scope = TEST, supportsDedicatedMasters = false, numDataNodes = 1) public class IpFilteringUpdateTests extends SecurityIntegTestCase { + private static final int NUMBER_OF_CLIENT_PORTS = Constants.WINDOWS ? 300 : 100; + private static int randomClientPort; private final boolean httpEnabled = randomBoolean(); @BeforeClass public static void getRandomPort() { - randomClientPort = randomIntBetween(49000, 65500); + randomClientPort = randomIntBetween(49152, 65535 - NUMBER_OF_CLIENT_PORTS); } @Override @@ -42,7 +45,7 @@ protected boolean addMockHttpTransport() { @Override protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) { - String randomClientPortRange = randomClientPort + "-" + (randomClientPort + 100); + String randomClientPortRange = randomClientPort + "-" + (randomClientPort + NUMBER_OF_CLIENT_PORTS); return Settings.builder() .put(super.nodeSettings(nodeOrdinal, otherSettings)) .put("xpack.security.transport.filter.deny", "127.0.0.200") diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportUpdateCrossClusterApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportUpdateCrossClusterApiKeyAction.java index 011b95565e030..a47bbb0301ebc 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportUpdateCrossClusterApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/apikey/TransportUpdateCrossClusterApiKeyAction.java @@ -50,7 +50,12 @@ void doExecuteUpdate( ) { apiKeyService.updateApiKeys( authentication, - new BaseBulkUpdateApiKeyRequest(List.of(request.getId()), request.getRoleDescriptors(), request.getMetadata()) { + new BaseBulkUpdateApiKeyRequest( + List.of(request.getId()), + request.getRoleDescriptors(), + request.getMetadata(), + request.getExpiration() + ) { @Override public ApiKey.Type getType() { return ApiKey.Type.CROSS_CLUSTER; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java index 98e17ad022483..6d9282618cdfb 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java @@ -410,7 +410,7 @@ private void createApiKeyAndIndexIt( ActionListener listener ) { final Instant created = clock.instant(); - final Instant expiration = getApiKeyExpiration(created, request); + final Instant expiration = getApiKeyExpiration(created, request.getExpiration()); final SecureString apiKey = UUIDs.randomBase64UUIDSecureString(); assert ApiKey.Type.CROSS_CLUSTER != request.getType() || API_KEY_SECRET_LENGTH == apiKey.length(); final Version version = clusterService.state().nodes().getMinNodeVersion(); @@ -751,7 +751,8 @@ static XContentBuilder maybeBuildUpdatedDocument( final Version targetDocVersion, final Authentication authentication, final BaseUpdateApiKeyRequest request, - final Set userRoleDescriptors + final Set userRoleDescriptors, + final Clock clock ) throws IOException { assert currentApiKeyDoc.type == request.getType(); if (isNoop(apiKeyId, currentApiKeyDoc, targetDocVersion, authentication, request, userRoleDescriptors)) { @@ -763,9 +764,14 @@ static XContentBuilder maybeBuildUpdatedDocument( .field("doc_type", "api_key") .field("type", currentApiKeyDoc.type.value()) .field("creation_time", currentApiKeyDoc.creationTime) - .field("expiration_time", currentApiKeyDoc.expirationTime == -1 ? null : currentApiKeyDoc.expirationTime) .field("api_key_invalidated", false); + if (request.getExpiration() != null) { + builder.field("expiration_time", getApiKeyExpiration(clock.instant(), request.getExpiration()).toEpochMilli()); + } else { + builder.field("expiration_time", currentApiKeyDoc.expirationTime == -1 ? null : currentApiKeyDoc.expirationTime); + } + addApiKeyHash(builder, currentApiKeyDoc.hash.toCharArray()); final List keyRoles = request.getRoleDescriptors(); @@ -816,6 +822,11 @@ private static boolean isNoop( return false; } + if (request.getExpiration() != null) { + // Since expiration is relative current time, it's not likely that it matches the stored value to the ms, so assume update + return false; + } + final Map currentCreator = apiKeyDoc.creator; final var user = authentication.getEffectiveSubject().getUser(); final var sourceRealm = authentication.getEffectiveSubject().getRealm(); @@ -1288,9 +1299,9 @@ protected void verifyKeyAgainstHash(String apiKeyHash, ApiKeyCredentials credent })); } - private static Instant getApiKeyExpiration(Instant now, AbstractCreateApiKeyRequest request) { - if (request.getExpiration() != null) { - return now.plusSeconds(request.getExpiration().getSeconds()); + private static Instant getApiKeyExpiration(Instant now, @Nullable TimeValue expiration) { + if (expiration != null) { + return now.plusSeconds(expiration.getSeconds()); } else { return null; } @@ -1480,7 +1491,8 @@ private IndexRequest maybeBuildIndexRequest( targetDocVersion, authentication, request, - userRoleDescriptors + userRoleDescriptors, + clock ); final boolean isNoop = builder == null; return isNoop diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestBulkUpdateApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestBulkUpdateApiKeyAction.java index c436370d67579..584ad08704ddd 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestBulkUpdateApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestBulkUpdateApiKeyAction.java @@ -9,6 +9,7 @@ import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.Scope; @@ -35,7 +36,12 @@ public final class RestBulkUpdateApiKeyAction extends ApiKeyBaseRestHandler { @SuppressWarnings("unchecked") static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "bulk_update_api_key_request", - a -> new BulkUpdateApiKeyRequest((List) a[0], (List) a[1], (Map) a[2]) + a -> new BulkUpdateApiKeyRequest( + (List) a[0], + (List) a[1], + (Map) a[2], + TimeValue.parseTimeValue((String) a[3], null, "expiration") + ) ); static { @@ -45,6 +51,7 @@ public final class RestBulkUpdateApiKeyAction extends ApiKeyBaseRestHandler { return RoleDescriptor.parse(n, p, false); }, new ParseField("role_descriptors")); PARSER.declareObject(optionalConstructorArg(), (p, c) -> p.map(), new ParseField("metadata")); + PARSER.declareString(optionalConstructorArg(), new ParseField("expiration")); } public RestBulkUpdateApiKeyAction(final Settings settings, final XPackLicenseState licenseState) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestUpdateApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestUpdateApiKeyAction.java index 16c323eaca76e..d64e7f4007387 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestUpdateApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestUpdateApiKeyAction.java @@ -9,6 +9,7 @@ import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.Scope; @@ -33,7 +34,11 @@ public final class RestUpdateApiKeyAction extends ApiKeyBaseRestHandler { @SuppressWarnings("unchecked") static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "update_api_key_request_payload", - a -> new Payload((List) a[0], (Map) a[1]) + a -> new Payload( + (List) a[0], + (Map) a[1], + TimeValue.parseTimeValue((String) a[2], null, "expiration") + ) ); static { @@ -42,6 +47,7 @@ public final class RestUpdateApiKeyAction extends ApiKeyBaseRestHandler { return RoleDescriptor.parse(n, p, false); }, new ParseField("role_descriptors")); PARSER.declareObject(optionalConstructorArg(), (p, c) -> p.map(), new ParseField("metadata")); + PARSER.declareString(optionalConstructorArg(), new ParseField("expiration")); } public RestUpdateApiKeyAction(final Settings settings, final XPackLicenseState licenseState) { @@ -64,13 +70,13 @@ protected RestChannelConsumer innerPrepareRequest(final RestRequest request, fin // `RestClearApiKeyCacheAction` and our current REST implementation requires that path params have the same wildcard if their paths // share a prefix final var apiKeyId = request.param("ids"); - final var payload = request.hasContent() == false ? new Payload(null, null) : PARSER.parse(request.contentParser(), null); + final var payload = request.hasContent() == false ? new Payload(null, null, null) : PARSER.parse(request.contentParser(), null); return channel -> client.execute( UpdateApiKeyAction.INSTANCE, - new UpdateApiKeyRequest(apiKeyId, payload.roleDescriptors, payload.metadata), + new UpdateApiKeyRequest(apiKeyId, payload.roleDescriptors, payload.metadata, payload.expiration), new RestToXContentListener<>(channel) ); } - record Payload(List roleDescriptors, Map metadata) {} + record Payload(List roleDescriptors, Map metadata, TimeValue expiration) {} } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestUpdateCrossClusterApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestUpdateCrossClusterApiKeyAction.java index a642723667639..e9244eaea0ec5 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestUpdateCrossClusterApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestUpdateCrossClusterApiKeyAction.java @@ -9,6 +9,7 @@ import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.license.LicenseUtils; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.rest.RestRequest; @@ -32,12 +33,17 @@ public final class RestUpdateCrossClusterApiKeyAction extends ApiKeyBaseRestHand @SuppressWarnings("unchecked") static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "update_cross_cluster_api_key_request_payload", - a -> new Payload((CrossClusterApiKeyRoleDescriptorBuilder) a[0], (Map) a[1]) + a -> new Payload( + (CrossClusterApiKeyRoleDescriptorBuilder) a[0], + (Map) a[1], + TimeValue.parseTimeValue((String) a[2], null, "expiration") + ) ); static { PARSER.declareObject(optionalConstructorArg(), CrossClusterApiKeyRoleDescriptorBuilder.PARSER, new ParseField("access")); PARSER.declareObject(optionalConstructorArg(), (p, c) -> p.map(), new ParseField("metadata")); + PARSER.declareString(optionalConstructorArg(), new ParseField("expiration")); } public RestUpdateCrossClusterApiKeyAction(final Settings settings, final XPackLicenseState licenseState) { @@ -61,7 +67,7 @@ protected RestChannelConsumer innerPrepareRequest(final RestRequest request, fin return channel -> client.execute( UpdateCrossClusterApiKeyAction.INSTANCE, - new UpdateCrossClusterApiKeyRequest(apiKeyId, payload.builder, payload.metadata), + new UpdateCrossClusterApiKeyRequest(apiKeyId, payload.builder, payload.metadata, payload.expiration), new RestToXContentListener<>(channel) ); } @@ -75,5 +81,5 @@ protected Exception innerCheckFeatureAvailable(RestRequest request) { } } - record Payload(CrossClusterApiKeyRoleDescriptorBuilder builder, Map metadata) {} + record Payload(CrossClusterApiKeyRoleDescriptorBuilder builder, Map metadata, TimeValue expiration) {} } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/apikey/TransportUpdateCrossClusterApiKeyActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/apikey/TransportUpdateCrossClusterApiKeyActionTests.java index 7ce920506d7d1..70190b70f3f1a 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/apikey/TransportUpdateCrossClusterApiKeyActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/apikey/TransportUpdateCrossClusterApiKeyActionTests.java @@ -74,7 +74,12 @@ public void testExecute() throws IOException { } final String id = randomAlphaOfLength(10); - final var request = new UpdateCrossClusterApiKeyRequest(id, roleDescriptorBuilder, metadata); + final var request = new UpdateCrossClusterApiKeyRequest( + id, + roleDescriptorBuilder, + metadata, + ApiKeyTests.randomFutureExpirationTime() + ); final int updateStatus = randomIntBetween(0, 2); // 0 - success, 1 - noop, 2 - error doAnswer(invocation -> { @@ -129,7 +134,12 @@ public void testAuthenticationCheck() { mock(ApiKeyService.class), securityContext ); - final var request = new UpdateCrossClusterApiKeyRequest(randomAlphaOfLength(10), null, Map.of()); + final var request = new UpdateCrossClusterApiKeyRequest( + randomAlphaOfLength(10), + null, + Map.of(), + ApiKeyTests.randomFutureExpirationTime() + ); // null authentication error when(securityContext.getAuthentication()).thenReturn(null); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java index 3385b02147890..9c48354b951d8 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java @@ -46,6 +46,7 @@ import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.core.XPackSettings; +import org.elasticsearch.xpack.core.security.action.apikey.ApiKeyTests; import org.elasticsearch.xpack.core.security.action.apikey.BulkUpdateApiKeyAction; import org.elasticsearch.xpack.core.security.action.apikey.BulkUpdateApiKeyRequest; import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyAction; @@ -629,7 +630,8 @@ public void testSecurityConfigChangeEventFormattingForRoles() throws IOException final var updateApiKeyRequest = new UpdateApiKeyRequest( keyId, randomBoolean() ? null : keyRoleDescriptors, - metadataWithSerialization.metadata() + metadataWithSerialization.metadata(), + ApiKeyTests.randomFutureExpirationTime() ); auditTrail.accessGranted(requestId, authentication, UpdateApiKeyAction.NAME, updateApiKeyRequest, authorizationInfo); final var expectedUpdateKeyAuditEventString = String.format( @@ -661,7 +663,8 @@ public void testSecurityConfigChangeEventFormattingForRoles() throws IOException final var bulkUpdateApiKeyRequest = new BulkUpdateApiKeyRequest( keyIds, randomBoolean() ? null : keyRoleDescriptors, - metadataWithSerialization.metadata() + metadataWithSerialization.metadata(), + ApiKeyTests.randomFutureExpirationTime() ); auditTrail.accessGranted(requestId, authentication, BulkUpdateApiKeyAction.NAME, bulkUpdateApiKeyRequest, authorizationInfo); final var expectedBulkUpdateKeyAuditEventString = String.format( @@ -875,7 +878,8 @@ public void testSecurityConfigChangeEventForCrossClusterApiKeys() throws IOExcep final var updateRequest = new UpdateCrossClusterApiKeyRequest( createRequest.getId(), updateAccess, - updateMetadataWithSerialization.metadata() + updateMetadataWithSerialization.metadata(), + ApiKeyTests.randomFutureExpirationTime() ); auditTrail.accessGranted(requestId, authentication, UpdateCrossClusterApiKeyAction.NAME, updateRequest, authorizationInfo); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java index 3f12c7d630dd3..7fd4f0dcd3598 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java @@ -2119,6 +2119,8 @@ public void testMaybeBuildUpdatedDocument() throws IOException { } else { oldKeyRoles = randomList(3, RoleDescriptorTests::randomRoleDescriptor); } + final long now = randomMillisUpToYear9999(); + when(clock.instant()).thenReturn(Instant.ofEpochMilli(now)); final Map oldMetadata = ApiKeyTests.randomMetadata(); final Version oldVersion = VersionUtils.randomVersion(random()); final ApiKeyDoc oldApiKeyDoc = ApiKeyDoc.fromXContent( @@ -2147,6 +2149,8 @@ public void testMaybeBuildUpdatedDocument() throws IOException { final boolean changeMetadata = randomBoolean(); final boolean changeVersion = randomBoolean(); final boolean changeCreator = randomBoolean(); + final boolean changeExpiration = randomBoolean(); + final Set newUserRoles = changeUserRoles ? randomValueOtherThan(oldUserRoles, () -> randomSet(0, 3, RoleDescriptorTests::randomRoleDescriptor)) : oldUserRoles; @@ -2180,11 +2184,14 @@ public void testMaybeBuildUpdatedDocument() throws IOException { .build(false) ) : oldAuthentication; + final TimeValue newExpiration = changeExpiration ? randomFrom(ApiKeyTests.randomFutureExpirationTime()) : null; final String apiKeyId = randomAlphaOfLength(10); final BaseUpdateApiKeyRequest request = mock(BaseUpdateApiKeyRequest.class); when(request.getType()).thenReturn(type); when(request.getRoleDescriptors()).thenReturn(newKeyRoles); when(request.getMetadata()).thenReturn(newMetadata); + when(request.getExpiration()).thenReturn(newExpiration); + final var service = createApiKeyService(); final XContentBuilder builder = ApiKeyService.maybeBuildUpdatedDocument( @@ -2193,10 +2200,16 @@ public void testMaybeBuildUpdatedDocument() throws IOException { newVersion, newAuthentication, request, - newUserRoles + newUserRoles, + clock ); - final boolean noop = (changeCreator || changeMetadata || changeKeyRoles || changeUserRoles || changeVersion) == false; + final boolean noop = (changeCreator + || changeMetadata + || changeKeyRoles + || changeUserRoles + || changeVersion + || changeExpiration) == false; if (noop) { assertNull(builder); } else { @@ -2207,7 +2220,6 @@ public void testMaybeBuildUpdatedDocument() throws IOException { assertEquals(oldApiKeyDoc.type, updatedApiKeyDoc.type); assertEquals(oldApiKeyDoc.name, updatedApiKeyDoc.name); assertEquals(oldApiKeyDoc.hash, updatedApiKeyDoc.hash); - assertEquals(oldApiKeyDoc.expirationTime, updatedApiKeyDoc.expirationTime); assertEquals(oldApiKeyDoc.creationTime, updatedApiKeyDoc.creationTime); assertEquals(oldApiKeyDoc.invalidated, updatedApiKeyDoc.invalidated); assertEquals(newVersion.id, updatedApiKeyDoc.version); @@ -2237,6 +2249,11 @@ public void testMaybeBuildUpdatedDocument() throws IOException { } else { assertEquals(newMetadata, XContentHelper.convertToMap(updatedApiKeyDoc.metadataFlattened, true, XContentType.JSON).v2()); } + if (newExpiration != null) { + assertEquals(clock.instant().plusSeconds(newExpiration.getSeconds()).toEpochMilli(), updatedApiKeyDoc.expirationTime); + } else { + assertEquals(oldApiKeyDoc.expirationTime, updatedApiKeyDoc.expirationTime); + } assertEquals(newAuthentication.getEffectiveSubject().getUser().principal(), updatedApiKeyDoc.creator.get("principal")); assertEquals(newAuthentication.getEffectiveSubject().getUser().fullName(), updatedApiKeyDoc.creator.get("full_name")); assertEquals(newAuthentication.getEffectiveSubject().getUser().email(), updatedApiKeyDoc.creator.get("email")); @@ -2602,7 +2619,8 @@ public void testCreateOrUpdateApiKeyWithWorkflowsRestrictionForUnsupportedVersio final BulkUpdateApiKeyRequest updateRequest = new BulkUpdateApiKeyRequest( randomList(1, 3, () -> randomAlphaOfLengthBetween(3, 5)), roleDescriptorsWithWorkflowsRestriction, - Map.of() + Map.of(), + ApiKeyTests.randomFutureExpirationTime() ); final PlainActionFuture updateFuture = new PlainActionFuture<>(); service.updateApiKeys(authentication, updateRequest, Set.of(), updateFuture); @@ -2664,7 +2682,8 @@ public void testValidateOwnerUserRoleDescriptorsWithWorkflowsRestriction() { final BulkUpdateApiKeyRequest updateRequest = new BulkUpdateApiKeyRequest( randomList(1, 3, () -> randomAlphaOfLengthBetween(3, 5)), requestRoleDescriptors, - Map.of() + Map.of(), + ApiKeyTests.randomFutureExpirationTime() ); final PlainActionFuture updateFuture = new PlainActionFuture<>(); service.updateApiKeys(authentication, updateRequest, userRoleDescriptorsWithWorkflowsRestriction, updateFuture); diff --git a/x-pack/plugin/slm/src/main/java/module-info.java b/x-pack/plugin/slm/src/main/java/module-info.java index 77fa369c7ef4b..bdfdbd85a434e 100644 --- a/x-pack/plugin/slm/src/main/java/module-info.java +++ b/x-pack/plugin/slm/src/main/java/module-info.java @@ -18,4 +18,6 @@ provides org.elasticsearch.reservedstate.ReservedClusterStateHandlerProvider with org.elasticsearch.xpack.slm.ReservedLifecycleStateHandlerProvider; + + provides org.elasticsearch.features.FeatureSpecification with org.elasticsearch.xpack.slm.SnapshotLifecycleFeatures; } diff --git a/x-pack/plugin/slm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycle.java b/x-pack/plugin/slm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycle.java index 74094c83d4bcb..693341f00ec01 100644 --- a/x-pack/plugin/slm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycle.java +++ b/x-pack/plugin/slm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycle.java @@ -124,6 +124,7 @@ public Collection createComponents(PluginServices services) { SnapshotLifecycleTemplateRegistry templateRegistry = new SnapshotLifecycleTemplateRegistry( settings, clusterService, + services.featureService(), threadPool, client, services.xContentRegistry() diff --git a/x-pack/plugin/slm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleFeatures.java b/x-pack/plugin/slm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleFeatures.java new file mode 100644 index 0000000000000..f3dfe4fb26f65 --- /dev/null +++ b/x-pack/plugin/slm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleFeatures.java @@ -0,0 +1,22 @@ +/* + * 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.slm; + +import org.elasticsearch.Version; +import org.elasticsearch.features.FeatureSpecification; +import org.elasticsearch.features.NodeFeature; +import org.elasticsearch.xpack.slm.history.SnapshotLifecycleTemplateRegistry; + +import java.util.Map; + +public class SnapshotLifecycleFeatures implements FeatureSpecification { + @Override + public Map getHistoricalFeatures() { + return Map.of(SnapshotLifecycleTemplateRegistry.MANAGED_BY_DATA_STREAM_LIFECYCLE, Version.V_8_12_0); + } +} diff --git a/x-pack/plugin/slm/src/main/java/org/elasticsearch/xpack/slm/history/SnapshotLifecycleTemplateRegistry.java b/x-pack/plugin/slm/src/main/java/org/elasticsearch/xpack/slm/history/SnapshotLifecycleTemplateRegistry.java index 31c624df67813..f40ea5a56463a 100644 --- a/x-pack/plugin/slm/src/main/java/org/elasticsearch/xpack/slm/history/SnapshotLifecycleTemplateRegistry.java +++ b/x-pack/plugin/slm/src/main/java/org/elasticsearch/xpack/slm/history/SnapshotLifecycleTemplateRegistry.java @@ -8,10 +8,13 @@ package org.elasticsearch.xpack.slm.history; import org.elasticsearch.client.internal.Client; +import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.ComposableIndexTemplate; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.features.FeatureService; +import org.elasticsearch.features.NodeFeature; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xpack.core.ilm.IndexLifecycleMetadata; @@ -44,11 +47,13 @@ public class SnapshotLifecycleTemplateRegistry extends IndexTemplateRegistry { // version 6: manage by data stream lifecycle // version 7: version the index template name so we can upgrade existing deployments public static final int INDEX_TEMPLATE_VERSION = 7; + public static final NodeFeature MANAGED_BY_DATA_STREAM_LIFECYCLE = new NodeFeature("slm-history-managed-by-dsl"); public static final String SLM_TEMPLATE_VERSION_VARIABLE = "xpack.slm.template.version"; public static final String SLM_TEMPLATE_NAME = ".slm-history-" + INDEX_TEMPLATE_VERSION; public static final String SLM_POLICY_NAME = "slm-history-ilm-policy"; + private final FeatureService featureService; @Override protected boolean requiresMasterNode() { @@ -60,11 +65,13 @@ protected boolean requiresMasterNode() { public SnapshotLifecycleTemplateRegistry( Settings nodeSettings, ClusterService clusterService, + FeatureService featureService, ThreadPool threadPool, Client client, NamedXContentRegistry xContentRegistry ) { super(nodeSettings, clusterService, threadPool, client, xContentRegistry); + this.featureService = featureService; slmHistoryEnabled = SLM_HISTORY_INDEX_ENABLED_SETTING.get(nodeSettings); } @@ -115,4 +122,9 @@ public boolean validate(ClusterState state) { boolean allPoliciesPresent = maybePolicies.map(policies -> policies.keySet().containsAll(policyNames)).orElse(false); return allTemplatesPresent && allPoliciesPresent; } + + @Override + protected boolean isClusterReady(ClusterChangedEvent event) { + return featureService.clusterHasFeature(event.state(), MANAGED_BY_DATA_STREAM_LIFECYCLE); + } } diff --git a/x-pack/plugin/slm/src/main/resources/META-INF/services/org.elasticsearch.features.FeatureSpecification b/x-pack/plugin/slm/src/main/resources/META-INF/services/org.elasticsearch.features.FeatureSpecification new file mode 100644 index 0000000000000..e8a2270235898 --- /dev/null +++ b/x-pack/plugin/slm/src/main/resources/META-INF/services/org.elasticsearch.features.FeatureSpecification @@ -0,0 +1,8 @@ +# +# 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. +# + +org.elasticsearch.xpack.slm.SnapshotLifecycleFeatures diff --git a/x-pack/plugin/slm/src/test/java/org/elasticsearch/xpack/slm/history/SnapshotLifecycleTemplateRegistryTests.java b/x-pack/plugin/slm/src/test/java/org/elasticsearch/xpack/slm/history/SnapshotLifecycleTemplateRegistryTests.java index 3601b3c010739..66c239bcda3c7 100644 --- a/x-pack/plugin/slm/src/test/java/org/elasticsearch/xpack/slm/history/SnapshotLifecycleTemplateRegistryTests.java +++ b/x-pack/plugin/slm/src/test/java/org/elasticsearch/xpack/slm/history/SnapshotLifecycleTemplateRegistryTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.TriFunction; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.features.FeatureService; import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.client.NoOpClient; @@ -46,6 +47,7 @@ import org.elasticsearch.xpack.core.ilm.RolloverAction; import org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType; import org.elasticsearch.xpack.core.ilm.action.PutLifecycleAction; +import org.elasticsearch.xpack.slm.SnapshotLifecycleFeatures; import org.junit.After; import org.junit.Before; @@ -98,7 +100,14 @@ public void createRegistryAndClient() { ) ); xContentRegistry = new NamedXContentRegistry(entries); - registry = new SnapshotLifecycleTemplateRegistry(Settings.EMPTY, clusterService, threadPool, client, xContentRegistry); + registry = new SnapshotLifecycleTemplateRegistry( + Settings.EMPTY, + clusterService, + new FeatureService(List.of(new SnapshotLifecycleFeatures())), + threadPool, + client, + xContentRegistry + ); } @After @@ -113,6 +122,7 @@ public void testDisabledDoesNotAddTemplates() { SnapshotLifecycleTemplateRegistry disabledRegistry = new SnapshotLifecycleTemplateRegistry( settings, clusterService, + new FeatureService(List.of(new SnapshotLifecycleFeatures())), threadPool, client, xContentRegistry