diff --git a/bottlerocket-settings-sdk/src/extension/mod.rs b/bottlerocket-settings-sdk/src/extension/mod.rs index 0401ce20..4ae53c89 100644 --- a/bottlerocket-settings-sdk/src/extension/mod.rs +++ b/bottlerocket-settings-sdk/src/extension/mod.rs @@ -62,7 +62,7 @@ where Ok(extension) } - /// Converts a list of models into a map of Version => Model while checing for uniqueness. + /// Converts a list of models into a map of Version => Model while checking for uniqueness. fn build_model_map( models: Vec, ) -> Result, SettingsExtensionError> { diff --git a/bottlerocket-settings-sdk/src/lib.rs b/bottlerocket-settings-sdk/src/lib.rs index 51c1483d..6d703a98 100644 --- a/bottlerocket-settings-sdk/src/lib.rs +++ b/bottlerocket-settings-sdk/src/lib.rs @@ -33,7 +33,7 @@ pub use helper::{template_helper, HelperDef, HelperError}; #[cfg(feature = "extension")] pub use migrate::{ LinearMigrator, LinearMigratorExtensionBuilder, LinearMigratorModel, LinearlyMigrateable, - Migrator, NoMigration, + Migrator, NoMigration, NullMigrator, NullMigratorExtensionBuilder, }; pub use model::{BottlerocketSetting, GenerateResult, SettingsModel}; diff --git a/bottlerocket-settings-sdk/src/migrate/mod.rs b/bottlerocket-settings-sdk/src/migrate/mod.rs index 2f5760eb..e46e37f9 100644 --- a/bottlerocket-settings-sdk/src/migrate/mod.rs +++ b/bottlerocket-settings-sdk/src/migrate/mod.rs @@ -16,6 +16,11 @@ pub use linear::{ LinearMigrator, LinearMigratorExtensionBuilder, LinearMigratorModel, LinearlyMigrateable, }; +pub mod null; +pub use null::{ + NullMigrator, NullMigratorExtensionBuilder, +}; + /// Implementors of the `Migrator` trait inform a [`SettingsExtension`](crate::SettingsExtension) /// how to migrate settings values between different versions. pub trait Migrator: Debug { diff --git a/bottlerocket-settings-sdk/src/migrate/null/extensionbuilder.rs b/bottlerocket-settings-sdk/src/migrate/null/extensionbuilder.rs new file mode 100644 index 00000000..1ad904d1 --- /dev/null +++ b/bottlerocket-settings-sdk/src/migrate/null/extensionbuilder.rs @@ -0,0 +1,9 @@ +use super::NullMigrator; +use crate::extension_builder; + +extension_builder!( + pub, + NullMigratorExtensionBuilder, + NullMigrator, + NullMigrator +); diff --git a/bottlerocket-settings-sdk/src/migrate/null/mod.rs b/bottlerocket-settings-sdk/src/migrate/null/mod.rs new file mode 100644 index 00000000..13831da0 --- /dev/null +++ b/bottlerocket-settings-sdk/src/migrate/null/mod.rs @@ -0,0 +1,87 @@ +//! Provides a `NullMigrator` for settings that do not require migration, e.g. settings with a +//! single version. +use std::any::Any; +use crate::migrate::{MigrationResult, ModelStore}; +use crate::Migrator; +use crate::model::{AsTypeErasedModel, TypeErasedModel}; + +mod extensionbuilder; + +pub use error::NullMigratorError; +pub use extensionbuilder::NullMigratorExtensionBuilder; + +/// `NullMigrator` is to be used for settings that do not require migration, e.g. settings with a +/// single version. For cases where multiple versions of a setting are required, you should use a +/// different Migrator, such as `LinearMigrator`, and define migrations between each version. +/// +/// As `NullMigrator` takes anything that implements `TypeErasedModel`, it can be used with any +/// existing `SettingsModel` without needing to implement any additional traits. +#[derive(Default, Debug, Clone)] +pub struct NullMigrator; + +impl Migrator for NullMigrator { + type ErrorKind = NullMigratorError; + type ModelKind = Box; + + /// Asserts that the `NullMigrator` is only used with a single version of a model. For cases + /// where multiple versions are required, you should use a different migrator, such as + /// `LinearMigrator`. + fn validate_migrations( + &self, + models: &dyn ModelStore + ) -> Result<(), Self::ErrorKind> { + if models.len() != 1 { + return Err(NullMigratorError::TooManyModelVersions); + } + Ok(()) + } + + /// Always returns a `NoMigration` error. Extensions that use `NullMigrator` should never need + /// to migrate. + fn perform_migration( + &self, + _models: &dyn ModelStore, + _starting_value: Box, + _starting_version: &str, + _target_version: &str + ) -> Result { + Err(NullMigratorError::NoMigration) + } + + /// Always returns a `NoMigration` error. Extensions that use `NullMigrator` should never need + /// to migrate. + fn perform_flood_migrations( + &self, + _models: &dyn ModelStore, + _starting_value: Box, + _starting_version: &str + ) -> Result, Self::ErrorKind> { + Err(NullMigratorError::NoMigration) + } +} + +// Needed to satisfy the type constraints of `ModelKind` in `Migrator`. Unfortunately, `Box` has no +// way of providing all traits implemented by the type it points to, so we need to reimplement this +// trait ourselves. +impl AsTypeErasedModel for Box { + fn as_model(&self) -> &dyn TypeErasedModel { + self.as_ref() + } +} + +mod error { + #![allow(missing_docs)] + use snafu::Snafu; + + /// The error type returned by `NullMigrator`. + #[derive(Debug, Snafu)] + #[snafu(visibility(pub))] + pub enum NullMigratorError { + #[snafu(display("No migration to perform"))] + NoMigration, + + #[snafu(display("NullMigrator cannot be used with models with multiple versions"))] + TooManyModelVersions, + } + +} diff --git a/bottlerocket-settings-sdk/tests/migration_validation/linear/mod.rs b/bottlerocket-settings-sdk/tests/migration_validation/linear/mod.rs new file mode 100644 index 00000000..575aad9a --- /dev/null +++ b/bottlerocket-settings-sdk/tests/migration_validation/linear/mod.rs @@ -0,0 +1,201 @@ +use anyhow::Result; +use bottlerocket_settings_sdk::{ + extension::SettingsExtensionError, BottlerocketSetting, GenerateResult, + LinearMigratorExtensionBuilder, LinearlyMigrateable, NoMigration, SettingsModel, +}; +use serde::{Deserialize, Serialize}; + +use super::*; + +macro_rules! define_model { + ($name:ident, $version:expr, $forward:ident, $backward:ident) => { + common::define_model!($name, $version); + + impl LinearlyMigrateable for $name { + type ForwardMigrationTarget = $forward; + type BackwardMigrationTarget = $backward; + + fn migrate_forward(&self) -> Result { + unimplemented!() + } + + fn migrate_backward(&self) -> Result { + unimplemented!() + } + } + }; +} + +define_model!(DisjointA, "v1", NoMigration, NoMigration); +define_model!(DisjointB, "v2", NoMigration, NoMigration); + +#[test] +fn test_no_small_disjoint_islands() { + // Given two models which do not link in a migration chain, + // When an linear migrator extension is built with those models, + // The extension will fail to build. + + assert!(matches!( + LinearMigratorExtensionBuilder::with_name("disjoint-models") + .with_models(vec![ + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + ]) + .build(), + Err(SettingsExtensionError::MigrationValidation { .. }) + )); +} + +// A <-> B <-> D +// E <-> C <-> A +define_model!(LargeDisjointA, "v1", LargeDisjointB, NoMigration); +define_model!(LargeDisjointB, "v2", LargeDisjointD, LargeDisjointA); +define_model!(LargeDisjointC, "v3", LargeDisjointA, LargeDisjointE); +define_model!(LargeDisjointD, "v4", NoMigration, LargeDisjointB); +define_model!(LargeDisjointE, "v5", NoMigration, LargeDisjointC); + +#[test] +fn test_no_large_disjoint_islands() { + assert!(matches!( + LinearMigratorExtensionBuilder::with_name("disjoint-models") + .with_models(vec![ + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + ]) + .build(), + Err(SettingsExtensionError::MigrationValidation { .. }) + )); +} + +// A <-> C <-> D +// B ---^ +define_model!(DoubleTailedA, "v1", DoubleTailedC, NoMigration); +define_model!(DoubleTailedB, "v2", DoubleTailedC, NoMigration); +define_model!(DoubleTailedC, "v3", DoubleTailedD, DoubleTailedA); +define_model!(DoubleTailedD, "v4", NoMigration, DoubleTailedC); + +#[test] +fn test_no_double_tail() { + assert!(matches!( + LinearMigratorExtensionBuilder::with_name("disjoint-models") + .with_models(vec![ + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + ]) + .build(), + Err(SettingsExtensionError::MigrationValidation { .. }) + )); +} + +// C <-> A <-> B <-> C +define_model!(LoopA, "v1", LoopC, LoopB); +define_model!(LoopB, "v2", LoopA, LoopC); +define_model!(LoopC, "v3", LoopB, LoopA); + +#[test] +fn test_no_migration_loops_simple_circle() { + // Given a simple loop of linear migrations between models, + // When an linear migrator extension is built with those models, + // The extension will fail to build. + + assert!(matches!( + LinearMigratorExtensionBuilder::with_name("circular-loop") + .with_models(vec![ + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + ]) + .build(), + Err(SettingsExtensionError::MigrationValidation { .. }) + )); +} + +// A <-> B -> C +// ^----------| +define_model!(BrokenBacklinkA, "v1", NoMigration, LoopB); +define_model!(BrokenBacklinkB, "v2", LoopA, LoopC); +// C mistakenly points back to A +define_model!(BrokenBacklinkC, "v3", LoopA, NoMigration); + +#[test] +fn test_no_migration_loops_backlink() { + // Given a set of models with a backwards migration resulting in a loop, + // When an linear migrator extension is built with those models, + // The extension will fail to build. + + assert!(matches!( + LinearMigratorExtensionBuilder::with_name("broken-backlink") + .with_models(vec![ + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + ]) + .build(), + Err(SettingsExtensionError::MigrationValidation { .. }) + )); +} + +// A mistakenly points back to C +define_model!(BackwardsCycleA, "v1", BackwardsCycleC, BackwardsCycleB); +define_model!(BackwardsCycleB, "v2", BackwardsCycleA, BackwardsCycleC); +define_model!(BackwardsCycleC, "v3", BackwardsCycleB, NoMigration); + +#[test] +fn test_no_migration_loops_backcycle() { + assert!(matches!( + LinearMigratorExtensionBuilder::with_name("backcycle") + .with_models(vec![ + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + ]) + .build(), + Err(SettingsExtensionError::MigrationValidation { .. }) + )); +} + +define_model!(ForwardsCycleA, "v1", NoMigration, ForwardsCycleB); +define_model!(ForwardsCycleB, "v2", ForwardsCycleA, ForwardsCycleC); +// C mistakenly points forward to A +define_model!(ForwardsCycleC, "v3", ForwardsCycleB, ForwardsCycleA); + +#[test] +fn test_no_migration_loops_forwardcycle() { + assert!(matches!( + LinearMigratorExtensionBuilder::with_name("forwards-cycle") + .with_models(vec![ + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + ]) + .build(), + Err(SettingsExtensionError::MigrationValidation { .. }) + )); +} + +// A -> B -> C -> D +// A <- C <- B <- D +define_model!(NotReversibleA, "v1", NotReversibleB, NoMigration); +define_model!(NotReversibleB, "v2", NotReversibleC, NotReversibleC); +define_model!(NotReversibleC, "v3", NotReversibleD, NotReversibleA); +define_model!(NotReversibleD, "v4", NoMigration, NotReversibleB); + +#[test] +fn test_no_non_reversible() { + assert!(matches!( + LinearMigratorExtensionBuilder::with_name("not-reversible") + .with_models(vec![ + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + ]) + .build(), + Err(SettingsExtensionError::MigrationValidation { .. }) + )); +} diff --git a/bottlerocket-settings-sdk/tests/migration_validation/mod.rs b/bottlerocket-settings-sdk/tests/migration_validation/mod.rs index b974c5d7..d45f6e71 100644 --- a/bottlerocket-settings-sdk/tests/migration_validation/mod.rs +++ b/bottlerocket-settings-sdk/tests/migration_validation/mod.rs @@ -1,224 +1,37 @@ -use anyhow::Result; -use bottlerocket_settings_sdk::{ - extension::SettingsExtensionError, BottlerocketSetting, GenerateResult, - LinearMigratorExtensionBuilder, LinearlyMigrateable, NoMigration, SettingsModel, -}; -use serde::{Deserialize, Serialize}; - -macro_rules! define_model { - ($name:ident, $version:expr, $forward:ident, $backward:ident) => { - #[derive(Debug, Serialize, Deserialize, Default, PartialEq, Eq)] - pub struct $name; - - impl SettingsModel for $name { - type PartialKind = Self; - type ErrorKind = anyhow::Error; - - fn get_version() -> &'static str { - $version - } - - fn set(_: Option, _: Self) -> Result<()> { - unimplemented!() - } - - fn generate( - _: Option, - _: Option, - ) -> Result> { - unimplemented!() - } - - fn validate(_: Self, _: Option) -> Result<()> { - unimplemented!() +mod linear; +mod null; + +mod common { + macro_rules! define_model { + ($name:ident, $version:expr) => { + #[derive(Debug, Serialize, Deserialize, Default, PartialEq, Eq)] + pub struct $name; + + impl SettingsModel for $name { + type PartialKind = Self; + type ErrorKind = anyhow::Error; + + fn get_version() -> &'static str { + $version + } + + fn set(_: Option, _: Self) -> Result<()> { + unimplemented!() + } + + fn generate( + _: Option, + _: Option, + ) -> Result> { + unimplemented!() + } + + fn validate(_: Self, _: Option) -> Result<()> { + unimplemented!() + } } } + } - impl LinearlyMigrateable for $name { - type ForwardMigrationTarget = $forward; - type BackwardMigrationTarget = $backward; - - fn migrate_forward(&self) -> Result { - unimplemented!() - } - - fn migrate_backward(&self) -> Result { - unimplemented!() - } - } - }; -} - -define_model!(DisjointA, "v1", NoMigration, NoMigration); -define_model!(DisjointB, "v2", NoMigration, NoMigration); - -#[test] -fn test_no_small_disjoint_islands() { - // Given two models which do not link in a migration chain, - // When an linear migrator extension is built with those models, - // The extension will fail to build. - - assert!(matches!( - LinearMigratorExtensionBuilder::with_name("disjoint-models") - .with_models(vec![ - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - ]) - .build(), - Err(SettingsExtensionError::MigrationValidation { .. }) - )); -} - -// A <-> B <-> D -// E <-> C <-> A -define_model!(LargeDisjointA, "v1", LargeDisjointB, NoMigration); -define_model!(LargeDisjointB, "v2", LargeDisjointD, LargeDisjointA); -define_model!(LargeDisjointC, "v3", LargeDisjointA, LargeDisjointE); -define_model!(LargeDisjointD, "v4", NoMigration, LargeDisjointB); -define_model!(LargeDisjointE, "v5", NoMigration, LargeDisjointC); - -#[test] -fn test_no_large_disjoint_islands() { - assert!(matches!( - LinearMigratorExtensionBuilder::with_name("disjoint-models") - .with_models(vec![ - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - ]) - .build(), - Err(SettingsExtensionError::MigrationValidation { .. }) - )); -} - -// A <-> C <-> D -// B ---^ -define_model!(DoubleTailedA, "v1", DoubleTailedC, NoMigration); -define_model!(DoubleTailedB, "v2", DoubleTailedC, NoMigration); -define_model!(DoubleTailedC, "v3", DoubleTailedD, DoubleTailedA); -define_model!(DoubleTailedD, "v4", NoMigration, DoubleTailedC); - -#[test] -fn test_no_double_tail() { - assert!(matches!( - LinearMigratorExtensionBuilder::with_name("disjoint-models") - .with_models(vec![ - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - ]) - .build(), - Err(SettingsExtensionError::MigrationValidation { .. }) - )); -} - -// C <-> A <-> B <-> C -define_model!(LoopA, "v1", LoopC, LoopB); -define_model!(LoopB, "v2", LoopA, LoopC); -define_model!(LoopC, "v3", LoopB, LoopA); - -#[test] -fn test_no_migration_loops_simple_circle() { - // Given a simple loop of linear migrations between models, - // When an linear migrator extension is built with those models, - // The extension will fail to build. - - assert!(matches!( - LinearMigratorExtensionBuilder::with_name("circular-loop") - .with_models(vec![ - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - ]) - .build(), - Err(SettingsExtensionError::MigrationValidation { .. }) - )); -} - -// A <-> B -> C -// ^----------| -define_model!(BrokenBacklinkA, "v1", NoMigration, LoopB); -define_model!(BrokenBacklinkB, "v2", LoopA, LoopC); -// C mistakenly points back to A -define_model!(BrokenBacklinkC, "v3", LoopA, NoMigration); - -#[test] -fn test_no_migration_loops_backlink() { - // Given a set of models with a backwards migration resulting in a loop, - // When an linear migrator extension is built with those models, - // The extension will fail to build. - - assert!(matches!( - LinearMigratorExtensionBuilder::with_name("broken-backlink") - .with_models(vec![ - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - ]) - .build(), - Err(SettingsExtensionError::MigrationValidation { .. }) - )); -} - -// A mistakenly points back to C -define_model!(BackwardsCycleA, "v1", BackwardsCycleC, BackwardsCycleB); -define_model!(BackwardsCycleB, "v2", BackwardsCycleA, BackwardsCycleC); -define_model!(BackwardsCycleC, "v3", BackwardsCycleB, NoMigration); - -#[test] -fn test_no_migration_loops_backcycle() { - assert!(matches!( - LinearMigratorExtensionBuilder::with_name("backcycle") - .with_models(vec![ - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - ]) - .build(), - Err(SettingsExtensionError::MigrationValidation { .. }) - )); -} - -define_model!(ForwardsCycleA, "v1", NoMigration, ForwardsCycleB); -define_model!(ForwardsCycleB, "v2", ForwardsCycleA, ForwardsCycleC); -// C mistakenly points forward to A -define_model!(ForwardsCycleC, "v3", ForwardsCycleB, ForwardsCycleA); - -#[test] -fn test_no_migration_loops_forwardcycle() { - assert!(matches!( - LinearMigratorExtensionBuilder::with_name("forwards-cycle") - .with_models(vec![ - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - ]) - .build(), - Err(SettingsExtensionError::MigrationValidation { .. }) - )); -} - -// A -> B -> C -> D -// A <- C <- B <- D -define_model!(NotReversibleA, "v1", NotReversibleB, NoMigration); -define_model!(NotReversibleB, "v2", NotReversibleC, NotReversibleC); -define_model!(NotReversibleC, "v3", NotReversibleD, NotReversibleA); -define_model!(NotReversibleD, "v4", NoMigration, NotReversibleB); - -#[test] -fn test_no_non_reversible() { - assert!(matches!( - LinearMigratorExtensionBuilder::with_name("not-reversible") - .with_models(vec![ - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - BottlerocketSetting::::model(), - ]) - .build(), - Err(SettingsExtensionError::MigrationValidation { .. }) - )); + pub(crate) use define_model; } diff --git a/bottlerocket-settings-sdk/tests/migration_validation/null/mod.rs b/bottlerocket-settings-sdk/tests/migration_validation/null/mod.rs new file mode 100644 index 00000000..411b1a9a --- /dev/null +++ b/bottlerocket-settings-sdk/tests/migration_validation/null/mod.rs @@ -0,0 +1,34 @@ +use anyhow::Result; +use bottlerocket_settings_sdk::{ + extension::SettingsExtensionError, BottlerocketSetting, GenerateResult, SettingsModel, + NullMigratorExtensionBuilder, +}; +use serde::{Deserialize, Serialize}; + +use super::common::define_model; + +define_model!(NullModelA, "v1"); +define_model!(NullModelB, "v2"); +#[test] +fn test_single_model() { + assert!(matches!( + NullMigratorExtensionBuilder::with_name("null-migrator") + .with_models(vec![ + BottlerocketSetting::::model(), + ]) + .build(), + Ok(_) + )); +} + +#[test] +fn test_multiple_models() { + assert!(matches!( + NullMigratorExtensionBuilder::with_name("multiple-models") + .with_models(vec![ + BottlerocketSetting::::model(), + BottlerocketSetting::::model(), + ]).build(), + Err(SettingsExtensionError::MigrationValidation { .. }) + )); +}