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

bstin · 2024-02-14T12:23:21Z

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 · 2024-02-14T12:23:23Z

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 · 2024-02-14T12:48:20Z

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

kristapsk · 2024-02-14T15:29:56Z

Concept ACK

BrandonOdiwuor

Concept ACk

tdb3

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

tdb3 · 2024-02-17T20:13:37Z

share/rpcauth/rpcauth.py

@@ -24,6 +25,7 @@ def main():
    parser = ArgumentParser(description='Create login credentials for a JSON-RPC user')
    parser.add_argument('username', help='the username for authentication')
    parser.add_argument('password', help='leave empty to generate a random password or specify "-" to prompt for password', nargs='?')
+    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.

thank you. yes single "-" makes more sense.

ACK for 713ab48

Here's an example for the README.md:

diff --git a/share/rpcauth/README.md b/share/rpcauth/README.md index 6b5e4446a5..1b3acb1dac 100644 --- a/share/rpcauth/README.md +++ b/share/rpcauth/README.md @@ -15,5 +15,5 @@ positional arguments: optional arguments: -h, --help show this help message and exit - -j, -json output data in json format + -j, --json output data in json format

luke-jr · 2024-02-20T22:16:16Z

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

kristapsk · 2024-02-20T22:30:10Z

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:

python3 "$SCRIPT_DIR/rpcauth.py" \
    "$bitcoin_rpcuser" "$bitcoin_rpcpassword" | grep rpcauth >> /etc/bitcoin/bitcoin.conf

bstin · 2024-02-20T23:41:26Z

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 · 2024-02-21T08:25:49Z

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

bstin · 2024-02-21T11:36:34Z

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

maflcko

Seems fine to add this.

share/rpcauth/rpcauth.py

maflcko · 2024-02-22T18:33:44Z

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 · 2024-02-22T19:06:12Z

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

maflcko · 2024-02-22T19:19:54Z

review ACK 4f74671

share/rpcauth/README.md

maflcko · 2024-04-23T06:37:01Z

Are you still working on this?

tdb3

Concept ACK.

Looks like these commits (4f74671, 1606590, and f2fb13b) should be squashed to clean up and tell a concise story of update.

Seems to work well:

$ ./rpcauth.py
usage: rpcauth.py [-h] [-j] username [password]
rpcauth.py: error: the following arguments are required: username

$ ./rpcauth.py -h
usage: rpcauth.py [-h] [-j] username [password]

Create login credentials for a JSON-RPC user

positional arguments:
  username    the username for authentication
  password    leave empty to generate a random password or specify "-" to prompt for password

options:
  -h, --help  show this help message and exit
  -j, --json  output to json instead of plain-text

$ ./rpcauth.py tester1
String to be appended to bitcoin.conf:
rpcauth=tester1:5be9e5538a73ee796d73c16fe7267343$fb3462c51b9a943a5235eea45311100e637c47ef76e2ce00bd109d21940e231c
Your password:
j70a0dzR8gEZfdU3Su1c9jZi6k107iAMKiMdchHhrQE

$ ./rpcauth.py -j tester2
{"username": "tester2", "password": "NymKCrxvVxWvDd3XwPUaqgZp9Wr5cf3Kg6Flvss2b_U", "rpcauth": "tester2:7e1296561e4830d9987d38b32b2daf1d$d30aac9fd97726f9dcf6015508b01ec5ad9d1fd7055e9b61d619578b2bb871f5"}

$ ./rpcauth.py --json tester3
{"username": "tester3", "password": "kjhzSg-2jtizz4Krjj1KmQxrY0Vemf0jI3og2MTHFVs", "rpcauth": "tester3:f7a9248c1b11fec9efa1c681c42a9209$c68a4465920456f3cd99b322b58dbed50b9d4643cb13ce61228c9b8343a65bde"}

$ ./rpcauth.py --json tester4 throwawaypass
{"username": "tester4", "password": "throwawaypass", "rpcauth": "tester4:09156b6ab4a6ae5ab11508c4a6f5f68b$8804e2f73c22bac9532e40b3c6cdb2c61668bad9a4d7c67f4284bdcffb9bedc1"}

$ ./rpcauth.py tester5 anotherthrowaypass
String to be appended to bitcoin.conf:
rpcauth=tester5:563ad3d4b62f746c689f3853b0147ebe$7cf8ca75dd3f3623d79f2e5eeca6a69eefdd61b8d1546cba571ed6385ebc8dad
Your password:
anotherthrowaypass

maflcko · 2024-04-24T05:28:16Z

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

…on format -j/--json update and remove un-needed parens Update README to reflect new -j/--json options

bstin · 2024-04-25T13:39:37Z

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

maflcko · 2024-04-25T14:13:08Z

ACK 9adf949

willcl-ark

tACK 9adf949

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

tdb3

Thanks for squashing commits.
ACK for 9adf949

achow101 · 2024-04-25T17:55:28Z

ACK 9adf949

