Skip to content
Merged
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
23 changes: 23 additions & 0 deletions crates/core/src/domain_config/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,29 @@ pub trait DomainConfigBackend: Send + Sync {
option: &'a str,
) -> Result<Option<DomainConfigOption>, DomainConfigProviderError>;

/// The IDs of every domain that stores `option` in `group`.
///
/// Backs the fan-out set of ADR 0034 §5: the assignment provider needs the
/// domains that have bound a non-global `assignment/driver` without reading
/// each domain's configuration in turn. The result order is unspecified and
/// may carry duplicates within a single driver; the caller deduplicates
/// across drivers.
///
/// # Parameters
/// - `state`: The current service state.
/// - `group`: The group to look in.
/// - `option`: The option that must be present.
///
/// # Returns
/// - `Result<Vec<String>, DomainConfigProviderError>` - The matching domain
/// IDs, or an error.
async fn list_domains_with_option<'a>(
&self,
state: &ServiceState,
group: DomainConfigGroupName,
option: &'a str,
) -> Result<Vec<String>, DomainConfigProviderError>;

/// Merge changes into the whole configuration of a domain.
///
/// Backs `PATCH /v3/domains/{domain_id}/config`: options absent from
Expand Down
123 changes: 109 additions & 14 deletions crates/core/src/domain_config/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,16 @@
//! `[identity] domain_config_dir` (the `fs` driver) and the database (the `sql`
//! driver, written through the config API). python-keystone layers the file
//! configuration first and lets the database one override it; this module does
//! the same, gated by the two `[identity]` switches:
//! the same, gated by two switches (ADR 0034 §2):
//!
//! - `domain_specific_drivers_enabled` — consult the file source at all;
//! - `domain_configurations_from_database` — consult the database source, which
//! wins where both set the same option.
//! - files — the `[domain_config] from_files` key, falling back to the
//! deprecated `[identity] domain_specific_drivers_enabled`;
//! - database — the `[domain_config] from_database` key, falling back to the
//! deprecated `[identity] domain_configurations_from_database`; the database
//! source wins where both set the same option.
//!
//! See [`effective_domain_config_sources`] for the fallback and the conflict
//! warning.
//!
//! The result is the raw, still-serializable [`DomainConfig`] the config API
//! returns. A consumer that needs a configuration a driver can use — the
Expand All @@ -32,28 +37,86 @@
//! not reach a response, which is why they are deliberately left to the
//! caller.

use std::collections::HashSet;
use std::sync::Arc;

use tracing::warn;

use openstack_keystone_config::Config;
use openstack_keystone_core_types::domain_config::DomainConfig;
use openstack_keystone_core_types::domain_config::{DomainConfig, DomainConfigGroupName};

use crate::domain_config::backend::DomainConfigBackend;
use crate::domain_config::error::DomainConfigProviderError;
use crate::keystone::ServiceState;
use crate::plugin_manager::PluginManagerApi;

/// Whether the `fs` and `sql` domain-config sources are consulted, taking the
/// `[domain_config]` keys where set and the deprecated `[identity]` switches
/// otherwise (ADR 0034 §2).
///
/// A `[domain_config]` key set to a value that contradicts an `[identity]`
/// switch the operator moved off its default logs one `WARN`; the
/// `[domain_config]` value still wins.
///
/// # Parameters
/// - `config`: The running service configuration.
///
/// # Returns
/// - `(bool, bool)` - `(consult files, consult database)`.
pub fn effective_domain_config_sources(config: &Config) -> (bool, bool) {
let from_files = resolve_source(
"from_files",
config.domain_config.from_files,
"domain_specific_drivers_enabled",
config.identity.domain_specific_drivers_enabled,
false,
);
let from_database = resolve_source(
"from_database",
config.domain_config.from_database,
"domain_configurations_from_database",
config.identity.domain_configurations_from_database,
true,
);
(from_files, from_database)
}

/// One source's effective value: the explicit `[domain_config]` key when set,
/// the `[identity]` switch otherwise. Warns only when both were set to
/// conflicting values (the `[identity]` switch differs from its own default and
/// from the explicit key).
fn resolve_source(
key: &str,
explicit: Option<bool>,
identity_key: &str,
identity_value: bool,
identity_default: bool,
) -> bool {
match explicit {
Some(value) => {
if identity_value != identity_default && value != identity_value {
warn!(
"[domain_config] {key} = {value} overrides the conflicting \
[identity] {identity_key} = {identity_value}"
);
}
value
}
None => identity_value,
}
}

