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 a Diff Visitor for BlueprintZoneConfigs #7336

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and newtype-uuid below must be merged first and this must be updated.

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" }
Expand Down Expand Up @@ -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
Expand Down
41 changes: 38 additions & 3 deletions common/src/api/external/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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),
}
}
}
}
Expand Down Expand Up @@ -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 {
Expand Down
47 changes: 43 additions & 4 deletions common/src/api/internal/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand All @@ -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>,
}

Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions common/src/zpool_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions nexus-sled-agent-shared/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
1 change: 1 addition & 0 deletions nexus-sled-agent-shared/src/inventory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
8 changes: 8 additions & 0 deletions nexus/types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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"]
Loading
Loading