gsadmin/rustfs
1
0
Fork 0
mirror of https://github.com/rustfs/rustfs.git synced 2026-07-28 17:18:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(targets): keep pulsar target online after restart without TLS (#4798)

Browse Source 
The pulsar config validation rejected any non-`pulsar+ssl` broker whenever
`tls_allow_insecure` was set or `tls_hostname_verification` was disabled.
Both toggles are inert on a plaintext `pulsar://` broker — the Pulsar client
only applies them for `pulsar+ssl` — so treating a non-default value as fatal
is wrong.

This surfaced as issue #4796: the console persists `tls_hostname_verification`
as `false` (the checkbox defaults to unchecked while the server default is
`on`). At creation time the value is not present in the unmerged request and
falls back to the safe default, so the connectivity check passes and the
target comes online. On restart the persisted config is merged and validated,
the `false` is read back, and the target is rejected — permanently offline
even though Pulsar is reachable.

Relax both the loader-side (`validate_pulsar_broker_config`) and runtime-side
(`PulsarArgs::validate`) checks so only a `tls_ca` bundle — real TLS trust
material that is never defaulted to a non-empty value — requires a
`pulsar+ssl` scheme. The inert toggles no longer fail the target. This also
heals configs already persisted with the bad value. Add regression coverage
for both paths.

