-
Notifications
You must be signed in to change notification settings - Fork 19
feat: add TLS configuration support to Ruby SDK #1136
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: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Change TlsConfig from Lombok @value to traditional class structure to match ClientOptions pattern and ensure proper JSON serialization Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1136 +/- ##
==========================================
- Coverage 80.36% 75.31% -5.05%
==========================================
Files 8 9 +1
Lines 4165 4477 +312
==========================================
+ Hits 3347 3372 +25
- Misses 818 1105 +287 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Add TlsConfig class with comprehensive TLS options matching Python/Java implementations - Support CA certificates, client certificates for mutual TLS, and insecure mode - Add convenience factory methods for common TLS scenarios - Update integration test framework to use HTTPS with TLS certificates for Ruby tests - Add comprehensive README documentation with TLS configuration examples Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
4db89b5
to
747cd91
Compare
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
Adds TLS configuration support to the Ruby SDK, aligning behavior with the Python and Java clients.
- Updates the integration test framework to run Ruby tests over HTTPS with CA fixtures
- Introduces
TlsConfig
for configuring CA certificates, mutual TLS, and insecure mode - Extends
Flipt::Client
to accept and validate atls_config
option and updates README accordingly
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
test/main.go | Enable HTTPS in Ruby containers; mount TLS fixtures and set CA env var |
flipt-client-ruby/spec/client_spec.rb | Wraps client instantiation in conditional TLS setup |
flipt-client-ruby/lib/flipt_client/models.rb | Adds TlsConfig class with factory methods, validation, and serialization |
flipt-client-ruby/lib/flipt_client.rb | Adds :tls_config option, validation method, and YARD docs |
flipt-client-ruby/README.md | Documents tls_config usage with examples and options |
Comments suppressed due to low confidence (3)
flipt-client-ruby/lib/flipt_client.rb:75
- Consider only adding the
:tls_config
key toopts
when non-nil to avoid serializing a null value in JSON. For example, mergetls_config: tls_hash
intoopts
only iftls_hash
is present.
opts[:tls_config] = validate_tls_config(opts.fetch(:tls_config, nil))
flipt-client-ruby/spec/client_spec.rb:8
- [nitpick] The initial client instantiation is immediately overridden when you set up TLS. You can simplify by removing this first call and including
tls_config
in the single constructor invocation.
auth_token = ENV.fetch('FLIPT_AUTH_TOKEN', 'secret')
flipt-client-ruby/lib/flipt_client/models.rb:45
- There are no unit tests for
TlsConfig
itself. Consider adding specs to verifyto_h
output, file validation errors, and data-vs-file precedence.
class TlsConfig
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.
nice
* 'main' of https://github.com/flipt-io/flipt-client-sdks: feat: add TLS configuration support to Ruby SDK (#1136) feat: add TLS configuration support to Java SDK (#1135)
* feat: add TLS configuration support to Java SDK Signed-off-by: Mark Phelps <[email protected]> * chore: fmt java Signed-off-by: Mark Phelps <[email protected]> * chore: update java tests to use https Signed-off-by: Mark Phelps <[email protected]> * chore: update java test params for https Signed-off-by: Mark Phelps <[email protected]> * chore: add debug info for failing tests Signed-off-by: Mark Phelps <[email protected]> * fix: TlsConfig serialization compatibility with Rust FFI Change TlsConfig from Lombok @value to traditional class structure to match ClientOptions pattern and ensure proper JSON serialization Signed-off-by: Mark Phelps <[email protected]> * chore: fmt Signed-off-by: Mark Phelps <[email protected]> * chore: rm debug env vars in java tests Signed-off-by: Mark Phelps <[email protected]> * chore: mount tls file Signed-off-by: Mark Phelps <[email protected]> * chore: add default for fetcher builder Signed-off-by: Mark Phelps <[email protected]> * chore: check for file existence in builder Signed-off-by: Mark Phelps <[email protected]> * chore: add file existence checks for python too Signed-off-by: Mark Phelps <[email protected]> * chore: fmt Signed-off-by: Mark Phelps <[email protected]> * chore: default to 120 seconds on java builder Signed-off-by: Mark Phelps <[email protected]> * chore: only set request_timeout and update_interval in fetcher if > 0 Signed-off-by: Mark Phelps <[email protected]> * feat: add TLS configuration support to Ruby SDK and enable HTTPS testing - Add TlsConfig class with comprehensive TLS options matching Python/Java implementations - Support CA certificates, client certificates for mutual TLS, and insecure mode - Add convenience factory methods for common TLS scenarios - Update integration test framework to use HTTPS with TLS certificates for Ruby tests - Add comprehensive README documentation with TLS configuration examples Signed-off-by: Mark Phelps <[email protected]> * chore: rubocop fmt Signed-off-by: Mark Phelps <[email protected]> --------- Signed-off-by: Mark Phelps <[email protected]>
Summary
Key Features
TlsConfig.insecure
,TlsConfig.with_ca_cert_file
,TlsConfig.with_mutual_tls
, etc.to_h
method for FFI layer integration