search

pingcap / tidb-binlog

A tool used to collect and merge tidb's binlog for real-time data backup and synchronization.
Apache License 2.0
292 stars 131 forks source link

drainer: Support reading downstream password from file #888

Open kolbe opened 4 years ago

kolbe commented 4 years ago

Feature Request

The drainer that writes to downstream MySQL/MariaDB/TiDB currently reads the password from a config file or from the MYSQL_PSWD environment variable.

The drainer should also be able to read the downstream password from a file.

This feature will support reading he password from a kubernetes secret.

IANTHEREAL commented 4 years ago

we can discuss the unified support for encryption way for tools-set here @kolbe @gregwebs @july2993 @suzaku

tennix commented 4 years ago

tidb-binlog can support both configure file and environment variable, but in k8s it's better to use environment variable.

kolbe commented 4 years ago

I think some customers are wary of using environment variables to pass credentials, and they may prefer having the k8s secret shared as a mounted file.

To clarify, my request in this issue is to have the password by itself in a plaintext file, not to be able to include it in a toml-formatted configuration file.

I don't think our built-in password encryption functionality is relevant for this issue.

gregwebs commented 4 years ago

You can see the argument against env vars here. Essentially there are some additional practical risks around env vars leaking that are not present for files. I think that our general approach should be to support both an env var and a file, because in some deployments env vars are much more convenient and a user may know that they won't be leaked in their deployment.

Please note that for MySQL compatibility, we should be using MYSQL_PWD, no S. For backwards compatibility with TiDB tools that started adding an S, we can support both.

in k8s it's better to use environment variable

K8s supports either env var or file, so I don't think there is any problem there

gregwebs commented 4 years ago

I suggest that our standard across the TiDB ecosystem would be to support both

I also suggest that we work towards removing passwords from configuration files (at least stop doing this in new tools, we may need to keep it in existing tools for backwards compatibility), since specifying passwords via environment variable is an easy alternative. Passwords are secrets. Secrets should not be mixed as plain text into non-secret configuration.

In the long-term we may also need to support password rotation for example in case the password changes during backup, but I don't think we need to get into that now.

suzaku commented 4 years ago

I agree that we should remove passwords from configuration files.

There are two passwords that we can specify in tidb-binlog, to add support for both password files and environment variables we can add the following configurations: