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.

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

feat: add fips option for aws-lc #20287

Merged
merged 10 commits into from
Feb 20, 2025
Merged

feat: add fips option for aws-lc #20287

merged 10 commits into from
Feb 20, 2025

Conversation

yuhao-su
Copy link
Contributor

@yuhao-su yuhao-su commented Jan 24, 2025

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

add fips option for aws-lc

Manually tested in ci and locally.

Need to add a pipeline to build fips images

Checklist

  • I have written necessary rustdoc comments.
  • I have added necessary unit tests and integration tests.
  • I have added test labels as necessary.
  • I have added fuzzing tests or opened an issue to track them.
  • My PR contains breaking changes.
  • My PR changes performance-critical code, so I will run (micro) benchmarks and present the results.
  • My PR contains critical fixes that are necessary to be merged into the latest release.

Documentation

  • My PR needs documentation updates.
Release note

@yuhao-su yuhao-su changed the title fips feat: add fips option for aws-lc Jan 24, 2025
@yuhao-su yuhao-su marked this pull request as ready for review January 24, 2025 00:40
@yuhao-su yuhao-su requested a review from a team as a code owner January 24, 2025 00:40
@yuhao-su yuhao-su requested a review from hzxa21 January 24, 2025 00:40
Copy link

gru-agent bot commented Jan 24, 2025

This pull request has been modified. If you want me to regenerate unit test for any of the files related, please find the file in "Files Changed" tab and add a comment @gru-agent. (The github "Comment on this file" feature is in the upper right corner of each file in "Files Changed" tab.)

@yuhao-su yuhao-su requested review from lmatz and xxchan January 24, 2025 04:10
Comment on lines +57 to +60
# FIPS
aws-lc-rs ={ version = "1.6", optional = true, default-features = false, features = [
"fips",
] }
Copy link
Member

Choose a reason for hiding this comment

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

Why not enable it by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have a strong reason against doing so. It's simply because I didn't see any other product enabling this option by default.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for trying enable it by default.It does not bring us any additional cost I think.

@yuhao-su yuhao-su requested review from BugenZhao and xxchan January 24, 2025 09:57
@lmatz lmatz requested a review from cyliu0 February 19, 2025 01:28
@cyliu0
Copy link
Collaborator

cyliu0 commented Feb 19, 2025

Need to add a pipeline to build fips images

Which images need to enable this? nightly? stable version?

@yuhao-su
Copy link
Contributor Author

Need to add a pipeline to build fips images

Which images need to enable this? nightly? stable version?

For now I guess there is no need to release with this option. But can we just build the image with the option and run a e2e test in our release pipeline? cc @lmatz

@tabVersion tabVersion self-requested a review February 19, 2025 04:40
@lmatz
Copy link
Contributor

lmatz commented Feb 19, 2025

Need to add a pipeline to build fips images

Which images need to enable this? nightly? stable version?

For now I guess there is no need to release with this option. But can we just build the image with the option and run a e2e test in our release pipeline? cc @lmatz

One potential customer is requesting fips compliance and we may need to deliver the image, not 100% sure but could be soon

but we can add the test first

And if nothing serious happens after some testing period, I suggest we enable it by default in the future.

@yuhao-su yuhao-su enabled auto-merge February 20, 2025 19:31
@yuhao-su yuhao-su added this pull request to the merge queue Feb 20, 2025
Merged via the queue into main with commit cee2d2d Feb 20, 2025
29 of 30 checks passed
@yuhao-su yuhao-su deleted the yuhao/aws-lc-fips branch February 20, 2025 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants