Use concrete string types in cfgsync APIs

This commit is contained in:
andrussal 2026-03-12 10:24:29 +01:00
parent 8db21f53dd
commit f7dba01161
16 changed files with 127 additions and 93 deletions

View File

@ -185,7 +185,7 @@ struct MyNodeMetadata {
api_port: u16,
}
let registration = NodeRegistration::new("node-1", "127.0.0.1".parse().unwrap())
let registration = NodeRegistration::new("node-1".to_string(), "127.0.0.1".parse().unwrap())
.with_metadata(&MyNodeMetadata {
network_port: 3000,
api_port: 18080,

View File

@ -16,7 +16,10 @@ pub struct MaterializedArtifacts {
impl MaterializedArtifacts {
/// Creates materialized artifacts from node-local artifact sets.
#[must_use]
pub fn from_nodes(nodes: impl IntoIterator<Item = (String, ArtifactSet)>) -> Self {
pub fn from_nodes<I>(nodes: I) -> Self
where
I: IntoIterator<Item = (String, ArtifactSet)>,
{
Self {
nodes: nodes.into_iter().collect(),
shared: ArtifactSet::default(),

View File

@ -208,13 +208,16 @@ mod tests {
let nodes = registrations.iter().map(|registration| {
(
registration.identifier.clone(),
ArtifactSet::new(vec![ArtifactFile::new("/config.yaml", "ready: true")]),
ArtifactSet::new(vec![ArtifactFile::new(
"/config.yaml".to_string(),
"ready: true".to_string(),
)]),
)
});
Ok(MaterializationResult::ready(
MaterializedArtifacts::from_nodes(nodes).with_shared(ArtifactSet::new(vec![
ArtifactFile::new("/shared.yaml", "cluster: ready"),
ArtifactFile::new("/shared.yaml".to_string(), "cluster: ready".to_string()),
])),
))
}
@ -235,7 +238,7 @@ mod tests {
fn cached_snapshot_materializer_reuses_previous_result() {
let materializer = CachedSnapshotMaterializer::new(CountingMaterializer);
let snapshot = RegistrationSnapshot::new(vec![cfgsync_core::NodeRegistration::new(
"node-1",
"node-1".to_string(),
"127.0.0.1".parse().expect("parse ip"),
)]);
@ -260,7 +263,7 @@ mod tests {
},
);
let snapshot = RegistrationSnapshot::new(vec![cfgsync_core::NodeRegistration::new(
"node-1",
"node-1".to_string(),
"127.0.0.1".parse().expect("parse ip"),
)]);

View File

@ -122,11 +122,15 @@ mod tests {
fn registration_source_resolves_identifier() {
let artifacts = MaterializedArtifacts::from_nodes([(
"node-1".to_owned(),
ArtifactSet::new(vec![ArtifactFile::new("/config.yaml", "a: 1")]),
ArtifactSet::new(vec![ArtifactFile::new(
"/config.yaml".to_string(),
"a: 1".to_string(),
)]),
)]);
let source = RegistrationConfigSource::new(artifacts);
let registration = NodeRegistration::new("node-1", "127.0.0.1".parse().expect("parse ip"));
let registration =
NodeRegistration::new("node-1".to_string(), "127.0.0.1".parse().expect("parse ip"));
let _ = source.register(registration.clone());
match source.resolve(&registration) {
@ -139,11 +143,15 @@ mod tests {
fn registration_source_reports_not_ready_before_registration() {
let artifacts = MaterializedArtifacts::from_nodes([(
"node-1".to_owned(),
ArtifactSet::new(vec![ArtifactFile::new("/config.yaml", "a: 1")]),
ArtifactSet::new(vec![ArtifactFile::new(
"/config.yaml".to_string(),
"a: 1".to_string(),
)]),
)]);
let source = RegistrationConfigSource::new(artifacts);
let registration = NodeRegistration::new("node-1", "127.0.0.1".parse().expect("parse ip"));
let registration =
NodeRegistration::new("node-1".to_string(), "127.0.0.1".parse().expect("parse ip"));
match source.resolve(&registration) {
ConfigResolveResponse::Config(_) => panic!("expected not-ready"),
@ -168,7 +176,7 @@ mod tests {
(
registration.identifier.clone(),
ArtifactSet::new(vec![ArtifactFile::new(
"/config.yaml",
"/config.yaml".to_string(),
format!("id: {}", registration.identifier),
)]),
)
@ -176,7 +184,7 @@ mod tests {
Ok(MaterializationResult::ready(
MaterializedArtifacts::from_nodes(nodes).with_shared(ArtifactSet::new(vec![
ArtifactFile::new("/shared.yaml", "cluster: ready"),
ArtifactFile::new("/shared.yaml".to_string(), "cluster: ready".to_string()),
])),
))
}
@ -185,8 +193,10 @@ mod tests {
#[test]
fn registration_source_materializes_from_registration_snapshot() {
let source = RegistrationConfigSource::new(ThresholdSnapshotMaterializer);
let node_1 = NodeRegistration::new("node-1", "127.0.0.1".parse().expect("parse ip"));
let node_2 = NodeRegistration::new("node-2", "127.0.0.2".parse().expect("parse ip"));
let node_1 =
NodeRegistration::new("node-1".to_string(), "127.0.0.1".parse().expect("parse ip"));
let node_2 =
NodeRegistration::new("node-2".to_string(), "127.0.0.2".parse().expect("parse ip"));
let _ = source.register(node_1.clone());
match source.resolve(&node_1) {
@ -224,7 +234,7 @@ mod tests {
(
registration.identifier.clone(),
ArtifactSet::new(vec![ArtifactFile::new(
"/config.yaml",
"/config.yaml".to_string(),
format!("id: {}", registration.identifier),
)]),
)
@ -240,7 +250,8 @@ mod tests {
calls: AtomicUsize::new(0),
},
));
let registration = NodeRegistration::new("node-1", "127.0.0.1".parse().expect("parse ip"));
let registration =
NodeRegistration::new("node-1".to_string(), "127.0.0.1".parse().expect("parse ip"));
let _ = source.register(registration.clone());
let _ = source.resolve(&registration);

View File

@ -12,11 +12,8 @@ pub struct ArtifactFile {
impl ArtifactFile {
#[must_use]
pub fn new(path: impl Into<String>, content: impl Into<String>) -> Self {
Self {
path: path.into(),
content: content.into(),
}
pub fn new(path: String, content: String) -> Self {
Self { path, content }
}
}

View File

@ -39,8 +39,8 @@ pub struct Client {
impl Client {
/// Creates a cfgsync client pointed at the given server base URL.
#[must_use]
pub fn new(base_url: impl Into<String>) -> Self {
let mut base_url = base_url.into();
pub fn new(base_url: String) -> Self {
let mut base_url = base_url;
while base_url.ends_with('/') {
base_url.pop();
}

View File

@ -122,9 +122,9 @@ pub struct NodeRegistration {
impl NodeRegistration {
/// Creates a registration with the generic node identity fields only.
#[must_use]
pub fn new(identifier: impl Into<String>, ip: Ipv4Addr) -> Self {
pub fn new(identifier: String, ip: Ipv4Addr) -> Self {
Self {
identifier: identifier.into(),
identifier,
ip,
metadata: RegistrationPayload::default(),
}
@ -212,10 +212,10 @@ impl CfgsyncErrorResponse {
/// Builds an internal cfgsync error.
#[must_use]
pub fn internal(message: impl Into<String>) -> Self {
pub fn internal(message: String) -> Self {
Self {
code: CfgsyncErrorCode::Internal,
message: message.into(),
message,
}
}
}
@ -251,12 +251,13 @@ mod tests {
#[test]
fn registration_payload_round_trips_typed_value() {
let registration = NodeRegistration::new("node-1", "127.0.0.1".parse().expect("parse ip"))
.with_metadata(&ExampleRegistration {
network_port: 3000,
service: "blend".to_owned(),
})
.expect("serialize registration metadata");
let registration =
NodeRegistration::new("node-1".to_string(), "127.0.0.1".parse().expect("parse ip"))
.with_metadata(&ExampleRegistration {
network_port: 3000,
service: "blend".to_owned(),
})
.expect("serialize registration metadata");
let encoded = serde_json::to_value(&registration).expect("serialize registration");
let metadata = encoded.get("metadata").expect("registration metadata");

View File

@ -213,7 +213,10 @@ mod tests {
fn sample_payload() -> NodeArtifactsPayload {
NodeArtifactsPayload {
schema_version: CFGSYNC_SCHEMA_VERSION,
files: vec![NodeArtifactFile::new("/app-config.yaml", "app: test")],
files: vec![NodeArtifactFile::new(
"/app-config.yaml".to_string(),
"app: test".to_string(),
)],
}
}
@ -227,7 +230,8 @@ mod tests {
registrations: std::sync::Mutex::new(HashMap::new()),
});
let state = Arc::new(CfgsyncServerState::new(provider));
let payload = NodeRegistration::new("node-a", "127.0.0.1".parse().expect("valid ip"));
let payload =
NodeRegistration::new("node-a".to_string(), "127.0.0.1".parse().expect("valid ip"));
let _ = register_node(State(state.clone()), Json(payload.clone()))
.await
@ -246,7 +250,10 @@ mod tests {
data: HashMap::new(),
});
let state = Arc::new(CfgsyncServerState::new(provider));
let payload = NodeRegistration::new("missing-node", "127.0.0.1".parse().expect("valid ip"));
let payload = NodeRegistration::new(
"missing-node".to_string(),
"127.0.0.1".parse().expect("valid ip"),
);
let response = node_config(State(state), Json(payload))
.await
@ -272,7 +279,8 @@ mod tests {
registrations: std::sync::Mutex::new(HashMap::new()),
});
let state = Arc::new(CfgsyncServerState::new(provider));
let payload = NodeRegistration::new("node-a", "127.0.0.1".parse().expect("valid ip"));
let payload =
NodeRegistration::new("node-a".to_string(), "127.0.0.1".parse().expect("valid ip"));
let response = node_config(State(state), Json(payload))
.await

View File

@ -187,7 +187,10 @@ mod tests {
};
fn sample_payload() -> NodeArtifactsPayload {
NodeArtifactsPayload::from_files(vec![NodeArtifactFile::new("/config.yaml", "key: value")])
NodeArtifactsPayload::from_files(vec![NodeArtifactFile::new(
"/config.yaml".to_string(),
"key: value".to_string(),
)])
}
#[test]
@ -197,7 +200,7 @@ mod tests {
let repo = StaticConfigSource { configs };
match repo.resolve(&NodeRegistration::new(
"node-1",
"node-1".to_string(),
"127.0.0.1".parse().expect("parse ip"),
)) {
ConfigResolveResponse::Config(payload) => {
@ -216,7 +219,7 @@ mod tests {
};
match repo.resolve(&NodeRegistration::new(
"unknown-node",
"unknown-node".to_string(),
"127.0.0.1".parse().expect("parse ip"),
)) {
ConfigResolveResponse::Config(_) => panic!("expected missing-config error"),
@ -245,12 +248,12 @@ nodes:
BundleConfigSource::from_yaml_file(bundle_file.path()).expect("load file provider");
let _ = provider.register(NodeRegistration::new(
"node-1",
"node-1".to_string(),
"127.0.0.1".parse().expect("parse ip"),
));
match provider.resolve(&NodeRegistration::new(
"node-1",
"node-1".to_string(),
"127.0.0.1".parse().expect("parse ip"),
)) {
ConfigResolveResponse::Config(payload) => assert_eq!(payload.files.len(), 1),
@ -265,7 +268,7 @@ nodes:
let repo = StaticConfigSource { configs };
match repo.resolve(&NodeRegistration::new(
"node-1",
"node-1".to_string(),
"127.0.0.1".parse().expect("parse ip"),
)) {
ConfigResolveResponse::Config(_) => {}

View File

@ -23,7 +23,7 @@ impl RegistrationSnapshotMaterializer for ExampleMaterializer {
(
registration.identifier.clone(),
ArtifactSet::new(vec![ArtifactFile::new(
"/config.yaml",
"/config.yaml".to_string(),
format!("id: {}\n", registration.identifier),
)]),
)
@ -44,8 +44,8 @@ async fn main() -> anyhow::Result<()> {
sleep(Duration::from_millis(100)).await;
let tempdir = tempdir()?;
let outputs = OutputMap::under(tempdir.path());
let registration = NodeRegistration::new("node-1", "127.0.0.1".parse()?);
let outputs = OutputMap::under(tempdir.path().to_path_buf());
let registration = NodeRegistration::new("node-1".to_string(), "127.0.0.1".parse()?);
Client::new("http://127.0.0.1:4400")
.fetch_and_write(&registration, &outputs)

View File

@ -11,16 +11,22 @@ async fn main() -> anyhow::Result<()> {
let artifacts = MaterializedArtifacts::from_nodes([
(
"node-1".to_owned(),
ArtifactSet::new(vec![ArtifactFile::new("/config.yaml", "id: node-1\n")]),
ArtifactSet::new(vec![ArtifactFile::new(
"/config.yaml".to_string(),
"id: node-1\n".to_string(),
)]),
),
(
"node-2".to_owned(),
ArtifactSet::new(vec![ArtifactFile::new("/config.yaml", "id: node-2\n")]),
ArtifactSet::new(vec![ArtifactFile::new(
"/config.yaml".to_string(),
"id: node-2\n".to_string(),
)]),
),
])
.with_shared(ArtifactSet::new(vec![ArtifactFile::new(
"/shared/cluster.yaml",
"cluster: demo\n",
"/shared/cluster.yaml".to_string(),
"cluster: demo\n".to_string(),
)]));
let server = tokio::spawn(async move { serve(port, artifacts).await });
@ -33,7 +39,7 @@ async fn main() -> anyhow::Result<()> {
node_1_dir.path().join("config.yaml"),
node_1_dir.path().join("shared"),
);
let node_1 = NodeRegistration::new("node-1", "127.0.0.1".parse()?);
let node_1 = NodeRegistration::new("node-1".to_string(), "127.0.0.1".parse()?);
Client::new("http://127.0.0.1:4401")
.fetch_and_write(&node_1, &node_1_outputs)
@ -53,7 +59,7 @@ async fn main() -> anyhow::Result<()> {
node_2_dir.path().join("config.yaml"),
node_2_dir.path().join("shared"),
);
let node_2 = NodeRegistration::new("node-2", "127.0.0.2".parse()?);
let node_2 = NodeRegistration::new("node-2".to_string(), "127.0.0.2".parse()?);
Client::new("http://127.0.0.1:4401")
.fetch_and_write(&node_2, &node_2_outputs)

View File

@ -23,7 +23,7 @@ impl RegistrationSnapshotMaterializer for ThresholdMaterializer {
(
registration.identifier.clone(),
ArtifactSet::new(vec![ArtifactFile::new(
"/config.yaml",
"/config.yaml".to_string(),
format!("id: {}\ncluster_ready: true\n", registration.identifier),
)]),
)
@ -43,8 +43,8 @@ async fn main() -> anyhow::Result<()> {
sleep(Duration::from_millis(100)).await;
let waiting_dir = tempdir()?;
let waiting_outputs = OutputMap::under(waiting_dir.path());
let waiting_node = NodeRegistration::new("node-1", "127.0.0.1".parse()?);
let waiting_outputs = OutputMap::under(waiting_dir.path().to_path_buf());
let waiting_node = NodeRegistration::new("node-1".to_string(), "127.0.0.1".parse()?);
let waiting_client = Client::new("http://127.0.0.1:4402");
let waiting_task = tokio::spawn(async move {
@ -58,8 +58,8 @@ async fn main() -> anyhow::Result<()> {
sleep(Duration::from_millis(400)).await;
let second_dir = tempdir()?;
let second_outputs = OutputMap::under(second_dir.path());
let second_node = NodeRegistration::new("node-2", "127.0.0.2".parse()?);
let second_outputs = OutputMap::under(second_dir.path().to_path_buf());
let second_node = NodeRegistration::new("node-2".to_string(), "127.0.0.2".parse()?);
Client::new("http://127.0.0.1:4402")
.fetch_and_write(&second_node, &second_outputs)

View File

@ -39,12 +39,8 @@ impl OutputMap {
/// Routes one artifact path from the payload to a local output path.
#[must_use]
pub fn route(
mut self,
artifact_path: impl Into<String>,
output_path: impl Into<PathBuf>,
) -> Self {
self.routes.insert(artifact_path.into(), output_path.into());
pub fn route(mut self, artifact_path: String, output_path: PathBuf) -> Self {
self.routes.insert(artifact_path, output_path);
self
}
@ -54,26 +50,20 @@ impl OutputMap {
/// `shared/deployment-settings.yaml` is written to
/// `<root>/shared/deployment-settings.yaml`.
#[must_use]
pub fn under(root: impl Into<PathBuf>) -> Self {
pub fn under(root: PathBuf) -> Self {
Self {
routes: HashMap::new(),
fallback: Some(FallbackRoute::Under(root.into())),
fallback: Some(FallbackRoute::Under(root)),
}
}
/// Writes the node config to `config_path` and all other files under
/// `shared_dir`, preserving their relative artifact paths.
#[must_use]
pub fn config_and_shared(
config_path: impl Into<PathBuf>,
shared_dir: impl Into<PathBuf>,
) -> Self {
let config_path = config_path.into();
let shared_dir = shared_dir.into();
pub fn config_and_shared(config_path: PathBuf, shared_dir: PathBuf) -> Self {
Self::default()
.route("/config.yaml", config_path.clone())
.route("config.yaml", config_path)
.route("/config.yaml".to_string(), config_path.clone())
.route("config.yaml".to_string(), config_path)
.with_fallback(FallbackRoute::Shared { dir: shared_dir })
}
@ -119,7 +109,7 @@ impl Client {
#[must_use]
pub fn new(server_addr: &str) -> Self {
Self {
inner: ProtocolClient::new(server_addr),
inner: ProtocolClient::new(server_addr.to_string()),
}
}
@ -306,16 +296,20 @@ fn build_output_map() -> OutputMap {
let mut outputs = OutputMap::default();
if let Ok(path) = env::var("CFG_FILE_PATH") {
let path = PathBuf::from(path);
outputs = outputs
.route("/config.yaml", path.clone())
.route("config.yaml", path);
.route("/config.yaml".to_string(), path.clone())
.route("config.yaml".to_string(), path);
}
if let Ok(path) = env::var("CFG_DEPLOYMENT_PATH") {
let path = PathBuf::from(path);
outputs = outputs
.route("/deployment.yaml", path.clone())
.route("deployment-settings.yaml", path.clone())
.route("/deployment-settings.yaml", path);
.route("/deployment.yaml".to_string(), path.clone())
.route("deployment-settings.yaml".to_string(), path.clone())
.route("/deployment-settings.yaml".to_string(), path);
}
outputs
@ -339,8 +333,14 @@ mod tests {
let bundle = NodeArtifactsBundle::new(vec![NodeArtifactsBundleEntry {
identifier: "node-1".to_owned(),
files: vec![
NodeArtifactFile::new(app_config_path.to_string_lossy(), "app_key: app_value"),
NodeArtifactFile::new(deployment_path.to_string_lossy(), "mode: local"),
NodeArtifactFile::new(
app_config_path.to_string_lossy().into_owned(),
"app_key: app_value".to_string(),
),
NodeArtifactFile::new(
deployment_path.to_string_lossy().into_owned(),
"mode: local".to_string(),
),
],
}]);
@ -356,7 +356,10 @@ mod tests {
Client::new(&address)
.fetch_and_write(
&NodeRegistration::new("node-1", "127.0.0.1".parse().expect("parse ip")),
&NodeRegistration::new(
"node-1".to_string(),
"127.0.0.1".parse().expect("parse ip"),
),
&OutputMap::default(),
)
.await

View File

@ -76,24 +76,20 @@ impl ServerConfig {
}
#[must_use]
pub fn for_static(port: u16, artifacts_path: impl Into<String>) -> Self {
pub fn for_static(port: u16, artifacts_path: String) -> Self {
Self {
port,
source: ServerSource::Static {
artifacts_path: artifacts_path.into(),
},
source: ServerSource::Static { artifacts_path },
}
}
/// Builds a config that serves precomputed artifacts through the
/// registration flow.
#[must_use]
pub fn for_registration(port: u16, artifacts_path: impl Into<String>) -> Self {
pub fn for_registration(port: u16, artifacts_path: String) -> Self {
Self {
port,
source: ServerSource::Registration {
artifacts_path: artifacts_path.into(),
},
source: ServerSource::Registration { artifacts_path },
}
}
}

View File

@ -85,7 +85,7 @@ fn config_file_content(artifacts: &cfgsync_artifacts::ArtifactSet) -> Option<Str
}
fn build_artifact_file(path: &str, content: String) -> ArtifactFile {
ArtifactFile::new(path, content)
ArtifactFile::new(path.to_string(), content.to_string())
}
fn extract_yaml_key(content: &str, key: &str) -> Result<String> {

View File

@ -79,7 +79,10 @@ pub fn build_static_artifacts<E: StaticArtifactRenderer>(
output.insert(
E::node_identifier(index, node),
ArtifactSet::new(vec![ArtifactFile::new("/config.yaml", &config_yaml)]),
ArtifactSet::new(vec![ArtifactFile::new(
"/config.yaml".to_string(),
config_yaml.clone(),
)]),
);
}