Skip to content

💥 enable TLS if api key provided #366

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

Merged
THardy98 merged 5 commits into from
Dec 4, 2025
Merged

💥 enable TLS if api key provided #366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
60f9769
enable TLS if api key provided
THardy98 Dec 2, 2025
a1a4a61
fix
THardy98 Dec 2, 2025
b9c418a
update sig files, push tls/api key check to bridge options construction
THardy98 Dec 3, 2025
22b9ed8
change cloud tests to use api key to enable TLS
THardy98 Dec 3, 2025
9f09c3a
Merge branch 'main' into enable-tls-if-api-key
THardy98 Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion temporalio/lib/temporalio/client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def self.connect(
target_host,
namespace,
api_key: nil,
tls: false,
tls: nil,
Copy link
Member

@cretz cretz Dec 4, 2025

Choose a reason for hiding this comment

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

Should also update the YARD docs above the method for this param (and explain the runtime default)

Copy link
Contributor Author

@THardy98 THardy98 Dec 4, 2025

Choose a reason for hiding this comment

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

I'm not seeing this diff

Copy link
Member

@cretz cretz Dec 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Above, there is:

    # @param tls [Boolean, Connection::TLSOptions] If false, do not use TLS. If true, use system default TLS options. If
    #   TLS options are present, those TLS options will be used.

It would change to:

      # @param tls [Boolean, Connection::TLSOptions, nil] If false, do not use TLS. If true, use system default TLS options. If TLS
      #   options are present, those TLS options will be used. If nil (the default), TLS will be auto-enabled if
      #   api_key is provided.

Same as you did in the connection file. But not a big deal.

data_converter: Converters::DataConverter.default,
interceptors: [],
logger: Logger.new($stdout, level: Logger::WARN),
Expand Down
23 changes: 14 additions & 9 deletions temporalio/lib/temporalio/client/connection.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,9 @@ class HTTPConnectProxyOptions; end # rubocop:disable Lint/EmptyClass
# +localhost:7233+.
# @param api_key [String, nil] API key for Temporal. This becomes the +Authorization+ HTTP header with +"Bearer "+
# prepended. This is only set if RPC metadata doesn't already have an +authorization+ key.
# @param tls [Boolean, TLSOptions] If false, do not use TLS. If true, use system default TLS options. If TLS
# options are present, those TLS options will be used.
# @param tls [Boolean, TLSOptions, nil] If false, do not use TLS. If true, use system default TLS options. If TLS
# options are present, those TLS options will be used. If nil (the default), TLS will be auto-enabled if
# api_key is provided.
# @param rpc_metadata [Hash<String, String>] Headers to use for all calls to the server. Keys here can be
# overriden by per-call RPC metadata keys.
# @param rpc_retry [RPCRetryOptions] Retry options for direct service calls (when opted in) or all high-level
Expand All @@ -173,7 +174,7 @@ class HTTPConnectProxyOptions; end # rubocop:disable Lint/EmptyClass
def initialize(
target_host:,
api_key: nil,
tls: false,
tls: nil,
Copy link
Member

@cretz cretz Dec 3, 2025

Choose a reason for hiding this comment

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

A bit surprised/disappointed this didn't require that you change sig/temporalio/client/connection.rbs (same for client.rbs). I would have expected a bundle exec rake steep call to fail, strange. Anyways, can we update those two sig files to support nil?

Copy link
Contributor Author

@THardy98 THardy98 Dec 3, 2025

Choose a reason for hiding this comment

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

done

rpc_metadata: {},
rpc_retry: RPCRetryOptions.new,
identity: "#{Process.pid}@#{Socket.gethostname}",
Expand Down Expand Up @@ -285,13 +286,17 @@ def new_core_client
),
identity: @options.identity || "#{Process.pid}@#{Socket.gethostname}"
)
if @options.tls
options.tls = if @options.tls.is_a?(TLSOptions)
# Auto-enable TLS when API key is provided and tls not explicitly set
tls = @options.tls
tls = true if tls.nil? && @options.api_key

if tls
Comment on lines +290 to +293
Copy link
Member

@cretz cretz Dec 4, 2025

