From 5656dcf801d7f3dba41e4a5a4a1b8ad425540df2 Mon Sep 17 00:00:00 2001 From: Magnus Jensen Date: Mon, 26 Jan 2026 12:55:26 -0500 Subject: [PATCH] improve missing label error message (#38636) **Related issue:** Resolves #37183 Software request: ```json { "message": "Bad request", "errors": [ { "name": "base", "reason": "Couldn't update. Label \"non-existing-label\" doesn't exist. Please remove the label from the software." } ], "uuid": "3a9a4da3-d7af-4ed5-8b39-73e9f465f103" } ``` Config profile: ```json { "message": "Bad request", "errors": [ { "name": "base", "reason": "Couldn't update. Label \"non-existent-label\" doesn't exist. Please remove the label from the configuration profile." } ], "uuid": "ea842e7b-d4eb-4b59-bf24-32ad66d538dd" } ``` # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] 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. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually --- changes/37183-unclear-missing-label-error | 1 + cmd/fleetctl/fleetctl/gitops.go | 4 ++-- ee/server/service/software_installers.go | 7 ++++++ server/fleet/labels.go | 24 +++++++++++++++++++ server/service/apple_mdm.go | 16 +++++-------- server/service/integration_enterprise_test.go | 5 ++-- .../service/integration_mdm_profiles_test.go | 20 ++++++++-------- server/service/integration_mdm_test.go | 2 +- server/service/labels.go | 5 +--- server/service/mdm.go | 14 ++++------- server/service/mdm_test.go | 2 +- server/service/software_installers_test.go | 4 ++-- 12 files changed, 62 insertions(+), 42 deletions(-) create mode 100644 changes/37183-unclear-missing-label-error diff --git a/changes/37183-unclear-missing-label-error b/changes/37183-unclear-missing-label-error new file mode 100644 index 0000000000..0f6efec516 --- /dev/null +++ b/changes/37183-unclear-missing-label-error @@ -0,0 +1 @@ +- Improved API error message when adding profiles or software with non-existent labels. \ No newline at end of file diff --git a/cmd/fleetctl/fleetctl/gitops.go b/cmd/fleetctl/fleetctl/gitops.go index f29b9c5226..686e0e2c8f 100644 --- a/cmd/fleetctl/fleetctl/gitops.go +++ b/cmd/fleetctl/fleetctl/gitops.go @@ -328,8 +328,8 @@ func gitopsCommand() *cli.Command { builtInLabelsUsed = true continue } - for _, labelUsed := range labelsUsed[labelUsed] { - logf("[!] Unknown label '%s' is referenced by %s '%s'\n", labelUsed, labelUsed.Type, labelUsed.Name) + for _, labelUsage := range labelsUsed[labelUsed] { + logf("[!] Unknown label '%s' is referenced by %s '%s'\n", labelUsed, labelUsage.Type, labelUsage.Name) } unknownLabelsUsed = true } diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index 18557ca6f2..20f39d2a49 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -221,6 +221,13 @@ func ValidateSoftwareLabels(ctx context.Context, svc fleet.Service, teamID *uint byName, err := svc.BatchValidateLabels(ctx, teamID, names) if err != nil { + var missingLabelErr *fleet.MissingLabelError + if errors.As(err, &missingLabelErr) { + return nil, &fleet.BadRequestError{ + InternalErr: missingLabelErr, + Message: fmt.Sprintf("Couldn't update. Label %q doesn't exist. Please remove the label from the software.", missingLabelErr.MissingLabelName), + } + } return nil, err } diff --git a/server/fleet/labels.go b/server/fleet/labels.go index 17d10d94e7..e6f65f4bcc 100644 --- a/server/fleet/labels.go +++ b/server/fleet/labels.go @@ -418,3 +418,27 @@ func parseHostVitalCriteria(criteria *HostVitalCriteria, foreignVitalsGroups map } return fmt.Sprintf("%s = ?", vital.Path), nil } + +type MissingLabelError struct { + *BadRequestError + MissingLabelName string +} + +// NewMissingLabelError creates a new MissingLabelError, determining which label name was missing +// based on the provided list of labels and the map of found labels. +func NewMissingLabelError(providedLabels []string, foundLabels map[string]uint) *MissingLabelError { + notFoundLabel := "" + for _, name := range providedLabels { + if _, ok := foundLabels[name]; !ok { + notFoundLabel = name + break + } + } + return &MissingLabelError{ + BadRequestError: &BadRequestError{ + Message: "some or all the labels provided don't exist", + InternalErr: fmt.Errorf("names provided: %v", providedLabels), + }, + MissingLabelName: notFoundLabel, + } +} diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index 4f670a9c2b..1e57addfef 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -967,22 +967,18 @@ func (svc *Service) batchValidateDeclarationLabels(ctx context.Context, labelNam return nil, nil } - labels, err := svc.ds.LabelIDsByName(ctx, labelNames, fleet.TeamFilter{User: authz.UserFromContext(ctx), TeamID: &teamID}) + uniqueNames := server.RemoveDuplicatesFromSlice(labelNames) + + labels, err := svc.ds.LabelIDsByName(ctx, uniqueNames, fleet.TeamFilter{User: authz.UserFromContext(ctx), TeamID: &teamID}) if err != nil { return nil, ctxerr.Wrap(ctx, err, "getting label IDs by name") } - uniqueNames := make(map[string]bool) - for _, entry := range labelNames { - if _, value := uniqueNames[entry]; !value { - uniqueNames[entry] = true - } - } - if len(labels) != len(uniqueNames) { + labelError := fleet.NewMissingLabelError(uniqueNames, labels) return nil, &fleet.BadRequestError{ - Message: "some or all the labels provided don't exist", - InternalErr: fmt.Errorf("names provided: %v", labelNames), + InternalErr: labelError, + Message: fmt.Sprintf("Couldn't update. Label %q doesn't exist. Please remove the label from the configuration profile.", labelError.MissingLabelName), } } diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 801a322793..7c7859de38 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -13221,7 +13221,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallers() { {URL: rubyURL, LabelsIncludeAny: []string{"no-such-label"}}, } res = s.Do("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusBadRequest) - require.Contains(t, extractServerErrorText(res.Body), `some or all the labels provided don't exist`) + require.Contains(t, extractServerErrorText(res.Body), `Couldn't update. Label "no-such-label" doesn't exist. Please remove the label from the software.`) // valid installer scoped by label softwareToInstall = []*fleet.SoftwareInstallerPayload{ @@ -19271,7 +19271,7 @@ func (s *integrationEnterpriseTestSuite) TestMaintainedApps() { } addMAResp = addFleetMaintainedAppResponse{} r = s.Do("POST", "/api/latest/fleet/software/fleet_maintained_apps", req, http.StatusBadRequest) - require.Contains(t, extractServerErrorText(r.Body), "some or all the labels provided don't exist") + require.Contains(t, extractServerErrorText(r.Body), `Couldn't update. Label "no-such-label" doesn't exist. Please remove the label from the software.`) // Can't set both labels_include_any and labels_exclude_any req.LabelsIncludeAny = []string{lbl1.Name, lbl2.Name} @@ -23383,7 +23383,6 @@ qcznMoapfGAjRwaheTlWbzyUh57ToALyx3xQbzqYIxiQCzY= "{\"bypass_disabled\": true}", 0, ) - } // generateTestCertForDeviceAuth generates a test certificate for device authentication. diff --git a/server/service/integration_mdm_profiles_test.go b/server/service/integration_mdm_profiles_test.go index 86e5ef314e..897e590204 100644 --- a/server/service/integration_mdm_profiles_test.go +++ b/server/service/integration_mdm_profiles_test.go @@ -3385,10 +3385,10 @@ func (s *integrationMDMTestSuite) TestMDMConfigProfileCRUD() { } // profiles with non-existent labels - assertAppleProfile("apple-profile-with-labels.mobileconfig", "apple-profile-with-labels", "ident-with-labels", 0, []string{"does-not-exist"}, http.StatusBadRequest, "some or all the labels provided don't exist") - assertAppleDeclaration("apple-declaration-with-labels.json", "ident-with-labels", 0, []string{"does-not-exist"}, http.StatusBadRequest, "some or all the labels provided don't exist") - assertWindowsProfile("win-profile-with-labels.xml", "./Test", 0, []string{"does-not-exist"}, http.StatusBadRequest, "some or all the labels provided don't exist") - assertAndroidProfile("android-with-labels.json", 0, []string{"does-not-exist"}, http.StatusBadRequest, "some or all the labels provided don't exist") + assertAppleProfile("apple-profile-with-labels.mobileconfig", "apple-profile-with-labels", "ident-with-labels", 0, []string{"does-not-exist"}, http.StatusBadRequest, `Couldn't update. Label "does-not-exist" doesn't exist. Please remove the label from the configuration profile.`) + assertAppleDeclaration("apple-declaration-with-labels.json", "ident-with-labels", 0, []string{"does-not-exist"}, http.StatusBadRequest, `Couldn't update. Label "does-not-exist" doesn't exist. Please remove the label from the configuration profile.`) + assertWindowsProfile("win-profile-with-labels.xml", "./Test", 0, []string{"does-not-exist"}, http.StatusBadRequest, `Couldn't update. Label "does-not-exist" doesn't exist. Please remove the label from the configuration profile.`) + assertAndroidProfile("android-with-labels.json", 0, []string{"does-not-exist"}, http.StatusBadRequest, `Couldn't update. Label "does-not-exist" doesn't exist. Please remove the label from the configuration profile.`) // create a couple of labels labelFoo := &fleet.Label{Name: "foo", Query: "select * from foo;"} @@ -3399,10 +3399,10 @@ func (s *integrationMDMTestSuite) TestMDMConfigProfileCRUD() { require.NoError(t, err) // profiles mixing existent and non-existent labels - assertAppleProfile("apple-profile-with-labels.mobileconfig", "apple-profile-with-labels", "ident-with-labels", 0, []string{"does-not-exist", "foo"}, http.StatusBadRequest, "some or all the labels provided don't exist") - assertAppleDeclaration("apple-declaration-with-labels.json", "ident-with-labels", 0, []string{"does-not-exist", "foo"}, http.StatusBadRequest, "some or all the labels provided don't exist") - assertWindowsProfile("win-profile-with-labels.xml", "./Test", 0, []string{"does-not-exist", "bar"}, http.StatusBadRequest, "some or all the labels provided don't exist") - assertAndroidProfile("android-profile-with-labels.json", 0, []string{"does-not-exist", "bar"}, http.StatusBadRequest, "some or all the labels provided don't exist") + assertAppleProfile("apple-profile-with-labels.mobileconfig", "apple-profile-with-labels", "ident-with-labels", 0, []string{"does-not-exist", "foo"}, http.StatusBadRequest, `Couldn't update. Label "does-not-exist" doesn't exist. Please remove the label from the configuration profile.`) + assertAppleDeclaration("apple-declaration-with-labels.json", "ident-with-labels", 0, []string{"does-not-exist", "foo"}, http.StatusBadRequest, `Couldn't update. Label "does-not-exist" doesn't exist. Please remove the label from the configuration profile.`) + assertWindowsProfile("win-profile-with-labels.xml", "./Test", 0, []string{"does-not-exist", "bar"}, http.StatusBadRequest, `Couldn't update. Label "does-not-exist" doesn't exist. Please remove the label from the configuration profile.`) + assertAndroidProfile("android-profile-with-labels.json", 0, []string{"does-not-exist", "bar"}, http.StatusBadRequest, `Couldn't update. Label "does-not-exist" doesn't exist. Please remove the label from the configuration profile.`) // profiles with invalid mix of labels assertAppleProfile("apple-invalid-profile-with-labels.mobileconfig", "apple-invalid-profile-with-labels", "ident-with-labels", 0, []string{"foo", "!bar"}, http.StatusBadRequest, `Only one of "labels_exclude_any", "labels_include_all", "labels_include_any", or "labels" can be included.`) @@ -4743,7 +4743,7 @@ func (s *integrationMDMTestSuite) TestBatchSetMDMProfiles() { {Name: "N1", Contents: mobileconfigForTest("N1", "I1"), Labels: []string{lbl1.Name, "no-such-label"}}, }}, http.StatusBadRequest) msg := extractServerErrorText(res.Body) - require.Contains(t, msg, "some or all the labels provided don't exist") + require.Contains(t, msg, `Couldn't update. Label "no-such-label" doesn't exist. Please remove the label from the configuration profile.`) // mix of labels fields res = s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{ @@ -5042,7 +5042,7 @@ func (s *integrationMDMTestSuite) TestBatchModifyMDMProfiles() { {DisplayName: "N1", Profile: mobileconfigForTest("N1", "I1"), LabelsIncludeAll: []string{lbl1.Name, "no-such-label"}}, }}, http.StatusBadRequest) msg := extractServerErrorText(res.Body) - require.Contains(t, msg, "some or all the labels provided don't exist") + require.Contains(t, msg, `Couldn't update. Label "no-such-label" doesn't exist. Please remove the label from the configuration profile.`) // mix of labels fields res = s.Do("POST", "/api/latest/fleet/configuration_profiles/batch", batchModifyMDMConfigProfilesRequest{ConfigurationProfiles: []fleet.BatchModifyMDMConfigProfilePayload{ diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index c0c3d91e63..953883a998 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -12720,7 +12720,7 @@ func (s *integrationMDMTestSuite) TestVPPApps() { updateAppReq.LabelsExcludeAny = []string{} updateAppReq.LabelsIncludeAny = []string{"404_notfound"} res = s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", titleID), updateAppReq, http.StatusBadRequest) - require.Contains(t, extractServerErrorText(res.Body), "some or all the labels provided don't exist") + require.Contains(t, extractServerErrorText(res.Body), `Couldn't update. Label "404_notfound" doesn't exist. Please remove the label from the software.`) // Update App2. Unset self service and update the labels updateAppReq.LabelsIncludeAny = []string{l2.Name} diff --git a/server/service/labels.go b/server/service/labels.go index 7ca452514f..01e47e788b 100644 --- a/server/service/labels.go +++ b/server/service/labels.go @@ -853,10 +853,7 @@ func (svc *Service) BatchValidateLabels(ctx context.Context, teamID *uint, label } if len(labels) != len(uniqueNames) { - return nil, &fleet.BadRequestError{ - Message: "some or all the labels provided don't exist", - InternalErr: fmt.Errorf("names provided: %v", labelNames), - } + return nil, fleet.NewMissingLabelError(uniqueNames, labels) } if err := verifyLabelsToAssociate(ctx, svc.ds, teamID, labelNames, authz.UserFromContext(ctx)); err != nil { diff --git a/server/service/mdm.go b/server/service/mdm.go index 084c769bce..6e8dc125ea 100644 --- a/server/service/mdm.go +++ b/server/service/mdm.go @@ -1828,22 +1828,18 @@ func (svc *Service) batchValidateProfileLabels(ctx context.Context, teamID *uint return nil, nil } + uniqueNames := server.RemoveDuplicatesFromSlice(labelNames) + labels, err := svc.ds.LabelIDsByName(ctx, labelNames, fleet.TeamFilter{User: authz.UserFromContext(ctx)}) if err != nil { return nil, ctxerr.Wrap(ctx, err, "getting label IDs by name") } - uniqueNames := make(map[string]bool) - for _, entry := range labelNames { - if _, value := uniqueNames[entry]; !value { - uniqueNames[entry] = true - } - } - if len(labels) != len(uniqueNames) { + labelError := fleet.NewMissingLabelError(uniqueNames, labels) return nil, &fleet.BadRequestError{ - Message: "some or all the labels provided don't exist", - InternalErr: fmt.Errorf("names provided: %v", labelNames), + InternalErr: labelError, + Message: fmt.Sprintf("Couldn't update. Label %q doesn't exist. Please remove the label from the configuration profile.", labelError.MissingLabelName), } } diff --git a/server/service/mdm_test.go b/server/service/mdm_test.go index 83022c4ac7..c057890468 100644 --- a/server/service/mdm_test.go +++ b/server/service/mdm_test.go @@ -2475,7 +2475,7 @@ func TestBatchSetMDMProfilesLabels(t *testing.T) { LabelsExcludeAny: []string{"baddy"}, }}, false, false, ptr.Bool(true), false) require.Error(t, err) - require.ErrorContains(t, err, "some or all the labels provided don't exist") + require.ErrorContains(t, err, `Label "baddy" doesn't exist. Please remove the label from the configuration profile.`) // ...unless we're in dry run mode err = svc.BatchSetMDMProfiles(authCtx, ptr.Uint(1), nil, []fleet.MDMProfileBatchPayload{{ diff --git a/server/service/software_installers_test.go b/server/service/software_installers_test.go index fc739aacfc..865a173216 100644 --- a/server/service/software_installers_test.go +++ b/server/service/software_installers_test.go @@ -313,7 +313,7 @@ func TestValidateSoftwareLabels(t *testing.T) { nil, nil, "", - "some or all the labels provided don't exist", + `Couldn't update. Label "qux" doesn't exist. Please remove the label from the software`, }, { "duplicate label", @@ -339,7 +339,7 @@ func TestValidateSoftwareLabels(t *testing.T) { []string{""}, nil, "", - "some or all the labels provided don't exist", + `Couldn't update. Label "" doesn't exist. Please remove the label from the software`, }, } for _, tt := range testCases {