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

[DNM] MCOL-5019: distributing cskeys secrets file, move cskeys amd cspasswd functions to mcs cli. #3427

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

[DNM] MCOL-5019: distributing cskeys secrets file, move cskeys amd cspasswd functions to mcs cli. #3427

wants to merge 3 commits into from

Conversation

mariadb-AlanMologorsky
Copy link
Contributor

@mariadb-AlanMologorsky mariadb-AlanMologorsky commented Mar 11, 2025 •
edited
Loading

  • [add] distribute .secrets file to all nodes while adding a new node
  • [add] encrypt_password, generate_secrets_data, save_secrets to CEJPasswordHandler
  • [add] tools section to mcs cli tool
  • [add] mcs_cluster_tool/tools_commands.py file with cskeys and cspasswd commands
  • [add] cskeys and cspasswd commands to tools section of mcs cli
  • [mv] backup/restore commands to tools section mcs cli
  • [fix] minor imports ordering
  • [fix] constants
  • [fix] CEJPasswordHandler class methods to use directory for cskeys file
  • [fix] CEJPasswordHandler.encrypt_password to return password in hex format
  • [fix] CEJPasswordHandler key_length
  • [fix] CEJPasswordHandler os.urandom call typo
  • [upd] mcs cli README.md and man page
  • [upd] mcs cli README_DEV.md
  • [fix] mcs_cluster_tool/decorators.py to handle typer.Exit exception
  • [add] various docstrings

@mariadb-AlanMologorsky mariadb-AlanMologorsky changed the title MCOL-5019: distributing cskeya secrets file, move cskeys amd cspasswd functions to mcs cli. MCOL-5019: distributing cskeys secrets file, move cskeys amd cspasswd functions to mcs cli. Mar 11, 2025
@mariadb-AlanMologorsky mariadb-AlanMologorsky changed the title MCOL-5019: distributing cskeys secrets file, move cskeys amd cspasswd functions to mcs cli. [DNM] MCOL-5019: distributing cskeys secrets file, move cskeys amd cspasswd functions to mcs cli. Mar 11, 2025
drrtuy
drrtuy approved these changes Mar 12, 2025
View reviewed changes
Copy link
Collaborator

@drrtuy drrtuy left a comment

Choose a reason for hiding this comment

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

untested ACK
How did you check that new passwords are compatible with the decoding mechanism used by the engine? We need a test that decodes password produced by the code added with this patch.

@mariadb-AlanMologorsky mariadb-AlanMologorsky changed the title [DNM] MCOL-5019: distributing cskeys secrets file, move cskeys amd cspasswd functions to mcs cli. MCOL-5019: distributing cskeys secrets file, move cskeys amd cspasswd functions to mcs cli. Mar 12, 2025
@mariadb-AlanMologorsky mariadb-AlanMologorsky changed the title MCOL-5019: distributing cskeys secrets file, move cskeys amd cspasswd functions to mcs cli. [DNM] MCOL-5019: distributing cskeys secrets file, move cskeys amd cspasswd functions to mcs cli. Mar 12, 2025
@mariadb-AlanMologorsky
Copy link
Contributor Author

untested ACK How did you check that new passwords are compatible with the decoding mechanism used by the engine? We need a test that decodes password produced by the code added with this patch.

I created .secrets file using cskeys binary and then encrypt end decrypt password using mcs cspasswd
It confirms that my code fully compatible with old .secrets file generated from engine cskeys (decoding was compatible since I wrote this in CMAPI BTW)
And then vise versa: created .secrets file using mcs cskeys command and then encrypt and decrypt passwords using both (cspasswd and mcs cspasswd) tools. It confirms that .secrets file generated by mcs cli is fully compatible with old engine binaries.

@mariadb-AlanMologorsky
Copy link
Contributor Author

untested ACK How did you check that new passwords are compatible with the decoding mechanism used by the engine? We need a test that decodes password produced by the code added with this patch.

Anyway I could add some test for that.

As I know engine code itself only have secrets.h include in resourcemanager.cpp

@mariadb-AlanMologorsky
feat(cmapi): MCOL-5133: Stage3 stand alone cli tool.
15850f7 
    MAJOR: Some logic inside node remove changed significantly using active
    nodes list from Columnstore.xml to broadcast config after remove.

 [fix] TransactionManager passsing  extra, remove and optional nodes arguments to start_transaction function
 [fix] commit and rollback methods of TransactionManager adding nodes argument
 [fix] TransactionManager using success_txn_nodes inside
 [fix] remove node logic to use Transaction manager
 [fix] cluster set api key call using totp on a top level cli call
 [add] missed docstrings
 [fix] cluster shutdown timeout for next release
@mariadb-AlanMologorsky
feat(cmapi): MCOL-5019: distributing cskeys secrets file, move cskeys…
6770fcc 
… amd cspasswd functions to mcs cli.

[add] distribute .secrets file to all nodes while adding a new node
[add] encrypt_password, generate_secrets_data, save_secrets to CEJPasswordHandler
[add] tools section to mcs cli tool
[add] mcs_cluster_tool/tools_commands.py file with cskeys and cspasswd commands
[add] cskeys and cspasswd commands to tools section of mcs cli
[mv] backup/restore commands to tools section mcs cli
[fix] minor imports ordering
[fix] constants
@mariadb-AlanMologorsky
feat(cmapi): MCOL-5019: review fixes.
7071065 
[fix] CEJPasswordHandler class methods to use directory for cskeys file
[fix] CEJPasswordHandler.encrypt_password to return password in hex format
[fix] CEJPasswordHandler key_length
[fix] CEJPasswordHandler os.urandom call typo
[upd] mcs cli README.md and man page
[upd] mcs cli README_DEV.md
[fix] mcs_cluster_tool/decorators.py to handle typer.Exit exception
[add] various docstrings
@mariadb-AlanMologorsky mariadb-AlanMologorsky force-pushed the feature/MCOL-5019-mcscli-add-cskeys-cspasswd branch from 26f3ac8 to 7071065 Compare March 13, 2025 04:51
drrtuy
drrtuy reviewed Mar 13, 2025
View reviewed changes
cmapi/mcs_cluster_tool/README.md
@@ -18,6 +18,8 @@ $ mcs [OPTIONS] COMMAND [ARGS]...
* `dbrm_backup`: Columnstore DBRM Backup.
* `restore`: Restore Columnstore (and/or MariaDB) data.
* `dbrm_restore`: Restore Columnstore DBRM data.
* `cskeys`: Generates a random AES encryption key and init vector and writes them to disk.
* `cspasswd`: Encrypt a Columnstore plaintext password...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove the trailing '...' plz

drrtuy
drrtuy approved these changes Mar 13, 2025
View reviewed changes
Copy link
Collaborator

@drrtuy drrtuy left a comment

Choose a reason for hiding this comment

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

untested ACK but address the comment plz.

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

@drrtuy drrtuy drrtuy approved these changes

@mariadb-LeonidFedorov mariadb-LeonidFedorov Awaiting requested review from mariadb-LeonidFedorov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mariadb-AlanMologorsky @drrtuy