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.

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 TLS to the server side, expose optional root certificate path on client side #1923

Merged
merged 4 commits into from
Oct 17, 2023

Conversation

anodar
Copy link
Contributor

@anodar anodar commented Oct 16, 2023

to allow self-signed certificates on external signer and ease of testing.

@cla-bot cla-bot bot added the s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA. label Oct 16, 2023
@anodar anodar marked this pull request as ready for review October 16, 2023 15:36
@anodar anodar requested a review from PlasmaPower October 16, 2023 15:36
amsanghi
amsanghi previously approved these changes Oct 17, 2023
Copy link
Collaborator

@PlasmaPower PlasmaPower left a comment

Choose a reason for hiding this comment

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

Generally LGTM but I have a couple optional recommendations

fmt.Println("Server is listening on port 1234...")
if err := httpSrv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
t.Errorf("ListenAndServe() unexpected error: %v", err)
fmt.Println("Server is listening on port 443...")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is still listening on port 1234

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

arbnode/dataposter/data_poster.go Show resolved Hide resolved
Base automatically changed from external-signer to master October 17, 2023 18:51
@anodar anodar dismissed amsanghi’s stale review October 17, 2023 18:51

The base branch was changed.

Copy link
Collaborator

@PlasmaPower PlasmaPower left a comment

Choose a reason for hiding this comment

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

LGTM

@PlasmaPower PlasmaPower merged commit ffec797 into master Oct 17, 2023
5 checks passed
@PlasmaPower PlasmaPower deleted the external-signer-mtls branch October 17, 2023 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants