Skip to content

Commit f081cd6

Browse files
committed
BUG/MINOR: fix ReferenceGrantsTo leak and add unit tests for ReferenceGrantManager
RemoveReferenceGrantWithCheck was not deleting the To key from tos in the normal deletion path (non-nil, non-empty ToReferenceGrantFrom entry), so len(tos) never reached 0 and the grant key was never removed from ReferenceGrantsTo. IsAccessGranted returned the correct result regardless because ToReferenceGrantFrom was cleaned up, but the inverse index leaked memory and would accumulate stale entries across update cycles. Unit tests cover: wildcard vs named To lookup, update wiping the previous footprint (the RemoveReferenceGrantWithCheck(_, false) path), delete with multiple Tos leaving no entries in either index, same-namespace always granted, and two grants sharing a To with independent Froms.
1 parent e917e8d commit f081cd6

2 files changed

Lines changed: 246 additions & 0 deletions

File tree

k8s/gate/tree/reference_grant_manager.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ func (mgr *ReferenceGrantManager) RemoveReferenceGrantWithCheck(referenceGrant R
195195
if len(referenceGrantFrom) == 0 {
196196
delete(mgr.ToReferenceGrantFrom, to)
197197
}
198+
delete(tos, to)
198199
}
199200
if len(tos) == 0 {
200201
delete(mgr.ReferenceGrantsTo, referenceGrantNamespacedName)
Lines changed: 245 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,245 @@
1+
// Copyright 2025 HAProxy Technologies LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package tree
15+
16+
import (
17+
"testing"
18+
19+
"github.com/haproxytech/haproxy-unified-gateway/k8s/gate/store"
20+
"github.com/stretchr/testify/assert"
21+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
22+
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
23+
gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
24+
)
25+
26+
// upserted builds a tree ReferenceGrant in StatusUpserted state.
27+
func upserted(k8s *gatewayv1beta1.ReferenceGrant) ReferenceGrant {
28+
return ReferenceGrant{
29+
K8sResource: k8s,
30+
TreeStatus: TreeUpdate[ReferenceGrant]{Status: store.StatusUpserted},
31+
}
32+
}
33+
34+
// deleted builds a tree ReferenceGrant in StatusDeleted state, with the
35+
// pre-deletion snapshot in OldTreeResource (as SetAsDeleted would produce).
36+
func deleted(k8s *gatewayv1beta1.ReferenceGrant) ReferenceGrant {
37+
return ReferenceGrant{
38+
K8sResource: nil,
39+
TreeStatus: TreeUpdate[ReferenceGrant]{
40+
Status: store.StatusDeleted,
41+
OldTreeResource: &ReferenceGrant{K8sResource: k8s},
42+
},
43+
}
44+
}
45+
46+
// TestReferenceGrantManager_WildcardTo verifies that a grant with an empty To.Name
47+
// (wildcard) allows access for any resource name in the target namespace.
48+
func TestReferenceGrantManager_WildcardTo(t *testing.T) {
49+
mgr := NewReferenceGrantManager()
50+
51+
k8s := &gatewayv1beta1.ReferenceGrant{
52+
ObjectMeta: metav1.ObjectMeta{Namespace: "backend-ns", Name: "grant-wildcard"},
53+
Spec: gatewayv1beta1.ReferenceGrantSpec{
54+
From: []gatewayv1beta1.ReferenceGrantFrom{{
55+
Group: gatewayv1.GroupName,
56+
Kind: "HTTPRoute",
57+
Namespace: "route-ns",
58+
}},
59+
To: []gatewayv1beta1.ReferenceGrantTo{{
60+
Group: "",
61+
Kind: "Service",
62+
Name: nil, // wildcard — any Service name
63+
}},
64+
},
65+
}
66+
67+
mgr.UpsertReferenceGrant(upserted(k8s))
68+
mgr.ComputeToFrom()
69+
70+
assert.True(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "route-ns", "", "Service", "backend-ns", "svc-a"),
71+
"wildcard grant should permit any named service")
72+
assert.True(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "route-ns", "", "Service", "backend-ns", "svc-b"),
73+
"wildcard grant should permit a different service name")
74+
assert.False(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "other-ns", "", "Service", "backend-ns", "svc-a"),
75+
"grant should not cover a route from a different namespace")
76+
}
77+
78+
// TestReferenceGrantManager_NamedTo verifies that a grant with a specific To.Name
79+
// allows only the named resource and not other names.
80+
func TestReferenceGrantManager_NamedTo(t *testing.T) {
81+
mgr := NewReferenceGrantManager()
82+
83+
k8s := &gatewayv1beta1.ReferenceGrant{
84+
ObjectMeta: metav1.ObjectMeta{Namespace: "backend-ns", Name: "grant-named"},
85+
Spec: gatewayv1beta1.ReferenceGrantSpec{
86+
From: []gatewayv1beta1.ReferenceGrantFrom{{
87+
Group: gatewayv1.GroupName,
88+
Kind: "HTTPRoute",
89+
Namespace: "route-ns",
90+
}},
91+
To: []gatewayv1beta1.ReferenceGrantTo{{
92+
Group: "",
93+
Kind: "Service",
94+
Name: new((gatewayv1.ObjectName)("my-svc")),
95+
}},
96+
},
97+
}
98+
99+
mgr.UpsertReferenceGrant(upserted(k8s))
100+
mgr.ComputeToFrom()
101+
102+
assert.True(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "route-ns", "", "Service", "backend-ns", "my-svc"),
103+
"named grant should permit the exact service")
104+
assert.False(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "route-ns", "", "Service", "backend-ns", "other-svc"),
105+
"named grant must not permit a different service name")
106+
}
107+
108+
// TestReferenceGrantManager_UpdateWipesPreviousFootprint verifies that upserting a
109+
// modified grant clears the old From entries before inserting the new ones.
110+
// This exercises the RemoveReferenceGrantWithCheck(_, false) call inside Upsert.
111+
func TestReferenceGrantManager_UpdateWipesPreviousFootprint(t *testing.T) {
112+
mgr := NewReferenceGrantManager()
113+
114+
k8sV1 := &gatewayv1beta1.ReferenceGrant{
115+
ObjectMeta: metav1.ObjectMeta{Namespace: "backend-ns", Name: "grant-update"},
116+
Spec: gatewayv1beta1.ReferenceGrantSpec{
117+
From: []gatewayv1beta1.ReferenceGrantFrom{{
118+
Group: gatewayv1.GroupName,
119+
Kind: "HTTPRoute",
120+
Namespace: "route-ns-old",
121+
}},
122+
To: []gatewayv1beta1.ReferenceGrantTo{{
123+
Group: "",
124+
Kind: "Service",
125+
Name: new((gatewayv1.ObjectName)("svc")),
126+
}},
127+
},
128+
}
129+
130+
mgr.UpsertReferenceGrant(upserted(k8sV1))
131+
mgr.ComputeToFrom()
132+
assert.True(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "route-ns-old", "", "Service", "backend-ns", "svc"))
133+
134+
// Update the grant: only route-ns-new is now authorised.
135+
k8sV2 := &gatewayv1beta1.ReferenceGrant{
136+
ObjectMeta: metav1.ObjectMeta{Namespace: "backend-ns", Name: "grant-update"},
137+
Spec: gatewayv1beta1.ReferenceGrantSpec{
138+
From: []gatewayv1beta1.ReferenceGrantFrom{{
139+
Group: gatewayv1.GroupName,
140+
Kind: "HTTPRoute",
141+
Namespace: "route-ns-new",
142+
}},
143+
To: []gatewayv1beta1.ReferenceGrantTo{{
144+
Group: "",
145+
Kind: "Service",
146+
Name: new((gatewayv1.ObjectName)("svc")),
147+
}},
148+
},
149+
}
150+
151+
mgr.UpsertReferenceGrant(upserted(k8sV2))
152+
mgr.ComputeToFrom()
153+
154+
assert.False(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "route-ns-old", "", "Service", "backend-ns", "svc"),
155+
"old From must be revoked after update")
156+
assert.True(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "route-ns-new", "", "Service", "backend-ns", "svc"),
157+
"new From must be allowed after update")
158+
}
159+
160+
// TestReferenceGrantManager_DeleteMultiToNoLeftovers verifies that deleting a grant
161+
// that covers multiple To entries leaves no stale entries in either ToReferenceGrantFrom
162+
// or ReferenceGrantsTo.
163+
func TestReferenceGrantManager_DeleteMultiToNoLeftovers(t *testing.T) {
164+
mgr := NewReferenceGrantManager()
165+
166+
k8s := &gatewayv1beta1.ReferenceGrant{
167+
ObjectMeta: metav1.ObjectMeta{Namespace: "backend-ns", Name: "grant-multi"},
168+
Spec: gatewayv1beta1.ReferenceGrantSpec{
169+
From: []gatewayv1beta1.ReferenceGrantFrom{{
170+
Group: gatewayv1.GroupName,
171+
Kind: "HTTPRoute",
172+
Namespace: "route-ns",
173+
}},
174+
To: []gatewayv1beta1.ReferenceGrantTo{
175+
{Group: "", Kind: "Service", Name: new((gatewayv1.ObjectName)("svc-a"))},
176+
{Group: "", Kind: "Service", Name: new((gatewayv1.ObjectName)("svc-b"))},
177+
},
178+
},
179+
}
180+
181+
mgr.UpsertReferenceGrant(upserted(k8s))
182+
mgr.ComputeToFrom()
183+
184+
mgr.RemoveReferenceGrant(deleted(k8s))
185+
mgr.ComputeToFrom()
186+
187+
assert.Empty(t, mgr.ToReferenceGrantFrom, "ToReferenceGrantFrom must be empty after delete")
188+
assert.Empty(t, mgr.ReferenceGrantsTo, "ReferenceGrantsTo must be empty after delete")
189+
assert.False(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "route-ns", "", "Service", "backend-ns", "svc-a"))
190+
assert.False(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "route-ns", "", "Service", "backend-ns", "svc-b"))
191+
}
192+
193+
// TestReferenceGrantManager_SameNamespaceAlwaysGranted verifies that same-namespace
194+
// references are always allowed regardless of whether any grant exists.
195+
func TestReferenceGrantManager_SameNamespaceAlwaysGranted(t *testing.T) {
196+
mgr := NewReferenceGrantManager()
197+
198+
assert.True(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "ns", "", "Service", "ns", "svc"),
199+
"same-namespace access must be permitted with no grants at all")
200+
}
201+
202+
// TestReferenceGrantManager_TwoGrantsSameTo_DeleteOneKeepsOther verifies that when two
203+
// grants cover the same To with different Froms, deleting one does not revoke the other.
204+
func TestReferenceGrantManager_TwoGrantsSameTo_DeleteOneKeepsOther(t *testing.T) {
205+
mgr := NewReferenceGrantManager()
206+
207+
k8sA := &gatewayv1beta1.ReferenceGrant{
208+
ObjectMeta: metav1.ObjectMeta{Namespace: "backend-ns", Name: "grant-a"},
209+
Spec: gatewayv1beta1.ReferenceGrantSpec{
210+
From: []gatewayv1beta1.ReferenceGrantFrom{{
211+
Group: gatewayv1.GroupName,
212+
Kind: "HTTPRoute",
213+
Namespace: "route-ns-a",
214+
}},
215+
To: []gatewayv1beta1.ReferenceGrantTo{{Group: "", Kind: "Service", Name: new((gatewayv1.ObjectName)("shared-svc"))}},
216+
},
217+
}
218+
k8sB := &gatewayv1beta1.ReferenceGrant{
219+
ObjectMeta: metav1.ObjectMeta{Namespace: "backend-ns", Name: "grant-b"},
220+
Spec: gatewayv1beta1.ReferenceGrantSpec{
221+
From: []gatewayv1beta1.ReferenceGrantFrom{{
222+
Group: gatewayv1.GroupName,
223+
Kind: "HTTPRoute",
224+
Namespace: "route-ns-b",
225+
}},
226+
To: []gatewayv1beta1.ReferenceGrantTo{{Group: "", Kind: "Service", Name: new((gatewayv1.ObjectName)("shared-svc"))}},
227+
},
228+
}
229+
230+
mgr.UpsertReferenceGrant(upserted(k8sA))
231+
mgr.UpsertReferenceGrant(upserted(k8sB))
232+
mgr.ComputeToFrom()
233+
234+
assert.True(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "route-ns-a", "", "Service", "backend-ns", "shared-svc"))
235+
assert.True(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "route-ns-b", "", "Service", "backend-ns", "shared-svc"))
236+
237+
// Delete grant-a only.
238+
mgr.RemoveReferenceGrant(deleted(k8sA))
239+
mgr.ComputeToFrom()
240+
241+
assert.False(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "route-ns-a", "", "Service", "backend-ns", "shared-svc"),
242+
"route-ns-a must lose access after its grant is deleted")
243+
assert.True(t, mgr.IsAccessGranted(gatewayv1.GroupName, "HTTPRoute", "route-ns-b", "", "Service", "backend-ns", "shared-svc"),
244+
"route-ns-b must retain access via its own grant")
245+
}

0 commit comments

Comments
 (0)