Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add deserialization to SocketAddr struct to make port for dns_config optional #1564

Merged
merged 14 commits into from
Mar 28, 2024
51 changes: 50 additions & 1 deletion crates/ott-common/src/discovery/dns.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use std::str::FromStr;

use async_trait::async_trait;
use hickory_resolver::{
config::{NameServerConfig, Protocol, ResolverConfig, ResolverOpts},
TokioAsyncResolver,
};
use serde::Deserializer;
use tracing::info;

use super::*;
Expand All @@ -12,6 +15,7 @@ pub struct DnsDiscoveryConfig {
/// The port that monoliths should be listening on for load balancer connections.
pub service_port: u16,
/// The DNS server to query. Optional. If not provided, the system configuration will be used instead.
#[serde(deserialize_with = "deserialize_dns_server")]
pub dns_server: Option<SocketAddr>,
/// The A record to query. If using docker-compose, this should be the service name for the monolith.
pub query: String,
Expand All @@ -21,6 +25,23 @@ pub struct DnsDiscoveryConfig {
pub polling_interval: Option<Duration>,
}

fn deserialize_dns_server<'de, D>(deserializer: D) -> Result<Option<SocketAddr>, D::Error>
where
D: Deserializer<'de>,
{
let buf = String::deserialize(deserializer)?;

if buf.is_empty() {
return Ok(None);
}
cjrkoa marked this conversation as resolved.
Show resolved Hide resolved

if IpAddr::from_str(&buf).is_ok() {
Ok(Some(SocketAddr::new(IpAddr::from_str(&buf).unwrap(), 53)))
cjrkoa marked this conversation as resolved.
Show resolved Hide resolved
} else {
Ok(Some(SocketAddr::from_str(&buf).unwrap()))
cjrkoa marked this conversation as resolved.
Show resolved Hide resolved
}
}

pub struct DnsServiceDiscoverer {
config: DnsDiscoveryConfig,
}
Expand Down Expand Up @@ -76,7 +97,7 @@ mod test {
use serde_json::json;

#[test]
fn server_deserializes_correctly() {
fn dns_server_deserializes_correctly() {
let json = json!({
"service_port": 8080,
"dns_server": "127.0.0.1:100",
Expand All @@ -88,4 +109,32 @@ mod test {

assert_eq!(config.dns_server, Some(([127, 0, 0, 1], 100).into()));
}

#[test]
fn dns_server_deserialization_defaults_to_port_53() {
let json = json!({
"service_port": 8080,
"dns_server": "127.0.0.1",
"query": "".to_string(),
});

let config: DnsDiscoveryConfig =
serde_json::from_value(json).expect("Failed to deserialize json");

assert_eq!(config.dns_server, Some(([127, 0, 0, 1], 53).into()));
}

#[test]
fn dns_server_is_optional() {
let json = json!({
"service_port": 8080,
"dns_server": "",
cjrkoa marked this conversation as resolved.
Show resolved Hide resolved
"query": "".to_string(),
});

let config: DnsDiscoveryConfig =
serde_json::from_value(json).expect("Failed to deserialize json");

assert_eq!(config.dns_server, None);
}
}
Loading