Skip to content

Commit 5aae821

Browse files
webhook: improve nodeSelectorsOverlap to detect subset overlaps
Previously, nodeSelectorsOverlap only detected identical selectors using string equality. This missed partial overlaps where one selector is a subset of another, allowing conflicting rules to be created. Fix by using sel.Matches() to check if one selector's matchLabels satisfies the other selector, catching subset overlap cases. Signed-off-by: Shreya2005-2005 <bhakatmistu2005@gmail.com>
1 parent 4f6e4bb commit 5aae821

2 files changed

Lines changed: 10 additions & 6 deletions

File tree

internal/webhook/nodereadinessgaterule_webhook.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222

2323
corev1 "k8s.io/api/core/v1"
2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
25+
"k8s.io/apimachinery/pkg/labels"
2526
"k8s.io/apimachinery/pkg/runtime"
2627
"k8s.io/apimachinery/pkg/util/validation/field"
2728
ctrl "sigs.k8s.io/controller-runtime"
@@ -122,9 +123,12 @@ func (w *NodeReadinessRuleWebhook) nodeSelectorsOverlap(selector1, selector2 met
122123
return true
123124
}
124125

125-
// Simple heuristic: if selectors are identical, they definitely overlap
126-
// For more complex overlap detection, we'd need to analyze the label requirements
127-
return sel1.String() == sel2.String()
126+
// Check overlap: if one selector's matchLabels is a subset of the other,
127+
// a node could match both selectors, causing a conflict.
128+
if sel1.Matches(labels.Set(selector2.MatchLabels)) {
129+
return true
130+
}
131+
return sel2.Matches(labels.Set(selector1.MatchLabels))
128132
}
129133

130134
// generateNoExecuteWarnings generates admission warnings for NoExecute taint usage.

internal/webhook/nodereadinessgaterule_webhook_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,18 +283,18 @@ var _ = Describe("NodeReadinessRule Validation Webhook", func() {
283283
Expect(overlaps).To(BeTrue()) // Both nil = both match all nodes
284284
})
285285

286-
It("should not overlap when one selector is nil", func() {
286+
It("should overlap when one selector is empty (matches all nodes)", func() {
287287
selector := metav1.LabelSelector{
288288
MatchLabels: map[string]string{
289289
"node-role.kubernetes.io/worker": "",
290290
},
291291
}
292292

293293
overlaps := webhook.nodeSelectorsOverlap(metav1.LabelSelector{}, selector)
294-
Expect(overlaps).To(BeFalse())
294+
Expect(overlaps).To(BeTrue())
295295

296296
overlaps = webhook.nodeSelectorsOverlap(selector, metav1.LabelSelector{})
297-
Expect(overlaps).To(BeFalse())
297+
Expect(overlaps).To(BeTrue())
298298
})
299299

300300
It("should detect identical selectors as overlapping", func() {

0 commit comments

Comments
 (0)