- 
                Notifications
    You must be signed in to change notification settings 
- Fork 25.6k
          Improve randomIdentifier usage in AWS tests
          #125775
        
          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 1 commit
0d518cc
              a13bb7f
              5a7c058
              a5935b7
              f15349c
              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 | 
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| /* | ||
| * 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 fixture.aws; | ||
|  | ||
| import org.elasticsearch.test.ESTestCase; | ||
|  | ||
| import java.util.Objects; | ||
| import java.util.concurrent.atomic.AtomicReference; | ||
| import java.util.function.Supplier; | ||
|  | ||
| /** | ||
| * Lazy supplier for a region name. We cannot use randomness like {@link ESTestCase#randomIdentifier()} when creating the test fixtures in | ||
| * the first place because this happens in static context, so instead we create one of these and defer the creation of the region name | ||
| * itself until the test actually starts running. | ||
| */ | ||
| public class DynamicRegionSupplier implements Supplier<String> { | ||
| private final AtomicReference<String> generatedRegion = new AtomicReference<>(); | ||
|  | ||
| @Override | ||
| public String get() { | ||
| return Objects.requireNonNullElseGet(generatedRegion.get(), this::generateAndGet); | ||
| } | ||
|  | ||
| private String generateAndGet() { | ||
| final var newRegion = "test-region-" + ESTestCase.randomIdentifier(); | ||
| return Objects.requireNonNullElse(generatedRegion.compareAndExchange(null, newRegion), newRegion); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -18,6 +18,7 @@ | |
| import java.io.IOException; | ||
| import java.net.URLDecoder; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.time.Clock; | ||
| import java.time.ZonedDateTime; | ||
| import java.time.format.DateTimeFormatter; | ||
| import java.util.Arrays; | ||
|  | @@ -73,7 +74,8 @@ public void handle(final HttpExchange exchange) throws IOException { | |
| exchange.close(); | ||
| return; | ||
| } | ||
| final var accessKey = randomIdentifier(); | ||
| // ATIA for a test key, similar to AKIA and ASIA used in real AWS credentials | ||
|          | ||
| final var accessKey = "ATIA_STS_" + randomIdentifier(); | ||
| final var sessionToken = randomIdentifier(); | ||
| newCredentialsConsumer.accept(accessKey, sessionToken); | ||
| final byte[] response = String.format( | ||
|  | @@ -104,7 +106,7 @@ public void handle(final HttpExchange exchange) throws IOException { | |
| ROLE_NAME, | ||
| sessionToken, | ||
| randomSecretKey(), | ||
| ZonedDateTime.now().plusDays(1L).format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ssZ")), | ||
| ZonedDateTime.now(Clock.systemUTC()).plusDays(1L).format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ssZ")), | ||
| 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. nice | ||
| accessKey | ||
| ).getBytes(StandardCharsets.UTF_8); | ||
| exchange.getResponseHeaders().add("Content-Type", "text/xml; charset=UTF-8"); | ||
|  | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could prefix something additional like "random-test-region-" or "dynamic-test-region-", if you want to make this a little more identifiable/unique that it comes from this class -- as opposed to a random test file generating its own test region where we'll likely to use the same prefix.