Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Test] Increase test secret key length #117675

Merged
merged 9 commits into from
Nov 29, 2024
Merged

[Test] Increase test secret key length #117675

merged 9 commits into from
Nov 29, 2024

Conversation

ywangd
Copy link
Member

@ywangd ywangd commented Nov 28, 2024 •
edited
Loading

Running with FIPS approved mode requires secret keys to be at least 114 bits long.

Relates: #117324
Resolves: #117596
Resolves: #117709
Resolves: #117710
Resolves: #117711
Resolves: #117712

@ywangd
[Test] Increase test secret key length
b6fa2c0 
Running with FIPS approved mode requires secret keys to be at least
114 bits long.

Resolves: elastic#117596
@ywangd ywangd added >test Issues or PRs that are addressing/adding tests :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v9.0.0 v8.18.0 labels Nov 28, 2024
@ywangd ywangd requested a review from DaveCTurner November 28, 2024 03:03
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Coordination Meta label for Distributed Coordination team label Nov 28, 2024
@elasticsearchmachine
Copy link
Collaborator

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

ywangd
ywangd commented Nov 28, 2024
View reviewed changes
test/fixtures/aws-sts-fixture/src/main/java/fixture/aws/sts/AwsStsHttpHandler.java Outdated
Comment on lines 105 to 106
randomIdentifier(),
randomAlphaOfLengthBetween(14, 20).toLowerCase(Locale.ROOT),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not really needed to fix the test failure. But I think might as well.

@ywangd ywangd added the auto-backport Automatically create backport pull requests when merged label Nov 28, 2024
DaveCTurner
DaveCTurner reviewed Nov 28, 2024
View reviewed changes
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha nice catch, thanks for working this one out.

Could you do the same to RepositoryS3RestReloadCredentialsIT and remove the assumeFalse("doesn't work in a FIPS JVM, but that's ok", inFipsJvm()); line there too?

@DaveCTurner
Copy link
Contributor

Actually could we also extract this to a randomSecretKey method in ESTestCase along with an explanation for why it exists?

This was referenced Nov 28, 2024
Closed
Closed
Closed
Closed
@ywangd
Copy link
Member Author

ywangd commented Nov 28, 2024

Added the suggested randomSecretKey method.

RepositoryS3RestReloadCredentialsIT

This requires a bit extra fix. It's working now 0809994

@ywangd ywangd requested a review from DaveCTurner November 28, 2024 22:31
DaveCTurner
DaveCTurner approved these changes Nov 28, 2024
View reviewed changes
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks Yang - one optional suggestion

...javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3RestReloadCredentialsIT.java Outdated Show resolved Hide resolved
ywangd and others added 3 commits November 29, 2024 09:53
@ywangd @DaveCTurner
Update modules/repository-s3/src/javaRestTest/java/org/elasticsearch/…
cc3a798 
…repositories/s3/RepositoryS3RestReloadCredentialsIT.java

Co-authored-by: David Turner <david.turner@elastic.co>
@ywangd
fix import
20fa1fd
@ywangd
Merge remote-tracking branch 'origin/main' into es-117596-fix
15e0fbc
@ywangd ywangd added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Nov 28, 2024
@ywangd
Copy link
Member Author

ywangd commented Nov 29, 2024

@elasticmachine update branch

@elasticsearchmachine elasticsearchmachine merged commit 24bc505 into elastic:main Nov 29, 2024
16 checks passed
@ywangd ywangd deleted the es-117596-fix branch November 29, 2024 03:08
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
❌ 8.x Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 117675

@ywangd ywangd mentioned this pull request Nov 29, 2024
Merged
@ywangd
Copy link
Member Author

ywangd commented Nov 29, 2024

💚 All backports created successfully

Status Branch Result
✅ 8.x

Questions ?

Please refer to the Backport tool documentation

ywangd added a commit to ywangd/elasticsearch that referenced this pull request Nov 29, 2024
@ywangd
[Test] Increase test secret key length (elastic#117675)
1db39a7 
Running with FIPS approved mode requires secret keys to be at least 114
bits long.

Relates: elastic#117324 Resolves: elastic#117596 Resolves: elastic#117709 Resolves: elastic#117710
Resolves: elastic#117711 Resolves: elastic#117712
(cherry picked from commit 24bc505)

# Conflicts:
#	modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3RestReloadCredentialsIT.java
#	muted-tests.yml
#	test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpFixtureWithSTS.java
elasticsearchmachine pushed a commit that referenced this pull request Nov 29, 2024
@ywangd
[Test] Increase test secret key length (#117675) (#117738)
f52b8c6 
Running with FIPS approved mode requires secret keys to be at least 114
bits long.

Relates: #117324 Resolves: #117596 Resolves: #117709 Resolves: #117710
Resolves: #117711 Resolves: #117712
(cherry picked from commit 24bc505)

# Conflicts:
#	modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3RestReloadCredentialsIT.java
#	muted-tests.yml
#	test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpFixtureWithSTS.java
@DaveCTurner DaveCTurner mentioned this pull request Nov 29, 2024
Merged
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Dec 3, 2024
@ywangd
[8.x] [Test] Increase test secret key length for AwsStsHttpHandler
dc61160 
The initial backport elastic#117738 did not contain the change for
AwsStsHttpHandler from elastic#117675. This PR adds it.

Backport of elastic#117675
This was referenced Dec 3, 2024
Merged
Closed
elasticsearchmachine pushed a commit that referenced this pull request Dec 5, 2024
@ywangd @elasticmachine
[8.x] [Test] Increase test secret key length for AwsStsHttpHandler (#…
1b185b7 
…117944)

* [8.x] [Test] Increase test secret key length for AwsStsHttpHandler

The initial backport #117738 did not contain the change for
AwsStsHttpHandler from #117675. This PR adds it.

Backport of #117675

* unmute

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DaveCTurner DaveCTurner DaveCTurner approved these changes

Assignees
No one assigned
Labels
auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport pending :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed Coordination Meta label for Distributed Coordination team >test Issues or PRs that are addressing/adding tests v8.18.0 v9.0.0
Projects
None yet
Milestone
No milestone
4 participants
@ywangd @elasticsearchmachine @DaveCTurner @elasticmachine