Entire FollowIndexSecurityIT suite sometimes fails

[jtibshirani]

There have been several build failures in the past few days where the whole FollowIndexSecurityIT suite fails with connection errors:

java.net.ConnectException: Connection refused
        at __randomizedtesting.SeedInfo.seed([D5A8084EC9E25925:E6A6A837ACA01A02]:0)
        at org.elasticsearch.client.RestClient.extractAndWrapCause(RestClient.java:918)
        at org.elasticsearch.client.RestClient.performRequest(RestClient.java:299)

Some interesting errors in logs:

[2022-02-14T23:30:20,568][WARN ][o.e.t.TcpTransport       ] [follow-cluster-0] exception caught on transport layer [Netty4TcpChannel{localAddress=/127.0.0.1:41444, remoteAddress=127.0.0.1/127.0.0.1:38375, profile=default}], closing connection
»  org.elasticsearch.ElasticsearchSecurityException: action [indices:admin/seq_no/renew_retention_lease] is unauthorized for user [test_ccr] with roles [ccruser], this action is granted by the index privileges [manage,all]
»  	at org.elasticsearch.xpack.core.security.support.Exceptions.authorizationError(Exceptions.java:36) ~[?:?]
»  	at org.elasticsearch.xpack.security.authz.AuthorizationService.denialException(AuthorizationService.java:919) ~[?:?]
»  	at org.elasticsearch.xpack.security.authz.AuthorizationService$AuthorizationResultListener.handleFailure(AuthorizationService.java:978) ~[?:?]
»  	at org.elasticsearch.xpack.security.authz.AuthorizationService$AuthorizationResultListener.onResponse(AuthorizationService.java:963) ~[?:?]

Examples:

• https://gradle-enterprise.elastic.co/s/tqlnn3z6a5i7m
• https://gradle-enterprise.elastic.co/s/eaj7lxro2bv4o
• https://gradle-enterprise.elastic.co/s/hmhadf6fezkli

[elasticmachine]

Pinging @elastic/es-distributed (Team:Distributed)

[DaveCTurner]

Henning and I have worked out what's going on here but I think I'm going to leave it to @elastic/es-security to choose a fix as the best path forward is far from obvious. You can reproduce the failure by sleeping here for long enough to trigger a renewal action (≥ 30s since that's the default and the test doesn't override it):

diff --git a/x-pack/plugin/ccr/qa/security/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexSecurityIT.java b/x-pack/plugin/ccr/qa/security/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexSecurityIT.java
index 25670121963..426c927cac9 100644
--- a/x-pack/plugin/ccr/qa/security/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexSecurityIT.java
+++ b/x-pack/plugin/ccr/qa/security/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexSecurityIT.java
@@ -178,6 +178,7 @@ public class FollowIndexSecurityIT extends ESCCRRestTestCase {
             verifyDocuments(allowedIndex, 5, "*:*");
         }, 30, TimeUnit.SECONDS);
         assertThat(indexExists(disallowedIndex), is(false));
+        Thread.sleep(40000);
         assertBusy(() -> {
             verifyCcrMonitoring(allowedIndex, allowedIndex);
             verifyAutoFollowMonitoring();

The explanation is as follows. It looks like we run the failing action in system context:

elasticsearch/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/ShardFollowTasksExecutor.java

Lines 522 to 523 in fb65f95

	// we have to execute under the system context so that if security is enabled the management is authorized
	threadContext.markAsSystemContext();

It turns out (after several hours of investigation) this is simply false. We execute the action using a client obtained from remoteClient():

elasticsearch/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/ShardFollowTasksExecutor.java

Line 533 in fb65f95

remoteClient(params),

This client comes from CcrLicenseChecker#wrapClient() which uses ClientHelper#executeWithHeadersAsync() and therefore ignores the context in which it's executed and effectively restores the original thread context in which the shard follow task was created.

In practice we don't see this exception in any production clusters. If we did, it would be logged here just after the tripping assertion:

elasticsearch/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/ShardFollowTasksExecutor.java

Lines 541 to 551 in fb65f95

	assert cause instanceof ElasticsearchSecurityException == false : cause;
	if (cause instanceof RetentionLeaseInvalidRetainingSeqNoException == false) {
	logger.warn(
	new ParameterizedMessage(
	"{} background management of retention lease [{}] failed while following",
	params.getFollowShardId(),
	retentionLeaseId
	),
	cause
	);
	}

Our conclusion is that in practice everyone must set up their shard follow tasks using a user that is sufficiently privileged to renew retention leases on the leader index. One possible fix is to stop using a wrapped client and execute the renewal in system context as claimed, but another is to treat this as a test bug which could be fixed by giving the test user the privileges it needs to do its stuff and drop the bogus call to threadContext.markAsSystemContext().

I suspect there's a similar problem with the call to asyncAddRetentionLease a few lines up, although to hit this in a test I think we'd have to wait ≥12h for the leases to expire.

It's unclear why this suite has started to fail more frequently since 2022-02-13, but we can reproduce the same failure in older versions too. Perhaps something just got slower to a point where the retention lease renewal has time to trigger.

More generally, I couldn't work out why everywhere in CCR uses these wrapped clients to fiddle with the thread context on execution instead of just restoring the task's thread context at the top level. I think running everything related to a task in the proper thread context would be less trappy.