Skip to content

Commit f36ce9e

Browse files
dockerized-nljesse.mirza@nslogin.nlclaudeCopilot
authored
Fix doNotDeploy bypassed when broader target precedes it in the target list (#4688)
`MatchTargetCustomizations` returns on the first matching target. If a broader-matching customization (e.g. `clusterGroup: all`) is listed before a `doNotDeploy: true` customization (e.g. `clusterGroup: one`), the doNotDeploy entry is never evaluated for clusters that match the broader entry first. Additionally, `target.DoNotDeploy` (from the main `Match()` call) was not being checked when no targetCustomization was found. Add `HasDoNotDeployTarget()` to `BundleMatch`, which scans all targets—not just the first match—for a `doNotDeploy` flag. Update `Targets()` in `builder.go` to consult this method before building a `Target`, and separately honour `target.DoNotDeploy` from the GitRepo match. Add an integration test covering the first-match bypass scenario. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: jesse.mirza@nslogin.nl <jesse.mirza@ns.nl> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 81cee5c commit f36ce9e

File tree

3 files changed

+85
-4
lines changed

3 files changed

+85
-4
lines changed

integrationtests/controller/bundle/bundle_targets_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,53 @@ var _ = Describe("Bundle targets", Ordered, func() {
459459
})
460460
})
461461

462+
// Regression test for https://github.com/rancher/fleet/issues/3580:
463+
// When a broader-matching target appears before a doNotDeploy target in the list,
464+
// the doNotDeploy target was previously bypassed due to first-match semantics.
465+
When("a broader-matching targetCustomization appears before a doNotDeploy targetCustomization", func() {
466+
BeforeEach(func() {
467+
bundleName = "donot-deploy-after-broader-match"
468+
bdLabels = map[string]string{
469+
"fleet.cattle.io/bundle-name": bundleName,
470+
"fleet.cattle.io/bundle-namespace": namespace,
471+
}
472+
expectedNumberOfBundleDeployments = 0
473+
// simulate targets in fleet.yaml:
474+
// - first entry matches all clusters (broader match)
475+
// - second entry matches cluster "one" with doNotDeploy=true
476+
// With the old first-match logic, cluster "one" would match the first entry and
477+
// the doNotDeploy entry would never be evaluated.
478+
targets = []v1alpha1.BundleTarget{
479+
{
480+
BundleDeploymentOptions: v1alpha1.BundleDeploymentOptions{
481+
Helm: &v1alpha1.HelmOptions{
482+
Values: &v1alpha1.GenericMap{Data: map[string]interface{}{"replicas": "1"}},
483+
},
484+
},
485+
ClusterGroup: "all",
486+
},
487+
{
488+
ClusterGroup: "one",
489+
DoNotDeploy: true,
490+
},
491+
}
492+
// simulate targets in GitRepo: only cluster group "one" is targeted
493+
targetsInGitRepo := []v1alpha1.BundleTarget{
494+
{
495+
ClusterGroup: "one",
496+
},
497+
}
498+
targetRestrictions = make([]v1alpha1.BundleTarget, len(targetsInGitRepo))
499+
copy(targetRestrictions, targetsInGitRepo)
500+
targets = append(targets, targetsInGitRepo...)
501+
})
502+
503+
It("no BundleDeployments are created", func() {
504+
waitForBundleToBeReady(bundleName)
505+
_ = verifyBundlesDeploymentsAreCreated(expectedNumberOfBundleDeployments, bdLabels, bundleName)
506+
})
507+
})
508+
462509
When("setting doNotDeploy to a target customization after the bundle has been deployed", func() {
463510
BeforeEach(func() {
464511
bundleName = "one-customized-do-not-deploy-two-later"

internal/cmd/controller/target/builder.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,24 @@ func (m *Manager) Targets(ctx context.Context, bundle *fleet.Bundle, manifestID
8383
if target == nil {
8484
continue
8585
}
86+
// Check all matching targetCustomizations for doNotDeploy, not just the first match.
87+
// This ensures that a doNotDeploy entry is honoured even when a broader-matching
88+
// target appears before it in the target list (fixes first-match bypass).
89+
doNotDeploy := target.DoNotDeploy || bm.HasDoNotDeployTarget(cluster.Name, ClusterGroupsToLabelMap(clusterGroups), cluster.Labels)
90+
if doNotDeploy {
91+
logger.V(1).Info("Skipping BundleDeployment creation because doNotDeploy is set to true.",
92+
"bundle", bundle.Name,
93+
"bundleNamespace", bundle.Namespace,
94+
"cluster", cluster.Name,
95+
"clusterNamespace", cluster.Namespace,
96+
"reason", "doNotDeploy",
97+
)
98+
continue
99+
}
86100
// check if there is any matching targetCustomization that should be applied
87101
targetOpts := target.BundleDeploymentOptions
88102
targetCustomized := bm.MatchTargetCustomizations(cluster.Name, ClusterGroupsToLabelMap(clusterGroups), cluster.Labels)
89103
if targetCustomized != nil {
90-
if targetCustomized.DoNotDeploy {
91-
logger.V(1).Info("BundleDeployment creation for Bundle was skipped because doNotDeploy is set to true.")
92-
continue
93-
}
94104
targetOpts = targetCustomized.BundleDeploymentOptions
95105
}
96106

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,30 @@ func (a *BundleMatch) MatchTargetCustomizations(clusterName string, clusterGroup
5151
return nil
5252
}
5353

54+
// HasDoNotDeployTarget returns true if any bundle target matching the given cluster
55+
// has DoNotDeploy set to true. Unlike MatchTargetCustomizations, this scans all matching
56+
// bundle targets instead of stopping at the first match, so a doNotDeploy entry does not have to
57+
// appear before a broader matching entry in the target list.
58+
func (a *BundleMatch) HasDoNotDeployTarget(clusterName string, clusterGroups map[string]map[string]string, clusterLabels map[string]string) bool {
59+
for _, tm := range a.matcher.matches {
60+
if !tm.bundleTarget.DoNotDeploy {
61+
continue
62+
}
63+
if len(clusterGroups) == 0 {
64+
if criteriaWithoutRestrictions(tm, clusterName, "", nil, clusterLabels) {
65+
return true
66+
}
67+
} else {
68+
for cg, cgLabels := range clusterGroups {
69+
if criteriaWithoutRestrictions(tm, clusterName, cg, cgLabels, clusterLabels) {
70+
return true
71+
}
72+
}
73+
}
74+
}
75+
return false
76+
}
77+
5478
type targetMatch struct {
5579
bundleTarget *fleet.BundleTarget
5680
criteria *ClusterMatcher

0 commit comments

Comments
 (0)