Choose a reason for hiding this comment

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

Suggested change
tls = @options.tls
tls = true if tls.nil? && @options.api_key
if tls
if @options.tls || (@options.tls.nil? && @options.api_key)

Reads a bit easier to me, but up to you

options.tls = if tls.is_a?(TLSOptions)
Internal::Bridge::Client::TLSOptions.new(
client_cert: @options.tls.client_cert, # steep:ignore
client_private_key: @options.tls.client_private_key, # steep:ignore
server_root_ca_cert: @options.tls.server_root_ca_cert, # steep:ignore
domain: @options.tls.domain # steep:ignore
client_cert: tls.client_cert, # steep:ignore
client_private_key: tls.client_private_key, # steep:ignore
server_root_ca_cert: tls.server_root_ca_cert, # steep:ignore
domain: tls.domain # steep:ignore
)
else
Internal::Bridge::Client::TLSOptions.new
Expand Down
2 changes: 1 addition & 1 deletion temporalio/sig/temporalio/client.rbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ module Temporalio
String target_host,
String namespace,
?api_key: String?,
?tls: bool | Connection::TLSOptions,
?tls: bool | Connection::TLSOptions | nil,
?data_converter: Converters::DataConverter,
?interceptors: Array[Interceptor],
?logger: Logger,
Expand Down
6 changes: 3 additions & 3 deletions temporalio/sig/temporalio/client/connection.rbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module Temporalio
class Options
attr_reader target_host: String
attr_reader api_key: String?
attr_reader tls: bool | Connection::TLSOptions
attr_reader tls: bool | Connection::TLSOptions | nil
attr_reader rpc_metadata: Hash[String, String]
attr_reader rpc_retry: RPCRetryOptions
attr_reader identity: String
Expand All @@ -16,7 +16,7 @@ module Temporalio
def initialize: (
target_host: String,
api_key: String?,
tls: bool | Connection::TLSOptions,
tls: bool | Connection::TLSOptions | nil,
rpc_metadata: Hash[String, String],
rpc_retry: RPCRetryOptions,
identity: String,
Expand Down Expand Up @@ -98,7 +98,7 @@ module Temporalio
def initialize: (
target_host: String,
?api_key: String?,
?tls: bool | Connection::TLSOptions,
?tls: bool | Connection::TLSOptions | nil,
?rpc_metadata: Hash[String, String],
?rpc_retry: RPCRetryOptions,
?identity: String,
Expand Down
7 changes: 4 additions & 3 deletions temporalio/test/client_cloud_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,15 @@ def test_mtls
end

def test_api_key
# This test validates the auto-TLS feature: TLS is auto-enabled when api_key is provided
# and tls is not explicitly set.
api_key = ENV.fetch('TEMPORAL_CLOUD_API_KEY_TEST_API_KEY', '')
skip('No cloud API key') if api_key.empty?

client = Temporalio::Client.connect(
ENV.fetch('TEMPORAL_CLOUD_API_KEY_TEST_TARGET_HOST'),
ENV.fetch('TEMPORAL_CLOUD_API_KEY_TEST_NAMESPACE'),
api_key:,
tls: true,
rpc_metadata: { 'temporal-namespace' => ENV.fetch('TEMPORAL_CLOUD_API_KEY_TEST_NAMESPACE') }
)
# Run workflow
Expand All @@ -52,14 +53,14 @@ def test_api_key
end

def test_cloud_ops
# This test also validates auto-TLS: TLS is auto-enabled when api_key is provided.
api_key = ENV.fetch('TEMPORAL_CLOUD_OPS_TEST_API_KEY', '')
skip('No cloud API key') if api_key.empty?

# Create connection
# Create connection (tls not set, auto-enabled due to api_key)
conn = Temporalio::Client::Connection.new(
target_host: ENV.fetch('TEMPORAL_CLOUD_OPS_TEST_TARGET_HOST'),
api_key:,
tls: true,
rpc_metadata: { 'temporal-cloud-api-version' => ENV.fetch('TEMPORAL_CLOUD_OPS_TEST_API_VERSION') }
)

Expand Down
Loading