From 8d376c89cee9d5d4e97daee897fb9e3d57747b42 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 7 Nov 2024 11:21:38 +1100 Subject: [PATCH] ProjectResolver.getProjectId now takes no argument (MP-1749) A ProjectResolver should be able to fetch the ProjectId without the need of a cluster state. This PR changes the method to take no argument and fixes cascading changes. It also adds a separate ProjectIdResolver interface to host the new no-arg getProjectId method. The existing ProjectResolver interface extends the new interface. The major impact of the change is for tests and test helpers. It has been made more obvious that most tests rely on the default project ID. Previously this was hidden by the no arg `singleProjectOnly()` that just pops the only project from cluster state. This only project is the default project anyway in the current tests. The change just made this fact explicit which I think is not a bad thing. We can definitely tidy up the tests and test helpers once more pieces fall into places. Happy to take suggestions. --- .../core/FixForMultiProject.java | 5 + .../multiproject/MultiProjectResolver.java | 8 +- .../MultiProjectResolverTests.java | 6 +- .../health/TransportClusterHealthAction.java | 2 +- .../create/TransportCreateIndexAction.java | 3 +- ...rtDeleteComposableIndexTemplateAction.java | 2 +- ...sportPutComposableIndexTemplateAction.java | 2 +- .../bulk/TransportAbstractBulkAction.java | 2 +- .../info/TransportClusterInfoAction.java | 4 +- .../metadata/IndexNameExpressionResolver.java | 6 +- .../cluster/metadata/Metadata.java | 1 + .../project/DefaultProjectResolver.java | 26 ++- .../cluster/project/ProjectIdResolver.java | 30 +++ .../cluster/project/ProjectResolver.java | 30 ++- ...sportAnalyzeIndexDiskUsageActionTests.java | 2 +- .../indices/get/GetIndexActionTests.java | 2 +- .../settings/get/GetSettingsActionTests.java | 2 +- .../bulk/TransportBulkActionIngestTests.java | 2 +- .../action/bulk/TransportBulkActionTests.java | 22 +- .../bulk/TransportBulkActionTookTests.java | 4 +- .../TransportSimulateBulkActionTests.java | 2 +- ...TransportResyncReplicationActionTests.java | 2 +- .../TransportBroadcastByNodeActionTests.java | 2 +- .../TransportWriteActionTests.java | 4 +- ...ortInstanceSingleOperationActionTests.java | 2 +- .../seqno/RetentionLeaseSyncActionTests.java | 6 +- .../snapshots/SnapshotResiliencyTests.java | 12 +- .../cluster/project/TestProjectResolvers.java | 120 +++++------ .../TestIndexNameExpressionResolver.java | 9 +- .../project/TestProjectResolversTests.java | 202 +++++------------- .../authz/AuthorizationServiceTests.java | 29 ++- .../TransformGetCheckpointTests.java | 2 +- 32 files changed, 259 insertions(+), 294 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/cluster/project/ProjectIdResolver.java diff --git a/libs/core/src/main/java/org/elasticsearch/core/FixForMultiProject.java b/libs/core/src/main/java/org/elasticsearch/core/FixForMultiProject.java index f3d7d78e8907..f89d87f5151c 100644 --- a/libs/core/src/main/java/org/elasticsearch/core/FixForMultiProject.java +++ b/libs/core/src/main/java/org/elasticsearch/core/FixForMultiProject.java @@ -23,4 +23,9 @@ import java.lang.annotation.Target; { ElementType.LOCAL_VARIABLE, ElementType.CONSTRUCTOR, ElementType.FIELD, ElementType.METHOD, ElementType.TYPE, ElementType.MODULE } ) public @interface FixForMultiProject { + + /** + * Some explanation on what and why for the future fix + */ + String description() default ""; } diff --git a/modules/multi-project/src/main/java/org/elasticsearch/multiproject/MultiProjectResolver.java b/modules/multi-project/src/main/java/org/elasticsearch/multiproject/MultiProjectResolver.java index 51502da7bbce..eeedbfc30a8a 100644 --- a/modules/multi-project/src/main/java/org/elasticsearch/multiproject/MultiProjectResolver.java +++ b/modules/multi-project/src/main/java/org/elasticsearch/multiproject/MultiProjectResolver.java @@ -37,14 +37,14 @@ public class MultiProjectResolver implements ProjectResolver { } @Override - public ProjectMetadata getProjectMetadata(Metadata metadata) { - var headerValue = getProjectIdFromThreadContext(); + public ProjectId getProjectId() { + final String headerValue = getProjectIdFromThreadContext(); // TODO: we temporarily fall back to the default project id when there is no project id present in the thread context. // This fallback should be converted into an exception once we merge to public/serverless. if (headerValue == null) { - return metadata.getProject(Metadata.DEFAULT_PROJECT_ID); + return Metadata.DEFAULT_PROJECT_ID; } - return findProject(metadata, headerValue); + return new ProjectId(headerValue); } @Override diff --git a/modules/multi-project/src/test/java/org/elasticsearch/multiproject/MultiProjectResolverTests.java b/modules/multi-project/src/test/java/org/elasticsearch/multiproject/MultiProjectResolverTests.java index b201d8216f0d..3ff6533c1712 100644 --- a/modules/multi-project/src/test/java/org/elasticsearch/multiproject/MultiProjectResolverTests.java +++ b/modules/multi-project/src/test/java/org/elasticsearch/multiproject/MultiProjectResolverTests.java @@ -78,7 +78,7 @@ public class MultiProjectResolverTests extends ESTestCase { var projects = createProjects(); var metadata = Metadata.builder().projectMetadata(projects).build(); threadPool.getThreadContext().putHeader(Task.X_ELASTIC_PROJECT_ID_HTTP_HEADER, randomUUID()); - assertThrows(IllegalArgumentException.class, () -> resolver.getProjectMetadata(metadata)); + assertThrows(AssertionError.class, () -> resolver.getProjectMetadata(metadata)); } public void testGetAllProjectIds() { @@ -130,7 +130,7 @@ public class MultiProjectResolverTests extends ESTestCase { resolver.executeOnProject(projectId1, () -> { assertThat(resolver.getProjectMetadata(state).id(), equalTo(projectId1)); - assertThat(resolver.getProjectId(state), equalTo(projectId1)); + assertThat(resolver.getProjectId(), equalTo(projectId1)); assertThat(threadContext.getHeader(Task.X_OPAQUE_ID_HTTP_HEADER), equalTo(opaqueId)); assertThat(threadContext.getHeader(randomHeaderName), equalTo(randomHeaderValue)); @@ -152,7 +152,7 @@ public class MultiProjectResolverTests extends ESTestCase { // Can set a new project id, after the previous one has been cleared resolver.executeOnProject(projectId2, () -> { assertThat(resolver.getProjectMetadata(state).id(), equalTo(projectId2)); - assertThat(resolver.getProjectId(state), equalTo(projectId2)); + assertThat(resolver.getProjectId(), equalTo(projectId2)); assertThat(threadContext.getHeader(Task.X_OPAQUE_ID_HTTP_HEADER), equalTo(opaqueId)); assertThat(threadContext.getHeader(randomHeaderName), equalTo(randomHeaderValue)); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthAction.java index 030e02c79d84..0662b25a7e3e 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthAction.java @@ -103,7 +103,7 @@ public class TransportClusterHealthAction extends TransportMasterNodeReadAction< final CancellableTask cancellableTask = (CancellableTask) task; final int waitCount = getWaitCount(request); - final ProjectId projectId = projectResolver.getProjectId(state); + final ProjectId projectId = projectResolver.getProjectId(); if (request.waitForEvents() != null) { waitForEventsAndExecuteHealth( diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/create/TransportCreateIndexAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/create/TransportCreateIndexAction.java index 973dd499d210..ef653b0d009a 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/create/TransportCreateIndexAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/create/TransportCreateIndexAction.java @@ -133,7 +133,8 @@ public class TransportCreateIndexAction extends TransportMasterNodeAction listener ) { - final var projectId = projectResolver.getProjectId(state); + final var projectId = projectResolver.getProjectId(); indexTemplateService.removeIndexTemplateV2(projectId, request.names(), request.masterNodeTimeout(), listener); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/TransportPutComposableIndexTemplateAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/TransportPutComposableIndexTemplateAction.java index 1c6eff57ae06..da045f0efa05 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/TransportPutComposableIndexTemplateAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/TransportPutComposableIndexTemplateAction.java @@ -97,7 +97,7 @@ public class TransportPutComposableIndexTemplateAction extends AcknowledgedTrans ) { verifyIfUsingReservedComponentTemplates(request, state); ComposableIndexTemplate indexTemplate = request.indexTemplate(); - ProjectId projectId = projectResolver.getProjectId(state); + ProjectId projectId = projectResolver.getProjectId(); indexTemplateService.putIndexTemplateV2( request.cause(), request.create(), diff --git a/server/src/main/java/org/elasticsearch/action/bulk/TransportAbstractBulkAction.java b/server/src/main/java/org/elasticsearch/action/bulk/TransportAbstractBulkAction.java index 2b92905df5d9..6e317376503e 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/TransportAbstractBulkAction.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/TransportAbstractBulkAction.java @@ -190,7 +190,7 @@ public abstract class TransportAbstractBulkAction extends HandledTransportAction throws IOException { boolean hasIndexRequestsWithPipelines = false; ClusterState state = clusterService.state(); - ProjectId projectId = projectResolver.getProjectId(state); + ProjectId projectId = projectResolver.getProjectId(); final Metadata metadata; Map componentTemplateSubstitutions = bulkRequest.getComponentTemplateSubstitutions(); Map indexTemplateSubstitutions = bulkRequest.getIndexTemplateSubstitutions(); diff --git a/server/src/main/java/org/elasticsearch/action/support/master/info/TransportClusterInfoAction.java b/server/src/main/java/org/elasticsearch/action/support/master/info/TransportClusterInfoAction.java index 9fcb5dbd7110..39f87a4c2e34 100644 --- a/server/src/main/java/org/elasticsearch/action/support/master/info/TransportClusterInfoAction.java +++ b/server/src/main/java/org/elasticsearch/action/support/master/info/TransportClusterInfoAction.java @@ -56,7 +56,7 @@ public abstract class TransportClusterInfoAction listener ) { - ProjectId projectId = projectResolver.getProjectId(state); + ProjectId projectId = projectResolver.getProjectId(); String[] concreteIndices = indexNameExpressionResolver.concreteIndexNames(state.metadata().getProject(projectId), request); doMasterOperation(task, request, concreteIndices, state, listener); } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 8bac1b429eba..278c5bdcbf00 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -166,7 +166,7 @@ public class IndexNameExpressionResolver { * indices options in the context don't allow such a case; if a remote index is requested. */ public String[] concreteIndexNames(ClusterState state, IndicesOptions options, String... indexExpressions) { - return concreteIndexNames(state.metadata().getProject(projectResolver.getProjectId(state)), options, indexExpressions); + return concreteIndexNames(state.metadata().getProject(projectResolver.getProjectId()), options, indexExpressions); } /** @@ -195,7 +195,7 @@ public class IndexNameExpressionResolver { public String[] concreteIndexNames(ClusterState state, IndicesOptions options, boolean includeDataStreams, String... indexExpressions) { return concreteIndexNames( - state.metadata().getProject(projectResolver.getProjectId(state)), + state.metadata().getProject(projectResolver.getProjectId()), options, includeDataStreams, indexExpressions @@ -222,7 +222,7 @@ public class IndexNameExpressionResolver { } public String[] concreteIndexNames(ClusterState state, IndicesOptions options, IndicesRequest request) { - return concreteIndexNames(state.metadata().getProject(projectResolver.getProjectId(state)), options, request); + return concreteIndexNames(state.metadata().getProject(projectResolver.getProjectId()), options, request); } public String[] concreteIndexNames(ProjectMetadata project, IndicesOptions options, IndicesRequest request) { diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java index 9759bad53e5c..f4c3c64eb6be 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java @@ -457,6 +457,7 @@ public class Metadata implements Diffable, ChunkedToXContent { return projectMetadata.containsKey(projectId); } + @FixForMultiProject(description = "We should throw for null metadata for production") public ProjectMetadata getProject(ProjectId projectId) { ProjectMetadata metadata = projectMetadata.get(projectId); assert metadata != null : "Project " + projectId.id() + " not found in " + projectMetadata.keySet(); diff --git a/server/src/main/java/org/elasticsearch/cluster/project/DefaultProjectResolver.java b/server/src/main/java/org/elasticsearch/cluster/project/DefaultProjectResolver.java index 95bb025a6ab1..513caeb0d15a 100644 --- a/server/src/main/java/org/elasticsearch/cluster/project/DefaultProjectResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/project/DefaultProjectResolver.java @@ -9,27 +9,49 @@ package org.elasticsearch.cluster.project; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.metadata.ProjectId; import org.elasticsearch.cluster.metadata.ProjectMetadata; import org.elasticsearch.core.CheckedRunnable; import org.elasticsearch.core.FixForMultiProject; +import java.util.Collection; +import java.util.Map; + /** * This is the {@link ProjectResolver} implementation that stateful uses. * It mainly ensures that there's a single and implicit project existing at all times. */ public class DefaultProjectResolver implements ProjectResolver { + public static final DefaultProjectResolver INSTANCE = new DefaultProjectResolver(); @Override @FixForMultiProject public ProjectMetadata getProjectMetadata(Metadata metadata) { - // TODO-multi-project assert no specific project id is requested, and/or that a sole project exists in the cluster state - return metadata.getProject(); + assert assertSingleProject(metadata); + return ProjectResolver.super.getProjectMetadata(metadata); } @Override + public ProjectId getProjectId() { + // TODO: We can assert the cluster state really has such one project if we let DefaultProjectResolver reference a clusterService + return Metadata.DEFAULT_PROJECT_ID; + } + + @Override + public Collection getProjectIds(ClusterState clusterState) { + assert assertSingleProject(clusterState.metadata()); + return ProjectResolver.super.getProjectIds(clusterState); + } + + private boolean assertSingleProject(Metadata metadata) { + final Map projects = metadata.projects(); + assert projects.size() == 1 && projects.containsKey(getProjectId()) : "expect only default projects, but got " + projects.keySet(); + return true; + } + public void executeOnProject(ProjectId projectId, CheckedRunnable body) throws E { if (projectId.equals(Metadata.DEFAULT_PROJECT_ID)) { body.run(); diff --git a/server/src/main/java/org/elasticsearch/cluster/project/ProjectIdResolver.java b/server/src/main/java/org/elasticsearch/cluster/project/ProjectIdResolver.java new file mode 100644 index 000000000000..ca9f8af4c3cd --- /dev/null +++ b/server/src/main/java/org/elasticsearch/cluster/project/ProjectIdResolver.java @@ -0,0 +1,30 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.cluster.project; + +import org.elasticsearch.cluster.metadata.ProjectId; + +/** + * This is a lightweight version of the {@link ProjectResolver}. It resolves for the {@link ProjectId} for the current + * request in the execution context. It intentionally does not take a {@link org.elasticsearch.cluster.ClusterState} + * so that it can be used in places where we currently do not want to expose {@link org.elasticsearch.cluster.ClusterState} + * such as REST handlers. + * NOTE this interface is also experimental since we have not fully settled on whether exposing it for REST handlers is + * a pattern to be followed. We should discuss it case-by-case by using it in more places until we settle on the issue. + */ +public interface ProjectIdResolver { + + /** + * Retrieve the project for the current request. + * + * @return The identifier of the current project. + */ + ProjectId getProjectId(); +} diff --git a/server/src/main/java/org/elasticsearch/cluster/project/ProjectResolver.java b/server/src/main/java/org/elasticsearch/cluster/project/ProjectResolver.java index cf8b2c207f4a..03c46e82e15a 100644 --- a/server/src/main/java/org/elasticsearch/cluster/project/ProjectResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/project/ProjectResolver.java @@ -17,42 +17,36 @@ import org.elasticsearch.cluster.metadata.ProjectMetadata; import org.elasticsearch.core.CheckedRunnable; import java.util.Collection; +import java.util.Objects; import java.util.Set; /** * This exposes methods for accessing project-scoped data from the global one. * The project in question is implied from the thread context. */ -public interface ProjectResolver { - ProjectMetadata getProjectMetadata(Metadata metadata); +public interface ProjectResolver extends ProjectIdResolver { + default ProjectMetadata getProjectMetadata(Metadata metadata) { + return metadata.getProject(getProjectId()); + } default ProjectMetadata getProjectMetadata(ClusterState clusterState) { return getProjectMetadata(clusterState.metadata()); } + // TODO: What happens if the context does not have a project? throw or return null? default ProjectState getProjectState(ClusterState clusterState) { - final ProjectId id = getProjectId(clusterState); + final ProjectId id = getProjectId(); final ProjectState projectState = clusterState.projectState(id); assert projectState != null : "Received null project state for [" + id + "] from " + clusterState; return projectState; } - // TODO multi-project: change this so it doesn't take in any parameters - /** - * @return The identifier of the current project. This will be the same value as - * {@link #getProjectMetadata(Metadata)}{@code .}{@link ProjectMetadata#id() id()}, but the resolver may implement it more efficiently - * and/or perform additional checks. - */ - default ProjectId getProjectId(ClusterState clusterState) { - return getProjectMetadata(clusterState).id(); - } - /** * Returns the identifiers of all projects on which this request should operate. * In practice, this will either be: *
    *
  • If the request is tied to a single project, then a collection with a single item that is the same as - * {@link #getProjectId(ClusterState)}
    • + * {@link #getProjectId()} if the project exists in the cluster state *
  • If the request is not tied to a single project and it is allowed to access all projects, * then a collection of all the project ids in the cluster
    • *
  • Otherwise an exception is thrown
    • @@ -61,14 +55,18 @@ public interface ProjectResolver { * @throws SecurityException if this request is required to provide a project id, but none was provided */ default Collection getProjectIds(ClusterState clusterState) { - return Set.of(this.getProjectId(clusterState)); + final ProjectId projectId = Objects.requireNonNull(getProjectId()); + if (clusterState.metadata().hasProject(projectId) == false) { + throw new IllegalArgumentException("Project [" + projectId + "] does not exist"); + } + return Set.of(getProjectId()); } /** * Execute a block in the context of a specific project. * * This method:
      - *
    1. Configures the execution (thread) context so that any calls to resolve a project (e.g. {@link #getProjectId(ClusterState)} + *
    2. Configures the execution (thread) context so that any calls to resolve a project (e.g. {@link #getProjectId()} * or {@link #getProjectMetadata(Metadata)}) will return the project specified by {@code projectId}.
      3. *
    3. Executes the {@link CheckedRunnable#run()} method on the supplied {@code body}
      4. *
    4. Restores the context to its original state
      5. diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/diskusage/TransportAnalyzeIndexDiskUsageActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/diskusage/TransportAnalyzeIndexDiskUsageActionTests.java index ddb2259c8147..2dad1307ccb9 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/diskusage/TransportAnalyzeIndexDiskUsageActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/diskusage/TransportAnalyzeIndexDiskUsageActionTests.java @@ -298,7 +298,7 @@ public class TransportAnalyzeIndexDiskUsageActionTests extends ESTestCase { new IndexNameExpressionResolver( new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, - TestProjectResolvers.singleProjectOnly() + TestProjectResolvers.DEFAULT_PROJECT_ONLY ) { @Override public String[] concreteIndexNames(ProjectMetadata project, IndicesRequest request) { diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexActionTests.java index 61109ac26fd3..8b2276ee9ae2 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexActionTests.java @@ -143,7 +143,7 @@ public class GetIndexActionTests extends ESSingleNodeTestCase { static class Resolver extends IndexNameExpressionResolver { Resolver() { - super(new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, TestProjectResolvers.singleProjectOnly()); + super(new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, TestProjectResolvers.DEFAULT_PROJECT_ONLY); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java index 407748cb08f3..eeeb55eb64bd 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/get/GetSettingsActionTests.java @@ -157,7 +157,7 @@ public class GetSettingsActionTests extends ESTestCase { static class Resolver extends IndexNameExpressionResolver { Resolver() { - super(new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, TestProjectResolvers.singleProjectOnly()); + super(new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, TestProjectResolvers.DEFAULT_PROJECT_ONLY); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIngestTests.java b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIngestTests.java index 634ef1237950..ef53e9e0808e 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIngestTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionIngestTests.java @@ -159,7 +159,7 @@ public class TransportBulkActionIngestTests extends ESTestCase { TestIndexNameExpressionResolver.newInstance(), new IndexingPressure(SETTINGS), EmptySystemIndices.INSTANCE, - TestProjectResolvers.singleProjectOnly(), + TestProjectResolvers.DEFAULT_PROJECT_ONLY, FailureStoreMetrics.NOOP ); } diff --git a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java index 3d7628d7e27c..733da7a96516 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTests.java @@ -37,7 +37,7 @@ import org.elasticsearch.cluster.metadata.ProjectId; import org.elasticsearch.cluster.metadata.ProjectMetadata; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodeUtils; -import org.elasticsearch.cluster.project.TestProjectResolvers; +import org.elasticsearch.cluster.project.ProjectResolver; import org.elasticsearch.cluster.routing.GlobalRoutingTableTestHelper; import org.elasticsearch.cluster.routing.RoutingTable; import org.elasticsearch.cluster.service.ClusterService; @@ -45,6 +45,7 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException; +import org.elasticsearch.core.CheckedRunnable; import org.elasticsearch.features.FeatureService; import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexNotFoundException; @@ -73,6 +74,7 @@ import java.util.SortedMap; import java.util.TreeMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import static org.elasticsearch.action.bulk.TransportBulkAction.prohibitCustomRoutingOnDataStream; import static org.elasticsearch.cluster.metadata.MetadataCreateDataStreamServiceTests.createDataStream; @@ -95,6 +97,7 @@ public class TransportBulkActionTests extends ESTestCase { private TestTransportBulkAction bulkAction; private FeatureService mockFeatureService; + private AtomicReference activeProjectId = new AtomicReference<>(); class TestTransportBulkAction extends TransportBulkAction { @@ -116,7 +119,17 @@ public class TransportBulkActionTests extends ESTestCase { new Resolver(), new IndexingPressure(Settings.EMPTY), EmptySystemIndices.INSTANCE, - TestProjectResolvers.singleProjectOnly(), + new ProjectResolver() { + @Override + public void executeOnProject(ProjectId projectId, CheckedRunnable body) throws E { + throw new UnsupportedOperationException(""); + } + + @Override + public ProjectId getProjectId() { + return activeProjectId.get(); + } + }, FailureStoreMetrics.NOOP ); } @@ -173,6 +186,7 @@ public class TransportBulkActionTests extends ESTestCase { transportService.acceptIncomingRequests(); mockFeatureService = mock(FeatureService.class); when(mockFeatureService.clusterHasFeature(any(), any())).thenReturn(true); + activeProjectId.set(Metadata.DEFAULT_PROJECT_ID); bulkAction = new TestTransportBulkAction(); } @@ -536,10 +550,11 @@ public class TransportBulkActionTests extends ESTestCase { // Construct a cluster state that contains the required data streams. // using a single, non-default project final ClusterState oldState = clusterService.state(); + final ProjectId projectId = new ProjectId(randomUUID()); final Metadata metadata = Metadata.builder(oldState.metadata()) .removeProject(Metadata.DEFAULT_PROJECT_ID) .put( - ProjectMetadata.builder(new ProjectId(randomUUID())) + ProjectMetadata.builder(projectId) .put(indexMetadata(".ds-data-stream-01")) .put(indexMetadata(".ds-failure-store-01")) .put(indexMetadata(".fs-failure-store-01")) @@ -574,6 +589,7 @@ public class TransportBulkActionTests extends ESTestCase { // And wait for it to be applied. latch.await(10L, TimeUnit.SECONDS); + activeProjectId.set(projectId); // Set the exceptions that the transport action should encounter. bulkAction.failIndexCreationException = new IndexNotFoundException("index"); bulkAction.failDataStreamRolloverException = new RuntimeException("data-stream-rollover-exception"); diff --git a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTookTests.java b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTookTests.java index a0d06989a04f..7f9afad854c0 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTookTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/TransportBulkActionTookTests.java @@ -221,7 +221,7 @@ public class TransportBulkActionTookTests extends ESTestCase { static class Resolver extends IndexNameExpressionResolver { Resolver() { - super(new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, TestProjectResolvers.singleProjectOnly()); + super(new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, TestProjectResolvers.DEFAULT_PROJECT_ONLY); } @Override @@ -252,7 +252,7 @@ public class TransportBulkActionTookTests extends ESTestCase { indexNameExpressionResolver, new IndexingPressure(Settings.EMPTY), EmptySystemIndices.INSTANCE, - TestProjectResolvers.singleProjectOnly(), + TestProjectResolvers.DEFAULT_PROJECT_ONLY, relativeTimeProvider, FailureStoreMetrics.NOOP ); diff --git a/server/src/test/java/org/elasticsearch/action/bulk/TransportSimulateBulkActionTests.java b/server/src/test/java/org/elasticsearch/action/bulk/TransportSimulateBulkActionTests.java index 34b2fc83c6bb..c422efed0f25 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/TransportSimulateBulkActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/TransportSimulateBulkActionTests.java @@ -92,7 +92,7 @@ public class TransportSimulateBulkActionTests extends ESTestCase { new ActionFilters(Set.of()), new IndexingPressure(Settings.EMPTY), EmptySystemIndices.INSTANCE, - TestProjectResolvers.singleProjectOnly(), + TestProjectResolvers.DEFAULT_PROJECT_ONLY, indicesService, NamedXContentRegistry.EMPTY, new IndexSettingProviders(Set.of()) diff --git a/server/src/test/java/org/elasticsearch/action/resync/TransportResyncReplicationActionTests.java b/server/src/test/java/org/elasticsearch/action/resync/TransportResyncReplicationActionTests.java index be5cbba0a4ec..b264eaa75bda 100644 --- a/server/src/test/java/org/elasticsearch/action/resync/TransportResyncReplicationActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/resync/TransportResyncReplicationActionTests.java @@ -176,7 +176,7 @@ public class TransportResyncReplicationActionTests extends ESTestCase { new ActionFilters(new HashSet<>()), new IndexingPressure(Settings.EMPTY), EmptySystemIndices.INSTANCE, - TestProjectResolvers.singleProjectOnly() + TestProjectResolvers.DEFAULT_PROJECT_ONLY ); assertThat(action.globalBlockLevel(), nullValue()); diff --git a/server/src/test/java/org/elasticsearch/action/support/broadcast/node/TransportBroadcastByNodeActionTests.java b/server/src/test/java/org/elasticsearch/action/support/broadcast/node/TransportBroadcastByNodeActionTests.java index eb7822b042c7..8e86044a3367 100644 --- a/server/src/test/java/org/elasticsearch/action/support/broadcast/node/TransportBroadcastByNodeActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/support/broadcast/node/TransportBroadcastByNodeActionTests.java @@ -237,7 +237,7 @@ public class TransportBroadcastByNodeActionTests extends ESTestCase { static class MyResolver extends IndexNameExpressionResolver { MyResolver() { - super(new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, TestProjectResolvers.singleProjectOnly()); + super(new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, TestProjectResolvers.DEFAULT_PROJECT_ONLY); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/support/replication/TransportWriteActionTests.java b/server/src/test/java/org/elasticsearch/action/support/replication/TransportWriteActionTests.java index 7fe1dc6841f4..5cf950ab0dc4 100644 --- a/server/src/test/java/org/elasticsearch/action/support/replication/TransportWriteActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/support/replication/TransportWriteActionTests.java @@ -431,7 +431,7 @@ public class TransportWriteActionTests extends ESTestCase { PrimaryActionExecution.RejectOnOverload, new IndexingPressure(Settings.EMPTY), EmptySystemIndices.INSTANCE, - TestProjectResolvers.singleProjectOnly(), + TestProjectResolvers.DEFAULT_PROJECT_ONLY, ReplicaActionExecution.SubjectToCircuitBreaker ); this.withDocumentFailureOnPrimary = withDocumentFailureOnPrimary; @@ -461,7 +461,7 @@ public class TransportWriteActionTests extends ESTestCase { PrimaryActionExecution.RejectOnOverload, new IndexingPressure(settings), EmptySystemIndices.INSTANCE, - TestProjectResolvers.singleProjectOnly(), + TestProjectResolvers.DEFAULT_PROJECT_ONLY, ReplicaActionExecution.SubjectToCircuitBreaker ); this.withDocumentFailureOnPrimary = false; diff --git a/server/src/test/java/org/elasticsearch/action/support/single/instance/TransportInstanceSingleOperationActionTests.java b/server/src/test/java/org/elasticsearch/action/support/single/instance/TransportInstanceSingleOperationActionTests.java index a3551a62c11c..7ed0435d1fef 100644 --- a/server/src/test/java/org/elasticsearch/action/support/single/instance/TransportInstanceSingleOperationActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/support/single/instance/TransportInstanceSingleOperationActionTests.java @@ -144,7 +144,7 @@ public class TransportInstanceSingleOperationActionTests extends ESTestCase { static class MyResolver extends IndexNameExpressionResolver { MyResolver() { - super(new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, TestProjectResolvers.singleProjectOnly()); + super(new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, TestProjectResolvers.DEFAULT_PROJECT_ONLY); } @Override diff --git a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncActionTests.java b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncActionTests.java index 238db194236b..93d56b45bd95 100644 --- a/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncActionTests.java +++ b/server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncActionTests.java @@ -111,7 +111,7 @@ public class RetentionLeaseSyncActionTests extends ESTestCase { new ActionFilters(Collections.emptySet()), new IndexingPressure(Settings.EMPTY), EmptySystemIndices.INSTANCE, - TestProjectResolvers.singleProjectOnly() + TestProjectResolvers.DEFAULT_PROJECT_ONLY ); final RetentionLeases retentionLeases = mock(RetentionLeases.class); final RetentionLeaseSyncAction.Request request = new RetentionLeaseSyncAction.Request(indexShard.shardId(), retentionLeases); @@ -149,7 +149,7 @@ public class RetentionLeaseSyncActionTests extends ESTestCase { new ActionFilters(Collections.emptySet()), new IndexingPressure(Settings.EMPTY), EmptySystemIndices.INSTANCE, - TestProjectResolvers.singleProjectOnly() + TestProjectResolvers.DEFAULT_PROJECT_ONLY ); final RetentionLeases retentionLeases = mock(RetentionLeases.class); final RetentionLeaseSyncAction.Request request = new RetentionLeaseSyncAction.Request(indexShard.shardId(), retentionLeases); @@ -191,7 +191,7 @@ public class RetentionLeaseSyncActionTests extends ESTestCase { new ActionFilters(Collections.emptySet()), new IndexingPressure(Settings.EMPTY), EmptySystemIndices.INSTANCE, - TestProjectResolvers.singleProjectOnly() + TestProjectResolvers.DEFAULT_PROJECT_ONLY ); assertNull(action.indexBlockLevel()); diff --git a/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java index f7c39d770a02..879e1e4173a1 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java +++ b/server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java @@ -105,7 +105,6 @@ import org.elasticsearch.cluster.node.DiscoveryNodeRole; import org.elasticsearch.cluster.node.DiscoveryNodeUtils; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.project.DefaultProjectResolver; -import org.elasticsearch.cluster.project.ProjectResolver; import org.elasticsearch.cluster.project.TestProjectResolvers; import org.elasticsearch.cluster.routing.BatchedRerouteService; import org.elasticsearch.cluster.routing.RerouteService; @@ -2168,7 +2167,6 @@ public class SnapshotResiliencyTests extends ESTestCase { null, emptySet() ); - ProjectResolver projectResolver = TestProjectResolvers.singleProjectOnly(); IndexNameExpressionResolver indexNameExpressionResolver = TestIndexNameExpressionResolver.newInstance(); bigArrays = new BigArrays(new PageCacheRecycler(settings), null, "test"); repositoriesService = new RepositoriesService( @@ -2360,7 +2358,7 @@ public class SnapshotResiliencyTests extends ESTestCase { actionFilters, new IndexingPressure(settings), EmptySystemIndices.INSTANCE, - projectResolver + TestProjectResolvers.DEFAULT_PROJECT_ONLY ) ), RetentionLeaseSyncer.EMPTY, @@ -2420,7 +2418,7 @@ public class SnapshotResiliencyTests extends ESTestCase { indexNameExpressionResolver, new IndexingPressure(settings), EmptySystemIndices.INSTANCE, - projectResolver, + TestProjectResolvers.DEFAULT_PROJECT_ONLY, FailureStoreMetrics.NOOP ) ); @@ -2436,7 +2434,7 @@ public class SnapshotResiliencyTests extends ESTestCase { actionFilters, indexingMemoryLimits, EmptySystemIndices.INSTANCE, - projectResolver, + TestProjectResolvers.DEFAULT_PROJECT_ONLY, DocumentParsingProvider.EMPTY_INSTANCE ); actions.put(TransportShardBulkAction.TYPE, transportShardBulkAction); @@ -2471,7 +2469,7 @@ public class SnapshotResiliencyTests extends ESTestCase { indexNameExpressionResolver, new RequestValidators<>(Collections.emptyList()), EmptySystemIndices.INSTANCE, - projectResolver + TestProjectResolvers.DEFAULT_PROJECT_ONLY ) ); actions.put( @@ -2499,7 +2497,7 @@ public class SnapshotResiliencyTests extends ESTestCase { searchPhaseController, clusterService, actionFilters, - projectResolver, + TestProjectResolvers.DEFAULT_PROJECT_ONLY, indexNameExpressionResolver, namedWriteableRegistry, EmptySystemIndices.INSTANCE.getExecutorSelector(), diff --git a/test/framework/src/main/java/org/elasticsearch/cluster/project/TestProjectResolvers.java b/test/framework/src/main/java/org/elasticsearch/cluster/project/TestProjectResolvers.java index a67bf555cff3..71fe131fd536 100644 --- a/test/framework/src/main/java/org/elasticsearch/cluster/project/TestProjectResolvers.java +++ b/test/framework/src/main/java/org/elasticsearch/cluster/project/TestProjectResolvers.java @@ -14,24 +14,28 @@ import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.metadata.ProjectId; import org.elasticsearch.cluster.metadata.ProjectMetadata; import org.elasticsearch.common.util.concurrent.ThreadContext; -import org.elasticsearch.common.util.set.Sets; import org.elasticsearch.core.CheckedRunnable; import org.elasticsearch.tasks.Task; import java.util.Collection; import java.util.Objects; -import java.util.Set; /** * An implementation of {@link ProjectResolver} that handles multiple projects for testing purposes. Not usable in production */ public final class TestProjectResolvers { + public static final ProjectResolver DEFAULT_PROJECT_ONLY = singleProject(Metadata.DEFAULT_PROJECT_ID, true); + + /** + * @return a ProjectResolver that must only be used in a cluster context. It throws in single project related methods. + */ public static ProjectResolver allProjects() { return new ProjectResolver() { + @Override - public ProjectMetadata getProjectMetadata(Metadata metadata) { - return singleProjectMetadata(metadata); + public ProjectId getProjectId() { + throw new UnsupportedOperationException("This resolver can only be used to resolve multiple projects"); } @Override @@ -46,21 +50,31 @@ public final class TestProjectResolvers { }; } + /** + * This method returns a ProjectResolver that is unable to provide the project-id unless explicitly specified + * with the executeOnProject method. This is mostly useful in places where we just need a placeholder to satisfy + * the constructor signature. + */ public static ProjectResolver singleProjectOnly() { return new ProjectResolver() { private ProjectId enforceProjectId = null; @Override - public ProjectMetadata getProjectMetadata(Metadata metadata) { - final ProjectMetadata project = TestProjectResolvers.singleProjectMetadata(metadata); - if (enforceProjectId == null || enforceProjectId.equals(project.id())) { - return project; + public ProjectId getProjectId() { + if (enforceProjectId == null) { + throw new UnsupportedOperationException("Cannot get project-id before it is set"); } else { - throw new IllegalArgumentException("Expected project-id [" + enforceProjectId + "] but was [" + project.id() + "]"); + return enforceProjectId; } } + @Override + public Collection getProjectIds(ClusterState clusterState) { + checkSingleProject(clusterState.metadata()); + return ProjectResolver.super.getProjectIds(clusterState); + } + @Override public void executeOnProject(ProjectId projectId, CheckedRunnable body) throws E { if (enforceProjectId != null) { @@ -76,12 +90,37 @@ public final class TestProjectResolvers { }; } + /** + * This method returns a ProjectResolver that gives back the specified project-id when its getProjectId method is called. + * It also assumes it is the only project in the cluster state and throws if that is not the case. + */ public static ProjectResolver singleProject(ProjectId projectId) { + return singleProject(projectId, false); + } + + private static ProjectResolver singleProject(ProjectId projectId, boolean only) { Objects.requireNonNull(projectId); return new ProjectResolver() { + @Override public ProjectMetadata getProjectMetadata(Metadata metadata) { - return metadata.getProject(projectId); + if (only) { + checkSingleProject(metadata); + } + return ProjectResolver.super.getProjectMetadata(metadata); + } + + @Override + public ProjectId getProjectId() { + return projectId; + } + + @Override + public Collection getProjectIds(ClusterState clusterState) { + if (only) { + checkSingleProject(clusterState.metadata()); + } + return ProjectResolver.super.getProjectIds(clusterState); } @Override @@ -95,63 +134,25 @@ public final class TestProjectResolvers { }; } - public static ProjectResolver projects(Set allowedProjectIds) { - if (allowedProjectIds.isEmpty()) { - throw new IllegalArgumentException("Project Ids cannot be empty"); - } - return new ProjectResolver() { - @Override - public ProjectMetadata getProjectMetadata(Metadata metadata) { - final Set matchingProjects = getMatchingProjectIds(metadata); - switch (matchingProjects.size()) { - case 1: - return metadata.getProject(matchingProjects.iterator().next()); - case 0: - throw new IllegalStateException( - "No projects matching [" + allowedProjectIds + "] in [" + metadata.projects().keySet() + "]" - ); - default: - throw new IllegalStateException( - "Multiple projects (" - + matchingProjects - + ") match [" - + allowedProjectIds - + "] in [" - + metadata.projects().keySet() - + "]" - ); - } - } - - @Override - public Collection getProjectIds(ClusterState clusterState) { - return getMatchingProjectIds(clusterState.metadata()); - } - - private Set getMatchingProjectIds(Metadata metadata) { - return Sets.intersection(metadata.projects().keySet(), allowedProjectIds); - } - - @Override - public void executeOnProject(ProjectId projectId, CheckedRunnable body) throws E { - throw new UnsupportedOperationException("Cannot execute on a specific project when using a resolver with multiple ids"); - } - }; - } - public static ProjectResolver usingRequestHeader(ThreadContext threadContext) { return new ProjectResolver() { + @Override public ProjectMetadata getProjectMetadata(Metadata metadata) { - String headerValue = threadContext.getHeader(Task.X_ELASTIC_PROJECT_ID_HTTP_HEADER); - var projectId = headerValue != null ? new ProjectId(headerValue) : Metadata.DEFAULT_PROJECT_ID; + final ProjectId projectId = getProjectId(); var project = metadata.projects().get(projectId); if (project == null) { - throw new IllegalArgumentException("Could not find project with id [" + headerValue + "]"); + throw new IllegalArgumentException("Could not find project with id [" + projectId.id() + "]"); } return project; } + @Override + public ProjectId getProjectId() { + String headerValue = threadContext.getHeader(Task.X_ELASTIC_PROJECT_ID_HTTP_HEADER); + return headerValue != null ? new ProjectId(headerValue) : Metadata.DEFAULT_PROJECT_ID; + } + @Override public void executeOnProject(ProjectId projectId, CheckedRunnable body) throws E { try (var ignore = threadContext.newStoredContext()) { @@ -167,9 +168,4 @@ public final class TestProjectResolvers { throw new IllegalStateException("Cluster has multiple projects: [" + metadata.projects().keySet() + "]"); } } - - private static ProjectMetadata singleProjectMetadata(Metadata metadata) { - checkSingleProject(metadata); - return metadata.projects().values().iterator().next(); - } } diff --git a/test/framework/src/main/java/org/elasticsearch/indices/TestIndexNameExpressionResolver.java b/test/framework/src/main/java/org/elasticsearch/indices/TestIndexNameExpressionResolver.java index a5d17b9ce20a..84ffed0df6b9 100644 --- a/test/framework/src/main/java/org/elasticsearch/indices/TestIndexNameExpressionResolver.java +++ b/test/framework/src/main/java/org/elasticsearch/indices/TestIndexNameExpressionResolver.java @@ -10,6 +10,7 @@ package org.elasticsearch.indices; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.project.ProjectResolver; import org.elasticsearch.cluster.project.TestProjectResolvers; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; @@ -30,10 +31,14 @@ public class TestIndexNameExpressionResolver { return new IndexNameExpressionResolver( new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, - TestProjectResolvers.singleProjectOnly() + TestProjectResolvers.DEFAULT_PROJECT_ONLY ); } + public static IndexNameExpressionResolver newInstance(ProjectResolver projectResolver) { + return new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, projectResolver); + } + /** * @return a new instance of a {@link IndexNameExpressionResolver} that has been created with the provided {@link ThreadContext} and * the default {@link SystemIndices} instance @@ -51,7 +56,7 @@ public class TestIndexNameExpressionResolver { * the provided {@link SystemIndices} instance */ public static IndexNameExpressionResolver newInstance(SystemIndices systemIndices) { - return new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY), systemIndices, TestProjectResolvers.singleProjectOnly()); + return new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY), systemIndices, TestProjectResolvers.DEFAULT_PROJECT_ONLY); } /** diff --git a/test/framework/src/test/java/org/elasticsearch/cluster/project/TestProjectResolversTests.java b/test/framework/src/test/java/org/elasticsearch/cluster/project/TestProjectResolversTests.java index 92e254c72ddd..6c05508ee875 100644 --- a/test/framework/src/test/java/org/elasticsearch/cluster/project/TestProjectResolversTests.java +++ b/test/framework/src/test/java/org/elasticsearch/cluster/project/TestProjectResolversTests.java @@ -16,192 +16,88 @@ import org.elasticsearch.cluster.metadata.ProjectId; import org.elasticsearch.cluster.metadata.ProjectMetadata; import org.elasticsearch.test.ESTestCase; -import java.util.Collection; -import java.util.List; -import java.util.Set; - import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.in; import static org.hamcrest.Matchers.notNullValue; public class TestProjectResolversTests extends ESTestCase { - public void testAllProjects_getProjectMetadata() { - { - ClusterState state = buildClusterState(1); - assertThat(state.metadata().projects().values(), hasSize(1)); + public void testAllProjects() { + final int numberOfProjects = randomIntBetween(1, 10); + ClusterState state = buildClusterState(numberOfProjects); + assertThat(state.metadata().projects().values(), hasSize(numberOfProjects)); - ProjectMetadata project = TestProjectResolvers.allProjects().getProjectMetadata(state); - assertThat(project, notNullValue()); - assertThat(project, in(state.metadata().projects().values())); - } - { - ClusterState state = buildClusterState(randomIntBetween(2, 10)); - assertThat(state.metadata().projects().values().size(), greaterThan(1)); - - expectThrows(IllegalStateException.class, () -> TestProjectResolvers.allProjects().getProjectMetadata(state)); - } + expectThrows(UnsupportedOperationException.class, () -> TestProjectResolvers.allProjects().getProjectMetadata(state)); + expectThrows(UnsupportedOperationException.class, () -> TestProjectResolvers.allProjects().getProjectId()); + assertThat(TestProjectResolvers.allProjects().getProjectIds(state), equalTo(state.metadata().projects().keySet())); } - public void testAllProjects_getProjectId() { - { - ClusterState state = buildClusterState(1); - assertThat(state.metadata().projects().values(), hasSize(1)); + public void testSingleProject() { + final ProjectId projectId = new ProjectId(randomUUID()); + final ProjectResolver projectResolver = TestProjectResolvers.singleProject(projectId); + assertThat(projectResolver.getProjectId(), equalTo(projectId)); - ProjectId project = TestProjectResolvers.allProjects().getProjectId(state); - assertThat(project, notNullValue()); - assertThat(project, in(state.metadata().projects().keySet())); - } - { - ClusterState state = buildClusterState(randomIntBetween(2, 10)); - assertThat(state.metadata().projects().values().size(), greaterThan(1)); - - expectThrows(IllegalStateException.class, () -> TestProjectResolvers.allProjects().getProjectId(state)); - } + ClusterState state = buildClusterState(projectId, randomIntBetween(0, 10)); + assertThat(projectResolver.getProjectMetadata(state), notNullValue()); } - public void testAllProjects_getProjectIds() { - for (int numberOfProjects : List.of(1, 3, 8)) { - ClusterState state = buildClusterState(numberOfProjects); - assertThat(state.metadata().projects().values(), hasSize(numberOfProjects)); + public void testSingleProjectOnly_getProjectIdAndMetadata() { + final ProjectId projectId = new ProjectId(randomUUID()); + final ClusterState state = buildClusterState(projectId); - Collection projects = TestProjectResolvers.allProjects().getProjectIds(state); - assertThat(projects, notNullValue()); - assertThat(projects, hasSize(numberOfProjects)); - assertThat(projects, equalTo(state.metadata().projects().keySet())); - } - } + final ProjectResolver projectResolver = TestProjectResolvers.singleProjectOnly(); + expectThrows(UnsupportedOperationException.class, projectResolver::getProjectId); + expectThrows(UnsupportedOperationException.class, () -> projectResolver.getProjectMetadata(state)); - public void testSingleProjectOnly_getProjectMetadata() { - { - ClusterState state = buildClusterState(1); - assertThat(state.metadata().projects().values(), hasSize(1)); - - ProjectMetadata project = TestProjectResolvers.singleProjectOnly().getProjectMetadata(state); - assertThat(project, notNullValue()); - assertThat(project, in(state.metadata().projects().values())); - } - { - ClusterState state = buildClusterState(randomIntBetween(2, 10)); - assertThat(state.metadata().projects().values().size(), greaterThan(1)); - - expectThrows(IllegalStateException.class, () -> TestProjectResolvers.singleProjectOnly().getProjectMetadata(state)); - } - } - - public void testSingleProjectOnly_getProjectId() { - { - ClusterState state = buildClusterState(1); - assertThat(state.metadata().projects().values(), hasSize(1)); - - ProjectId project = TestProjectResolvers.singleProjectOnly().getProjectId(state); - assertThat(project, notNullValue()); - assertThat(project, in(state.metadata().projects().keySet())); - } - { - ClusterState state = buildClusterState(randomIntBetween(2, 10)); - assertThat(state.metadata().projects().values().size(), greaterThan(1)); - - expectThrows(IllegalStateException.class, () -> TestProjectResolvers.singleProjectOnly().getProjectId(state)); - } + projectResolver.executeOnProject(projectId, () -> { + assertThat(projectResolver.getProjectId(), equalTo(projectId)); + assertThat(projectResolver.getProjectMetadata(state), equalTo(state.metadata().getProject(projectId))); + }); } public void testSingleProjectOnly_getProjectIds() { { - ClusterState state = buildClusterState(1); + final ProjectResolver projectResolver = TestProjectResolvers.singleProjectOnly(); + final ProjectId projectId = new ProjectId(randomUUID()); + ClusterState state = buildClusterState(projectId); assertThat(state.metadata().projects().values(), hasSize(1)); - Collection projects = TestProjectResolvers.singleProjectOnly().getProjectIds(state); - assertThat(projects, notNullValue()); - assertThat(projects, hasSize(1)); - assertThat(projects, equalTo(state.metadata().projects().keySet())); + expectThrows(UnsupportedOperationException.class, () -> projectResolver.getProjectIds(state)); + projectResolver.executeOnProject(projectId, () -> assertThat(projectResolver.getProjectIds(state), contains(projectId))); + projectResolver.executeOnProject(new ProjectId(randomUUID()), () -> { + expectThrows(IllegalArgumentException.class, () -> projectResolver.getProjectIds(state)); + }); } { - ClusterState state = buildClusterState(randomIntBetween(2, 10)); + final ProjectResolver projectResolver = TestProjectResolvers.singleProjectOnly(); + final ProjectId projectId = new ProjectId(randomUUID()); + ClusterState state = buildClusterState(projectId, randomIntBetween(1, 10)); assertThat(state.metadata().projects().values().size(), greaterThan(1)); - expectThrows(IllegalStateException.class, () -> TestProjectResolvers.singleProjectOnly().getProjectIds(state)); + projectResolver.executeOnProject( + projectId, + () -> expectThrows(IllegalStateException.class, () -> projectResolver.getProjectIds(state)) + ); } } - public void testSpecifiedProjects_getProjectMetadata() { - for (int numberOfProjects : List.of(1, 3, 8)) { - ClusterState state = buildClusterState(numberOfProjects); - assertThat(state.metadata().projects().values(), hasSize(numberOfProjects)); - final Set allIds = state.metadata().projects().keySet(); - - final ProjectId id = randomFrom(allIds); - ProjectMetadata project = TestProjectResolvers.projects(Set.of(id)).getProjectMetadata(state); - assertThat(project, notNullValue()); - assertThat(project, in(state.metadata().projects().values())); - assertThat(project.id(), equalTo(id)); - - final ProjectId other = randomValueOtherThanMany(allIds::contains, () -> new ProjectId(randomUUID())); - expectThrows(IllegalStateException.class, () -> TestProjectResolvers.projects(Set.of(other)).getProjectMetadata(state)); - - project = TestProjectResolvers.projects(Set.of(id, other)).getProjectMetadata(state); - assertThat(project, notNullValue()); - assertThat(project, in(state.metadata().projects().values())); - assertThat(project.id(), equalTo(id)); - - if (numberOfProjects > 1) { - expectThrows(IllegalStateException.class, () -> TestProjectResolvers.projects(allIds).getProjectMetadata(state)); - } + private ClusterState buildClusterState(ProjectId... projectIds) { + Metadata.Builder metadata = Metadata.builder(); + for (var projectId : projectIds) { + metadata.put(ProjectMetadata.builder(projectId).build()); } + return ClusterState.builder(new ClusterName(randomAlphaOfLengthBetween(4, 8))).metadata(metadata).build(); } - public void testSpecifiedProjects_getProjectId() { - for (int numberOfProjects : List.of(1, 3, 8)) { - ClusterState state = buildClusterState(numberOfProjects); - assertThat(state.metadata().projects().values(), hasSize(numberOfProjects)); - final Set allIds = state.metadata().projects().keySet(); - - final ProjectId id = randomFrom(allIds); - ProjectId project = TestProjectResolvers.projects(Set.of(id)).getProjectId(state); - assertThat(project, notNullValue()); - assertThat(project, equalTo(id)); - - final ProjectId other = randomValueOtherThanMany(allIds::contains, () -> new ProjectId(randomUUID())); - expectThrows(IllegalStateException.class, () -> TestProjectResolvers.projects(Set.of(other)).getProjectId(state)); - - project = TestProjectResolvers.projects(Set.of(id, other)).getProjectId(state); - assertThat(project, notNullValue()); - assertThat(project, equalTo(id)); - - if (numberOfProjects > 1) { - expectThrows(IllegalStateException.class, () -> TestProjectResolvers.projects(allIds).getProjectId(state)); - } - } - } - - public void testSpecifiedProjects_getProjectIds() { - for (int numberOfProjects : List.of(1, 3, 8)) { - ClusterState state = buildClusterState(numberOfProjects); - assertThat(state.metadata().projects().values(), hasSize(numberOfProjects)); - final Set allIds = state.metadata().projects().keySet(); - - Collection projects = TestProjectResolvers.projects(allIds).getProjectIds(state); - assertThat(projects, notNullValue()); - assertThat(projects, equalTo(allIds)); - - final ProjectId id = randomFrom(allIds); - projects = TestProjectResolvers.projects(Set.of(id)).getProjectIds(state); - assertThat(projects, notNullValue()); - assertThat(projects, hasSize(1)); - assertThat(projects, contains(id)); - - final ProjectId other = randomValueOtherThanMany(allIds::contains, () -> new ProjectId(randomUUID())); - assertThat(TestProjectResolvers.projects(Set.of(other)).getProjectIds(state), empty()); - - projects = TestProjectResolvers.projects(Set.of(id, other)).getProjectIds(state); - assertThat(projects, notNullValue()); - assertThat(projects, hasSize(1)); - assertThat(projects, contains(id)); + private ClusterState buildClusterState(ProjectId projectId, int numberOfExtraProjects) { + Metadata.Builder metadata = Metadata.builder(); + metadata.put(ProjectMetadata.builder(projectId).build()); + for (int i = 0; i < numberOfExtraProjects; i++) { + metadata.put(ProjectMetadata.builder(new ProjectId("p" + i + "_" + randomAlphaOfLength(8))).build()); } + return ClusterState.builder(new ClusterName(randomAlphaOfLengthBetween(4, 8))).metadata(metadata).build(); } private ClusterState buildClusterState(int numberOfProjects) { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java index 0c351883b5d6..c4b57feb5a3f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java @@ -89,6 +89,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.AliasMetadata; import org.elasticsearch.cluster.metadata.IndexAbstraction; import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.metadata.ProjectId; import org.elasticsearch.cluster.metadata.ProjectMetadata; @@ -265,6 +266,7 @@ public class AuthorizationServiceTests extends ESTestCase { private boolean setFakeOriginatingAction = true; private SecurityContext securityContext; private ProjectResolver projectResolver; + private IndexNameExpressionResolver indexNameExpressionResolver; @SuppressWarnings("unchecked") @Before @@ -316,11 +318,8 @@ public class AuthorizationServiceTests extends ESTestCase { }).when(rolesStore).getRole(any(Subject.class), anyActionListener()); roleMap.put(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR.getName(), ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR); operatorPrivilegesService = mock(OperatorPrivileges.OperatorPrivilegesService.class); - projectResolver = mock(ProjectResolver.class); - when(projectResolver.getProjectMetadata(any(ClusterState.class))).thenAnswer(inv -> { - ClusterState clusterState = inv.getArgument(0); - return clusterState.metadata().getProject(projectId); - }); + projectResolver = TestProjectResolvers.singleProject(projectId); + indexNameExpressionResolver = TestIndexNameExpressionResolver.newInstance(projectResolver); authorizationService = new AuthorizationService( settings, rolesStore, @@ -333,7 +332,7 @@ public class AuthorizationServiceTests extends ESTestCase { null, Collections.emptySet(), licenseState, - TestIndexNameExpressionResolver.newInstance(), + indexNameExpressionResolver, operatorPrivilegesService, RESTRICTED_INDICES, new AuthorizationDenialMessages.Default(), @@ -1768,11 +1767,11 @@ public class AuthorizationServiceTests extends ESTestCase { null, Collections.emptySet(), new XPackLicenseState(() -> 0), - TestIndexNameExpressionResolver.newInstance(), + indexNameExpressionResolver, operatorPrivilegesService, RESTRICTED_INDICES, new AuthorizationDenialMessages.Default(), - TestProjectResolvers.singleProjectOnly() + projectResolver ); RoleDescriptor role = new RoleDescriptor( @@ -1818,11 +1817,11 @@ public class AuthorizationServiceTests extends ESTestCase { null, Collections.emptySet(), new XPackLicenseState(() -> 0), - TestIndexNameExpressionResolver.newInstance(), + indexNameExpressionResolver, operatorPrivilegesService, RESTRICTED_INDICES, new AuthorizationDenialMessages.Default(), - TestProjectResolvers.singleProjectOnly() + projectResolver ); RoleDescriptor role = new RoleDescriptor( @@ -2427,8 +2426,6 @@ public class AuthorizationServiceTests extends ESTestCase { } public void testSuperusersCanExecuteReadOperationAgainstSecurityIndexWithWildcard() { - // TODO This test depends on IndexAbstractionResolver (via IndicesAndAliasesResolver), which only works with the default project - projectId = Metadata.DEFAULT_PROJECT_ID; final User superuser = new User("custom_admin", ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR.getName()); final Authentication authentication = createAuthentication(superuser); roleMap.put(ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR.getName(), ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR); @@ -3358,11 +3355,11 @@ public class AuthorizationServiceTests extends ESTestCase { engine, Collections.emptySet(), licenseState, - TestIndexNameExpressionResolver.newInstance(), + indexNameExpressionResolver, operatorPrivilegesService, RESTRICTED_INDICES, new AuthorizationDenialMessages.Default(), - TestProjectResolvers.singleProjectOnly() + projectResolver ); Subject subject = new Subject(new User("test", "a role"), mock(RealmRef.class)); @@ -3515,11 +3512,11 @@ public class AuthorizationServiceTests extends ESTestCase { engine, Collections.emptySet(), licenseState, - TestIndexNameExpressionResolver.newInstance(), + indexNameExpressionResolver, operatorPrivilegesService, RESTRICTED_INDICES, new AuthorizationDenialMessages.Default(), - TestProjectResolvers.singleProjectOnly() + projectResolver ); Authentication authentication; try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { diff --git a/x-pack/plugin/transform/src/internalClusterTest/java/org/elasticsearch/xpack/transform/checkpoint/TransformGetCheckpointTests.java b/x-pack/plugin/transform/src/internalClusterTest/java/org/elasticsearch/xpack/transform/checkpoint/TransformGetCheckpointTests.java index 4e02d127e67a..a7f3d1f81218 100644 --- a/x-pack/plugin/transform/src/internalClusterTest/java/org/elasticsearch/xpack/transform/checkpoint/TransformGetCheckpointTests.java +++ b/x-pack/plugin/transform/src/internalClusterTest/java/org/elasticsearch/xpack/transform/checkpoint/TransformGetCheckpointTests.java @@ -272,7 +272,7 @@ public class TransformGetCheckpointTests extends ESSingleNodeTestCase { static class MockResolver extends IndexNameExpressionResolver { MockResolver() { - super(new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, TestProjectResolvers.singleProjectOnly()); + super(new ThreadContext(Settings.EMPTY), EmptySystemIndices.INSTANCE, TestProjectResolvers.DEFAULT_PROJECT_ONLY); } @Override