diff --git a/changes/28154-fix-gitops-dry-run-labels b/changes/28154-fix-gitops-dry-run-labels new file mode 100644 index 0000000000..8c7caf2021 --- /dev/null +++ b/changes/28154-fix-gitops-dry-run-labels @@ -0,0 +1 @@ +- Fixed issue where `fleetctl gitops --dry-run` would sometimes fail when creating and using labels in the same run diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index e55d710c50..f120f3ed33 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -1501,12 +1501,13 @@ func (svc *Service) BatchSetSoftwareInstallers( fmt.Sprintf("Couldn't edit software. URL (%q) is invalid", payload.URL), ) } - validatedLabels, err := ValidateSoftwareLabels(ctx, svc, payload.LabelsIncludeAny, payload.LabelsExcludeAny) - if err != nil { - return "", err + if !dryRun { + validatedLabels, err := ValidateSoftwareLabels(ctx, svc, payload.LabelsIncludeAny, payload.LabelsExcludeAny) + if err != nil { + return "", err + } + payload.ValidatedLabels = validatedLabels } - payload.ValidatedLabels = validatedLabels - allScripts = append(allScripts, payload.InstallScript, payload.PostInstallScript, payload.UninstallScript) } diff --git a/server/service/mdm.go b/server/service/mdm.go index 35e9fdadb7..c1c8bf1243 100644 --- a/server/service/mdm.go +++ b/server/service/mdm.go @@ -1635,9 +1635,12 @@ func (svc *Service) BatchSetMDMProfiles( labels = append(labels, profiles[i].LabelsIncludeAny...) labels = append(labels, profiles[i].LabelsExcludeAny...) } - labelMap, err := svc.batchValidateProfileLabels(ctx, labels) - if err != nil { - return ctxerr.Wrap(ctx, err, "validating labels") + var labelMap map[string]fleet.ConfigurationProfileLabel + if !dryRun { + labelMap, err = svc.batchValidateProfileLabels(ctx, labels) + if err != nil { + return ctxerr.Wrap(ctx, err, "validating labels") + } } // We will not validate the profiles containing secret variables during dry run. diff --git a/server/service/mdm_test.go b/server/service/mdm_test.go index cb86c9426b..72592562c4 100644 --- a/server/service/mdm_test.go +++ b/server/service/mdm_test.go @@ -1219,9 +1219,11 @@ func TestUploadWindowsMDMConfigProfileValidations(t *testing.T) { {"duplicate profile name", 0, `duplicate`, true, "configuration profile with this name already exists"}, {"multiple Replace", 0, `ab`, true, ""}, {"Replace and non-Replace", 0, `ab`, true, "Windows configuration profiles can only have or top level elements."}, - {"BitLocker profile", 0, + { + "BitLocker profile", 0, `./Device/Vendor/MSFT/BitLocker/AllowStandardUserEncryption`, true, - syncml.DiskEncryptionProfileRestrictionErrMsg}, + syncml.DiskEncryptionProfileRestrictionErrMsg, + }, {"Windows updates profile", 0, ` ./Device/Vendor/MSFT/Policy/Config/Update/ConfigureDeadlineNoAutoRebootForFeatureUpdates `, true, "Custom configuration profiles can't include Windows updates settings."}, {"unsupported Fleet variable", 0, `$FLEET_VAR_BOZO`, true, "Fleet variable"}, @@ -1233,9 +1235,11 @@ func TestUploadWindowsMDMConfigProfileValidations(t *testing.T) { {"team duplicate profile name", 1, `duplicate`, true, "configuration profile with this name already exists"}, {"team multiple Replace", 1, `ab`, true, ""}, {"team Replace and non-Replace", 1, `ab`, true, "Windows configuration profiles can only have or top level elements."}, - {"team BitLocker profile", 1, + { + "team BitLocker profile", 1, `./Device/Vendor/MSFT/BitLocker/AllowStandardUserEncryption`, true, - syncml.DiskEncryptionProfileRestrictionErrMsg}, + syncml.DiskEncryptionProfileRestrictionErrMsg, + }, {"team Windows updates profile", 1, ` ./Device/Vendor/MSFT/Policy/Config/Update/ConfigureDeadlineNoAutoRebootForFeatureUpdates `, true, "Custom configuration profiles can't include Windows updates settings."}, {"invalid team", 2, ``, true, "not found"}, @@ -2117,8 +2121,10 @@ func TestBatchSetMDMProfilesLabels(t *testing.T) { ds.LabelIDsByNameFunc = func(ctx context.Context, labels []string) (map[string]uint, error) { m := map[string]uint{} for _, label := range labels { - labelID++ - m[label] = labelID + if label != "baddy" { + labelID++ + m[label] = labelID + } } return m, nil } @@ -2196,4 +2202,21 @@ func TestBatchSetMDMProfilesLabels(t *testing.T) { assert.Equal(t, ProfileLabels{IncludeAll: true}, *profileLabels["DIncAll"]) assert.Equal(t, ProfileLabels{IncludeAny: true}, *profileLabels["DIncAny"]) assert.Equal(t, ProfileLabels{ExcludeAny: true}, *profileLabels["DExclAny"]) + + // Test that a bad label doesn't pass validation... + err = svc.BatchSetMDMProfiles(authCtx, ptr.Uint(1), nil, []fleet.MDMProfileBatchPayload{{ + Name: "Baddy", + Contents: declarationForTest("Baddy"), + 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") + + // ...unless we're in dry run mode + err = svc.BatchSetMDMProfiles(authCtx, ptr.Uint(1), nil, []fleet.MDMProfileBatchPayload{{ + Name: "Baddy", + Contents: declarationForTest("Baddy"), + LabelsExcludeAny: []string{"baddy"}, + }}, true, false, ptr.Bool(true), false) + require.NoError(t, err) }