Bugfix: config profile deployment based on label exclusion (#23533)

changes/22162-exclude-labels-fix-default-behavior

@@ -0,0 +1 @@
+* Fixed the MDM configuration profiles deployment when based on excluded labels - prior to this fix, hosts were considered "not a member" of the label by default, even if they had not yet returned results for the excluded labels. The fix checks the label's creation time vs the host's last reported label results timestamp to prevent deploying a configuration profile if it does not yet know if the host is a member or not of those labels.

server/datastore/mysql/apple_mdm.go

@@ -2318,7 +2318,8 @@ func generateDesiredStateQuery(entityType string) string {
 		mae.checksum as checksum,
 		0 as ${countEntityLabelsColumn},
 		0 as count_non_broken_labels,
-		0 as count_host_labels
+		0 as count_host_labels,
+		0 as count_host_updated_after_labels
 	FROM
 		${mdmAppleEntityTable} mae
 			JOIN hosts h
@@ -2350,7 +2351,8 @@ func generateDesiredStateQuery(entityType string) string {
 		mae.checksum as checksum,
 		COUNT(*) as ${countEntityLabelsColumn},
 		COUNT(mel.label_id) as count_non_broken_labels,
-		COUNT(lm.label_id) as count_host_labels
+		COUNT(lm.label_id) as count_host_labels,
+		0 as count_host_updated_after_labels
 	FROM
 		${mdmAppleEntityTable} mae
 			JOIN hosts h
@@ -2374,7 +2376,10 @@ func generateDesiredStateQuery(entityType string) string {
 	UNION
 
 	-- label-based entities where the host is NOT a member of any of the labels (exclude-any).
-	-- explicitly ignore profiles with broken excluded labels so that they are never applied.
+	-- explicitly ignore profiles with broken excluded labels so that they are never applied,
+	-- and ignore profiles that depend on labels created _after_ the label_updated_at timestamp
+	-- of the host (because we don't have results for that label yet, the host may or may not be
+	-- a member).
 	SELECT
 		mae.${entityUUIDColumn},
 		h.uuid as host_uuid,
@@ -2384,7 +2389,10 @@ func generateDesiredStateQuery(entityType string) string {
 		mae.checksum as checksum,
 		COUNT(*) as ${countEntityLabelsColumn},
 		COUNT(mel.label_id) as count_non_broken_labels,
-		COUNT(lm.label_id) as count_host_labels
+		COUNT(lm.label_id) as count_host_labels,
+		-- this helps avoid the case where the host is not a member of a label 
+		-- just because it hasn't reported results for that label yet.
+		SUM(CASE WHEN lbl.created_at IS NOT NULL AND h.label_updated_at >= lbl.created_at THEN 1 ELSE 0 END) as count_host_updated_after_labels
 	FROM
 		${mdmAppleEntityTable} mae
 			JOIN hosts h
@@ -2393,6 +2401,8 @@ func generateDesiredStateQuery(entityType string) string {
 				ON ne.device_id = h.uuid
 			JOIN ${mdmEntityLabelsTable} mel
 				ON mel.${appleEntityUUIDColumn} = mae.${entityUUIDColumn} AND mel.exclude = 1
+			LEFT OUTER JOIN labels lbl
+				ON lbl.id = mel.label_id
 			LEFT OUTER JOIN label_membership lm
 				ON lm.label_id = mel.label_id AND lm.host_id = h.id
 	WHERE
@@ -2403,8 +2413,8 @@ func generateDesiredStateQuery(entityType string) string {
 	GROUP BY
 		mae.${entityUUIDColumn}, h.uuid, h.platform, mae.identifier, mae.name, mae.checksum
 	HAVING
-		-- considers only the profiles with labels, without any broken label, and with the host not in any label
-		${countEntityLabelsColumn} > 0 AND ${countEntityLabelsColumn} = count_non_broken_labels AND count_host_labels = 0
+		-- considers only the profiles with labels, without any broken label, with results reported after all labels were created and with the host not in any label
+		${countEntityLabelsColumn} > 0 AND ${countEntityLabelsColumn} = count_non_broken_labels AND ${countEntityLabelsColumn} = count_host_updated_after_labels AND count_host_labels = 0
 	`, func(s string) string { return dynamicNames[s] })
 }
 

server/datastore/mysql/mdm_test.go

@@ -7358,7 +7358,7 @@ func testBulkSetPendingMDMHostProfilesExcludeAny(t *testing.T, ds *Datastore) {
 	}
 	allProfs := getProfs(nil)
 
-	// create an Apple and Windows hosts, not members of any host
+	// create an Apple and Windows hosts, not members of any label
 	var i int
 	winHost, err := ds.NewHost(ctx, &fleet.Host{
 		Hostname:      fmt.Sprintf("win-host%d-name", i),
@@ -7381,14 +7381,33 @@ func testBulkSetPendingMDMHostProfilesExcludeAny(t *testing.T, ds *Datastore) {
 	require.NoError(t, err)
 	nanoEnroll(t, ds, appleHost, false)
 
-	// do a sync, they get all platform-specific profiles since they are not part
-	// of any label
+	// at this point the hosts have not reported any label results, so a sync
+	// does NOT install the exclude any profiles as we don't know yet if the
+	// hosts will be members or not
+	updates, err = ds.BulkSetPendingMDMHostProfiles(ctx, []uint{winHost.ID, appleHost.ID}, nil, nil, nil)
+	require.NoError(t, err)
+	assert.False(t, updates.AppleConfigProfile)
+	assert.False(t, updates.WindowsConfigProfile)
+	assert.False(t, updates.AppleDeclaration)
+	assertHostProfiles(t, ds, map[*fleet.Host][]anyProfile{
+		appleHost: {},
+		winHost:   {},
+	})
+
+	// setting the LabelsUpdatedAt timestamp means that labels results were reported, so now
+	// the profiles will be installed as the hosts are not members of the excluded labels.
+	winHost.LabelUpdatedAt = time.Now()
+	appleHost.LabelUpdatedAt = time.Now()
+	err = ds.UpdateHost(ctx, winHost)
+	require.NoError(t, err)
+	err = ds.UpdateHost(ctx, appleHost)
+	require.NoError(t, err)
+
 	updates, err = ds.BulkSetPendingMDMHostProfiles(ctx, []uint{winHost.ID, appleHost.ID}, nil, nil, nil)
 	require.NoError(t, err)
 	assert.True(t, updates.AppleConfigProfile)
 	assert.True(t, updates.WindowsConfigProfile)
 	assert.True(t, updates.AppleDeclaration)
-
 	assertHostProfiles(t, ds, map[*fleet.Host][]anyProfile{
 		appleHost: {
 			{
@@ -7555,6 +7574,13 @@ func testBulkSetPendingMDMHostProfilesExcludeAny(t *testing.T, ds *Datastore) {
 	require.NoError(t, err)
 	nanoEnroll(t, ds, appleHost2, false)
 
+	winHost2.LabelUpdatedAt = time.Now()
+	appleHost2.LabelUpdatedAt = time.Now()
+	err = ds.UpdateHost(ctx, winHost2)
+	require.NoError(t, err)
+	err = ds.UpdateHost(ctx, appleHost2)
+	require.NoError(t, err)
+
 	updates, err = ds.BulkSetPendingMDMHostProfiles(ctx, []uint{winHost.ID, appleHost.ID, winHost2.ID, appleHost2.ID}, nil, nil, nil)
 	require.NoError(t, err)
 	assert.False(t, updates.AppleConfigProfile)

server/datastore/mysql/microsoft_mdm.go

@@ -1131,7 +1131,8 @@ const windowsMDMProfilesDesiredStateQuery = `
 		h.uuid as host_uuid,
 		0 as count_profile_labels,
 		0 as count_non_broken_labels,
-		0 as count_host_labels
+		0 as count_host_labels,
+		0 as count_host_updated_after_labels
 	FROM
 		mdm_windows_configuration_profiles mwcp
 			JOIN hosts h
@@ -1158,7 +1159,8 @@ const windowsMDMProfilesDesiredStateQuery = `
 		h.uuid as host_uuid,
 		COUNT(*) as count_profile_labels,
 		COUNT(mcpl.label_id) as count_non_broken_labels,
-		COUNT(lm.label_id) as count_host_labels
+		COUNT(lm.label_id) as count_host_labels,
+		0 as count_host_updated_after_labels
 	FROM
 		mdm_windows_configuration_profiles mwcp
 			JOIN hosts h
@@ -1180,14 +1182,20 @@ const windowsMDMProfilesDesiredStateQuery = `
 	UNION
 
 	-- label-based entities where the host is NOT a member of any of the labels (exclude-any).
-	-- explicitly ignore profiles with broken excluded labels so that they are never applied.
+	-- explicitly ignore profiles with broken excluded labels so that they are never applied,
+	-- and ignore profiles that depend on labels created _after_ the label_updated_at timestamp
+	-- of the host (because we don't have results for that label yet, the host may or may not be
+	-- a member).
 	SELECT
 		mwcp.profile_uuid,
 		mwcp.name,
 		h.uuid as host_uuid,
 		COUNT(*) as count_profile_labels,
 		COUNT(mcpl.label_id) as count_non_broken_labels,
-		COUNT(lm.label_id) as count_host_labels
+		COUNT(lm.label_id) as count_host_labels,
+		-- this helps avoid the case where the host is not a member of a label
+		-- just because it hasn't reported results for that label yet.
+		SUM(CASE WHEN lbl.created_at IS NOT NULL AND h.label_updated_at >= lbl.created_at THEN 1 ELSE 0 END) as count_host_updated_after_labels
 	FROM
 		mdm_windows_configuration_profiles mwcp
 			JOIN hosts h
@@ -1196,6 +1204,8 @@ const windowsMDMProfilesDesiredStateQuery = `
 				ON mwe.host_uuid = h.uuid
 			JOIN mdm_configuration_profile_labels mcpl
 				ON mcpl.windows_profile_uuid = mwcp.profile_uuid AND mcpl.exclude = 1
+			LEFT OUTER JOIN labels lbl
+				ON lbl.id = mcpl.label_id
 			LEFT OUTER JOIN label_membership lm
 				ON lm.label_id = mcpl.label_id AND lm.host_id = h.id
 	WHERE
@@ -1204,8 +1214,8 @@ const windowsMDMProfilesDesiredStateQuery = `
 	GROUP BY
 		mwcp.profile_uuid, mwcp.name, h.uuid
 	HAVING
-		-- considers only the profiles with labels, without any broken label, and with the host not in any label
-		count_profile_labels > 0 AND count_profile_labels = count_non_broken_labels AND count_host_labels = 0
+		-- considers only the profiles with labels, without any broken label, with results reported after all labels were created and with the host not in any label
+		count_profile_labels > 0 AND count_profile_labels = count_non_broken_labels AND count_profile_labels = count_host_updated_after_labels AND count_host_labels = 0
 `
 
 func (ds *Datastore) ListMDMWindowsProfilesToInstall(ctx context.Context) ([]*fleet.MDMWindowsProfilePayload, error) {

server/service/integration_mdm_profiles_test.go

@@ -4653,6 +4653,13 @@ func (s *integrationMDMTestSuite) TestHostMDMProfilesExcludeLabels() {
 		require.NoError(t, err)
 		labels[i] = label
 	}
+	// simulate reporting label results for those hosts
+	appleHost.LabelUpdatedAt = time.Now()
+	windowsHost.LabelUpdatedAt = time.Now()
+	err := s.ds.UpdateHost(ctx, appleHost)
+	require.NoError(t, err)
+	err = s.ds.UpdateHost(ctx, windowsHost)
+	require.NoError(t, err)
 
 	// set an Apple profile and declaration and a Windows profile
 	s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{
@@ -4694,7 +4701,7 @@ func (s *integrationMDMTestSuite) TestHostMDMProfilesExcludeLabels() {
 	})
 
 	// mark some profiles as verified (despite accepting a HostMacOSProfile struct, it supports Windows too)
-	err := apple_mdm.VerifyHostMDMProfiles(ctx, s.ds, appleHost, map[string]*fleet.HostMacOSProfile{
+	err = apple_mdm.VerifyHostMDMProfiles(ctx, s.ds, appleHost, map[string]*fleet.HostMacOSProfile{
 		"A1": {Identifier: "A1", DisplayName: "A1", InstallDate: time.Now()},
 	})
 	require.NoError(t, err)