-
Notifications
You must be signed in to change notification settings - Fork 1k
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
SSL Configuration Fails even EnbleSsl property is set to false #6038
SSL Configuration Fails even EnbleSsl property is set to false #6038
Conversation
Thanks @aminchenkov - I'll review this. |
Looks like this PR broke some of the other tests in the affected file. Looking at that... |
Ah, it's your own test that is failing |
|
Disabled the inner assertion around the exact error message. But otherwise, this PR looks good to me. |
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.
LGTM
@Arkatufus we're going to need to backport this change to v1.4 as well. |
Build failed due to a temporary NuGet.org failure. I'll re-run it. |
@Aaronontheweb , what is next? I see it was approved, but when it will be merged? Can we expect it to release it soon? |
@aminchenkov I'm making sure that those compilation issues (NuGet-related) have been resolved. Once that's done we'll backport this PR into the v1.4 branch and do a release soon. This week probably. |
We still need to check for |
You're right about that |
@@ -87,7 +87,7 @@ public static DotNettyTransportSettings Create(Config config) | |||
serverSocketWorkerPoolSize: ComputeWorkerPoolSize(config.GetConfig("server-socket-worker-pool")), | |||
clientSocketWorkerPoolSize: ComputeWorkerPoolSize(config.GetConfig("client-socket-worker-pool")), | |||
maxFrameSize: ToNullableInt(config.GetByteSize("maximum-frame-size", null)) ?? 128000, | |||
ssl: config.HasPath("ssl") ? SslSettings.Create(config.GetConfig("ssl")) : SslSettings.Empty, | |||
ssl: config.HasPath("ssl") && config.GetBoolean("enable-ssl", false) ? SslSettings.Create(config.GetConfig("ssl")) : SslSettings.Empty, |
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.
I think this should address your issue @ismaelhamed
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.
enableSsl && config.HasPath("ssl")
can be achieved by moving line #81 outside the return, but otherwise LGTM.
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.
@ismaelhamed , I pushed the change. Please check if it is what you wanted.
Head branch was pushed to by a user without write access
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.
LGTM
…t to false (#6043) * SSL Configuration Fails even EnbleSsl property is set to false (#6038) * Respect EnableSsl configuration propert * Update DotNettySslSupportSpec.cs * Update DotNettySslSupportSpec.cs * Update DotNettyTransportSettings.cs * Moved enableSsl variable initialization outside return statement Co-authored-by: Aliaksei Minchankou <[email protected]> Co-authored-by: Aaron Stannard <[email protected]> * SSL * (Parameter 'certificatePath') * TestKit * Fixed up assertion to no longer be whitespace sensitive * [Bug] using FluentAssertions; Co-authored-by: aminchenkov <[email protected]> Co-authored-by: Aliaksei Minchankou <[email protected]> Co-authored-by: Aaron Stannard <[email protected]>
@Aaronontheweb, I upgraded our project to use 1.40 and 1.39 and our application is broken. We use Ask/Tell pattern to communicate between nodes and this does not work anymore - message does not send. Any ideas why it might happen? |
I see in logs (EnableSsl is true): |
Custer is formed correctly with all required nodes. |
Glad to hear it |
@Aaronontheweb , it is actually not good for us - we cannot use latest libraries. do you know what might be the root cause of why a message cannot be sent remotely anymore even the cluster is formed? The issue in 1.14.39 as well. |
I missed your earlier comments - no idea. I'm at a conference and can only see messages on my mobile. I recommend filing an issue. |
Also, going to need a lot more information (logs.) Strongly recommend you diff the releases on GitHub and identify where a change could have affected you. We have tons of users running 1.4.39 without issue. |
There's no reason why Akka.NET wouldn't allow an outbound message to be sent - if Akka.Remote / Akka.Cluster are still functioning normally then the issue isn't remoting / clustering. It's probably your application code and possibly the way we changed how |
Thank you. Will try to find the root cause. Will keep you posted. |
It is release 1.4.24. 23 works fine, Reading release notes. @Aaronontheweb , if you already know what might be the reason - please reply |
Last logging warning before nothing: "heartbeat interval is growing too large for address" |
@Aaronontheweb , in 23 the code below returns 1 routee but in 1.24 it returns empty list. Do you have an idea why it might be. We are using deployment.
|
Sorry, now it return 1 as well. |
So release v1.4.24 works without any issues now too? |
No. 1.24 does not work even a routee is resolved. |
Here's everything that occurred in v1.4.24 https://github.com/akkadotnet/akka.net/releases/tag/1.4.24 - the only issue I can think of that would affect you is the transmission of unsafe types in Hyperion was disallowed then. Does that apply to you? |
@Aaronontheweb , I don't know how I can answer to your question. We send in messages between nodes only integers and strings. |
That'd be a "no" then - Hyperion doesn't filter out primitives. You should probably just message us in Discord because in order for us to help you you're going to need to share logs / config. Whatever is going on is specific to your application. |
@Aaronontheweb , do you already have a Discord Server? |
Fixes # #3594
Changes
If remote:dot-netty:enablessl property is set false and certificate and a password are not provided, a SslSettings object is still trying to be created.
After this change, SslSettings object will be attempted to create only if enablessl property is set to false.