Skip to content

enhance:[2.6] force CRC32C when OpenSSL FIPS is enabled#50360

Open
jiaqizho wants to merge 1 commit into
milvus-io:2.6from
jiaqizho:2.6-cp-fips-force-use-crc32c
Open

enhance:[2.6] force CRC32C when OpenSSL FIPS is enabled#50360
jiaqizho wants to merge 1 commit into
milvus-io:2.6from
jiaqizho:2.6-cp-fips-force-use-crc32c

Conversation

@jiaqizho

@jiaqizho jiaqizho commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

issue: #48359
pr: #50239

This change makes Milvus automatically use CRC32C for S3 PutObject requests when OpenSSL FIPS mode is active. In FIPS mode, checksum choices are more constrained, so relying on the configured MinIO checksum behavior can lead to requests using an algorithm that is not acceptable in that runtime.

The implementation moves OpenSSL FIPS enablement into the shared pkg/util/fips package and makes it return whether FIPS was actually enabled. That lets startup code keep enabling OpenSSL FIPS as before, while paramtable can also reuse the same state during config initialization.

When FIPS is successfully enabled and minio.ssl.useCRC32C is still false, Milvus now logs a warning and temporarily overrides the runtime value to true. This keeps the user-facing config unchanged, but makes the effective runtime behavior compatible with FIPS requirements.

This change makes Milvus automatically use CRC32C for S3 PutObject requests when OpenSSL FIPS mode is active. In FIPS mode, checksum choices are more constrained, so relying on the configured MinIO checksum behavior can lead to requests using an algorithm that is not acceptable in that runtime.

The implementation moves OpenSSL FIPS enablement into the shared `pkg/util/fips` package and makes it return whether FIPS was actually enabled. That lets startup code keep enabling OpenSSL FIPS as before, while paramtable can also reuse the same state during config initialization.

When FIPS is successfully enabled and `minio.ssl.useCRC32C` is still false, Milvus now logs a warning and temporarily overrides the runtime value to true. This keeps the user-facing config unchanged, but makes the effective runtime behavior compatible with FIPS requirements.

Signed-off-by: jiaqizho <jiaqi.zhou@zilliz.com>
@sre-ci-robot sre-ci-robot added the size/L Denotes a PR that changes 100-499 lines. label Jun 8, 2026
@sre-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jiaqizho
To complete the pull request process, please assign xiaofan-luan after the PR has been reviewed.
You can assign the PR to them by writing /assign @xiaofan-luan in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot requested review from godchen0212 and sunby June 8, 2026 02:30
@mergify

mergify Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

@jiaqizho Please associate the related pr of master to the body of your Pull Request. (eg. "pr: #")

@mergify mergify Bot added dco-passed DCO check passed. do-not-merge/missing-related-pr kind/enhancement Issues or changes related to enhancement labels Jun 8, 2026
@sre-ci-robot

Copy link
Copy Markdown
Contributor

[ci-v2-notice]
Notice: New ci-v2 system is enabled for this PR.

To rerun ci-v2 checks, comment with:

  • /ci-rerun-code-check // for ci-v2/code-check
  • /ci-rerun-code-check-macos // for Code Checker MacOS (GitHub Actions)
  • /ci-rerun-build // for ci-v2/build
  • /ci-rerun-build-all // for ci-v2/build-all (multi-arch builds)
  • /ci-rerun-buildenv // for ci-v2/build-env (build milvus-env builder images)
  • /ci-rerun-ut-integration // for ci-v2/ut-integration, will rerun ci-v2/build
  • /ci-rerun-ut-go // for ci-v2/ut-go, will rerun ci-v2/build
  • /ci-rerun-ut-cpp // for ci-v2/ut-cpp
  • /ci-rerun-ut // for all ci-v2/ut-integration, ci-v2/ut-go, ci-v2/ut-cpp, will rerun ci-v2/build
  • /ci-rerun-e2e-default // for ci-v2/e2e-default
  • /ci-rerun-e2e-amd // for ci-v2/e2e-amd (e2e pool dispatcher)
  • /ci-rerun-build-ut-cov // for ci-v2/build-ut-cov (build + unit tests in one pipeline)
  • /ci-rerun-gosdk // for ci-v2/go-sdk (Go SDK E2E tests, ARM)

If you have any questions or requests, please contact @zhikunyao.

@mergify

mergify Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

@jiaqizho Please associate the related issue to the body of your Pull Request. (eg. "issue: #")

@sre-ci-robot sre-ci-robot added do-not-merge/need-merge-master-first any pr merge to release branch need to merge master first do-not-merge/need-milestone generate by v2-label-manager labels Jun 8, 2026
@sre-ci-robot

Copy link
Copy Markdown
Contributor

[INFO] PR Label Summary by Default
[WARNING] No dependent PR reference found

  • Target branch '2.6' requires a PR merged to master first
  • Please add reference in format 'pr: #number'

[WARNING] Milestone not set

You can set milestone by commenting:
/set-milestone
Example:
/set-milestone 2.5.0

Use /refresh-label to update related check and label manually

@codecov

codecov Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.73%. Comparing base (cb08db0) to head (b8c9733).
⚠️ Report is 941 commits behind head on 2.6.

Files with missing lines Patch % Lines
pkg/util/paramtable/component_param.go 0.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              2.6   #50360      +/-   ##
==========================================
+ Coverage   76.99%   77.73%   +0.73%     
==========================================
  Files        1700     2000     +300     
  Lines      262533   328898   +66365     
==========================================
+ Hits       202142   255670   +53528     
- Misses      53550    65385   +11835     
- Partials     6841     7843    +1002     
Components Coverage Δ
Client 79.48% <74.73%> (+1.34%) ⬆️
Core 84.58% <45.83%> (+2.37%) ⬆️
Go 75.80% <55.09%> (+0.41%) ⬆️
Files with missing lines Coverage Δ
pkg/util/fips/openssl_disabled.go 100.00% <100.00%> (ø)
pkg/util/paramtable/component_param.go 97.30% <0.00%> (-0.45%) ⬇️

... and 1411 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jiaqizho

jiaqizho commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

/ci-rerun-e2e-default

@jiaqizho

jiaqizho commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

/ci-rerun-ut-integration

@sre-ci-robot sre-ci-robot added the low-code-coverage add test-label from zhikun, diff coverage > 80% label Jun 8, 2026
@jiaqizho

jiaqizho commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

/ci-rerun-ut-cpp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-passed DCO check passed. do-not-merge/need-merge-master-first any pr merge to release branch need to merge master first do-not-merge/need-milestone generate by v2-label-manager kind/enhancement Issues or changes related to enhancement low-code-coverage add test-label from zhikun, diff coverage > 80% size/L Denotes a PR that changes 100-499 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants