Skip to content

Commit a094580

Browse files
committed
fix: use hybrid approach to distinguish customizations from GitRepo targets
Fixes bug where targetCustomizations with identical selectors to GitRepo targets were misclassified as GitRepo targets and skipped. The hybrid approach uses an explicit Source field ("customization" or "gitrepo") for new bundles, with position-based detection as fallback for backward compatibility. This fixes the collision bug immediately for all bundles without requiring recreation. Changes: - Add optional Source field to BundleTarget with enum validation - Update bundlereader to populate Source field for all target types - Replace hasMatchingRestriction() with determineIsCustomization() - Add comprehensive tests for hybrid logic and collision cases
1 parent b601ed3 commit a094580

5 files changed

Lines changed: 256 additions & 87 deletions

File tree

charts/fleet-crd/templates/crds.yaml

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3211,6 +3211,34 @@ spec:
32113211
this deployment.
32123212
nullable: true
32133213
type: string
3214+
source:
3215+
description: 'Source indicates the origin of this target.
3216+
3217+
"customization" - target comes from fleet.yaml targetCustomizations
3218+
3219+
"gitrepo" - target comes from GitRepo.Spec.Targets (via
3220+
targets file)
3221+
3222+
3223+
If empty, provenance is determined by position in the Targets
3224+
array:
3225+
3226+
- First N targets are customizations where N = len(Targets)
3227+
- len(TargetRestrictions)
3228+
3229+
- Remaining targets are GitRepo targets
3230+
3231+
3232+
This field enables explicit provenance tracking for better
3233+
maintainability
3234+
3235+
while maintaining backward compatibility with Bundles created
3236+
before this field existed.'
3237+
enum:
3238+
- customization
3239+
- gitrepo
3240+
- ''
3241+
type: string
32143242
yaml:
32153243
description: 'YAML options, if using raw YAML these are names
32163244
that map to
@@ -9471,6 +9499,34 @@ spec:
94719499
this deployment.
94729500
nullable: true
94739501
type: string
9502+
source:
9503+
description: 'Source indicates the origin of this target.
9504+
9505+
"customization" - target comes from fleet.yaml targetCustomizations
9506+
9507+
"gitrepo" - target comes from GitRepo.Spec.Targets (via
9508+
targets file)
9509+
9510+
9511+
If empty, provenance is determined by position in the Targets
9512+
array:
9513+
9514+
- First N targets are customizations where N = len(Targets)
9515+
- len(TargetRestrictions)
9516+
9517+
- Remaining targets are GitRepo targets
9518+
9519+
9520+
This field enables explicit provenance tracking for better
9521+
maintainability
9522+
9523+
while maintaining backward compatibility with Bundles created
9524+
before this field existed.'
9525+
enum:
9526+
- customization
9527+
- gitrepo
9528+
- ''
9529+
type: string
94749530
yaml:
94759531
description: 'YAML options, if using raw YAML these are names
94769532
that map to

internal/bundlereader/read.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,11 @@ func bundleFromDir(ctx context.Context, name, baseDir string, bundleData []byte,
184184
})
185185
}
186186

187-
fy.Targets = append(fy.Targets, fy.TargetCustomizations...)
187+
// Mark targetCustomizations with explicit source
188+
for _, tc := range fy.TargetCustomizations {
189+
tc.Source = "customization"
190+
fy.Targets = append(fy.Targets, tc)
191+
}
188192
fy.BundleSpec.TargetCustomizationMode = fy.TargetCustomizationMode
189193

190194
meta, err := readMetadata(bundleData)
@@ -245,6 +249,7 @@ func bundleFromDir(ctx context.Context, name, baseDir string, bundleData []byte,
245249
ClusterSelector: target.ClusterSelector,
246250
ClusterGroup: target.ClusterGroup,
247251
ClusterGroupSelector: target.ClusterGroupSelector,
252+
Source: "gitrepo", // OverrideTargets replace GitRepo targets
248253
})
249254
bundle.Spec.TargetRestrictions = append(bundle.Spec.TargetRestrictions, fleet.BundleTargetRestriction(target))
250255
}
@@ -329,7 +334,11 @@ func appendTargets(def *fleet.Bundle, targetsFile string) (*fleet.Bundle, error)
329334
return nil, err
330335
}
331336

332-
def.Spec.Targets = append(def.Spec.Targets, spec.Targets...)
337+
// Mark GitRepo targets with explicit source
338+
for _, target := range spec.Targets {
339+
target.Source = "gitrepo"
340+
def.Spec.Targets = append(def.Spec.Targets, target)
341+
}
333342
def.Spec.TargetRestrictions = append(def.Spec.TargetRestrictions, spec.TargetRestrictions...)
334343

335344
return def, nil

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

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package matcher
22

33
import (
4-
"reflect"
5-
64
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
75
)
86

@@ -115,6 +113,11 @@ type matcher struct {
115113
func (a *BundleMatch) initMatcher() error {
116114
m := &matcher{}
117115

116+
// Calculate position-based split point for fallback
117+
// The first N targets are customizations, where N = total - restrictions
118+
numRestrictions := len(a.bundle.Spec.TargetRestrictions)
119+
numCustomizations := len(a.bundle.Spec.Targets) - numRestrictions
120+
118121
for i, target := range a.bundle.Spec.Targets {
119122
clusterMatcher, err := NewClusterMatcher(target.ClusterName, target.ClusterGroup, target.ClusterGroupSelector, target.ClusterSelector)
120123
if err != nil {
@@ -123,7 +126,7 @@ func (a *BundleMatch) initMatcher() error {
123126
t := targetMatch{
124127
bundleTarget: &a.bundle.Spec.Targets[i],
125128
criteria: clusterMatcher,
126-
isCustomization: !hasMatchingRestriction(target, a.bundle.Spec.TargetRestrictions),
129+
isCustomization: determineIsCustomization(target, i, numCustomizations),
127130
}
128131

129132
m.matches = append(m.matches, t)
@@ -141,20 +144,24 @@ func (a *BundleMatch) initMatcher() error {
141144
return nil
142145
}
143146

144-
// hasMatchingRestriction returns true if the target has an equivalent entry in the
145-
// TargetRestrictions list. GitRepo targets always have one (bundlereader adds them
146-
// in parallel); fleet.yaml targetCustomizations never do.
147-
func hasMatchingRestriction(target fleet.BundleTarget, restrictions []fleet.BundleTargetRestriction) bool {
148-
for _, r := range restrictions {
149-
if r.Name == target.Name &&
150-
r.ClusterName == target.ClusterName &&
151-
r.ClusterGroup == target.ClusterGroup &&
152-
reflect.DeepEqual(r.ClusterSelector, target.ClusterSelector) &&
153-
reflect.DeepEqual(r.ClusterGroupSelector, target.ClusterGroupSelector) {
154-
return true
155-
}
147+
// determineIsCustomization uses explicit Source field if present,
148+
// falls back to position-based detection for backward compatibility.
149+
func determineIsCustomization(target fleet.BundleTarget, index int, numCustomizations int) bool {
150+
// NEW BUNDLES: Source field is populated by bundlereader
151+
// This is the preferred method for long-term maintainability
152+
if target.Source != "" {
153+
return target.Source == "customization"
156154
}
157-
return false
155+
156+
// OLD BUNDLES: Source field is empty (created before this field existed)
157+
// Use position-based detection as fallback:
158+
// - bundlereader appends targetCustomizations first (read.go:187)
159+
// - Then appends GitRepo targets from targets file (read.go:332)
160+
// - Therefore: first N targets are customizations
161+
// where N = len(Targets) - len(TargetRestrictions)
162+
//
163+
// This fixes the collision bug for old Bundles without requiring recreation
164+
return index < numCustomizations
158165
}
159166

160167
func (m *matcher) isRestricted(clusterName, clusterGroup string, clusterGroupLabels, clusterLabels map[string]string) bool {

0 commit comments

Comments
 (0)