Add a Diff Visitor for BlueprintZoneConfigs

This visitor walks a diffus generated diff and provides callbacks
for users to hook into the parts of the diff that they care about.

To keep things minimal, we don't provide callback trait methods for
copies (when nothing changes), and we don't provide methods for most
diffus_derive generated types (the ones that start with `Edited`).

We anticipate composing these visitors to build up a full diff of a
Blueprint. They can also be used individually as demonstrated with the
included property based test.

In order to easily generate `BlueprintZoneConfigs` for property tests we
derived `Arbitrary` for `BlueprintZoneConfigs` and its fields using the
`test-strategy` crate. Some of the types in the hierarchy don't actually
implement `Arbitrary` and they come from foreign crates. In this case a
corresponding generation strategy was added that allows those types to
be generated based on more primitive types that do implement arbitrary.

This PR is just one of many coming to build up and use automated
blueprint diffs. It builds on the following PRs which must be merged
in first:

 * oxidecomputer/newtype-uuid#56
 * oxidecomputer/diffus#8

Cargo.toml

@@ -369,7 +369,8 @@ derive-where = "1.2.7"
 # Having the i-implement-... feature here makes diesel go away from the workspace-hack
 diesel = { version = "2.2.4", features = ["i-implement-a-third-party-backend-and-opt-into-breaking-changes", "postgres", "r2d2", "chrono", "serde_json", "network-address", "uuid"] }
 diesel-dtrace = "0.4.2"
-diffus = { git = "https://github.com/oxidecomputer/diffus", branch = "oxide/main", features = ["uuid-impl", "derive", "newtype-uuid-impl", "oxnet-impl"] }
+#diffus = { git = "https://github.com/oxidecomputer/diffus", branch = "oxide/main", features = ["uuid-impl", "derive", "newtype-uuid-impl", "oxnet-impl"] }
+diffus = { path = "../diffus/diffus", features = ["uuid-impl", "derive", "newtype-uuid-impl", "oxnet-impl"] }
 dns-server = { path = "dns-server" }
 dns-server-api = { path = "dns-server-api" }
 dns-service-client = { path = "clients/dns-service-client" }
@@ -679,7 +680,8 @@ zone = { version = "0.3", default-features = false, features = ["async"] }
 # the kinds). However, uses of omicron-uuid-kinds _within omicron_ will have
 # std and the other features enabled because they'll refer to it via
 # omicron-uuid-kinds.workspace = true.
-newtype-uuid = { version = "1.1.3", default-features = false }
+#newtype-uuid = { version = "1.1.3", default-features = false }
+newtype-uuid = { path = "../newtype-uuid/crates/newtype-uuid", default-features = false }
 omicron-uuid-kinds = { path = "uuid-kinds", features = ["serde", "schemars08", "uuid-v4"] }
 
 # NOTE: The test profile inherits from the dev profile, so settings under

common/src/api/external/mod.rs

@@ -46,6 +46,9 @@ use std::num::{NonZeroU16, NonZeroU32};
 use std::str::FromStr;
 use uuid::Uuid;
 
+#[cfg(any(test, feature = "testing"))]
+use proptest::{arbitrary::any, strategy::Strategy};
+
 // The type aliases below exist primarily to ensure consistency among return
 // types for functions in the `nexus::Nexus` and `nexus::DataStore`.  The
 // type argument `T` generally implements `Object`.
@@ -213,6 +216,7 @@ impl<'a> TryFrom<&DataPageParams<'a, NameOrId>> for DataPageParams<'a, Uuid> {
 #[display("{0}")]
 #[serde(try_from = "String")]
 #[derive(Diffus)]
+#[cfg_attr(any(test, feature = "testing"), derive(test_strategy::Arbitrary))]
 pub struct Name(String);
 
 /// `Name::try_from(String)` is the primary method for constructing an Name
@@ -751,10 +755,28 @@ impl From<ByteCount> for i64 {
     PartialEq,
     PartialOrd,
     Serialize,
-    Diffus,
 )]
+#[cfg_attr(feature = "testing", derive(test_strategy::Arbitrary))]
 pub struct Generation(u64);
 
