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

Use TLS endpoint when connecting to Sandbox #3570

Merged
merged 2 commits into from
Nov 3, 2023
Merged

Use TLS endpoint when connecting to Sandbox #3570

merged 2 commits into from
Nov 3, 2023

Conversation

sophokles73
Copy link
Contributor

Addresses #3569

The command line client was still trying to connect to the insecure ports of the Sandbox.

This has been changed so that the client now uses the TLS endpoints and requires the user to specify a trust store for validating the server certificate.

The Getting Started guide has been adapted accordingly.

@sophokles73 sophokles73 added this to the 2.5.0 milestone Oct 14, 2023
@sophokles73 sophokles73 requested review from calohmn and removed request for kaniyan October 27, 2023 15:04
@sophokles73
Copy link
Contributor Author

@calohmn would you mind taking a look

@sophokles73 sophokles73 force-pushed the fix_connection_to_sandbox branch 2 times, most recently from ac0d2a1 to 951d0da Compare October 29, 2023 12:52
@sophokles73
Copy link
Contributor Author

@calohmn would you mind taking another look?

@sophokles73
[#3569] Use TLS endpoint when connecting to Sandbox
8741fe1 
The command line client was still trying to connect to the insecure
ports of the Sandbox.

This has been changed so that the client now uses the TLS endpoints and
requires the user to specify a trust store for validating the server
certificate.

The Getting Started guide has been adapted accordingly.
Also updated the Python Getting Started example script to use Kafka
instead of AMQP 1.0 based messaging infrastructure.
@sophokles73
[#3569] Add release notes entry
307d2d9
calohmn
calohmn approved these changes Nov 3, 2023
View reviewed changes
Copy link
Contributor

@calohmn calohmn left a comment

Choose a reason for hiding this comment

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

LGTM

@sophokles73 sophokles73 merged commit c291d27 into master Nov 3, 2023
10 checks passed
@sophokles73 sophokles73 deleted the fix_connection_to_sandbox branch November 3, 2023 07:43
@sophokles73 sophokles73 mentioned this pull request Nov 4, 2023
Closed
@calohmn
Copy link
Contributor

calohmn commented Nov 27, 2023

An afterthought here:
The sandbox is using a letsencrypt-certificate, which can be validated via the Java root CAs since Java 8u141 (see here). So, it seems unnecessary to me, that the CLI requires the --ca-file parameter, set with the system default ca-certificates.crt,, when connecting to the sandbox.
If I remove the check for the existence of the --ca-file parameter and add clientConfig.setTlsEnabled(true);, the CLI connections to the sandbox AMQP adapter and AMQP messaging network also work without the --ca-file parameter.
This also applies to the connection to Kafka.
@sophokles73 So, can't we just remove the --ca-file requirement when connecting to the sandbox?

@sophokles73
Copy link
Contributor Author

Does it also work without the --ca-file param when using the native executable?

@calohmn
Copy link
Contributor

calohmn commented Nov 27, 2023

Yes, I've verified this by running

./hono-cli-2.5.0-SNAPSHOT-exec app --amqp --sandbox consume
./hono-cli-2.5.0-SNAPSHOT-exec app --sandbox consume
./hono-cli-2.5.0-SNAPSHOT-exec amqp --sandbox -u gw@DEFAULT_TENANT -p gw-secret

@calohmn calohmn mentioned this pull request Dec 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@calohmn calohmn calohmn approved these changes

Assignees
No one assigned
Labels
bug Client
Projects
None yet
Milestone
2.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@sophokles73 @calohmn