[RFC] [dns-server] move to new code organization defined in RFD 619

Cargo.toml

@@ -67,6 +67,7 @@ members = [
     "internal-dns/cli",
     "internal-dns/resolver",
     "internal-dns/types",
+    "internal-dns/types/migrations",
     "ipcc",
     "key-manager",
     "live-tests",
@@ -229,6 +230,7 @@ default-members = [
     "internal-dns/cli",
     "internal-dns/resolver",
     "internal-dns/types",
+    "internal-dns/types/migrations",
     "ipcc",
     "key-manager",
     "live-tests",
@@ -515,6 +517,7 @@ installinator-client = { path = "clients/installinator-client" }
 installinator-common = { path = "installinator-common" }
 internal-dns-resolver = { path = "internal-dns/resolver" }
 internal-dns-types = { path = "internal-dns/types" }
+internal-dns-types-migrations = { path = "internal-dns/types/migrations" }
 ipcc = { path = "ipcc" }
 ipnet = "2.9"
 itertools = "0.14.0"

dns-server-api/Cargo.toml

@@ -11,7 +11,7 @@ workspace = true
 chrono.workspace = true
 dropshot.workspace = true
 dropshot-api-manager-types.workspace = true
-internal-dns-types.workspace = true
+internal-dns-types-migrations.workspace = true
 omicron-workspace-hack.workspace = true
 schemars.workspace = true
 semver.workspace = true

dns-server-api/src/lib.rs

@@ -133,7 +133,7 @@ pub trait DnsServerApi {
     async fn dns_config_get_v1(
         rqctx: RequestContext<Self::Context>,
     ) -> Result<
-        HttpResponseOk<internal_dns_types::v1::config::DnsConfig>,
+        HttpResponseOk<internal_dns_types_migrations::v1::config::DnsConfig>,
         HttpError,
     >;
 
@@ -146,7 +146,7 @@ pub trait DnsServerApi {
     async fn dns_config_get_v2(
         rqctx: RequestContext<Self::Context>,
     ) -> Result<
-        HttpResponseOk<internal_dns_types::v2::config::DnsConfig>,
+        HttpResponseOk<internal_dns_types_migrations::v2::config::DnsConfig>,
         HttpError,
     >;
 
@@ -159,7 +159,7 @@ pub trait DnsServerApi {
     async fn dns_config_put_v1(
         rqctx: RequestContext<Self::Context>,
         rq: dropshot::TypedBody<
-            internal_dns_types::v1::config::DnsConfigParams,
+            internal_dns_types_migrations::v1::config::DnsConfigParams,
         >,
     ) -> Result<dropshot::HttpResponseUpdatedNoContent, dropshot::HttpError>;
 
@@ -172,7 +172,7 @@ pub trait DnsServerApi {
     async fn dns_config_put_v2(
         rqctx: RequestContext<Self::Context>,
         rq: dropshot::TypedBody<
-            internal_dns_types::v2::config::DnsConfigParams,
+            internal_dns_types_migrations::v2::config::DnsConfigParams,
         >,
     ) -> Result<dropshot::HttpResponseUpdatedNoContent, dropshot::HttpError>;
 }

dns-server/Cargo.toml

@@ -21,6 +21,7 @@ hickory-resolver.workspace = true
 hickory-server.workspace = true
 http.workspace = true
 internal-dns-types.workspace = true
+internal-dns-types-migrations.workspace = true
 omicron-common.workspace = true
 oxide-tokio-rt.workspace = true
 pretty-hex.workspace = true

dns-server/src/http_server.rs

@@ -11,9 +11,9 @@ use dns_service_client::{
     ERROR_CODE_UPDATE_IN_PROGRESS,
 };
 use dropshot::RequestContext;
-use internal_dns_types::{
-    v1::{self, config::TranslationError as V1TranslationError},
-    v2::{self, config::TranslationError as V2TranslationError},
+use internal_dns_types_migrations::{
+    v1, v2,
+    v2::config::{V1ToV2TranslationError, V2ToV1TranslationError},
 };
 
 pub struct Context {
@@ -45,7 +45,7 @@ impl DnsServerApi for DnsServerApiImpl {
         let result = Self::dns_config_get(rqctx).await?;
         match result.0.try_into() {
             Ok(config) => Ok(dropshot::HttpResponseOk(config)),
-            Err(V2TranslationError::IncompatibleRecord) => {
+            Err(V2ToV1TranslationError::IncompatibleRecord) => {
                 Err(dropshot::HttpError::for_bad_request(
                     None,
                     ERROR_CODE_INCOMPATIBLE_RECORD.to_string(),
@@ -70,7 +70,7 @@ impl DnsServerApi for DnsServerApiImpl {
     {
         let provided_config = match rq.into_inner().try_into() {
             Ok(config) => config,
-            Err(V1TranslationError::GenerationTooLarge) => {
+            Err(V1ToV2TranslationError::GenerationTooLarge) => {
                 return Err(dropshot::HttpError::for_bad_request(
                     None,
                     ERROR_CODE_INCOMPATIBLE_RECORD.to_string(),

dns-server/tests/cross_version_test.rs

@@ -23,7 +23,7 @@ const TEST_ZONE: &'static str = "oxide.internal";
 // well.
 mod v1_client {
     use anyhow::Context;
-    use internal_dns_types::v1;
+    use internal_dns_types_migrations::v1;
 
     use std::collections::HashMap;
 
@@ -116,8 +116,8 @@ mod v1_client {
 pub async fn cross_version_works() -> Result<(), anyhow::Error> {
     let test_ctx = init_client_server("cross_version_works").await?;
 
-    use internal_dns_types::v1::config::DnsRecord as V1DnsRecord;
-    use internal_dns_types::v2::config::DnsRecord as V2DnsRecord;
+    use internal_dns_types_migrations::v1::config::DnsRecord as V1DnsRecord;
+    use internal_dns_types_migrations::v2::config::DnsRecord as V2DnsRecord;
 
     let ns1_addr = Ipv6Addr::new(0xfd, 0, 0, 0, 0, 0, 0, 0x1);
     let ns1_name = format!("ns1.{TEST_ZONE}.");

internal-dns/types/Cargo.toml

@@ -9,6 +9,7 @@ workspace = true
 [dependencies]
 anyhow.workspace = true
 chrono.workspace = true
+internal-dns-types-migrations.workspace = true
 omicron-common.workspace = true
 omicron-workspace-hack.workspace = true
 omicron-uuid-kinds.workspace = true

internal-dns/types/migrations/Cargo.toml

@@ -0,0 +1,15 @@
+[package]
+name = "internal-dns-types-migrations"
+version = "0.1.0"
+edition.workspace = true
+
+[lints]
+workspace = true
+
+[dependencies]
+anyhow.workspace = true
+chrono.workspace = true
+omicron-common.workspace = true
+omicron-workspace-hack.workspace = true
+schemars.workspace = true
+serde.workspace = true

internal-dns/types/migrations/src/lib.rs

@@ -0,0 +1,12 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+//! Type migrations for the DNS server API.
+//!
+//! This crate contains versioned types for the DNS server API. Types are
+//! organized by the API version they were introduced in, following the
+//! principles outlined in [RFD 619](https://rfd.shared.oxide.computer/rfd/0619).
+
+pub mod v1;
+pub mod v2;

internal-dns/types/src/v1/config.rs → internal-dns/types/migrations/src/v1/config.rs

@@ -2,7 +2,8 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
-use crate::v2;
+//! DNS configuration types for API version 1.0.0 (INITIAL).
+
 use anyhow::ensure;
 use omicron_common::api::external::Generation;
 use schemars::JsonSchema;
@@ -35,34 +36,6 @@ impl DnsConfigParams {
     }
 }
 
-pub enum TranslationError {
-    GenerationTooLarge,
-}
-
-impl TryInto<v2::config::DnsConfigParams> for DnsConfigParams {
-    type Error = TranslationError;
-
-    fn try_into(self) -> Result<v2::config::DnsConfigParams, Self::Error> {
-        let serial: u32 = self
-            .generation
-            .as_u64()
-            .try_into()
-            .map_err(|_| TranslationError::GenerationTooLarge)?;
-
-        let mut converted_zones: Vec<v2::config::DnsConfigZone> = Vec::new();
-        for zone in self.zones.into_iter() {
-            converted_zones.push(zone.into());
-        }
-
-        Ok(v2::config::DnsConfigParams {
-            generation: self.generation,
-            serial,
-            time_created: self.time_created,
-            zones: converted_zones,
-        })
-    }
-}
-
 #[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)]
 pub struct DnsConfig {
     pub generation: Generation,
@@ -71,40 +44,16 @@ pub struct DnsConfig {
     pub zones: Vec<DnsConfigZone>,
 }
 
-// See docs on [`v2::config::DnsConfigZone`] for more about this struct. They are functionally
-// equivalent. We would include that doc comment here, but altering docs to existing types
-// makes them appear different in OpenAPI terms and would be "breaking" for the time being.
+// See docs on [`crate::v2::config::DnsConfigZone`] for more about this struct.
+// They are functionally equivalent. We would include that doc comment here,
+// but altering docs to existing types makes them appear different in OpenAPI
+// terms and would be "breaking" for the time being.
 #[derive(Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq)]
 pub struct DnsConfigZone {
     pub zone_name: String,
     pub records: HashMap<String, Vec<DnsRecord>>,
 }
 
-impl Into<v2::config::DnsConfigZone> for DnsConfigZone {
-    fn into(self) -> v2::config::DnsConfigZone {
-        let converted_records: HashMap<String, Vec<v2::config::DnsRecord>> =
-            self.records
-                .into_iter()
-                .filter_map(|(name, name_records)| {
-                    let converted_name_records: Vec<v2::config::DnsRecord> =
-                        name_records
-                            .into_iter()
-                            .map(|rec| rec.into())
-                            .collect();
-                    if converted_name_records.is_empty() {
-                        None
-                    } else {
-                        Some((name, converted_name_records))
-                    }
-                })
-                .collect();
-        v2::config::DnsConfigZone {
-            zone_name: self.zone_name,
-            records: converted_records,
-        }
-    }
-}
-
 #[derive(
     Clone,
     Debug,
@@ -128,16 +77,6 @@ pub enum DnsRecord {
     Srv(Srv),
 }
 
-impl Into<v2::config::DnsRecord> for DnsRecord {
-    fn into(self) -> v2::config::DnsRecord {
-        match self {
-            DnsRecord::A(ip) => v2::config::DnsRecord::A(ip),
-            DnsRecord::Aaaa(ip) => v2::config::DnsRecord::Aaaa(ip),
-            DnsRecord::Srv(srv) => v2::config::DnsRecord::Srv(srv.into()),
-        }
-    }
-}
-
 // The `From<Ipv4Addr>` and `From<Ipv6Addr>` implementations are very slightly
 // dubious, because a v4 or v6 address could also theoretically map to a DNS
 // PTR record
@@ -180,14 +119,3 @@ pub struct Srv {
     pub port: u16,
     pub target: String,
 }
-
-impl From<v2::config::Srv> for Srv {
-    fn from(other: v2::config::Srv) -> Self {
-        Srv {
-            prio: other.prio,
-            weight: other.weight,
-            port: other.port,
-            target: other.target,
-        }
-    }
-}