+// We have to manually implement `Diffable` because this is newtype with private
+// data, and we want to see the diff on the newtype not the inner data.
+impl<'a> Diffable<'a> for Generation {
+    type Diff = (&'a Generation, &'a Generation);
+
+    fn diff(&'a self, other: &'a Self) -> edit::Edit<'a, Self> {
+        if self == other {
+            edit::Edit::Copy(self)
+        } else {
+            edit::Edit::Change {
+                before: self,
+                after: other,
+                diff: (self, other),
+            }
+        }
+    }
+}
+
 impl Generation {
     pub const fn new() -> Generation {
         Generation(1)
@@ -1939,15 +1961,27 @@ impl JsonSchema for L4PortRange {
     SerializeDisplay,
     Hash,
 )]
-pub struct MacAddr(pub macaddr::MacAddr6);
+#[cfg_attr(any(test, feature = "testing"), derive(test_strategy::Arbitrary))]
+pub struct MacAddr(
+    #[cfg_attr(
+        any(test, feature = "testing"),
+        strategy(any::<(u8, u8, u8, u8, u8, u8)>()
+            .prop_map(|(a,b,c,d,e,f)| macaddr::MacAddr6::new(a,b,c,d,e,f))))
+    ]
+    pub macaddr::MacAddr6,
+);
 
 impl<'a> Diffable<'a> for MacAddr {
     type Diff = (&'a Self, &'a Self);
     fn diff(&'a self, other: &'a Self) -> edit::Edit<'a, Self> {
         if self == other {
             edit::Edit::Copy(self)
         } else {
-            edit::Edit::Change((self, other))
+            edit::Edit::Change {
+                before: self,
+                after: other,
+                diff: (self, other),
+            }
         }
     }
 }
@@ -2117,6 +2151,7 @@ impl JsonSchema for MacAddr {
     JsonSchema,
     Diffus,
 )]
+#[cfg_attr(any(test, feature = "testing"), derive(test_strategy::Arbitrary))]
 pub struct Vni(u32);
 
 impl Vni {

common/src/api/internal/shared.rs

@@ -22,6 +22,8 @@ use strum::EnumCount;
 use uuid::Uuid;
 
 use super::nexus::HostIdentifier;
+#[cfg(any(test, feature = "testing"))]
+use proptest::{arbitrary::any, strategy::Strategy};
 
 /// The type of network interface
 #[derive(
@@ -39,13 +41,32 @@ use super::nexus::HostIdentifier;
     Diffus,
 )]
 #[serde(tag = "type", rename_all = "snake_case")]
+#[cfg_attr(any(test, feature = "testing"), derive(test_strategy::Arbitrary))]
 pub enum NetworkInterfaceKind {
     /// A vNIC attached to a guest instance
-    Instance { id: Uuid },
+    Instance {
+        #[cfg_attr(
+            any(test, feature = "testing"),
+            strategy(any::<u128>().prop_map(Uuid::from_u128)))
+        ]
+        id: Uuid,
+    },
     /// A vNIC associated with an internal service
-    Service { id: Uuid },
+    Service {
+        #[cfg_attr(
+            any(test, feature = "testing"),
+            strategy(any::<u128>().prop_map(Uuid::from_u128)))
+        ]
+        id: Uuid,
+    },
     /// A vNIC associated with a probe
-    Probe { id: Uuid },
+    Probe {
+        #[cfg_attr(
+            any(test, feature = "testing"),
+            strategy(any::<u128>().prop_map(Uuid::from_u128)))
+        ]
+        id: Uuid,
+    },
 }
 
 /// Information required to construct a virtual network interface
@@ -62,17 +83,34 @@ pub enum NetworkInterfaceKind {
     Hash,
     Diffus,
 )]
+#[cfg_attr(any(test, feature = "testing"), derive(test_strategy::Arbitrary))]
 pub struct NetworkInterface {
+    #[cfg_attr(
+        any(test, feature = "testing"),
+        strategy(any::<u128>().prop_map(Uuid::from_u128)))
+    ]
     pub id: Uuid,
     pub kind: NetworkInterfaceKind,
     pub name: Name,
     pub ip: IpAddr,
     pub mac: external::MacAddr,
+    #[cfg_attr(
+        any(test, feature = "testing"),
+        strategy(any::<(IpAddr, u8)>()
+            .prop_map(|(addr, prefix)| IpNet::new_unchecked(addr, prefix))))
+    ]
     pub subnet: IpNet,
     pub vni: Vni,
     pub primary: bool,
     pub slot: u8,
     #[serde(default)]
