-
Notifications
You must be signed in to change notification settings - Fork 913
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.
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
Add extra configuration options to gRPC #7672
base: master
Are you sure you want to change the base?
Conversation
05e01e0
to
025bfc4
Compare
The I tried to reproduce the failure locally.
Subsequently, I ran the test a 100 times.
I haven't observed a single failure. I'm not convinced the test is actually broken and don't have a way to reproduce |
This needs a rebase, now that the grpc-host config option has been included. |
025bfc4
to
2821c3d
Compare
This needs a rebase, since the grpc-host option is now merged (separately). |
I did rebase it on grpc-host option but need to rebase again after 7807 merge. I also need to clean up some commits. Working on them right now. |
5b75526
to
ec955e1
Compare
A small refactor that allows to specify a more flexible configuration in the grpc-plugin. By default the grpc-plugin will bind to 127.0.0.1 instead of 0.0.0.0.
Also support using the cln-grpc plugin over http. This is useful for testing. As a safe-guard we only allow this configuration if the services binds to the loopback address. Changelog-Added: `grpc-scheme` option for grpc plugin
ec955e1
to
fdf3cf3
Compare
Some of the functionality overlaps #7479. I was not aware at the time of writing. Both this PR and #7479 introduce a default on the host and port. I believe it is sensible to close this PR and tackle the http-interface in a new PR. |
@ErikDeSmedt I have rebased this PR onto master, which now includes the merged PR #7479 (default |
ACK fdf3cf3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebase looks great
Pull Request Title
Description
I've added a few configuration options to the gRPC-plugin.
grpc-ip-address
: I've changed the default ip-address to which the gRPC interface will bind from0.0.0.0
to127.0.0.1
. This new default should be more secure. Users who need to access the gRPC-interface externally can setgrpc-ip-address
to the port they like. Users can also configure an IPv6 address.grpc-scheme
: Previously, the gRPC connection only allowed https over mTLS. Configurting mTLS is somewhat cumbersome. Users that only need the gRPC interface locally can now setgrpc-scheme=http
. This will only work ifgrpc-ip-address
is set to a loopback address (127.0.0.1 or equivalent for IPv6).Related Issues
cln-grpc
: Specify a host and port #7654Changes Made
Checklist
Ensure the following tasks are completed before submitting the PR:
TODOs
have been addressed or removed.Additional Notes
Any additional information or context about this PR that reviewers should know.