-
Notifications
You must be signed in to change notification settings - Fork 4
Add support for insecure TLS connections #164
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
Conversation
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
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.
Pull request overview
This PR adds support for insecure TLS connections by introducing a new UseInsecureTls configuration option that allows bypassing certificate verification when connecting to Valkey servers via TLS. This is intended for development and testing environments where self-signed certificates are used.
Key changes:
- Added
UseInsecureTlsproperty andWithInsecureTls()fluent method for configuring insecure TLS mode - Converted
TlsModefrom nullable to non-nullable enum with explicitNoTls,InsecureTls, andSecureTlsvalues - Updated
UseTlsproperty logic to handle both secure and insecure TLS modes
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
sources/Valkey.Glide/ConnectionConfiguration.cs |
Adds UseInsecureTls property and WithInsecureTls() method; updates UseTls property to handle multiple TLS modes |
sources/Valkey.Glide/Internals/FFI.structs.cs |
Adds InsecureTls enum value and changes TlsMode from nullable to non-nullable type |
tests/Valkey.Glide.UnitTests/ConnectionConfigurationTests.cs |
Adds 5 unit tests for insecure TLS functionality and updates existing TLS tests to verify explicit enum values |
tests/Valkey.Glide.UnitTests/ConnectionMultiplexerReadFromMappingTests.cs |
Updates assertion to verify exact SecureTls mode instead of just non-null |
tests/Valkey.Glide.IntegrationTests/TlsTests.cs |
Adds integration tests for cluster and standalone insecure TLS connections |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
…Tls` and `UseInsecureTls` property setters. Signed-off-by: currantw <[email protected]>
…at TLS much be enabled first. Signed-off-by: currantw <[email protected]>
xShinnRyuu
left a comment
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
…ponding parameter on the Rust side is type Option<TlsMode>, so it is needed for Rust to parse the value and causes alignment issues on the Rust side when omitted. Signed-off-by: currantw <[email protected]>
…er_WithInsecureTls_NoTlsServerThrows` to `TlsTests`. Signed-off-by: currantw <[email protected]>
xShinnRyuu
left a comment
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 again
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Summary
Fixes #163: Adds support for insecure TLS mode.
Changes
ConnectionConfiguration.UseInsecureTlsproperty: Boolean property with validation.ConnectionConfiguration.WithInsecureTls()method: builder for enabling/disabling insecure mode.ConnectionConfiguration.UseTlsproperty: Now handles bothSecureTlsandInsecureTls.Testing
✅ All tests pass on .NET 8.0.
See Also
⚪ None