/// Merges a domain's file-based and database-stored configuration into one.
///
/// Holds a handle to each source it is configured to use; a source that is
/// switched off in `[identity]` is simply `None`, and a resolver with neither
/// resolves every domain to the empty configuration.
pub struct DomainConfigResolver {
/// The `fs` driver, `Some` when
/// `[identity] domain_specific_drivers_enabled` is set.
/// The `fs` driver, `Some` when [`effective_domain_config_sources`] reports
/// the file source on.
file: Option<Arc<dyn DomainConfigBackend>>,
/// The `sql` driver, `Some` when
/// `[identity] domain_configurations_from_database` is set. Overrides the
/// file source option by option.
/// The `sql` driver, `Some` when [`effective_domain_config_sources`] reports
/// the database source on. Overrides the file source option by option.
database: Option<Arc<dyn DomainConfigBackend>>,
}

Expand All @@ -62,8 +125,8 @@ impl DomainConfigResolver {
/// domain-config backends.
///
/// # Parameters
/// - `config`: The running service configuration; its `[identity]`
/// switches decide which sources are consulted.
/// - `config`: The running service configuration; [`effective_domain_config_sources`]
/// decides which sources are consulted.
/// - `plugin_manager`: Provides the `"fs"` / `"sql"` backends by name.
///
/// # Returns
Expand All @@ -74,12 +137,13 @@ impl DomainConfigResolver {
config: &Config,
plugin_manager: &P,
) -> Result<Self, DomainConfigProviderError> {
let file = if config.identity.domain_specific_drivers_enabled {
let (from_files, from_database) = effective_domain_config_sources(config);
let file = if from_files {
Some(plugin_manager.get_domain_config_backend("fs")?.clone())
} else {
None
};
let database = if config.identity.domain_configurations_from_database {
let database = if from_database {
Some(plugin_manager.get_domain_config_backend("sql")?.clone())
} else {
None
Expand Down Expand Up @@ -143,6 +207,37 @@ impl DomainConfigResolver {
}
Ok(resolved)
}

/// Every domain that stores an `assignment/driver` binding in any active
/// source, deduplicated across sources (ADR 0034 §5).
///
/// This is the fan-out set the assignment provider sizes its per-domain
/// backend instances against: a domain absent from it resolves to the
/// global driver and needs no dedicated instance.
///
/// # Parameters
/// - `state`: The current service state, handed to each backend.
///
/// # Returns
/// - `Result<Vec<String>, DomainConfigProviderError>` - The bound domain
/// IDs in no particular order, or the first error a source returns.
pub async fn bound_domains(
&self,
state: &ServiceState,
) -> Result<Vec<String>, DomainConfigProviderError> {
let mut domains: HashSet<String> = HashSet::new();
for source in [self.file.as_ref(), self.database.as_ref()]
.into_iter()
.flatten()
{
domains.extend(
source
.list_domains_with_option(state, DomainConfigGroupName::Assignment, "driver")
.await?,
);
}
Ok(domains.into_iter().collect())
}
}

#[cfg(test)]
Expand Down
115 changes: 115 additions & 0 deletions crates/core/src/domain_config/resolver/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,118 @@ async fn an_error_from_a_source_propagates() {
};
assert!(resolver.effective_config(&state, "d1").await.is_err());
}

mod effective_sources {
use openstack_keystone_config::Config;
use tracing_test::traced_test;

use super::super::effective_domain_config_sources;

/// A config with the two `[identity]` switches and the two
/// `[domain_config]` keys set as given.
fn config(
identity_files: bool,
identity_db: bool,
from_files: Option<bool>,
from_database: Option<bool>,
) -> Config {
let mut config = Config::default();
config.identity.domain_specific_drivers_enabled = identity_files;
config.identity.domain_configurations_from_database = identity_db;
config.domain_config.from_files = from_files;
config.domain_config.from_database = from_database;
config
}

#[test]
fn unset_keys_inherit_the_identity_switches_including_defaults() {
// `[identity]` defaults: files off, database on.
assert_eq!(
effective_domain_config_sources(&config(false, true, None, None)),
(false, true)
);
assert_eq!(
effective_domain_config_sources(&config(true, false, None, None)),
(true, false)
);
}

#[traced_test]
#[test]
fn an_explicit_key_wins_over_the_identity_switch_at_its_default() {
// `domain_specific_drivers_enabled` still at its default `false`, so
// turning files on through `[domain_config]` is not a conflict.
assert_eq!(
effective_domain_config_sources(&config(false, true, Some(true), Some(false))),
(true, false)
);
assert!(!logs_contain("overrides the conflicting"));
}

#[traced_test]
#[test]
fn an_explicit_key_that_contradicts_a_moved_identity_switch_warns() {
// Operator set `domain_specific_drivers_enabled = true` and then
// `[domain_config] from_files = false`: the new key wins, with a warning.
assert_eq!(
effective_domain_config_sources(&config(true, false, Some(false), Some(true))),
(false, true)
);
assert!(logs_contain(
"[domain_config] from_files = false overrides the conflicting [identity] domain_specific_drivers_enabled = true"
));
assert!(logs_contain(
"[domain_config] from_database = true overrides the conflicting [identity] domain_configurations_from_database = false"
));
}
}

/// A backend whose `list_domains_with_option` always answers with `domains`.
fn listing_source(domains: Vec<String>) -> Arc<dyn DomainConfigBackend> {
let mut mock = MockDomainConfigBackend::new();
mock.expect_list_domains_with_option()
.returning(move |_, _, _| Ok(domains.clone()));
Arc::new(mock)
}

#[tokio::test]
async fn bound_domains_unions_and_dedups_across_sources() {
let state = get_mocked_state(None, None).await;
let resolver = DomainConfigResolver {
file: Some(listing_source(vec!["a".to_string(), "b".to_string()])),
database: Some(listing_source(vec!["b".to_string(), "c".to_string()])),
};

let mut domains = resolver.bound_domains(&state).await.expect("resolvable");
domains.sort();
assert_eq!(domains, ["a", "b", "c"]);
}

#[tokio::test]
async fn bound_domains_is_empty_with_no_source() {
let state = get_mocked_state(None, None).await;
let resolver = DomainConfigResolver {
file: None,
database: None,
};
assert!(
resolver
.bound_domains(&state)
.await
.expect("resolvable")
.is_empty()
);
}

#[tokio::test]
async fn bound_domains_propagates_a_source_error() {
let state = get_mocked_state(None, None).await;
let mut mock = MockDomainConfigBackend::new();
mock.expect_list_domains_with_option()
.returning(|_, _, _| Err(DomainConfigProviderError::Driver("boom".to_string())));
let resolver = DomainConfigResolver {
file: Some(Arc::new(mock)),
database: None,
};
assert!(resolver.bound_domains(&state).await.is_err());
}
29 changes: 29 additions & 0 deletions crates/domain-config-driver-fs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,35 @@ impl DomainConfigBackend for FsBackend {
get::get_option(&self.store, &name, group, option)
}

/// The IDs of every domain whose file sets `group`/`option`.
///
/// The files are keyed by domain name, so each match is resolved back to an
/// ID through the resource provider; a name with no live domain is skipped.
async fn list_domains_with_option<'a>(
&self,
state: &ServiceState,
group: DomainConfigGroupName,
option: &'a str,
) -> Result<Vec<String>, DomainConfigProviderError> {
let names = self.store.domains_with_option(group, option);
if names.is_empty() {
return Ok(Vec::new());
}
let ctx = ExecutionContext::internal(state);
let resource = state.provider.get_resource_provider();
let mut ids = Vec::with_capacity(names.len());
for name in names {
let found = resource
.find_domain_by_name(&ctx, name)
.await
.map_err(|err| DomainConfigProviderError::Driver(err.to_string()))?;
if let Some(domain) = found {
ids.push(domain.id);
}
}
Ok(ids)
}

/// Read-only: always [`DomainConfigProviderError::Readonly`].
async fn update_domain_config<'a>(
&self,
Expand Down
20 changes: 20 additions & 0 deletions crates/domain-config-driver-fs/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,26 @@ impl DomainConfigStore {
self.0.get(domain_name)
}

/// The names of every domain whose file sets `group`/`option`.
///
/// Used to size the assignment fan-out set (ADR 0034 §5); the caller maps
/// the names back to domain IDs.
pub(crate) fn domains_with_option(
&self,
group: DomainConfigGroupName,
option: &str,
) -> Vec<&str> {
self.0
.iter()
.filter(|(_, config)| {
config
.group(group)
.is_some_and(|stored| stored.get(option).is_some())
})
.map(|(name, _)| name.as_str())
.collect()
}

/// Build a store straight from parsed configurations. Test helper.
#[cfg(test)]
pub(crate) fn from_map(map: HashMap<String, DomainConfig>) -> Self {
Expand Down
Loading
Loading