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

Refine dmctl error messages for "secret key is not initialized " #12046

Open
Frank945946 opened this issue Feb 6, 2025 · 2 comments
Open

Refine dmctl error messages for "secret key is not initialized " #12046

Frank945946 opened this issue Feb 6, 2025 · 2 comments
Labels
affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-8.5 This bug affects the 8.5.x(LTS) versions. area/dm Issues or PRs related to DM. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. severity/moderate type/bug The issue is confirmed as a bug.

Comments

@Frank945946
Copy link

Frank945946 commented Feb 6, 2025

What did you do?

Since v8.0, the hardcoded secret key for dmctl has been removed. As a result, when customers want to use dmctl to encrypt and decrypt the passwords of the source and target databases in the DM task configuration files, they need to upload the secret key to the DM Master and set the secret-key-path parameter in the DM Master configuration file. Otherwise, an error will be returned:

[root@svr1 ~]# tiup dmctl encrypt 'abc!@#123' --master-addr [192.168.0.2:8261](http://192.168.0.2:8261/)
Starting component dmctl: /root/.tiup/components/dmctl/v8.1.0/dmctl/dmctl encrypt abc!@#123 --master-addr [192.168.0.2:8261](http://192.168.0.2:8261/)
Error: secret key is not initialized 

The error message is unhelpful, as users do not know how to resolve the issue.

What did you expect to see?

Refine the error message to help users resolve the issue easily on their own.
For example:
Error: The secret key for dmctl has not been initialized. Please ensure that the required key has been uploaded to the DM Master node and that the secret-key-path parameter in the DM Master configuration file is set to its path on the DM Master.

What did you see instead?

Error: secret key is not initialized

Versions of the cluster

v8.1 and v8.5

(paste DM version here, and you must ensure versions of dmctl, DM-worker and DM-master are same)

Upstream MySQL/MariaDB server version:

(paste upstream MySQL/MariaDB server version here)

Downstream TiDB cluster version (execute SELECT tidb_version(); in a MySQL client):

(paste TiDB cluster version here)

How did you deploy DM: tiup or manually?

(leave TiUP or manually here)

Other interesting information (system version, hardware config, etc):

>
>

current status of DM cluster (execute query-status <task-name> in dmctl)

(paste current status of DM cluster here)
@Frank945946 Frank945946 added area/dm Issues or PRs related to DM. type/bug The issue is confirmed as a bug. labels Feb 6, 2025
@alastori
Copy link

alastori commented Feb 6, 2025

Hi @Frank945946,

Thanks for submitting this issue and for the initial proposed improvement:

Earlier Proposal:
"Error: The secret key for dmctl has not been initialized. Please ensure that the required key has been uploaded to the DM Master node and that the secret-key-path parameter in the DM Master configuration file is set to its path on the DM Master."

This is a good improvement over the original message, but I believe we can refine it further to improve clarity, readability, and make it more actionable.

Suggested Refinement

To make the message more structured and easier to follow, I suggest the following format:

Error: Secret key not initialized. 
To enable encryption:
1. Create a file containing a 32-byte (64-character) hexadecimal AES-256 secret key.
2. Set `secret-key-path` in DM-master’s configuration file to point to this key file.
3. Restart DM-master to apply the configuration.
For details, see: https://docs.pingcap.com/tidb/stable/dm-customized-secret-key

Why This Change?

  • Many CLI tools, such as AWS CLI, Terraform, and Microsoft CLI, follow a user-actionable and structured error message approach
  • Message should tell the user what is required as a fix, rather than only complaining that there is an error
  • The numbered format makes it easier for users to scan and follow.
  • Instead of just describing the issue, it provides specific steps to resolve it.
  • Users don’t need to infer next steps from a single sentence.
  • I see in errors.toml we already reference URLs.

Let me know your thoughts. Does this revision make sense?

Best,
@alastori

@D3Hunter D3Hunter added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-8.5 This bug affects the 8.5.x(LTS) versions. labels Feb 7, 2025
@D3Hunter D3Hunter changed the title Refine dmctl error messages Refine dmctl error messages for "secret key is not initialized " Feb 7, 2025
@Frank945946
Copy link
Author

@alastori LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-8.5 This bug affects the 8.5.x(LTS) versions. area/dm Issues or PRs related to DM. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. severity/moderate type/bug The issue is confirmed as a bug.
Projects
None yet
Development

No branches or pull requests

4 participants