This is a simple change to rpcauth.py utility in order to output as json instead raw text.
This is beneficial because integrating json output is simpler with multiple different forms of automation and tooling
This is a simple change to rpcauth.py utility in order to output as json instead raw text.
This is beneficial because integrating json output is simpler with multiple different forms of automation and tooling
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process.
Type | Reviewers |
---|---|
ACK | maflcko, willcl-ark, tdb3, achow101 |
Concept ACK | kristapsk |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
contrib:
prefix in pull title? Also, the title should explain the change, both for the pull, as well as the commit title.
24@@ -24,6 +25,7 @@ def main():
25 parser = ArgumentParser(description='Create login credentials for a JSON-RPC user')
26 parser.add_argument('username', help='the username for authentication')
27 parser.add_argument('password', help='leave empty to generate a random password or specify "-" to prompt for password', nargs='?')
28+ parser.add_argument("-json", help="output to json instead of plain-text", action='store_true')
The share/rpcauth/README.md
file should also be updated to explain the argument and capability change in rpcauth.py
.
nit:
Recommend making the argument be a single letter (e.g. -j
for json) to follow the existing style with -h
for help and prevent conflation with -j -s -o -n
.
Here’s an example for the README.md:
0diff --git a/share/rpcauth/README.md b/share/rpcauth/README.md
1index 6b5e4446a5..1b3acb1dac 100644
2--- a/share/rpcauth/README.md
3+++ b/share/rpcauth/README.md
4@@ -15,5 +15,5 @@ positional arguments:
5
6 optional arguments:
7 -h, --help show this help message and exit
8- -j, -json output data in json format
9+ -j, --json output data in json format
Not sure it makes sense to do this. If you’re parsing JSON, surely you can do the trivial things inside rpcauth yourself?
Probably also true, my Bitcoin node setup script currently just does something like:
0python3 "$SCRIPT_DIR/rpcauth.py" \
1 "$bitcoin_rpcuser" "$bitcoin_rpcpassword" | grep rpcauth >> /etc/bitcoin/bitcoin.conf
The default behavior is the same, so no change unless someone specifically wants json output.
For anyone just outputting “rpcauth=XXXXX” directly into bitcoin.conf, yes the grep solution is fine.
But if, in addition to that, you wanted to extract the value of rpcauth, then its more steps. (further, this assumes “=” will always be the delimiter)
By providing a machine-friendly output format that makes such deployments less fragile in the future.
36@@ -35,9 +37,13 @@ def main():
37 salt = generate_salt(16)
38 password_hmac = password_to_hmac(salt, args.password)
39
40- print('String to be appended to bitcoin.conf:')
41- print(f'rpcauth={args.username}:{salt}${password_hmac}')
42- print(f'Your password:\n{args.password}')
43+ if (args.json):
0 if args.json:
nit: This is not needed in python
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits once they are ready for review.
Also, the contrib:
prefix is still missing.
14@@ -15,4 +15,5 @@ positional arguments:
15
16 optional arguments:
17 -h, --help show this help message and exit
18+ -j, -json output data in json format
-json
, should be --json
, same as --help
(not -help
).
Here’s an example:
0diff --git a/share/rpcauth/rpcauth.py b/share/rpcauth/rpcauth.py
1index 5dd49c90b8..a40e981502 100755
2--- a/share/rpcauth/rpcauth.py
3+++ b/share/rpcauth/rpcauth.py
4@@ -25,7 +25,7 @@ def main():
5 parser = ArgumentParser(description='Create login credentials for a JSON-RPC user')
6 parser.add_argument('username', help='the username for authentication')
7 parser.add_argument('password', help='leave empty to generate a random password or specify "-" to prompt for password', nargs='?')
8- parser.add_argument("-json", help="output to json instead of plain-text", action='store_true')
9+ parser.add_argument("-j", "--json", help="output to json instead of plain-text", action='store_true')
10 args = parser.parse_args()
11
12 if not args.password:
Concept ACK.
Looks like these commits (4f746716c008216ff01da48ccfda559308b4232e, 1606590bbffa46f88fa6e4333960e82b919efd8a, and f2fb13bb1ac392abc4ccd656b025aa017b12060e) should be squashed to clean up and tell a concise story of update.
Seems to work well:
0$ ./rpcauth.py
1usage: rpcauth.py [-h] [-j] username [password]
2rpcauth.py: error: the following arguments are required: username
3
4$ ./rpcauth.py -h
5usage: rpcauth.py [-h] [-j] username [password]
6
7Create login credentials for a JSON-RPC user
8
9positional arguments:
10 username the username for authentication
11 password leave empty to generate a random password or specify "-" to prompt for password
12
13options:
14 -h, --help show this help message and exit
15 -j, --json output to json instead of plain-text
16
17$ ./rpcauth.py tester1
18String to be appended to bitcoin.conf:
19rpcauth=tester1:5be9e5538a73ee796d73c16fe7267343$fb3462c51b9a943a5235eea45311100e637c47ef76e2ce00bd109d21940e231c
20Your password:
21j70a0dzR8gEZfdU3Su1c9jZi6k107iAMKiMdchHhrQE
22
23$ ./rpcauth.py -j tester2
24{"username": "tester2", "password": "NymKCrxvVxWvDd3XwPUaqgZp9Wr5cf3Kg6Flvss2b_U", "rpcauth": "tester2:7e1296561e4830d9987d38b32b2daf1d$d30aac9fd97726f9dcf6015508b01ec5ad9d1fd7055e9b61d619578b2bb871f5"}
25
26$ ./rpcauth.py --json tester3
27{"username": "tester3", "password": "kjhzSg-2jtizz4Krjj1KmQxrY0Vemf0jI3og2MTHFVs", "rpcauth": "tester3:f7a9248c1b11fec9efa1c681c42a9209$c68a4465920456f3cd99b322b58dbed50b9d4643cb13ce61228c9b8343a65bde"}
28
29$ ./rpcauth.py --json tester4 throwawaypass
30{"username": "tester4", "password": "throwawaypass", "rpcauth": "tester4:09156b6ab4a6ae5ab11508c4a6f5f68b$8804e2f73c22bac9532e40b3c6cdb2c61668bad9a4d7c67f4284bdcffb9bedc1"}
31
32$ ./rpcauth.py tester5 anotherthrowaypass
33String to be appended to bitcoin.conf:
34rpcauth=tester5:563ad3d4b62f746c689f3853b0147ebe$7cf8ca75dd3f3623d79f2e5eeca6a69eefdd61b8d1546cba571ed6385ebc8dad
35Your password:
36anotherthrowaypass
-j/--json update and remove un-needed parens
Update README to reflect new -j/--json options
tACK 9adf949d2aa6d199b85295b18c08967395b5570a
Makes sense to me to be able to output this in JSON for automated setups.