Skip to content

Commit 882c866

Browse files
authored
Implement tri-state disabled field for TLS configuration (#1002)
* Implement tri-state disabled field for TLS configuration Change TLS disabled field from bool to Option<bool> to support three states: - None: TLS behavior depends on other factors (API key, etc.) - Some(false): TLS explicitly enabled - Some(true): TLS explicitly disabled This fixes TOML serialization asymmetry where "not set" vs "explicitly false" could not be distinguished, ensuring proper round-trip compatibility. * formatting * linting
1 parent 042372d commit 882c866

1 file changed

Lines changed: 124 additions & 13 deletions

File tree

core-api/src/envconfig.rs

Lines changed: 124 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,9 @@ pub struct ClientConfigProfile {
138138
/// ClientConfigTLS is TLS configuration for a client.
139139
#[derive(Debug, Clone, PartialEq, Default)]
140140
pub struct ClientConfigTLS {
141-
/// If true, TLS is explicitly disabled. If false/unset, whether TLS is enabled or not depends on other factors such
142-
/// as whether this struct is present or None, and whether API key exists (which enables TLS by default).
143-
pub disabled: bool,
141+
/// If Some(true), TLS is explicitly disabled. If Some(false), TLS is explicitly enabled.
142+
/// If None, TLS behavior depends on other factors (API key presence, etc.)
143+
pub disabled: Option<bool>,
144144

145145
/// Client certificate source.
146146
pub client_cert: Option<DataSource>,
@@ -446,7 +446,7 @@ impl ClientConfigProfile {
446446
if let Some(disabled_str) = env_provider.get("TEMPORAL_TLS")?
447447
&& let Some(disabled) = env_var_to_bool(&disabled_str)
448448
{
449-
tls.disabled = !disabled;
449+
tls.disabled = Some(!disabled);
450450
}
451451

452452
apply_data_source_env_var(
@@ -729,8 +729,8 @@ impl TomlClientConfigProfile {
729729

730730
#[derive(Debug, Clone, Serialize, Deserialize)]
731731
struct TomlClientConfigTLS {
732-
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
733-
disabled: bool,
732+
#[serde(default, skip_serializing_if = "Option::is_none")]
733+
disabled: Option<bool>,
734734

735735
#[serde(skip_serializing_if = "Option::is_none")]
736736
client_cert_path: Option<String>,
@@ -760,7 +760,7 @@ struct TomlClientConfigTLS {
760760
impl TomlClientConfigTLS {
761761
fn new() -> Self {
762762
Self {
763-
disabled: false,
763+
disabled: None,
764764
client_cert_path: None,
765765
client_cert_data: None,
766766
client_key_path: None,
@@ -951,7 +951,7 @@ mod strict {
951951
#[serde(deny_unknown_fields)]
952952
struct StrictTomlClientConfigTLS {
953953
#[serde(default)]
954-
disabled: bool,
954+
disabled: Option<bool>,
955955
#[serde(default)]
956956
client_cert_path: Option<String>,
957957
#[serde(default)]
@@ -1068,7 +1068,7 @@ namespace = "production"
10681068
assert_eq!(default_profile.api_key.as_ref().unwrap(), "test-key");
10691069

10701070
let tls = default_profile.tls.as_ref().unwrap();
1071-
assert!(!tls.disabled);
1071+
assert_eq!(tls.disabled, Some(false)); // Explicitly set to false
10721072
assert_eq!(
10731073
tls.client_cert,
10741074
Some(DataSource::Path("/path/to/cert".to_string()))
@@ -1244,7 +1244,7 @@ some-other-header = "some-value2"
12441244
assert_eq!(profile.api_key.as_ref().unwrap(), "my-api-key-new");
12451245

12461246
let tls = profile.tls.as_ref().unwrap();
1247-
assert!(!tls.disabled); // TLS enabled via env var
1247+
assert_eq!(tls.disabled, Some(false)); // TLS enabled via env var
12481248
assert_eq!(
12491249
tls.client_cert,
12501250
Some(DataSource::Path("my-client-cert-path-new".to_string()))
@@ -1313,7 +1313,7 @@ sOme-hEader_key = "some-value"
13131313
assert_eq!(codec.auth.as_ref().unwrap(), "my-auth");
13141314

13151315
let tls = profile.tls.as_ref().unwrap();
1316-
assert!(tls.disabled);
1316+
assert_eq!(tls.disabled, Some(true)); // Explicitly disabled
13171317
assert_eq!(
13181318
tls.client_cert,
13191319
Some(DataSource::Path("my-client-cert-path".to_string()))
@@ -1361,7 +1361,7 @@ api_key = "my-api-key"
13611361
assert!(profile.tls.is_some());
13621362

13631363
let tls = profile.tls.as_ref().unwrap();
1364-
assert!(!tls.disabled); // default value
1364+
assert_eq!(tls.disabled, None); // Not explicitly set // default value
13651365
assert!(tls.client_cert.is_none());
13661366
}
13671367

@@ -1600,7 +1600,7 @@ api_key = "my-api-key"
16001600
// TLS should be enabled due to API key presence
16011601
assert!(profile.tls.is_some());
16021602
let tls = profile.tls.as_ref().unwrap();
1603-
assert!(!tls.disabled);
1603+
assert_eq!(tls.disabled, None); // Not explicitly set
16041604
}
16051605

16061606
#[test]
@@ -1646,4 +1646,115 @@ address = "some-address"
16461646
std::env::remove_var("TEMPORAL_NAMESPACE");
16471647
}
16481648
}
1649+
1650+
#[test]
1651+
fn test_tls_disabled_tri_state_behavior() {
1652+
// Test 1: disabled = None (unset) - should not appear in TOML
1653+
let tls_unset = ClientConfigTLS {
1654+
disabled: None,
1655+
..Default::default()
1656+
};
1657+
let profile_unset = ClientConfigProfile {
1658+
address: Some("localhost:7233".to_string()),
1659+
tls: Some(tls_unset),
1660+
..Default::default()
1661+
};
1662+
let mut config_unset = ClientConfig::default();
1663+
config_unset
1664+
.profiles
1665+
.insert("test".to_string(), profile_unset);
1666+
1667+
let toml_unset = config_unset.to_toml().unwrap();
1668+
let toml_str_unset = String::from_utf8(toml_unset).unwrap();
1669+
1670+
// Unset disabled should not appear in TOML output
1671+
assert!(!toml_str_unset.contains("disabled"));
1672+
1673+
// Round-trip test - should remain None
1674+
let parsed_unset =
1675+
ClientConfig::from_toml(toml_str_unset.as_bytes(), Default::default()).unwrap();
1676+
assert_eq!(
1677+
parsed_unset
1678+
.profiles
1679+
.get("test")
1680+
.unwrap()
1681+
.tls
1682+
.as_ref()
1683+
.unwrap()
1684+
.disabled,
1685+
None
1686+
);
1687+
1688+
// Test 2: disabled = Some(false) (explicitly enabled) - should appear in TOML as false
1689+
let tls_enabled = ClientConfigTLS {
1690+
disabled: Some(false),
1691+
..Default::default()
1692+
};
1693+
let profile_enabled = ClientConfigProfile {
1694+
address: Some("localhost:7233".to_string()),
1695+
tls: Some(tls_enabled),
1696+
..Default::default()
1697+
};
1698+
let mut config_enabled = ClientConfig::default();
1699+
config_enabled
1700+
.profiles
1701+
.insert("test".to_string(), profile_enabled);
1702+
1703+
let toml_enabled = config_enabled.to_toml().unwrap();
1704+
let toml_str_enabled = String::from_utf8(toml_enabled).unwrap();
1705+
1706+
// Explicitly disabled=false should appear in TOML
1707+
assert!(toml_str_enabled.contains("disabled = false"));
1708+
1709+
// Round-trip test - should remain Some(false)
1710+
let parsed_enabled =
1711+
ClientConfig::from_toml(toml_str_enabled.as_bytes(), Default::default()).unwrap();
1712+
assert_eq!(
1713+
parsed_enabled
1714+
.profiles
1715+
.get("test")
1716+
.unwrap()
1717+
.tls
1718+
.as_ref()
1719+
.unwrap()
1720+
.disabled,
1721+
Some(false)
1722+
);
1723+
1724+
// Test 3: disabled = Some(true) (explicitly disabled) - should appear in TOML as true
1725+
let tls_disabled = ClientConfigTLS {
1726+
disabled: Some(true),
1727+
..Default::default()
1728+
};
1729+
let profile_disabled = ClientConfigProfile {
1730+
address: Some("localhost:7233".to_string()),
1731+
tls: Some(tls_disabled),
1732+
..Default::default()
1733+
};
1734+
let mut config_disabled = ClientConfig::default();
1735+
config_disabled
1736+
.profiles
1737+
.insert("test".to_string(), profile_disabled);
1738+
1739+
let toml_disabled = config_disabled.to_toml().unwrap();
1740+
let toml_str_disabled = String::from_utf8(toml_disabled).unwrap();
1741+
1742+
// Explicitly disabled=true should appear in TOML
1743+
assert!(toml_str_disabled.contains("disabled = true"));
1744+
1745+
// Round-trip test - should remain Some(true)
1746+
let parsed_disabled =
1747+
ClientConfig::from_toml(toml_str_disabled.as_bytes(), Default::default()).unwrap();
1748+
assert_eq!(
1749+
parsed_disabled
1750+
.profiles
1751+
.get("test")
1752+
.unwrap()
1753+
.tls
1754+
.as_ref()
1755+
.unwrap()
1756+
.disabled,
1757+
Some(true)
1758+
);
1759+
}
16491760
}

0 commit comments

Comments
 (0)