diff --git a/changes/18477-apply-builtin-labels b/changes/18477-apply-builtin-labels new file mode 100644 index 0000000000..70a008e744 --- /dev/null +++ b/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` diff --git a/cmd/fleetctl/apply_test.go b/cmd/fleetctl/apply_test.go index c0b2796ce7..82d1d8e091 100644 --- a/cmd/fleetctl/apply_test.go +++ b/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) { diff --git a/server/datastore/mysql/labels.go b/server/datastore/mysql/labels.go index 272fd7fe12..2a66763750 100644 --- a/server/datastore/mysql/labels.go +++ b/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 { diff --git a/server/datastore/mysql/labels_test.go b/server/datastore/mysql/labels_test.go index cba41dbcc0..df5aee381f 100644 --- a/server/datastore/mysql/labels_test.go +++ b/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(), diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index 5d1bf64534..7374982d1e 100644 --- a/server/fleet/datastore.go +++ b/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 diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 834bb4499c..ef52b01295 100644 --- a/server/mock/datastore_mock.go +++ b/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 diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 4a739e189e..5e3b167e12 100644 --- a/server/service/integration_core_test.go +++ b/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{ diff --git a/server/service/labels.go b/server/service/labels.go index 6b5892b79d..44dc6490b4 100644 --- a/server/service/labels.go +++ b/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) } //////////////////////////////////////////////////////////////////////////////// diff --git a/server/service/labels_test.go b/server/service/labels_test.go index 362bfb2434..510758d223 100644 --- a/server/service/labels_test.go +++ b/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) + +}