Select hosts for gitops labels using hardware_serial (#29639)

#28511

changes/28511-gitops-labels-hardware_serial

@@ -0,0 +1 @@
+- Fixed manual labels in gitops not selecting hosts by hardware serial or uuid

server/datastore/mysql/labels.go

@@ -90,13 +90,13 @@ DELETE FROM label_membership WHERE label_id = ?
 			}
 
 			// Split hostnames into batches to avoid parameter limit in MySQL.
-			for _, hostnames := range batchHostnames(s.Hosts) {
+			for _, hostIdentifiers := range batchHostnames(s.Hosts) {
 				// Use ignore because duplicate hostnames could appear in
 				// different batches and would result in duplicate key errors.
 				sql = `
-INSERT IGNORE INTO label_membership (label_id, host_id) (SELECT ?, id FROM hosts where hostname IN (?))
+INSERT IGNORE INTO label_membership (label_id, host_id) (SELECT DISTINCT ?, id FROM hosts where hostname IN (?) OR hardware_serial IN (?) OR uuid IN (?))
 `
-				sql, args, err := sqlx.In(sql, labelID, hostnames)
+				sql, args, err := sqlx.In(sql, labelID, hostIdentifiers, hostIdentifiers, hostIdentifiers)
 				if err != nil {
 					return ctxerr.Wrap(ctx, err, "build membership IN statement")
 				}
@@ -118,7 +118,12 @@ func batchHostnames(hostnames []string) [][]string {
 	// overflowing the MySQL max number of parameters (somewhere around 65,000
 	// but not well documented). Algorithm from
 	// https://github.com/golang/go/wiki/SliceTricks#batching-with-minimal-allocation
-	const batchSize = 50000 // Large, but well under the undocumented limit
+	//
+	// WARNING: This is used in ApplyLabelSpecsWithAuthor and the batch sizes have to be small
+	// enough to allow for three copies each hostname list in the query. The batch size is 20_000
+	// because 60_001 binding arguments is less than the maximum of 65,535.
+
+	const batchSize = 20_000 // Large, but well under the undocumented limit
 	batches := make([][]string, 0, (len(hostnames)+batchSize-1)/batchSize)
 
 	for batchSize < len(hostnames) {

server/datastore/mysql/labels_test.go

@@ -29,16 +29,17 @@ func TestBatchHostnamesSmall(t *testing.T) {
 
 func TestBatchHostnamesLarge(t *testing.T) {
 	large := []string{}
-	for i := 0; i < 230000; i++ {
+	for i := range 110_000 {
 		large = append(large, strconv.Itoa(i))
 	}
 	batched := batchHostnames(large)
-	require.Equal(t, 5, len(batched))
-	assert.Equal(t, large[:50000], batched[0])
-	assert.Equal(t, large[50000:100000], batched[1])
-	assert.Equal(t, large[100000:150000], batched[2])
-	assert.Equal(t, large[150000:200000], batched[3])
-	assert.Equal(t, large[200000:230000], batched[4])
+	require.Equal(t, 6, len(batched))
+	assert.Equal(t, large[:20_000], batched[0])
+	assert.Equal(t, large[20_000:40_000], batched[1])
+	assert.Equal(t, large[40_000:60_000], batched[2])
+	assert.Equal(t, large[60_000:80_000], batched[3])
+	assert.Equal(t, large[80_000:100_000], batched[4])
+	assert.Equal(t, large[100_000:110_000], batched[5])
 }
 
 func TestBatchHostIdsSmall(t *testing.T) {
@@ -94,6 +95,7 @@ func TestLabels(t *testing.T) {
 		{"HostMemberOfAllLabels", testHostMemberOfAllLabels},
 		{"ListHostsInLabelOSSettings", testLabelsListHostsInLabelOSSettings},
 		{"AddDeleteLabelsToFromHost", testAddDeleteLabelsToFromHost},
+		{"ApplyLabelSpecSerialUUID", testApplyLabelSpecsForSerialUUID},
 	}
 	// call TruncateTables first to remove migration-created labels
 	TruncateTables(t, ds)
@@ -1953,3 +1955,55 @@ func testUpdateLabelMembershipByHostIDs(t *testing.T, ds *Datastore) {
 	require.Equal(t, host2.Hostname, labelSpec.Hosts[1])
 	require.Equal(t, host3.Hostname, labelSpec.Hosts[2])
 }
+
+func testApplyLabelSpecsForSerialUUID(t *testing.T, ds *Datastore) {
+	ctx := context.Background()
+
+	host1, err := ds.NewHost(ctx, &fleet.Host{
+		OsqueryHostID:  ptr.String("1"),
+		NodeKey:        ptr.String("1"),
+		UUID:           "1",
+		Hostname:       "foo.local",
+		HardwareSerial: "hwd1",
+		Platform:       "darwin",
+	})
+	require.NoError(t, err)
+	host2, err := ds.NewHost(ctx, &fleet.Host{
+		OsqueryHostID:  ptr.String("2"),
+		NodeKey:        ptr.String("2"),
+		UUID:           "2",
+		Hostname:       "bar.local",
+		HardwareSerial: "hwd2",
+		Platform:       "windows",
+	})
+	require.NoError(t, err)
+	host3, err := ds.NewHost(ctx, &fleet.Host{
+		OsqueryHostID:  ptr.String("3"),
+		NodeKey:        ptr.String("3"),
+		UUID:           "uuid3",
+		Hostname:       "baz.local",
+		HardwareSerial: "hwd3",
+		Platform:       "windows",
+	})
+	require.NoError(t, err)
+
+	err = ds.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{
+		{
+			Name:                "label1",
+			LabelMembershipType: fleet.LabelMembershipTypeManual,
+			Hosts: []string{
+				"foo.local",
+				"hwd2",
+				"uuid3",
+			},
+		},
+	})
+	require.NoError(t, err)
+
+	hosts, err := ds.ListHostsInLabel(ctx, fleet.TeamFilter{User: test.UserAdmin}, 1, fleet.HostListOptions{})
+	require.NoError(t, err)
+	require.Len(t, hosts, 3)
+	require.Equal(t, host1.ID, hosts[0].ID)
+	require.Equal(t, host2.ID, hosts[1].ID)
+	require.Equal(t, host3.ID, hosts[2].ID)
+}