fa3e115 doc: Correct pull request prefix for scripts and tools (MarcoFalke) Pull request description: `script` is confusing, because in the context of Bitcoin, it usually means Bitcoin script (c.f. `CScript` in `script.h`, or pull requests such as #27122 using the prefix). This could be fixed by renaming it to `scripts` (with a plural `s` at the end), however, looking at the current usage `contrib` and `cli` seem more common (#29687, #26953, #26584, #24864, #30074, #29433 ...) ACKs for top commit: fanquake: ACK fa3e115 willcl-ark: ACK fa3e115 hebasto: ACK fa3e115. theuni: ACK fa3e115 Tree-SHA512: fb3a3892ca5f859e590c8a620350c397ef1f9eafd9e174c70ef50095d401a396758d6c93ad41888da8025c41e25e691f30c18f9e974af13597f2266bb2c53b6d

BrandonOdiwuor reviewed Feb 15, 2024

View reviewed changes

tdb3 reviewed Feb 17, 2024

View reviewed changes

bstin changed the title ~~Update rpcauth.py~~ rpcauth.py - Add new option (-json) to output text in json format Feb 21, 2024

maflcko reviewed Feb 22, 2024

View reviewed changes

share/rpcauth/rpcauth.py Outdated Show resolved Hide resolved

bstin force-pushed the patch-1 branch from 713ab48 to 4f74671 Compare February 22, 2024 19:00

bstin changed the title ~~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 Feb 22, 2024

DrahtBot added the Scripts and tools label Feb 22, 2024

DrahtBot requested a review from tdb3 February 22, 2024 19:19

kristapsk reviewed Feb 22, 2024

View reviewed changes

share/rpcauth/README.md Outdated Show resolved Hide resolved

DrahtBot requested review from kristapsk and tdb3 and removed request for tdb3 and kristapsk February 22, 2024 20:02

DrahtBot added the CI failed label Apr 19, 2024

DrahtBot removed the CI failed label Apr 23, 2024

tdb3 reviewed Apr 24, 2024

View reviewed changes

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

9adf949

…on format -j/--json update and remove un-needed parens Update README to reflect new -j/--json options

bstin force-pushed the patch-1 branch from f2fb13b to 9adf949 Compare April 25, 2024 13:38

DrahtBot requested a review from tdb3 April 25, 2024 14:13

willcl-ark approved these changes Apr 25, 2024

View reviewed changes

tdb3 reviewed Apr 25, 2024

View reviewed changes

achow101 merged commit 2066295 into bitcoin:master Apr 25, 2024
9 of 13 checks passed

NikolaiNikolaevichBoyko approved these changes Apr 27, 2024

View reviewed changes

maflcko mentioned this pull request May 22, 2024

doc: Correct pull request prefix for scripts and tools #30150

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

bstin commented Feb 14, 2024

DrahtBot commented Feb 14, 2024 •

edited

Loading

maflcko commented Feb 14, 2024 •

edited

Loading

kristapsk commented Feb 14, 2024

BrandonOdiwuor left a comment

tdb3 left a comment •

edited

Loading

tdb3 Feb 17, 2024

bstin Feb 17, 2024

tdb3 Feb 21, 2024

tdb3 Mar 12, 2024

luke-jr commented Feb 20, 2024

kristapsk commented Feb 20, 2024

bstin commented Feb 20, 2024

maflcko commented Feb 21, 2024

bstin commented Feb 21, 2024

maflcko left a comment

maflcko commented Feb 22, 2024

bstin commented Feb 22, 2024

maflcko commented Feb 22, 2024

maflcko commented Apr 23, 2024

tdb3 left a comment

maflcko commented Apr 24, 2024

bstin commented Apr 25, 2024

maflcko commented Apr 25, 2024

willcl-ark left a comment

tdb3 left a comment

achow101 commented Apr 25, 2024

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

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

Conversation

bstin commented Feb 14, 2024

DrahtBot commented Feb 14, 2024 • edited Loading

Code Coverage

Reviews

maflcko commented Feb 14, 2024 • edited Loading

kristapsk commented Feb 14, 2024

BrandonOdiwuor left a comment

Choose a reason for hiding this comment

tdb3 left a comment • edited Loading

Choose a reason for hiding this comment

tdb3 Feb 17, 2024

Choose a reason for hiding this comment

bstin Feb 17, 2024

Choose a reason for hiding this comment

tdb3 Feb 21, 2024

Choose a reason for hiding this comment

tdb3 Mar 12, 2024

Choose a reason for hiding this comment

luke-jr commented Feb 20, 2024

kristapsk commented Feb 20, 2024

bstin commented Feb 20, 2024

maflcko commented Feb 21, 2024

bstin commented Feb 21, 2024

maflcko left a comment

Choose a reason for hiding this comment

maflcko commented Feb 22, 2024

bstin commented Feb 22, 2024

maflcko commented Feb 22, 2024

maflcko commented Apr 23, 2024

tdb3 left a comment

Choose a reason for hiding this comment

maflcko commented Apr 24, 2024

bstin commented Apr 25, 2024

maflcko commented Apr 25, 2024

willcl-ark left a comment

Choose a reason for hiding this comment

tdb3 left a comment

Choose a reason for hiding this comment

achow101 commented Apr 25, 2024

DrahtBot commented Feb 14, 2024 •

edited

Loading

maflcko commented Feb 14, 2024 •

edited

Loading

tdb3 left a comment •

edited

Loading