-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Enable queryable built-in roles feature by default #120323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1f1f841
71dff22
1fadace
d4a7f19
aa23217
be5fedc
5ae202b
c00c82b
f9d550d
0bdf414
68b1431
43bcf32
c3e0f5a
ecb1d90
a49b86b
d5b5cd7
d1c46e0
864bbb4
db933e1
eff001d
86dbcd3
353191b
0d2c32b
6c7cd5b
122a0c3
ce64c85
2a5968b
ec6b390
7e10d30
ce66ed1
2f0d846
5ede368
6585c22
8803225
ef8e159
31d78a7
42ff42f
36bc5d8
6cd2425
fa2a29b
2bc36cb
b05408d
e24c6e6
b47523e
ee42e42
0eab0b7
bcb29f9
8707455
4b76919
4b85915
edf4d77
3279bdb
0b2b151
55d3055
5c76728
09924df
637fc5f
4fb3493
df0e17b
25987ad
e389c3e
1db9511
c94a637
0f347b3
6c57650
aa38644
adcbb0f
d458e5d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |
| import java.nio.file.Path; | ||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.concurrent.Callable; | ||
| import java.util.regex.Matcher; | ||
| import java.util.regex.Pattern; | ||
| import java.util.stream.Stream; | ||
|
|
@@ -47,7 +48,9 @@ public void test010Install() throws Exception { | |
| public void test20GeneratePasswords() throws Exception { | ||
| assertWhileRunning(() -> { | ||
| ServerUtils.waitForElasticsearch(installation); | ||
| Shell.Result result = installation.executables().setupPasswordsTool.run("auto --batch", null); | ||
| Shell.Result result = retryOnAuthenticationErrors( | ||
jfreden marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| () -> installation.executables().setupPasswordsTool.run("auto --batch", null) | ||
| ); | ||
| Map<String, String> userpasses = parseUsersAndPasswords(result.stdout()); | ||
| for (Map.Entry<String, String> userpass : userpasses.entrySet()) { | ||
| String response = ServerUtils.makeRequest( | ||
|
|
@@ -102,20 +105,26 @@ public void test30AddBootstrapPassword() throws Exception { | |
| installation.executables().keystoreTool.run("add --stdin bootstrap.password", BOOTSTRAP_PASSWORD); | ||
|
|
||
| assertWhileRunning(() -> { | ||
| String response = ServerUtils.makeRequest( | ||
| Request.Get("http://localhost:9200/_cluster/health?wait_for_status=green&timeout=180s"), | ||
| "elastic", | ||
| BOOTSTRAP_PASSWORD, | ||
| null | ||
| ServerUtils.waitForElasticsearch("green", null, installation, "elastic", BOOTSTRAP_PASSWORD, null); | ||
| final String response = retryOnAuthenticationErrors( | ||
| () -> ServerUtils.makeRequest( | ||
| Request.Get("http://localhost:9200/_cluster/health?wait_for_status=green&timeout=180s"), | ||
| "elastic", | ||
| BOOTSTRAP_PASSWORD, | ||
| null | ||
| ) | ||
| ); | ||
| assertThat(response, containsString("\"status\":\"green\"")); | ||
| }); | ||
|
|
||
| } | ||
|
|
||
| public void test40GeneratePasswordsBootstrapAlreadySet() throws Exception { | ||
| assertWhileRunning(() -> { | ||
|
|
||
| Shell.Result result = installation.executables().setupPasswordsTool.run("auto --batch", null); | ||
| ServerUtils.waitForElasticsearch("green", null, installation, "elastic", BOOTSTRAP_PASSWORD, null); | ||
| Shell.Result result = retryOnAuthenticationErrors( | ||
| () -> installation.executables().setupPasswordsTool.run("auto --batch", null) | ||
| ); | ||
| Map<String, String> userpasses = parseUsersAndPasswords(result.stdout()); | ||
| assertThat(userpasses, hasKey("elastic")); | ||
| for (Map.Entry<String, String> userpass : userpasses.entrySet()) { | ||
|
|
@@ -130,6 +139,48 @@ public void test40GeneratePasswordsBootstrapAlreadySet() throws Exception { | |
| }); | ||
| } | ||
|
|
||
| /** | ||
| * The security index is created on startup. | ||
| * It can happen that even when the security index exists, we get an authentication failure as `elastic` | ||
| * user because the reserved realm checks the security index first. | ||
| * This is because we check the security index too early (just after the creation) when all shards did not get allocated yet. | ||
| * Hence, the call can result in an `UnavailableShardsException` and cause the authentication to fail. | ||
| * We retry here on authentication errors for a couple of seconds just to verify that this is not the case. | ||
| */ | ||
| private <R> R retryOnAuthenticationErrors(final Callable<R> callable) throws Exception { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Retrying here because the calls would have succeeded if we just waited a bit longer: |
||
| Exception failure = null; | ||
| int retries = 5; | ||
| while (retries-- > 0) { | ||
| try { | ||
| return callable.call(); | ||
| } catch (Exception e) { | ||
| if (e.getMessage() != null | ||
| && (e.getMessage().contains("401 Unauthorized") || e.getMessage().contains("Failed to authenticate user"))) { | ||
| logger.info( | ||
| "Authentication failed (possibly due to UnavailableShardsException for the security index), retrying [{}].", | ||
| retries, | ||
| e | ||
| ); | ||
| if (failure == null) { | ||
| failure = e; | ||
| } else { | ||
| failure.addSuppressed(e); | ||
| } | ||
| try { | ||
| Thread.sleep(1000); | ||
| } catch (InterruptedException interrupted) { | ||
| Thread.currentThread().interrupt(); | ||
| failure.addSuppressed(interrupted); | ||
| throw failure; | ||
| } | ||
| } else { | ||
| throw e; | ||
| } | ||
| } | ||
| } | ||
| throw failure; | ||
| } | ||
|
|
||
| private Map<String, String> parseUsersAndPasswords(String output) { | ||
| Matcher matcher = USERPASS_REGEX.matcher(output); | ||
| assertNotNull(matcher); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |
| import org.apache.logging.log4j.LogManager; | ||
| import org.apache.logging.log4j.Logger; | ||
| import org.apache.lucene.store.AlreadyClosedException; | ||
| import org.elasticsearch.action.UnavailableShardsException; | ||
| import org.elasticsearch.action.admin.cluster.configuration.AddVotingConfigExclusionsRequest; | ||
| import org.elasticsearch.action.admin.cluster.configuration.ClearVotingConfigExclusionsRequest; | ||
| import org.elasticsearch.action.admin.cluster.configuration.TransportAddVotingConfigExclusionsAction; | ||
|
|
@@ -146,6 +147,8 @@ | |
| import static org.elasticsearch.node.Node.INITIAL_STATE_TIMEOUT_SETTING; | ||
| import static org.elasticsearch.test.ESTestCase.TEST_REQUEST_TIMEOUT; | ||
| import static org.elasticsearch.test.ESTestCase.assertBusy; | ||
| import static org.elasticsearch.test.ESTestCase.assertFalse; | ||
| import static org.elasticsearch.test.ESTestCase.assertTrue; | ||
| import static org.elasticsearch.test.ESTestCase.randomFrom; | ||
| import static org.elasticsearch.test.ESTestCase.runInParallel; | ||
| import static org.elasticsearch.test.ESTestCase.safeAwait; | ||
|
|
@@ -160,9 +163,7 @@ | |
| import static org.hamcrest.Matchers.greaterThanOrEqualTo; | ||
| import static org.hamcrest.Matchers.not; | ||
| import static org.junit.Assert.assertEquals; | ||
| import static org.junit.Assert.assertFalse; | ||
| import static org.junit.Assert.assertThat; | ||
| import static org.junit.Assert.assertTrue; | ||
| import static org.junit.Assert.fail; | ||
|
|
||
| /** | ||
|
|
@@ -1240,16 +1241,29 @@ public synchronized void validateClusterFormed() { | |
| } | ||
| logger.trace("validating cluster formed, expecting {}", expectedNodes); | ||
|
|
||
| assertFalse( | ||
| client().admin() | ||
| .cluster() | ||
| .prepareHealth(TEST_REQUEST_TIMEOUT) | ||
| .setWaitForEvents(Priority.LANGUID) | ||
| .setWaitForNodes(Integer.toString(expectedNodes.size())) | ||
| .get(TimeValue.timeValueSeconds(40)) | ||
| .isTimedOut() | ||
| ); | ||
| try { | ||
| assertBusy(() -> { | ||
| try { | ||
| final boolean timeout = client().admin() | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This call may fail when client user has role with application privileges and security index is still not available for searching. See my other #120323 (comment) |
||
| .cluster() | ||
| .prepareHealth(TEST_REQUEST_TIMEOUT) | ||
| .setWaitForEvents(Priority.LANGUID) | ||
| .setWaitForNodes(Integer.toString(expectedNodes.size())) | ||
| .get(TimeValue.timeValueSeconds(40)) | ||
| .isTimedOut(); | ||
| if (timeout) { | ||
| throw new IllegalStateException("timed out waiting for cluster to form"); | ||
| } | ||
| } catch (UnavailableShardsException e) { | ||
| if (e.getMessage() != null && e.getMessage().contains(".security")) { | ||
| // security index may not be ready yet, throwing assertion error to retry | ||
| throw new AssertionError(e); | ||
| } else { | ||
| throw e; | ||
| } | ||
| } | ||
| }, 30, TimeUnit.SECONDS); | ||
|
|
||
| final Object[] previousStates = new Object[1]; | ||
| assertBusy(() -> { | ||
| final List<ClusterState> states = nodes.values() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,3 +104,6 @@ tasks.named("yamlRestCompatTestTransform").configure({ task -> | |
|
|
||
| }) | ||
|
|
||
| tasks.named('yamlRestCompatTest').configure { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only disabling it for |
||
| systemProperty 'es.queryable_built_in_roles_enabled', 'false' | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -218,7 +218,7 @@ public void testAnonymousUserFromQueryClusterWorks() throws Exception { | |
| { "password": "%s" }""", PASS)); | ||
| assertOK(client().performRequest(changePasswordRequest)); | ||
|
|
||
| final Request elasticUserSearchRequest = new Request("GET", "/*:.security*/_search"); | ||
| final Request elasticUserSearchRequest = new Request("GET", "/*:.security*/_search?size=1"); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We get now more than one result from |
||
| elasticUserSearchRequest.setOptions( | ||
| RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", basicAuthHeaderValue("elastic", PASS)) | ||
| ); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.