improve missing label error message (#38636)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **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
This commit is contained in:
@@ -0,0 +1 @@
|
||||
- Improved API error message when adding profiles or software with non-existent labels.
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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{
|
||||
|
||||
@@ -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}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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{{
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user