Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 12 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ jobs:
- name: Confirm bench works
run: dotnet run --project tests/Temporalio.SimpleBench/Temporalio.SimpleBench.csproj -- --workflow-count 5 --max-cached-workflows 100 --max-concurrent 100

- name: Test cloud
# Only supported in non-fork runs, since secrets are not available in forks
- name: Test cloud (mTLS)
# Only supported in non-fork runs, since secrets are not available in forks.
if: ${{ matrix.cloudTestTarget && (github.event.pull_request.head.repo.full_name == '' || github.event.pull_request.head.repo.full_name == 'temporalio/sdk-dotnet') }}
env:
TEMPORAL_TEST_CLIENT_TARGET_HOST: ${{ vars.TEMPORAL_CLIENT_NAMESPACE }}.tmprl.cloud:7233
Expand All @@ -108,6 +108,16 @@ jobs:
TEMPORAL_TEST_CLIENT_KEY: ${{ secrets.TEMPORAL_CLIENT_KEY }}
run: dotnet run --project tests/Temporalio.Tests -- -verbose -method "*.ExecuteWorkflowAsync_Simple_Succeeds"

- name: Test cloud (API key)
# Only supported in non-fork runs, since secrets are not available in forks.
# Uses API key auth to test auto-TLS feature (TLS auto-enabled when API key provided).
if: ${{ matrix.cloudTestTarget && (github.event.pull_request.head.repo.full_name == '' || github.event.pull_request.head.repo.full_name == 'temporalio/sdk-dotnet') }}
env:
TEMPORAL_TEST_CLIENT_TARGET_HOST: us-west-2.aws.api.temporal.io:7233
TEMPORAL_TEST_CLIENT_NAMESPACE: ${{ vars.TEMPORAL_CLIENT_NAMESPACE }}
TEMPORAL_CLIENT_CLOUD_API_KEY: ${{ secrets.TEMPORAL_CLIENT_CLOUD_API_KEY }}
run: dotnet run --project tests/Temporalio.Tests -- -verbose -method "*.ExecuteWorkflowAsync_Simple_Succeeds"

