Align software category name comparison with DB (#47983)
Normalize category name comparisons to match MySQL's utf8mb4_unicode_ci collation (case-insensitive and ignoring Unicode variation selectors) to avoid duplicate-entry errors. Add normalizeSoftwareCategoryName and SoftwareCategoryNamesEqual (server/fleet/software.go) and use them where categories are deduped (ee/server/service/software_installers.go). Make batch insert idempotent by using ON DUPLICATE KEY UPDATE in the MySQL batch insert (server/datastore/mysql/software.go). Add tests for name-equality behavior and idempotent batch inserts (server/fleet/software_test.go, server/datastore/mysql/software_test.go). This prevents collisions between visually identical emoji forms (e.g. with/without U+FE0F) and tolerates concurrent/default category inserts. <!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #47981 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [ ] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [ ] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [ ] Timeouts are implemented and retries are limited to avoid infinite loops - [ ] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [ ] Added/updated automated tests - [ ] 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) - [ ] QA'd all new/changed functionality manually For unreleased bug fixes in a release candidate, one of: - [ ] Confirmed that the fix is not expected to adversely impact load test results - [ ] Alerted the release DRI if additional load testing is needed ## Database migrations - [ ] Checked schema for all modified table for columns that will auto-update timestamps during migration. - [ ] Confirmed that updating the timestamps is acceptable, and will not cause unwanted side effects. - [ ] Ensured the correct collation is explicitly set for character columns (`COLLATE utf8mb4_unicode_ci`). ## New Fleet configuration settings - [ ] Setting(s) is/are explicitly excluded from GitOps If you didn't check the box above, follow this checklist for GitOps-enabled settings: - [ ] Verified that the setting is exported via `fleetctl generate-gitops` - [ ] Verified the setting is documented in a separate PR to [the GitOps documentation](https://github.com/fleetdm/fleet/blob/main/docs/Configuration/yaml-files.md#L485) - [ ] Verified that the setting is cleared on the server if it is not supplied in a YAML file (or that it is documented as being optional) - [ ] Verified that any relevant UI is disabled when GitOps mode is enabled ## fleetd/orbit/Fleet Desktop - [ ] Verified compatibility with the latest released version of Fleet (see [Must rule](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/workflows/fleetd-development-and-release-strategy.md)) - [ ] If the change applies to only one platform, confirmed that `runtime.GOOS` is used as needed to isolate changes - [ ] Verified that fleetd runs on macOS, Linux and Windows - [ ] Verified auto-update works from the released version of component to the new version (see [tools/tuf/test](../tools/tuf/test/README.md)) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed GitOps runs failing due to software category duplicate-entry errors when names contain certain Unicode characters (e.g., emoji variation selectors). * **Improvements** * Enhanced software category deduplication to properly handle Unicode-equivalent names. * Made batch category insertion operations idempotent to prevent duplicate-key errors. * **Tests** * Added tests for software category idempotency and Unicode character handling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -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").
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user