diff --git a/changes/21163-config-profile-label b/changes/21163-config-profile-label new file mode 100644 index 0000000000..fe23787fba --- /dev/null +++ b/changes/21163-config-profile-label @@ -0,0 +1 @@ +- Fixed bug where configuration profile was still showing the old label name after the name was updated. diff --git a/server/datastore/mysql/labels.go b/server/datastore/mysql/labels.go index 2f654ebe33..d604b286a4 100644 --- a/server/datastore/mysql/labels.go +++ b/server/datastore/mysql/labels.go @@ -227,6 +227,14 @@ func (ds *Datastore) SaveLabel(ctx context.Context, label *fleet.Label, teamFilt if err != nil { return nil, nil, ctxerr.Wrap(ctx, err, "saving label") } + + // Update the label name in mdm_configuration_profile_labels + query = `UPDATE mdm_configuration_profile_labels SET label_name = ? WHERE label_id = ?` + _, err = ds.writer(ctx).ExecContext(ctx, query, label.Name, label.ID) + if err != nil { + return nil, nil, ctxerr.Wrap(ctx, err, "updating mdm configuration profile label") + } + return ds.labelDB(ctx, label.ID, teamFilter, ds.writer(ctx)) } diff --git a/server/datastore/mysql/labels_test.go b/server/datastore/mysql/labels_test.go index 17d97278cd..ca01edfdda 100644 --- a/server/datastore/mysql/labels_test.go +++ b/server/datastore/mysql/labels_test.go @@ -673,6 +673,31 @@ func testLabelsChangeDetails(t *testing.T, db *Datastore) { saved, _, err := db.Label(context.Background(), label.ID, filter) require.Nil(t, err) assert.Equal(t, label.Name, saved.Name) + assert.Equal(t, label.Description, saved.Description) + + // Create an Apple config profile, which should reflect a change in label's name + profA, err := db.NewMDMAppleConfigProfile(context.Background(), *generateCP("a", "a", 0)) + require.NoError(t, err) + ExecAdhocSQL(t, db, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(context.Background(), + "INSERT INTO mdm_configuration_profile_labels (apple_profile_uuid, label_name, label_id) VALUES (?, ?, ?)", + profA.ProfileUUID, label.Name, label.ID) + return err + }) + label.Name = "changed name" + // ApplyLabelSpecs can't update the name -- it simply creates a new label, so we need to call SaveLabel. + saved.Name = label.Name + saved2, _, err := db.SaveLabel(context.Background(), saved, filter) + require.NoError(t, err) + assert.Equal(t, label.Name, saved2.Name) + assert.Equal(t, label.Description, saved2.Description) + + var configProfileLabelName string + ExecAdhocSQL(t, db, func(q sqlx.ExtContext) error { + return sqlx.GetContext(context.Background(), q, &configProfileLabelName, + "SELECT label_name FROM mdm_configuration_profile_labels WHERE label_id = ?", label.ID) + }) + assert.Equal(t, label.Name, configProfileLabelName) } func setupLabelSpecsTest(t *testing.T, ds fleet.Datastore) []*fleet.LabelSpec { @@ -806,6 +831,17 @@ func testLabelsSave(t *testing.T, db *Datastore) { } label, err = db.NewLabel(context.Background(), label) require.NoError(t, err) + + // Create an Apple config profile + profA, err := db.NewMDMAppleConfigProfile(context.Background(), *generateCP("a", "a", 0)) + require.NoError(t, err) + ExecAdhocSQL(t, db, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(context.Background(), + "INSERT INTO mdm_configuration_profile_labels (apple_profile_uuid, label_name, label_id) VALUES (?, ?, ?)", + profA.ProfileUUID, label.Name, label.ID) + return err + }) + label.Name = "changed name" label.Description = "changed description" @@ -819,6 +855,13 @@ func testLabelsSave(t *testing.T, db *Datastore) { assert.Equal(t, label.Name, saved.Name) assert.Equal(t, label.Description, saved.Description) assert.Equal(t, 1, saved.HostCount) + + var configProfileLabelName string + ExecAdhocSQL(t, db, func(q sqlx.ExtContext) error { + return sqlx.GetContext(context.Background(), q, &configProfileLabelName, + "SELECT label_name FROM mdm_configuration_profile_labels WHERE label_id = ?", label.ID) + }) + assert.Equal(t, label.Name, configProfileLabelName) } func testLabelsQueriesForCentOSHost(t *testing.T, db *Datastore) { diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 96643cc718..3d4da386f8 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -4048,21 +4048,28 @@ func (s *integrationTestSuite) TestLabels() { // modify manual label 1 without modifying its hosts modResp = modifyLabelResponse{} - s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/labels/%d", manualLbl1.ID), &fleet.ModifyLabelPayload{Name: ptr.String("modified_manual_label1")}, http.StatusOK, &modResp) + newName := "modified_manual_label1" + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/labels/%d", manualLbl1.ID), &fleet.ModifyLabelPayload{Name: &newName}, http.StatusOK, + &modResp) assert.Equal(t, manualLbl1.ID, modResp.Label.ID) assert.Equal(t, fleet.LabelTypeRegular, modResp.Label.LabelType) assert.Equal(t, fleet.LabelMembershipTypeManual, modResp.Label.LabelMembershipType) assert.ElementsMatch(t, []uint{manualHosts[0].ID, manualHosts[1].ID, manualHosts[2].ID}, modResp.Label.HostIDs) assert.EqualValues(t, 3, modResp.Label.HostCount) + assert.Equal(t, newName, modResp.Label.Name) // modify manual label 2 adding some hosts modResp = modifyLabelResponse{} - s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/labels/%d", manualLbl2.ID), &fleet.ModifyLabelPayload{Hosts: []string{manualHosts[0].UUID}}, http.StatusOK, &modResp) + newName = "modified_manual_label2" + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/labels/%d", manualLbl2.ID), + &fleet.ModifyLabelPayload{Name: &newName, Hosts: []string{manualHosts[0].UUID}}, http.StatusOK, &modResp) assert.Equal(t, manualLbl2.ID, modResp.Label.ID) assert.Equal(t, fleet.LabelTypeRegular, modResp.Label.LabelType) assert.Equal(t, fleet.LabelMembershipTypeManual, modResp.Label.LabelMembershipType) assert.ElementsMatch(t, []uint{manualHosts[0].ID}, modResp.Label.HostIDs) assert.EqualValues(t, 1, modResp.Label.HostCount) + assert.Equal(t, newName, modResp.Label.Name) + manualLbl2.Name = newName // modify manual label 2 clearing its hosts modResp = modifyLabelResponse{} diff --git a/server/service/labels.go b/server/service/labels.go index c79e8fadb9..2922ec51d8 100644 --- a/server/service/labels.go +++ b/server/service/labels.go @@ -165,6 +165,7 @@ func (svc *Service) ModifyLabel(ctx context.Context, id uint, payload fleet.Modi if label.LabelType == fleet.LabelTypeBuiltIn { return nil, nil, fleet.NewInvalidArgumentError("label_type", fmt.Sprintf("cannot modify built-in label '%s'", label.Name)) } + originalLabelName := label.Name if payload.Name != nil { // Check if the new name is a reserved label name for name := range fleet.ReservedLabelNames() { @@ -188,7 +189,7 @@ func (svc *Service) ModifyLabel(ctx context.Context, id uint, payload fleet.Modi // hostnames). if label.LabelMembershipType == fleet.LabelMembershipTypeManual && payload.Hosts != nil { spec := fleet.LabelSpec{ - Name: label.Name, + Name: originalLabelName, Description: label.Description, Query: label.Query, Platform: label.Platform, @@ -200,11 +201,16 @@ func (svc *Service) ModifyLabel(ctx context.Context, id uint, payload fleet.Modi return nil, nil, err } spec.Hosts = hostnames + // Note: ApplyLabelSpecs cannot update label name since it uses the name as a key. + // So, we must handle it later. if err := svc.ds.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{&spec}); err != nil { return nil, nil, err } - - // must reload it to get the host counts information + // If the label name has changed, we must update it. + if originalLabelName != label.Name { + return svc.ds.SaveLabel(ctx, label, filter) + } + // Otherwise, simply reload label to get the host counts information return svc.ds.Label(ctx, id, filter) } return svc.ds.SaveLabel(ctx, label, filter)