+    #[cfg_attr(
+        any(test, feature = "testing"),
+        strategy(any::<Vec<(IpAddr, u8)>>()
+            .prop_map(|v| v.into_iter()
+                .map(|(addr, prefix)|
+                    IpNet::new_unchecked(addr, prefix)).collect())))
+    ]
     pub transit_ips: Vec<IpNet>,
 }
 
@@ -93,6 +131,7 @@ pub struct NetworkInterface {
     Hash,
     Diffus,
 )]
+#[cfg_attr(any(test, feature = "testing"), derive(test_strategy::Arbitrary))]
 pub struct SourceNatConfig {
     /// The external address provided to the instance or service.
     pub ip: IpAddr,
@@ -898,7 +937,7 @@ pub struct ExternalIpGatewayMap {
 #[derive(
     Debug, Clone, PartialEq, Eq, Ord, PartialOrd, Hash, EnumCount, Diffus,
 )]
-#[cfg_attr(feature = "testing", derive(test_strategy::Arbitrary))]
+#[cfg_attr(any(test, feature = "testing"), derive(test_strategy::Arbitrary))]
 pub enum DatasetKind {
     // Durable datasets for zones
     Cockroach,

common/src/zpool_name.rs

@@ -19,6 +19,7 @@ pub const ZPOOL_INTERNAL_PREFIX: &str = "oxi_";
     Copy, Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord, JsonSchema, Diffus,
 )]
 #[serde(rename_all = "snake_case")]
+#[cfg_attr(feature = "testing", derive(test_strategy::Arbitrary))]
 pub enum ZpoolKind {
     // This zpool is used for external storage (u.2)
     External,
@@ -31,6 +32,7 @@ pub enum ZpoolKind {
 /// This expects that the format will be: `ox{i,p}_<UUID>` - we parse the prefix
 /// when reading the structure, and validate that the UUID can be utilized.
 #[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord, Diffus)]
+#[cfg_attr(feature = "testing", derive(test_strategy::Arbitrary))]
 pub struct ZpoolName {
     id: ZpoolUuid,
     kind: ZpoolKind,

nexus-sled-agent-shared/Cargo.toml

@@ -13,11 +13,16 @@ omicron-common.workspace = true
 omicron-passwords.workspace = true
 omicron-uuid-kinds.workspace = true
 omicron-workspace-hack.workspace = true
+proptest = { workspace = true, optional = true }
 # TODO: replace uses of propolis_client with local types
 schemars.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 sled-hardware-types.workspace = true
 strum.workspace = true
+test-strategy = { workspace = true, optional = true }
 thiserror.workspace = true
 uuid.workspace = true
+
+[features]
+testing = ["proptest", "test-strategy"]

nexus-sled-agent-shared/src/inventory.rs

@@ -186,6 +186,7 @@ impl OmicronZoneConfig {
     Hash,
     Diffus,
 )]
+#[cfg_attr(any(test, feature = "testing"), derive(test_strategy::Arbitrary))]
 pub struct OmicronZoneDataset {
     pub pool_name: ZpoolName,
 }

nexus/types/Cargo.toml

@@ -30,6 +30,7 @@ openssl.workspace = true
 oxql-types.workspace = true
 oxnet.workspace = true
 parse-display.workspace = true
+proptest = { workspace = true, optional = true }
 schemars = { workspace = true, features = ["chrono", "uuid1"] }
 serde.workspace = true
 serde_json.workspace = true
@@ -38,6 +39,7 @@ slog.workspace = true
 slog-error-chain.workspace = true
 steno.workspace = true
 strum.workspace = true
+test-strategy = { workspace = true, optional = true }
 thiserror.workspace = true
 newtype-uuid.workspace = true
 update-engine.workspace = true
@@ -55,5 +57,11 @@ omicron-workspace-hack.workspace = true
 # common to both, put them in `omicron-common` or `nexus-sled-agent-shared`.
 
 [dev-dependencies]
+nexus-sled-agent-shared = { workspace = true, features = ["testing"] }
+omicron-uuid-kinds = { workspace = true, features = ["testing"] }
+omicron-common = { workspace = true, features = ["testing"] }
 proptest.workspace = true
 test-strategy.workspace = true
+
+[features]
+testing = ["proptest", "test-strategy"]