fix(ecstore): reject Azure tier storageClass/spAuth instead of silently ignoring them (#6817)

TierAzure.storage_class and .sp_auth round-trip faithfully through the
admin API and on-disk config (ExternalTierAzure encode/decode in
tier.rs), so an operator can configure them, read them back via
ListTier, and never learn they do nothing. They are dropped only at the
WarmBackendAzure construction boundary: the Azure warm backend goes
through the same S3-compatible TransitionClient as every other
provider and has no Azure Blob-native client or Azure AD dependency
(confirmed: no azure_* crate anywhere in the workspace), so neither
field can actually be honored today. MinIO's reference implementation
(cmd/warm-backend-azure.go) treats both as first-class: storage_class
sets the blob access tier on every PUT, and sp_auth is a full
alternative to access/secret-key auth via azidentity, mutually
exclusive with it.

Rather than the larger, riskier options (add a native Azure SDK
dependency and a parallel non-S3 client path, or break the persisted
config format by removing the fields), this closes the silent-failure
gap with the minimal safe fix: TierConfigMgr::add now rejects an Azure
tier config with either field set, before backend construction,
returning ERR_TIER_INVALID_CONFIG with an explicit message instead of
accepting and ignoring. The fields stay in the config type (no format
break); already-persisted tiers with these fields set are grandfathered
in un-rejected (edit does not touch sp_auth or storage_class either).
Full support remains a larger follow-up if ever prioritized.

Also removes TierAzure::is_sp_enabled(), which had zero callers
repo-wide (backlog#2055 flagged this) and would have been misleading
dead weight once this decision was made — reusing it for the new gate
would also have been wrong, since it requires *all three* sp_auth
fields non-empty (&&), while the gate must reject on *any* one being set.

Refs rustfs/backlog#2055

(cherry picked from commit 8d148c4e9b2507a1c5075e3d9513adb8b5851ef5)
This commit is contained in:
Zhengchao An
2026-08-29 01:12:30 +08:00
committed by GitHub
parent 73c9dd4c9d
commit 75cd3885f3
2 changed files with 69 additions and 6 deletions
+69
View File
@@ -2545,6 +2545,24 @@ impl TierConfigMgr {
})?;
}
// The Azure warm backend goes through the same S3-compatible TransitionClient as every
// other provider (backlog#2055): it has no Azure Blob-native client and no Azure AD
// dependency, so `storage_class` and `sp_auth` cannot be honored today even though the
// config type carries them. Reject them explicitly here instead of silently accepting
// and then dropping them at the WarmBackendAzure construction boundary.
if matches!(&tier_config.tier_type, TierType::Azure)
&& let Some(azure) = tier_config.azure.as_ref()
{
let sp_auth_set = !azure.sp_auth.tenant_id.is_empty()
|| !azure.sp_auth.client_id.is_empty()
|| !azure.sp_auth.client_secret.is_empty();
if !azure.storage_class.is_empty() || sp_auth_set {
let mut err = ERR_TIER_INVALID_CONFIG.clone();
err.message = "Azure remote tiers do not support storageClass or spAuth yet; leave both unset".to_string();
return Err(err);
}
}
let d = new_warm_backend(&tier_config, true).await?;
if !force {
@@ -6384,6 +6402,57 @@ mod tests {
assert!(mgr.tiers.is_empty());
}
#[tokio::test]
async fn test_add_rejects_azure_storage_class_before_backend_setup() {
let mut mgr = empty_mgr();
let mut tier = build_azure_tier("account-a");
tier.azure.as_mut().expect("Azure payload should exist").storage_class = "HOT".to_string();
let err = mgr
.add(tier, true)
.await
.expect_err("a non-empty Azure storageClass must be rejected before backend setup");
assert_eq!(err.code, ERR_TIER_INVALID_CONFIG.code);
assert!(err.message.contains("storageClass"), "{}", err.message);
assert!(mgr.tiers.is_empty());
}
#[tokio::test]
async fn test_add_rejects_azure_partial_sp_auth_before_backend_setup() {
// Only `tenant_id` is set: `TierAzure::is_sp_enabled()`-style "all three fields"
// logic would miss this, so the check must reject on *any* sp_auth sub-field
// being non-empty rather than requiring all three.
let mut mgr = empty_mgr();
let mut tier = build_azure_tier("account-a");
tier.azure.as_mut().expect("Azure payload should exist").sp_auth.tenant_id = "tenant".to_string();
let err = mgr
.add(tier, true)
.await
.expect_err("a partially-filled Azure spAuth must be rejected before backend setup");
assert_eq!(err.code, ERR_TIER_INVALID_CONFIG.code);
assert!(err.message.contains("spAuth"), "{}", err.message);
assert!(mgr.tiers.is_empty());
}
#[tokio::test]
async fn test_add_does_not_reject_azure_config_without_storage_class_or_sp_auth() {
// A plain Azure config (the common case: static access/secret key, no
// storageClass, no spAuth) must sail past the new gate. `new_warm_backend`
// builds the S3-compatible client lazily (no eager DNS/connect), so with
// `force: true` (which also skips the `in_use` probe) this succeeds even
// against a fake endpoint — the point here is only that the gate itself
// does not fire.
let mut mgr = empty_mgr();
let tier = build_azure_tier("account-a");
let tier_name = tier.name.clone();
mgr.add(tier, true)
.await
.expect("a config with no storageClass/spAuth must not trip the new gate");
assert!(mgr.tiers.contains_key(&tier_name));
}
#[tokio::test]
async fn test_add_rejects_reserved_names() {
// Supersedes the former `test_add_does_not_reserve_standard_name_regression_anchor`
@@ -646,12 +646,6 @@ pub struct TierAzure {
pub sp_auth: ServicePrincipalAuth,
}
impl TierAzure {
pub fn is_sp_enabled(&self) -> bool {
!self.sp_auth.tenant_id.is_empty() && !self.sp_auth.client_id.is_empty() && !self.sp_auth.client_secret.is_empty()
}
}
/*
fn AzureServicePrincipal(tenantID, clientID, clientSecret string) func(az *TierAzure) error {
return func(az *TierAzure) error {