Skip to content

Commit 7b9610b

Browse files
authored
Rename NOT_CROSS_PROJECT and also refine slightly (#136528)
As said in the title, this PR changes the definition slightly so that its field values form a combination that is not possible when CPS is enabled. This plus the rename should improve the clarity of this field and its usage. Resolves: ES-13171
1 parent d100c83 commit 7b9610b

File tree

7 files changed

+56
-20
lines changed

7 files changed

+56
-20
lines changed

server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolver.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,10 @@ public ResolvedIndexExpressions resolveIndexAbstractions(
7676
final TargetProjects targetProjects,
7777
final boolean includeDataStreams
7878
) {
79-
assert targetProjects != TargetProjects.NOT_CROSS_PROJECT
80-
: "cannot resolve indices cross project if target set is NOT_CROSS_PROJECT";
79+
assert targetProjects != TargetProjects.LOCAL_ONLY_FOR_CPS_DISABLED
80+
: "cannot resolve indices cross project if target set is local only";
8181
if (false == targetProjects.crossProject()) {
82-
final String message = "cannot resolve indices cross project if target set is empty";
82+
final String message = "cannot resolve indices cross project if target set is not cross project";
8383
assert false : message;
8484
throw new IllegalArgumentException(message);
8585
}

server/src/main/java/org/elasticsearch/search/crossproject/TargetProjects.java

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@
1111

1212
import org.elasticsearch.core.Nullable;
1313

14-
import java.util.Collections;
1514
import java.util.List;
1615
import java.util.Set;
1716
import java.util.stream.Collectors;
17+
import java.util.stream.Stream;
1818

1919
/**
2020
* Holds information about the target projects for a cross-project search request. This record is used both by the
@@ -23,8 +23,26 @@
2323
* project routing
2424
* @param linkedProjects all projects that are linked and authorized, can be empty if the request is not cross-project
2525
*/
26-
public record TargetProjects(@Nullable ProjectRoutingInfo originProject, List<ProjectRoutingInfo> linkedProjects) {
27-
public static final TargetProjects NOT_CROSS_PROJECT = new TargetProjects(null, List.of());
26+
public record TargetProjects(
27+
@Nullable ProjectRoutingInfo originProject, // null when CPS is disabled or the local project is excluded by routing
28+
@Nullable List<ProjectRoutingInfo> linkedProjects // null when CPS is disabled
29+
) {
30+
// Constant for representing no target project at all. Note this has a non-null empty linkedProjects field.
31+
public static final TargetProjects EMPTY = new TargetProjects(null, List.of());
32+
33+
// A placeholder constant to satisfy the AuthorizedProjectResolver contract when CPS is disabled. The field values
34+
// are chosen so that it is a combination that is impossible to have when CPS is enabled. Hence, they are not
35+
// meant to be always interpreted literally, i.e. the `null` originProject does NOT mean the local project is excluded.
36+
// Instead, actions are always and only executed against the local project when CPS is disabled. =
37+
// This is different the above EMPTY field and its isEmpty check returns `false`.
38+
public static final TargetProjects LOCAL_ONLY_FOR_CPS_DISABLED = new TargetProjects(null, null);
39+
40+
static {
41+
assert EMPTY.isEmpty();
42+
assert EMPTY.crossProject() == false;
43+
assert LOCAL_ONLY_FOR_CPS_DISABLED.isEmpty() == false;
44+
assert LOCAL_ONLY_FOR_CPS_DISABLED.crossProject() == false;
45+
}
2846

2947
public TargetProjects(ProjectRoutingInfo originProject) {
3048
this(originProject, List.of());
@@ -37,14 +55,18 @@ public String originProjectAlias() {
3755

3856
public Set<String> allProjectAliases() {
3957
// TODO consider caching this
40-
final Set<String> allProjectAliases = linkedProjects.stream().map(ProjectRoutingInfo::projectAlias).collect(Collectors.toSet());
41-
if (originProject != null) {
42-
allProjectAliases.add(originProject.projectAlias());
43-
}
44-
return Collections.unmodifiableSet(allProjectAliases);
58+
return Stream.concat(
59+
originProject != null ? Stream.of(originProject) : Stream.empty(),
60+
linkedProjects != null ? linkedProjects.stream() : Stream.empty()
61+
).map(ProjectRoutingInfo::projectAlias).collect(Collectors.toUnmodifiableSet());
4562
}
4663

64+
// TODO: Either change the definition or the method name since it allows targeting only the origin project without any remotes
4765
public boolean crossProject() {
48-
return originProject != null || linkedProjects.isEmpty() == false;
66+
return originProject != null || (linkedProjects != null && linkedProjects.isEmpty() == false);
67+
}
68+
69+
public boolean isEmpty() {
70+
return originProject == null && (linkedProjects != null && linkedProjects.isEmpty());
4971
}
5072
}

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizedProjectsResolver.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,15 @@
1212

1313
/**
1414
* A resolver of authorized projects for the current user. This includes the origin project and all linked projects the user has access to.
15-
* If we are not in a cross-project search context, the supplier returns {@link TargetProjects#NOT_CROSS_PROJECT}.
15+
* If we are not in a cross-project search context, the supplier returns {@link TargetProjects#LOCAL_ONLY_FOR_CPS_DISABLED}.
1616
*/
1717
public interface AuthorizedProjectsResolver {
1818
void resolveAuthorizedProjects(ActionListener<TargetProjects> listener);
1919

2020
class Default implements AuthorizedProjectsResolver {
2121
@Override
2222
public void resolveAuthorizedProjects(ActionListener<TargetProjects> listener) {
23-
listener.onResponse(TargetProjects.NOT_CROSS_PROJECT);
23+
listener.onResponse(TargetProjects.LOCAL_ONLY_FOR_CPS_DISABLED);
2424
}
2525
}
2626
}

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ private void authorizeAction(
506506
targetProjectListener = new SubscribableListener<>();
507507
authorizedProjectsResolver.resolveAuthorizedProjects(targetProjectListener);
508508
} else {
509-
targetProjectListener = SubscribableListener.newSucceeded(TargetProjects.NOT_CROSS_PROJECT);
509+
targetProjectListener = SubscribableListener.newSucceeded(TargetProjects.LOCAL_ONLY_FOR_CPS_DISABLED);
510510
}
511511

512512
targetProjectListener.addListener(ActionListener.wrap(targetProjects -> {

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,13 @@ ResolvedIndices resolveIndicesAndAliases(
296296
ProjectMetadata projectMetadata,
297297
AuthorizationEngine.AuthorizedIndices authorizedIndices
298298
) {
299-
return resolveIndicesAndAliases(action, indicesRequest, projectMetadata, authorizedIndices, TargetProjects.NOT_CROSS_PROJECT);
299+
return resolveIndicesAndAliases(
300+
action,
301+
indicesRequest,
302+
projectMetadata,
303+
authorizedIndices,
304+
TargetProjects.LOCAL_ONLY_FOR_CPS_DISABLED
305+
);
300306
}
301307

302308
ResolvedIndices resolveIndicesAndAliases(
@@ -370,11 +376,13 @@ ResolvedIndices resolveIndicesAndAliases(
370376
// we honour allow_no_indices like es core does.
371377
} else {
372378
assert indicesRequest.indices() != null : "indices() cannot be null when resolving non-all-index expressions";
379+
// TODO: consider short-circuit when authorizedProjects.linkedProjects is empty or is filtered to empty
373380
if (crossProjectModeDecider.resolvesCrossProject(replaceable)
374381
// a none expression should not go through cross-project resolution -- fall back to local resolution logic
375382
&& false == IndexNameExpressionResolver.isNoneExpression(replaceable.indices())) {
376-
assert replaceable.allowsRemoteIndices() : "cross-project requests must allow remote indices";
377-
assert authorizedProjects.crossProject() : "cross-project requests must have cross-project target set";
383+
assert replaceable.allowsRemoteIndices() : "cross-project request [" + indicesRequest + "] must allow remote indices";
384+
assert authorizedProjects.crossProject()
385+
: "cross-project request [" + indicesRequest + "] must have cross-project target set";
378386

379387
final ResolvedIndexExpressions resolved = indexAbstractionResolver.resolveIndexAbstractions(
380388
Arrays.asList(replaceable.indices()),

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ public void setup() {
335335
authorizedProjectsResolver = mock(AuthorizedProjectsResolver.class);
336336
doAnswer(invocation -> {
337337
ActionListener<TargetProjects> callback = (ActionListener<TargetProjects>) invocation.getArguments()[0];
338-
callback.onResponse(TargetProjects.NOT_CROSS_PROJECT);
338+
callback.onResponse(TargetProjects.LOCAL_ONLY_FOR_CPS_DISABLED);
339339
return null;
340340
}).when(authorizedProjectsResolver).resolveAuthorizedProjects(anyActionListener());
341341
crossProjectModeDecider = mock(CrossProjectModeDecider.class);

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2911,7 +2911,13 @@ private ResolvedIndices resolveIndices(TransportRequest request, AuthorizedIndic
29112911
}
29122912

29132913
private ResolvedIndices resolveIndices(String action, TransportRequest request, AuthorizedIndices authorizedIndices) {
2914-
return defaultIndicesResolver.resolve(action, request, this.projectMetadata, authorizedIndices, TargetProjects.NOT_CROSS_PROJECT);
2914+
return defaultIndicesResolver.resolve(
2915+
action,
2916+
request,
2917+
this.projectMetadata,
2918+
authorizedIndices,
2919+
TargetProjects.LOCAL_ONLY_FOR_CPS_DISABLED
2920+
);
29152921
}
29162922

29172923
private static void assertNoIndices(IndicesRequest.Replaceable request, ResolvedIndices resolvedIndices) {

0 commit comments

Comments
 (0)