diff --git a/changes/47981-gitops-category-collation b/changes/47981-gitops-category-collation new file mode 100644 index 0000000000..fd282beb95 --- /dev/null +++ b/changes/47981-gitops-category-collation @@ -0,0 +1 @@ +- Fixed GitOps runs failing with a `software_categories` duplicate-entry error when a software category's name differed only by characters MySQL's collation treats as equal (such as the Unicode variation selector in default categories like "🖥️ Productivity"). diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index 7022bc462c..c38cebc6b4 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -3879,9 +3879,15 @@ func getInstallScript(extension string, packageIDs []string, currentScript strin // batchAddSelfServiceCategories only adds categories, because it is used across both the installer and vpp // endpoints and we cannot know what categories to delete before those are both done. func (svc *Service) batchAddSelfServiceCategories(ctx context.Context, teamID *uint, categoryNames []string, dryRun bool) ([]string, error) { + // Compare names with fleet.SoftwareCategoryNamesEqual rather than a plain + // case-insensitive comparison: the software_categories unique index uses the + // utf8mb4_unicode_ci collation, which ignores variation selectors, so two + // names Go considers distinct (e.g. "🖥️ Productivity" with vs. without U+FE0F) + // are the same row to MySQL. Deduping/matching on the DB's terms here avoids + // attempting an insert that would fail with a 1062 duplicate-entry error. var allCategories []string for _, name := range fleet.TranslateLegacySoftwareCategoryNames(categoryNames) { - if slices.ContainsFunc(allCategories, func(c string) bool { return strings.EqualFold(c, name) }) { + if slices.ContainsFunc(allCategories, func(c string) bool { return fleet.SoftwareCategoryNamesEqual(c, name) }) { continue } allCategories = append(allCategories, name) @@ -3898,7 +3904,7 @@ func (svc *Service) batchAddSelfServiceCategories(ctx context.Context, teamID *u var categoriesToInsert []string for _, name := range allCategories { - if !slices.ContainsFunc(existingCategories, func(c fleet.SoftwareCategory) bool { return strings.EqualFold(c.Name, name) }) { + if !slices.ContainsFunc(existingCategories, func(c fleet.SoftwareCategory) bool { return fleet.SoftwareCategoryNamesEqual(c.Name, name) }) { categoriesToInsert = append(categoriesToInsert, name) } } diff --git a/server/datastore/mysql/software.go b/server/datastore/mysql/software.go index 477142139f..c136dfbf85 100644 --- a/server/datastore/mysql/software.go +++ b/server/datastore/mysql/software.go @@ -6983,7 +6983,16 @@ func batchNewSoftwareCategoriesDB(ctx context.Context, q sqlx.ExtContext, teamID return nil } placeholders := strings.TrimSuffix(strings.Repeat("(?, ?), ", len(names)), ", ") - stmt := `INSERT INTO software_categories (name, team_id) VALUES ` + placeholders + // ON DUPLICATE KEY UPDATE makes this insert idempotent against the + // (team_id, name) unique index. Callers already filter out names that exist, + // but that check runs in Go and can't perfectly mirror the utf8mb4_unicode_ci + // collation (which ignores variation selectors and gives many emoji equal + // weight), so a name that's distinct to Go may collide in the index. The + // upsert lets the existing row win instead of failing the batch with a 1062 + // duplicate-entry error; it also tolerates concurrent inserts of the same + // default categories. + stmt := `INSERT INTO software_categories (name, team_id) VALUES ` + placeholders + + ` ON DUPLICATE KEY UPDATE name = name` args := make([]any, 0, len(names)*2) for _, name := range names { args = append(args, name, teamID) diff --git a/server/datastore/mysql/software_test.go b/server/datastore/mysql/software_test.go index 41ba65d6e4..d5e04cde9f 100644 --- a/server/datastore/mysql/software_test.go +++ b/server/datastore/mysql/software_test.go @@ -135,6 +135,7 @@ func TestSoftware(t *testing.T) { {"SoftwareLiteByID", testSoftwareLiteByID}, {"GetDisplayNamesByTeamAndTitleIdsBatching", testGetDisplayNamesByTeamAndTitleIdsBatching}, {"GetSoftwareCategoryNameToIDMap", testGetSoftwareCategoryNameToIDMap}, + {"BatchNewSoftwareCategoriesIdempotent", testBatchNewSoftwareCategoriesIdempotent}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -13025,6 +13026,51 @@ func testGetSoftwareCategoryNameToIDMap(t *testing.T, ds *Datastore) { assert.Empty(t, got) } +func testBatchNewSoftwareCategoriesIdempotent(t *testing.T, ds *Datastore) { + ctx := context.Background() + + team, err := ds.NewTeam(ctx, &fleet.Team{Name: t.Name()}) + require.NoError(t, err) + + // Teams auto-seed "🖥️ Productivity" with the U+FE0F variation selector. The + // utf8mb4_unicode_ci collation on the (team_id, name) unique index ignores that + // selector, so re-inserting the same category WITHOUT the selector — a common + // form in GitOps files — collides in the index even though the bytes differ. + // The batch insert must be idempotent (ON DUPLICATE KEY UPDATE) so this does + // not fail with a 1062 duplicate-entry error and does not create a second row. + const ( + productivityCanonical = "\U0001F5A5\uFE0F Productivity" // seeded form, with VS-16 + productivityNoVS = "\U0001F5A5 Productivity" // colliding form, no VS-16 + customName = "\U0001F195 Custom" // a genuinely new category + ) + + countProductivity := func() int { + var n int + require.NoError(t, sqlx.GetContext(ctx, ds.reader(ctx), &n, + `SELECT COUNT(*) FROM software_categories WHERE team_id = ? AND name = ?`, team.ID, productivityCanonical)) + return n + } + require.Equal(t, 1, countProductivity(), "team should be seeded with exactly one Productivity category") + + // Re-inserting the colliding form alongside a brand-new category must succeed. + require.NoError(t, ds.BatchNewSoftwareCategories(ctx, team.ID, []string{productivityNoVS, customName})) + + // The collision was absorbed (no second Productivity row) and the new category + // was created. + require.Equal(t, 1, countProductivity(), "colliding insert must not create a duplicate Productivity row") + cats, err := ds.ListSoftwareCategories(ctx, team.ID) + require.NoError(t, err) + require.True(t, slices.ContainsFunc(cats, func(c fleet.SoftwareCategory) bool { return c.Name == customName }), + "genuinely new category should have been inserted") + + // Repeating the same batch remains a no-op: no error and no new rows. + before := len(cats) + require.NoError(t, ds.BatchNewSoftwareCategories(ctx, team.ID, []string{productivityNoVS, customName})) + cats, err = ds.ListSoftwareCategories(ctx, team.ID) + require.NoError(t, err) + require.Len(t, cats, before) +} + // The next three tests guard the per-host software detail queries against the // OR-dominance drop-out: a host with two queued activities for the same // installer/app (one lower priority, the other later created_at) used to fail diff --git a/server/fleet/software.go b/server/fleet/software.go index 87ad8271c2..77138ce541 100644 --- a/server/fleet/software.go +++ b/server/fleet/software.go @@ -930,11 +930,38 @@ func TranslateLegacySoftwareCategoryNames(names []string) []string { return out } +// normalizeSoftwareCategoryName strips Unicode variation selectors (U+FE00–U+FE0F) +// from a category name. These code points carry zero weight (they are ignorable) +// under the utf8mb4_unicode_ci collation that backs the software_categories +// (team_id, name) unique index, so names differing only by a variation selector — +// e.g. "🖥️ Productivity" (U+1F5A5 U+FE0F) vs "🖥 Productivity" (U+1F5A5) — are the +// SAME row to MySQL even though Go's byte/rune comparisons treat them as distinct. +// Normalizing before comparing in Go keeps our notion of category identity aligned +// with the database's, so we don't try to insert a name the DB already considers a +// duplicate (which would fail with a 1062 error) and we correctly resolve such a +// name back to its existing category. +func normalizeSoftwareCategoryName(name string) string { + return strings.Map(func(r rune) rune { + if r >= 0xFE00 && r <= 0xFE0F { // variation selectors VS1-VS16 (ignorable in utf8mb4_unicode_ci) + return -1 + } + return r + }, name) +} + +// SoftwareCategoryNamesEqual reports whether two category names refer to the same +// category as far as the software_categories unique index is concerned: +// case-insensitive and ignoring variation selectors, matching the column's +// utf8mb4_unicode_ci collation. +func SoftwareCategoryNamesEqual(a, b string) bool { + return strings.EqualFold(normalizeSoftwareCategoryName(a), normalizeSoftwareCategoryName(b)) +} + func SoftwareCategoryReferenceMatches(reference string, name string) bool { - if strings.EqualFold(reference, name) { + if SoftwareCategoryNamesEqual(reference, name) { return true } - if t, ok := LegacySoftwareCategoryNames[reference]; ok && strings.EqualFold(t, name) { + if t, ok := LegacySoftwareCategoryNames[reference]; ok && SoftwareCategoryNamesEqual(t, name) { return true } return false diff --git a/server/fleet/software_test.go b/server/fleet/software_test.go index 663b814976..3afd84c2d2 100644 --- a/server/fleet/software_test.go +++ b/server/fleet/software_test.go @@ -3,6 +3,7 @@ package fleet import ( "encoding/json" "fmt" + "strings" "testing" "time" @@ -1023,3 +1024,64 @@ func TestAutoUpdateScheduleValidation(t *testing.T) { }) } } + +func TestSoftwareCategoryNamesEqual(t *testing.T) { + // "🖥️ Productivity" is the canonical default (U+1F5A5 + U+FE0F variation + // selector). MySQL's utf8mb4_unicode_ci collation ignores the variation + // selector, so the form without it must compare equal even though Go's + // strings.EqualFold treats them as distinct byte sequences. + const ( + productivityVS = "\U0001F5A5\uFE0F Productivity" // with VS-16 + productivityNoVS = "\U0001F5A5 Productivity" // without VS-16 + browsers = "\U0001F30E Browsers" + ) + + // Sanity check that the two forms really are byte-distinct to plain Go + // comparison, otherwise this test wouldn't be exercising anything. + require.NotEqual(t, productivityVS, productivityNoVS) + require.False(t, strings.EqualFold(productivityVS, productivityNoVS)) + + cases := []struct { + name string + a string + b string + want bool + }{ + {"identical", productivityVS, productivityVS, true}, + {"variation selector ignored", productivityVS, productivityNoVS, true}, + {"variation selector ignored, reversed", productivityNoVS, productivityVS, true}, + {"case insensitive", "Security", "security", true}, + {"distinct categories", productivityVS, browsers, false}, + {"empty equal", "", "", true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + assert.Equal(t, c.want, SoftwareCategoryNamesEqual(c.a, c.b)) + }) + } +} + +func TestSoftwareCategoryReferenceMatches(t *testing.T) { + const ( + productivityVS = "\U0001F5A5\uFE0F Productivity" + productivityNoVS = "\U0001F5A5 Productivity" + ) + + cases := []struct { + name string + reference string + stored string + want bool + }{ + {"exact emoji name", productivityVS, productivityVS, true}, + {"emoji name ignoring variation selector", productivityNoVS, productivityVS, true}, + {"legacy name maps to emoji default", "Productivity", productivityVS, true}, + {"legacy name maps even without stored variation selector", "Productivity", productivityNoVS, true}, + {"unrelated", "Productivity", "\U0001F30E Browsers", false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + assert.Equal(t, c.want, SoftwareCategoryReferenceMatches(c.reference, c.stored)) + }) + } +}