Last active
December 11, 2025 04:58
-
-
Save ywangd/5581d0a47f03be4b763037a248017103 to your computer and use it in GitHub Desktop.
Suggestion for 137840
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| diff --git a/server/src/main/java/org/elasticsearch/search/crossproject/CrossProjectIndexResolutionValidator.java b/server/src/main/java/org/elasticsearch/search/crossproject/CrossProjectIndexResolutionValidator.java | |
| index 09f8242ea15..8a620e72ee6 100644 | |
| --- a/server/src/main/java/org/elasticsearch/search/crossproject/CrossProjectIndexResolutionValidator.java | |
| +++ b/server/src/main/java/org/elasticsearch/search/crossproject/CrossProjectIndexResolutionValidator.java | |
| @@ -25,7 +25,7 @@ import java.util.ArrayList; | |
| import java.util.HashMap; | |
| import java.util.List; | |
| import java.util.Map; | |
| -import java.util.Set; | |
| +import java.util.function.BiConsumer; | |
| import static org.elasticsearch.action.ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE; | |
| import static org.elasticsearch.action.ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_UNAUTHORIZED; | |
| @@ -86,10 +86,14 @@ public class CrossProjectIndexResolutionValidator { | |
| return null; | |
| } | |
| + // For each unauthorized expression, we report 403 for the first project if the expression is unqualified. | |
| + // Otherwise, we report 403 for all projects where the expression is unauthorized. | |
| Map<String, ElasticsearchSecurityException> remoteAuthorizationExceptions = null; | |
| Map<String, List<String>> remoteUnauthorizedIndices = null; | |
| ElasticsearchSecurityException localAuthorizationException = null; | |
| List<String> localUnauthorizedIndices = null; | |
| + | |
| + // We report only the first 404 error when there is no 403 error. | |
| IndexNotFoundException notFoundException = null; | |
| final boolean hasProjectRouting = Strings.isEmpty(projectRouting) == false; | |
| @@ -108,7 +112,8 @@ public class CrossProjectIndexResolutionValidator { | |
| // Check if this is a qualified resource (project:index pattern) | |
| boolean isQualifiedExpression = RemoteClusterAware.isRemoteIndexName(originalExpression); | |
| - Set<String> remoteExpressions = localResolvedIndices.remoteExpressions(); | |
| + // TODO: Sort to ensure the behavior is deterministic. Consider pre-sorting during rewriting to avoid sorting here. | |
| + final var remoteExpressions = localResolvedIndices.remoteExpressions().stream().sorted().toList(); | |
| ResolvedIndexExpression.LocalExpressions localExpressions = localResolvedIndices.localExpressions(); | |
| ResolvedIndexExpression.LocalIndexResolutionResult result = localExpressions.localIndexResolutionResult(); | |
| ElasticsearchException localException = checkResolutionFailure(localExpressions, result, originalExpression, indicesOptions); | |
| @@ -122,7 +127,9 @@ public class CrossProjectIndexResolutionValidator { | |
| } | |
| localUnauthorizedIndices.add(originalExpression); | |
| } else { | |
| - notFoundException = (IndexNotFoundException) localException; | |
| + if (notFoundException == null) { | |
| + notFoundException = (IndexNotFoundException) localException; | |
| + } | |
| } | |
| } | |
| // qualified linked project expression | |
| @@ -158,10 +165,16 @@ public class CrossProjectIndexResolutionValidator { | |
| } | |
| assert localExpressions != ResolvedIndexExpression.LocalExpressions.NONE || false == remoteExpressions.isEmpty() | |
| : "both local expression and remote expressions are empty which should have errored earlier at index rewriting time"; | |
| - ElasticsearchSecurityException localSecurityException = null; | |
| - if (localException instanceof ElasticsearchSecurityException securityException) localSecurityException = securityException; | |
| + ElasticsearchSecurityException currentExpressionSecurityException = null; | |
| + if (localException instanceof ElasticsearchSecurityException securityException) { | |
| + currentExpressionSecurityException = securityException; | |
| + } | |
| boolean foundFlat = false; | |
| + BiConsumer< | |
| + Map<String, ElasticsearchSecurityException>, | |
| + Map<String, List<String>>> populateRemoteSecurityExceptionAndUnauthorizedIndices = null; | |
| + | |
| // checking if flat expression matched remotely | |
| for (String remoteExpression : remoteExpressions) { | |
| String[] splitResource = splitQualifiedResource(remoteExpression); | |
| @@ -178,39 +191,56 @@ public class CrossProjectIndexResolutionValidator { | |
| if (remoteException == null) { | |
| // found flat expression somewhere | |
| foundFlat = true; | |
| - // ensure we don't report this index as unauthorized if it's unauthorized on another remote | |
| - if (remoteUnauthorizedIndices != null) { | |
| - for (var entry : remoteUnauthorizedIndices.entrySet()) { | |
| - entry.setValue(entry.getValue().stream().filter(index -> index.endsWith(":" + resource) == false).toList()); | |
| - } | |
| - } | |
| break; | |
| } | |
| - if (localSecurityException == null && remoteException instanceof ElasticsearchSecurityException securityException) { | |
| - if (remoteAuthorizationExceptions == null) { | |
| - remoteAuthorizationExceptions = new HashMap<>(); | |
| - remoteUnauthorizedIndices = new HashMap<>(); | |
| - } | |
| - remoteAuthorizationExceptions.putIfAbsent(projectAlias, securityException); | |
| - remoteUnauthorizedIndices.computeIfAbsent(projectAlias, k -> new ArrayList<>()).add(remoteExpression); | |
| + if (currentExpressionSecurityException == null | |
| + && remoteException instanceof ElasticsearchSecurityException securityException) { | |
| + currentExpressionSecurityException = securityException; | |
| + // It is possible that the expression is found on a later linked project. So we defer its exception propagation | |
| + // with a lambda so that it is executed only when the expression is not found anywhere. | |
| + assert populateRemoteSecurityExceptionAndUnauthorizedIndices == null; | |
| + populateRemoteSecurityExceptionAndUnauthorizedIndices = (exceptionsMap, unauthorizedIndicesMap) -> { | |
| + exceptionsMap.putIfAbsent(projectAlias, securityException); | |
| + unauthorizedIndicesMap.computeIfAbsent(projectAlias, k -> new ArrayList<>()).add(remoteExpression); | |
| + }; | |
| } | |
| } | |
| if (foundFlat) { | |
| continue; | |
| } | |
| - if (localSecurityException != null) { | |
| + if (populateRemoteSecurityExceptionAndUnauthorizedIndices != null) { | |
| + assert false == localException instanceof ElasticsearchSecurityException; | |
| + if (remoteAuthorizationExceptions == null) { | |
| + remoteAuthorizationExceptions = new HashMap<>(); | |
| + remoteUnauthorizedIndices = new HashMap<>(); | |
| + } | |
| + // The expression is not found anywhere and we have a remote security exception to populate | |
| + populateRemoteSecurityExceptionAndUnauthorizedIndices.accept(remoteAuthorizationExceptions, remoteUnauthorizedIndices); | |
| + } | |
| + | |
| + if (currentExpressionSecurityException != null && currentExpressionSecurityException == localException) { | |
| + // We have a local security exception for this unqualified expression. That's what we want to report, i.e. | |
| + // we are no longer interested in whether any linked project also returns a security exception. | |
| + assert populateRemoteSecurityExceptionAndUnauthorizedIndices == null; | |
| if (localAuthorizationException == null) { | |
| - localAuthorizationException = localSecurityException; | |
| + localAuthorizationException = currentExpressionSecurityException; | |
| localUnauthorizedIndices = new ArrayList<>(); | |
| } | |
| localUnauthorizedIndices.add(originalExpression); | |
| } else if (localException != null) { | |
| + // We have a local 404 for this unqualified expression which takes priority over any remote 404 | |
| assert localException instanceof IndexNotFoundException | |
| : "Expected local exception to be IndexNotFoundException, but found: " + localException; | |
| - notFoundException = (IndexNotFoundException) localException; | |
| + if (notFoundException == null) { | |
| + notFoundException = (IndexNotFoundException) localException; | |
| + } | |
| } else { | |
| + // Local project is excluded and 404 from all remotes, we report 404 for the first remote project | |
| + assert localExpressions == ResolvedIndexExpression.LocalExpressions.NONE : localExpressions; | |
| assert false == remoteExpressions.isEmpty() : "expected remote expressions to be non-empty"; | |
| - notFoundException = new IndexNotFoundException(remoteExpressions.iterator().next()); | |
| + if (notFoundException == null) { | |
| + notFoundException = new IndexNotFoundException(remoteExpressions.iterator().next()); | |
| + } | |
| } | |
| } | |
| } | |
| diff --git a/server/src/test/java/org/elasticsearch/search/crossproject/CrossProjectIndexResolutionValidatorTests.java b/server/src/test/java/org/elasticsearch/search/crossproject/CrossProjectIndexResolutionValidatorTests.java | |
| index 4e3f3ed9344..7b4a971470d 100644 | |
| --- a/server/src/test/java/org/elasticsearch/search/crossproject/CrossProjectIndexResolutionValidatorTests.java | |
| +++ b/server/src/test/java/org/elasticsearch/search/crossproject/CrossProjectIndexResolutionValidatorTests.java | |
| @@ -25,6 +25,7 @@ import java.util.Set; | |
| import static org.hamcrest.Matchers.arrayWithSize; | |
| import static org.hamcrest.Matchers.containsString; | |
| +import static org.hamcrest.Matchers.emptyArray; | |
| import static org.hamcrest.Matchers.equalTo; | |
| import static org.hamcrest.Matchers.instanceOf; | |
| import static org.hamcrest.Matchers.is; | |
| @@ -999,7 +1000,7 @@ public class CrossProjectIndexResolutionValidatorTests extends ESTestCase { | |
| assertThat(e.getMessage(), containsString("no such index [P1:logs*]")); | |
| } | |
| - public void testFlatExpressionWithMultipleRemotesPartiallyUnauthorizedStrictIgnoreUnavailable() { | |
| + public void testUnqualifiedExpressionSuccessWhenFoundAnyProject() { | |
| var local = new ResolvedIndexExpressions( | |
| List.of( | |
| new ResolvedIndexExpression( | |
| @@ -1048,7 +1049,43 @@ public class CrossProjectIndexResolutionValidatorTests extends ESTestCase { | |
| assertThat(e, is(nullValue())); | |
| } | |
| - public void testFlatExpressionUnauthorizedAndFlatExpressionPartiallyAuthorizedStrictIgnoreUnavailable() { | |
| + public void testReport403Over404() { | |
| + var local = new ResolvedIndexExpressions( | |
| + List.of( | |
| + new ResolvedIndexExpression( | |
| + "logs", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of("logs"), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE, | |
| + null | |
| + ), | |
| + Set.of("P1:logs", "P2:logs") | |
| + ) | |
| + ) | |
| + ); | |
| + var remote = Map.of( | |
| + "P1", | |
| + new ResolvedIndexExpressions( | |
| + List.of( | |
| + new ResolvedIndexExpression( | |
| + "logs", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of("logs"), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_UNAUTHORIZED, | |
| + new ElasticsearchSecurityException("Unauthorized for -*") | |
| + ), | |
| + Set.of() | |
| + ) | |
| + ) | |
| + ) | |
| + ); | |
| + | |
| + var e = CrossProjectIndexResolutionValidator.validate(getStrictIgnoreUnavailable(), null, local, remote); | |
| + assertThat(e, instanceOf(ElasticsearchSecurityException.class)); | |
| + assertThat(e.getMessage(), containsString("P1:logs")); | |
| + } | |
| + | |
| + public void testUnqualifiedExpressionShouldReportFirst403() { | |
| var local = new ResolvedIndexExpressions( | |
| List.of( | |
| new ResolvedIndexExpression( | |
| @@ -1126,10 +1163,251 @@ public class CrossProjectIndexResolutionValidatorTests extends ESTestCase { | |
| var e = CrossProjectIndexResolutionValidator.validate(getStrictIgnoreUnavailable(), null, local, remote); | |
| assertThat(e, is(notNullValue())); | |
| assertThat(e.getMessage(), equalTo("Unauthorized for P1:metrics")); | |
| + assertThat(e.getSuppressed(), emptyArray()); | |
| + } | |
| + | |
| + public void testQualifiedExpressionShouldReport403FromAllProjects() { | |
| + var local = new ResolvedIndexExpressions( | |
| + List.of( | |
| + new ResolvedIndexExpression( | |
| + "*:metrics", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE, | |
| + null | |
| + ), | |
| + Set.of("P1:metrics", "P2:metrics") | |
| + ), | |
| + new ResolvedIndexExpression( | |
| + "*:logs", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE, | |
| + null | |
| + ), | |
| + Set.of("P1:logs", "P2:logs") | |
| + ) | |
| + ) | |
| + ); | |
| + var remote = new LinkedHashMap<String, ResolvedIndexExpressions>(); | |
| + remote.put( | |
| + "P1", | |
| + new ResolvedIndexExpressions( | |
| + List.of( | |
| + new ResolvedIndexExpression( | |
| + "metrics", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_UNAUTHORIZED, | |
| + new ElasticsearchSecurityException("Unauthorized for -*") | |
| + ), | |
| + Set.of() | |
| + ), | |
| + new ResolvedIndexExpression( | |
| + "logs", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_UNAUTHORIZED, | |
| + new ElasticsearchSecurityException("Unauthorized for -*") | |
| + ), | |
| + Set.of() | |
| + ) | |
| + ) | |
| + ) | |
| + ); | |
| + remote.put( | |
| + "P2", | |
| + new ResolvedIndexExpressions( | |
| + List.of( | |
| + new ResolvedIndexExpression( | |
| + "metrics", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_UNAUTHORIZED, | |
| + new ElasticsearchSecurityException("Unauthorized for -*") | |
| + ), | |
| + Set.of() | |
| + ), | |
| + new ResolvedIndexExpression( | |
| + "logs", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.SUCCESS, | |
| + null | |
| + ), | |
| + Set.of() | |
| + ) | |
| + ) | |
| + ) | |
| + ); | |
| + | |
| + var e = CrossProjectIndexResolutionValidator.validate(getStrictIgnoreUnavailable(), null, local, remote); | |
| + assertThat(e, is(notNullValue())); | |
| + assertThat(e.getMessage(), equalTo("Unauthorized for P1:metrics,P1:logs")); | |
| assertThat(e.getSuppressed(), arrayWithSize(1)); | |
| assertThat(e.getSuppressed()[0].getMessage(), equalTo("Unauthorized for P2:metrics")); | |
| } | |
| + public void testShouldReportFirst404ExceptionWhenNo403() { | |
| + final String originalExpression = randomFrom("metrics", "*:metrics"); | |
| + var local = new ResolvedIndexExpressions( | |
| + List.of( | |
| + new ResolvedIndexExpression( | |
| + originalExpression, | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE, | |
| + null | |
| + ), | |
| + Set.of("P1:metrics", "P2:metrics") | |
| + ), | |
| + new ResolvedIndexExpression( | |
| + randomFrom("logs", "*:logs"), | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE, | |
| + null | |
| + ), | |
| + Set.of("P1:logs", "P2:logs") | |
| + ) | |
| + ) | |
| + ); | |
| + var remote = new LinkedHashMap<String, ResolvedIndexExpressions>(); | |
| + remote.put( | |
| + "P1", | |
| + new ResolvedIndexExpressions( | |
| + List.of( | |
| + new ResolvedIndexExpression( | |
| + "metrics", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE, | |
| + null | |
| + ), | |
| + Set.of() | |
| + ), | |
| + new ResolvedIndexExpression( | |
| + "logs", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE, | |
| + null | |
| + ), | |
| + Set.of() | |
| + ) | |
| + ) | |
| + ) | |
| + ); | |
| + remote.put( | |
| + "P2", | |
| + new ResolvedIndexExpressions( | |
| + List.of( | |
| + new ResolvedIndexExpression( | |
| + "metrics", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE, | |
| + null | |
| + ), | |
| + Set.of() | |
| + ), | |
| + new ResolvedIndexExpression( | |
| + "logs", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE, | |
| + null | |
| + ), | |
| + Set.of() | |
| + ) | |
| + ) | |
| + ) | |
| + ); | |
| + | |
| + var e = CrossProjectIndexResolutionValidator.validate(getStrictIgnoreUnavailable(), null, local, remote); | |
| + assertThat(e, is(notNullValue())); | |
| + assertThat(e.getMessage(), equalTo("no such index [" + originalExpression + "]")); | |
| + assertThat(e.getSuppressed(), emptyArray()); | |
| + } | |
| + | |
| + public void testShouldReportFirstRemote404WhenNo403AndLocalProjectIsExcluded() { | |
| + final String originalExpression = randomFrom("metrics", "*:metrics"); | |
| + var local = new ResolvedIndexExpressions( | |
| + List.of( | |
| + new ResolvedIndexExpression( | |
| + originalExpression, | |
| + ResolvedIndexExpression.LocalExpressions.NONE, | |
| + Set.of("P1:metrics", "P2:metrics") | |
| + ), | |
| + new ResolvedIndexExpression( | |
| + randomFrom("logs", "*:logs"), | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE, | |
| + null | |
| + ), | |
| + Set.of("P1:logs", "P2:logs") | |
| + ) | |
| + ) | |
| + ); | |
| + var remote = new LinkedHashMap<String, ResolvedIndexExpressions>(); | |
| + remote.put( | |
| + "P1", | |
| + new ResolvedIndexExpressions( | |
| + List.of( | |
| + new ResolvedIndexExpression( | |
| + "metrics", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE, | |
| + null | |
| + ), | |
| + Set.of() | |
| + ), | |
| + new ResolvedIndexExpression( | |
| + "logs", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE, | |
| + null | |
| + ), | |
| + Set.of() | |
| + ) | |
| + ) | |
| + ) | |
| + ); | |
| + remote.put( | |
| + "P2", | |
| + new ResolvedIndexExpressions( | |
| + List.of( | |
| + new ResolvedIndexExpression( | |
| + "metrics", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE, | |
| + null | |
| + ), | |
| + Set.of() | |
| + ), | |
| + new ResolvedIndexExpression( | |
| + "logs", | |
| + new ResolvedIndexExpression.LocalExpressions( | |
| + Set.of(), | |
| + ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE, | |
| + null | |
| + ), | |
| + Set.of() | |
| + ) | |
| + ) | |
| + ) | |
| + ); | |
| + | |
| + var e = CrossProjectIndexResolutionValidator.validate(getStrictIgnoreUnavailable(), null, local, remote); | |
| + assertThat(e, is(notNullValue())); | |
| + assertThat(e.getMessage(), equalTo("no such index [P1:metrics]")); | |
| + assertThat(e.getSuppressed(), emptyArray()); | |
| + } | |
| + | |
| private IndicesOptions getStrictAllowNoIndices() { | |
| return getIndicesOptions(true, false); | |
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment