From b0866dcb1091296fb6f5551cc5ab5ee812fcc011 Mon Sep 17 00:00:00 2001 From: Jordan Montgomery Date: Mon, 6 Oct 2025 16:01:26 -0400 Subject: [PATCH] Fix failing migration by conditionally deleting old constraint (#33878) **Related issue:** Resolves #33876 Modifies the migration to check for the old constraint before deleting it in case it was created on mysql 5.7 No changes file since this is an unreleased bug # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually For unreleased bug fixes in a release candidate, one of: - [x] Confirmed that the fix is not expected to adversely impact load test results ## Database migrations - [x] Checked table schema to confirm autoupdate - [x] Checked schema for all modified table for columns that will auto-update timestamps during migration. - [x] Confirmed that updating the timestamps is acceptable, and will not cause unwanted side effects. - [x] Ensured the correct collation is explicitly set for character columns (`COLLATE utf8mb4_unicode_ci`). --- ...250922083056_AddTableMDMAndroidProfiles.go | 22 ++++++++++++++++++- ...2083056_AddTableMDMAndroidProfiles_test.go | 8 +++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/server/datastore/mysql/migrations/tables/20250922083056_AddTableMDMAndroidProfiles.go b/server/datastore/mysql/migrations/tables/20250922083056_AddTableMDMAndroidProfiles.go index bf1b328d18..b6bc9576c8 100644 --- a/server/datastore/mysql/migrations/tables/20250922083056_AddTableMDMAndroidProfiles.go +++ b/server/datastore/mysql/migrations/tables/20250922083056_AddTableMDMAndroidProfiles.go @@ -127,7 +127,6 @@ ALTER TABLE mdm_configuration_profile_labels ADD COLUMN android_profile_uuid VARCHAR(37) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NULL DEFAULT NULL, ADD FOREIGN KEY (android_profile_uuid) REFERENCES mdm_android_configuration_profiles(profile_uuid) ON DELETE CASCADE, ADD UNIQUE KEY idx_mdm_configuration_profile_labels_android_label_name (android_profile_uuid, label_name), - DROP CONSTRAINT ck_mdm_configuration_profile_labels_apple_or_windows, -- only one of apple, android or windows profile uuid must be set ADD CONSTRAINT ck_mdm_configuration_profile_labels_profile_uuid CHECK (IF(ISNULL(apple_profile_uuid), 0, 1) + IF(ISNULL(windows_profile_uuid), 0, 1) + IF(ISNULL(android_profile_uuid), 0, 1) = 1) @@ -136,6 +135,27 @@ ALTER TABLE mdm_configuration_profile_labels return fmt.Errorf("alter mdm_configuration_profile_labels table: %w", err) } + // our mysql version at the time this constraint was added did not support CHECK constraints so this may or may not exist for us + // to delete, so we create the new wider constraint above then, optionally, delete the older narrow one + checkIfOldConstraintExists := ` + SELECT COUNT(*) + FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS + WHERE CONSTRAINT_SCHEMA = DATABASE() AND CONSTRAINT_TYPE = 'CHECK' AND TABLE_NAME = 'mdm_configuration_profile_labels' + AND CONSTRAINT_NAME = 'ck_mdm_configuration_profile_labels_apple_or_windows'` + var constraintCount int + if err := tx.QueryRow(checkIfOldConstraintExists).Scan(&constraintCount); err != nil { + return fmt.Errorf("check for old CHECK constraint on mdm_configuration_profile_labels: %w", err) + } + if constraintCount > 0 { + dropOldConstraint := ` + ALTER TABLE mdm_configuration_profile_labels + DROP CONSTRAINT ck_mdm_configuration_profile_labels_apple_or_windows + ` + if _, err := tx.Exec(dropOldConstraint); err != nil { + return fmt.Errorf("drop old CHECK constraint on mdm_configuration_profile_labels: %w", err) + } + } + alterAndroidDevicesTable := ` ALTER TABLE android_devices ADD COLUMN applied_policy_id VARCHAR(100) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci DEFAULT NULL, diff --git a/server/datastore/mysql/migrations/tables/20250922083056_AddTableMDMAndroidProfiles_test.go b/server/datastore/mysql/migrations/tables/20250922083056_AddTableMDMAndroidProfiles_test.go index 7ed3471c83..5cee33ad67 100644 --- a/server/datastore/mysql/migrations/tables/20250922083056_AddTableMDMAndroidProfiles_test.go +++ b/server/datastore/mysql/migrations/tables/20250922083056_AddTableMDMAndroidProfiles_test.go @@ -104,3 +104,11 @@ func TestUp_20250922083056(t *testing.T) { // empty android host got updated, non-empty stayed the same, mac host not updated require.Equal(t, []string{"from-enterprise", "got-one", ""}, got) } + +func TestUp_20250922083056_checkconstraint_missing(t *testing.T) { + // Create a DB without the old apple-or-windows constraint and verify the migration does not error + db := applyUpToPrev(t) + _, err := db.Exec(`ALTER TABLE mdm_configuration_profile_labels DROP CONSTRAINT ck_mdm_configuration_profile_labels_apple_or_windows`) + require.NoError(t, err) + applyNext(t, db) +}