diff --git a/Cargo.lock b/Cargo.lock index af4763cdfc0..3167dea6590 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2859,6 +2859,7 @@ dependencies = [ "hickory-server", "http", "internal-dns-types", + "internal-dns-types-versions", "omicron-common", "omicron-test-utils", "omicron-workspace-hack", @@ -2892,7 +2893,7 @@ dependencies = [ "chrono", "dropshot", "dropshot-api-manager-types", - "internal-dns-types", + "internal-dns-types-versions", "omicron-workspace-hack", "schemars 0.8.22", "semver 1.0.27", @@ -5463,6 +5464,7 @@ dependencies = [ "anyhow", "chrono", "expectorate", + "internal-dns-types-versions", "omicron-common", "omicron-uuid-kinds", "omicron-workspace-hack", @@ -5472,6 +5474,18 @@ dependencies = [ "strum 0.27.2", ] +[[package]] +name = "internal-dns-types-versions" +version = "0.1.0" +dependencies = [ + "anyhow", + "chrono", + "omicron-common", + "omicron-workspace-hack", + "schemars 0.8.22", + "serde", +] + [[package]] name = "internet-checksum" version = "0.2.1" diff --git a/Cargo.toml b/Cargo.toml index 4c38098a7b6..ba658803e96 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,6 +67,7 @@ members = [ "internal-dns/cli", "internal-dns/resolver", "internal-dns/types", + "internal-dns/types/versions", "ipcc", "key-manager", "live-tests", @@ -229,6 +230,7 @@ default-members = [ "internal-dns/cli", "internal-dns/resolver", "internal-dns/types", + "internal-dns/types/versions", "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-versions = { path = "internal-dns/types/versions" } ipcc = { path = "ipcc" } ipnet = "2.9" itertools = "0.14.0" diff --git a/dns-server-api/Cargo.toml b/dns-server-api/Cargo.toml index f374b314d4a..9a3400c4c98 100644 --- a/dns-server-api/Cargo.toml +++ b/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-versions.workspace = true omicron-workspace-hack.workspace = true schemars.workspace = true semver.workspace = true diff --git a/dns-server-api/src/lib.rs b/dns-server-api/src/lib.rs index ba35e6c2175..e853f0864cb 100644 --- a/dns-server-api/src/lib.rs +++ b/dns-server-api/src/lib.rs @@ -91,6 +91,7 @@ use dropshot::{HttpError, HttpResponseOk, RequestContext}; use dropshot_api_manager_types::api_versions; +use internal_dns_types_versions::{v1, v2}; api_versions!([ // WHEN CHANGING THE API (part 1 of 2): @@ -132,10 +133,7 @@ pub trait DnsServerApi { )] async fn dns_config_get_v1( rqctx: RequestContext, - ) -> Result< - HttpResponseOk, - HttpError, - >; + ) -> Result, HttpError>; #[endpoint( method = GET, @@ -145,10 +143,7 @@ pub trait DnsServerApi { )] async fn dns_config_get_v2( rqctx: RequestContext, - ) -> Result< - HttpResponseOk, - HttpError, - >; + ) -> Result, HttpError>; #[endpoint( method = PUT, @@ -158,9 +153,7 @@ pub trait DnsServerApi { )] async fn dns_config_put_v1( rqctx: RequestContext, - rq: dropshot::TypedBody< - internal_dns_types::v1::config::DnsConfigParams, - >, + rq: dropshot::TypedBody, ) -> Result; #[endpoint( @@ -171,8 +164,6 @@ pub trait DnsServerApi { )] async fn dns_config_put_v2( rqctx: RequestContext, - rq: dropshot::TypedBody< - internal_dns_types::v2::config::DnsConfigParams, - >, + rq: dropshot::TypedBody, ) -> Result; } diff --git a/dns-server/Cargo.toml b/dns-server/Cargo.toml index 864746d49ec..fc43ef463ed 100644 --- a/dns-server/Cargo.toml +++ b/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-versions.workspace = true omicron-common.workspace = true oxide-tokio-rt.workspace = true pretty-hex.workspace = true diff --git a/dns-server/src/http_server.rs b/dns-server/src/http_server.rs index e1ed4fe9b76..c001d9e1369 100644 --- a/dns-server/src/http_server.rs +++ b/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_versions::{ + 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(), diff --git a/dns-server/tests/cross_version_test.rs b/dns-server/tests/cross_version_test.rs index 835bbc6cd9a..a0b0d92b047 100644 --- a/dns-server/tests/cross_version_test.rs +++ b/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_versions::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_versions::v1::config::DnsRecord as V1DnsRecord; + use internal_dns_types_versions::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}."); diff --git a/internal-dns/types/Cargo.toml b/internal-dns/types/Cargo.toml index 2be2de13e22..b5a9d85b066 100644 --- a/internal-dns/types/Cargo.toml +++ b/internal-dns/types/Cargo.toml @@ -9,6 +9,7 @@ workspace = true [dependencies] anyhow.workspace = true chrono.workspace = true +internal-dns-types-versions.workspace = true omicron-common.workspace = true omicron-workspace-hack.workspace = true omicron-uuid-kinds.workspace = true diff --git a/internal-dns/types/src/config.rs b/internal-dns/types/src/config.rs index 961d4fac554..d5bef144343 100644 --- a/internal-dns/types/src/config.rs +++ b/internal-dns/types/src/config.rs @@ -69,9 +69,9 @@ use omicron_uuid_kinds::{OmicronZoneUuid, SledUuid}; use std::collections::BTreeMap; use std::net::{Ipv6Addr, SocketAddrV6}; -// "v2" types are the most recent, so we re-export them here for dependents that +// Re-export the latest versions from the versions crate for dependents that // just want "latest". -pub use crate::v2::config::*; +pub use internal_dns_types_versions::latest::config::*; /// Used to construct the DNS name for a control plane host #[derive(Clone, Debug, PartialEq, PartialOrd)] diff --git a/internal-dns/types/src/lib.rs b/internal-dns/types/src/lib.rs index 9feba356f8a..fb86fdf1a13 100644 --- a/internal-dns/types/src/lib.rs +++ b/internal-dns/types/src/lib.rs @@ -7,29 +7,7 @@ //! //! ## Organization //! -//! Some types in this crate are exposed by its dependents as part of versioned -//! HTTP interfaces, such as `dns-server`'s management interfaces. In those -//! cases we may want to support multiple HTTP interface versions concurrently, -//! and so this crate preserves old versions of select public items used in this -//! way. -//! -//! The alternative here would be to require dependents of `internal-dns` to -//! declare duplicate dependencies on `internal-dns` at different revisions. -//! That would force dependents to take all of `internal-dns`' dependencies at -//! versions of interest as transitive dependencies, and precludes maintenance -//! that would otherwise be able to preserve API compatibility of the public -//! types. -//! -//! `cargo xtask openapi` helps us check that we don't unintentionally break an -//! existing committed version, which also helps us be confident that future -//! maintenance on old versions' types does not introduce breaking changes. -//! -//! The top-level items here can be thought of as the "current" version, where -//! versioned items (and their previous versions) are in the `vN` modules with -//! their latest form re-exported as the "current" version. - -pub mod v1; -pub mod v2; +//! See [RFD 619](https://rfd.shared.oxide.computer/rfd/0619). pub mod config; pub mod diff; diff --git a/internal-dns/types/src/v2/mod.rs b/internal-dns/types/src/v2/mod.rs deleted file mode 100644 index 2ff47775af3..00000000000 --- a/internal-dns/types/src/v2/mod.rs +++ /dev/null @@ -1,5 +0,0 @@ -// 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/. - -pub mod config; diff --git a/internal-dns/types/versions/Cargo.toml b/internal-dns/types/versions/Cargo.toml new file mode 100644 index 00000000000..12e492f22cb --- /dev/null +++ b/internal-dns/types/versions/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "internal-dns-types-versions" +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 diff --git a/internal-dns/types/src/v1/config.rs b/internal-dns/types/versions/src/initial/config.rs similarity index 53% rename from internal-dns/types/src/v1/config.rs rename to internal-dns/types/versions/src/initial/config.rs index c5a6c702b0a..564946d5545 100644 --- a/internal-dns/types/src/v1/config.rs +++ b/internal-dns/types/versions/src/initial/config.rs @@ -2,7 +2,6 @@ // 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; use anyhow::ensure; use omicron_common::api::external::Generation; use schemars::JsonSchema; @@ -35,34 +34,6 @@ impl DnsConfigParams { } } -pub enum TranslationError { - GenerationTooLarge, -} - -impl TryInto for DnsConfigParams { - type Error = TranslationError; - - fn try_into(self) -> Result { - let serial: u32 = self - .generation - .as_u64() - .try_into() - .map_err(|_| TranslationError::GenerationTooLarge)?; - - let mut converted_zones: Vec = 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 +42,16 @@ pub struct DnsConfig { pub zones: Vec, } -// 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>, } -impl Into for DnsConfigZone { - fn into(self) -> v2::config::DnsConfigZone { - let converted_records: HashMap> = - self.records - .into_iter() - .filter_map(|(name, name_records)| { - let converted_name_records: Vec = - 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 +75,6 @@ pub enum DnsRecord { Srv(Srv), } -impl Into 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` and `From` implementations are very slightly // dubious, because a v4 or v6 address could also theoretically map to a DNS // PTR record @@ -180,14 +117,3 @@ pub struct Srv { pub port: u16, pub target: String, } - -impl From for Srv { - fn from(other: v2::config::Srv) -> Self { - Srv { - prio: other.prio, - weight: other.weight, - port: other.port, - target: other.target, - } - } -} diff --git a/internal-dns/types/src/v1/mod.rs b/internal-dns/types/versions/src/initial/mod.rs similarity index 78% rename from internal-dns/types/src/v1/mod.rs rename to internal-dns/types/versions/src/initial/mod.rs index 2ff47775af3..83135dfe7eb 100644 --- a/internal-dns/types/src/v1/mod.rs +++ b/internal-dns/types/versions/src/initial/mod.rs @@ -2,4 +2,6 @@ // 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/. +//! DNS configuration types for API version 1.0.0 (INITIAL). + pub mod config; diff --git a/internal-dns/types/versions/src/latest.rs b/internal-dns/types/versions/src/latest.rs new file mode 100644 index 00000000000..4dc2c18817c --- /dev/null +++ b/internal-dns/types/versions/src/latest.rs @@ -0,0 +1,13 @@ +// 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/. + +//! Re-exports of the latest versions of each type. + +pub mod config { + pub use crate::v2::config::DnsConfig; + pub use crate::v2::config::DnsConfigParams; + pub use crate::v2::config::DnsConfigZone; + pub use crate::v2::config::DnsRecord; + pub use crate::v2::config::Srv; +} diff --git a/internal-dns/types/versions/src/lib.rs b/internal-dns/types/versions/src/lib.rs new file mode 100644 index 00000000000..d990e0778e7 --- /dev/null +++ b/internal-dns/types/versions/src/lib.rs @@ -0,0 +1,16 @@ +// 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/. + +//! Versioned types for the DNS server API. +//! +//! # Organization +//! +//! Types are organized based on the rules outlined in [RFD +//! 619](https://rfd.shared.oxide.computer/rfd/0619). + +pub mod latest; +#[path = "initial/mod.rs"] +pub mod v1; +#[path = "soa_and_ns/mod.rs"] +pub mod v2; diff --git a/internal-dns/types/src/v2/config.rs b/internal-dns/types/versions/src/soa_and_ns/config.rs similarity index 68% rename from internal-dns/types/src/v2/config.rs rename to internal-dns/types/versions/src/soa_and_ns/config.rs index a787ed5c3b9..a87152abea8 100644 --- a/internal-dns/types/src/v2/config.rs +++ b/internal-dns/types/versions/src/soa_and_ns/config.rs @@ -51,32 +51,17 @@ pub struct DnsConfig { pub zones: Vec, } -pub enum TranslationError { - IncompatibleRecord, +/// Error type for conversions from v1 to v2. +pub enum V1ToV2TranslationError { + /// The generation number is too large to fit in a u32 serial field. + GenerationTooLarge, } -impl TryFrom for v1::config::DnsConfig { - type Error = TranslationError; - - fn try_from(v2: DnsConfig) -> Result { - let DnsConfig { - generation, - serial: _, - time_created, - time_applied, - zones, - } = v2; - - Ok(v1::config::DnsConfig { - generation, - time_created, - time_applied, - zones: zones - .into_iter() - .map(|zone| zone.try_into()) - .collect::, _>>()?, - }) - } +/// Error type for conversions from v2 to v1. +pub enum V2ToV1TranslationError { + /// The configuration contains records (such as NS records) that cannot + /// be represented in v1. + IncompatibleRecord, } /// Configuration for a specific DNS zone, as opposed to illumos zones in which @@ -94,28 +79,6 @@ pub struct DnsConfigZone { pub records: HashMap>, } -impl TryFrom for v1::config::DnsConfigZone { - type Error = TranslationError; - - fn try_from(v2: DnsConfigZone) -> Result { - let DnsConfigZone { zone_name, records } = v2; - - Ok(v1::config::DnsConfigZone { - zone_name, - records: records - .into_iter() - .map(|(name, records)| { - let converted_records = records - .into_iter() - .map(|v| v.try_into()) - .collect::>(); - converted_records.map(|records| (name, records)) - }) - .collect::>()?, - }) - } -} - #[derive( Clone, Debug, @@ -141,19 +104,6 @@ pub enum DnsRecord { Ns(String), } -impl TryFrom for v1::config::DnsRecord { - type Error = TranslationError; - - fn try_from(v2: DnsRecord) -> Result { - match v2 { - DnsRecord::A(ip) => Ok(v1::config::DnsRecord::A(ip)), - DnsRecord::Aaaa(ip) => Ok(v1::config::DnsRecord::Aaaa(ip)), - DnsRecord::Srv(srv) => Ok(v1::config::DnsRecord::Srv(srv.into())), - DnsRecord::Ns(_) => Err(TranslationError::IncompatibleRecord), - } - } -} - // The `From` and `From` implementations are very slightly // dubious, because a v4 or v6 address could also theoretically map to a DNS // PTR record @@ -197,13 +147,134 @@ pub struct Srv { pub target: String, } +impl TryFrom for DnsConfigParams { + type Error = V1ToV2TranslationError; + + fn try_from(v1: v1::config::DnsConfigParams) -> Result { + let serial: u32 = v1 + .generation + .as_u64() + .try_into() + .map_err(|_| V1ToV2TranslationError::GenerationTooLarge)?; + + let converted_zones: Vec = + v1.zones.into_iter().map(|zone| zone.into()).collect(); + + Ok(DnsConfigParams { + generation: v1.generation, + serial, + time_created: v1.time_created, + zones: converted_zones, + }) + } +} + +impl From for DnsConfigZone { + fn from(v1: v1::config::DnsConfigZone) -> Self { + let converted_records: HashMap> = v1 + .records + .into_iter() + .filter_map(|(name, name_records)| { + let converted_name_records: Vec = + name_records.into_iter().map(|rec| rec.into()).collect(); + if converted_name_records.is_empty() { + None + } else { + Some((name, converted_name_records)) + } + }) + .collect(); + DnsConfigZone { zone_name: v1.zone_name, records: converted_records } + } +} + +impl From for DnsRecord { + fn from(v1: v1::config::DnsRecord) -> Self { + match v1 { + v1::config::DnsRecord::A(ip) => DnsRecord::A(ip), + v1::config::DnsRecord::Aaaa(ip) => DnsRecord::Aaaa(ip), + v1::config::DnsRecord::Srv(srv) => DnsRecord::Srv(srv.into()), + } + } +} + impl From for Srv { - fn from(other: v1::config::Srv) -> Self { + fn from(v1: v1::config::Srv) -> Self { Srv { - prio: other.prio, - weight: other.weight, - port: other.port, - target: other.target, + prio: v1.prio, + weight: v1.weight, + port: v1.port, + target: v1.target, + } + } +} + +impl TryFrom for v1::config::DnsConfig { + type Error = V2ToV1TranslationError; + + fn try_from(v2: DnsConfig) -> Result { + let DnsConfig { + generation, + serial: _, + time_created, + time_applied, + zones, + } = v2; + + Ok(v1::config::DnsConfig { + generation, + time_created, + time_applied, + zones: zones + .into_iter() + .map(|zone| zone.try_into()) + .collect::, _>>()?, + }) + } +} + +impl TryFrom for v1::config::DnsConfigZone { + type Error = V2ToV1TranslationError; + + fn try_from(v2: DnsConfigZone) -> Result { + let DnsConfigZone { zone_name, records } = v2; + + Ok(v1::config::DnsConfigZone { + zone_name, + records: records + .into_iter() + .map(|(name, records)| { + let converted_records = records + .into_iter() + .map(|v| v.try_into()) + .collect::>(); + converted_records.map(|records| (name, records)) + }) + .collect::>()?, + }) + } +} + +impl TryFrom for v1::config::DnsRecord { + type Error = V2ToV1TranslationError; + + fn try_from(v2: DnsRecord) -> Result { + match v2 { + DnsRecord::A(ip) => Ok(v1::config::DnsRecord::A(ip)), + DnsRecord::Aaaa(ip) => Ok(v1::config::DnsRecord::Aaaa(ip)), + DnsRecord::Srv(srv) => Ok(v1::config::DnsRecord::Srv(srv.into())), + DnsRecord::Ns(_) => Err(V2ToV1TranslationError::IncompatibleRecord), + } + } +} + +impl From for v1::config::Srv { + fn from(v2: Srv) -> Self { + v1::config::Srv { + prio: v2.prio, + weight: v2.weight, + port: v2.port, + target: v2.target, } } } diff --git a/internal-dns/types/versions/src/soa_and_ns/mod.rs b/internal-dns/types/versions/src/soa_and_ns/mod.rs new file mode 100644 index 00000000000..5590f7512ac --- /dev/null +++ b/internal-dns/types/versions/src/soa_and_ns/mod.rs @@ -0,0 +1,13 @@ +// 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/. + +//! DNS configuration types for API version 2.0.0 (SOA_AND_NS). +//! +//! This version adds: +//! +//! - A `serial` field to [`config::DnsConfigParams`] and +//! [`config::DnsConfig`] for SOA records. +//! - The [`config::DnsRecord::Ns`] variant for nameserver records. + +pub mod config;