From 605381e7f7ec77a2dd5f16d2c8047412847679e1 Mon Sep 17 00:00:00 2001 From: David Turner Date: Fri, 2 Jan 2026 08:20:25 +0000 Subject: [PATCH 1/2] Improve testing of STS credentials reloading Today `CustomWebIdentityTokenCredentialsProviderTests` verifies that a S3 repository will reload credentials from STS, but fakes out the interactions with the environment variables and system properties via special test-only abstractions in the production code. This makes it hard to be confident that the real system behaves as expected. This commit replaces this suite with ones that verify the behaviour of a real Elasticsearch node, removing the need for the extra abstractions in the production code. --- .../qa/web-identity-token/build.gradle | 20 -- ...IdentityTokenCredentialsProviderTests.java | 230 ------------------ ...epositoryS3StsCredentialsReloadRestIT.java | 161 ++++++++++++ .../s3/RepositoryS3StsCredentialsRestIT.java | 4 +- .../repositories/s3/S3Service.java | 30 +-- .../fixture/aws/DynamicAwsCredentials.java | 4 + .../fixture/aws/sts/AwsStsHttpFixture.java | 16 +- .../fixture/aws/sts/AwsStsHttpHandler.java | 21 +- .../aws/sts/AwsStsHttpHandlerTests.java | 27 +- .../local/DefaultLocalClusterHandle.java | 4 + .../DefaultLocalElasticsearchCluster.java | 7 + .../cluster/local/LocalClusterHandle.java | 6 + 12 files changed, 239 insertions(+), 291 deletions(-) delete mode 100644 modules/repository-s3/qa/web-identity-token/build.gradle delete mode 100644 modules/repository-s3/qa/web-identity-token/src/test/java/org/elasticsearch/repositories/s3/CustomWebIdentityTokenCredentialsProviderTests.java create mode 100644 modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsReloadRestIT.java diff --git a/modules/repository-s3/qa/web-identity-token/build.gradle b/modules/repository-s3/qa/web-identity-token/build.gradle deleted file mode 100644 index b87c52663d241..0000000000000 --- a/modules/repository-s3/qa/web-identity-token/build.gradle +++ /dev/null @@ -1,20 +0,0 @@ -/* - * 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; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -apply plugin: 'elasticsearch.java' - -dependencies { - testImplementation project(':modules:repository-s3') - testImplementation project(':test:framework') - testImplementation project(':server') - testImplementation "software.amazon.awssdk:auth:${versions.awsv2sdk}" - implementation "software.amazon.awssdk:identity-spi:${versions.awsv2sdk}" -} - -tasks.named("test").configure { - environment 'AWS_REGION', 'es-test-region' -} diff --git a/modules/repository-s3/qa/web-identity-token/src/test/java/org/elasticsearch/repositories/s3/CustomWebIdentityTokenCredentialsProviderTests.java b/modules/repository-s3/qa/web-identity-token/src/test/java/org/elasticsearch/repositories/s3/CustomWebIdentityTokenCredentialsProviderTests.java deleted file mode 100644 index 03ac986d038f7..0000000000000 --- a/modules/repository-s3/qa/web-identity-token/src/test/java/org/elasticsearch/repositories/s3/CustomWebIdentityTokenCredentialsProviderTests.java +++ /dev/null @@ -1,230 +0,0 @@ -/* - * 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.repositories.s3; - -import software.amazon.awssdk.auth.credentials.AwsCredentials; -import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; - -import com.sun.net.httpserver.HttpServer; - -import org.apache.logging.log4j.LogManager; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.core.Strings; -import org.elasticsearch.core.SuppressForbidden; -import org.elasticsearch.core.TimeValue; -import org.elasticsearch.env.Environment; -import org.elasticsearch.mocksocket.MockHttpServer; -import org.elasticsearch.rest.RestStatus; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.threadpool.TestThreadPool; -import org.elasticsearch.watcher.ResourceWatcherService; -import org.junit.After; -import org.junit.Assert; -import org.mockito.Mockito; - -import java.io.IOException; -import java.net.InetAddress; -import java.net.InetSocketAddress; -import java.net.URLDecoder; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; -import java.time.Clock; -import java.time.Instant; -import java.time.ZoneOffset; -import java.time.ZonedDateTime; -import java.time.format.DateTimeFormatter; -import java.util.Arrays; -import java.util.Map; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; -import java.util.function.Consumer; -import java.util.stream.Collectors; - -public class CustomWebIdentityTokenCredentialsProviderTests extends ESTestCase { - - private static final String ROLE_ARN = "arn:aws:iam::123456789012:role/FederatedWebIdentityRole"; - private static final String ROLE_NAME = "aws-sdk-java-1651084775908"; - private final TestThreadPool threadPool = new TestThreadPool("test"); - private final Settings settings = Settings.builder().put("resource.reload.interval.low", TimeValue.timeValueMillis(100)).build(); - private final ResourceWatcherService resourceWatcherService = new ResourceWatcherService(settings, threadPool); - - @After - public void shutdown() throws Exception { - resourceWatcherService.close(); - threadPool.shutdown(); - } - - private static Environment getEnvironment() throws IOException { - Path configDirectory = createTempDir("web-identity-token-test"); - Files.createDirectory(configDirectory.resolve("repository-s3")); - Files.writeString(configDirectory.resolve("repository-s3/aws-web-identity-token-file"), "YXdzLXdlYi1pZGVudGl0eS10b2tlbi1maWxl"); - Environment environment = Mockito.mock(Environment.class); - Mockito.when(environment.configDir()).thenReturn(configDirectory); - return environment; - } - - @SuppressForbidden(reason = "HTTP server is used for testing") - private static HttpServer getHttpServer(Consumer webIdentityTokenCheck) throws IOException { - HttpServer httpServer = MockHttpServer.createHttp(new InetSocketAddress(InetAddress.getLoopbackAddress().getHostAddress(), 0), 0); - httpServer.createContext("/", exchange -> { - try (exchange) { - String body = new String(exchange.getRequestBody().readAllBytes(), StandardCharsets.UTF_8); - Map params = Arrays.stream(body.split("&")) - .map(e -> e.split("=")) - .collect(Collectors.toMap(e -> e[0], e -> URLDecoder.decode(e[1], StandardCharsets.UTF_8))); - assertEquals(ROLE_NAME, params.get("RoleSessionName")); - webIdentityTokenCheck.accept(params.get("WebIdentityToken")); - - exchange.getResponseHeaders().add("Content-Type", "text/xml; charset=UTF-8"); - byte[] response = Strings.format( - """ - - - amzn1.account.AF6RHO7KZU5XRVQJGXK6HB56KR2A - client.5498841531868486423.1548@apps.example.com - - %s - AROACLKWSDQRAOEXAMPLE:%s - - - sts_session_token - secret_access_key - %s - sts_access_key - - SourceIdentityValue - www.amazon.com - - - ad4156e9-bce1-11e2-82e6-6b6efEXAMPLE - - - """, - ROLE_ARN, - ROLE_NAME, - ZonedDateTime.now(Clock.systemUTC()) - .plusSeconds(1L) // short expiry to force a reload - .format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ssZ")) - ).getBytes(StandardCharsets.UTF_8); - exchange.sendResponseHeaders(RestStatus.OK.getStatus(), response.length); - exchange.getResponseBody().write(response); - } - }); - httpServer.start(); - return httpServer; - } - - @SuppressForbidden(reason = "HTTP server is used for testing") - private static Map getSystemProperties(HttpServer httpServer) { - return Map.of( - "org.elasticsearch.repositories.s3.stsEndpointOverride", - "http://" + httpServer.getAddress().getHostName() + ":" + httpServer.getAddress().getPort() - ); - } - - private static Map environmentVariables() { - return Map.of("AWS_WEB_IDENTITY_TOKEN_FILE", "/var/run/secrets/eks.amazonaws.com/serviceaccount/token", "AWS_ROLE_ARN", ROLE_ARN); - } - - private static void assertCredentials(AwsCredentials credentials) { - Assert.assertFalse(credentials.accessKeyId().isEmpty()); - Assert.assertFalse(credentials.secretAccessKey().isEmpty()); - } - - @SuppressForbidden(reason = "HTTP server is used for testing") - public void testCreateWebIdentityTokenCredentialsProvider() throws Exception { - HttpServer httpServer = getHttpServer(s -> assertEquals("YXdzLXdlYi1pZGVudGl0eS10b2tlbi1maWxl", s)); - - Environment environment = getEnvironment(); - - // No region is set, but the SDK shouldn't fail because of that - Map environmentVariables = environmentVariables(); - Map systemProperties = getSystemProperties(httpServer); - var webIdentityTokenCredentialsProvider = new S3Service.CustomWebIdentityTokenCredentialsProvider( - environment, - environmentVariables::get, - systemProperties::getOrDefault, - Clock.fixed(Instant.ofEpochMilli(1651084775908L), ZoneOffset.UTC), - resourceWatcherService - ); - try { - AwsCredentials credentials = S3Service.buildCredentials( - LogManager.getLogger(S3Service.class), - S3ClientSettings.getClientSettings(Settings.EMPTY, randomAlphaOfLength(8)), - webIdentityTokenCredentialsProvider - ).resolveCredentials(); - - assertCredentials(credentials); - } finally { - webIdentityTokenCredentialsProvider.close(); - httpServer.stop(0); - } - } - - private static class DelegatingConsumer implements Consumer { - private Consumer delegate; - - private DelegatingConsumer(Consumer delegate) { - this.delegate = delegate; - } - - private void setDelegate(Consumer delegate) { - this.delegate = delegate; - } - - @Override - public void accept(String s) { - delegate.accept(s); - } - } - - @SuppressForbidden(reason = "HTTP server is used for testing") - public void testPickUpNewWebIdentityTokenWhenItsChanged() throws Exception { - DelegatingConsumer webIdentityTokenCheck = new DelegatingConsumer(s -> assertEquals("YXdzLXdlYi1pZGVudGl0eS10b2tlbi1maWxl", s)); - - HttpServer httpServer = getHttpServer(webIdentityTokenCheck); - Environment environment = getEnvironment(); - Map environmentVariables = environmentVariables(); - Map systemProperties = getSystemProperties(httpServer); - var webIdentityTokenCredentialsProvider = new S3Service.CustomWebIdentityTokenCredentialsProvider( - environment, - environmentVariables::get, - systemProperties::getOrDefault, - Clock.fixed(Instant.ofEpochMilli(1651084775908L), ZoneOffset.UTC), - resourceWatcherService - ); - try { - AwsCredentialsProvider awsCredentialsProvider = S3Service.buildCredentials( - LogManager.getLogger(S3Service.class), - S3ClientSettings.getClientSettings(Settings.EMPTY, randomAlphaOfLength(8)), - webIdentityTokenCredentialsProvider - ); - assertCredentials(awsCredentialsProvider.resolveCredentials()); - - var latch = new CountDownLatch(1); - String newWebIdentityToken = "88f84342080d4671a511e10ae905b2b0"; - webIdentityTokenCheck.setDelegate(s -> { - if (s.equals(newWebIdentityToken)) { - latch.countDown(); - } - }); - Files.writeString(environment.configDir().resolve("repository-s3/aws-web-identity-token-file"), newWebIdentityToken); - do { - // re-resolve credentials in order to trigger a refresh - assertCredentials(awsCredentialsProvider.resolveCredentials()); - } while (latch.await(500, TimeUnit.MILLISECONDS) == false); - assertCredentials(awsCredentialsProvider.resolveCredentials()); - } finally { - webIdentityTokenCredentialsProvider.close(); - httpServer.stop(0); - } - } -} diff --git a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsReloadRestIT.java b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsReloadRestIT.java new file mode 100644 index 0000000000000..191ebfa5362c5 --- /dev/null +++ b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsReloadRestIT.java @@ -0,0 +1,161 @@ +/* + * 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.repositories.s3; + +import fixture.aws.DynamicAwsCredentials; +import fixture.aws.DynamicRegionSupplier; +import fixture.aws.sts.AwsStsHttpFixture; +import fixture.aws.sts.AwsStsHttpHandler; +import fixture.s3.S3ConsistencyModel; +import fixture.s3.S3HttpFixture; +import io.netty.handler.codec.http.HttpMethod; + +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; + +import org.elasticsearch.client.Request; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.TimeValue; +import org.elasticsearch.test.cluster.ElasticsearchCluster; +import org.elasticsearch.test.cluster.util.resource.Resource; +import org.elasticsearch.test.fixtures.testcontainers.TestContainersThreadFilter; +import org.elasticsearch.test.rest.ESRestTestCase; +import org.jetbrains.annotations.NotNull; +import org.junit.ClassRule; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; + +import java.io.IOException; +import java.nio.file.Files; +import java.util.function.Supplier; + +import static org.elasticsearch.repositories.s3.AbstractRepositoryS3RestTestCase.getIdentifierPrefix; +import static org.hamcrest.Matchers.equalTo; + +@ThreadLeakFilters(filters = { TestContainersThreadFilter.class }) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) // https://github.com/elastic/elasticsearch/issues/102482 +public class RepositoryS3StsCredentialsReloadRestIT extends ESRestTestCase { + + private static final String PREFIX = getIdentifierPrefix("RepositoryS3StsCredentialsRestIT"); + private static final String BUCKET = PREFIX + "bucket"; + private static final String BASE_PATH = PREFIX + "base_path"; + private static final String CLIENT = "sts_credentials_reload_client"; + + private static final Supplier regionSupplier = new DynamicRegionSupplier(); + private static final DynamicAwsCredentials dynamicCredentials = new DynamicAwsCredentials(regionSupplier, "s3"); + + private static final S3HttpFixture s3HttpFixture = new S3HttpFixture( + true, + BUCKET, + BASE_PATH, + S3ConsistencyModel::randomConsistencyModel, + dynamicCredentials::isAuthorized + ); + + private static volatile String expectedWebIdentityTokenFileContents = "first token"; + + private static final AwsStsHttpFixture stsHttpFixture = new AwsStsHttpFixture( + dynamicCredentials::addValidCredentials, + () -> expectedWebIdentityTokenFileContents, + TimeValue.timeValueSeconds(115) // SDK tries to refresh credentials if they expire in less than 2 minutes == 120s + ); + + public static ElasticsearchCluster cluster = ElasticsearchCluster.local() + .module("repository-s3") + .setting("s3.client." + CLIENT + ".endpoint", s3HttpFixture::getAddress) + .systemProperty("org.elasticsearch.repositories.s3.stsEndpointOverride", stsHttpFixture::getAddress) + .configFile( + S3Service.CustomWebIdentityTokenCredentialsProvider.WEB_IDENTITY_TOKEN_FILE_LOCATION, + Resource.fromString("not ready yet") + ) + // When running in EKS with container identity the environment variable `AWS_WEB_IDENTITY_TOKEN_FILE` will point to a file which + // ES cannot access due to its security policy; we override it with `${ES_CONF_PATH}/repository-s3/aws-web-identity-token-file` + // and require the user to set up a symlink at this location. Thus we can set `AWS_WEB_IDENTITY_TOKEN_FILE` to any old path: + .environment("AWS_WEB_IDENTITY_TOKEN_FILE", () -> randomIdentifier() + "/" + randomIdentifier()) + // The AWS STS SDK requires the role ARN, it also accepts a session name but will make one up if it's not set. + // These are checked in AwsStsHttpHandler: + .environment("AWS_ROLE_ARN", AwsStsHttpHandler.ROLE_ARN) + .environment("AWS_ROLE_SESSION_NAME", AwsStsHttpHandler.ROLE_NAME) + // SDKv2 always uses regional endpoints + .environment("AWS_STS_REGIONAL_ENDPOINTS", () -> randomBoolean() ? "regional" : null) + .environment("AWS_REGION", regionSupplier) + .build(); + + @ClassRule + public static TestRule ruleChain = RuleChain.outerRule(s3HttpFixture).around(stsHttpFixture).around(cluster); + + @Override + protected String getTestRestCluster() { + return cluster.getHttpAddresses(); + } + + public void testReloadCredentials() throws IOException { + final var repositoryName = "repo-" + randomIdentifier(); + + // token file starts out invalid, so we cannot create the repository + assertThat( + expectThrows(ResponseException.class, () -> client().performRequest(getPutRepositoryRequest(repositoryName))).getResponse() + .getStatusLine() + .getStatusCode(), + equalTo(500) + ); + + // filling in the expected file contents yields success immediately, so we must be re-reading this file correctly + final var webIdentityTokenFilePath = cluster.getNodeConfigPath(0) + .resolve(S3Service.CustomWebIdentityTokenCredentialsProvider.WEB_IDENTITY_TOKEN_FILE_LOCATION); + Files.writeString(webIdentityTokenFilePath, expectedWebIdentityTokenFileContents); + client().performRequest(getPutRepositoryRequest(repositoryName)); + assertVerifySuccess(repositoryName); + + // doesn't matter if the current credentials all become invalid, because they're so close to expiry that the SDK is refreshing them + // (as confirmed by the success of the verify command) + dynamicCredentials.clearValidCredentials(); + assertVerifySuccess(repositoryName); + + // if the refresh fails (incorrect token file contents) we keep on re-using the last good credentials + expectedWebIdentityTokenFileContents = randomAlphanumericOfLength(100); + assertVerifySuccess(repositoryName); + + // if the last good credentials stop working then verification starts to fail + dynamicCredentials.clearValidCredentials(); + assertVerifyFailure(repositoryName); + + // however we keep on trying to refresh and once the token file contents are correct it will succeed + Files.writeString(webIdentityTokenFilePath, expectedWebIdentityTokenFileContents); + assertVerifySuccess(repositoryName); + + } + + private static void assertVerifyFailure(String repositoryName) { + assertThat( + expectThrows( + ResponseException.class, + () -> client().performRequest(new Request("POST", "/_snapshot/" + repositoryName + "/_verify")) + ).getResponse().getStatusLine().getStatusCode(), + equalTo(500) + ); + } + + private static void assertVerifySuccess(String repositoryName) throws IOException { + client().performRequest(new Request("POST", "/_snapshot/" + repositoryName + "/_verify")); + } + + private static @NotNull Request getPutRepositoryRequest(String repositoryName) throws IOException { + return newXContentRequest( + HttpMethod.PUT, + "/_snapshot/" + repositoryName, + (b, p) -> b.field("type", S3Repository.TYPE) + .startObject("settings") + .value(Settings.builder().put("bucket", BUCKET).put("base_path", BASE_PATH).put("client", CLIENT).build()) + .endObject() + ); + } +} diff --git a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsRestIT.java b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsRestIT.java index 767a99dab24c9..f1218ca253763 100644 --- a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsRestIT.java +++ b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsRestIT.java @@ -19,6 +19,7 @@ import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.test.cluster.util.resource.Resource; import org.elasticsearch.test.fixtures.testcontainers.TestContainersThreadFilter; @@ -55,7 +56,8 @@ public class RepositoryS3StsCredentialsRestIT extends AbstractRepositoryS3RestTe private static final AwsStsHttpFixture stsHttpFixture = new AwsStsHttpFixture( dynamicCredentials::addValidCredentials, - WEB_IDENTITY_TOKEN_FILE_CONTENTS + () -> WEB_IDENTITY_TOKEN_FILE_CONTENTS, + TimeValue.timeValueDays(1) ); public static ElasticsearchCluster cluster = ElasticsearchCluster.local() diff --git a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java index 181a97ea01d98..861a19aace701 100644 --- a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java +++ b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java @@ -119,8 +119,6 @@ class S3Service extends AbstractLifecycleComponent { final Settings nodeSettings = clusterService.getSettings(); webIdentityTokenCredentialsProvider = new CustomWebIdentityTokenCredentialsProvider( environment, - System::getenv, - System::getProperty, Clock.systemUTC(), resourceWatcherService ); @@ -448,15 +446,9 @@ static class CustomWebIdentityTokenCredentialsProvider implements AwsCredentials private StsWebIdentityTokenFileCredentialsProvider credentialsProvider; private StsClient securityTokenServiceClient; - CustomWebIdentityTokenCredentialsProvider( - Environment environment, - SystemEnvironment systemEnvironment, - JvmEnvironment jvmEnvironment, - Clock clock, - ResourceWatcherService resourceWatcherService - ) { - // Check whether the original environment variable exists. If it doesn't, the system doesn't support AWS web identity tokens - final var webIdentityTokenFileEnvVar = systemEnvironment.getEnv(AWS_WEB_IDENTITY_TOKEN_FILE.name()); + CustomWebIdentityTokenCredentialsProvider(Environment environment, Clock clock, ResourceWatcherService resourceWatcherService) { + // Check whether the original environment variable exists. If it doesn't, the system doesn't support AWS web identity tokens. + final var webIdentityTokenFileEnvVar = System.getenv(AWS_WEB_IDENTITY_TOKEN_FILE.name()); if (webIdentityTokenFileEnvVar == null) { return; } @@ -487,7 +479,7 @@ static class CustomWebIdentityTokenCredentialsProvider implements AwsCredentials ); } - final var roleArn = systemEnvironment.getEnv(AWS_ROLE_ARN.name()); + final var roleArn = System.getenv(AWS_ROLE_ARN.name()); if (roleArn == null) { LOGGER.warn( """ @@ -499,7 +491,7 @@ static class CustomWebIdentityTokenCredentialsProvider implements AwsCredentials } final var roleSessionName = Objects.requireNonNullElseGet( - systemEnvironment.getEnv(AWS_ROLE_SESSION_NAME.name()), + System.getenv(AWS_ROLE_SESSION_NAME.name()), // Mimic the default behaviour of the AWS SDK in case the session name is not set // See `com.amazonaws.auth.WebIdentityTokenCredentialsProvider#45` () -> "aws-sdk-java-" + clock.millis() @@ -508,7 +500,7 @@ static class CustomWebIdentityTokenCredentialsProvider implements AwsCredentials { final var securityTokenServiceClientBuilder = StsClient.builder(); // allow an endpoint override in tests - final var endpointOverride = jvmEnvironment.getProperty("org.elasticsearch.repositories.s3.stsEndpointOverride", null); + final var endpointOverride = System.getProperty("org.elasticsearch.repositories.s3.stsEndpointOverride", null); if (endpointOverride != null) { securityTokenServiceClientBuilder.endpointOverride(URI.create(endpointOverride)); } @@ -674,14 +666,4 @@ public String toString() { return "ErrorLogging[" + delegate + "]"; } } - - @FunctionalInterface - interface SystemEnvironment { - String getEnv(String name); - } - - @FunctionalInterface - interface JvmEnvironment { - String getProperty(String key, String defaultValue); - } } diff --git a/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/DynamicAwsCredentials.java b/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/DynamicAwsCredentials.java index 04c50607681a6..44db821ff1633 100644 --- a/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/DynamicAwsCredentials.java +++ b/test/fixtures/aws-fixture-utils/src/main/java/fixture/aws/DynamicAwsCredentials.java @@ -79,4 +79,8 @@ public void addValidCredentials(String accessKey, String sessionToken) { t -> ConcurrentCollections.newConcurrentSet() ).add(Objects.requireNonNull(accessKey, "accessKey")); } + + public void clearValidCredentials() { + validCredentialsMap.clear(); + } } diff --git a/test/fixtures/aws-sts-fixture/src/main/java/fixture/aws/sts/AwsStsHttpFixture.java b/test/fixtures/aws-sts-fixture/src/main/java/fixture/aws/sts/AwsStsHttpFixture.java index 13ba7eaf8ba67..ee6857e04d9fb 100644 --- a/test/fixtures/aws-sts-fixture/src/main/java/fixture/aws/sts/AwsStsHttpFixture.java +++ b/test/fixtures/aws-sts-fixture/src/main/java/fixture/aws/sts/AwsStsHttpFixture.java @@ -11,6 +11,7 @@ import com.sun.net.httpserver.HttpHandler; import com.sun.net.httpserver.HttpServer; +import org.elasticsearch.core.TimeValue; import org.junit.rules.ExternalResource; import java.net.InetAddress; @@ -18,21 +19,28 @@ import java.net.UnknownHostException; import java.util.Objects; import java.util.function.BiConsumer; +import java.util.function.Supplier; public class AwsStsHttpFixture extends ExternalResource { private HttpServer server; private final BiConsumer newCredentialsConsumer; - private final String webIdentityToken; + private final Supplier webIdentityTokenSupplier; + private final TimeValue tokenValidDuration; - public AwsStsHttpFixture(BiConsumer newCredentialsConsumer, String webIdentityToken) { + public AwsStsHttpFixture( + BiConsumer newCredentialsConsumer, + Supplier webIdentityTokenSupplier, + TimeValue tokenValidDuration + ) { this.newCredentialsConsumer = Objects.requireNonNull(newCredentialsConsumer); - this.webIdentityToken = Objects.requireNonNull(webIdentityToken); + this.webIdentityTokenSupplier = Objects.requireNonNull(webIdentityTokenSupplier); + this.tokenValidDuration = tokenValidDuration; } protected HttpHandler createHandler() { - return new AwsStsHttpHandler(newCredentialsConsumer, webIdentityToken); + return new AwsStsHttpHandler(newCredentialsConsumer, webIdentityTokenSupplier, tokenValidDuration); } public String getAddress() { diff --git a/test/fixtures/aws-sts-fixture/src/main/java/fixture/aws/sts/AwsStsHttpHandler.java b/test/fixtures/aws-sts-fixture/src/main/java/fixture/aws/sts/AwsStsHttpHandler.java index b99986793141d..52d7ff76b9057 100644 --- a/test/fixtures/aws-sts-fixture/src/main/java/fixture/aws/sts/AwsStsHttpHandler.java +++ b/test/fixtures/aws-sts-fixture/src/main/java/fixture/aws/sts/AwsStsHttpHandler.java @@ -13,6 +13,7 @@ import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.core.SuppressForbidden; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.rest.RestStatus; import java.io.IOException; @@ -26,6 +27,7 @@ import java.util.Map; import java.util.Objects; import java.util.function.BiConsumer; +import java.util.function.Supplier; import java.util.stream.Collectors; import static org.elasticsearch.test.ESTestCase.randomIdentifier; @@ -41,11 +43,18 @@ public class AwsStsHttpHandler implements HttpHandler { public static final String ROLE_NAME = "sts-fixture-test"; private final BiConsumer newCredentialsConsumer; - private final String webIdentityToken; + private final Supplier webIdentityTokenSupplier; + private final TimeValue tokenValidDuration; - public AwsStsHttpHandler(BiConsumer newCredentialsConsumer, String webIdentityToken) { + public AwsStsHttpHandler( + BiConsumer newCredentialsConsumer, + Supplier webIdentityTokenSupplier, + TimeValue tokenValidDuration + ) { this.newCredentialsConsumer = Objects.requireNonNull(newCredentialsConsumer); - this.webIdentityToken = Objects.requireNonNull(webIdentityToken); + this.webIdentityTokenSupplier = Objects.requireNonNull(webIdentityTokenSupplier); + this.tokenValidDuration = Objects.requireNonNull(tokenValidDuration); + assert tokenValidDuration.seconds() > 0 : tokenValidDuration.getStringRep(); } @Override @@ -68,7 +77,7 @@ public void handle(final HttpExchange exchange) throws IOException { return; } if (ROLE_NAME.equals(params.get("RoleSessionName")) == false - || webIdentityToken.equals(params.get("WebIdentityToken")) == false + || Objects.requireNonNull(webIdentityTokenSupplier.get()).equals(params.get("WebIdentityToken")) == false || ROLE_ARN.equals(params.get("RoleArn")) == false) { exchange.sendResponseHeaders(RestStatus.UNAUTHORIZED.getStatus(), 0); exchange.close(); @@ -105,7 +114,9 @@ public void handle(final HttpExchange exchange) throws IOException { ROLE_NAME, sessionToken, randomSecretKey(), - ZonedDateTime.now(Clock.systemUTC()).plusDays(1L).format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ssZ")), + ZonedDateTime.now(Clock.systemUTC()) + .plusSeconds(tokenValidDuration.seconds()) + .format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ssZ")), accessKey ).getBytes(StandardCharsets.UTF_8); exchange.getResponseHeaders().add("Content-Type", "text/xml; charset=UTF-8"); diff --git a/test/fixtures/aws-sts-fixture/src/test/java/fixture/aws/sts/AwsStsHttpHandlerTests.java b/test/fixtures/aws-sts-fixture/src/test/java/fixture/aws/sts/AwsStsHttpHandlerTests.java index b9193f31dbd18..7b31441fe97ba 100644 --- a/test/fixtures/aws-sts-fixture/src/test/java/fixture/aws/sts/AwsStsHttpHandlerTests.java +++ b/test/fixtures/aws-sts-fixture/src/test/java/fixture/aws/sts/AwsStsHttpHandlerTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESTestCase; @@ -29,6 +30,8 @@ import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; import static org.hamcrest.Matchers.aMapWithSize; @@ -40,7 +43,7 @@ public void testGenerateCredentials() { final Map generatedCredentials = new HashMap<>(); final var webIdentityToken = randomUnicodeOfLength(10); - final var handler = new AwsStsHttpHandler(generatedCredentials::put, webIdentityToken); + final var handler = new AwsStsHttpHandler(generatedCredentials::put, () -> webIdentityToken, randomValidDuration()); final var response = handleRequest( handler, @@ -67,14 +70,14 @@ public void testGenerateCredentials() { } public void testInvalidAction() { - final var handler = new AwsStsHttpHandler((key, token) -> fail(), randomUnicodeOfLength(10)); + final var handler = new AwsStsHttpHandler((key, token) -> fail(), () -> randomUnicodeOfLength(10), randomValidDuration()); final var response = handleRequest(handler, Map.of("Action", "Unsupported")); assertEquals(RestStatus.BAD_REQUEST, response.status()); } public void testInvalidRole() { final var webIdentityToken = randomUnicodeOfLength(10); - final var handler = new AwsStsHttpHandler((key, token) -> fail(), webIdentityToken); + final var handler = new AwsStsHttpHandler((key, token) -> fail(), () -> webIdentityToken, randomValidDuration()); final var response = handleRequest( handler, Map.of( @@ -92,8 +95,15 @@ public void testInvalidRole() { } public void testInvalidToken() { - final var webIdentityToken = randomUnicodeOfLength(10); - final var handler = new AwsStsHttpHandler((key, token) -> fail(), webIdentityToken); + final var webIdentityTokenRef = new AtomicReference<>(randomUnicodeOfLength(10)); + final String incorrectToken; + final var handler = new AwsStsHttpHandler((key, token) -> fail(), webIdentityTokenRef::get, randomValidDuration()); + if (randomBoolean()) { + incorrectToken = webIdentityTokenRef.get(); + webIdentityTokenRef.set(randomValueOtherThan(incorrectToken, () -> randomUnicodeOfLength(10))); + } else { + incorrectToken = randomValueOtherThan(webIdentityTokenRef.get(), () -> randomUnicodeOfLength(10)); + } final var response = handleRequest( handler, Map.of( @@ -104,7 +114,7 @@ public void testInvalidToken() { "RoleArn", AwsStsHttpHandler.ROLE_ARN, "WebIdentityToken", - randomValueOtherThan(webIdentityToken, () -> randomUnicodeOfLength(10)) + incorrectToken ) ); assertEquals(RestStatus.UNAUTHORIZED, response.status()); @@ -112,7 +122,7 @@ public void testInvalidToken() { public void testInvalidARN() { final var webIdentityToken = randomUnicodeOfLength(10); - final var handler = new AwsStsHttpHandler((key, token) -> fail(), webIdentityToken); + final var handler = new AwsStsHttpHandler((key, token) -> fail(), () -> webIdentityToken, randomValidDuration()); final var response = handleRequest( handler, Map.of( @@ -265,4 +275,7 @@ public HttpPrincipal getPrincipal() { } } + private static TimeValue randomValidDuration() { + return randomTimeValue(1, 100000, TimeUnit.SECONDS); + } } diff --git a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/DefaultLocalClusterHandle.java b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/DefaultLocalClusterHandle.java index 6c40174541de7..f85f74aa4cc26 100644 --- a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/DefaultLocalClusterHandle.java +++ b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/DefaultLocalClusterHandle.java @@ -210,6 +210,10 @@ public InputStream getNodeLog(int index, LogType logType) { return nodes.get(index).getLog(logType); } + public Path getNodeConfigPath(int index) { + return nodes.get(index).getConfigDir(); + } + @Override public void updateStoredSecureSettings() { execute(() -> nodes.parallelStream().forEach(Node::updateStoredSecureSettings)); diff --git a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/DefaultLocalElasticsearchCluster.java b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/DefaultLocalElasticsearchCluster.java index fca525a2b4d04..928b2a13570b6 100644 --- a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/DefaultLocalElasticsearchCluster.java +++ b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/DefaultLocalElasticsearchCluster.java @@ -18,6 +18,7 @@ import java.io.InputStream; import java.lang.annotation.Annotation; import java.lang.reflect.InvocationTargetException; +import java.nio.file.Path; import java.util.Arrays; import java.util.List; import java.util.function.Supplier; @@ -169,6 +170,12 @@ public InputStream getNodeLog(int index, LogType logType) { return handle.getNodeLog(index, logType); } + @Override + public Path getNodeConfigPath(int index) { + checkHandle(); + return handle.getNodeConfigPath(index); + } + @Override public void updateStoredSecureSettings() { checkHandle(); diff --git a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/LocalClusterHandle.java b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/LocalClusterHandle.java index 7c55da3a4e1b9..4543918d9e6df 100644 --- a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/LocalClusterHandle.java +++ b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/LocalClusterHandle.java @@ -15,6 +15,7 @@ import org.elasticsearch.test.cluster.util.Version; import java.io.InputStream; +import java.nio.file.Path; import java.util.List; public interface LocalClusterHandle extends ClusterHandle { @@ -108,6 +109,11 @@ public interface LocalClusterHandle extends ClusterHandle { */ InputStream getNodeLog(int index, LogType logType); + /** + * Returns the {@link Path} to the given node's config directory. + */ + Path getNodeConfigPath(int index); + /** * Writes secure settings to the relevant secure config file on each node. Use this method if you are dynamically updating secure * settings via a {@link MutableSettingsProvider} and need the update to be written to file, without a cluster restart. From 1208a93843a063b22d6553443c05a48ecd92717a Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 8 Jan 2026 13:34:53 +0000 Subject: [PATCH 2/2] Expand comments --- ...epositoryS3StsCredentialsReloadRestIT.java | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsReloadRestIT.java b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsReloadRestIT.java index 191ebfa5362c5..723bb85198a43 100644 --- a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsReloadRestIT.java +++ b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3StsCredentialsReloadRestIT.java @@ -100,7 +100,8 @@ protected String getTestRestCluster() { public void testReloadCredentials() throws IOException { final var repositoryName = "repo-" + randomIdentifier(); - // token file starts out invalid, so we cannot create the repository + // Token file starts out invalid, causing the STS fixture to return 403s rather than the credentials the SDK asks for, so we cannot + // create the repository: assertThat( expectThrows(ResponseException.class, () -> client().performRequest(getPutRepositoryRequest(repositoryName))).getResponse() .getStatusLine() @@ -108,27 +109,31 @@ public void testReloadCredentials() throws IOException { equalTo(500) ); - // filling in the expected file contents yields success immediately, so we must be re-reading this file correctly + // However the S3 SDK just keeps on attempting to get credentials, including re-reading the token file, which we can confirm by + // replacing the file with valid contents so that the STS fixture starts to return credentials: final var webIdentityTokenFilePath = cluster.getNodeConfigPath(0) .resolve(S3Service.CustomWebIdentityTokenCredentialsProvider.WEB_IDENTITY_TOKEN_FILE_LOCATION); Files.writeString(webIdentityTokenFilePath, expectedWebIdentityTokenFileContents); client().performRequest(getPutRepositoryRequest(repositoryName)); assertVerifySuccess(repositoryName); - // doesn't matter if the current credentials all become invalid, because they're so close to expiry that the SDK is refreshing them - // (as confirmed by the success of the verify command) + // Moreover the STS fixture is configured to return credentials which have almost expired, so the S3 SDK uses these credentials but + // also attempts to refresh them. We can confirm this by invalidating all the credentials returned so far and observing that these + // API calls still succeed. dynamicCredentials.clearValidCredentials(); assertVerifySuccess(repositoryName); - // if the refresh fails (incorrect token file contents) we keep on re-using the last good credentials + // Also, if we reconfigure the STS fixture to expect a different token in the token file then it'll start returning 403s to these + // token-refresh requests, and in this case the SDK continues to use the last-known-good credentials. expectedWebIdentityTokenFileContents = randomAlphanumericOfLength(100); assertVerifySuccess(repositoryName); - // if the last good credentials stop working then verification starts to fail + // However if we now invalidate the last-known-good credentials while the STS fixture is returning 403s then API requests must fail. dynamicCredentials.clearValidCredentials(); assertVerifyFailure(repositoryName); - // however we keep on trying to refresh and once the token file contents are correct it will succeed + // But the SDK keeps on trying, which we can confirm by restore the STS fixture to health again and observe that this is enough to + // allow API requests to succeed. Files.writeString(webIdentityTokenFilePath, expectedWebIdentityTokenFileContents); assertVerifySuccess(repositoryName);