Skip to content

Commit a3bf9b6

Browse files
committed
fix(webhook): use Status.Hard when validating ResourceQuota limits
Signed-off-by: saddamr3e <saddamr3e@gmail.com>
1 parent 6074b58 commit a3bf9b6

2 files changed

Lines changed: 72 additions & 1 deletion

File tree

internal/webhook/resourcequota.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ func validateResourceQuota(resourceList corev1.ResourceList, resourceQuota corev
251251
continue
252252
}
253253
quantity.Add(resourceQuota.Status.Used[key])
254-
if quantity.Cmp(resourceQuota.Spec.Hard[key]) > 0 {
254+
if quantity.Cmp(resourceQuota.Status.Hard[key]) > 0 {
255255
return false
256256
}
257257
}

internal/webhook/resourcequota_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ package webhook
1818

1919
import (
2020
"testing"
21+
22+
corev1 "k8s.io/api/core/v1"
23+
"k8s.io/apimachinery/pkg/api/resource"
24+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2125
)
2226

2327
func assertMemory(memoryString string, expectedBytes int64, t *testing.T) {
@@ -39,3 +43,70 @@ func TestJavaMemoryString(t *testing.T) {
3943
assertMemory("10TB", 10*1024*1024*1024*1024, t)
4044
assertMemory("10PB", 10*1024*1024*1024*1024*1024, t)
4145
}
46+
47+
// TestValidateResourceQuota_SpecHardDivergence is a regression test that
48+
// verifies validateResourceQuota enforces the limit from Status.Hard (the
49+
// authoritative, controller-reconciled value) rather than Spec.Hard (the
50+
// desired limit written by the administrator).
51+
//
52+
// Kubernetes' own ResourceQuota admission controller always checks
53+
// Status.Used + request ≤ Status.Hard. The spark-operator webhook must mirror
54+
// that behaviour, because Status.Hard is what the quota controller actually
55+
// enforces and tracks Status.Used against.
56+
//
57+
// In steady state Status.Hard == Spec.Hard, so the bug is normally invisible.
58+
// It surfaces when the two fields diverge, for example immediately after a
59+
// quota spec update before the quota controller has reconciled the status.
60+
func TestValidateResourceQuota_SpecHardDivergence(t *testing.T) {
61+
// Construct a ResourceQuota where Spec.Hard ≠ Status.Hard.
62+
//
63+
// Scenario: an administrator recently raised the CPU limit in Spec to 100,
64+
// but the quota controller has not yet reconciled Status.Hard, which still
65+
// reflects the old enforced limit of 4.
66+
//
67+
// Status.Hard[cpu] = 4 ← what Kubernetes actually enforces
68+
// Status.Used[cpu] = 0
69+
// Spec.Hard[cpu] = 100 ← desired new limit, not yet enforced
70+
//
71+
// A request for 5 CPU cores must be REJECTED, because 0 + 5 > Status.Hard(4).
72+
// The buggy implementation compares against Spec.Hard(100) and would ALLOW it.
73+
quota := corev1.ResourceQuota{
74+
ObjectMeta: metav1.ObjectMeta{
75+
Name: "diverged-quota",
76+
Namespace: "default",
77+
},
78+
Spec: corev1.ResourceQuotaSpec{
79+
Hard: corev1.ResourceList{
80+
// Spec says 100 CPUs — not yet enforced.
81+
corev1.ResourceCPU: resource.MustParse("100"),
82+
},
83+
},
84+
Status: corev1.ResourceQuotaStatus{
85+
Hard: corev1.ResourceList{
86+
// Status says 4 CPUs — what the quota controller actually enforces.
87+
corev1.ResourceCPU: resource.MustParse("4"),
88+
},
89+
Used: corev1.ResourceList{
90+
corev1.ResourceCPU: resource.MustParse("0"),
91+
},
92+
},
93+
}
94+
95+
// Request 5 CPUs: exceeds Status.Hard(4) but within Spec.Hard(100).
96+
request := corev1.ResourceList{
97+
corev1.ResourceCPU: resource.MustParse("5"),
98+
}
99+
100+
// validateResourceQuota must return false (reject), because the request
101+
// exceeds the enforced Status.Hard limit.
102+
//
103+
// On the BUGGY upstream code this returns true (allow), because the
104+
// comparison is done against Spec.Hard(100) instead of Status.Hard(4).
105+
if got := validateResourceQuota(request, quota); got != false {
106+
t.Errorf(
107+
"validateResourceQuota incorrectly returned %v (allow) for a request that exceeds the status hard limit",
108+
got,
109+
)
110+
}
111+
}
112+

0 commit comments

Comments
 (0)