- name: Test cloud operations client
# Only supported in non-fork runs, since secrets are not available in forks
if: ${{ matrix.cloudTestTarget && (github.event.pull_request.head.repo.full_name == '' || github.event.pull_request.head.repo.full_name == 'temporalio/sdk-dotnet') }}
Expand Down
16 changes: 14 additions & 2 deletions src/Temporalio/Bridge/OptionsExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,19 @@ public static unsafe Interop.TemporalCoreClientOptions ToInteropOptions(
{
throw new ArgumentException("Identity missing from options.");
}
var scheme = options.Tls == null ? "http" : "https";

// Auto-enable TLS when API key is provided and TLS is not explicitly disabled
var tls = options.Tls;
if (tls?.Disabled == true)
{
tls = null;
}
else if (tls == null && !string.IsNullOrEmpty(options.ApiKey))
{
tls = new Temporalio.Client.TlsOptions();
}

var scheme = tls == null ? "http" : "https";
return new Interop.TemporalCoreClientOptions()
{
target_url = scope.ByteArray($"{scheme}://{options.TargetHost}"),
Expand All @@ -251,7 +263,7 @@ public static unsafe Interop.TemporalCoreClientOptions ToInteropOptions(
api_key = scope.ByteArray(options.ApiKey),
identity = scope.ByteArray(options.Identity),
tls_options =
options.Tls == null ? null : scope.Pointer(options.Tls.ToInteropOptions(scope)),
tls == null ? null : scope.Pointer(tls.ToInteropOptions(scope)),
retry_options =
options.RpcRetry == null
? null
Expand Down
4 changes: 3 additions & 1 deletion src/Temporalio/Client/TemporalConnectionOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ public TemporalConnectionOptions()
/// Gets or sets the TLS options for connection.
/// </summary>
/// <remarks>
/// This must be set, even to a default instance, to do any TLS connection.
/// This must be set, even to a default instance, to do any TLS connection. If not set and
/// <see cref="ApiKey"/> is provided, TLS will be automatically enabled with default options.
/// To explicitly disable TLS, set instance with <see cref="TlsOptions.Disabled"/> set to true.
/// </remarks>
public TlsOptions? Tls { get; set; }

Expand Down
9 changes: 9 additions & 0 deletions src/Temporalio/Client/TlsOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ public class TlsOptions : ICloneable
/// </remarks>
public byte[]? ClientPrivateKey { get; set; }

/// <summary>
/// Gets or sets a value indicating whether TLS should be explicitly disabled.
/// </summary>
/// <remarks>
/// When set to true, TLS will be disabled even when an API key is provided (which normally
/// auto-enables TLS).
/// </remarks>
public bool Disabled { get; set; }

/// <summary>
/// Create a shallow copy of these options.
/// </summary>
Expand Down
72 changes: 72 additions & 0 deletions tests/Temporalio.Tests/Client/TemporalConnectionOptionsTests.cs
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.

Hrmm, these tests may be bit too detailed/low-level. You are testing internals and only relying on us making internals visible to this test project. Note we don't have any other tests confirming the bridge options conversions. Usually we test user-facing behavior.

How would a user write a test to confirm their setting of TLS disabled or API key or whatever does the right thing? Granted it's probably not very easy to do this. One approach that you may take is to alter the "Test cloud" CI test to use API key instead of mTLS to at least confirm the API key default.

Regardless, not a big deal if you keep these, just a bit strange for this repo (and why there never was anything like this in the past).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably going to keep them. Changed the cloud CI workflow to use api key secret and remove the mTLS secrets

Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
namespace Temporalio.Tests.Client;

using Temporalio.Bridge;
using Temporalio.Client;
using Xunit;

public unsafe class TemporalConnectionOptionsTests
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add tests for when:

  1. ApiKey is not set, is null, or is empty?
  2. Tls is set to a non-null value?

{
[Fact]
public void ToInteropOptions_AutoEnablesTls_WhenApiKeyProvidedAndTlsNotSet()
{
var options = new TemporalConnectionOptions("localhost:7233")
{
ApiKey = "test-api-key",
Identity = "test-identity",
};

using var scope = new Scope();
var interopOptions = options.ToInteropOptions(scope);

// TLS should be auto-enabled when API key is provided and TLS not set
Assert.True(interopOptions.tls_options != null);
}

[Fact]
public void ToInteropOptions_TlsDisabled_WhenExplicitlyDisabledWithApiKey()
{
var options = new TemporalConnectionOptions("localhost:7233")
{
ApiKey = "test-api-key",
Identity = "test-identity",
Tls = new TlsOptions { Disabled = true },
};

using var scope = new Scope();
var interopOptions = options.ToInteropOptions(scope);

// TLS should remain disabled when explicitly disabled, even with API key
Assert.True(interopOptions.tls_options == null);
}

[Fact]
public void ToInteropOptions_TlsDisabled_WhenNoApiKeyAndTlsNotSet()
{
var options = new TemporalConnectionOptions("localhost:7233")
{
Identity = "test-identity",
};

using var scope = new Scope();
var interopOptions = options.ToInteropOptions(scope);

// TLS should be disabled when no API key and TLS not set
Assert.True(interopOptions.tls_options == null);
}

[Fact]
public void ToInteropOptions_TlsEnabled_WhenExplicitlySet()
{
var options = new TemporalConnectionOptions("localhost:7233")
{
Identity = "test-identity",
Tls = new TlsOptions(),
};

using var scope = new Scope();
var interopOptions = options.ToInteropOptions(scope);

// TLS should be enabled when explicitly set
Assert.True(interopOptions.tls_options != null);
}
}
1 change: 1 addition & 0 deletions tests/Temporalio.Tests/Temporalio.Tests.csproj
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
<GenerateProgramFile>false</GenerateProgramFile>
<ImplicitUsings>enable</ImplicitUsings>
Expand Down
8 changes: 7 additions & 1 deletion tests/Temporalio.Tests/WorkflowEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ public async Task InitializeAsync()
};
var clientCert = Environment.GetEnvironmentVariable("TEMPORAL_TEST_CLIENT_CERT");
var clientKey = Environment.GetEnvironmentVariable("TEMPORAL_TEST_CLIENT_KEY");
if ((clientCert == null) != (clientKey == null))
var apiKey = Environment.GetEnvironmentVariable("TEMPORAL_CLIENT_CLOUD_API_KEY");
if (clientCert == null != (clientKey == null))
{
throw new InvalidOperationException("Must have both cert/key or neither");
}
Expand All @@ -52,6 +53,11 @@ public async Task InitializeAsync()
ClientPrivateKey = System.Text.Encoding.ASCII.GetBytes(clientKey),
};
}
if (apiKey != null)
{
// API key auto-enables TLS when Tls is not set
options.ApiKey = apiKey;
}
env = new(await TemporalClient.ConnectAsync(options));
}
else
Expand Down
Loading