Revert independent doNotDeploy behavior

Author: p-se

integrationtests/controller/bundle/bundle_targets_test.go

@@ -459,22 +459,23 @@ var _ = Describe("Bundle targets", Ordered, func() {
 		})
 	})
 
-	// Regression test for https://github.com/rancher/fleet/issues/3580:
+	// With FirstMatch semantics (the default), only the first matching targetCustomization is applied.
 	// When a broader-matching target appears before a doNotDeploy target in the list,
-	// the doNotDeploy target was previously bypassed due to first-match semantics.
+	// the broader match wins and the doNotDeploy entry is never evaluated.
+	// Users must order their targetCustomizations from specific to general to use doNotDeploy effectively.
 	When("a broader-matching targetCustomization appears before a doNotDeploy targetCustomization", func() {
 		BeforeEach(func() {
 			bundleName = "donot-deploy-after-broader-match"
 			bdLabels = map[string]string{
 				"fleet.cattle.io/bundle-name":      bundleName,
 				"fleet.cattle.io/bundle-namespace": namespace,
 			}
-			expectedNumberOfBundleDeployments = 0
+			expectedNumberOfBundleDeployments = 1
 			// simulate targets in fleet.yaml:
 			// - first entry matches all clusters (broader match)
 			// - second entry matches cluster "one" with doNotDeploy=true
-			// With the old first-match logic, cluster "one" would match the first entry and
-			// the doNotDeploy entry would never be evaluated.
+			// With FirstMatch semantics, cluster "one" matches the first entry and gets deployed.
+			// The doNotDeploy entry is never evaluated because the first match wins.
 			targets = []v1alpha1.BundleTarget{
 				{
 					BundleDeploymentOptions: v1alpha1.BundleDeploymentOptions{
@@ -500,7 +501,7 @@ var _ = Describe("Bundle targets", Ordered, func() {
 			targets = append(targets, targetsInGitRepo...)
 		})
 
-		It("no BundleDeployments are created", func() {
+		It("creates a BundleDeployment because first match wins", func() {
 			waitForBundleToBeReady(bundleName)
 			_ = verifyBundlesDeploymentsAreCreated(expectedNumberOfBundleDeployments, bdLabels, bundleName)
 		})

internal/cmd/controller/target/builder.go

@@ -72,6 +72,7 @@ func (m *Manager) Targets(ctx context.Context, bundle *fleet.Bundle, manifestID
 		if err != nil {
 			return nil, false, err
 		}
+	clusterLoop:
 		for _, cluster := range clusters.Items {
 			logger.V(4).Info("Cluster has namespace?", "cluster", cluster.Name, "namespace", cluster.Status.Namespace)
 			clusterGroups, err := m.clusterGroupsForCluster(ctx, &cluster)
@@ -85,28 +86,50 @@ func (m *Manager) Targets(ctx context.Context, bundle *fleet.Bundle, manifestID
 			if target == nil {
 				continue
 			}
-			// Check all matching targetCustomizations for doNotDeploy, not just the first match.
-			// This ensures that a doNotDeploy entry is honoured even when a broader-matching
-			// target appears before it in the target list (fixes first-match bypass).
-			doNotDeploy := target.DoNotDeploy || bm.HasDoNotDeployTarget(cluster.Name, clusterGroupsAsLabelMap, cluster.Labels)
-			if doNotDeploy {
+			// Check if the GitRepo target has doNotDeploy set
+			if target.DoNotDeploy {
 				logger.V(1).Info("Skipping BundleDeployment creation because doNotDeploy is set to true.",
 					"bundle", bundle.Name,
 					"bundleNamespace", bundle.Namespace,
 					"cluster", cluster.Name,
 					"clusterNamespace", cluster.Namespace,
-					"reason", "doNotDeploy",
+					"reason", "doNotDeploy on GitRepo target",
 				)
 				continue
 			}
 			// check if there is any matching targetCustomization that should be applied
 			targetOpts := target.BundleDeploymentOptions
 			if bundle.Spec.TargetCustomizationMode == fleet.TargetCustomizationModeAllMatches {
-				for _, tc := range bm.MatchAllTargetCustomizations(cluster.Name, clusterGroupsAsLabelMap, cluster.Labels) {
+				// AllMatches mode: merge all matching customizations
+				// Check if any matching customization has doNotDeploy=true (OR logic)
+				matchedCustomizations := bm.MatchAllTargetCustomizations(cluster.Name, clusterGroupsAsLabelMap, cluster.Labels)
+				for _, tc := range matchedCustomizations {
+					if tc.DoNotDeploy {
+						logger.V(1).Info("Skipping BundleDeployment creation because doNotDeploy is set to true.",
+							"bundle", bundle.Name,
+							"bundleNamespace", bundle.Namespace,
+							"cluster", cluster.Name,
+							"clusterNamespace", cluster.Namespace,
+							"reason", "doNotDeploy on targetCustomization (AllMatches mode)",
+						)
+						continue clusterLoop
+					}
 					targetOpts = options.Merge(targetOpts, tc.BundleDeploymentOptions)
 				}
 			} else {
+				// FirstMatch mode: apply only the first matching customization
 				if targetCustomized := bm.MatchTargetCustomizations(cluster.Name, clusterGroupsAsLabelMap, cluster.Labels); targetCustomized != nil {
+					// Check if the first matching targetCustomization has doNotDeploy set
+					if targetCustomized.DoNotDeploy {
+						logger.V(1).Info("Skipping BundleDeployment creation because doNotDeploy is set to true.",
+							"bundle", bundle.Name,
+							"bundleNamespace", bundle.Namespace,
+							"cluster", cluster.Name,
+							"clusterNamespace", cluster.Namespace,
+							"reason", "doNotDeploy on targetCustomization (FirstMatch mode)",
+						)
+						continue
+					}
 					targetOpts = targetCustomized.BundleDeploymentOptions
 				}
 			}

internal/cmd/controller/target/matcher/bundlematch.go

@@ -75,30 +75,6 @@ func (a *BundleMatch) MatchAllTargetCustomizations(clusterName string, clusterGr
 	return result
 }
 
-// HasDoNotDeployTarget returns true if any bundle target matching the given cluster
-// has DoNotDeploy set to true. Unlike MatchTargetCustomizations, this scans all matching
-// bundle targets instead of stopping at the first match, so a doNotDeploy entry does not have to
-// appear before a broader-matching entry in the target list.
-func (a *BundleMatch) HasDoNotDeployTarget(clusterName string, clusterGroups map[string]map[string]string, clusterLabels map[string]string) bool {
-	for _, tm := range a.matcher.matches {
-		if !tm.bundleTarget.DoNotDeploy {
-			continue
-		}
-		if len(clusterGroups) == 0 {
-			if criteriaWithoutRestrictions(tm, clusterName, "", nil, clusterLabels) {
-				return true
-			}
-		} else {
-			for cg, cgLabels := range clusterGroups {
-				if criteriaWithoutRestrictions(tm, clusterName, cg, cgLabels, clusterLabels) {
-					return true
-				}
-			}
-		}
-	}
-	return false
-}
-
 type targetMatch struct {
 	bundleTarget    *fleet.BundleTarget
 	criteria        *ClusterMatcher
@@ -190,13 +166,6 @@ func (m *matcher) criteriaWithRestrictions(targetMatch targetMatch, clusterName,
 	return false
 }
 
-// criteriaWithoutRestrictions checks targetMatch's criteria for a match on the specified cluster
-// name, group and labels, without checking if target is inside the targetRestrictions.
-// This is used for TargetCustomizations.
-func criteriaWithoutRestrictions(targetMatch targetMatch, clusterName, clusterGroup string, clusterGroupLabels, clusterLabels map[string]string) bool {
-	return targetMatch.criteria.Match(clusterName, clusterGroup, clusterGroupLabels, clusterLabels)
-}
-
 // match returns the first BundleTarget, from the matcher's target matches, which matches the specified cluster name, groups and labels, using matching logic implemented via findCriteriaMatch.
 func (m *matcher) match(clusterName string, clusterLabels map[string]string, clusterGroups map[string]map[string]string, findCriteriaMatch findCriteriaMatch) *fleet.BundleTarget {
 	for _, targetMatch := range m.matches {