- 
                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
  
    Improve randomIdentifier usage in AWS tests
  
  #125775
              Conversation
Adds prefixes to various randomly-generated values to make it easier to pin down where they're coming from in debugging sessions. Also forces the STS expiry time to be rendered in UTC.
| Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) | 
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.
Left some suggestions, lgtm 👍
| } | ||
|  | ||
| private String generateAndGet() { | ||
| final var newRegion = "test-region-" + ESTestCase.randomIdentifier(); | 
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.
| return; | ||
| } | ||
| final var accessKey = randomIdentifier(); | ||
| // ATIA for a test key, similar to AKIA and ASIA used in real AWS credentials | 
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.
So ATIA is not typical for AWS credentials, so this is more visible? Add something saying ATIA deliberately differs for identifiability?
Or just prefix 'test' or 'access-key-sts' -- ACCESS_KEY_STS_<>. There are no requirements for the access key, right? We just want to be able to read what the random string represents, IIUC.
| 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 comment
The reason will be displayed to describe this comment to others. Learn more.
nice
| Thanks yeah applied some changes. Doesn't really matter what the prefixes are as long as you can search for them. | 
Adds prefixes to various randomly-generated values to make it easier to pin down where they're coming from in debugging sessions. Also forces the STS expiry time to be rendered in UTC.
| 💚 Backport successful
 | 
Adds prefixes to various randomly-generated values to make it easier to pin down where they're coming from in debugging sessions. Also forces the STS expiry time to be rendered in UTC.
Adds prefixes to various randomly-generated values to make it easier to pin down where they're coming from in debugging sessions. Also forces the STS expiry time to be rendered in UTC. Co-authored-by: Rene Groeschke <[email protected]>
Adds prefixes to various randomly-generated values to make it easier to
pin down where they're coming from in debugging sessions. Also forces
the STS expiry time to be rendered in UTC.