Skip to content

Remove root key env var dead code #8942

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

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Neon-White
Copy link
Contributor

Describe the Problem

In continuation to noobaa/noobaa-operator#1533 (which also contains a more elaborate explanation), this PR removes the dead code paths that are no longer relevant due to the removal of the NOOBAA_ROOT_SECRET environment variable

Explain the Changes

  1. load_root_key() was removed since it only performed an env var loading
  2. system_store now calls load_root_keys_from_mount() instead of load_root_key() to load the root key from the mount point instead of the env var
  3. test_encryption no longer tests load_root_key() since it was removed
  4. get_root_key_id() no longer handles cases where the key might be in the env
  5. Removed the env var from the NVA build as well

Issues:

Fixed:

  1. DFBUGS-1608

Testing Instructions:

  1. Deploy NooBaa
  2. Perform some day 1 operations (bucket creation, bucket IO)
  • Doc added/updated
  • Tests added

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

src/test/unit_tests/test_encryption.js:906

  • Verify that the removal of the subsequent load calls (previously used to reload the system state) does not leave the active root key state unvalidated. Consider adding explicit assertions to ensure that the state update is properly reflected after modifying the active root key file.
await fs.promises.writeFile(config.ROOT_KEY_MOUNT + '/active_root_key', 'key1');

src/server/system_services/master_key_manager.js:52

  • Confirm that the revised get_root_key_id behavior (which now solely relies on this.active_root_key) correctly handles cases when active_root_key is undefined, since the environment variable fallback was removed.
return this.active_root_key && this.active_root_key.toString();

@Neon-White Neon-White marked this pull request as draft April 7, 2025 19:44
@pull-request-size pull-request-size bot added size/M and removed size/S labels Apr 14, 2025
Signed-off-by: Ben <[email protected]>
@Neon-White Neon-White force-pushed the remove-root-secret-env-dead-code branch from 9e76432 to 751a904 Compare April 14, 2025 17:34
@pull-request-size pull-request-size bot added size/L and removed size/M labels Apr 17, 2025
@pull-request-size pull-request-size bot added size/M and removed size/L labels Apr 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant