contrib: rpcauth.py - Add new option (-json) to output text in json format

bstin commented at 12:23 pm on February 14, 2024: contributor

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

DrahtBot commented at 12:23 pm on February 14, 2024: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

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.

maflcko commented at 12:48 pm on February 14, 2024: member

Missing contrib: prefix in pull title? Also, the title should explain the change, both for the pull, as well as the commit title.

kristapsk commented at 3:29 pm on February 14, 2024: contributor

Concept ACK

BrandonOdiwuor commented at 10:27 am on February 15, 2024: contributor

Concept ACk

in share/rpcauth/rpcauth.py:28 in ae121d6cb3 outdated

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')

tdb3 commented at 8:13 pm on February 17, 2024:

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.

bstin commented at 8:16 pm on February 17, 2024:

thank you. yes single “-” makes more sense.

tdb3 commented at 1:08 pm on February 21, 2024:

ACK for 713ab485d7b0aa86ea5af35e44917ef15a5fb22a

tdb3 commented at 11:25 pm on March 12, 2024:

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

tdb3 commented at 8:13 pm on February 17, 2024: contributor

ACK for 713ab485d7b0aa86ea5af35e44917ef15a5fb22a. Cool feature. Inline comments provided.

luke-jr commented at 10:16 pm on February 20, 2024: member

Not sure it makes sense to do this. If you’re parsing JSON, surely you can do the trivial things inside rpcauth yourself?

kristapsk commented at 10:30 pm on February 20, 2024: contributor

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

bstin commented at 11:41 pm on February 20, 2024: contributor

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.

maflcko commented at 8:25 am on February 21, 2024: member

Are you still working on this? #29433 (comment)

bstin renamed this:
~~Update rpcauth.py~~
rpcauth.py - Add new option (-json) to output text in json format
on Feb 21, 2024

bstin commented at 11:36 am on February 21, 2024: contributor

Apologies for delay - PR now has changed prefix / title. I also included update to README.md to document change.

in share/rpcauth/rpcauth.py:40 in 713ab485d7 outdated

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):

maflcko commented at 12:01 pm on February 21, 2024:

0    if args.json:

nit: This is not needed in python

maflcko commented at 6:33 pm on February 22, 2024: member

Seems fine to add this.

maflcko commented at 6:33 pm on February 22, 2024: member

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.

bstin force-pushed on Feb 22, 2024

bstin renamed this:
~~rpcauth.py - Add new option (-json) to output text in json format~~
contrib: rpcauth.py - Add new option (-json) to output text in json format
on Feb 22, 2024

DrahtBot added the label Scripts and tools on Feb 22, 2024

bstin commented at 7:06 pm on February 22, 2024: contributor

Thanks for the walk-thru…..I followed the steps so hopefully all is ok…

maflcko commented at 7:19 pm on February 22, 2024: member

review ACK 4f746716c008216ff01da48ccfda559308b4232e

DrahtBot requested review from tdb3 on Feb 22, 2024

in share/rpcauth/README.md:18 in 4f746716c0 outdated

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

kristapsk commented at 8:02 pm on February 22, 2024:

I disagree with -json, should be --json, same as --help (not -help).

tdb3 commented at 10:23 pm on February 22, 2024:

It makes sense to go with -j / –json for me as well.

tdb3 commented at 11:23 pm on March 12, 2024:

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:

DrahtBot removed review request from tdb3 on Feb 22, 2024

DrahtBot requested review from kristapsk on Feb 22, 2024

DrahtBot requested review from tdb3 on Feb 22, 2024

DrahtBot removed review request from kristapsk on Feb 22, 2024

DrahtBot removed review request from tdb3 on Feb 22, 2024

DrahtBot requested review from tdb3 on Feb 22, 2024

DrahtBot requested review from kristapsk on Feb 22, 2024

DrahtBot added the label CI failed on Apr 19, 2024

maflcko commented at 6:37 am on April 23, 2024: member

Are you still working on this?

DrahtBot removed the label CI failed on Apr 23, 2024

tdb3 commented at 1:40 am on April 24, 2024: contributor

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

maflcko commented at 5:28 am on April 24, 2024: member

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

contrib: rpcauth.py - Add new option (-j/--json) to output text in json format

-j/--json update and remove un-needed parens

Update README to reflect new -j/--json options

9adf949d2a

bstin force-pushed on Apr 25, 2024

bstin commented at 1:39 pm on April 25, 2024: contributor

Thanks for your help @maflcko, I squashed and hope that it looks ok now.

maflcko commented at 2:13 pm on April 25, 2024: member

ACK 9adf949d2aa6d199b85295b18c08967395b5570a

DrahtBot requested review from tdb3 on Apr 25, 2024

willcl-ark approved

willcl-ark commented at 2:40 pm on April 25, 2024: member

tACK 9adf949d2aa6d199b85295b18c08967395b5570a

Makes sense to me to be able to output this in JSON for automated setups.

tdb3 commented at 4:05 pm on April 25, 2024: contributor

Thanks for squashing commits. ACK for 9adf949d2aa6d199b85295b18c08967395b5570a

achow101 commented at 5:55 pm on April 25, 2024: member

ACK 9adf949d2aa6d199b85295b18c08967395b5570a

achow101 merged this on Apr 25, 2024

achow101 closed this on Apr 25, 2024

NikolaiNikolaevichBoyko approved

contrib: rpcauth.py - Add new option (-json) to output text in json format #29433

Code Coverage

Reviews