Co-authored-by: heihutu <heihutu@gmail.com>
This commit is contained in:
houseme
2026-07-14 00:07:08 +08:00
committed by GitHub
parent 0f83a27f6a
commit 27a7cc739e
2 changed files with 61 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/targets/src/config/common.rs
+29 -9
View File
@@ -19,7 +19,7 @@ use rustfs_config::server_config::KVS;
use rustfs_config::{
DEFAULT_DELIMITER, ENABLE_KEY, EnableState, NATS_CREDENTIALS_FILE, NATS_PASSWORD, NATS_QUEUE_DIR, NATS_SUBJECT, NATS_TLS_CA,
NATS_TLS_CLIENT_CERT, NATS_TLS_CLIENT_KEY, NATS_TOKEN, NATS_USERNAME, PULSAR_AUTH_TOKEN, PULSAR_PASSWORD, PULSAR_QUEUE_DIR,
PULSAR_TLS_ALLOW_INSECURE, PULSAR_TLS_CA, PULSAR_TLS_HOSTNAME_VERIFICATION, PULSAR_TOPIC, PULSAR_USERNAME,
PULSAR_TLS_CA, PULSAR_TOPIC, PULSAR_USERNAME,
};
use rustfs_utils::egress::validate_outbound_url;
use std::collections::HashSet;
@@ -156,15 +156,20 @@ pub(super) fn validate_pulsar_broker_config(broker: &str, config: &KVS, default_
}
let tls_ca = config.lookup(PULSAR_TLS_CA).unwrap_or_default();
let tls_allow_insecure = parse_target_bool(config.lookup(PULSAR_TLS_ALLOW_INSECURE).as_deref()).unwrap_or(false);
let tls_hostname_verification = parse_target_bool(config.lookup(PULSAR_TLS_HOSTNAME_VERIFICATION).as_deref()).unwrap_or(true);
if !tls_ca.is_empty() && !Path::new(&tls_ca).is_absolute() {
return Err(TargetError::Configuration("Pulsar tls_ca must be an absolute path".to_string()));
}
if url.scheme() != "pulsar+ssl" && (!tls_ca.is_empty() || tls_allow_insecure || !tls_hostname_verification) {
// A CA bundle is TLS trust material a plaintext `pulsar://` broker silently
// ignores, so treat it as a genuine misconfiguration. The
// `tls_allow_insecure` / `tls_hostname_verification` toggles, however, are
// inert on a non-TLS broker (the Pulsar client only honours them for
// `pulsar+ssl`); rejecting non-default values would leave a persisted
// target permanently offline after a restart even though the flags do
// nothing — see issue #4796.
if url.scheme() != "pulsar+ssl" && !tls_ca.is_empty() {
return Err(TargetError::Configuration(
"Pulsar TLS settings are only allowed with pulsar+ssl brokers".to_string(),
"Pulsar tls_ca is only allowed with pulsar+ssl brokers".to_string(),
));
}
@@ -192,7 +197,8 @@ mod tests {
use async_nats::ServerAddr;
use rustfs_config::server_config::KVS;
use rustfs_config::{
NATS_PASSWORD, NATS_QUEUE_DIR, NATS_SUBJECT, NATS_TOKEN, NATS_USERNAME, PULSAR_TLS_ALLOW_INSECURE, PULSAR_TOPIC,
NATS_PASSWORD, NATS_QUEUE_DIR, NATS_SUBJECT, NATS_TOKEN, NATS_USERNAME, PULSAR_TLS_ALLOW_INSECURE, PULSAR_TLS_CA,
PULSAR_TLS_HOSTNAME_VERIFICATION, PULSAR_TOPIC,
};
use std::str::FromStr;
@@ -223,14 +229,28 @@ mod tests {
}
#[test]
fn validate_pulsar_broker_config_rejects_tls_flags_without_tls_scheme() {
fn validate_pulsar_broker_config_rejects_tls_ca_without_tls_scheme() {
let mut config = KVS::new();
config.insert(PULSAR_TOPIC.to_string(), "events".to_string());
config.insert(PULSAR_TLS_ALLOW_INSECURE.to_string(), "on".to_string());
config.insert(PULSAR_TLS_CA.to_string(), "/etc/ssl/certs/ca.pem".to_string());
let err = validate_pulsar_broker_config("pulsar://127.0.0.1:6650", &config, "")
.expect_err("TLS flags should require pulsar+ssl");
.expect_err("a CA bundle should require pulsar+ssl");
assert!(err.to_string().contains("only allowed with pulsar+ssl"));
}
#[test]
fn validate_pulsar_broker_config_accepts_inert_tls_toggles_without_tls_scheme() {
// The TLS toggles are no-ops on a plaintext broker, so a persisted
// config carrying non-default values must still load — otherwise the
// target is stuck offline after a restart (issue #4796).
let mut config = KVS::new();
config.insert(PULSAR_TOPIC.to_string(), "events".to_string());
config.insert(PULSAR_TLS_ALLOW_INSECURE.to_string(), "on".to_string());
config.insert(PULSAR_TLS_HOSTNAME_VERIFICATION.to_string(), "off".to_string());
validate_pulsar_broker_config("pulsar://127.0.0.1:6650", &config, "")
.expect("inert TLS toggles must not fail a plaintext broker");
}
}
crates/targets/src/target/pulsar.rs
+32 -2
View File
@@ -113,9 +113,16 @@ impl PulsarArgs {
let parsed = Url::parse(&self.broker)
.map_err(|e| TargetError::Configuration(format!("Invalid Pulsar broker URL: {e} (value: '{}')", self.broker)))?;
let tls_enabled = parsed.scheme() == "pulsar+ssl";
if !tls_enabled && (!self.tls_ca.is_empty() || self.tls_allow_insecure || !self.tls_hostname_verification) {
// A CA bundle is TLS trust material a plaintext `pulsar://` broker
// silently ignores, so treat it as a genuine misconfiguration. The
// `tls_allow_insecure` / `tls_hostname_verification` toggles, however,
// are inert on a non-TLS broker (they only take effect for
// `pulsar+ssl`); rejecting non-default values would leave a persisted
// target permanently offline after a restart even though the flags do
// nothing — see issue #4796.
if !tls_enabled && !self.tls_ca.is_empty() {
return Err(TargetError::Configuration(
"Pulsar TLS settings are only allowed with pulsar+ssl brokers".to_string(),
"Pulsar tls_ca is only allowed with pulsar+ssl brokers".to_string(),
));
}
@@ -540,4 +547,27 @@ mod tests {
};
assert!(args.validate().is_err());
}
#[test]
fn validate_pulsar_accepts_inert_tls_toggles_on_plaintext_broker() {
// `tls_allow_insecure` / `tls_hostname_verification` are no-ops on a
// `pulsar://` broker, so a target persisted with these non-default
// values must still validate — otherwise it stays offline after a
// restart (issue #4796).
let args = PulsarArgs {
tls_allow_insecure: true,
tls_hostname_verification: false,
..base_args()
};
args.validate().expect("inert TLS toggles must not fail a plaintext broker");
}
#[test]
fn validate_pulsar_rejects_tls_ca_on_plaintext_broker() {
let args = PulsarArgs {
tls_ca: "/etc/ssl/certs/ca.pem".to_string(),
..base_args()
};
assert!(args.validate().is_err());
}
}