Add development taint flag to prevent mismatch of server versions (#2821)

* Add development taint flag to prevent mismatch of server versions
* Update server/lib/src/constants/schema.rs

---------

Co-authored-by: James Hodgkinson <james@terminaloutcomes.com>

proto/src/constants.rs

@@ -75,6 +75,7 @@ pub const ATTR_DESCRIPTION: &str = "description";
 pub const ATTR_DIRECTMEMBEROF: &str = "directmemberof";
 pub const ATTR_DISPLAYNAME: &str = "displayname";
 pub const ATTR_DN: &str = "dn";
+pub const ATTR_DOMAIN_DEVELOPMENT_TAINT: &str = "domain_development_taint";
 pub const ATTR_DOMAIN_DISPLAY_NAME: &str = "domain_display_name";
 pub const ATTR_DOMAIN_LDAP_BASEDN: &str = "domain_ldap_basedn";
 pub const ATTR_DOMAIN_NAME: &str = "domain_name";

server/lib/src/constants/entries.rs

@@ -72,6 +72,7 @@ pub enum Attribute {
     DisplayName,
     Dn,
     Domain,
+    DomainDevelopmentTaint,
     DomainDisplayName,
     DomainLdapBasedn,
     DomainName,
@@ -269,6 +270,7 @@ impl TryFrom<String> for Attribute {
             ATTR_DN => Attribute::Dn,
             ATTR_DOMAIN => Attribute::Domain,
             ATTR_DOMAIN_DISPLAY_NAME => Attribute::DomainDisplayName,
+            ATTR_DOMAIN_DEVELOPMENT_TAINT => Attribute::DomainDevelopmentTaint,
             ATTR_DOMAIN_LDAP_BASEDN => Attribute::DomainLdapBasedn,
             ATTR_DOMAIN_NAME => Attribute::DomainName,
             ATTR_DOMAIN_SSID => Attribute::DomainSsid,
@@ -440,6 +442,7 @@ impl From<Attribute> for &'static str {
             Attribute::DisplayName => ATTR_DISPLAYNAME,
             Attribute::Dn => ATTR_DN,
             Attribute::Domain => ATTR_DOMAIN,
+            Attribute::DomainDevelopmentTaint => ATTR_DOMAIN_DEVELOPMENT_TAINT,
             Attribute::DomainDisplayName => ATTR_DOMAIN_DISPLAY_NAME,
             Attribute::DomainLdapBasedn => ATTR_DOMAIN_LDAP_BASEDN,
             Attribute::DomainName => ATTR_DOMAIN_NAME,

server/lib/src/constants/mod.rs

@@ -77,7 +77,8 @@ pub const DOMAIN_LEVEL_7: DomainVersion = 7;
 /// Deprcated as of 1.6.0
 pub const DOMAIN_LEVEL_8: DomainVersion = 8;
 
-// The minimum level that we can re-migrate from
+// The minimum level that we can re-migrate from.
+// This should be DOMAIN_TGT_LEVEL minus 2
 pub const DOMAIN_MIN_REMIGRATION_LEVEL: DomainVersion = DOMAIN_LEVEL_5;
 // The minimum supported domain functional level
 pub const DOMAIN_MIN_LEVEL: DomainVersion = DOMAIN_TGT_LEVEL;
@@ -88,7 +89,7 @@ pub const DOMAIN_TGT_LEVEL: DomainVersion = DOMAIN_LEVEL_7;
 pub const DOMAIN_TGT_PATCH_LEVEL: u32 = PATCH_LEVEL_1;
 // The maximum supported domain functional level
 pub const DOMAIN_MAX_LEVEL: DomainVersion = DOMAIN_LEVEL_7;
-// The maximum supported domain functional level
+// The next maximum supported domain functional level
 pub const DOMAIN_NEXT_LEVEL: DomainVersion = DOMAIN_LEVEL_8;
 
 // On test builds define to 60 seconds

server/lib/src/constants/schema.rs

@@ -698,6 +698,14 @@ pub static ref SCHEMA_ATTR_PATCH_LEVEL_DL7: SchemaAttribute = SchemaAttribute {
     ..Default::default()
 };
 
+pub static ref SCHEMA_ATTR_DOMAIN_DEVELOPMENT_TAINT_DL7: SchemaAttribute = SchemaAttribute {
+    uuid: UUID_SCHEMA_ATTR_DOMAIN_DEVELOPMENT_TAINT,
+    name: Attribute::DomainDevelopmentTaint.into(),
+    description: "A flag to show that the domain has been run on a development build, and will need additional work to upgrade/migrate.".to_string(),
+    syntax: SyntaxType::Boolean,
+    ..Default::default()
+};
+
 // === classes ===
 
 pub static ref SCHEMA_CLASS_PERSON: SchemaClass = SchemaClass {
@@ -1083,6 +1091,7 @@ pub static ref SCHEMA_CLASS_DOMAIN_INFO_DL7: SchemaClass = SchemaClass {
         Attribute::DomainLdapBasedn.into(),
         Attribute::LdapAllowUnixPwBind.into(),
         Attribute::PatchLevel.into(),
+        Attribute::DomainDevelopmentTaint.into(),
     ],
     systemmust: vec![
         Attribute::Name.into(),

server/lib/src/constants/uuids.rs

@@ -302,6 +302,8 @@ pub const UUID_SCHEMA_ATTR_KEY_ACTION_IMPORT_JWS_ES256: Uuid =
 pub const UUID_SCHEMA_CLASS_KEY_OBJECT_JWE_A128GCM: Uuid =
     uuid!("00000000-0000-0000-0000-ffff00000174");
 pub const UUID_SCHEMA_ATTR_PATCH_LEVEL: Uuid = uuid!("00000000-0000-0000-0000-ffff00000175");
+pub const UUID_SCHEMA_ATTR_DOMAIN_DEVELOPMENT_TAINT: Uuid =
+    uuid!("00000000-0000-0000-0000-ffff00000176");
 
 // System and domain infos
 // I'd like to strongly criticise william of the past for making poor choices about these allocations.

server/lib/src/repl/consumer.rs

@@ -333,7 +333,11 @@ impl<'a> QueryServerWriteTransaction<'a> {
             return Err(OperationError::ReplDomainLevelUnsatisfiable);
         };
 
-        let domain_patch_level = self.get_domain_patch_level();
+        let domain_patch_level = if self.get_domain_development_taint() {
+            u32::MAX
+        } else {
+            self.get_domain_patch_level()
+        };
 
         if ctx_domain_patch_level != domain_patch_level {
             error!("Unable to proceed with consumer incremental - incoming domain patch level is not equal to our patch level. {} != {}", ctx_domain_patch_level, domain_patch_level);
@@ -449,6 +453,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
         match ctx {
             ReplRefreshContext::V1 {
                 domain_version,
+                domain_devel,
                 domain_uuid,
                 ranges,
                 schema_entries,
@@ -456,6 +461,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
                 entries,
             } => self.consumer_apply_refresh_v1(
                 *domain_version,
+                *domain_devel,
                 *domain_uuid,
                 ranges,
                 schema_entries,
@@ -525,6 +531,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
     fn consumer_apply_refresh_v1(
         &mut self,
         ctx_domain_version: DomainVersion,
+        ctx_domain_devel: bool,
         ctx_domain_uuid: Uuid,
         ctx_ranges: &BTreeMap<Uuid, ReplAnchoredCidRange>,
         ctx_schema_entries: &[ReplEntryV1],
@@ -533,6 +540,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
     ) -> Result<(), OperationError> {
         // Can we apply the domain version validly?
         // if domain_version >= min_support ...
+        let current_devel_flag = option_env!("KANIDM_PRE_RELEASE").is_some();
 
         if ctx_domain_version < DOMAIN_MIN_LEVEL {
             error!("Unable to proceed with consumer refresh - incoming domain level is lower than our minimum supported level. {} < {}", ctx_domain_version, DOMAIN_MIN_LEVEL);
@@ -540,6 +548,12 @@ impl<'a> QueryServerWriteTransaction<'a> {
         } else if ctx_domain_version > DOMAIN_MAX_LEVEL {
             error!("Unable to proceed with consumer refresh - incoming domain level is greater than our maximum supported level. {} > {}", ctx_domain_version, DOMAIN_MAX_LEVEL);
             return Err(OperationError::ReplDomainLevelUnsatisfiable);
+        } else if ctx_domain_devel && !current_devel_flag {
+            error!("Unable to proceed with consumer refresh - incoming domain is from a development version while this server is a stable release.");
+            return Err(OperationError::ReplDomainLevelUnsatisfiable);
+        } else if !ctx_domain_devel && current_devel_flag {
+            error!("Unable to proceed with consumer refresh - incoming domain is from a stable version while this server is a development release.");
+            return Err(OperationError::ReplDomainLevelUnsatisfiable);
         } else {
             debug!(
                 "Proceeding to refresh from domain at level {}",

server/lib/src/repl/proto.rs

@@ -733,6 +733,7 @@ impl ReplIncrementalEntryV1 {
 pub enum ReplRefreshContext {
     V1 {
         domain_version: DomainVersion,
+        domain_devel: bool,
         domain_uuid: Uuid,
         // We need to send the current state of the ranges to populate into
         // the ranges so that lookups and ranges work properly.

server/lib/src/repl/supplier.rs

@@ -225,7 +225,11 @@ impl<'a> QueryServerReadTransaction<'a> {
 
         let schema = self.get_schema();
         let domain_version = self.d_info.d_vers;
-        let domain_patch_level = self.d_info.d_patch_level;
+        let domain_patch_level = if self.d_info.d_devel_taint {
+            u32::MAX
+        } else {
+            self.d_info.d_patch_level
+        };
         let domain_uuid = self.d_info.d_uuid;
 
         let schema_entries: Vec<_> = schema_entries
@@ -268,6 +272,7 @@ impl<'a> QueryServerReadTransaction<'a> {
         //
         // * the current domain version
         let domain_version = self.d_info.d_vers;
+        let domain_devel = self.d_info.d_devel_taint;
         let domain_uuid = self.d_info.d_uuid;
 
         let trim_cid = self.trim_cid().clone();
@@ -355,6 +360,7 @@ impl<'a> QueryServerReadTransaction<'a> {
 
         Ok(ReplRefreshContext::V1 {
             domain_version,
+            domain_devel,
             domain_uuid,
             ranges,
             schema_entries,

server/lib/src/server/migrations.rs

@@ -66,6 +66,11 @@ impl QueryServer {
             // Domain info was present, so we need to reflect that in our server
             // domain structures. If we don't do this, the in memory domain level
             // is stuck at 0 which can confuse init domain info below.
+            //
+            // This also is where the former domain taint flag will be loaded to
+            // d_info so that if the *previous* execution of the database was
+            // a devel version, we'll still trigger the forced remigration in
+            // in the case that we are moving from dev -> stable.
             write_txn.force_domain_reload();
 
             write_txn.reload()?;
@@ -111,7 +116,13 @@ impl QueryServer {
         // The reloads will have populated this structure now.
         let domain_info_version = write_txn.get_domain_version();
         let domain_patch_level = write_txn.get_domain_patch_level();
-        debug!(?db_domain_version, "After setting internal domain info");
+        let domain_development_taint = write_txn.get_domain_development_taint();
+        debug!(
+            ?db_domain_version,
+            ?domain_patch_level,
+            ?domain_development_taint,
+            "After setting internal domain info"
+        );
 
         if domain_info_version < domain_target_level {
             write_txn
@@ -129,7 +140,7 @@ impl QueryServer {
             // Reload if anything in migrations requires it - this triggers the domain migrations
             // which in turn can trigger schema reloads etc.
             write_txn.reload()?;
-        } else {
+        } else if domain_development_taint {
             // This forces pre-release versions to re-migrate each start up. This solves
             // the domain-version-sprawl issue so that during a development cycle we can
             // do a single domain version bump, and continue to extend the migrations
@@ -140,11 +151,9 @@ impl QueryServer {
             // we are NOT in a test environment
             // AND
             // We did not already need a version migration as above
-            if option_env!("KANIDM_PRE_RELEASE").is_some() && !cfg!(test) {
             write_txn.domain_remigrate(DOMAIN_PREVIOUS_TGT_LEVEL)?;
             write_txn.reload()?;
         }
-        }
 
         if domain_target_level >= DOMAIN_LEVEL_7 && domain_patch_level < DOMAIN_TGT_PATCH_LEVEL {
             write_txn
@@ -159,10 +168,26 @@ impl QueryServer {
                     warn!("Domain level has been raised to {}", domain_target_level);
                 })?;
 
-            // Run the patch migrations
+            // Run the patch migrations if any.
             write_txn.reload()?;
         };
 
+        // Now set the db/domain devel taint flag to match our current release status
+        // if it changes. This is what breaks the cycle of db taint from dev -> stable
+        let current_devel_flag = option_env!("KANIDM_PRE_RELEASE").is_some();
+        if current_devel_flag {
+            warn!("Domain Development Taint mode is enabled");
+        }
+        if domain_development_taint != current_devel_flag {
+            write_txn.internal_modify_uuid(
+                UUID_DOMAIN_INFO,
+                &ModifyList::new_purge_and_set(
+                    Attribute::DomainDevelopmentTaint,
+                    Value::Bool(current_devel_flag),
+                ),
+            )?;
+        }
+
         // We are ready to run
         write_txn.set_phase(ServerPhase::Running);
 
@@ -653,6 +678,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
         // Now update schema
         let idm_schema_classes = [
             SCHEMA_ATTR_PATCH_LEVEL_DL7.clone().into(),
+            SCHEMA_ATTR_DOMAIN_DEVELOPMENT_TAINT_DL7.clone().into(),
             SCHEMA_CLASS_DOMAIN_INFO_DL7.clone().into(),
             SCHEMA_CLASS_SERVICE_ACCOUNT_DL7.clone().into(),
             SCHEMA_CLASS_SYNC_ACCOUNT_DL7.clone().into(),

server/lib/src/server/mod.rs

@@ -73,6 +73,7 @@ pub struct DomainInfo {
     pub(crate) d_display: String,
     pub(crate) d_vers: DomainVersion,
     pub(crate) d_patch_level: u32,
+    pub(crate) d_devel_taint: bool,
     pub(crate) d_ldap_allow_unix_pw_bind: bool,
 }
 
@@ -195,6 +196,8 @@ pub trait QueryServerTransaction<'a> {
 
     fn get_domain_patch_level(&self) -> u32;
 
+    fn get_domain_development_taint(&self) -> bool;
+
     fn get_domain_uuid(&self) -> Uuid;
 
     fn get_domain_name(&self) -> &str;
@@ -1074,6 +1077,10 @@ impl<'a> QueryServerTransaction<'a> for QueryServerReadTransaction<'a> {
         self.d_info.d_patch_level
     }
 
+    fn get_domain_development_taint(&self) -> bool {
+        self.d_info.d_devel_taint
+    }
+
     fn get_domain_uuid(&self) -> Uuid {
         self.d_info.d_uuid
     }
@@ -1224,6 +1231,10 @@ impl<'a> QueryServerTransaction<'a> for QueryServerWriteTransaction<'a> {
         self.d_info.d_patch_level
     }
 
+    fn get_domain_development_taint(&self) -> bool {
+        self.d_info.d_devel_taint
+    }
+
     fn get_domain_uuid(&self) -> Uuid {
         self.d_info.d_uuid
     }
@@ -1272,6 +1283,8 @@ impl QueryServer {
             // we set the domain_display_name to the configuration file's domain_name
             // here because the database is not started, so we cannot pull it from there.
             d_display: domain_name,
+            // Automatically derive our current taint mode based on the PRERELEASE setting.
+            d_devel_taint: option_env!("KANIDM_PRE_RELEASE").is_some(),
             d_ldap_allow_unix_pw_bind: false,
         }));
 
@@ -1814,6 +1827,15 @@ impl<'a> QueryServerWriteTransaction<'a> {
             .get_ava_single_uint32(Attribute::PatchLevel)
             .unwrap_or(0);
 
+        // If we have moved from stable to dev, this triggers the taint. If we
+        // are moving from dev to stable, the db will be true triggering the
+        // taint flag. If we are stable to stable this will be false.
+        let current_devel_flag = option_env!("KANIDM_PRE_RELEASE").is_some();
+        let domain_info_devel_taint = current_devel_flag
+            || domain_info
+                .get_ava_single_bool(Attribute::DomainDevelopmentTaint)
+                .unwrap_or_default();
+
         // We have to set the domain version here so that features which check for it
         // will now see it's been increased. This also prevents recursion during reloads
         // inside of a domain migration.
@@ -1822,6 +1844,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
         let previous_patch_level = mut_d_info.d_patch_level;
         mut_d_info.d_vers = domain_info_version;
         mut_d_info.d_patch_level = domain_info_patch_level;
+        mut_d_info.d_devel_taint = domain_info_devel_taint;
 
         // We must both be at the correct domain version *and* the correct patch level. If we are
         // not, then we only proceed to migrate *if* our server boot phase is correct.