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.

Sign up for GitHub

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

Jump to bottom

Add RPC HTTP options to specify custom truststore and password #7978

Merged
merged 18 commits into from
Dec 18, 2024
Merged

Add RPC HTTP options to specify custom truststore and password #7978

merged 18 commits into from
Dec 18, 2024
+252 −17

Conversation

pullurib
Copy link
Contributor

@pullurib pullurib commented Dec 3, 2024

PR description

  • Introduce a new option to specify the truststore for the JSON-RPC HTTP client.
  • Support passing a password file for the truststore through a new option to avoid exposing the password in logs or the CLI.
  • Ensure the client certificates are validated successfully using the provided truststore - added test cases
  • Added testcase to verify option use

Fixed Issue(s)

#7932

@macfarla macfarla added the doc-change-required Indicates an issue or PR that requires doc to be updated label Dec 5, 2024
@macfarla macfarla changed the title Add RPC HTTP options to specify custom truststore and it's password Add RPC HTTP options to specify custom truststore and its password Dec 5, 2024
macfarla
macfarla reviewed Dec 5, 2024
View reviewed changes
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with the error messages, it's helpful for the user to indicate exactly which CLI option is missing or required. Otherwise i think it's looking good

besu/src/main/java/org/hyperledger/besu/cli/options/JsonRpcHttpOptions.java Outdated Show resolved Hide resolved
besu/src/main/java/org/hyperledger/besu/cli/options/JsonRpcHttpOptions.java Outdated Show resolved Hide resolved
pullurib and others added 4 commits December 9, 2024 09:23
@macfarla macfarla marked this pull request as draft December 11, 2024 05:17
@macfarla
Copy link
Contributor

There is a unit test failing. You can run these tests locally, this will benefit your time and reviewers. You'll get feedback waaaayy faster. converting to draft since it's not ready to review

JsonRpcHttpOptionsTest > rpcHttpTlsClientAuthWithTrustStoreAndKnownClientsFileReportsError() FAILED
    java.lang.AssertionError at JsonRpcHttpOptionsTest.java:508

@macfarla macfarla self-assigned this Dec 11, 2024
Update changed logs in tests
1f2de3a 
Signed-off-by: Bhanu Pulluri <[email protected]>
@pullurib pullurib marked this pull request as ready for review December 11, 2024 20:59
@pullurib
Copy link
Contributor Author

There is a unit test failing. You can run these tests locally, this will benefit your time and reviewers. You'll get feedback waaaayy faster. converting to draft since it's not ready to review

JsonRpcHttpOptionsTest > rpcHttpTlsClientAuthWithTrustStoreAndKnownClientsFileReportsError() FAILED
    java.lang.AssertionError at JsonRpcHttpOptionsTest.java:508

I agree, no change is too small to skip running local tests. Fixed and ran the tests locally.

@macfarla macfarla changed the title Add RPC HTTP options to specify custom truststore and its password Add RPC HTTP options to specify custom truststore and password Dec 18, 2024
@macfarla macfarla merged commit 43c8a6a into hyperledger:main Dec 18, 2024
43 checks passed
daniellehrner pushed a commit to daniellehrner/besu that referenced this pull request Dec 18, 2024
@pullurib @macfarla @daniellehrner
Add RPC HTTP options to specify custom truststore and password (hyper…
384e293 
…ledger#7978)

* Add RPC HTTP options to specify custom truststore and it's password

* Update error logs to indicate options to use

Signed-off-by: Bhanu Pulluri <[email protected]>

---------

Signed-off-by: Bhanu Pulluri <[email protected]>
Signed-off-by: Bhanu Pulluri <[email protected]>
Co-authored-by: Bhanu Pulluri <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
Signed-off-by: Daniel Lehrner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@macfarla macfarla macfarla approved these changes

Assignees

@macfarla macfarla

Labels
doc-change-required Indicates an issue or PR that requires doc to be updated
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pullurib @macfarla