Skip to content

Fix KMS configuration synchronization across cluster nodes #855

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
Merged
houseme merged 11 commits into from
Nov 15, 2025
Merged

Fix KMS configuration synchronization across cluster nodes #855

houseme merged 11 commits into from
Nov 15, 2025

Conversation

Copy link
Contributor

Copilot AI commented Nov 14, 2025 •
edited
Loading

Type of Change

  • Bug Fix
  • New Feature
  • Documentation
  • Performance Improvement
  • Test/CI
  • Refactor
  • Other:

Related Issues

Fixes #841

Summary of Changes

This PR fixes a critical bug where KMS configuration was not synchronized across multiple servers in a cluster, causing the configuration to alternate between old and new values on browser refresh in multi-node setups behind nginx load balancers.

Root Cause:

  • KMS configuration was stored only in-memory in each server instance
  • No persistence to cluster metadata storage (.rustfs.sys/config/)
  • Each node in a 3-node setup maintained isolated configuration
  • Browser refresh hits different servers with different configurations

Solution:
Added cluster-wide configuration persistence using the existing metadata storage system to ensure all nodes share the same KMS configuration state.

Main Changes

  1. Added persistence functions in rustfs/src/admin/handlers/kms_dynamic.rs:

    • save_kms_config(): Serializes and saves KMS config to .rustfs.sys/config/kms_config.json
    • load_kms_config(): Loads and deserializes KMS config from cluster storage
    • Uses existing save_config and read_config utilities from ecstore

  2. Updated KMS configuration handlers:

    • ConfigureKmsHandler: Now persists configuration after successful in-memory configuration
    • ReconfigureKmsHandler: Now persists configuration after successful reconfiguration
    • Both handlers provide appropriate error messages if persistence fails

  3. Updated server initialization in rustfs/src/main.rs:

    • init_kms_system() now attempts to load persisted KMS configuration on startup
    • If command-line KMS is not enabled, checks for persisted config from previous sessions
    • If persisted config is found, automatically configures and starts the KMS service
    • Ensures all nodes in a cluster start with the same configuration

  4. Fixed dependabot configuration:

    • Merged overlapping cargo ecosystem configurations into a single entry
    • Removed duplicate directory and target-branch combination
    • Maintained s3s dependency grouping with weekly schedule

Checklist

  • I have read and followed the CONTRIBUTING.md guidelines
  • Passed make pre-commit (cargo fmt, cargo check, cargo clippy)
  • Added/updated necessary tests
  • Documentation updated (if needed)
  • CI/CD passed (if applicable)

Impact

  • Breaking change (compatibility)
  • Requires doc/config/deployment update
  • Other impact: Fixes cluster configuration synchronization

Testing:

  • ✅ All KMS unit tests passed (22 passed, 1 ignored)
  • ✅ cargo fmt --all applied successfully
  • ✅ cargo check passes without errors
  • ✅ cargo clippy --all-targets --all-features -- -D warnings passes without warnings
  • ✅ Dependabot YAML validation passes

Additional Notes

Benefits:

  • All nodes in a cluster now share the same KMS configuration
  • Configuration persists across server restarts
  • No more alternating configuration states when load balancer switches between nodes
  • Fixes the reported issue where switching from AppRole to Token authentication would alternate between states on refresh

The fix is minimal and surgical, adding only the necessary persistence layer without changing the existing KMS functionality or API contracts.

Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md) and sign the CLA if this is your first contribution.

Original prompt

This section details on the original issue you should resolve

<issue_title>KMS Configuration changes on every browser refresh</issue_title>
<issue_description>Describe the bug
Hi!

When I was testing and setting up SSE-KMS, I first setup using an application role. Upon saving the configuration, it said application role was not supported at this time and use a token instead.

I configured it to use a token. If I refresh the status it rotates between "KMS is running and healthy" and "Failed to create KMS backend: Backend error: AppRole authentication not yet implemented. Please use token authentication."

It's like there is somehow some residual configuration? I dont know how to delete and start over with KMS configuration. Trying to create the key results in the "failed to save key".

Ubuntu 24.04 latest
3 node setup
nginx reverse proxy
rustfs 1.0.0-alpha.67
Hashicorp Vault 1.20.1

To Reproduce
Steps to reproduce the behavior:
These steps assume you have access to a working Hashicorp Vault Instance with proper permissions etc.

  1. Setup KMS configuration to use app role
  2. Save configuration (get error about not supported)
  3. Edit Configuration again and switch to a token. Provide valid token
  4. Save configuration again.
  5. Status saying running and healthy.
  6. Refresh "KMS Status Overview"
  7. It says ERROR and it trying to use the previous configuration

Expected behavior
I would expect being able to modify the SSE-KMS configuration (switch from app role to token) to save and persist... across refreshes/browser reloads. Or some way to delete and recreate?

Screenshots

Image Image

Desktop (please complete the following information):

  • MacOS 26.1
  • Mozilla 144.0.2
    </issue_description>

<agent_instructions>Analyze the project code and analyze in depth what are the problems with the current issue? And how and what caused it, and how to fix the problem in the future, and how to improve it more gracefully and efficiently.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@loverustfs Okay, we'll give that feedback to our KMS contributors. @guojidan @loverustfs It appears that the KMS configuration was not synchronized across multiple servers.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI requested a review from houseme November 14, 2025 13:00
@github-actions
Copy link

github-actions bot commented Nov 14, 2025 •
edited
Loading

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails
cargo/fs-err 3.2.0 🟢 4.5
Details
CheckScoreReason
Binary-Artifacts🟢 10no binaries found in the repo
Packaging⚠️ -1packaging workflow not detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Maintained🟢 67 commit(s) and 1 issue activity found in the last 90 days -- score normalized to 6
Code-Review🟢 3Found 9/26 approved changesets -- score normalized to 3
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Security-Policy⚠️ 0security policy file not detected
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Vulnerabilities🟢 82 existing vulnerabilities detected
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0

Scanned Files

  • Cargo.lock

@houseme
Copy link
Contributor

houseme commented Nov 15, 2025

@copilot fix Update configs must have a unique combination of 'package-ecosystem', 'directory', and 'target-branch'. Ecosystem 'cargo' has overlapping directories..

Copy link
Contributor Author

Copilot AI commented Nov 15, 2025

@copilot fix Update configs must have a unique combination of 'package-ecosystem', 'directory', and 'target-branch'. Ecosystem 'cargo' has overlapping directories..

Fixed in commit 1574a34. Merged the two overlapping cargo configurations into a single weekly schedule with s3s dependency grouping.

Copilot AI changed the title [WIP] Fix KMS configuration changes on browser refresh Fix KMS configuration synchronization across cluster nodes Nov 15, 2025
@loverustfs loverustfs mentioned this pull request Nov 15, 2025
Closed
@loverustfs loverustfs marked this pull request as ready for review November 15, 2025 15:19
@houseme houseme merged commit b796408 into main Nov 15, 2025
16 checks passed
@houseme houseme deleted the copilot/fix-kms-configuration-issue branch November 15, 2025 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@houseme houseme Awaiting requested review from houseme

Assignees

@houseme houseme

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

KMS Configuration changes on every browser refresh

3 participants

@houseme @loverustfs