Skip to content

Conversation

markphelps
Copy link
Contributor

Summary

  • Add comprehensive TLS configuration options to the Ruby client SDK, matching the functionality from Python and Java implementations
  • Add TlsConfig class with Ruby-idiomatic patterns (keyword arguments, attr_reader, factory methods)
  • Support CA certificates (file paths and raw data), client certificates for mutual TLS, and insecure mode for development
  • Update integration test framework to enable HTTPS testing for Ruby SDK with TLS certificates
  • Update Ruby tests to handle HTTPS URLs with TLS configuration automatically like Python/Java
  • Add comprehensive README documentation with Ruby-specific TLS examples

Key Features

  • Ruby-idiomatic design: Uses keyword arguments, attr_reader, and factory methods
  • File validation: Automatically validates certificate file existence
  • Convenience methods: TlsConfig.insecure, TlsConfig.with_ca_cert_file, TlsConfig.with_mutual_tls, etc.
  • Data precedence: Raw certificate data takes precedence over file paths
  • JSON serialization: Proper to_h method for FFI layer integration
  • Test integration: Updates integration test framework to use HTTPS with TLS certificates

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]>
@markphelps markphelps requested a review from a team as a code owner June 29, 2025 20:38
@markphelps markphelps changed the base branch from main to feat/java-tls-config June 29, 2025 20:38
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jun 29, 2025
@codecov-commenter
Copy link

codecov-commenter commented Jun 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.31%. Comparing base (f701007) to head (76221eb).
Report is 434 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- 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]>
@markphelps markphelps force-pushed the feat/ruby-tls-config branch from 4db89b5 to 747cd91 Compare June 29, 2025 20:42
@dosubot dosubot bot added documentation Improvements or additions to documentation enhancement New feature or request labels Jun 29, 2025
@markphelps markphelps requested review from erka and Copilot June 29, 2025 20:58
Copy link
Contributor

@Copilot Copilot AI left a 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 a tls_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 to opts when non-nil to avoid serializing a null value in JSON. For example, merge tls_config: tls_hash into opts only if tls_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 verify to_h output, file validation errors, and data-vs-file precedence.
  class TlsConfig

Base automatically changed from feat/java-tls-config to main June 29, 2025 23:01
Copy link
Collaborator

@erka erka left a comment

Choose a reason for hiding this comment

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

nice

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 29, 2025
@kodiakhq kodiakhq bot merged commit 143e715 into main Jun 29, 2025
22 checks passed
@kodiakhq kodiakhq bot deleted the feat/ruby-tls-config branch June 29, 2025 23:09
markphelps added a commit that referenced this pull request Jun 30, 2025
* '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)
markphelps added a commit that referenced this pull request Jun 30, 2025
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge documentation Improvements or additions to documentation enhancement New feature or request lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants