Allow applying built-in label specs without modifications. (#18804)

#18477 

Built-in labels can now be applied via `fleetctl apply` as long as no
changes are made to them. This allows the following workflow:
  1. `fleetctl get labels --yaml > labels.yml`
  2. (Optional) Edit/add non-built in labels in labels.yml
  3. fleetctl apply -f labels.yml

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

<!-- Note that API documentation changes are now addressed by the
product design team. -->

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality

---------

Co-authored-by: Tim Lee <timlee@fleetdm.com>

changes/18477-apply-builtin-labels

@@ -0,0 +1,4 @@
+Built-in labels can now be applied via `fleetctl apply` as long as no changes are made to them. This allows the following workflow:
+  1. `fleetctl get labels --yaml > labels.yml`
+  2. (Optional) Edit/add non-built in labels in labels.yml
+  3. `fleetctl apply -f labels.yml`

cmd/fleetctl/apply_test.go

@@ -1000,6 +1000,18 @@ spec:
   hosts:
   platforms:
     - darwin
+`
+	builtinLabelSpec = `---
+apiVersion: v1
+kind: label
+spec:
+  description: All Ubuntu hosts
+  hosts: null
+  id: 8
+  label_membership_type: dynamic
+  label_type: builtin
+  name: Ubuntu Linux
+  query: select 1 from os_version where platform = 'ubuntu';
 `
 	packsSpec = `---
 apiVersion: v1
@@ -1614,6 +1626,36 @@ func TestApplyLabels(t *testing.T) {
 	_, err := runAppNoChecks([]string{"apply", "-f", name})
 	require.Error(t, err)
 	require.ErrorContains(t, err, "declared as manual but contains no `hosts key`")
+
+	// Apply built-in label (no changes)
+	// The label values below should match the spec.
+	ubuntuLabel := &fleet.Label{
+		ID:                  8,
+		Name:                fleet.BuiltinLabelNameUbuntuLinux,
+		Query:               "select 1 from os_version where platform = 'ubuntu';",
+		Description:         "All Ubuntu hosts",
+		LabelType:           fleet.LabelTypeBuiltIn,
+		LabelMembershipType: fleet.LabelMembershipTypeDynamic,
+	}
+	ds.LabelsByNameFunc = func(ctx context.Context, names []string) (map[string]*fleet.Label, error) {
+		assert.ElementsMatch(t, []string{fleet.BuiltinLabelNameUbuntuLinux}, names)
+		return map[string]*fleet.Label{
+			fleet.BuiltinLabelNameUbuntuLinux: ubuntuLabel,
+		}, nil
+	}
+
+	name = writeTmpYml(t, builtinLabelSpec)
+	assert.Equal(t, "[+] applied 1 labels\n", runAppForTest(t, []string{"apply", "-f", name}))
+	assert.False(t, ds.ApplyLabelSpecsFuncInvoked)
+	assert.True(t, ds.LabelsByNameFuncInvoked)
+
+	// Apply built-in label (with changes)
+	ubuntuLabel.Description = "CHANGED"
+	name = writeTmpYml(t, builtinLabelSpec)
+	_, err = runAppNoChecks([]string{"apply", "-f", name})
+	require.Error(t, err)
+	require.ErrorContains(t, err, "cannot modify or add built-in label")
+
 }
 
 func TestApplyPacks(t *testing.T) {

server/datastore/mysql/labels.go

@@ -910,6 +910,34 @@ func (ds *Datastore) LabelIDsByName(ctx context.Context, names []string) (map[st
 	return result, nil
 }
 
+func (ds *Datastore) LabelsByName(ctx context.Context, names []string) (map[string]*fleet.Label, error) {
+	if len(names) == 0 {
+		return map[string]*fleet.Label{}, nil
+	}
+
+	sqlStatement := `
+		SELECT * FROM labels
+		WHERE name IN (?)
+	`
+
+	sqlStatement, args, err := sqlx.In(sqlStatement, names)
+	if err != nil {
+		return nil, ctxerr.Wrap(ctx, err, "building query to get label ids by name")
+	}
+
+	var labels []*fleet.Label
+	if err := sqlx.SelectContext(ctx, ds.reader(ctx), &labels, sqlStatement, args...); err != nil {
+		return nil, ctxerr.Wrap(ctx, err, "get label ids by name")
+	}
+
+	result := make(map[string]*fleet.Label, len(labels))
+	for _, label := range labels {
+		result[label.Name] = label
+	}
+
+	return result, nil
+}
+
 // AsyncBatchInsertLabelMembership inserts into the label_membership table the
 // batch of label_id + host_id tuples represented by the [2]uint array.
 func (ds *Datastore) AsyncBatchInsertLabelMembership(ctx context.Context, batch [][2]uint) error {

server/datastore/mysql/labels_test.go

@@ -59,6 +59,7 @@ func TestLabels(t *testing.T) {
 		{"GetSpec", testLabelsGetSpec},
 		{"ApplySpecsRoundtrip", testLabelsApplySpecsRoundtrip},
 		{"IDsByName", testLabelsIDsByName},
+		{"ByName", testLabelsByName},
 		{"Save", testLabelsSave},
 		{"QueriesForCentOSHost", testLabelsQueriesForCentOSHost},
 		{"RecordNonExistentQueryLabelExecution", testLabelsRecordNonexistentQueryLabelExecution},
@@ -755,6 +756,30 @@ func testLabelsIDsByName(t *testing.T, ds *Datastore) {
 	assert.Equal(t, map[string]uint{"foo": 1, "bar": 2, "bing": 3}, labels)
 }
 
+func testLabelsByName(t *testing.T, ds *Datastore) {
+	setupLabelSpecsTest(t, ds)
+
+	names := []string{"foo", "bar", "bing"}
+	labels, err := ds.LabelsByName(context.Background(), names)
+	require.NoError(t, err)
+	require.Len(t, labels, 3)
+	for _, name := range names {
+		assert.Contains(t, labels, name)
+		assert.Equal(t, name, labels[name].Name)
+		switch name {
+		case "foo":
+			assert.Equal(t, uint(1), labels[name].ID)
+			assert.Equal(t, "foo description", labels[name].Description)
+		case "bar":
+			assert.Equal(t, uint(2), labels[name].ID)
+			assert.Empty(t, labels[name].Description)
+		case "bing":
+			assert.Equal(t, uint(3), labels[name].ID)
+			assert.Empty(t, labels[name].Description)
+		}
+	}
+}
+
 func testLabelsSave(t *testing.T, db *Datastore) {
 	h1, err := db.NewHost(context.Background(), &fleet.Host{
 		DetailUpdatedAt: time.Now(),

server/fleet/datastore.go

@@ -211,6 +211,8 @@ type Datastore interface {
 
 	// LabelIDsByName retrieves the IDs associated with the given label names
 	LabelIDsByName(ctx context.Context, labels []string) (map[string]uint, error)
+	// LabelsByName retrieves the labels associated with the given label names
+	LabelsByName(ctx context.Context, names []string) (map[string]*Label, error)
 
 	// Methods used for async processing of host label query results.
 	AsyncBatchInsertLabelMembership(ctx context.Context, batch [][2]uint) error

server/mock/datastore_mock.go

@@ -157,6 +157,8 @@ type SearchLabelsFunc func(ctx context.Context, filter fleet.TeamFilter, query s
 
 type LabelIDsByNameFunc func(ctx context.Context, labels []string) (map[string]uint, error)
 
+type LabelsByNameFunc func(ctx context.Context, names []string) (map[string]*fleet.Label, error)
+
 type AsyncBatchInsertLabelMembershipFunc func(ctx context.Context, batch [][2]uint) error
 
 type AsyncBatchDeleteLabelMembershipFunc func(ctx context.Context, batch [][2]uint) error
@@ -1133,6 +1135,9 @@ type DataStore struct {
 	LabelIDsByNameFunc        LabelIDsByNameFunc
 	LabelIDsByNameFuncInvoked bool
 
+	LabelsByNameFunc        LabelsByNameFunc
+	LabelsByNameFuncInvoked bool
+
 	AsyncBatchInsertLabelMembershipFunc        AsyncBatchInsertLabelMembershipFunc
 	AsyncBatchInsertLabelMembershipFuncInvoked bool
 
@@ -2771,6 +2776,13 @@ func (s *DataStore) LabelIDsByName(ctx context.Context, labels []string) (map[st
 	return s.LabelIDsByNameFunc(ctx, labels)
 }
 
+func (s *DataStore) LabelsByName(ctx context.Context, names []string) (map[string]*fleet.Label, error) {
+	s.mu.Lock()
+	s.LabelsByNameFuncInvoked = true
+	s.mu.Unlock()
+	return s.LabelsByNameFunc(ctx, names)
+}
+
 func (s *DataStore) AsyncBatchInsertLabelMembership(ctx context.Context, batch [][2]uint) error {
 	s.mu.Lock()
 	s.AsyncBatchInsertLabelMembershipFuncInvoked = true

server/service/integration_core_test.go

@@ -4278,7 +4278,8 @@ func (s *integrationTestSuite) TestLabelSpecs() {
 				Hosts:               []string{"abc"},
 			},
 		},
-	}, http.StatusInternalServerError, &applyResp)
+	}, http.StatusUnprocessableEntity, &applyResp,
+	)
 
 	// apply an invalid label spec - manual membership without a host specified
 	s.DoJSON("POST", "/api/latest/fleet/spec/labels", applyLabelSpecsRequest{
@@ -4290,7 +4291,8 @@ func (s *integrationTestSuite) TestLabelSpecs() {
 				LabelMembershipType: fleet.LabelMembershipTypeManual,
 			},
 		},
-	}, http.StatusInternalServerError, &applyResp)
+	}, http.StatusUnprocessableEntity, &applyResp,
+	)
 
 	// apply an invalid label spec - builtin label type
 	s.DoJSON("POST", "/api/latest/fleet/spec/labels", applyLabelSpecsRequest{

server/service/labels.go

@@ -486,24 +486,63 @@ func (svc *Service) ApplyLabelSpecs(ctx context.Context, specs []*fleet.LabelSpe
 		return err
 	}
 
+	regularSpecs := make([]*fleet.LabelSpec, 0, len(specs))
+	var builtInSpecs []*fleet.LabelSpec
+	var builtInSpecNames []string
 	for _, spec := range specs {
 		if spec.LabelMembershipType == fleet.LabelMembershipTypeDynamic && len(spec.Hosts) > 0 {
-			return ctxerr.Errorf(ctx, "label %s is declared as dynamic but contains `hosts` key", spec.Name)
+			return fleet.NewUserMessageError(
+				ctxerr.Errorf(ctx, "label %s is declared as dynamic but contains `hosts` key", spec.Name), http.StatusUnprocessableEntity,
+			)
 		}
 		if spec.LabelMembershipType == fleet.LabelMembershipTypeManual && spec.Hosts == nil {
 			// Hosts list doesn't need to contain anything, but it should at least not be nil.
-			return ctxerr.Errorf(ctx, "label %s is declared as manual but contains no `hosts key`", spec.Name)
+			return fleet.NewUserMessageError(
+				ctxerr.Errorf(ctx, "label %s is declared as manual but contains no `hosts key`", spec.Name), http.StatusUnprocessableEntity,
+			)
 		}
 		if spec.LabelType == fleet.LabelTypeBuiltIn {
-			return fleet.NewUserMessageError(ctxerr.Errorf(ctx, "cannot modify built-in label '%s'", spec.Name), http.StatusUnprocessableEntity)
+			// We allow specs to contain built-in labels as long as they are not being modified.
+			// This allows the user to do the following workflow without manually removing built-in labels:
+			// 1. fleetctl get labels --yaml > labels.yml
+			// 2. (Optional) Edit labels.yml
+			// 3. fleetctl apply -f labels.yml
+			builtInSpecs = append(builtInSpecs, spec)
+			builtInSpecNames = append(builtInSpecNames, spec.Name)
+			continue
 		}
 		for name := range fleet.ReservedLabelNames() {
 			if spec.Name == name {
 				return fleet.NewUserMessageError(ctxerr.Errorf(ctx, "cannot modify built-in label '%s'", name), http.StatusUnprocessableEntity)
 			}
 		}
+		regularSpecs = append(regularSpecs, spec)
 	}
-	return svc.ds.ApplyLabelSpecs(ctx, specs)
+
+	// If built-in labels have been provided, ensure that they are not attempted to be modified
+	if len(builtInSpecs) > 0 {
+		labelMap, err := svc.ds.LabelsByName(ctx, builtInSpecNames)
+		if err != nil {
+			return err
+		}
+		for _, spec := range builtInSpecs {
+			label, ok := labelMap[spec.Name]
+			if !ok ||
+				label.Description != spec.Description ||
+				label.Query != spec.Query ||
+				label.Platform != spec.Platform ||
+				label.LabelType != fleet.LabelTypeBuiltIn ||
+				label.LabelMembershipType != spec.LabelMembershipType {
+				return fleet.NewUserMessageError(
+					ctxerr.Errorf(ctx, "cannot modify or add built-in label '%s'", spec.Name), http.StatusUnprocessableEntity,
+				)
+			}
+		}
+	}
+	if len(regularSpecs) == 0 {
+		return nil
+	}
+	return svc.ds.ApplyLabelSpecs(ctx, regularSpecs)
 }
 
 ////////////////////////////////////////////////////////////////////////////////

server/service/labels_test.go

@@ -172,3 +172,84 @@ func testLabelsListLabels(t *testing.T, ds *mysql.Datastore) {
 	require.NoError(t, err)
 	require.Len(t, labelsSummary, 8)
 }
+
+func TestApplyLabelSpecsWithBuiltInLabels(t *testing.T) {
+	t.Parallel()
+	ds := new(mock.Store)
+	user := &fleet.User{
+		ID:         3,
+		Email:      "foo@bar.com",
+		GlobalRole: ptr.String(fleet.RoleAdmin),
+	}
+	svc, ctx := newTestService(t, ds, nil, nil)
+	ctx = viewer.NewContext(ctx, viewer.Viewer{User: user})
+	name := "foo"
+	description := "bar"
+	query := "select * from foo;"
+	platform := ""
+	labelType := fleet.LabelTypeBuiltIn
+	labelMembershipType := fleet.LabelMembershipTypeDynamic
+	spec := &fleet.LabelSpec{
+		Name:                name,
+		Description:         description,
+		Query:               query,
+		LabelType:           labelType,
+		LabelMembershipType: labelMembershipType,
+	}
+
+	ds.LabelsByNameFunc = func(ctx context.Context, names []string) (map[string]*fleet.Label, error) {
+		return map[string]*fleet.Label{
+			name: {
+				Name:                name,
+				Description:         description,
+				Query:               query,
+				Platform:            platform,
+				LabelType:           labelType,
+				LabelMembershipType: labelMembershipType,
+			},
+		}, nil
+	}
+
+	// all good
+	err := svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{spec})
+	require.NoError(t, err)
+
+	const errorMessage = "cannot modify or add built-in label"
+	// not ok -- built-in label name doesn't exist
+	name = "not-foo"
+	err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{spec})
+	assert.ErrorContains(t, err, errorMessage)
+	name = "foo"
+
+	// not ok -- description does not match
+	description = "not-bar"
+	err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{spec})
+	assert.ErrorContains(t, err, errorMessage)
+	description = "bar"
+
+	// not ok -- query does not match
+	query = "select * from not-foo;"
+	err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{spec})
+	assert.ErrorContains(t, err, errorMessage)
+	query = "select * from foo;"
+
+	// not ok -- label type does not match
+	labelType = fleet.LabelTypeRegular
+	err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{spec})
+	assert.ErrorContains(t, err, errorMessage)
+	labelType = fleet.LabelTypeBuiltIn
+
+	// not ok -- label membership type does not match
+	labelMembershipType = fleet.LabelMembershipTypeManual
+	err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{spec})
+	assert.ErrorContains(t, err, errorMessage)
+	labelMembershipType = fleet.LabelMembershipTypeDynamic
+
+	// not ok -- DB error
+	ds.LabelsByNameFunc = func(ctx context.Context, names []string) (map[string]*fleet.Label, error) {
+		return nil, assert.AnError
+	}
+	err = svc.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{spec})
+	assert.ErrorIs(t, err, assert.AnError)
+
+}